From ecb90c47a1152a0be87b036b0b223f56bf7f288b Mon Sep 17 00:00:00 2001 From: Antoine Leblanc Date: Tue, 18 May 2021 09:47:55 +0100 Subject: [PATCH] server: fix action output fields return type (fix hasura/graphql-engine#6631) GitOrigin-RevId: 9be5ac87fe69c55aafb6b5af918eb4deb97e27d5 --- CHANGELOG.md | 4 +- .../Hasura/GraphQL/Parser/Internal/Parser.hs | 17 ++-- .../src-lib/Hasura/GraphQL/Schema/Action.hs | 44 ++++----- .../src-lib/Hasura/GraphQL/Schema/Remote.hs | 33 +++---- .../src-lib/Hasura/RQL/Types/CustomTypes.hs | 7 +- .../introspection/output_types_query.yaml | 94 +++++++++++++++++++ .../queries/actions/introspection/setup.yaml | 4 +- server/tests-py/test_actions.py | 5 +- 8 files changed, 146 insertions(+), 62 deletions(-) create mode 100644 server/tests-py/queries/actions/introspection/output_types_query.yaml diff --git a/CHANGELOG.md b/CHANGELOG.md index f622e1a298e64..48ea33cfa85f3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,8 +6,8 @@ (Add entries below in the order of: server, console, cli, docs, others) -- server: introduce `--graceful-shutdown-timeout` server config which will wait for the in-flight scheduled and event trigger - events and async actions to complete before shutting down +- server: fix erroneous schema type for action output fields (fix #6631) +- server: introduce `--graceful-shutdown-timeout` server config which will wait for the in-flight scheduled and event trigger events and async actions to complete before shutting down - server: fix a regression from V1 and allow string values for most Postgres column types - server: sanitise event trigger and scheduled trigger logs to omit possibly sensitive request body and headers - server: fix parsing of values for Postgres char columns (fix #6814) diff --git a/server/src-lib/Hasura/GraphQL/Parser/Internal/Parser.hs b/server/src-lib/Hasura/GraphQL/Parser/Internal/Parser.hs index 72397e7dbf995..633d4cf845b7a 100644 --- a/server/src-lib/Hasura/GraphQL/Parser/Internal/Parser.hs +++ b/server/src-lib/Hasura/GraphQL/Parser/Internal/Parser.hs @@ -261,15 +261,11 @@ nonNullableField (FieldParser (Definition n u d (FieldInfo as t)) p) = nullableField :: forall m a . FieldParser m a -> FieldParser m a nullableField (FieldParser (Definition n u d (FieldInfo as t)) p) = FieldParser (Definition n u d (FieldInfo as (nullableType t))) p -{- -field = field - { fDefinition = (fDefinition field) - { dInfo = (dInfo (fDefinition field)) - { fType = nonNullableType (fType (dInfo (fDefinition field))) - } - } - } --} + +multipleField :: forall m a. FieldParser m a -> FieldParser m a +multipleField (FieldParser (Definition n u d (FieldInfo as t)) p) = + FieldParser (Definition n u d (FieldInfo as (Nullable (TList t)))) p + -- | Decorate a schema output type as NON_NULL nonNullableParser :: forall m a . Parser 'Output m a -> Parser 'Output m a nonNullableParser parser = parser { pType = nonNullableType (pType parser) } @@ -278,9 +274,10 @@ nonNullableParser parser = parser { pType = nonNullableType (pType parser) } nullableParser :: forall m a . Parser 'Output m a -> Parser 'Output m a nullableParser parser = parser { pType = nullableType (pType parser) } -multiple :: Parser 'Output m a -> Parser 'Output m a +multiple :: forall m a . Parser 'Output m a -> Parser 'Output m a multiple parser = parser { pType = Nullable $ TList $ pType parser } + list :: forall k m a. (MonadParse m, 'Input <: k) => Parser k m a -> Parser k m [a] list parser = gcastWith (inputParserInput @k) Parser { pType = schemaType diff --git a/server/src-lib/Hasura/GraphQL/Schema/Action.hs b/server/src-lib/Hasura/GraphQL/Schema/Action.hs index 1e2403b0b2164..10493dcf13640 100644 --- a/server/src-lib/Hasura/GraphQL/Schema/Action.hs +++ b/server/src-lib/Hasura/GraphQL/Schema/Action.hs @@ -175,31 +175,39 @@ actionOutputFields -> m (Parser 'Output n (RQL.AnnFieldsG ('Postgres 'Vanilla) (UnpreparedValue ('Postgres 'Vanilla)))) actionOutputFields outputType annotatedObject = do let outputObject = _aotDefinition annotatedObject - scalarOrEnumFields = map scalarOrEnumFieldParser $ toList $ _otdFields outputObject + scalarOrEnumFields = map outputFieldParser $ toList $ _otdFields outputObject relationshipFields <- forM (_otdRelationships outputObject) $ traverse relationshipFieldParser let allFieldParsers = scalarOrEnumFields <> maybe [] (concat . catMaybes . toList) relationshipFields outputTypeName = unObjectTypeName $ _otdName outputObject outputTypeDescription = _otdDescription outputObject - pure $ mkOutputParserModifier outputType $ + pure $ outputParserModifier outputType $ P.selectionSet outputTypeName outputTypeDescription allFieldParsers <&> parsedSelectionsToFields RQL.AFExpression where - scalarOrEnumFieldParser + outputParserModifier :: G.GType -> Parser 'Output n a -> Parser 'Output n a + outputParserModifier = \case + G.TypeNamed (G.Nullability True) _ -> P.nullableParser + G.TypeNamed (G.Nullability False) _ -> P.nonNullableParser + G.TypeList (G.Nullability True) t -> P.nullableParser . P.multiple . outputParserModifier t + G.TypeList (G.Nullability False) t -> P.nonNullableParser . P.multiple . outputParserModifier t + + outputFieldParser :: ObjectFieldDefinition (G.GType, AnnotatedObjectFieldType) -> FieldParser n (RQL.AnnFieldG ('Postgres 'Vanilla) (UnpreparedValue ('Postgres 'Vanilla))) - scalarOrEnumFieldParser (ObjectFieldDefinition name _ description ty) = - let (gType, objectFieldType) = ty - fieldName = unObjectFieldName name - -- FIXME? (from master) - pgColumnInfo = ColumnInfo (unsafePGCol $ G.unName fieldName) - fieldName 0 (ColumnScalar PGJSON) (G.isNullable gType) Nothing - fieldParser = case objectFieldType of + outputFieldParser (ObjectFieldDefinition name _ description (gType, objectFieldType)) = + let fieldName = unObjectFieldName name + selection = P.selection_ fieldName description $ case objectFieldType of AOFTScalar def -> customScalarParser def - AOFTEnum def -> customEnumParser def - in bool P.nonNullableField id (G.isNullable gType) $ - P.selection_ (unObjectFieldName name) description fieldParser - $> RQL.mkAnnColumnField pgColumnInfo Nothing Nothing + AOFTEnum def -> customEnumParser def + fieldParser = \case + G.TypeNamed (G.Nullability True) _ -> P.nullableField selection + G.TypeNamed (G.Nullability False) _ -> P.nonNullableField selection + G.TypeList (G.Nullability True) t -> P.nullableField $ P.multipleField $ fieldParser t + G.TypeList (G.Nullability False) t -> P.nonNullableField $ P.multipleField $ fieldParser t + pgColumnInfo = + ColumnInfo (unsafePGCol $ G.unName fieldName) fieldName 0 (ColumnScalar PGJSON) (G.isNullable gType) Nothing + in fieldParser gType $> RQL.mkAnnColumnField pgColumnInfo Nothing Nothing relationshipFieldParser :: TypeRelationship (TableInfo ('Postgres 'Vanilla)) (ColumnInfo ('Postgres 'Vanilla)) @@ -234,14 +242,6 @@ actionOutputFields outputType annotatedObject = do , fmap (RQL.AFArrayRelation . RQL.ASAggregate . RQL.AnnRelationSelectG tableRelName columnMapping) <$> tableAggField ] -mkOutputParserModifier :: G.GType -> Parser 'Output m a -> Parser 'Output m a -mkOutputParserModifier = \case - G.TypeNamed nullable _ -> nullableModifier nullable - G.TypeList nullable ty -> - nullableModifier nullable . P.multiple . mkOutputParserModifier ty - where - nullableModifier nullable = - if G.unNullability nullable then P.nullableParser else P.nonNullableParser mkDefinitionList :: AnnotatedObjectType -> [(PGCol, ScalarType ('Postgres 'Vanilla))] mkDefinitionList AnnotatedObjectType{..} = diff --git a/server/src-lib/Hasura/GraphQL/Schema/Remote.hs b/server/src-lib/Hasura/GraphQL/Schema/Remote.hs index 96bde2d204820..b0367deb33c8a 100644 --- a/server/src-lib/Hasura/GraphQL/Schema/Remote.hs +++ b/server/src-lib/Hasura/GraphQL/Schema/Remote.hs @@ -83,29 +83,18 @@ remoteField' => RemoteSchemaIntrospection -> RemoteSchemaFieldDefinition -> m (FieldParser n (Field NoFragments RemoteSchemaVariable)) -remoteField' schemaDoc (G.FieldDefinition description name argsDefinition gType _) = - let - addNullableList :: FieldParser n (Field NoFragments RemoteSchemaVariable) -> FieldParser n (Field NoFragments RemoteSchemaVariable) - addNullableList (P.FieldParser (Definition name' un desc (FieldInfo args typ)) parser) - = P.FieldParser (Definition name' un desc (FieldInfo args (Nullable (TList typ)))) parser - - addNonNullableList :: FieldParser n (Field NoFragments RemoteSchemaVariable) -> FieldParser n (Field NoFragments RemoteSchemaVariable) - addNonNullableList (P.FieldParser (Definition name' un desc (FieldInfo args typ)) parser) - = P.FieldParser (Definition name' un desc (FieldInfo args (NonNullable (TList typ)))) parser - +remoteField' schemaDoc (G.FieldDefinition description name argsDefinition gType _) = convertType gType + where -- TODO add directives, deprecation - convertType :: G.GType -> m (FieldParser n (Field NoFragments RemoteSchemaVariable)) - convertType gType' = do - case gType' of - G.TypeNamed (Nullability True) fieldTypeName -> - P.nullableField <$> remoteFieldFromName schemaDoc name description fieldTypeName argsDefinition - G.TypeList (Nullability True) gType'' -> - addNullableList <$> convertType gType'' - G.TypeNamed (Nullability False) fieldTypeName -> do - P.nonNullableField <$> remoteFieldFromName schemaDoc name description fieldTypeName argsDefinition - G.TypeList (Nullability False) gType'' -> - addNonNullableList <$> convertType gType'' - in convertType gType + convertType = \case + G.TypeNamed (Nullability True) fieldTypeName -> + P.nullableField <$> remoteFieldFromName schemaDoc name description fieldTypeName argsDefinition + G.TypeNamed (Nullability False) fieldTypeName -> do + P.nonNullableField <$> remoteFieldFromName schemaDoc name description fieldTypeName argsDefinition + G.TypeList (Nullability True) gType' -> + P.nullableField . P.multipleField <$> convertType gType' + G.TypeList (Nullability False) gType' -> + P.nonNullableField . P.multipleField <$> convertType gType' -- | 'remoteSchemaObject' returns a output parser for a given 'ObjectTypeDefinition'. remoteSchemaObject diff --git a/server/src-lib/Hasura/RQL/Types/CustomTypes.hs b/server/src-lib/Hasura/RQL/Types/CustomTypes.hs index 4d957e6b73ea9..08f4c34988a92 100644 --- a/server/src-lib/Hasura/RQL/Types/CustomTypes.hs +++ b/server/src-lib/Hasura/RQL/Types/CustomTypes.hs @@ -33,8 +33,7 @@ module Hasura.RQL.Types.CustomTypes , emptyAnnotatedCustomTypes ) where -import Control.Lens.TH (makeLenses) -import Data.Text.Extended +import Hasura.Prelude import qualified Data.Aeson as J import qualified Data.Aeson.TH as J @@ -46,10 +45,12 @@ import qualified Language.GraphQL.Draft.Printer as GPrint import qualified Language.GraphQL.Draft.Syntax as G import qualified Text.Builder as T +import Control.Lens.TH (makeLenses) +import Data.Text.Extended + import Hasura.Backends.Postgres.Instances.Types () import Hasura.Backends.Postgres.SQL.Types import Hasura.Incremental (Cacheable) -import Hasura.Prelude import Hasura.RQL.Types.Backend import Hasura.RQL.Types.Column import Hasura.RQL.Types.Common diff --git a/server/tests-py/queries/actions/introspection/output_types_query.yaml b/server/tests-py/queries/actions/introspection/output_types_query.yaml new file mode 100644 index 0000000000000..734a6174e07bc --- /dev/null +++ b/server/tests-py/queries/actions/introspection/output_types_query.yaml @@ -0,0 +1,94 @@ +# Test case for bug reported at https://github.com/hasura/graphql-engine/issues/6631 +description: Action introspection to check return types +url: /v1/graphql +status: 200 +query: + query: | + query { + __schema { + mutationType { + fields { + name + type { + name + kind + fields { + name + type { + kind + ofType { + name + kind + ofType { + name + kind + } + } + } + } + ofType { + name + kind + ofType { + name + kind + fields { + name + type { + kind + ofType { + name + kind + ofType { + name + kind + } + } + } + } + } + } + } + } + } + } + } +response: + data: + __schema: + mutationType: + fields: + - name: actionName + type: + kind: LIST + name: + ofType: + kind: NON_NULL + name: + ofType: + kind: OBJECT + name: SampleOutput + fields: + - name: id + type: + kind: LIST + ofType: + kind: NON_NULL + name: + ofType: + kind: SCALAR + name: String + fields: + - name: actionName2 + type: + kind: OBJECT + name: SampleOutput2 + ofType: + fields: + - name: id + type: + kind: NON_NULL + ofType: + kind: SCALAR + name: String + ofType: diff --git a/server/tests-py/queries/actions/introspection/setup.yaml b/server/tests-py/queries/actions/introspection/setup.yaml index 7e508c41abf46..6380afb4fe151 100644 --- a/server/tests-py/queries/actions/introspection/setup.yaml +++ b/server/tests-py/queries/actions/introspection/setup.yaml @@ -8,7 +8,7 @@ args: - name: actionName definition: handler: http://localhost:3000 - output_type: SampleOutput + output_type: "[SampleOutput!]" arguments: - name: text type: jsonb! @@ -39,7 +39,7 @@ args: - name: SampleOutput fields: - name: id - type: String! + type: "[String!]" - name: SampleOutput2 fields: - name: id diff --git a/server/tests-py/test_actions.py b/server/tests-py/test_actions.py index 96a3f4ca52511..8a52bb1611def 100644 --- a/server/tests-py/test_actions.py +++ b/server/tests-py/test_actions.py @@ -500,7 +500,6 @@ def test_recreate_permission(self, hge_ctx): def test_create_with_headers(self, hge_ctx): check_query_f(hge_ctx, self.dir() + '/create_with_headers.yaml') -# Test case for bug reported at https://github.com/hasura/graphql-engine/issues/5166 @pytest.mark.usefixtures('per_class_tests_db_state') class TestActionIntrospection: @@ -518,6 +517,10 @@ def test_introspection_query(self, hge_ctx): assert code == 200, resp assert 'data' in resp, resp + def test_output_types(self, hge_ctx): + check_query_f(hge_ctx, self.dir() + '/output_types_query.yaml') + + @use_action_fixtures class TestActionTimeout: