diff --git a/CHANGELOG.md b/CHANGELOG.md index d5fb0e51bca..1073d6232b4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## Next release (Add entries below in the order of server, console, cli, docs, others) + ### Function field names customization (#7405) It is now possible to specify the GraphQL names of tracked SQL functions in Postgres sources, and different names may be given to the `_aggregate` and @@ -12,6 +13,7 @@ suffix-less versions. Aliases may be set by both ### Bug fixes and improvements +- server: allow nullable action responses (#4405) - server: add support for openapi json of REST Endpoints - server: enable inherited roles by default in the graphql-engine - server: support MSSQL insert mutations diff --git a/docs/graphql/core/actions/debugging.rst b/docs/graphql/core/actions/debugging.rst index d6d6866aa56..7b51f2c9304 100644 --- a/docs/graphql/core/actions/debugging.rst +++ b/docs/graphql/core/actions/debugging.rst @@ -38,12 +38,12 @@ of the webhook calls in the ``extensions.internal`` field. { "errors": [ { - "message": "expecting object or array of objects for action webhook response", + "message": "expecting null, object or array of objects for action webhook response", "extensions": { "code": "parse-failed", "path": "$", "internal": { - "error": "expecting object or array of objects for action webhook response", + "error": "expecting null, object or array of objects for action webhook response", "response": { "status": 200, "headers": [ diff --git a/server/src-lib/Hasura/GraphQL/Execute/Action.hs b/server/src-lib/Hasura/GraphQL/Execute/Action.hs index 469766911e0..8b0b52f5942 100644 --- a/server/src-lib/Hasura/GraphQL/Execute/Action.hs +++ b/server/src-lib/Hasura/GraphQL/Execute/Action.hs @@ -209,6 +209,7 @@ makeActionResponseNoRelations annFields webhookResponse = case webhookResponse of AWRArray objs -> AO.array $ map mkResponseObject objs AWRObject obj -> mkResponseObject obj + AWRNull -> AO.Null {- Note: [Async action architecture] ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -543,9 +544,16 @@ callWebhook if | HTTP.statusIsSuccessful responseStatus -> do let expectingArray = isListType outputType + expectingNull = isNullableType outputType modifyQErr addInternalToErr $ do webhookResponse <- decodeValue responseValue case webhookResponse of + AWRNull -> do + unless expectingNull $ + if expectingArray + then throwUnexpected "expecting array for action webhook response but got null" + else throwUnexpected "expecting object for action webhook response but got null" + validateResponseObject Map.empty AWRArray objs -> do unless expectingArray $ throwUnexpected "expecting object for action webhook response but got array" diff --git a/server/src-lib/Hasura/GraphQL/Execute/Action/Types.hs b/server/src-lib/Hasura/GraphQL/Execute/Action/Types.hs index 21f92dc8e33..11bc48f6aeb 100644 --- a/server/src-lib/Hasura/GraphQL/Execute/Action/Types.hs +++ b/server/src-lib/Hasura/GraphQL/Execute/Action/Types.hs @@ -80,17 +80,20 @@ $(J.deriveJSON (J.aesonDrop 5 J.snakeCase) ''ActionWebhookErrorResponse) data ActionWebhookResponse = AWRArray ![Map.HashMap G.Name J.Value] | AWRObject !(Map.HashMap G.Name J.Value) + | AWRNull deriving (Show, Eq) instance J.FromJSON ActionWebhookResponse where parseJSON v = case v of J.Array {} -> AWRArray <$> J.parseJSON v J.Object {} -> AWRObject <$> J.parseJSON v - _ -> fail "expecting object or array of objects for action webhook response" + J.Null {} -> pure AWRNull + _ -> fail "expecting null, object or array of objects for action webhook response" instance J.ToJSON ActionWebhookResponse where toJSON (AWRArray objects) = J.toJSON objects toJSON (AWRObject obj) = J.toJSON obj + toJSON (AWRNull) = J.Null data ActionRequestInfo = ActionRequestInfo { _areqiUrl :: !Text, diff --git a/server/src-lib/Hasura/RQL/Types/CustomTypes.hs b/server/src-lib/Hasura/RQL/Types/CustomTypes.hs index dfe7cf24e53..bb027e21628 100644 --- a/server/src-lib/Hasura/RQL/Types/CustomTypes.hs +++ b/server/src-lib/Hasura/RQL/Types/CustomTypes.hs @@ -3,6 +3,7 @@ module Hasura.RQL.Types.CustomTypes emptyCustomTypes, GraphQLType (..), isListType, + isNullableType, EnumTypeName (..), EnumValueDefinition (..), EnumTypeDefinition (..), @@ -79,6 +80,9 @@ instance J.FromJSON GraphQLType where isListType :: GraphQLType -> Bool isListType (GraphQLType ty) = G.isListType ty +isNullableType :: GraphQLType -> Bool +isNullableType (GraphQLType ty) = G.isNullable ty + newtype InputObjectFieldName = InputObjectFieldName {unInputObjectFieldName :: G.Name} deriving (Show, Eq, Ord, Hashable, J.FromJSON, J.ToJSON, ToTxt, Generic, NFData, Cacheable) diff --git a/server/tests-py/context.py b/server/tests-py/context.py index f09bde90e26..465a73e42b8 100644 --- a/server/tests-py/context.py +++ b/server/tests-py/context.py @@ -353,6 +353,10 @@ class ActionsWebhookHandler(http.server.BaseHTTPRequestHandler): resp, status = self.intentional_error() self._send_response(status, resp) + elif req_path == "/null-response": + resp, status = self.null_response() + self._send_response(status, resp) + else: self.send_response(HTTPStatus.NO_CONTENT) self.end_headers() @@ -471,6 +475,10 @@ class ActionsWebhookHandler(http.server.BaseHTTPRequestHandler): return resp['data']['user'][0], HTTPStatus.OK else: return resp['data']['user'], HTTPStatus.OK + + def null_response(self): + response = None + return response, HTTPStatus.OK diff --git a/server/tests-py/queries/actions/sync/expecting_array_response_got_null.yaml b/server/tests-py/queries/actions/sync/expecting_array_response_got_null.yaml new file mode 100644 index 00000000000..0deeca6da22 --- /dev/null +++ b/server/tests-py/queries/actions/sync/expecting_array_response_got_null.yaml @@ -0,0 +1,69 @@ +- description: Update actions webhook to another route which returns null response (also, output_type is non-nullable) + url: /v1/query + status: 200 + response: + message: success + query: + type: update_action + args: + name: create_users + definition: + kind: synchronous + arguments: + output_type: '[UserId]!' + handler: http://127.0.0.1:5593/null-response + +- description: Run create_users sync action (with null_response handler) + url: /v1/graphql + status: 200 + query: + query: | + mutation { + create_users { + id + } + } + + response: + errors: + - extensions: + internal: + error: unexpected response + response: + status: 200 + body: + headers: + - value: application/json + name: Content-Type + - value: abcd + name: Set-Cookie + request: + body: + session_variables: + x-hasura-role: admin + input: {} + action: + name: create_users + request_query: "mutation {\n create_users {\n\ + \ id\n }\n}\n" + url: http://127.0.0.1:5593/null-response + headers: [] + path: $ + code: unexpected + message: expecting array for action webhook response but got null +- description: Revert action definition + url: /v1/query + status: 200 + response: + message: success + query: + type: update_action + args: + name: create_users + definition: + kind: synchronous + arguments: + - name: users + type: '[UserInput!]!' + output_type: '[UserId]' + handler: http://127.0.0.1:5593/create-users diff --git a/server/tests-py/queries/actions/sync/expecting_object_response_got_null.yaml b/server/tests-py/queries/actions/sync/expecting_object_response_got_null.yaml new file mode 100644 index 00000000000..8977fbb531e --- /dev/null +++ b/server/tests-py/queries/actions/sync/expecting_object_response_got_null.yaml @@ -0,0 +1,70 @@ +- description: Update actions webhook to another route which returns null response (also, output_type is non-nullable) + url: /v1/query + status: 200 + response: + message: success + query: + type: update_action + args: + name: create_user + definition: + kind: synchronous + arguments: + output_type: UserId! + handler: http://127.0.0.1:5593/null-response + +- description: Run create_user sync action (with null_response handler) + url: /v1/graphql + status: 200 + query: + query: | + mutation { + create_user { + id + } + } + + response: + errors: + - extensions: + internal: + error: unexpected response + response: + status: 200 + body: + headers: + - value: application/json + name: Content-Type + - value: abcd + name: Set-Cookie + request: + body: + session_variables: + x-hasura-role: admin + input: {} + action: + name: create_user + request_query: "mutation {\n create_user {\n id\n }\n}\n" + url: http://127.0.0.1:5593/null-response + headers: [] + path: $ + code: unexpected + message: expecting object for action webhook response but got null +- description: Revert action definition + url: /v1/query + status: 200 + response: + message: success + query: + type: update_action + args: + name: create_user + definition: + kind: synchronous + arguments: + - name: email + type: String! + - name: name + type: String! + output_type: UserId + handler: http://127.0.0.1:5593/create-user diff --git a/server/tests-py/queries/actions/sync/invalid_webhook_response.yaml b/server/tests-py/queries/actions/sync/invalid_webhook_response.yaml index c22ba0317df..da39c47de52 100644 --- a/server/tests-py/queries/actions/sync/invalid_webhook_response.yaml +++ b/server/tests-py/queries/actions/sync/invalid_webhook_response.yaml @@ -63,7 +63,7 @@ headers: [] path: $ code: parse-failed - message: expecting object or array of objects for action webhook response + message: expecting null, object or array of objects for action webhook response - description: Revert action wehbook url: /v1/query status: 200 diff --git a/server/tests-py/queries/actions/sync/null_response.yaml b/server/tests-py/queries/actions/sync/null_response.yaml new file mode 100644 index 00000000000..0fb44d71208 --- /dev/null +++ b/server/tests-py/queries/actions/sync/null_response.yaml @@ -0,0 +1,13 @@ +description: Null response should be allowed +url: /v1/graphql +status: 200 +query: + query: | + mutation { + null_response { + id + } + } +response: + data: + null_response: null diff --git a/server/tests-py/queries/actions/sync/schema_setup.yaml b/server/tests-py/queries/actions/sync/schema_setup.yaml index c074687234d..c6344864887 100644 --- a/server/tests-py/queries/actions/sync/schema_setup.yaml +++ b/server/tests-py/queries/actions/sync/schema_setup.yaml @@ -84,6 +84,11 @@ args: type: ID! # For issue https://github.com/hasura/graphql-engine/issues/4061 - name: name type: String + + - name: NullableResp + fields: + - name: id + type: Int - type: create_action args: @@ -187,6 +192,15 @@ args: output_type: '[UserId]!' handler: http://127.0.0.1:5593/intentional-error +- type: create_action + args: + name: null_response + definition: + kind: synchronous + arguments: + output_type: NullableResp + handler: http://127.0.0.1:5593/null-response + - type: create_select_permission args: table: user diff --git a/server/tests-py/queries/actions/sync/schema_teardown.yaml b/server/tests-py/queries/actions/sync/schema_teardown.yaml index fc621948057..5c0948b96d4 100644 --- a/server/tests-py/queries/actions/sync/schema_teardown.yaml +++ b/server/tests-py/queries/actions/sync/schema_teardown.yaml @@ -28,6 +28,10 @@ args: args: name: intentional_error clear_data: true +- type: drop_action + args: + name: null_response + clear_data: true # clear custom types - type: set_custom_types args: {} diff --git a/server/tests-py/test_actions.py b/server/tests-py/test_actions.py index 8ec1f0ac052..ed4f672b5d0 100644 --- a/server/tests-py/test_actions.py +++ b/server/tests-py/test_actions.py @@ -63,11 +63,21 @@ class TestActionsSync: def test_invalid_webhook_response(self, hge_ctx): check_query_secret(hge_ctx, self.dir() + '/invalid_webhook_response.yaml') - def test_expecting_object_response(self, hge_ctx): + def test_null_response(self, hge_ctx): + check_query_secret(hge_ctx, self.dir() + '/null_response.yaml') + + def test_expecting_object_response_got_null(self, hge_ctx): + check_query_secret(hge_ctx, self.dir() + '/expecting_object_response_got_null.yaml') + + def test_expecting_array_response_got_null(self, hge_ctx): + check_query_secret(hge_ctx, self.dir() + '/expecting_array_response_got_null.yaml') + + def test_expecting_object_response_got_array(self, hge_ctx): check_query_secret(hge_ctx, self.dir() + '/expecting_object_response.yaml') - def test_expecting_array_response(self, hge_ctx): + def test_expecting_array_response_got_object(self, hge_ctx): check_query_secret(hge_ctx, self.dir() + '/expecting_array_response.yaml') + # Webhook response validation tests. See https://github.com/hasura/graphql-engine/issues/3977 def test_mirror_action_not_null(self, hge_ctx):