Skip to content

Commit

Permalink
server: fix action output fields return type (fix hasura#6631)
Browse files Browse the repository at this point in the history
GitOrigin-RevId: 9be5ac8
  • Loading branch information
Antoine Leblanc authored and hasura-bot committed May 18, 2021
1 parent 9dfed5d commit ecb90c4
Show file tree
Hide file tree
Showing 8 changed files with 146 additions and 62 deletions.
4 changes: 2 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
17 changes: 7 additions & 10 deletions server/src-lib/Hasura/GraphQL/Parser/Internal/Parser.hs
Original file line number Diff line number Diff line change
Expand Up @@ -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) }
Expand All @@ -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
Expand Down
44 changes: 22 additions & 22 deletions server/src-lib/Hasura/GraphQL/Schema/Action.hs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down Expand Up @@ -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{..} =
Expand Down
33 changes: 11 additions & 22 deletions server/src-lib/Hasura/GraphQL/Schema/Remote.hs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 4 additions & 3 deletions server/src-lib/Hasura/RQL/Types/CustomTypes.hs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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:
4 changes: 2 additions & 2 deletions server/tests-py/queries/actions/introspection/setup.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ args:
- name: actionName
definition:
handler: http://localhost:3000
output_type: SampleOutput
output_type: "[SampleOutput!]"
arguments:
- name: text
type: jsonb!
Expand Down Expand Up @@ -39,7 +39,7 @@ args:
- name: SampleOutput
fields:
- name: id
type: String!
type: "[String!]"
- name: SampleOutput2
fields:
- name: id
Expand Down
5 changes: 4 additions & 1 deletion server/tests-py/test_actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:

Expand All @@ -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:

Expand Down

0 comments on commit ecb90c4

Please sign in to comment.