hasura/graphql-engine
1
0
Fork 0
mirror of https://github.com/hasura/graphql-engine.git synced 2024-10-04 22:07:40 +03:00
Code Issues Projects Releases Wiki Activity

server: don't forward absent variable values as nulls

Browse Source 
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/9871
GitOrigin-RevId: 71191f89d7de36ff1fad0e29e8f1f07a9ddca9f7
This commit is contained in:
Auke Booij 2023-07-18 14:34:50 +02:00 committed by hasura-bot
parent e05657a620
commit 9a09af4f20
13 changed files with 207 additions and 54 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
server/lib/schema-parsers/schema-parsers.cabal
View File

@ -119,6 +119,7 @@ library
, graphql-parser
, hashable
, hasura-error-message
, hasura-prelude
, insert-ordered-containers
, lens
, mtl

2
server/lib/schema-parsers/src/Hasura/GraphQL/Parser.hs
View File

@ -45,7 +45,9 @@ module Hasura.GraphQL.Parser
selectionSetUnion,
field,
fieldWithDefault,
fieldWithDefault',
fieldOptional,
fieldOptional',
wrapFieldParser,
handleTypename,
selection,

8
server/lib/schema-parsers/src/Hasura/GraphQL/Parser/Internal/Convert.hs
View File

@ -48,9 +48,11 @@ graphQLToJSON = \case
G.VEnum (G.EnumValue n) -> J.toJSON n
G.VList values -> J.toJSON $ graphQLToJSON <$> values
G.VObject objects -> J.toJSON $ graphQLToJSON <$> objects
G.VVariable variable -> case absurd <$> vValue variable of
JSONValue j -> j
GraphQLValue g -> graphQLToJSON g
G.VVariable variable -> case vValue variable of
-- TODO: Absent values can be encoded as nulls, but this arguably breaks GraphQL spec June 2018, section 2.9.5.
Nothing -> J.Null
Just (JSONValue j) -> j
Just (GraphQLValue g) -> graphQLToJSON (absurd <$> g)
jsonToGraphQL :: J.Value -> Either ErrorMessage (G.Value Void)
jsonToGraphQL = \case

68
server/lib/schema-parsers/src/Hasura/GraphQL/Parser/Internal/Input.hs
View File

@ -7,7 +7,9 @@ module Hasura.GraphQL.Parser.Internal.Input
enum,
field,
fieldOptional,
fieldOptional',
fieldWithDefault,
fieldWithDefault',
inputParserInput,
list,
object,
@ -18,7 +20,7 @@ where
import Control.Applicative (Alternative ((<|>)), liftA2)
import Control.Arrow ((>>>))
import Control.Lens hiding (enum, index)
import Control.Monad (unless, (<=<), (>=>))
import Control.Monad (join, unless, (<=<), (>=>))
import Data.Aeson qualified as J
import Data.Aeson.Key qualified as K
import Data.Aeson.KeyMap qualified as KM
@ -293,6 +295,29 @@ fieldOptional name description parser =
where
expectedType = toGraphQLType $ nullableType $ pType parser
fieldOptional' ::
(MonadParse m, 'Input <: k) =>
Name ->
Maybe Description ->
Parser origin k m a ->
InputFieldsParser origin m (Maybe a)
fieldOptional' name description parser =
InputFieldsParser
{ ifDefinitions =
[ Definition name description Nothing [] $
InputFieldInfo (nullableType $ pType parser) Nothing
],
ifParser =
HashMap.lookup name
>>> withKey (J.Key (K.fromText (unName name)))
. fmap join
. traverse \x -> do
val <- peelVariableWith False expectedType x
for val $ pInputParser parser
}
where
expectedType = toGraphQLType $ nullableType $ pType parser
-- | Creates a parser for an input field with the given default value. The
-- resulting field will always be optional, even if the underlying parser
-- rejects `null` values. The underlying parser is always called.
@ -311,7 +336,46 @@ fieldWithDefault name description defaultValue parser =
ifParser =
HashMap.lookup name
>>> withKey (J.Key (K.fromText (unName name))) . \case
Just value -> peelVariableWith True expectedType value >>= pInputParser parser
Just value -> do
val <- peelVariableWith True expectedType value
case val of
Just x -> pInputParser parser x
-- TODO: If the variable has no value, then we SHOULD use
-- `defaultValue` rather than null. This corresponds to step
-- 5.h.i. in algorithm `CoerceArgumentValues` in section 6.4.1
-- in the June 2018 GraphQL spec:
-- http://spec.graphql.org/June2018/#sec-Coercing-Field-Arguments
-- Currently using null instead to avoid breakages.
Nothing -> pInputParser parser $ GraphQLValue VNull
Nothing -> pInputParser parser $ GraphQLValue $ literal defaultValue
}
where
expectedType = toGraphQLType $ pType parser
fieldWithDefault' ::
(MonadParse m, 'Input <: k) =>
Name ->
Maybe Description ->
-- | default value
Value Void ->
Parser origin k m a ->
InputFieldsParser origin m a
fieldWithDefault' name description defaultValue parser =
InputFieldsParser
{ ifDefinitions = [Definition name description Nothing [] $ InputFieldInfo (pType parser) (Just defaultValue)],
ifParser =
HashMap.lookup name
>>> withKey (J.Key (K.fromText (unName name))) . \case
Just value -> do
val <- peelVariableWith True expectedType value
case val of
Just x -> pInputParser parser x
-- If the variable has no value, then we SHOULD use
-- `defaultValue` rather than null. This corresponds to step
-- 5.h.i. in algorithm `CoerceArgumentValues` in section 6.4.1
-- in the June 2018 GraphQL spec:
-- http://spec.graphql.org/June2018/#sec-Coercing-Field-Arguments
Nothing -> pInputParser parser $ GraphQLValue $ literal defaultValue
Nothing -> pInputParser parser $ GraphQLValue $ literal defaultValue
}
where

30
server/lib/schema-parsers/src/Hasura/GraphQL/Parser/Internal/Parser.hs
View File

@ -84,17 +84,37 @@ handleTypename :: (Name -> a) -> ParsedSelection a -> a
handleTypename _ (SelectField value) = value
handleTypename f (SelectTypename name) = f name
data NullableInput a
= NullableInputValue a
| NullableInputNull
| NullableInputAbsent
deriving (Show, Functor)
nullableToMaybe :: NullableInput a -> Maybe a
nullableToMaybe = fromNullableInput Nothing . (Just <$>)
fromNullableInput :: a -> NullableInput a -> a
fromNullableInput _ (NullableInputValue x) = x
fromNullableInput d _ = d
nullable :: forall origin k m a. (MonadParse m, 'Input <: k) => Parser origin k m a -> Parser origin k m (Maybe a)
nullable parser =
nullable = fmap nullableToMaybe . nullableExact
-- | Distinguishes between inputs with an explicit null value, and inputs whose
-- variable value is absent without default. See GraphQL spec June 2018 section
-- 2.9.5.
nullableExact :: forall origin k m a. (MonadParse m, 'Input <: k) => Parser origin k m a -> Parser origin k m (NullableInput a)
nullableExact parser =
gcastWith
(inputParserInput @k)
Parser
{ pType = schemaType,
pParser =
peelVariable (toGraphQLType schemaType) >=> \case
JSONValue J.Null -> pure Nothing
GraphQLValue VNull -> pure Nothing
value -> Just <$> pParser parser value
peelVariableWith False (toGraphQLType schemaType) >=> \case
Just (JSONValue J.Null) -> pure NullableInputNull
Just (GraphQLValue VNull) -> pure NullableInputNull
Just value -> NullableInputValue <$> pParser parser value
Nothing -> pure NullableInputAbsent
}
where
schemaType = nullableType $ pType parser

48
server/lib/schema-parsers/src/Hasura/GraphQL/Parser/Internal/TypeChecking.hs
View File

@ -9,16 +9,13 @@ module Hasura.GraphQL.Parser.Internal.TypeChecking
)
where
import Control.Arrow ((>>>))
import Control.Monad (unless)
import Data.Aeson qualified as J
import Data.Function (on)
import Data.Void (absurd)
import Hasura.Base.ErrorMessage
import Hasura.Base.ToErrorValue
import Hasura.GraphQL.Parser.Class
import Hasura.GraphQL.Parser.Names
import Hasura.GraphQL.Parser.Variable
import Hasura.Prelude
import Language.GraphQL.Draft.Syntax hiding (Definition)
-- | Peeling a variable.
@ -30,31 +27,25 @@ import Language.GraphQL.Draft.Syntax hiding (Definition)
-- where(limit: 42)
-- where(limit: $limit)
--
-- In the case of a variable, we need to "open" it and inspect its content. This
-- forces us to do several things:
-- In the case of a variable, we are now in the context type-check the variable
-- usage: we know what the variable was declared as, and we know what type we
-- expect at our current location in the schema: we can check that they match.
--
-- 1. Type-checking
-- We know what the variable was declared as, we know what type we expect
-- at our current location in the schema: we can check that they match.
--
-- 2. Update re-usability
-- In the past, we used to cache the generated execution plan for a given
-- query. To do so properly, we had to identify queries that couldn't be
-- cached; and any root field that uses the content of a variable (outside
-- of a column's value) couldn't be cached: the same graphql expression
-- would lead to a different execution plan if the value of the variable
-- were to change. We no longer cache execution plans; but we might do it
-- again in the future, which is why we haven't removed some of the code
-- that deals with re-usability.
-- Note: this conflates nulls and absent variable values (GraphQL spec, June
-- 2018, section 2.9.5), converting the latter into the former.
peelVariable :: (MonadParse m) => GType -> InputValue Variable -> m (InputValue Variable)
peelVariable = peelVariableWith False
peelVariable locationType value =
peelVariableWith False locationType value
`onNothingM` if isNullable locationType
then pure $ GraphQLValue VNull
else parseError $ "no variable value specified for non-nullable variable " <> describeValue value
peelVariableWith :: (MonadParse m) => Bool -> GType -> InputValue Variable -> m (InputValue Variable)
peelVariableWith :: (MonadParse m) => Bool -> GType -> InputValue Variable -> m (Maybe (InputValue Variable))
peelVariableWith locationHasDefaultValue locationType = \case
GraphQLValue (VVariable var) -> do
typeCheck locationHasDefaultValue locationType var
pure $ absurd <$> vValue var
value -> pure value
pure $ fmap absurd <$> vValue var
value -> pure $ Just value
-- | Type-checking.
--
@ -126,10 +117,15 @@ typeMismatch name expected given =
"expected " <> expected <> " for type " <> toErrorValue (getName name) <> ", but found " <> describeValue given
describeValue :: InputValue Variable -> ErrorMessage
describeValue = describeValueWith (describeValueWith absurd . vValue)
describeValue = describeValueWith describeVariable
describeVariable :: Variable -> ErrorMessage
describeVariable v@Variable {..} = case vValue of
Nothing -> "no value specified for variable " <> toErrorValue (getName v)
Just val -> describeValueWith absurd val
describeValueWith :: (var -> ErrorMessage) -> InputValue var -> ErrorMessage
describeValueWith describeVariable = \case
describeValueWith describeVariable' = \case
JSONValue jval -> describeJSON jval
GraphQLValue gval -> describeGraphQL gval
where
@ -141,7 +137,7 @@ describeValueWith describeVariable = \case
J.Array _ -> "a list"
J.Object _ -> "an object"
describeGraphQL = \case
VVariable var -> describeVariable var
VVariable var -> describeVariable' var
VInt _ -> "an integer"
VFloat _ -> "a float"
VString _ -> "a string"

9
server/lib/schema-parsers/src/Hasura/GraphQL/Parser/Variable.hs
View File

@ -82,9 +82,12 @@ instance (Hashable v) => Hashable (InputValue v)
data Variable = Variable
{ vInfo :: VariableInfo,
vType :: GType,
-- | Note: if the variable was null or was not provided and the field has a
-- non-null default value, this field contains the default value, not 'VNull'.
vValue :: InputValue Void
-- | The following cases are distinguished:
--
-- 1. A JSON value (including `null`) was provided: Just (JSONValue ...)
-- 2. No JSON value was provided, but a default value exists: Just (GraphQLValue ...)
-- 3. No JSON value was provided, and no default value exists: Nothing
vValue :: Maybe (InputValue Void)
}
deriving (Show, Eq, Generic, Ord, TH.Lift)

10
server/src-lib/Hasura/GraphQL/Execute/Remote.hs
View File

@ -43,8 +43,10 @@ getVariableDefinitionAndValue var@(Variable varInfo gType varValue) =
varJSONValue =
case varValue of
JSONValue v -> v
GraphQLValue val -> graphQLValueToJSON val
Just (JSONValue v) -> v
Just (GraphQLValue val) -> graphQLValueToJSON val
-- TODO: is this semantically correct RE: GraphQL spec June 2018, section 2.9.5?
Nothing -> J.Null
unresolveVariables ::
forall fragments.
@ -202,7 +204,7 @@ resolveRemoteVariable userInfo = \case
False -> throw400 CoercionError $ sessionVarEnumVal <<> " is not one of the valid enum values"
-- nullability is false, because we treat presets as hard presets
let variableGType = G.TypeNamed (G.Nullability False) typeName
pure $ Variable (VIRequired varName) variableGType (GraphQLValue coercedValue)
pure $ Variable (VIRequired varName) variableGType $ Just $ GraphQLValue coercedValue
RemoteJSONValue gtype jsonValue -> do
let key = RemoteJSONVariableKey gtype jsonValue
varMap <- gets coerce
@ -216,7 +218,7 @@ resolveRemoteVariable userInfo = \case
varName <-
G.mkName varText
`onNothing` throw500 ("'" <> varText <> "' is not a valid GraphQL name")
pure $ Variable (VIRequired varName) gtype $ JSONValue jsonValue
pure $ Variable (VIRequired varName) gtype $ Just $ JSONValue jsonValue
QueryVariable variable -> pure variable
-- | TODO: Documentation.

6
server/src-lib/Hasura/GraphQL/Execute/Resolve.hs
View File

@ -83,9 +83,11 @@ resolveVariables definitions jsonValues directives selSet = do
let defaultValue = fromMaybe G.VNull _vdDefaultValue
isOptional = isJust _vdDefaultValue || G.isNullable _vdType
value <- case HashMap.lookup _vdName jsonValues of
Just jsonValue -> pure $ JSONValue jsonValue
Just jsonValue -> pure $ Just $ JSONValue jsonValue
Nothing
| isOptional -> pure $ GraphQLValue $ absurd <$> defaultValue
-- If the variable value was not provided, and the variable has no
-- default value, then don't store a value for the variable.
| isOptional -> pure $ GraphQLValue . fmap absurd <$> _vdDefaultValue
| otherwise ->
throw400 ValidationFailed
$ "expecting a value for non-nullable variable: "

4
server/src-lib/Hasura/GraphQL/ParameterizedQueryHash.hs
View File

@ -179,7 +179,9 @@ normalizeSelectionSet = (normalizeSelection =<<)
G.VEnum _ -> G.VNull
G.VList l -> G.VList $ map normalizeValue l
G.VObject obj -> G.VObject $ HashMap.map normalizeValue obj
G.VVariable (Variable _info _type value) ->
-- Pretend that variables without values are just nulls.
G.VVariable (Variable _info _type Nothing) -> G.VNull
G.VVariable (Variable _info _type (Just value)) ->
case value of
GraphQLValue val -> normalizeConstValue val
JSONValue v -> jsonToNormalizedGQLVal v

7
server/src-lib/Hasura/GraphQL/Schema/Common.hs
View File

@ -531,7 +531,12 @@ peelWithOrigin parser =
P.GraphQLValue (G.VVariable var@P.Variable {vInfo, vValue}) -> do
-- Check types c.f. 5.8.5 of the June 2018 GraphQL spec
P.typeCheck False (P.toGraphQLType $ P.pType parser) var
IR.ValueWithOrigin vInfo <$> P.pParser parser (absurd <$> vValue)
fmap (IR.ValueWithOrigin vInfo)
$ P.pParser parser
$ case vValue of
-- TODO: why is faking a null value here semantically correct? RE: GraphQL spec June 2018, section 2.9.5
Nothing -> P.GraphQLValue G.VNull
Just val -> absurd <$> val
value -> IR.ValueNoOrigin <$> P.pParser parser value
}

13
server/src-lib/Hasura/GraphQL/Schema/Remote.hs
View File

@ -21,7 +21,7 @@ import Data.Text.Extended
import Data.Type.Equality
import Hasura.Base.Error
import Hasura.GraphQL.Namespace
import Hasura.GraphQL.Parser.Internal.Parser qualified as P (inputParserInput, nonNullableField, nullableField)
import Hasura.GraphQL.Parser.Internal.Parser qualified as P (NullableInput (..), inputParserInput, nonNullableField, nullableExact, nullableField)
import Hasura.GraphQL.Parser.Internal.TypeChecking qualified as P
import Hasura.GraphQL.Parser.Name qualified as GName
import Hasura.GraphQL.Schema.Common
@ -274,9 +274,10 @@ inputValueDefinitionParser schemaDoc (G.InputValueDefinition desc name fieldType
Parser k n (Maybe (Altered, G.Value RemoteSchemaVariable)) ->
Parser k n (Maybe (Altered, G.Value RemoteSchemaVariable))
doNullability (G.Nullability True) parser =
nullable parser `bind` \case
Just x -> pure x
Nothing -> pure $ Just (Altered False, G.VNull)
P.nullableExact parser `bind` \case
P.NullableInputValue x -> pure x
P.NullableInputNull -> pure $ Just (Altered False, G.VNull)
P.NullableInputAbsent -> pure Nothing
doNullability (G.Nullability False) parser = parser
fieldConstructor ::
@ -288,9 +289,9 @@ inputValueDefinitionParser schemaDoc (G.InputValueDefinition desc name fieldType
case maybeDefaultVal of
Nothing ->
if G.isNullable fieldType
then join <$> fieldOptional name desc parser
then join <$> fieldOptional' name desc parser
else field name desc parser
Just defaultVal -> fieldWithDefault name desc defaultVal parser
Just defaultVal -> fieldWithDefault' name desc defaultVal parser
buildField ::
( forall k.

55
server/src-test/Hasura/GraphQL/Schema/RemoteSpec.hs
View File

@ -184,6 +184,8 @@ spec = do
testNoVarExpansionIfNoPresetUnlessTopLevelOptionalField
testNoVarExpansionIfNoPresetUnlessTopLevelOptionalFieldSendNullField
testNoVarExpansionIfNoPresetUnlessTopLevelOptionalFieldSendNullFieldForObjectField
testAbsentValuesDontGetForwarded
testAbsentValuesWithDefault
testPartialVarExpansionIfPreset
testVariableSubstitutionCollision
@ -238,7 +240,7 @@ query($a: A!) {
$ Variable
(VIRequired _a)
(G.TypeNamed (G.Nullability False) _A)
(JSONValue $ J.Object $ KM.fromList [("b", J.Object $ KM.fromList [("c", J.Object $ KM.fromList [("i", J.Number 0)])])])
(Just $ JSONValue $ J.Object $ KM.fromList [("b", J.Object $ KM.fromList [("c", J.Object $ KM.fromList [("i", J.Number 0)])])])
)
testNoVarExpansionIfNoPresetUnlessTopLevelOptionalField :: Spec
@ -329,6 +331,57 @@ query ($a: Int) {
(J.Null)
)
testAbsentValuesDontGetForwarded :: Spec
testAbsentValuesDontGetForwarded = it "don't forward variables without values" $ do
field <-
run
-- schema
[raw|
scalar Int
type Query {
test(a: Int): Int
}
|]
-- query
[raw|
query ($a: Int) {
test(a: $a)
}
|]
-- variables
[raw|
{
}
|]
length (_fArguments field) `shouldBe` 0
testAbsentValuesWithDefault :: Spec
testAbsentValuesWithDefault = it "variable without value doesn't cause field with default to become null" $ do
field <-
run
-- schema
[raw|
scalar Int
type Query {
test(a: Int = 3): Int
}
|]
-- query
[raw|
query ($a: Int) {
test(a: $a)
}
|]
-- variables
[raw|
{
}
|]
-- Actually, even better would be if `_fArguments` would be empty.
head (toList (_fArguments field)) `shouldBe` G.VInt 3
testNoVarExpansionIfNoPresetUnlessTopLevelOptionalFieldSendNullFieldForObjectField :: Spec
testNoVarExpansionIfNoPresetUnlessTopLevelOptionalFieldSendNullFieldForObjectField = it "send null value in the input variable for nullable object field " $ do
field <-