From 86336aea811526ecc12fcd6913646c54beb6ed16 Mon Sep 17 00:00:00 2001 From: rakeshkky <12475069+rakeshkky@users.noreply.github.com> Date: Wed, 8 Apr 2020 16:58:33 +0530 Subject: [PATCH 01/14] allow re-using Postgres scalars in custom types, close #4125 --- .../Hasura/GraphQL/Schema/CustomTypes.hs | 11 ++- server/src-lib/Hasura/RQL/DDL/CustomTypes.hs | 78 ++++++++++++------- server/src-lib/Hasura/RQL/DDL/Schema/Cache.hs | 12 ++- server/src-lib/Hasura/RQL/Types/Catalog.hs | 1 + server/src-rsr/catalog_metadata.sql | 9 ++- 5 files changed, 71 insertions(+), 40 deletions(-) diff --git a/server/src-lib/Hasura/GraphQL/Schema/CustomTypes.hs b/server/src-lib/Hasura/GraphQL/Schema/CustomTypes.hs index d5dae888e005d..39e4d03b5c330 100644 --- a/server/src-lib/Hasura/GraphQL/Schema/CustomTypes.hs +++ b/server/src-lib/Hasura/GraphQL/Schema/CustomTypes.hs @@ -31,7 +31,7 @@ buildObjectTypeInfo roleName annotatedObjectType = \(TypeRelationship name ty remoteTableInfo _) -> if isJust (getSelectPermissionInfoM remoteTableInfo roleName) || roleName == adminRole - then Just (relationshipToFieldInfo name ty $ _tciName $ _tiCoreInfo $ remoteTableInfo) + then Just (relationshipToFieldInfo name ty $ _tciName $ _tiCoreInfo remoteTableInfo) else Nothing where relationshipToFieldInfo name relTy remoteTableName = @@ -116,14 +116,13 @@ annotateObjectType tableCache nonObjectTypeMap objectDefinition = do buildCustomTypesSchemaPartial :: (QErrM m) - => TableCache -> CustomTypes -> m (NonObjectTypeMap, AnnotatedObjects) -buildCustomTypesSchemaPartial tableCache customTypes = do + => TableCache -> CustomTypes -> [PGScalarType] -> m (NonObjectTypeMap, AnnotatedObjects) +buildCustomTypesSchemaPartial tableCache customTypes pgScalars = do let typeInfos = map (VT.TIEnum . convertEnumDefinition) enumDefinitions <> - -- map (VT.TIObj . convertObjectDefinition) objectDefinitions <> map (VT.TIInpObj . convertInputObjectDefinition) inputObjectDefinitions <> - map (VT.TIScalar . convertScalarDefinition) scalarDefinitions - -- <> defaultTypes + map (VT.TIScalar . convertScalarDefinition) scalarDefinitions <> + map (VT.TIScalar . VT.mkHsraScalarTyInfo) pgScalars nonObjectTypeMap = NonObjectTypeMap $ mapFromL VT.getNamedTy typeInfos annotatedObjectTypes <- mapFromL (_otdName . _aotDefinition) <$> diff --git a/server/src-lib/Hasura/RQL/DDL/CustomTypes.hs b/server/src-lib/Hasura/RQL/DDL/CustomTypes.hs index aeca714b47787..79c1511d93f53 100644 --- a/server/src-lib/Hasura/RQL/DDL/CustomTypes.hs +++ b/server/src-lib/Hasura/RQL/DDL/CustomTypes.hs @@ -15,6 +15,7 @@ import qualified Database.PG.Query as Q import qualified Language.GraphQL.Draft.Syntax as G import Hasura.EncJSON +import Hasura.GraphQL.Validate.Types (mkScalarTy) import Hasura.Prelude import Hasura.RQL.Types import Hasura.SQL.Types @@ -23,12 +24,13 @@ import Hasura.GraphQL.Schema.CustomTypes (buildCustomTypesSchemaPartia validateCustomTypeDefinitions :: (MonadValidate [CustomTypeValidationError] m) - => TableCache -> CustomTypes -> m () -validateCustomTypeDefinitions tableCache customTypes = do + => TableCache -> CustomTypes -> [PGScalarType] -> m [PGScalarType] +validateCustomTypeDefinitions tableCache customTypes allPGScalars = do unless (null duplicateTypes) $ dispute $ pure $ DuplicateTypeNames duplicateTypes traverse_ validateEnum enumDefinitions - traverse_ validateInputObject inputObjectDefinitions - traverse_ validateObject objectDefinitions + inputObjectReusedPGScalars <- concat <$> traverse validateInputObject inputObjectDefinitions + objectReusedPGScalars <- concat <$> traverse validateObject objectDefinitions + pure $ inputObjectReusedPGScalars <> objectReusedPGScalars where inputObjectDefinitions = fromMaybe [] $ _ctInputObjects customTypes objectDefinitions = fromMaybe [] $ _ctObjects customTypes @@ -63,7 +65,7 @@ validateCustomTypeDefinitions tableCache customTypes = do validateInputObject :: (MonadValidate [CustomTypeValidationError] m) - => InputObjectTypeDefinition -> m () + => InputObjectTypeDefinition -> m [PGScalarType] validateInputObject inputObjectDefinition = do let inputObjectTypeName = _iotdName inputObjectDefinition duplicateFieldNames = @@ -83,16 +85,22 @@ validateCustomTypeDefinitions tableCache customTypes = do scalarTypes `Set.union` enumTypes `Set.union` inputObjectTypes -- check that fields reference input types - for_ (_iotdFields inputObjectDefinition) $ \inputObjectField -> do - let fieldBaseType = G.getBaseType $ unGraphQLType $ _iofdType inputObjectField - unless (Set.member fieldBaseType inputTypes) $ - refute $ pure $ InputObjectFieldTypeDoesNotExist - (_iotdName inputObjectDefinition) - (_iofdName inputObjectField) fieldBaseType + fmap (catMaybes . toList) $ + for (_iotdFields inputObjectDefinition) $ \inputObjectField -> do + let fieldBaseType = G.getBaseType $ unGraphQLType $ _iofdType inputObjectField + -- Check if field type uses any Postgres scalar type and return the same + case findReusedPGScalar fieldBaseType of + Just reusedScalar -> pure $ Just reusedScalar + Nothing -> do + unless (Set.member fieldBaseType inputTypes) $ + refute $ pure $ InputObjectFieldTypeDoesNotExist + (_iotdName inputObjectDefinition) + (_iofdName inputObjectField) fieldBaseType + pure Nothing validateObject :: (MonadValidate [CustomTypeValidationError] m) - => ObjectTypeDefinition -> m () + => ObjectTypeDefinition -> m [PGScalarType] validateObject objectDefinition = do let objectTypeName = _otdName objectDefinition fieldNames = map (unObjectFieldName . _ofdName) $ @@ -106,7 +114,8 @@ validateCustomTypeDefinitions tableCache customTypes = do unless (null duplicateFieldNames) $ dispute $ pure $ ObjectDuplicateFields objectTypeName duplicateFieldNames - scalarFields <- fmap (Map.fromList . catMaybes) $ + (scalarFields, reusedScalars) <- + fmap (((Map.fromList . catMaybes) *** catMaybes) . unzip) $ for fields $ \objectField -> do let fieldType = _ofdType objectField fieldBaseType = G.getBaseType $ unGraphQLType fieldType @@ -120,21 +129,27 @@ validateCustomTypeDefinitions tableCache customTypes = do let objectTypes = Set.fromList $ map (unObjectTypeName . _otdName) objectDefinitions - -- check that the fields only reference scalars and enums - -- and not other object types - if | Set.member fieldBaseType scalarTypes -> return () - | Set.member fieldBaseType enumTypes -> return () - | Set.member fieldBaseType objectTypes -> - dispute $ pure $ ObjectFieldObjectBaseType - objectTypeName fieldName fieldBaseType - | otherwise -> - dispute $ pure $ ObjectFieldTypeDoesNotExist - objectTypeName fieldName fieldBaseType + -- Check if field type uses any Postgres scalar type and return the same + maybeReusedPGScalar <- case findReusedPGScalar fieldBaseType of + Just ty -> pure $ Just ty + Nothing -> do + -- check that the fields only reference scalars and enums + -- and not other object types + if | Set.member fieldBaseType scalarTypes -> pure () + | Set.member fieldBaseType enumTypes -> pure () + | Set.member fieldBaseType objectTypes -> + dispute $ pure $ ObjectFieldObjectBaseType + objectTypeName fieldName fieldBaseType + | otherwise -> + dispute $ pure $ ObjectFieldTypeDoesNotExist + objectTypeName fieldName fieldBaseType + pure Nothing -- collect all non list scalar types of this object - if (not (isListType fieldType) && Set.member fieldBaseType scalarTypes) + maybeScalar <- if (not (isListType fieldType) && Set.member fieldBaseType scalarTypes) then pure $ Just (fieldName, fieldBaseType) else pure Nothing + pure (maybeScalar, maybeReusedPGScalar) for_ relationships $ \relationshipField -> do let relationshipName = _trName relationshipField @@ -160,6 +175,11 @@ validateCustomTypeDefinitions tableCache customTypes = do objectTypeName relationshipName remoteTable columnName return () + pure reusedScalars + + findReusedPGScalar baseType = + find ((==) baseType . mkScalarTy) allPGScalars + data CustomTypeValidationError = DuplicateTypeNames !(Set.HashSet G.NamedType) -- ^ type names have to be unique across all types @@ -265,11 +285,11 @@ clearCustomTypes = do resolveCustomTypes :: (MonadError QErr m) - => TableCache -> CustomTypes -> m (NonObjectTypeMap, AnnotatedObjects) -resolveCustomTypes tableCache customTypes = do - either (throw400 ConstraintViolation . showErrors) pure - =<< runValidateT (validateCustomTypeDefinitions tableCache customTypes) - buildCustomTypesSchemaPartial tableCache customTypes + => TableCache -> CustomTypes -> [PGScalarType] -> m (NonObjectTypeMap, AnnotatedObjects) +resolveCustomTypes tableCache customTypes allPGScalars = do + reusedPGScalars <- either (throw400 ConstraintViolation . showErrors) pure + =<< runValidateT (validateCustomTypeDefinitions tableCache customTypes allPGScalars) + buildCustomTypesSchemaPartial tableCache customTypes reusedPGScalars where showErrors :: [CustomTypeValidationError] -> T.Text showErrors allErrors = diff --git a/server/src-lib/Hasura/RQL/DDL/Schema/Cache.hs b/server/src-lib/Hasura/RQL/DDL/Schema/Cache.hs index 25329e7417535..fb22f02b88ac0 100644 --- a/server/src-lib/Hasura/RQL/DDL/Schema/Cache.hs +++ b/server/src-lib/Hasura/RQL/DDL/Schema/Cache.hs @@ -198,7 +198,7 @@ buildSchemaCacheRule = proc (catalogMetadata, invalidationKeys) -> do buildAndCollectInfo = proc (catalogMetadata, invalidationKeys) -> do let CatalogMetadata tables relationships permissions eventTriggers remoteSchemas functions allowlistDefs - computedFields customTypes actions = catalogMetadata + computedFields customTypes actions pgScalars = catalogMetadata -- tables tableRawInfos <- buildTableCache -< (tables, Inc.selectD #_ikMetadata invalidationKeys) @@ -256,7 +256,10 @@ buildSchemaCacheRule = proc (catalogMetadata, invalidationKeys) -> do & HS.fromList -- custom types - resolvedCustomTypes <- bindA -< resolveCustomTypes tableCache customTypes + maybeResolvedCustomTypes <- + (| withRecordInconsistency + (bindErrorA -< resolveCustomTypes tableCache customTypes pgScalars) + |) (MetadataObject MOCustomTypes $ toJSON customTypes) -- actions actionCache <- (mapFromL _amName actions >- returnA) @@ -267,6 +270,9 @@ buildSchemaCacheRule = proc (catalogMetadata, invalidationKeys) -> do addActionContext e = "in action " <> name <<> "; " <> e (| withRecordInconsistency ( (| modifyErrA ( do + resolvedCustomTypes <- + (| onNothingA (throwA -< err400 Unexpected "custom types are inconsistent") + |) maybeResolvedCustomTypes (resolvedDef, outFields) <- bindErrorA -< resolveAction resolvedCustomTypes def let permissionInfos = map (ActionPermissionInfo . _apmRole) actionPermissions permissionMap = mapFromL _apiRole permissionInfos @@ -287,7 +293,7 @@ buildSchemaCacheRule = proc (catalogMetadata, invalidationKeys) -> do , _boFunctions = functionCache , _boRemoteSchemas = remoteSchemaMap , _boAllowlist = allowList - , _boCustomTypes = resolvedCustomTypes + , _boCustomTypes = fromMaybe (NonObjectTypeMap mempty, mempty) maybeResolvedCustomTypes } mkEventTriggerMetadataObject (CatalogEventTrigger qt trn configuration) = diff --git a/server/src-lib/Hasura/RQL/Types/Catalog.hs b/server/src-lib/Hasura/RQL/Types/Catalog.hs index 0f9eb2d34e4e1..68f49c35e90e6 100644 --- a/server/src-lib/Hasura/RQL/Types/Catalog.hs +++ b/server/src-lib/Hasura/RQL/Types/Catalog.hs @@ -153,6 +153,7 @@ data CatalogMetadata , _cmComputedFields :: ![CatalogComputedField] , _cmCustomTypes :: !CustomTypes , _cmActions :: ![CatalogAction] + , _cmScalarTypes :: ![PGScalarType] } deriving (Show, Eq, Generic) instance NFData CatalogMetadata instance Cacheable CatalogMetadata diff --git a/server/src-rsr/catalog_metadata.sql b/server/src-rsr/catalog_metadata.sql index a3a2ef1613d7b..7bc985a5b4d47 100644 --- a/server/src-rsr/catalog_metadata.sql +++ b/server/src-rsr/catalog_metadata.sql @@ -9,7 +9,8 @@ select 'allowlist_collections', allowlist.item, 'computed_fields', computed_field.items, 'custom_types', coalesce((select custom_types from hdb_catalog.hdb_custom_types), '{}'), - 'actions', actions.items + 'actions', actions.items, + 'scalar_types', scalar_types.items ) from ( @@ -205,4 +206,8 @@ from hdb_catalog.hdb_action_permission hap where hap.action_name = ha.action_name ) p on 'true' - ) as actions + ) as actions, + ( + select coalesce(json_agg(typname), '[]') as items + from pg_catalog.pg_type where typtype = 'b' + ) as scalar_types From ae335eebc73ec417c4630a8ddc9be3969d8be549 Mon Sep 17 00:00:00 2001 From: rakeshkky <12475069+rakeshkky@users.noreply.github.com> Date: Wed, 8 Apr 2020 17:31:28 +0530 Subject: [PATCH 02/14] add pytest tests --- .../actions/custom-types/reuse_pgscalars.yaml | 20 ++++++ .../custom-types/reuse_unknown_pgscalar.yaml | 61 +++++++++++++++++++ .../queries/actions/custom-types/setup.yaml | 19 ++++++ .../actions/custom-types/teardown.yaml | 11 ++++ server/tests-py/test_actions.py | 13 ++++ 5 files changed, 124 insertions(+) create mode 100644 server/tests-py/queries/actions/custom-types/reuse_pgscalars.yaml create mode 100644 server/tests-py/queries/actions/custom-types/reuse_unknown_pgscalar.yaml create mode 100644 server/tests-py/queries/actions/custom-types/setup.yaml create mode 100644 server/tests-py/queries/actions/custom-types/teardown.yaml diff --git a/server/tests-py/queries/actions/custom-types/reuse_pgscalars.yaml b/server/tests-py/queries/actions/custom-types/reuse_pgscalars.yaml new file mode 100644 index 0000000000000..892da3a37f569 --- /dev/null +++ b/server/tests-py/queries/actions/custom-types/reuse_pgscalars.yaml @@ -0,0 +1,20 @@ +description: Set custom types with Postgres scalars +url: /v1/query +status: 200 +query: + type: set_custom_types + args: + objects: + - name: User + fields: + - name: user_id + type: uuid! + - name: location + type: geography + input_objects: + - name: UserInput + fields: + - name: name + type: String! + - name: id + type: uuid! diff --git a/server/tests-py/queries/actions/custom-types/reuse_unknown_pgscalar.yaml b/server/tests-py/queries/actions/custom-types/reuse_unknown_pgscalar.yaml new file mode 100644 index 0000000000000..e72e09502b0c4 --- /dev/null +++ b/server/tests-py/queries/actions/custom-types/reuse_unknown_pgscalar.yaml @@ -0,0 +1,61 @@ +description: Set custom types with Postgres scalars +url: /v1/query +status: 400 +response: + internal: + - definition: + input_objects: + - name: UserInput + description: + fields: + - name: name + type: String! + description: + - name: id + type: uuid! + description: + objects: + - name: User + relationships: + description: + fields: + - arguments: + name: user_id + type: uuid! + description: + - arguments: + name: location + type: geography + description: + - arguments: + name: unknown_pgtype + type: unknown_type + description: + scalars: + enums: + reason: validation for the given custom types failed because the type "unknown_type" + for field "unknown_pgtype" in object type "User" does not exist + type: custom_types + path: $.args + error: validation for the given custom types failed because the type "unknown_type" + for field "unknown_pgtype" in object type "User" does not exist + code: constraint-violation +query: + type: set_custom_types + args: + objects: + - name: User + fields: + - name: user_id + type: uuid! + - name: location + type: geography + - name: unknown_pgtype + type: unknown_type + input_objects: + - name: UserInput + fields: + - name: name + type: String! + - name: id + type: uuid! diff --git a/server/tests-py/queries/actions/custom-types/setup.yaml b/server/tests-py/queries/actions/custom-types/setup.yaml new file mode 100644 index 0000000000000..93be265c95bd5 --- /dev/null +++ b/server/tests-py/queries/actions/custom-types/setup.yaml @@ -0,0 +1,19 @@ +type: bulk +args: +- type: run_sql + args: + sql: | + CREATE EXTENSION IF NOT EXISTS postgis; + CREATE EXTENSION IF NOT EXISTS postgis_topology; + DO $$ + BEGIN + IF PostGIS_lib_version() ~ '^3.*' THEN + CREATE EXTENSION IF NOT EXISTS postgis_raster; + END IF; + END$$; + + CREATE TABLE "user"( + id UUID PRIMARY KEY DEFAULT gen_random_uuid(), + name TEXT NOT NULL, + location geography + ); diff --git a/server/tests-py/queries/actions/custom-types/teardown.yaml b/server/tests-py/queries/actions/custom-types/teardown.yaml new file mode 100644 index 0000000000000..bcb2f590a6e5e --- /dev/null +++ b/server/tests-py/queries/actions/custom-types/teardown.yaml @@ -0,0 +1,11 @@ +type: bulk +args: +# reset custom types +- type: set_custom_types + args: {} + +- type: run_sql + args: + cascade: true + sql: | + DROP TABLE "user"; diff --git a/server/tests-py/test_actions.py b/server/tests-py/test_actions.py index 2ea388301ca1a..f2f240f710ff1 100644 --- a/server/tests-py/test_actions.py +++ b/server/tests-py/test_actions.py @@ -297,3 +297,16 @@ def test_create_user_roles(self, hge_ctx): # Query the action as user-id 1 # Make request without auth using admin_secret check_query(hge_ctx, conf_user_1, add_auth = False) + +@pytest.mark.usefixtures('per_class_tests_db_state') +class TestSetCustomTypes: + + @classmethod + def dir(cls): + return 'queries/actions/custom-types' + + def test_resuse_pgscalars(self, hge_ctx): + check_query_f(hge_ctx, self.dir() + '/reuse_pgscalars.yaml') + + def test_resuse_unknown_pgscalar(self, hge_ctx): + check_query_f(hge_ctx, self.dir() + '/reuse_unknown_pgscalar.yaml') From 36b246201ffe637e4bf6187d74badd132c116f12 Mon Sep 17 00:00:00 2001 From: rakeshkky <12475069+rakeshkky@users.noreply.github.com> Date: Wed, 8 Apr 2020 17:32:25 +0530 Subject: [PATCH 03/14] update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 833f79bddb62c..3de88e391a2b0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ The order and collapsed state of columns is now persisted across page navigation ### Bug fixes and improvements +- server: reusing Postgres scalars is allowed for custom types - cli: allow customization of server api paths (close #4016) - cli: clean up migration files created during a failed migrate api (close #4312) (#4319) - cli: add support for multiple versions of plugin (close #4105) From 0ed402367b86b6eb6153b4d20685f110acae27f7 Mon Sep 17 00:00:00 2001 From: rakeshkky <12475069+rakeshkky@users.noreply.github.com> Date: Wed, 8 Apr 2020 18:10:13 +0530 Subject: [PATCH 04/14] add a doc pointer for reusable postgres scalars --- CHANGELOG.md | 2 +- docs/graphql/manual/actions/types/index.rst | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3de88e391a2b0..1bdc3e79eee06 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,7 +11,7 @@ The order and collapsed state of columns is now persisted across page navigation ### Bug fixes and improvements -- server: reusing Postgres scalars is allowed for custom types +- server: reusing Postgres scalars in custom types is allowed (close #4125) - cli: allow customization of server api paths (close #4016) - cli: clean up migration files created during a failed migrate api (close #4312) (#4319) - cli: add support for multiple versions of plugin (close #4105) diff --git a/docs/graphql/manual/actions/types/index.rst b/docs/graphql/manual/actions/types/index.rst index cc2552ea57244..2b7d1dd363d55 100644 --- a/docs/graphql/manual/actions/types/index.rst +++ b/docs/graphql/manual/actions/types/index.rst @@ -141,6 +141,11 @@ a scalar called ``Date``, you can define it like. These scalars can be used as arguments of the mutation or as fields of object types and input types. +.. admonition:: Postgres scalars + + The base types from Postgres can be reusable as custom scalars without explicitly declaring them. + For example, the ``uuid`` type from ``pgcrypto`` extension can be used to resolve a UUID data. + Enum types ---------- @@ -165,4 +170,3 @@ This means that wherever we use the type ``Color`` in our schema, we expect it to be exactly one of RED, GREEN, or BLUE. `See reference `__ - From 2cdbd731ee15219933806a43b899533d99cecc0f Mon Sep 17 00:00:00 2001 From: rakeshkky <12475069+rakeshkky@users.noreply.github.com> Date: Wed, 8 Apr 2020 20:46:16 +0530 Subject: [PATCH 05/14] document the code, improve the CHANGELOG entry As suggested by @lexi-lambda --- CHANGELOG.md | 2 +- .../Hasura/GraphQL/Schema/CustomTypes.hs | 5 +- server/src-lib/Hasura/RQL/DDL/CustomTypes.hs | 53 +++++++++++-------- server/src-lib/Hasura/RQL/DDL/Schema/Cache.hs | 5 +- server/src-lib/Hasura/RQL/Types/Catalog.hs | 18 ++++++- server/src-rsr/catalog_metadata.sql | 20 ++++--- 6 files changed, 67 insertions(+), 36 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1bdc3e79eee06..3ff818916456f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,7 +11,7 @@ The order and collapsed state of columns is now persisted across page navigation ### Bug fixes and improvements -- server: reusing Postgres scalars in custom types is allowed (close #4125) +- server: support reusing Postgres scalars in custom types (close #4125) - cli: allow customization of server api paths (close #4016) - cli: clean up migration files created during a failed migrate api (close #4312) (#4319) - cli: add support for multiple versions of plugin (close #4105) diff --git a/server/src-lib/Hasura/GraphQL/Schema/CustomTypes.hs b/server/src-lib/Hasura/GraphQL/Schema/CustomTypes.hs index 39e4d03b5c330..1f83357c20a0f 100644 --- a/server/src-lib/Hasura/GraphQL/Schema/CustomTypes.hs +++ b/server/src-lib/Hasura/GraphQL/Schema/CustomTypes.hs @@ -116,7 +116,10 @@ annotateObjectType tableCache nonObjectTypeMap objectDefinition = do buildCustomTypesSchemaPartial :: (QErrM m) - => TableCache -> CustomTypes -> [PGScalarType] -> m (NonObjectTypeMap, AnnotatedObjects) + => TableCache -- ^ The Postgres tables + -> CustomTypes -- ^ Custom types to be resolved + -> [PGScalarType] -- ^ Any reused Postgres base types (as scalars) in custom types + -> m (NonObjectTypeMap, AnnotatedObjects) buildCustomTypesSchemaPartial tableCache customTypes pgScalars = do let typeInfos = map (VT.TIEnum . convertEnumDefinition) enumDefinitions <> diff --git a/server/src-lib/Hasura/RQL/DDL/CustomTypes.hs b/server/src-lib/Hasura/RQL/DDL/CustomTypes.hs index 79c1511d93f53..3ab5ee416213e 100644 --- a/server/src-lib/Hasura/RQL/DDL/CustomTypes.hs +++ b/server/src-lib/Hasura/RQL/DDL/CustomTypes.hs @@ -22,9 +22,13 @@ import Hasura.SQL.Types import Hasura.GraphQL.Schema.CustomTypes (buildCustomTypesSchemaPartial) +-- | Validate the custom types and return any reused Postgres base types (as scalars) validateCustomTypeDefinitions :: (MonadValidate [CustomTypeValidationError] m) - => TableCache -> CustomTypes -> [PGScalarType] -> m [PGScalarType] + => TableCache -- ^ The Postgres tables + -> CustomTypes -- ^ Custom types subject to validation + -> [PGScalarType] -- ^ List of all Postgres base types + -> m [PGScalarType] -- ^ List of reused Postgres base types (as scalars) in custom types validateCustomTypeDefinitions tableCache customTypes allPGScalars = do unless (null duplicateTypes) $ dispute $ pure $ DuplicateTypeNames duplicateTypes traverse_ validateEnum enumDefinitions @@ -88,15 +92,16 @@ validateCustomTypeDefinitions tableCache customTypes allPGScalars = do fmap (catMaybes . toList) $ for (_iotdFields inputObjectDefinition) $ \inputObjectField -> do let fieldBaseType = G.getBaseType $ unGraphQLType $ _iofdType inputObjectField - -- Check if field type uses any Postgres scalar type and return the same - case findReusedPGScalar fieldBaseType of - Just reusedScalar -> pure $ Just reusedScalar - Nothing -> do - unless (Set.member fieldBaseType inputTypes) $ + if Set.member fieldBaseType inputTypes then pure Nothing + else + -- Check if field type uses any Postgres scalar type and return the same + case findReusedPGScalar fieldBaseType of + Just reusedScalar -> pure $ Just reusedScalar + Nothing -> do refute $ pure $ InputObjectFieldTypeDoesNotExist - (_iotdName inputObjectDefinition) - (_iofdName inputObjectField) fieldBaseType - pure Nothing + (_iotdName inputObjectDefinition) + (_iofdName inputObjectField) fieldBaseType + pure Nothing validateObject :: (MonadValidate [CustomTypeValidationError] m) @@ -129,21 +134,23 @@ validateCustomTypeDefinitions tableCache customTypes allPGScalars = do let objectTypes = Set.fromList $ map (unObjectTypeName . _otdName) objectDefinitions - -- Check if field type uses any Postgres scalar type and return the same - maybeReusedPGScalar <- case findReusedPGScalar fieldBaseType of - Just ty -> pure $ Just ty - Nothing -> do - -- check that the fields only reference scalars and enums - -- and not other object types - if | Set.member fieldBaseType scalarTypes -> pure () - | Set.member fieldBaseType enumTypes -> pure () - | Set.member fieldBaseType objectTypes -> - dispute $ pure $ ObjectFieldObjectBaseType + maybeReusedPGScalar <- + -- check that the fields only reference scalars and enums + -- and not other object types + if | Set.member fieldBaseType scalarTypes -> pure Nothing + | Set.member fieldBaseType enumTypes -> pure Nothing + | Set.member fieldBaseType objectTypes -> do + dispute $ pure $ ObjectFieldObjectBaseType objectTypeName fieldName fieldBaseType - | otherwise -> - dispute $ pure $ ObjectFieldTypeDoesNotExist - objectTypeName fieldName fieldBaseType - pure Nothing + pure Nothing + | otherwise -> + -- Check if field type uses any Postgres scalar type and return the same + case findReusedPGScalar fieldBaseType of + Just ty -> pure $ Just ty + Nothing -> do + dispute $ pure $ ObjectFieldTypeDoesNotExist + objectTypeName fieldName fieldBaseType + pure Nothing -- collect all non list scalar types of this object maybeScalar <- if (not (isListType fieldType) && Set.member fieldBaseType scalarTypes) diff --git a/server/src-lib/Hasura/RQL/DDL/Schema/Cache.hs b/server/src-lib/Hasura/RQL/DDL/Schema/Cache.hs index fb22f02b88ac0..bc82a13ede369 100644 --- a/server/src-lib/Hasura/RQL/DDL/Schema/Cache.hs +++ b/server/src-lib/Hasura/RQL/DDL/Schema/Cache.hs @@ -198,7 +198,7 @@ buildSchemaCacheRule = proc (catalogMetadata, invalidationKeys) -> do buildAndCollectInfo = proc (catalogMetadata, invalidationKeys) -> do let CatalogMetadata tables relationships permissions eventTriggers remoteSchemas functions allowlistDefs - computedFields customTypes actions pgScalars = catalogMetadata + computedFields catalogCustomTypes actions = catalogMetadata -- tables tableRawInfos <- buildTableCache -< (tables, Inc.selectD #_ikMetadata invalidationKeys) @@ -256,6 +256,7 @@ buildSchemaCacheRule = proc (catalogMetadata, invalidationKeys) -> do & HS.fromList -- custom types + let CatalogCustomTypes customTypes pgScalars = catalogCustomTypes maybeResolvedCustomTypes <- (| withRecordInconsistency (bindErrorA -< resolveCustomTypes tableCache customTypes pgScalars) @@ -293,6 +294,8 @@ buildSchemaCacheRule = proc (catalogMetadata, invalidationKeys) -> do , _boFunctions = functionCache , _boRemoteSchemas = remoteSchemaMap , _boAllowlist = allowList + -- If 'maybeResolvedCustomTypes' is 'Nothing', then custom types are inconsinstent. + -- In such case, use empty resolved value of custom types. , _boCustomTypes = fromMaybe (NonObjectTypeMap mempty, mempty) maybeResolvedCustomTypes } diff --git a/server/src-lib/Hasura/RQL/Types/Catalog.hs b/server/src-lib/Hasura/RQL/Types/Catalog.hs index 68f49c35e90e6..916765a7e0ca3 100644 --- a/server/src-lib/Hasura/RQL/Types/Catalog.hs +++ b/server/src-lib/Hasura/RQL/Types/Catalog.hs @@ -12,6 +12,7 @@ module Hasura.RQL.Types.Catalog , CatalogPermission(..) , CatalogEventTrigger(..) , CatalogFunction(..) + , CatalogCustomTypes(..) ) where import Hasura.Prelude @@ -139,6 +140,20 @@ instance NFData CatalogFunction instance Cacheable CatalogFunction $(deriveFromJSON (aesonDrop 3 snakeCase) ''CatalogFunction) +data CatalogCustomTypes + = CatalogCustomTypes + { _cctCustomTypes :: !CustomTypes + , _cctPgScalars :: ![PGScalarType] + -- ^ Base types from Postgres required for validating custom types + -- to check if any one of them uses the Postgres base type name + -- and publish them in generated GraphQL schema as Scalars. + -- These won't be tracked in the metadata. + -- See https://github.com/hasura/graphql-engine/issues/4125 + } deriving (Show, Eq, Generic) +instance NFData CatalogCustomTypes +instance Cacheable CatalogCustomTypes +$(deriveFromJSON (aesonDrop 4 snakeCase) ''CatalogCustomTypes) + type CatalogAction = ActionMetadata data CatalogMetadata @@ -151,9 +166,8 @@ data CatalogMetadata , _cmFunctions :: ![CatalogFunction] , _cmAllowlistCollections :: ![CollectionDef] , _cmComputedFields :: ![CatalogComputedField] - , _cmCustomTypes :: !CustomTypes + , _cmCustomTypes :: !CatalogCustomTypes , _cmActions :: ![CatalogAction] - , _cmScalarTypes :: ![PGScalarType] } deriving (Show, Eq, Generic) instance NFData CatalogMetadata instance Cacheable CatalogMetadata diff --git a/server/src-rsr/catalog_metadata.sql b/server/src-rsr/catalog_metadata.sql index 7bc985a5b4d47..54183f2b9d60e 100644 --- a/server/src-rsr/catalog_metadata.sql +++ b/server/src-rsr/catalog_metadata.sql @@ -8,9 +8,8 @@ select 'functions', functions.items, 'allowlist_collections', allowlist.item, 'computed_fields', computed_field.items, - 'custom_types', coalesce((select custom_types from hdb_catalog.hdb_custom_types), '{}'), - 'actions', actions.items, - 'scalar_types', scalar_types.items + 'custom_types', custom_types.item, + 'actions', actions.items ) from ( @@ -174,6 +173,15 @@ from where function_name = cc.function_name and function_schema = cc.function_schema ) fi on 'true' ) as computed_field, + ( + select + json_build_object( + 'custom_types', + coalesce((select custom_types from hdb_catalog.hdb_custom_types), '{}'), + 'pg_scalars', + coalesce((select json_agg(typname) from pg_catalog.pg_type where typtype = 'b'), '[]') + ) as item + ) as custom_types, ( select coalesce( @@ -206,8 +214,4 @@ from hdb_catalog.hdb_action_permission hap where hap.action_name = ha.action_name ) p on 'true' - ) as actions, - ( - select coalesce(json_agg(typname), '[]') as items - from pg_catalog.pg_type where typtype = 'b' - ) as scalar_types + ) as actions From 879853ecefcc47760ee2ef9c13034ca93008dfb1 Mon Sep 17 00:00:00 2001 From: rakeshkky <12475069+rakeshkky@users.noreply.github.com> Date: Thu, 9 Apr 2020 12:04:22 +0530 Subject: [PATCH 06/14] a bit more source code documentation, use WriterT to collect reused scalars --- server/src-lib/Hasura/RQL/DDL/CustomTypes.hs | 118 +++++++++++-------- server/src-lib/Hasura/RQL/Types/Catalog.hs | 13 +- server/src-rsr/catalog_metadata.sql | 2 +- 3 files changed, 80 insertions(+), 53 deletions(-) diff --git a/server/src-lib/Hasura/RQL/DDL/CustomTypes.hs b/server/src-lib/Hasura/RQL/DDL/CustomTypes.hs index 3ab5ee416213e..02e2c79c2bfd3 100644 --- a/server/src-lib/Hasura/RQL/DDL/CustomTypes.hs +++ b/server/src-lib/Hasura/RQL/DDL/CustomTypes.hs @@ -7,6 +7,7 @@ module Hasura.RQL.DDL.CustomTypes import Control.Monad.Validate +import qualified Control.Monad.Writer.Strict as Writer import qualified Data.HashMap.Strict as Map import qualified Data.HashSet as Set import qualified Data.List.Extended as L @@ -22,19 +23,47 @@ import Hasura.SQL.Types import Hasura.GraphQL.Schema.CustomTypes (buildCustomTypesSchemaPartial) +{- Note [Postgres scalars in custom types] +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +It’s very convenient to be able to reference Postgres scalars in custom type +definitions. For example, we might have a type like this: + + type User { + id: uuid! + name: String! + location: geography + } + +The uuid and geography types are Postgres scalars, not separately-defined +GraphQL types. To support this, we have to take a few extra steps: + + 1. The set of Postgres base types is not fixed; extensions like PostGIS add + new ones, and users can even define their own. Therefore, we fetch the + currently defined base types from the @pg_catalog.pg_type@ system table as part of + loading the metadata. + + 2. It’s possible for a custom type definition to use a type that doesn’t + appear elsewhere in the GraphQL schema, so we record which base types were + referenced while validating the custom type definitions and make sure to + include them in the generated schema explicitly. +-} + -- | Validate the custom types and return any reused Postgres base types (as scalars) validateCustomTypeDefinitions :: (MonadValidate [CustomTypeValidationError] m) - => TableCache -- ^ The Postgres tables - -> CustomTypes -- ^ Custom types subject to validation + => TableCache + -> CustomTypes -> [PGScalarType] -- ^ List of all Postgres base types - -> m [PGScalarType] -- ^ List of reused Postgres base types (as scalars) in custom types + -> m (Set.HashSet PGScalarType) -- ^ Reused Postgres base types (as scalars) in custom types validateCustomTypeDefinitions tableCache customTypes allPGScalars = do unless (null duplicateTypes) $ dispute $ pure $ DuplicateTypeNames duplicateTypes traverse_ validateEnum enumDefinitions - inputObjectReusedPGScalars <- concat <$> traverse validateInputObject inputObjectDefinitions - objectReusedPGScalars <- concat <$> traverse validateObject objectDefinitions - pure $ inputObjectReusedPGScalars <> objectReusedPGScalars + -- The following validations also accumulates reused Postgres scalar types + -- in a 'WriterT' monad. Using 'execWriterT' to peel of the monad and collect + -- Postgres scalar types. + Writer.execWriterT $ do + traverse_ validateInputObject inputObjectDefinitions + traverse_ validateObject objectDefinitions where inputObjectDefinitions = fromMaybe [] $ _ctInputObjects customTypes objectDefinitions = fromMaybe [] $ _ctObjects customTypes @@ -68,8 +97,10 @@ validateCustomTypeDefinitions tableCache customTypes allPGScalars = do (_etdName enumDefinition) duplicateEnumValues validateInputObject - :: (MonadValidate [CustomTypeValidationError] m) - => InputObjectTypeDefinition -> m [PGScalarType] + :: ( MonadValidate [CustomTypeValidationError] m + , MonadWriter (Set.HashSet PGScalarType) m + ) + => InputObjectTypeDefinition -> m () validateInputObject inputObjectDefinition = do let inputObjectTypeName = _iotdName inputObjectDefinition duplicateFieldNames = @@ -89,23 +120,23 @@ validateCustomTypeDefinitions tableCache customTypes allPGScalars = do scalarTypes `Set.union` enumTypes `Set.union` inputObjectTypes -- check that fields reference input types - fmap (catMaybes . toList) $ - for (_iotdFields inputObjectDefinition) $ \inputObjectField -> do - let fieldBaseType = G.getBaseType $ unGraphQLType $ _iofdType inputObjectField - if Set.member fieldBaseType inputTypes then pure Nothing - else - -- Check if field type uses any Postgres scalar type and return the same - case findReusedPGScalar fieldBaseType of - Just reusedScalar -> pure $ Just reusedScalar - Nothing -> do - refute $ pure $ InputObjectFieldTypeDoesNotExist - (_iotdName inputObjectDefinition) - (_iofdName inputObjectField) fieldBaseType - pure Nothing + for_ (_iotdFields inputObjectDefinition) $ \inputObjectField -> do + let fieldBaseType = G.getBaseType $ unGraphQLType $ _iofdType inputObjectField + if Set.member fieldBaseType inputTypes then pure () + else + -- Check if field type uses any Postgres scalar type and return the same + case findReusedPGScalar fieldBaseType of + Just reusedScalar -> Writer.tell $ Set.singleton reusedScalar + Nothing -> + refute $ pure $ InputObjectFieldTypeDoesNotExist + (_iotdName inputObjectDefinition) + (_iofdName inputObjectField) fieldBaseType validateObject - :: (MonadValidate [CustomTypeValidationError] m) - => ObjectTypeDefinition -> m [PGScalarType] + :: ( MonadValidate [CustomTypeValidationError] m + , MonadWriter (Set.HashSet PGScalarType) m + ) + => ObjectTypeDefinition -> m () validateObject objectDefinition = do let objectTypeName = _otdName objectDefinition fieldNames = map (unObjectFieldName . _ofdName) $ @@ -119,8 +150,7 @@ validateCustomTypeDefinitions tableCache customTypes allPGScalars = do unless (null duplicateFieldNames) $ dispute $ pure $ ObjectDuplicateFields objectTypeName duplicateFieldNames - (scalarFields, reusedScalars) <- - fmap (((Map.fromList . catMaybes) *** catMaybes) . unzip) $ + scalarFields <- fmap (Map.fromList . catMaybes) $ for fields $ \objectField -> do let fieldType = _ofdType objectField fieldBaseType = G.getBaseType $ unGraphQLType fieldType @@ -134,29 +164,25 @@ validateCustomTypeDefinitions tableCache customTypes allPGScalars = do let objectTypes = Set.fromList $ map (unObjectTypeName . _otdName) objectDefinitions - maybeReusedPGScalar <- - -- check that the fields only reference scalars and enums - -- and not other object types - if | Set.member fieldBaseType scalarTypes -> pure Nothing - | Set.member fieldBaseType enumTypes -> pure Nothing - | Set.member fieldBaseType objectTypes -> do - dispute $ pure $ ObjectFieldObjectBaseType - objectTypeName fieldName fieldBaseType - pure Nothing - | otherwise -> - -- Check if field type uses any Postgres scalar type and return the same - case findReusedPGScalar fieldBaseType of - Just ty -> pure $ Just ty - Nothing -> do - dispute $ pure $ ObjectFieldTypeDoesNotExist - objectTypeName fieldName fieldBaseType - pure Nothing + -- check that the fields only reference scalars and enums + -- and not other object types + if | Set.member fieldBaseType scalarTypes -> pure () + | Set.member fieldBaseType enumTypes -> pure () + | Set.member fieldBaseType objectTypes -> + dispute $ pure $ ObjectFieldObjectBaseType + objectTypeName fieldName fieldBaseType + | otherwise -> + -- Check if field type uses any Postgres scalar type and return the same + case findReusedPGScalar fieldBaseType of + Just ty -> Writer.tell $ Set.singleton ty + Nothing -> + dispute $ pure $ ObjectFieldTypeDoesNotExist + objectTypeName fieldName fieldBaseType -- collect all non list scalar types of this object - maybeScalar <- if (not (isListType fieldType) && Set.member fieldBaseType scalarTypes) + if (not (isListType fieldType) && Set.member fieldBaseType scalarTypes) then pure $ Just (fieldName, fieldBaseType) else pure Nothing - pure (maybeScalar, maybeReusedPGScalar) for_ relationships $ \relationshipField -> do let relationshipName = _trName relationshipField @@ -182,8 +208,6 @@ validateCustomTypeDefinitions tableCache customTypes allPGScalars = do objectTypeName relationshipName remoteTable columnName return () - pure reusedScalars - findReusedPGScalar baseType = find ((==) baseType . mkScalarTy) allPGScalars @@ -296,7 +320,7 @@ resolveCustomTypes resolveCustomTypes tableCache customTypes allPGScalars = do reusedPGScalars <- either (throw400 ConstraintViolation . showErrors) pure =<< runValidateT (validateCustomTypeDefinitions tableCache customTypes allPGScalars) - buildCustomTypesSchemaPartial tableCache customTypes reusedPGScalars + buildCustomTypesSchemaPartial tableCache customTypes $ toList reusedPGScalars where showErrors :: [CustomTypeValidationError] -> T.Text showErrors allErrors = diff --git a/server/src-lib/Hasura/RQL/Types/Catalog.hs b/server/src-lib/Hasura/RQL/Types/Catalog.hs index 916765a7e0ca3..e84d6c823e90b 100644 --- a/server/src-lib/Hasura/RQL/Types/Catalog.hs +++ b/server/src-lib/Hasura/RQL/Types/Catalog.hs @@ -144,11 +144,14 @@ data CatalogCustomTypes = CatalogCustomTypes { _cctCustomTypes :: !CustomTypes , _cctPgScalars :: ![PGScalarType] - -- ^ Base types from Postgres required for validating custom types - -- to check if any one of them uses the Postgres base type name - -- and publish them in generated GraphQL schema as Scalars. - -- These won't be tracked in the metadata. - -- See https://github.com/hasura/graphql-engine/issues/4125 + -- ^ All Postgres base types, which may be referenced in custom type definitions. + -- When we validate the custom types (see 'validateCustomTypeDefinitions'), + -- we record which base types were referenced so that we can be sure to include them + -- in the generated GraphQL schema. + -- These are not actually part of the Hasura metadata --- we fetch them from + -- @pg_catalog.pg_type@ --- but they’re needed when validating the custom type + -- metadata, so we include them here. + -- See Note [Postgres scalars in custom types] for more details. } deriving (Show, Eq, Generic) instance NFData CatalogCustomTypes instance Cacheable CatalogCustomTypes diff --git a/server/src-rsr/catalog_metadata.sql b/server/src-rsr/catalog_metadata.sql index 54183f2b9d60e..4c4f7fc52090f 100644 --- a/server/src-rsr/catalog_metadata.sql +++ b/server/src-rsr/catalog_metadata.sql @@ -178,7 +178,7 @@ from json_build_object( 'custom_types', coalesce((select custom_types from hdb_catalog.hdb_custom_types), '{}'), - 'pg_scalars', + 'pg_scalars', -- See Note [Postgres scalars in custom types] coalesce((select json_agg(typname) from pg_catalog.pg_type where typtype = 'b'), '[]') ) as item ) as custom_types, From 5d4f54d83d8c4f1254d01d4bf5ec8618880028ae Mon Sep 17 00:00:00 2001 From: Rakesh Emmadi <12475069+rakeshkky@users.noreply.github.com> Date: Thu, 9 Apr 2020 12:09:14 +0530 Subject: [PATCH 07/14] Apply suggestions from code review Co-Authored-By: Marion Schleifer --- docs/graphql/manual/actions/types/index.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/graphql/manual/actions/types/index.rst b/docs/graphql/manual/actions/types/index.rst index 2b7d1dd363d55..a413990364eba 100644 --- a/docs/graphql/manual/actions/types/index.rst +++ b/docs/graphql/manual/actions/types/index.rst @@ -143,8 +143,8 @@ types and input types. .. admonition:: Postgres scalars - The base types from Postgres can be reusable as custom scalars without explicitly declaring them. - For example, the ``uuid`` type from ``pgcrypto`` extension can be used to resolve a UUID data. + Postgres base types are reusable as custom scalars without explicitly declaring them. + For example, the ``uuid`` type from the ``pgcrypto`` extension can be used to resolve a ``uuid`` data field. Enum types ---------- From d12bab3c76cc9758a7c06eeca7c7064b55c0eee7 Mon Sep 17 00:00:00 2001 From: rakeshkky <12475069+rakeshkky@users.noreply.github.com> Date: Thu, 9 Apr 2020 12:30:36 +0530 Subject: [PATCH 08/14] improve doc for Postgres scalars in custom graphql types --- docs/graphql/manual/actions/types/index.rst | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/docs/graphql/manual/actions/types/index.rst b/docs/graphql/manual/actions/types/index.rst index a413990364eba..2d8915a1834b7 100644 --- a/docs/graphql/manual/actions/types/index.rst +++ b/docs/graphql/manual/actions/types/index.rst @@ -144,7 +144,17 @@ types and input types. .. admonition:: Postgres scalars Postgres base types are reusable as custom scalars without explicitly declaring them. - For example, the ``uuid`` type from the ``pgcrypto`` extension can be used to resolve a ``uuid`` data field. + For example, we might have a ``type`` like this: + + .. code-block:: graphql + + type User { + id: uuid! + name: String! + location: geography + } + + The ``uuid`` and ``geography`` types are Postgres scalars, not separately-defined custom GraphQL types. Enum types ---------- From bf7b1bf1da633f63699c39ed0f937811510dff1f Mon Sep 17 00:00:00 2001 From: Alexis King Date: Thu, 9 Apr 2020 03:25:19 -0500 Subject: [PATCH 09/14] Add some more references to Note; fix Haddock syntax Also a few very minor tweaks: * Use HashSet instead of [] more pervasively * Export execWriterT from Hasura.Prelude * Use pattern guards in multi-way if * Tweak a few names/comments --- .../Hasura/GraphQL/Schema/CustomTypes.hs | 10 ++-- server/src-lib/Hasura/Prelude.hs | 3 +- server/src-lib/Hasura/RQL/DDL/CustomTypes.hs | 50 ++++++++----------- server/src-lib/Hasura/RQL/Types/Catalog.hs | 4 +- 4 files changed, 32 insertions(+), 35 deletions(-) diff --git a/server/src-lib/Hasura/GraphQL/Schema/CustomTypes.hs b/server/src-lib/Hasura/GraphQL/Schema/CustomTypes.hs index 1f83357c20a0f..bd313b4a703bd 100644 --- a/server/src-lib/Hasura/GraphQL/Schema/CustomTypes.hs +++ b/server/src-lib/Hasura/GraphQL/Schema/CustomTypes.hs @@ -116,16 +116,18 @@ annotateObjectType tableCache nonObjectTypeMap objectDefinition = do buildCustomTypesSchemaPartial :: (QErrM m) - => TableCache -- ^ The Postgres tables - -> CustomTypes -- ^ Custom types to be resolved - -> [PGScalarType] -- ^ Any reused Postgres base types (as scalars) in custom types + => TableCache + -> CustomTypes + -> HashSet PGScalarType + -- ^ Postgres base types used in the custom type definitions; + -- see Note [Postgres scalars in custom types]. -> m (NonObjectTypeMap, AnnotatedObjects) buildCustomTypesSchemaPartial tableCache customTypes pgScalars = do let typeInfos = map (VT.TIEnum . convertEnumDefinition) enumDefinitions <> map (VT.TIInpObj . convertInputObjectDefinition) inputObjectDefinitions <> map (VT.TIScalar . convertScalarDefinition) scalarDefinitions <> - map (VT.TIScalar . VT.mkHsraScalarTyInfo) pgScalars + map (VT.TIScalar . VT.mkHsraScalarTyInfo) (toList pgScalars) nonObjectTypeMap = NonObjectTypeMap $ mapFromL VT.getNamedTy typeInfos annotatedObjectTypes <- mapFromL (_otdName . _aotDefinition) <$> diff --git a/server/src-lib/Hasura/Prelude.hs b/server/src-lib/Hasura/Prelude.hs index 5a93216d828c1..87e71b978836d 100644 --- a/server/src-lib/Hasura/Prelude.hs +++ b/server/src-lib/Hasura/Prelude.hs @@ -28,7 +28,8 @@ import Control.Monad.Fail as M (MonadFail) import Control.Monad.Identity as M import Control.Monad.Reader as M import Control.Monad.State.Strict as M -import Control.Monad.Writer.Strict as M (MonadWriter (..), WriterT (..)) +import Control.Monad.Writer.Strict as M (MonadWriter (..), WriterT (..), + execWriterT, runWriterT) import Data.Align as M (Align (align, alignWith)) import Data.Align.Key as M (AlignWithKey (..)) import Data.Bool as M (bool) diff --git a/server/src-lib/Hasura/RQL/DDL/CustomTypes.hs b/server/src-lib/Hasura/RQL/DDL/CustomTypes.hs index 02e2c79c2bfd3..eca1f928b3aa1 100644 --- a/server/src-lib/Hasura/RQL/DDL/CustomTypes.hs +++ b/server/src-lib/Hasura/RQL/DDL/CustomTypes.hs @@ -7,7 +7,6 @@ module Hasura.RQL.DDL.CustomTypes import Control.Monad.Validate -import qualified Control.Monad.Writer.Strict as Writer import qualified Data.HashMap.Strict as Map import qualified Data.HashSet as Set import qualified Data.List.Extended as L @@ -48,22 +47,19 @@ GraphQL types. To support this, we have to take a few extra steps: include them in the generated schema explicitly. -} --- | Validate the custom types and return any reused Postgres base types (as scalars) +-- | Validate the custom types and return any reused Postgres base types (as +-- scalars). validateCustomTypeDefinitions :: (MonadValidate [CustomTypeValidationError] m) => TableCache -> CustomTypes - -> [PGScalarType] -- ^ List of all Postgres base types - -> m (Set.HashSet PGScalarType) -- ^ Reused Postgres base types (as scalars) in custom types -validateCustomTypeDefinitions tableCache customTypes allPGScalars = do + -> HashSet PGScalarType -- ^ all Postgres base types + -> m (HashSet PGScalarType) -- ^ see Note [Postgres scalars in custom types] +validateCustomTypeDefinitions tableCache customTypes allPGScalars = execWriterT do unless (null duplicateTypes) $ dispute $ pure $ DuplicateTypeNames duplicateTypes traverse_ validateEnum enumDefinitions - -- The following validations also accumulates reused Postgres scalar types - -- in a 'WriterT' monad. Using 'execWriterT' to peel of the monad and collect - -- Postgres scalar types. - Writer.execWriterT $ do - traverse_ validateInputObject inputObjectDefinitions - traverse_ validateObject objectDefinitions + traverse_ validateInputObject inputObjectDefinitions + traverse_ validateObject objectDefinitions where inputObjectDefinitions = fromMaybe [] $ _ctInputObjects customTypes objectDefinitions = fromMaybe [] $ _ctObjects customTypes @@ -122,15 +118,13 @@ validateCustomTypeDefinitions tableCache customTypes allPGScalars = do -- check that fields reference input types for_ (_iotdFields inputObjectDefinition) $ \inputObjectField -> do let fieldBaseType = G.getBaseType $ unGraphQLType $ _iofdType inputObjectField - if Set.member fieldBaseType inputTypes then pure () - else - -- Check if field type uses any Postgres scalar type and return the same - case findReusedPGScalar fieldBaseType of - Just reusedScalar -> Writer.tell $ Set.singleton reusedScalar - Nothing -> - refute $ pure $ InputObjectFieldTypeDoesNotExist - (_iotdName inputObjectDefinition) - (_iofdName inputObjectField) fieldBaseType + if | Set.member fieldBaseType inputTypes -> pure () + | Just pgScalar <- lookupPGScalar fieldBaseType -> + tell $ Set.singleton pgScalar + | otherwise -> + refute $ pure $ InputObjectFieldTypeDoesNotExist + (_iotdName inputObjectDefinition) + (_iofdName inputObjectField) fieldBaseType validateObject :: ( MonadValidate [CustomTypeValidationError] m @@ -171,13 +165,11 @@ validateCustomTypeDefinitions tableCache customTypes allPGScalars = do | Set.member fieldBaseType objectTypes -> dispute $ pure $ ObjectFieldObjectBaseType objectTypeName fieldName fieldBaseType + | Just pgScalar <- lookupPGScalar fieldBaseType -> + tell $ Set.singleton pgScalar | otherwise -> - -- Check if field type uses any Postgres scalar type and return the same - case findReusedPGScalar fieldBaseType of - Just ty -> Writer.tell $ Set.singleton ty - Nothing -> - dispute $ pure $ ObjectFieldTypeDoesNotExist - objectTypeName fieldName fieldBaseType + dispute $ pure $ ObjectFieldTypeDoesNotExist + objectTypeName fieldName fieldBaseType -- collect all non list scalar types of this object if (not (isListType fieldType) && Set.member fieldBaseType scalarTypes) @@ -208,7 +200,7 @@ validateCustomTypeDefinitions tableCache customTypes allPGScalars = do objectTypeName relationshipName remoteTable columnName return () - findReusedPGScalar baseType = + lookupPGScalar baseType = -- see Note [Postgres scalars in custom types] find ((==) baseType . mkScalarTy) allPGScalars data CustomTypeValidationError @@ -316,11 +308,11 @@ clearCustomTypes = do resolveCustomTypes :: (MonadError QErr m) - => TableCache -> CustomTypes -> [PGScalarType] -> m (NonObjectTypeMap, AnnotatedObjects) + => TableCache -> CustomTypes -> HashSet PGScalarType -> m (NonObjectTypeMap, AnnotatedObjects) resolveCustomTypes tableCache customTypes allPGScalars = do reusedPGScalars <- either (throw400 ConstraintViolation . showErrors) pure =<< runValidateT (validateCustomTypeDefinitions tableCache customTypes allPGScalars) - buildCustomTypesSchemaPartial tableCache customTypes $ toList reusedPGScalars + buildCustomTypesSchemaPartial tableCache customTypes reusedPGScalars where showErrors :: [CustomTypeValidationError] -> T.Text showErrors allErrors = diff --git a/server/src-lib/Hasura/RQL/Types/Catalog.hs b/server/src-lib/Hasura/RQL/Types/Catalog.hs index e84d6c823e90b..e69b6208b90bd 100644 --- a/server/src-lib/Hasura/RQL/Types/Catalog.hs +++ b/server/src-lib/Hasura/RQL/Types/Catalog.hs @@ -143,14 +143,16 @@ $(deriveFromJSON (aesonDrop 3 snakeCase) ''CatalogFunction) data CatalogCustomTypes = CatalogCustomTypes { _cctCustomTypes :: !CustomTypes - , _cctPgScalars :: ![PGScalarType] + , _cctPgScalars :: !(HashSet PGScalarType) -- ^ All Postgres base types, which may be referenced in custom type definitions. -- When we validate the custom types (see 'validateCustomTypeDefinitions'), -- we record which base types were referenced so that we can be sure to include them -- in the generated GraphQL schema. + -- -- These are not actually part of the Hasura metadata --- we fetch them from -- @pg_catalog.pg_type@ --- but they’re needed when validating the custom type -- metadata, so we include them here. + -- -- See Note [Postgres scalars in custom types] for more details. } deriving (Show, Eq, Generic) instance NFData CatalogCustomTypes From a154ff3b9f37515e0350fcd90f4009dd637783a6 Mon Sep 17 00:00:00 2001 From: Alexis King Date: Thu, 9 Apr 2020 06:07:10 -0500 Subject: [PATCH 10/14] Pull buildActions out of buildAndCollectInfo, use buildInfoMap --- server/src-lib/Hasura/RQL/DDL/Schema/Cache.hs | 53 ++++++++++++------- 1 file changed, 33 insertions(+), 20 deletions(-) diff --git a/server/src-lib/Hasura/RQL/DDL/Schema/Cache.hs b/server/src-lib/Hasura/RQL/DDL/Schema/Cache.hs index bc82a13ede369..2d5a553085937 100644 --- a/server/src-lib/Hasura/RQL/DDL/Schema/Cache.hs +++ b/server/src-lib/Hasura/RQL/DDL/Schema/Cache.hs @@ -263,26 +263,15 @@ buildSchemaCacheRule = proc (catalogMetadata, invalidationKeys) -> do |) (MetadataObject MOCustomTypes $ toJSON customTypes) -- actions - actionCache <- (mapFromL _amName actions >- returnA) - >-> (| Inc.keyed (\_ action -> do - let ActionMetadata name comment def actionPermissions = action - metadataObj = MetadataObject (MOAction name) $ toJSON $ - CreateAction name def comment - addActionContext e = "in action " <> name <<> "; " <> e - (| withRecordInconsistency ( - (| modifyErrA ( do - resolvedCustomTypes <- - (| onNothingA (throwA -< err400 Unexpected "custom types are inconsistent") - |) maybeResolvedCustomTypes - (resolvedDef, outFields) <- bindErrorA -< resolveAction resolvedCustomTypes def - let permissionInfos = map (ActionPermissionInfo . _apmRole) actionPermissions - permissionMap = mapFromL _apiRole permissionInfos - returnA -< ActionInfo name outFields resolvedDef permissionMap comment - ) - |) addActionContext) - |) metadataObj) - |) - >-> (\actionMap -> returnA -< M.catMaybes actionMap) + actionCache <- case maybeResolvedCustomTypes of + Just resolvedCustomTypes -> buildActions -< (resolvedCustomTypes, actions) + + -- If the custom types themselves are inconsistent, we can’t really do + -- anything with actions, so just mark them all inconsistent. + Nothing -> do + recordInconsistencies -< ( map mkActionMetadataObject actions + , "custom types are inconsistent" ) + returnA -< M.empty -- remote schemas let remoteSchemaInvalidationKeys = Inc.selectD #_ikRemoteSchemas invalidationKeys @@ -304,6 +293,9 @@ buildSchemaCacheRule = proc (catalogMetadata, invalidationKeys) -> do definition = object ["table" .= qt, "configuration" .= configuration] in MetadataObject objectId definition + mkActionMetadataObject (ActionMetadata name comment defn _) = + MetadataObject (MOAction name) (toJSON $ CreateAction name defn comment) + mkRemoteSchemaMetadataObject remoteSchema = MetadataObject (MORemoteSchema (_arsqName remoteSchema)) (toJSON remoteSchema) @@ -362,6 +354,27 @@ buildSchemaCacheRule = proc (catalogMetadata, invalidationKeys) -> do liftTx $ delTriggerQ triggerName -- executes DROP IF EXISTS.. sql mkAllTriggersQ triggerName tableName (M.elems tableColumns) triggerDefinition + buildActions + :: ( ArrowChoice arr, Inc.ArrowDistribute arr, Inc.ArrowCache m arr + , ArrowWriter (Seq CollectedInfo) arr, MonadIO m ) + => ( (NonObjectTypeMap, AnnotatedObjects) + , [ActionMetadata] + ) `arr` HashMap ActionName ActionInfo + buildActions = buildInfoMap _amName mkActionMetadataObject buildAction + where + buildAction = proc (resolvedCustomTypes, action) -> do + let ActionMetadata name comment def actionPermissions = action + addActionContext e = "in action " <> name <<> "; " <> e + (| withRecordInconsistency ( + (| modifyErrA (do + (resolvedDef, outFields) <- liftEitherA <<< bindA -< + runExceptT $ resolveAction resolvedCustomTypes def + let permissionInfos = map (ActionPermissionInfo . _apmRole) actionPermissions + permissionMap = mapFromL _apiRole permissionInfos + returnA -< ActionInfo name outFields resolvedDef permissionMap comment) + |) addActionContext) + |) (mkActionMetadataObject action) + buildRemoteSchemas :: ( ArrowChoice arr, Inc.ArrowDistribute arr, ArrowWriter (Seq CollectedInfo) arr , Inc.ArrowCache m arr , MonadIO m, HasHttpManager m ) From 6f3b8c0c17598d1c1c60ed5056094127911b258f Mon Sep 17 00:00:00 2001 From: Alexis King Date: Thu, 9 Apr 2020 06:15:27 -0500 Subject: [PATCH 11/14] Tweak wording in documentation --- docs/graphql/manual/actions/types/index.rst | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/docs/graphql/manual/actions/types/index.rst b/docs/graphql/manual/actions/types/index.rst index 2d8915a1834b7..c1669df56c378 100644 --- a/docs/graphql/manual/actions/types/index.rst +++ b/docs/graphql/manual/actions/types/index.rst @@ -143,8 +143,8 @@ types and input types. .. admonition:: Postgres scalars - Postgres base types are reusable as custom scalars without explicitly declaring them. - For example, we might have a ``type`` like this: + Postgres base types are implicitly made available as GraphQL scalars; there + is no need to declare them separately. For example, in the definition .. code-block:: graphql @@ -154,7 +154,8 @@ types and input types. location: geography } - The ``uuid`` and ``geography`` types are Postgres scalars, not separately-defined custom GraphQL types. + the ``uuid`` and ``geography`` types are assumed to refer to Postgres + scalars (assuming no other definition for them is provided). Enum types ---------- From 8b08810e32c40541cd79841f31649e0b2d072b9f Mon Sep 17 00:00:00 2001 From: Aleksandra Sikora Date: Thu, 9 Apr 2020 17:20:58 +0200 Subject: [PATCH 12/14] incorporate changes in console code --- .../Services/Actions/Common/utils.js | 4 +-- console/src/shared/utils/deriveAction.js | 33 ++++++++++--------- console/src/shared/utils/sdlUtils.js | 1 + 3 files changed, 20 insertions(+), 18 deletions(-) diff --git a/console/src/components/Services/Actions/Common/utils.js b/console/src/components/Services/Actions/Common/utils.js index 711afaea3e40c..d9406a17b4535 100644 --- a/console/src/components/Services/Actions/Common/utils.js +++ b/console/src/components/Services/Actions/Common/utils.js @@ -229,7 +229,7 @@ export const getActionTypes = (currentAction, allTypes) => { const type = findType(allTypes, typename); actionTypes[typename] = type; - if (type.fields) { + if (type && type.fields) { type.fields.forEach(f => { getDependentTypes(f.type); if (f.arguments) { @@ -266,7 +266,7 @@ export const getOverlappingTypeConfirmation = ( const action = otherActions[i]; const actionTypes = getActionTypes(action, allTypes); actionTypes.forEach(t => { - if (typeCollisionMap[t.name]) return; + if (!t || typeCollisionMap[t.name]) return; overlappingTypenames.forEach(ot => { if (ot === t.name) { typeCollisionMap[ot] = true; diff --git a/console/src/shared/utils/deriveAction.js b/console/src/shared/utils/deriveAction.js index d852843ed008b..04a39d9f72e97 100644 --- a/console/src/shared/utils/deriveAction.js +++ b/console/src/shared/utils/deriveAction.js @@ -110,6 +110,10 @@ const deriveAction = ( const allHasuraTypes = clientSchema._typeMap; const operationType = clientSchema._mutationType; // TODO better handling for queries + const isHasuraScalar = name => { + return allHasuraTypes[name] && isScalarType(allHasuraTypes[name]); + }; + const actionArguments = []; const newTypes = {}; @@ -121,7 +125,7 @@ const deriveAction = ( newType.name = typename; if (isScalarType(type)) { - if (!inbuiltTypes[type.name]) { + if (!inbuiltTypes[type.name] && !allHasuraTypes[type.name]) { newType.kind = 'scalar'; newTypes[typename] = newType; } @@ -149,7 +153,10 @@ const deriveAction = ( type: underLyingType, wraps: fieldTypeWraps, } = getUnderlyingType(tf.type); - if (inbuiltTypes[underLyingType.name]) { + if ( + inbuiltTypes[underLyingType.name] || + isHasuraScalar(underLyingType.name) + ) { _tf.type = wrapTypename(underLyingType.name, fieldTypeWraps); } else { _tf.type = wrapTypename( @@ -170,7 +177,10 @@ const deriveAction = ( name: v.variable.name.value, }; const argTypeMetadata = getAstTypeMetadata(v.type); - if (!inbuiltTypes[argTypeMetadata.typename]) { + if ( + !inbuiltTypes[argTypeMetadata.typename] && + !isHasuraScalar(argTypeMetadata.type) + ) { const argTypename = prefixTypename(argTypeMetadata.typename); generatedArg.type = wrapTypename(argTypename, argTypeMetadata.stack); const typeInSchema = allHasuraTypes[argTypeMetadata.typename]; @@ -201,19 +211,10 @@ const deriveAction = ( outputTypeField => { const fieldTypeMetadata = getUnderlyingType(outputTypeField.type); if (isScalarType(fieldTypeMetadata.type)) { - if (inbuiltTypes[fieldTypeMetadata.type.name]) { - outputTypeFields[outputTypeField.name] = wrapTypename( - fieldTypeMetadata.type.name, - fieldTypeMetadata.wraps - ); - } else { - const fieldTypename = prefixTypename(fieldTypeMetadata.type.name); - outputTypeFields[outputTypeField.name] = wrapTypename( - fieldTypename, - fieldTypeMetadata.wraps - ); - handleType(fieldTypeMetadata.type, fieldTypename); - } + outputTypeFields[outputTypeField.name] = wrapTypename( + fieldTypeMetadata.type.name, + fieldTypeMetadata.wraps + ); } } ); diff --git a/console/src/shared/utils/sdlUtils.js b/console/src/shared/utils/sdlUtils.js index 12f57ab0e90aa..46078bb60e492 100644 --- a/console/src/shared/utils/sdlUtils.js +++ b/console/src/shared/utils/sdlUtils.js @@ -224,6 +224,7 @@ ${enumValuesSdl.join('\n')} }; const getTypeSdl = type => { + if (!type) return ''; switch (type.kind) { case 'scalar': return getScalarTypeSdl(type); From 73aeea1e2342ed9996a30a3b5f5cff9292694626 Mon Sep 17 00:00:00 2001 From: rakeshkky <12475069+rakeshkky@users.noreply.github.com> Date: Fri, 10 Apr 2020 12:44:41 +0530 Subject: [PATCH 13/14] account Postgres scalars for action input arguments -> Avoid unnecessary 'throw500' in making action schema --- .../src-lib/Hasura/GraphQL/Resolve/Action.hs | 2 +- server/src-lib/Hasura/GraphQL/Schema.hs | 6 +- .../src-lib/Hasura/GraphQL/Schema/Action.hs | 72 +++++++++---------- server/src-lib/Hasura/RQL/DDL/Action.hs | 70 +++++++++++++----- server/src-lib/Hasura/RQL/DDL/Schema/Cache.hs | 14 ++-- server/src-lib/Hasura/RQL/Types/Action.hs | 12 +++- .../custom-types/create_action_pg_scalar.yaml | 28 ++++++++ .../queries/actions/custom-types/setup.yaml | 8 +++ server/tests-py/test_actions.py | 3 + 9 files changed, 143 insertions(+), 72 deletions(-) create mode 100644 server/tests-py/queries/actions/custom-types/create_action_pg_scalar.yaml diff --git a/server/src-lib/Hasura/GraphQL/Resolve/Action.hs b/server/src-lib/Hasura/GraphQL/Resolve/Action.hs index 6f782a179d9a0..f148e0d0f41ed 100644 --- a/server/src-lib/Hasura/GraphQL/Resolve/Action.hs +++ b/server/src-lib/Hasura/GraphQL/Resolve/Action.hs @@ -295,7 +295,7 @@ asyncActionsProcessor cacheRef pgPool httpManager = forever $ do Nothing -> return () Just actionInfo -> do let definition = _aiDefinition actionInfo - outputFields = _aiOutputFields actionInfo + outputFields = getActionOutputFields $ _aiOutputObject actionInfo webhookUrl = _adHandler definition forwardClientHeaders = _adForwardClientHeaders definition confHeaders = _adHeaders definition diff --git a/server/src-lib/Hasura/GraphQL/Schema.hs b/server/src-lib/Hasura/GraphQL/Schema.hs index 151e5602d8c13..6e80216c7be2e 100644 --- a/server/src-lib/Hasura/GraphQL/Schema.hs +++ b/server/src-lib/Hasura/GraphQL/Schema.hs @@ -720,11 +720,11 @@ noFilter = annBoolExpTrue mkGCtxMap :: forall m. (MonadError QErr m) - => AnnotatedObjects -> TableCache -> FunctionCache -> ActionCache -> m GCtxMap -mkGCtxMap annotatedObjects tableCache functionCache actionCache = do + => TableCache -> FunctionCache -> ActionCache -> m GCtxMap +mkGCtxMap tableCache functionCache actionCache = do typesMapL <- mapM (mkGCtxMapTable tableCache functionCache) $ filter (tableFltr . _tiCoreInfo) $ Map.elems tableCache - actionsSchema <- mkActionsSchema annotatedObjects actionCache + let actionsSchema = mkActionsSchema actionCache typesMap <- combineTypes actionsSchema typesMapL let gCtxMap = flip Map.map typesMap $ \(ty, flds, insCtxMap) -> mkGCtx ty flds insCtxMap diff --git a/server/src-lib/Hasura/GraphQL/Schema/Action.hs b/server/src-lib/Hasura/GraphQL/Schema/Action.hs index f39df90b95d9c..1f4b79921089e 100644 --- a/server/src-lib/Hasura/GraphQL/Schema/Action.hs +++ b/server/src-lib/Hasura/GraphQL/Schema/Action.hs @@ -3,6 +3,7 @@ module Hasura.GraphQL.Schema.Action ) where import qualified Data.HashMap.Strict as Map +import qualified Data.HashSet as Set import qualified Language.GraphQL.Draft.Syntax as G import Data.Coerce (coerce) @@ -68,14 +69,14 @@ mkMutationField actionName actionInfo definitionList = ActionSynchronous -> ActionExecutionSyncWebhook $ SyncActionExecutionContext actionName (_adOutputType definition) - (_aiOutputFields actionInfo) + (getActionOutputFields $ _aiOutputObject actionInfo) definitionList (_adHandler definition) (_adHeaders definition) (_adForwardClientHeaders definition) ActionAsynchronous -> ActionExecutionAsync - description = mkDescriptionWith (PGDescription <$> (_aiComment actionInfo)) $ + description = mkDescriptionWith (PGDescription <$> _aiComment actionInfo) $ "perform the action: " <>> actionName fieldInfo = @@ -123,22 +124,21 @@ mkQueryField actionName comment definition definitionList = idDescription = G.Description $ "id of the action: " <>> actionName mkActionFieldsAndTypes - :: (QErrM m) - => ActionInfo - -> AnnotatedObjectType + :: ActionInfo -> ActionPermissionInfo - -> m ( Maybe (ActionSelectOpContext, ObjFldInfo, TypeInfo) + -> ( Maybe (ActionSelectOpContext, ObjFldInfo, TypeInfo) -- context, field, response type info , (ActionExecutionContext, ObjFldInfo) -- mutation field , FieldMap ) -mkActionFieldsAndTypes actionInfo annotatedOutputType permission = - return ( mkQueryField actionName comment definition definitionList - , mkMutationField actionName actionInfo definitionList - , fieldMap - ) +mkActionFieldsAndTypes actionInfo permission = + ( mkQueryField actionName comment definition definitionList + , mkMutationField actionName actionInfo definitionList + , fieldMap + ) where actionName = _aiName actionInfo + annotatedOutputType = _aiOutputObject actionInfo definition = _aiDefinition actionInfo roleName = _apiRole permission comment = _aiComment actionInfo @@ -220,46 +220,38 @@ mkActionFieldsAndTypes actionInfo annotatedOutputType permission = G.getBaseType $ unGraphQLType $ _adOutputType $ _aiDefinition actionInfo mkActionSchemaOne - :: (QErrM m) - => AnnotatedObjects - -> ActionInfo - -> m (Map.HashMap RoleName - ( Maybe (ActionSelectOpContext, ObjFldInfo, TypeInfo) - , (ActionExecutionContext, ObjFldInfo) - , FieldMap - ) + :: ActionInfo + -> Map.HashMap RoleName + ( Maybe (ActionSelectOpContext, ObjFldInfo, TypeInfo) + , (ActionExecutionContext, ObjFldInfo) + , FieldMap ) -mkActionSchemaOne annotatedObjects actionInfo = do - annotatedOutputType <- onNothing - (Map.lookup (ObjectTypeName actionOutputBaseType) annotatedObjects) $ - throw500 $ "missing annotated type for: " <> showNamedTy actionOutputBaseType - forM permissions $ \permission -> - mkActionFieldsAndTypes actionInfo annotatedOutputType permission +mkActionSchemaOne actionInfo = + flip Map.map permissions $ \permission -> + mkActionFieldsAndTypes actionInfo permission where adminPermission = ActionPermissionInfo adminRole permissions = Map.insert adminRole adminPermission $ _aiPermissions actionInfo - actionOutputBaseType = - G.getBaseType $ unGraphQLType $ _adOutputType $ _aiDefinition actionInfo mkActionsSchema - :: (QErrM m) - => AnnotatedObjects - -> ActionCache - -> m (Map.HashMap RoleName (RootFields, TyAgg)) -mkActionsSchema annotatedObjects = - foldM + :: ActionCache + -> Map.HashMap RoleName (RootFields, TyAgg) +mkActionsSchema = + foldl' (\aggregate actionInfo -> - Map.foldrWithKey f aggregate <$> - mkActionSchemaOne annotatedObjects actionInfo + Map.foldrWithKey (accumulate (_aiPgScalars actionInfo)) aggregate $ mkActionSchemaOne actionInfo ) mempty where -- we'll need to add uuid and timestamptz for actions - newRoleState = (mempty, addScalarToTyAgg PGJSON $ - addScalarToTyAgg PGTimeStampTZ $ - addScalarToTyAgg PGUUID mempty) - f roleName (queryFieldM, mutationField, fields) = - Map.alter (Just . addToState . fromMaybe newRoleState) roleName + mkNewRoleState pgScalars = + ( mempty + , foldr addScalarToTyAgg mempty $ + pgScalars <> Set.fromList [PGJSON, PGTimeStampTZ, PGUUID] + ) + + accumulate pgScalars roleName (queryFieldM, mutationField, fields) = + Map.alter (Just . addToState . fromMaybe (mkNewRoleState pgScalars)) roleName where addToState = case queryFieldM of Just (fldCtx, fldDefinition, responseTypeInfo) -> diff --git a/server/src-lib/Hasura/RQL/DDL/Action.hs b/server/src-lib/Hasura/RQL/DDL/Action.hs index 84e7200f63fed..d1868dee4f44e 100644 --- a/server/src-lib/Hasura/RQL/DDL/Action.hs +++ b/server/src-lib/Hasura/RQL/DDL/Action.hs @@ -36,6 +36,7 @@ import qualified Data.Aeson as J import qualified Data.Aeson.Casing as J import qualified Data.Aeson.TH as J import qualified Data.HashMap.Strict as Map +import qualified Data.HashSet as Set import qualified Data.Text as T import qualified Database.PG.Query as Q import qualified Language.GraphQL.Draft.Syntax as G @@ -77,36 +78,67 @@ persistCreateAction (CreateAction actionName actionDefinition comment) = do VALUES ($1, $2, $3) |] (actionName, Q.AltJ actionDefinition, comment) True +{- Note [Postgres scalars in action input arguments] +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +It's very comfortable to be able to reference Postgres scalars in actions +input arguments. For example, see the following action mutation: + + extend type mutation_root { + create_user ( + name: String! + created_at: timestamptz + ): User + } + +The timestamptz is a Postgres scalar. We need to validate the presence of +timestamptz type in the Postgres database. So, the 'resolveAction' function +takes all Postgres scalar types as one of the inputs and returns the set of +referred scalars. +-} + resolveAction :: (QErrM m, MonadIO m) => (NonObjectTypeMap, AnnotatedObjects) + -> HashSet PGScalarType -- ^ List of all Postgres scalar types. -> ActionDefinitionInput - -> m (ResolvedActionDefinition, ActionOutputFields) -resolveAction customTypes actionDefinition = do + -> m ( ResolvedActionDefinition + , AnnotatedObjectType + , HashSet PGScalarType -- ^ see Note [Postgres scalars in action input arguments]. + ) +resolveAction customTypes allPGScalars actionDefinition = do let responseType = unGraphQLType $ _adOutputType actionDefinition responseBaseType = G.getBaseType responseType - forM (_adArguments actionDefinition) $ \argument -> do - let argumentBaseType = G.getBaseType $ unGraphQLType $ _argType argument - argTypeInfo <- getNonObjectTypeInfo argumentBaseType - case argTypeInfo of - VT.TIScalar _ -> return () - VT.TIEnum _ -> return () - VT.TIInpObj _ -> return () - _ -> throw400 InvalidParams $ "the argument's base type: " - <> showNamedTy argumentBaseType <> - " should be a scalar/enum/input_object" + + reusedPGScalars <- execWriterT $ + forM (_adArguments actionDefinition) $ \argument -> do + let argumentBaseType = G.getBaseType $ unGraphQLType $ _argType argument + maybeArgTypeInfo = getNonObjectTypeInfo argumentBaseType + maybePGScalar = find ((==) argumentBaseType . VT.mkScalarTy) allPGScalars + + if | Just argTypeInfo <- maybeArgTypeInfo -> + case argTypeInfo of + VT.TIScalar _ -> pure () + VT.TIEnum _ -> pure () + VT.TIInpObj _ -> pure () + _ -> throw400 InvalidParams $ "the argument's base type: " + <> showNamedTy argumentBaseType <> + " should be a scalar/enum/input_object" + -- Collect the referred Postgres scalar. See Note [Postgres scalars in action input arguments]. + | Just pgScalar <- maybePGScalar -> tell $ Set.singleton pgScalar + | Nothing <- maybeArgTypeInfo -> + throw400 NotExists $ "the type: " <> showNamedTy argumentBaseType + <> " is not defined in custom types" + | otherwise -> pure () + -- Check if the response type is an object - annFields <- _aotAnnotatedFields <$> getObjectTypeInfo responseBaseType - let outputFields = Map.fromList $ map (unObjectFieldName *** fst) $ Map.toList annFields + outputObject <- getObjectTypeInfo responseBaseType resolvedDef <- traverse resolveWebhook actionDefinition - pure (resolvedDef, outputFields) + pure (resolvedDef, outputObject, reusedPGScalars) where - getNonObjectTypeInfo typeName = do + getNonObjectTypeInfo typeName = let nonObjectTypeMap = unNonObjectTypeMap $ fst $ customTypes inputTypeInfos = nonObjectTypeMap <> mapFromL VT.getNamedTy defaultTypes - onNothing (Map.lookup typeName inputTypeInfos) $ - throw400 NotExists $ "the type: " <> showNamedTy typeName <> - " is not defined in custom types" + in Map.lookup typeName inputTypeInfos resolveWebhook (InputWebhook urlTemplate) = do eitherRenderedTemplate <- renderURLTemplate urlTemplate diff --git a/server/src-lib/Hasura/RQL/DDL/Schema/Cache.hs b/server/src-lib/Hasura/RQL/DDL/Schema/Cache.hs index 2d5a553085937..1a4f7790c5451 100644 --- a/server/src-lib/Hasura/RQL/DDL/Schema/Cache.hs +++ b/server/src-lib/Hasura/RQL/DDL/Schema/Cache.hs @@ -264,7 +264,7 @@ buildSchemaCacheRule = proc (catalogMetadata, invalidationKeys) -> do -- actions actionCache <- case maybeResolvedCustomTypes of - Just resolvedCustomTypes -> buildActions -< (resolvedCustomTypes, actions) + Just resolvedCustomTypes -> buildActions -< ((resolvedCustomTypes, pgScalars), actions) -- If the custom types themselves are inconsistent, we can’t really do -- anything with actions, so just mark them all inconsistent. @@ -357,21 +357,21 @@ buildSchemaCacheRule = proc (catalogMetadata, invalidationKeys) -> do buildActions :: ( ArrowChoice arr, Inc.ArrowDistribute arr, Inc.ArrowCache m arr , ArrowWriter (Seq CollectedInfo) arr, MonadIO m ) - => ( (NonObjectTypeMap, AnnotatedObjects) + => ( ((NonObjectTypeMap, AnnotatedObjects), HashSet PGScalarType) , [ActionMetadata] ) `arr` HashMap ActionName ActionInfo buildActions = buildInfoMap _amName mkActionMetadataObject buildAction where - buildAction = proc (resolvedCustomTypes, action) -> do + buildAction = proc ((resolvedCustomTypes, pgScalars), action) -> do let ActionMetadata name comment def actionPermissions = action addActionContext e = "in action " <> name <<> "; " <> e (| withRecordInconsistency ( (| modifyErrA (do - (resolvedDef, outFields) <- liftEitherA <<< bindA -< - runExceptT $ resolveAction resolvedCustomTypes def + (resolvedDef, outObject, reusedPgScalars) <- liftEitherA <<< bindA -< + runExceptT $ resolveAction resolvedCustomTypes pgScalars def let permissionInfos = map (ActionPermissionInfo . _apmRole) actionPermissions permissionMap = mapFromL _apiRole permissionInfos - returnA -< ActionInfo name outFields resolvedDef permissionMap comment) + returnA -< ActionInfo name outObject resolvedDef permissionMap reusedPgScalars comment) |) addActionContext) |) (mkActionMetadataObject action) @@ -406,7 +406,7 @@ buildSchemaCacheRule = proc (catalogMetadata, invalidationKeys) -> do , ActionCache ) `arr` (RemoteSchemaMap, GS.GCtxMap, GS.GCtx) buildGQLSchema = proc (tableCache, functionCache, remoteSchemas, customTypes, actionCache) -> do - baseGQLSchema <- bindA -< GS.mkGCtxMap (snd customTypes) tableCache functionCache actionCache + baseGQLSchema <- bindA -< GS.mkGCtxMap tableCache functionCache actionCache (| foldlA' (\(remoteSchemaMap, gqlSchemas, remoteGQLSchemas) (remoteSchemaName, (remoteSchema, metadataObject)) -> (| withRecordInconsistency (do diff --git a/server/src-lib/Hasura/RQL/Types/Action.hs b/server/src-lib/Hasura/RQL/Types/Action.hs index 82c268a332435..1b8d08026e0ea 100644 --- a/server/src-lib/Hasura/RQL/Types/Action.hs +++ b/server/src-lib/Hasura/RQL/Types/Action.hs @@ -14,10 +14,12 @@ module Hasura.RQL.Types.Action , ResolvedActionDefinition , ActionOutputFields + , getActionOutputFields , ActionInfo(..) , aiName - , aiOutputFields + , aiOutputObject , aiDefinition + , aiPgScalars , aiPermissions , aiComment , ActionPermissionInfo(..) @@ -119,14 +121,20 @@ data ActionPermissionInfo $(J.deriveToJSON (J.aesonDrop 4 J.snakeCase) ''ActionPermissionInfo) type ActionPermissionMap = Map.HashMap RoleName ActionPermissionInfo + type ActionOutputFields = Map.HashMap G.Name G.GType +getActionOutputFields :: AnnotatedObjectType -> ActionOutputFields +getActionOutputFields = + Map.fromList . map (unObjectFieldName *** fst) . Map.toList . _aotAnnotatedFields + data ActionInfo = ActionInfo { _aiName :: !ActionName - , _aiOutputFields :: !ActionOutputFields + , _aiOutputObject :: !AnnotatedObjectType , _aiDefinition :: !ResolvedActionDefinition , _aiPermissions :: !ActionPermissionMap + , _aiPgScalars :: !(HashSet PGScalarType) , _aiComment :: !(Maybe Text) } deriving (Show, Eq) $(J.deriveToJSON (J.aesonDrop 3 J.snakeCase) ''ActionInfo) diff --git a/server/tests-py/queries/actions/custom-types/create_action_pg_scalar.yaml b/server/tests-py/queries/actions/custom-types/create_action_pg_scalar.yaml new file mode 100644 index 0000000000000..89fab236377bd --- /dev/null +++ b/server/tests-py/queries/actions/custom-types/create_action_pg_scalar.yaml @@ -0,0 +1,28 @@ +- description: Create an action with PG scalars in input arguments + url: /v1/query + status: 200 + response: + message: success + query: + type: create_action + args: + name: some_action + definition: + kind: synchronous + arguments: + - name: user_id + type: ID! + - name: location + type: geography! + output_type: User! + handler: http://127.0.0.1:5593/create-user + +- description: Remove action + url: /v1/query + status: 200 + response: + message: success + query: + type: drop_action + args: + name: some_action diff --git a/server/tests-py/queries/actions/custom-types/setup.yaml b/server/tests-py/queries/actions/custom-types/setup.yaml index 93be265c95bd5..dfdecff8bcc0c 100644 --- a/server/tests-py/queries/actions/custom-types/setup.yaml +++ b/server/tests-py/queries/actions/custom-types/setup.yaml @@ -17,3 +17,11 @@ args: name TEXT NOT NULL, location geography ); + +- type: set_custom_types + args: + objects: + - name: User + fields: + - name: user_id + type: ID! diff --git a/server/tests-py/test_actions.py b/server/tests-py/test_actions.py index f2f240f710ff1..28dde77e47566 100644 --- a/server/tests-py/test_actions.py +++ b/server/tests-py/test_actions.py @@ -310,3 +310,6 @@ def test_resuse_pgscalars(self, hge_ctx): def test_resuse_unknown_pgscalar(self, hge_ctx): check_query_f(hge_ctx, self.dir() + '/reuse_unknown_pgscalar.yaml') + + def test_create_action_pg_scalar(self, hge_ctx): + check_query_f(hge_ctx, self.dir() + '/create_action_pg_scalar.yaml') From 37297d316de7f8f7e068141d22cca7d3cd19ad3d Mon Sep 17 00:00:00 2001 From: Aleksandra Sikora Date: Fri, 10 Apr 2020 11:46:25 +0200 Subject: [PATCH 14/14] Review changes --- console/src/shared/utils/deriveAction.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/console/src/shared/utils/deriveAction.js b/console/src/shared/utils/deriveAction.js index 04a39d9f72e97..a236b432d5888 100644 --- a/console/src/shared/utils/deriveAction.js +++ b/console/src/shared/utils/deriveAction.js @@ -111,7 +111,7 @@ const deriveAction = ( const operationType = clientSchema._mutationType; // TODO better handling for queries const isHasuraScalar = name => { - return allHasuraTypes[name] && isScalarType(allHasuraTypes[name]); + return isScalarType(allHasuraTypes[name]); }; const actionArguments = []; @@ -179,7 +179,7 @@ const deriveAction = ( const argTypeMetadata = getAstTypeMetadata(v.type); if ( !inbuiltTypes[argTypeMetadata.typename] && - !isHasuraScalar(argTypeMetadata.type) + !isHasuraScalar(argTypeMetadata.typename) ) { const argTypename = prefixTypename(argTypeMetadata.typename); generatedArg.type = wrapTypename(argTypename, argTypeMetadata.stack);