diff --git a/CHANGELOG.md b/CHANGELOG.md index a63e51de9c3..b5f131a53ba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -109,6 +109,7 @@ Response 2: ### Bug fixes and improvements +- server: add support for custom scalar in action output type (#4185) - server: add support for MSSQL event triggers (#7228) - server: update pg_dump to be compatible with postgres 14 (#7676) - server: fix bug where timestamp values sent to postgres would erroneously trim leading zeroes (#8096) diff --git a/server/src-lib/Hasura/Backends/Postgres/SQL/Types.hs b/server/src-lib/Hasura/Backends/Postgres/SQL/Types.hs index 0aea44a48e7..7d4845743a9 100644 --- a/server/src-lib/Hasura/Backends/Postgres/SQL/Types.hs +++ b/server/src-lib/Hasura/Backends/Postgres/SQL/Types.hs @@ -39,6 +39,7 @@ module Hasura.Backends.Postgres.SQL.Types qualifiedObjectToName, PGScalarType (..), textToPGScalarType, + pgScalarTranslations, pgScalarTypeToText, PGTypeKind (..), QualifiedPGType (..), diff --git a/server/src-lib/Hasura/GraphQL/Execute/Action.hs b/server/src-lib/Hasura/GraphQL/Execute/Action.hs index 0e6de538c27..29fe16f1e8a 100644 --- a/server/src-lib/Hasura/GraphQL/Execute/Action.hs +++ b/server/src-lib/Hasura/GraphQL/Execute/Action.hs @@ -142,7 +142,7 @@ resolveActionExecution :: Maybe GQLQueryText -> ActionExecution resolveActionExecution env logger _userInfo IR.AnnActionExecution {..} ActionExecContext {..} gqlQueryText = - ActionExecution $ first (encJFromOrderedValue . makeActionResponseNoRelations _aaeFields) <$> runWebhook + ActionExecution $ first (encJFromOrderedValue . makeActionResponseNoRelations _aaeFields _aaeOutputType _aaeOutputFields True) <$> runWebhook where handlerPayload = ActionWebhookPayload (ActionContext _aaeName) _aecSessionVariables _aaePayload gqlQueryText @@ -166,8 +166,8 @@ resolveActionExecution env logger _userInfo IR.AnnActionExecution {..} ActionExe _aaeResponseTransform -- | Build action response from the Webhook JSON response when there are no relationships defined -makeActionResponseNoRelations :: IR.ActionFields -> ActionWebhookResponse -> AO.Value -makeActionResponseNoRelations annFields webhookResponse = +makeActionResponseNoRelations :: IR.ActionFields -> GraphQLType -> IR.ActionOutputFields -> Bool -> ActionWebhookResponse -> AO.Value +makeActionResponseNoRelations annFields outputType outputF shouldCheckOutputField webhookResponse = let mkResponseObject :: IR.ActionFields -> HashMap Text J.Value -> AO.Value mkResponseObject fields obj = AO.object $ @@ -191,13 +191,27 @@ makeActionResponseNoRelations annFields webhookResponse = in -- NOTE (Sam): This case would still not allow for aliased fields to be -- a part of the response. Also, seeing that none of the other `annField` -- types would be caught in the example, I've chosen to leave it as it is. - case webhookResponse of - AWRArray objs -> AO.array $ map mkResponseArray objs - AWRObject obj -> mkResponseObject annFields (mapKeys G.unName obj) - AWRNum n -> AO.toOrdered n - AWRBool b -> AO.toOrdered b - AWRString s -> AO.toOrdered s - AWRNull -> AO.Null + -- NOTE: (Pranshi) Bool here is applied to specify if we want to check ActionOutputFields + -- (in async actions, we have object types, which need to be validated + -- and ActionOutputField information is not present in `resolveAsyncActionQuery` - + -- so added a boolean which will make sure that the response is validated) + if gTypeContains isCustomScalar (unGraphQLType outputType) outputF && shouldCheckOutputField + then AO.toOrdered $ J.toJSON webhookResponse + else case webhookResponse of + AWRArray objs -> AO.array $ map mkResponseArray objs + AWRObject obj -> + mkResponseObject annFields (mapKeys G.unName obj) + AWRNull -> AO.Null + _ -> AO.toOrdered $ J.toJSON webhookResponse + +gTypeContains :: (G.GType -> IR.ActionOutputFields -> Bool) -> G.GType -> IR.ActionOutputFields -> Bool +gTypeContains fun gType aof = case gType of + t@(G.TypeNamed _ _) -> fun t aof + (G.TypeList _ expectedType) -> gTypeContains fun expectedType aof + +isCustomScalar :: G.GType -> IR.ActionOutputFields -> Bool +isCustomScalar (G.TypeNamed _ name) outputF = isJust (lookup (G.unName name) pgScalarTranslations) || (Map.null outputF && (not (isInBuiltScalar (G.unName name)))) +isCustomScalar (G.TypeList _ _) _ = False {- Note: [Async action architecture] ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -266,7 +280,7 @@ resolveAsyncActionQuery userInfo annAction = IR.AsyncOutput annFields -> fromMaybe AO.Null <$> forM _alrResponsePayload - \response -> makeActionResponseNoRelations annFields <$> decodeValue response + \response -> makeActionResponseNoRelations annFields outputType Map.empty False <$> decodeValue response IR.AsyncId -> pure $ AO.String $ actionIdToText actionId IR.AsyncCreatedAt -> pure $ AO.toOrdered $ J.toJSON _alrCreatedAt IR.AsyncErrors -> pure $ AO.toOrdered $ J.toJSON _alrErrors @@ -570,7 +584,7 @@ callWebhook | HTTP.statusIsSuccessful responseStatus -> do modifyQErr addInternalToErr $ do webhookResponse <- decodeValue responseValue - validateResponse responseValue outputType + validateResponse responseValue outputType outputFields pure (webhookResponse, mkSetCookieHeaders responseWreq) | HTTP.statusIsClientError responseStatus -> do ActionWebhookErrorResponse message maybeCode maybeExtensions <- @@ -604,44 +618,37 @@ callWebhook throwUnexpected $ "expecting not null value for field " <>> fieldName - isScalar :: Text -> Bool - isScalar s - | s == "Int" = True - | s == "Float" = True - | s == "String" = True - | s == "Boolean" = True - | otherwise = False - -- Validates the webhook response against the output type - validateResponse :: J.Value -> GraphQLType -> m () - validateResponse webhookResponse' outputType' = - case (webhookResponse', outputType') of - (J.Null, _) -> unless (isNullableType outputType') $ throwUnexpected "got null for the action webhook response" - (J.Number _, (GraphQLType (G.TypeNamed _ name))) -> do - unless (G.unName name == "Int" || G.unName name == "Float") $ - throwUnexpected $ "got scalar Number for the action webhook response, expecting " <> G.unName name - (J.Bool _, (GraphQLType (G.TypeNamed _ name))) -> - unless (G.unName name == "Boolean") $ - throwUnexpected $ "got scalar Boolean for the action webhook response, expecting " <> G.unName name - (J.String _, (GraphQLType (G.TypeNamed _ name))) -> - unless (G.unName name == "String") $ - throwUnexpected $ "got scalar String for the action webhook response, expecting " <> G.unName name - (J.Array _, (GraphQLType (G.TypeNamed _ name))) -> - throwUnexpected $ "got array for the action webhook response, expecting " <> G.unName name - (J.Array objs, (GraphQLType (G.TypeList _ outputType''))) -> - traverse_ (\o -> validateResponse o (GraphQLType outputType'')) objs - (ob@(J.Object _), (GraphQLType (G.TypeNamed _ name))) -> do - case J.parse (fmap AWRObject . J.parseJSON) ob of - J.Error s -> throwUnexpected $ "failed to parse webhookResponse as Object, error: " <> T.pack s -- This should never happen - J.Success (AWRObject awr) -> do - when (isScalar (G.unName name)) $ - throwUnexpected $ "got object for the action webhook response, expecting " <> G.unName name - validateResponseObject awr - J.Success _ -> - throwUnexpected "Webhook response not an object" -- This should never happen - -- Expected output type is an array and webhook response is not an array - (_, (GraphQLType (G.TypeList _ _))) -> - throwUnexpected $ "expecting array for the action webhook response" + validateResponse :: J.Value -> GraphQLType -> IR.ActionOutputFields -> m () + validateResponse webhookResponse' outputType' outputF = + unless (isCustomScalar (unGraphQLType outputType') outputF) do + case (webhookResponse', outputType') of + (J.Null, _) -> unless (isNullableType outputType') $ throwUnexpected "got null for the action webhook response" + (J.Number _, (GraphQLType (G.TypeNamed _ name))) -> do + unless (G.unName name == G.unName intScalar || G.unName name == G.unName floatScalar) $ + throwUnexpected $ "got scalar Number for the action webhook response, expecting " <> G.unName name + (J.Bool _, (GraphQLType (G.TypeNamed _ name))) -> + unless (G.unName name == G.unName boolScalar) $ + throwUnexpected $ "got scalar Boolean for the action webhook response, expecting " <> G.unName name + (J.String _, (GraphQLType (G.TypeNamed _ name))) -> + unless (G.unName name == G.unName stringScalar || G.unName name == G.unName idScalar) $ + throwUnexpected $ "got scalar String for the action webhook response, expecting " <> G.unName name + (J.Array _, (GraphQLType (G.TypeNamed _ name))) -> + throwUnexpected $ "got array for the action webhook response, expecting " <> G.unName name + (J.Array objs, (GraphQLType (G.TypeList _ outputType''))) -> + traverse_ (\o -> validateResponse o (GraphQLType outputType'') outputF) objs + (ob@(J.Object _), (GraphQLType (G.TypeNamed _ name))) -> do + case J.parse (fmap AWRObject . J.parseJSON) ob of + J.Error s -> throwUnexpected $ "failed to parse webhookResponse as Object, error: " <> T.pack s -- This should never happen + J.Success (AWRObject awr) -> do + when (isInBuiltScalar (G.unName name)) $ + throwUnexpected $ "got object for the action webhook response, expecting " <> G.unName name + validateResponseObject awr + J.Success _ -> + throwUnexpected "Webhook response not an object" -- This should never happen + -- Expected output type is an array and webhook response is not an array + (_, (GraphQLType (G.TypeList _ _))) -> + throwUnexpected $ "expecting array for the action webhook response" processOutputSelectionSet :: TF.ArgumentExp v -> diff --git a/server/src-lib/Hasura/RQL/DDL/Action.hs b/server/src-lib/Hasura/RQL/DDL/Action.hs index 62bdda91a5e..5de0f5bb17c 100644 --- a/server/src-lib/Hasura/RQL/DDL/Action.hs +++ b/server/src-lib/Hasura/RQL/DDL/Action.hs @@ -147,8 +147,10 @@ resolveAction env AnnotatedCustomTypes {..} ActionDefinition {..} allScalars = d pure aoTScalar | Just objectType <- Map.lookup outputBaseType _actObjects -> pure $ AOTObject objectType + | Just (NOCTScalar s) <- Map.lookup outputBaseType _actNonObjects -> do + pure (AOTScalar s) | otherwise -> - throw400 NotExists ("the type: " <> dquote outputBaseType <> " is not an object type defined in custom types") + throw400 NotExists ("the type: " <> dquote outputBaseType <> " is not an object or scalar type defined in custom types") -- If the Action is sync: -- 1. Check if the output type has only top level relations (if any) -- If the Action is async: diff --git a/server/src-lib/Hasura/RQL/Types/CustomTypes.hs b/server/src-lib/Hasura/RQL/Types/CustomTypes.hs index 5091273e2f0..f6cb0b28354 100644 --- a/server/src-lib/Hasura/RQL/Types/CustomTypes.hs +++ b/server/src-lib/Hasura/RQL/Types/CustomTypes.hs @@ -6,6 +6,7 @@ module Hasura.RQL.Types.CustomTypes GraphQLType (..), isListType, isNullableType, + isInBuiltScalar, EnumTypeName (..), EnumValueDefinition (..), EnumTypeDefinition (..), @@ -86,6 +87,15 @@ isListType (GraphQLType ty) = G.isListType ty isNullableType :: GraphQLType -> Bool isNullableType (GraphQLType ty) = G.isNullable ty +isInBuiltScalar :: Text -> Bool +isInBuiltScalar s + | s == G.unName intScalar = True + | s == G.unName floatScalar = True + | s == G.unName stringScalar = True + | s == G.unName boolScalar = True + | s == G.unName idScalar = True + | otherwise = False + 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 9b13fafdc01..41c39d179fb 100644 --- a/server/tests-py/context.py +++ b/server/tests-py/context.py @@ -372,7 +372,15 @@ class ActionsWebhookHandler(http.server.BaseHTTPRequestHandler): elif req_path == "/scalar-response": self._send_response(HTTPStatus.OK, "some-string") + + elif req_path == "/json-response": + resp, status = self.json_response() + self._send_response(status, resp) + elif req_path == "/custom-scalar-array-response": + resp, status = self.custom_scalar_array_response() + self._send_response(status, resp) + elif req_path == "/scalar-array-response": self._send_response(HTTPStatus.OK, ["foo", "bar", None]) @@ -602,6 +610,18 @@ class ActionsWebhookHandler(http.server.BaseHTTPRequestHandler): def null_response(self): response = None return response, HTTPStatus.OK + + def json_response(self): + response = { + 'foo': 'bar' + } + return response, HTTPStatus.OK + + def custom_scalar_array_response(self): + response = [{ + 'foo': 'bar' + }] + return response, HTTPStatus.OK def recursive_output(self): return { diff --git a/server/tests-py/queries/actions/sync/expecting_custom_scalar_array_response_success.yaml b/server/tests-py/queries/actions/sync/expecting_custom_scalar_array_response_success.yaml new file mode 100644 index 00000000000..cd7b50fdc42 --- /dev/null +++ b/server/tests-py/queries/actions/sync/expecting_custom_scalar_array_response_success.yaml @@ -0,0 +1,12 @@ +description: custom scalar array response should be allowed +url: /v1/graphql +status: 200 +query: + query: | + mutation { + custom_scalar_array_response + } +response: + data: + custom_scalar_array_response: + [foo: bar] diff --git a/server/tests-py/queries/actions/sync/expecting_custom_scalar_response_success.yaml b/server/tests-py/queries/actions/sync/expecting_custom_scalar_response_success.yaml new file mode 100644 index 00000000000..2eb6d2f4900 --- /dev/null +++ b/server/tests-py/queries/actions/sync/expecting_custom_scalar_response_success.yaml @@ -0,0 +1,12 @@ +description: custom scalar response should be allowed +url: /v1/graphql +status: 200 +query: + query: | + mutation { + custom_scalar_response + } +response: + data: + custom_scalar_response: + foo: bar \ No newline at end of file diff --git a/server/tests-py/queries/actions/sync/expecting_jsonb_response_success.yaml b/server/tests-py/queries/actions/sync/expecting_jsonb_response_success.yaml new file mode 100644 index 00000000000..d7d984402f2 --- /dev/null +++ b/server/tests-py/queries/actions/sync/expecting_jsonb_response_success.yaml @@ -0,0 +1,12 @@ +description: pgScalar (custom scalar) response should be allowed +url: /v1/graphql +status: 200 +query: + query: | + mutation { + pgscalar_response + } +response: + data: + pgscalar_response: + foo: bar \ No newline at end of file diff --git a/server/tests-py/queries/actions/sync/schema_setup.yaml b/server/tests-py/queries/actions/sync/schema_setup.yaml index 2a186c07ba6..04fb490521e 100644 --- a/server/tests-py/queries/actions/sync/schema_setup.yaml +++ b/server/tests-py/queries/actions/sync/schema_setup.yaml @@ -60,6 +60,9 @@ args: type: String - name: extensions type: json + + scalars: + - name: myCustomScalar objects: - name: UserId @@ -442,6 +445,24 @@ args: output_type: String! handler: http://127.0.0.1:5593/scalar-response +- type: create_action + args: + name: pgscalar_response + definition: + kind: synchronous + arguments: + output_type: json! + handler: http://127.0.0.1:5593/json-response + +- type: create_action + args: + name: custom_scalar_response + definition: + kind: synchronous + arguments: + output_type: myCustomScalar! + handler: http://127.0.0.1:5593/json-response + - type: create_action args: name: scalar_array_response @@ -451,6 +472,24 @@ args: output_type: '[String]!' handler: http://127.0.0.1:5593/scalar-array-response +- type: create_action + args: + name: custom_scalar_array_response + definition: + kind: synchronous + arguments: + output_type: '[myCustomScalar!]!' + handler: http://127.0.0.1:5593/custom-scalar-array-response + +- type: create_action + args: + name: custom_scalar_nested_array_response + definition: + kind: synchronous + arguments: + output_type: '[[myCustomScalar!]]!' + handler: http://127.0.0.1:5593/custom-scalar-array-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 3117fcd3d0c..76755cddc14 100644 --- a/server/tests-py/queries/actions/sync/schema_teardown.yaml +++ b/server/tests-py/queries/actions/sync/schema_teardown.yaml @@ -60,10 +60,26 @@ args: args: name: scalar_response clear_data: true +- type: drop_action + args: + name: pgscalar_response + clear_data: true +- type: drop_action + args: + name: custom_scalar_response + clear_data: true - type: drop_action args: name: scalar_array_response clear_data: true +- type: drop_action + args: + name: custom_scalar_array_response + clear_data: true +- type: drop_action + args: + name: custom_scalar_nested_array_response + clear_data: true - type: drop_action args: name: recursive_output diff --git a/server/tests-py/test_actions.py b/server/tests-py/test_actions.py index 30ca9d77afd..f2a40234fd3 100644 --- a/server/tests-py/test_actions.py +++ b/server/tests-py/test_actions.py @@ -96,7 +96,7 @@ class TestActionsSync: def test_expecting_scalar_string_output_type_got_object(self, hge_ctx): check_query_secret(hge_ctx, self.dir() + '/expecting_scalar_response_got_object.yaml') - + def test_expecting_object_output_type_got_scalar_string(self, hge_ctx): check_query_secret(hge_ctx, self.dir() + '/expecting_object_response_got_scalar.yaml') @@ -118,6 +118,34 @@ class TestActionsSync: def test_expecting_object_response_with_nested_null(self, hge_ctx): check_query_f(hge_ctx, self.dir() + '/expecting_object_response_with_nested_null.yaml') + + def test_expecting_jsonb_response_success(self, hge_ctx): + check_query_f(hge_ctx, self.dir() + '/expecting_jsonb_response_success.yaml') + + def test_expecting_custom_scalar_response_success(self, hge_ctx): + check_query_f(hge_ctx, self.dir() + '/expecting_custom_scalar_response_success.yaml') + + def test_expecting_custom_scalar_array_response_success(self, hge_ctx): + check_query_f(hge_ctx, self.dir() + '/expecting_custom_scalar_array_response_success.yaml') + + def test_expecting_custom_scalar_array_response_got_different_type(self, hge_ctx): + query_obj = { + "query": """ + mutation { + custom_scalar_nested_array_response + } + """ + } + headers = {} + admin_secret = hge_ctx.hge_key + if admin_secret is not None: + headers['X-Hasura-Admin-Secret'] = admin_secret + code, resp, _ = hge_ctx.anyq('/v1/graphql', query_obj, headers) + assert code == 200, resp + + error_message = resp['errors'][0]['message'] + + assert error_message == 'expecting array for the action webhook response', error_message def test_expecting_object_response_with_nested_null_wrong_field(self, hge_ctx): query_obj = {