Prevent empty subscription roots (fix hasura/graphql-engine#6898)

### Description

We always build a subscription root, even when there was no possible fields. This breaks some third party clients, as the spec does not allow empty types in the schema. This PR fixes this by changing the `buildSubscriptionParser` helper to return a `Maybe` value, and harmonizes / cleans places where we build the subscription root.

https://github.com/hasura/graphql-engine-mono/pull/2357

GitOrigin-RevId: 1aeae25e321eee957e7645c436d17e69207309fd
This commit is contained in:
Antoine Leblanc 2021-09-16 08:41:52 +01:00 committed by hasura-bot
parent 9bae641ac4
commit e2ce1972f6
5 changed files with 68 additions and 38 deletions

View File

@ -9,6 +9,7 @@
- server: update `create_scheduled_event` API to return `event_id` in response - server: update `create_scheduled_event` API to return `event_id` in response
- server: fix bug which allowed inconsistent metadata to exist after the `replace_metadata` API even though `allow_inconsistent_object` is set to `false`. - server: fix bug which allowed inconsistent metadata to exist after the `replace_metadata` API even though `allow_inconsistent_object` is set to `false`.
- server: fix explicit `null` values not allowed in nested object relationship inserts (#7484) - server: fix explicit `null` values not allowed in nested object relationship inserts (#7484)
- server: prevent empty subscription roots in the schema (#6898)
- console: support tracking of functions with return a single row - console: support tracking of functions with return a single row
## v2.0.9 ## v2.0.9

View File

@ -92,9 +92,7 @@
"queryType": { "queryType": {
"name": "query_root" "name": "query_root"
}, },
"subscriptionType": { "subscriptionType": null,
"name": "subscription_root"
},
"types": [ "types": [
{ {
"inputFields": null, "inputFields": null,
@ -766,16 +764,6 @@
"enumValues": null, "enumValues": null,
"description": null, "description": null,
"fields": [] "fields": []
},
{
"inputFields": null,
"kind": "OBJECT",
"possibleTypes": null,
"interfaces": [],
"name": "subscription_root",
"enumValues": null,
"description": null,
"fields": []
} }
], ],
"mutationType": null "mutationType": null

View File

@ -256,7 +256,7 @@ buildRelayRoleContext
mutationParserBackend <- mutationParserBackend <-
buildMutationParser mempty allActionInfos nonObjectCustomTypes mutationBackendFields buildMutationParser mempty allActionInfos nonObjectCustomTypes mutationBackendFields
subscriptionParser <- subscriptionParser <-
P.safeSelectionSet subscriptionRoot Nothing queryPGFields <&> fmap (fmap typenameToRawRF) buildSubscriptionParser queryPGFields []
queryParserFrontend <- queryParserFrontend <-
queryWithIntrospectionHelper queryPGFields mutationParserFrontend subscriptionParser queryWithIntrospectionHelper queryPGFields mutationParserFrontend subscriptionParser
queryParserBackend <- queryParserBackend <-
@ -341,16 +341,14 @@ unauthenticatedContext
-> RemoteSchemaPermsCtx -> RemoteSchemaPermsCtx
-> m GQLContext -> m GQLContext
unauthenticatedContext adminQueryRemotes adminMutationRemotes remoteSchemaPermsCtx = P.runSchemaT $ do unauthenticatedContext adminQueryRemotes adminMutationRemotes remoteSchemaPermsCtx = P.runSchemaT $ do
let isRemoteSchemaPermsEnabled = remoteSchemaPermsCtx == RemoteSchemaPermsEnabled let
queryFields = bool (fmap (fmap RFRemote) adminQueryRemotes) [] isRemoteSchemaPermsEnabled isRemoteSchemaPermsEnabled = remoteSchemaPermsCtx == RemoteSchemaPermsEnabled
mutationFields = bool (fmap (fmap RFRemote) adminMutationRemotes) [] isRemoteSchemaPermsEnabled queryFields = bool (fmap (fmap RFRemote) adminQueryRemotes) [] isRemoteSchemaPermsEnabled
mutationParser <- mutationFields = bool (fmap (fmap RFRemote) adminMutationRemotes) [] isRemoteSchemaPermsEnabled
if null adminMutationRemotes mutationParser <- whenMaybe (not $ null mutationFields) $
then pure Nothing P.safeSelectionSet mutationRoot (Just $ G.Description "mutation root") mutationFields
else P.safeSelectionSet mutationRoot Nothing mutationFields <&> Just . fmap (fmap typenameToRawRF) <&> fmap (fmap typenameToRawRF)
subscriptionParser <- queryParser <- queryWithIntrospectionHelper queryFields mutationParser Nothing
P.safeSelectionSet subscriptionRoot Nothing [] <&> fmap (fmap typenameToRawRF)
queryParser <- queryWithIntrospectionHelper queryFields mutationParser subscriptionParser
pure $ GQLContext (finalizeParser queryParser) (finalizeParser <$> mutationParser) pure $ GQLContext (finalizeParser queryParser) (finalizeParser <$> mutationParser)
@ -543,7 +541,7 @@ buildQueryParser
-> [ActionInfo] -> [ActionInfo]
-> NonObjectTypeMap -> NonObjectTypeMap
-> Maybe (Parser 'Output n (OMap.InsOrdHashMap G.Name (MutationRootField UnpreparedValue))) -> Maybe (Parser 'Output n (OMap.InsOrdHashMap G.Name (MutationRootField UnpreparedValue)))
-> Parser 'Output n (OMap.InsOrdHashMap G.Name (QueryRootField UnpreparedValue)) -> Maybe (Parser 'Output n (OMap.InsOrdHashMap G.Name (QueryRootField UnpreparedValue)))
-> m (Parser 'Output n (OMap.InsOrdHashMap G.Name (QueryRootField UnpreparedValue))) -> m (Parser 'Output n (OMap.InsOrdHashMap G.Name (QueryRootField UnpreparedValue)))
buildQueryParser pgQueryFields remoteFields allActions nonObjectCustomTypes mutationParser subscriptionParser = do buildQueryParser pgQueryFields remoteFields allActions nonObjectCustomTypes mutationParser subscriptionParser = do
actionQueryFields <- concat <$> traverse (buildActionQueryFields nonObjectCustomTypes) allActions actionQueryFields <- concat <$> traverse (buildActionQueryFields nonObjectCustomTypes) allActions
@ -554,18 +552,18 @@ queryWithIntrospectionHelper
:: forall n m. (MonadSchema n m, MonadError QErr m) :: forall n m. (MonadSchema n m, MonadError QErr m)
=> [P.FieldParser n (QueryRootField UnpreparedValue)] => [P.FieldParser n (QueryRootField UnpreparedValue)]
-> Maybe (Parser 'Output n (OMap.InsOrdHashMap G.Name (MutationRootField UnpreparedValue))) -> Maybe (Parser 'Output n (OMap.InsOrdHashMap G.Name (MutationRootField UnpreparedValue)))
-> Parser 'Output n (OMap.InsOrdHashMap G.Name (QueryRootField UnpreparedValue)) -> Maybe (Parser 'Output n (OMap.InsOrdHashMap G.Name (QueryRootField UnpreparedValue)))
-> m (Parser 'Output n (OMap.InsOrdHashMap G.Name (QueryRootField UnpreparedValue))) -> m (Parser 'Output n (OMap.InsOrdHashMap G.Name (QueryRootField UnpreparedValue)))
queryWithIntrospectionHelper basicQueryFP mutationP subscriptionP = do queryWithIntrospectionHelper basicQueryFP mutationP subscriptionP = do
basicQueryP <- queryRootFromFields basicQueryFP basicQueryP <- queryRootFromFields basicQueryFP
emptyIntro <- emptyIntrospection emptyIntro <- emptyIntrospection
let directives = directivesInfo @n let directives = directivesInfo @n
allBasicTypes <- collectTypes $ allBasicTypes <- collectTypes $ catMaybes
[ P.TypeDefinitionsWrapper $ P.parserType basicQueryP [ Just $ P.TypeDefinitionsWrapper $ P.parserType basicQueryP
, P.TypeDefinitionsWrapper $ P.parserType subscriptionP , Just $ P.TypeDefinitionsWrapper $ P.diArguments =<< directives
, P.TypeDefinitionsWrapper $ P.diArguments =<< directives , P.TypeDefinitionsWrapper . P.parserType <$> mutationP
, P.TypeDefinitionsWrapper . P.parserType <$> subscriptionP
] ]
++ maybeToList (P.TypeDefinitionsWrapper . P.parserType <$> mutationP)
allIntrospectionTypes <- collectTypes . P.parserType =<< queryRootFromFields emptyIntro allIntrospectionTypes <- collectTypes . P.parserType =<< queryRootFromFields emptyIntro
let allTypes = Map.unions let allTypes = Map.unions
[ allBasicTypes [ allBasicTypes
@ -576,7 +574,7 @@ queryWithIntrospectionHelper basicQueryFP mutationP subscriptionP = do
, sTypes = allTypes , sTypes = allTypes
, sQueryType = P.parserType basicQueryP , sQueryType = P.parserType basicQueryP
, sMutationType = P.parserType <$> mutationP , sMutationType = P.parserType <$> mutationP
, sSubscriptionType = Just $ P.parserType subscriptionP , sSubscriptionType = P.parserType <$> subscriptionP
, sDirectives = directives , sDirectives = directives
} }
let partialQueryFields = let partialQueryFields =
@ -630,11 +628,13 @@ buildSubscriptionParser
) )
=> [P.FieldParser n (QueryRootField UnpreparedValue)] => [P.FieldParser n (QueryRootField UnpreparedValue)]
-> [ActionInfo] -> [ActionInfo]
-> m (Parser 'Output n (OMap.InsOrdHashMap G.Name (QueryRootField UnpreparedValue))) -> m (Maybe (Parser 'Output n (OMap.InsOrdHashMap G.Name (QueryRootField UnpreparedValue))))
buildSubscriptionParser queryFields allActions = do buildSubscriptionParser queryFields allActions = do
actionSubscriptionFields <- concat <$> traverse buildActionSubscriptionFields allActions actionSubscriptionFields <- concat <$> traverse buildActionSubscriptionFields allActions
let subscriptionFields = queryFields <> actionSubscriptionFields let subscriptionFields = queryFields <> actionSubscriptionFields
P.safeSelectionSet subscriptionRoot Nothing subscriptionFields <&> fmap (fmap typenameToRawRF) whenMaybe (not $ null subscriptionFields) $
P.safeSelectionSet subscriptionRoot Nothing subscriptionFields
<&> fmap (fmap typenameToRawRF)
buildMutationParser buildMutationParser
:: forall m n r :: forall m n r
@ -654,10 +654,9 @@ buildMutationParser allRemotes allActions nonObjectCustomTypes mutationFields =
mutationFields <> mutationFields <>
actionParsers <> actionParsers <>
fmap (fmap RFRemote) allRemotes fmap (fmap RFRemote) allRemotes
if null mutationFieldsParser whenMaybe (not $ null mutationFieldsParser) $
then pure Nothing P.safeSelectionSet mutationRoot (Just $ G.Description "mutation root") mutationFieldsParser
else P.safeSelectionSet mutationRoot (Just $ G.Description "mutation root") mutationFieldsParser <&> fmap (fmap typenameToRawRF)
<&> Just . fmap (fmap typenameToRawRF)

View File

@ -0,0 +1,31 @@
- description: Empty schema does not have a mutation root object
url: /v1/graphql
status: 200
response:
data:
__type: null
query:
query: |
query {
__type(name: "mutation_root") {
fields {
name
}
}
}
- description: Empty schema does not have a subscription root object
url: /v1/graphql
status: 200
response:
data:
__type: null
query:
query: |
query {
__type(name: "subscription_root") {
fields {
name
}
}
}

View File

@ -45,6 +45,17 @@ class TestGraphQLQueryBasicMySQL:
return 'queries/graphql_query/mysql' return 'queries/graphql_query/mysql'
@pytest.mark.parametrize("transport", ['http', 'websocket'])
class TestGraphQLEmpty:
def test_no_empty_roots(self, hge_ctx, transport):
check_query_f(hge_ctx, self.dir() + '/check_no_empty_roots.yaml', transport)
@classmethod
def dir(cls):
return 'queries/graphql_query/empty'
@pytest.mark.parametrize("transport", ['http', 'websocket']) @pytest.mark.parametrize("transport", ['http', 'websocket'])
@pytest.mark.parametrize("backend", ['bigquery']) @pytest.mark.parametrize("backend", ['bigquery'])
@usefixtures('per_class_tests_db_state') @usefixtures('per_class_tests_db_state')