Move path to the execution error

Since it isn't possible to get a path during validation, without
executing the query.
This commit is contained in:
Eugen Wissner 2020-09-16 09:12:49 +02:00
parent 4c10ce9204
commit 6e644c5b4b
9 changed files with 30 additions and 56 deletions

View File

@ -17,6 +17,10 @@ and this project adheres to
rules. `rules` was a part of the `Validation` context to pass it easier rules. `rules` was a part of the `Validation` context to pass it easier
around, but since the rules are traversed once now and applied to all nodes in around, but since the rules are traversed once now and applied to all nodes in
the tree at the beginning, it isn't required anymore. the tree at the beginning, it isn't required anymore.
- `Validate.Validation.Error`: `path` is removed since it isn't possible to get
the path without executing the query.
- `Error.Error`: `path` added. It is currently always empty.
- `Validate.Validation.Path` was moved to `Error`.
### Added ### Added
- `Validate.Validation.Rule`: `SelectionRule`, `FieldRule`, `FragmentRule`, - `Validate.Validation.Rule`: `SelectionRule`, `FieldRule`, `FragmentRule`,

View File

@ -11,6 +11,7 @@ import Control.Monad.Catch (MonadCatch)
import qualified Data.Aeson as Aeson import qualified Data.Aeson as Aeson
import qualified Data.Aeson.Types as Aeson import qualified Data.Aeson.Types as Aeson
import qualified Data.HashMap.Strict as HashMap import qualified Data.HashMap.Strict as HashMap
import Data.Maybe (catMaybes)
import qualified Data.Sequence as Seq import qualified Data.Sequence as Seq
import Data.Text (Text) import Data.Text (Text)
import Language.GraphQL.AST import Language.GraphQL.AST
@ -55,24 +56,19 @@ graphqlSubs schema operationName variableValues document' =
[ ("data", data'') [ ("data", data'')
, ("errors", Aeson.toJSON $ fromError <$> errors') , ("errors", Aeson.toJSON $ fromError <$> errors')
] ]
fromError Error{ locations = [], ..} = fromError Error{..} = Aeson.object $ catMaybes
Aeson.object [("message", Aeson.toJSON message)] [ Just ("message", Aeson.toJSON message)
fromError Error{..} = Aeson.object , toMaybe fromLocation "locations" locations
, toMaybe fromPath "path" path
]
fromValidationError Validate.Error{..} = Aeson.object
[ ("message", Aeson.toJSON message) [ ("message", Aeson.toJSON message)
, ("locations", Aeson.listValue fromLocation locations) , ("locations", Aeson.listValue fromLocation locations)
] ]
fromValidationError Validate.Error{..} toMaybe _ _ [] = Nothing
| [] <- path = Aeson.object toMaybe f key xs = Just (key, Aeson.listValue f xs)
[ ("message", Aeson.toJSON message) fromPath (Segment segment) = Aeson.String segment
, ("locations", Aeson.listValue fromLocation locations) fromPath (Index index) = Aeson.toJSON index
]
| otherwise = Aeson.object
[ ("message", Aeson.toJSON message)
, ("locations", Aeson.listValue fromLocation locations)
, ("path", Aeson.listValue fromPath path)
]
fromPath (Validate.Segment segment) = Aeson.String segment
fromPath (Validate.Index index) = Aeson.toJSON index
fromLocation Location{..} = Aeson.object fromLocation Location{..} = Aeson.object
[ ("line", Aeson.toJSON line) [ ("line", Aeson.toJSON line)
, ("column", Aeson.toJSON column) , ("column", Aeson.toJSON column)

View File

@ -8,6 +8,7 @@ module Language.GraphQL.Error
( parseError ( parseError
, CollectErrsT , CollectErrsT
, Error(..) , Error(..)
, Path(..)
, Resolution(..) , Resolution(..)
, ResolverException(..) , ResolverException(..)
, Response(..) , Response(..)
@ -57,6 +58,7 @@ parseError ParseErrorBundle{..} =
errorObject s SourcePos{..} = Error errorObject s SourcePos{..} = Error
{ message = Text.pack $ init $ parseErrorTextPretty s { message = Text.pack $ init $ parseErrorTextPretty s
, locations = [Location (unPos' sourceLine) (unPos' sourceColumn)] , locations = [Location (unPos' sourceLine) (unPos' sourceColumn)]
, path = []
} }
unPos' = fromIntegral . unPos unPos' = fromIntegral . unPos
go (result, state) x = go (result, state) x =
@ -75,7 +77,7 @@ addErr v = modify appender
appender resolution@Resolution{..} = resolution{ errors = errors |> v } appender resolution@Resolution{..} = resolution{ errors = errors |> v }
makeErrorMessage :: Text -> Error makeErrorMessage :: Text -> Error
makeErrorMessage s = Error s [] makeErrorMessage s = Error s [] []
-- | Constructs a response object containing only the error with the given -- | Constructs a response object containing only the error with the given
-- message. -- message.
@ -86,10 +88,20 @@ singleError message = Response null $ Seq.singleton $ makeErrorMessage message
addErrMsg :: (Monad m, Serialize a) => Text -> CollectErrsT m a addErrMsg :: (Monad m, Serialize a) => Text -> CollectErrsT m a
addErrMsg errorMessage = (addErr . makeErrorMessage) errorMessage >> pure null addErrMsg errorMessage = (addErr . makeErrorMessage) errorMessage >> pure null
-- | If an error can be associated to a particular field in the GraphQL result,
-- it must contain an entry with the key path that details the path of the
-- response field which experienced the error. This allows clients to identify
-- whether a null result is intentional or caused by a runtime error.
data Path
= Segment Text -- ^ Field name.
| Index Int -- ^ List index if a field returned a list.
deriving (Eq, Show)
-- | @GraphQL@ error. -- | @GraphQL@ error.
data Error = Error data Error = Error
{ message :: Text { message :: Text
, locations :: [Location] , locations :: [Location]
, path :: [Path]
} deriving (Eq, Show) } deriving (Eq, Show)
-- | The server\'s response describes the result of executing the requested -- | The server\'s response describes the result of executing the requested

View File

@ -43,7 +43,7 @@ resolveFieldValue result args resolver =
=> ResolverException => ResolverException
-> CollectErrsT m Type.Value -> CollectErrsT m Type.Value
handleFieldError e = handleFieldError e =
addErr (Error (Text.pack $ displayException e) []) >> pure Type.Null addErr (Error (Text.pack $ displayException e) [] []) >> pure Type.Null
context = Type.Context context = Type.Context
{ Type.arguments = Type.Arguments args { Type.arguments = Type.Arguments args
, Type.values = result , Type.values = result

View File

@ -7,7 +7,6 @@
-- | GraphQL validator. -- | GraphQL validator.
module Language.GraphQL.Validate module Language.GraphQL.Validate
( Error(..) ( Error(..)
, Path(..)
, document , document
, module Language.GraphQL.Validate.Rules , module Language.GraphQL.Validate.Rules
) where ) where

View File

@ -70,7 +70,6 @@ executableDefinitionsRule = DefinitionRule $ \case
{ message = { message =
"Definition must be OperationDefinition or FragmentDefinition." "Definition must be OperationDefinition or FragmentDefinition."
, locations = [location] , locations = [location]
, path = []
} }
-- | Subscription operations must have exactly one root field. -- | Subscription operations must have exactly one root field.
@ -88,12 +87,10 @@ singleFieldSubscriptionsRule = OperationDefinitionRule $ \case
, "must select only one top level field." , "must select only one top level field."
] ]
, locations = [location] , locations = [location]
, path = []
} }
| otherwise -> pure $ Error | otherwise -> pure $ Error
{ message = errorMessage { message = errorMessage
, locations = [location] , locations = [location]
, path = []
} }
_ -> lift mempty _ -> lift mempty
where where
@ -173,7 +170,6 @@ loneAnonymousOperationRule = OperationDefinitionRule $ \case
{ message = { message =
"This anonymous operation must be the only defined operation." "This anonymous operation must be the only defined operation."
, locations = [location] , locations = [location]
, path = []
} }
-- | Each named operation definition must be unique within a document when -- | Each named operation definition must be unique within a document when
@ -209,7 +205,6 @@ findDuplicates filterByName thisLocation errorMessage = do
error' locations' = Error error' locations' = Error
{ message = errorMessage { message = errorMessage
, locations = locations' , locations = locations'
, path = []
} }
viewOperation :: Definition -> Maybe OperationDefinition viewOperation :: Definition -> Maybe OperationDefinition
@ -257,7 +252,6 @@ fragmentSpreadTargetDefinedRule = FragmentSpreadRule $ \case
Nothing -> pure $ Error Nothing -> pure $ Error
{ message = error' fragmentName { message = error' fragmentName
, locations = [location] , locations = [location]
, path = []
} }
Just _ -> lift mempty Just _ -> lift mempty
where where
@ -288,7 +282,6 @@ fragmentSpreadTypeExistenceRule = SelectionRule $ \case
Nothing -> pure $ Error Nothing -> pure $ Error
{ message = spreadError fragmentName typeCondition { message = spreadError fragmentName typeCondition
, locations = [location] , locations = [location]
, path = []
} }
Just _ -> lift mempty Just _ -> lift mempty
InlineFragmentSelection fragmentSelection InlineFragmentSelection fragmentSelection
@ -299,7 +292,6 @@ fragmentSpreadTypeExistenceRule = SelectionRule $ \case
Nothing -> pure $ Error Nothing -> pure $ Error
{ message = inlineError typeCondition { message = inlineError typeCondition
, locations = [location] , locations = [location]
, path = []
} }
Just _ -> lift mempty Just _ -> lift mempty
_ -> lift mempty _ -> lift mempty
@ -344,7 +336,6 @@ fragmentsOnCompositeTypesRule = FragmentRule definitionRule inlineRule
Nothing -> pure $ Error Nothing -> pure $ Error
{ message = errorMessage typeCondition { message = errorMessage typeCondition
, locations = [location] , locations = [location]
, path = []
} }
Just _ -> lift mempty Just _ -> lift mempty
errorMessage typeCondition = concat errorMessage typeCondition = concat
@ -363,7 +354,6 @@ noUnusedFragmentsRule = FragmentDefinitionRule $ \fragment ->
| otherwise = pure $ Error | otherwise = pure $ Error
{ message = errorMessage fragName { message = errorMessage fragName
, locations = [location] , locations = [location]
, path = []
} }
errorMessage fragName = concat errorMessage fragName = concat
[ "Fragment \"" [ "Fragment \""
@ -413,7 +403,6 @@ noFragmentCyclesRule = FragmentDefinitionRule $ \case
, ")." , ")."
] ]
, locations = [location] , locations = [location]
, path = []
} }
_ -> lift mempty _ -> lift mempty
where where

View File

@ -5,7 +5,6 @@
-- | Definitions used by the validation rules and the validator itself. -- | Definitions used by the validation rules and the validator itself.
module Language.GraphQL.Validate.Validation module Language.GraphQL.Validate.Validation
( Error(..) ( Error(..)
, Path(..)
, Rule(..) , Rule(..)
, RuleT , RuleT
, Validation(..) , Validation(..)
@ -14,25 +13,14 @@ module Language.GraphQL.Validate.Validation
import Control.Monad.Trans.Reader (ReaderT) import Control.Monad.Trans.Reader (ReaderT)
import Data.HashMap.Strict (HashMap) import Data.HashMap.Strict (HashMap)
import Data.Sequence (Seq) import Data.Sequence (Seq)
import Data.Text (Text)
import Language.GraphQL.AST.Document import Language.GraphQL.AST.Document
import Language.GraphQL.Type.Schema (Schema) import Language.GraphQL.Type.Schema (Schema)
import qualified Language.GraphQL.Type.Schema as Schema import qualified Language.GraphQL.Type.Schema as Schema
-- | If an error can be associated to a particular field in the GraphQL result,
-- it must contain an entry with the key path that details the path of the
-- response field which experienced the error. This allows clients to identify
-- whether a null result is intentional or caused by a runtime error.
data Path
= Segment Text -- ^ Field name.
| Index Int -- ^ List index if a field returned a list.
deriving (Eq, Show)
-- | Validation error. -- | Validation error.
data Error = Error data Error = Error
{ message :: String { message :: String
, locations :: [Location] , locations :: [Location]
, path :: [Path]
} deriving (Eq, Show) } deriving (Eq, Show)
-- | Validation rule context. -- | Validation rule context.

View File

@ -19,6 +19,6 @@ import Test.Hspec ( Spec
spec :: Spec spec :: Spec
spec = describe "singleError" $ spec = describe "singleError" $
it "constructs an error with the given message" $ it "constructs an error with the given message" $
let errors'' = Seq.singleton $ Error "Message." [] let errors'' = Seq.singleton $ Error "Message." [] []
expected = Response Aeson.Null errors'' expected = Response Aeson.Null errors''
in singleError "Message." `shouldBe` expected in singleError "Message." `shouldBe` expected

View File

@ -168,7 +168,6 @@ spec =
{ message = { message =
"Definition must be OperationDefinition or FragmentDefinition." "Definition must be OperationDefinition or FragmentDefinition."
, locations = [AST.Location 9 15] , locations = [AST.Location 9 15]
, path = []
} }
in validate queryString `shouldBe` Seq.singleton expected in validate queryString `shouldBe` Seq.singleton expected
@ -186,7 +185,6 @@ spec =
{ message = { message =
"Subscription sub must select only one top level field." "Subscription sub must select only one top level field."
, locations = [AST.Location 2 15] , locations = [AST.Location 2 15]
, path = []
} }
in validate queryString `shouldBe` Seq.singleton expected in validate queryString `shouldBe` Seq.singleton expected
@ -208,7 +206,6 @@ spec =
{ message = { message =
"Subscription sub must select only one top level field." "Subscription sub must select only one top level field."
, locations = [AST.Location 2 15] , locations = [AST.Location 2 15]
, path = []
} }
in validate queryString `shouldBe` Seq.singleton expected in validate queryString `shouldBe` Seq.singleton expected
@ -232,7 +229,6 @@ spec =
{ message = { message =
"This anonymous operation must be the only defined operation." "This anonymous operation must be the only defined operation."
, locations = [AST.Location 2 15] , locations = [AST.Location 2 15]
, path = []
} }
in validate queryString `shouldBe` Seq.singleton expected in validate queryString `shouldBe` Seq.singleton expected
@ -254,7 +250,6 @@ spec =
{ message = { message =
"There can be only one operation named \"dogOperation\"." "There can be only one operation named \"dogOperation\"."
, locations = [AST.Location 2 15, AST.Location 8 15] , locations = [AST.Location 2 15, AST.Location 8 15]
, path = []
} }
in validate queryString `shouldBe` Seq.singleton expected in validate queryString `shouldBe` Seq.singleton expected
@ -280,7 +275,6 @@ spec =
{ message = { message =
"There can be only one fragment named \"fragmentOne\"." "There can be only one fragment named \"fragmentOne\"."
, locations = [AST.Location 8 15, AST.Location 12 15] , locations = [AST.Location 8 15, AST.Location 12 15]
, path = []
} }
in validate queryString `shouldBe` Seq.singleton expected in validate queryString `shouldBe` Seq.singleton expected
@ -296,7 +290,6 @@ spec =
{ message = { message =
"Fragment target \"undefinedFragment\" is undefined." "Fragment target \"undefinedFragment\" is undefined."
, locations = [AST.Location 4 19] , locations = [AST.Location 4 19]
, path = []
} }
in validate queryString `shouldBe` Seq.singleton expected in validate queryString `shouldBe` Seq.singleton expected
@ -316,7 +309,6 @@ spec =
"Fragment \"notOnExistingType\" is specified on type \ "Fragment \"notOnExistingType\" is specified on type \
\\"NotInSchema\" which doesn't exist in the schema." \\"NotInSchema\" which doesn't exist in the schema."
, locations = [AST.Location 4 19] , locations = [AST.Location 4 19]
, path = []
} }
in validate queryString `shouldBe` Seq.singleton expected in validate queryString `shouldBe` Seq.singleton expected
@ -333,7 +325,6 @@ spec =
"Inline fragment is specified on type \"NotInSchema\" \ "Inline fragment is specified on type \"NotInSchema\" \
\which doesn't exist in the schema." \which doesn't exist in the schema."
, locations = [AST.Location 3 17] , locations = [AST.Location 3 17]
, path = []
} }
in validate queryString `shouldBe` Seq.singleton expected in validate queryString `shouldBe` Seq.singleton expected
@ -353,7 +344,6 @@ spec =
"Fragment cannot condition on non composite type \ "Fragment cannot condition on non composite type \
\\"Int\"." \\"Int\"."
, locations = [AST.Location 7 15] , locations = [AST.Location 7 15]
, path = []
} }
in validate queryString `shouldBe` Seq.singleton expected in validate queryString `shouldBe` Seq.singleton expected
@ -370,7 +360,6 @@ spec =
"Fragment cannot condition on non composite type \ "Fragment cannot condition on non composite type \
\\"Boolean\"." \\"Boolean\"."
, locations = [AST.Location 3 17] , locations = [AST.Location 3 17]
, path = []
} }
in validate queryString `shouldBe` Seq.singleton expected in validate queryString `shouldBe` Seq.singleton expected
@ -390,7 +379,6 @@ spec =
{ message = { message =
"Fragment \"nameFragment\" is never used." "Fragment \"nameFragment\" is never used."
, locations = [AST.Location 2 15] , locations = [AST.Location 2 15]
, path = []
} }
in validate queryString `shouldBe` Seq.singleton expected in validate queryString `shouldBe` Seq.singleton expected
@ -416,7 +404,6 @@ spec =
\itself (via barkVolumeFragment -> nameFragment -> \ \itself (via barkVolumeFragment -> nameFragment -> \
\barkVolumeFragment)." \barkVolumeFragment)."
, locations = [AST.Location 11 15] , locations = [AST.Location 11 15]
, path = []
} }
error2 = Error error2 = Error
{ message = { message =
@ -424,6 +411,5 @@ spec =
\(via nameFragment -> barkVolumeFragment -> \ \(via nameFragment -> barkVolumeFragment -> \
\nameFragment)." \nameFragment)."
, locations = [AST.Location 7 15] , locations = [AST.Location 7 15]
, path = []
} }
in validate queryString `shouldBe` Seq.fromList [error1, error2] in validate queryString `shouldBe` Seq.fromList [error1, error2]