Fix action relationship type and input arguments (closes #6402) (#284)

Co-authored-by: Antoine Leblanc <antoine@hasura.io>
GITHUB_PR_NUMBER: 6417
GITHUB_PR_URL: https://github.com/hasura/graphql-engine/pull/6417
GitOrigin-RevId: 37b67a4d04e0ed3b16fc5fc9bf025b24b1f1bf6e

CHANGELOG.md

@@ -88,6 +88,8 @@ and be accessible according to the permissions that were configured for the role
 - server: various changes to ensure timely cleanup of background threads and other resources in the event of a SIGTERM signal.
 - server: fix issue when the `relationships` field in `objects` field is passed `[]` in the `set_custom_types` API (fix #6357)
 - server: fix issue with event triggers defined on a table which is partitioned (fixes #6261)
+- server: action array relationships now support the same input arguments (such as where or distinct_on) as usual relationships
+- server: action array relationships now support aggregate relationships
 - server: fix issue with non-optional fields of the remote schema being added as optional in the graphql-engine (fix #6401)
 - server: accept new config `allowed_skew` in JWT config to provide leeway for JWT expiry (fixes #2109)
 - server: fix issue with query actions with relationship with permissions configured on the remote table (fix #6385)

server/src-lib/Hasura/GraphQL/Schema/Action.hs

@@ -4,20 +4,21 @@ module Hasura.GraphQL.Schema.Action
   , actionAsyncQuery
   ) where
 
-import           Data.Has
 import           Hasura.Prelude
 
 import qualified Data.Aeson                            as J
 import qualified Data.HashMap.Strict                   as Map
 import qualified Language.GraphQL.Draft.Syntax         as G
 
+import           Data.Has
+import           Data.Text.Extended
+import           Data.Text.NonEmpty
+
 import qualified Hasura.GraphQL.Parser                 as P
 import qualified Hasura.GraphQL.Parser.Internal.Parser as P
 import qualified Hasura.RQL.DML.Internal               as RQL
 import qualified Hasura.RQL.IR.Select                  as RQL
 
-import           Data.Text.Extended
-import           Data.Text.NonEmpty
 import           Hasura.Backends.Postgres.SQL.Types
 import           Hasura.Backends.Postgres.SQL.Value
 import           Hasura.GraphQL.Parser                 (FieldParser, InputFieldsParser, Kind (..),
@@ -171,7 +172,7 @@ actionOutputFields annotatedObject = do
       scalarOrEnumFields = map scalarOrEnumFieldParser $ toList $ _otdFields outputObject
   relationshipFields <- forM (_otdRelationships outputObject) $ traverse relationshipFieldParser
   let allFieldParsers = scalarOrEnumFields <>
-                        maybe [] (catMaybes . toList) relationshipFields
+                        maybe [] (concat . catMaybes . toList) relationshipFields
       outputTypeName = unObjectTypeName $ _otdName outputObject
       outputTypeDescription = _otdDescription outputObject
   pure $ P.selectionSet outputTypeName outputTypeDescription allFieldParsers
@@ -195,26 +196,37 @@ actionOutputFields annotatedObject = do
 
     relationshipFieldParser
       :: TypeRelationship (TableInfo 'Postgres) (ColumnInfo 'Postgres)
-      -> m (Maybe (FieldParser n (RQL.AnnFieldG 'Postgres (UnpreparedValue 'Postgres))))
-    relationshipFieldParser typeRelationship = runMaybeT do
-      let TypeRelationship relName relType _ tableInfo fieldMapping = typeRelationship
-          tableName = _tciName $ _tiCoreInfo tableInfo
-          fieldName = unRelationshipName relName
-      roleName <- lift askRoleName
+      -> m (Maybe [FieldParser n (RQL.AnnFieldG 'Postgres (UnpreparedValue 'Postgres))])
+    relationshipFieldParser (TypeRelationship relName relType _ tableInfo fieldMapping) = runMaybeT do
+      let tableName     = _tciName $ _tiCoreInfo tableInfo
+          fieldName     = unRelationshipName relName
+          tableRelName  = RelName $ mkNonEmptyTextUnsafe $ G.unName fieldName
+          columnMapping = Map.fromList $ do
+            (k, v) <- Map.toList fieldMapping
+            pure (unsafePGCol $ G.unName $ unObjectFieldName k, pgiColumn v)
+      roleName   <- lift askRoleName
       tablePerms <- MaybeT $ pure $ RQL.getPermInfoMaybe roleName PASelect tableInfo
-      tableParser <- lift $ selectTable tableName fieldName Nothing tablePerms
-      pure $ tableParser <&> \selectExp ->
-        let tableRelName = RelName $ mkNonEmptyTextUnsafe $ G.unName fieldName
-            columnMapping = Map.fromList $
-              [ (unsafePGCol $ G.unName $ unObjectFieldName k, pgiColumn v)
-              | (k, v) <- Map.toList fieldMapping
-              ]
-        in case relType of
-              ObjRel -> RQL.AFObjectRelation $ RQL.AnnRelationSelectG tableRelName columnMapping $
-                        RQL.AnnObjectSelectG (RQL._asnFields selectExp) tableName $
-                        RQL._tpFilter $ RQL._asnPerm selectExp
-              ArrRel -> RQL.AFArrayRelation $ RQL.ASSimple $
-                        RQL.AnnRelationSelectG tableRelName columnMapping selectExp
+      case relType of
+        ObjRel -> do
+          let desc = Just $ G.Description "An object relationship"
+          selectionSetParser <- lift $ tableSelectionSet tableName tablePerms
+          pure $ pure $ P.nonNullableField $
+            P.subselection_ fieldName desc selectionSetParser
+              <&> \fields -> RQL.AFObjectRelation $ RQL.AnnRelationSelectG tableRelName columnMapping $
+                             RQL.AnnObjectSelectG fields tableName $
+                             fmapAnnBoolExp partialSQLExpToUnpreparedValue $ spiFilter tablePerms
+        ArrRel -> do
+          let desc = Just $ G.Description "An array relationship"
+          otherTableParser <- lift $ selectTable tableName fieldName desc tablePerms
+          let arrayRelField = otherTableParser <&> \selectExp -> RQL.AFArrayRelation $
+                RQL.ASSimple $ RQL.AnnRelationSelectG tableRelName columnMapping selectExp
+              relAggFieldName = fieldName <> $$(G.litName "_aggregate")
+              relAggDesc      = Just $ G.Description "An aggregate relationship"
+          tableAggField <- lift $ selectTableAggregate tableName relAggFieldName relAggDesc tablePerms
+          pure $ catMaybes [ Just arrayRelField
+                           , fmap (RQL.AFArrayRelation . RQL.ASAggregate . RQL.AnnRelationSelectG tableRelName columnMapping) <$> tableAggField
+                           ]
+
 
 mkDefinitionList :: AnnotatedObjectType 'Postgres -> [(PGCol, ScalarType 'Postgres)]
 mkDefinitionList AnnotatedObjectType{..} =

server/tests-py/queries/actions/sync/create_user_relationship.yaml

@@ -0,0 +1,125 @@
+- description: Run create_user sync action mutation with valid email
+  url: /v1/graphql
+  status: 200
+  query:
+    query: |
+      mutation {
+        create_user(email: "clarke@gmail.com", name: "Clarke"){
+          __typename
+          id
+          user {
+            __typename
+            name
+            email
+            is_admin
+          }
+        }
+      }
+  response:
+    data:
+      create_user:
+        __typename: UserId
+        id: 1
+        user:
+          __typename: user
+          name: Clarke
+          email: clarke@gmail.com
+          is_admin: false
+
+- description: Use user_by_email to get our user and test array relationship
+  url: /v1/graphql
+  status: 200
+  query:
+    query: |
+      query {
+        get_user_by_email(email: "clarke@gmail.com"){
+          __typename
+          id
+          articles {
+            name
+          }
+        }
+      }
+  response:
+    data:
+      get_user_by_email:
+        __typename: UserId
+        id: 1
+        articles:
+          - name: foo
+          - name: bar
+          - name: bar
+
+- description: Use user_by_email to get our user and test array relationship with limit input parameter
+  url: /v1/graphql
+  status: 200
+  query:
+    query: |
+      query {
+        get_user_by_email(email: "clarke@gmail.com"){
+          __typename
+          id
+          articles(limit: 1) {
+            name
+          }
+        }
+      }
+  response:
+    data:
+      get_user_by_email:
+        __typename: UserId
+        id: 1
+        articles:
+          - name: foo
+
+
+- description: Use user_by_email to get our user and test array relationship with distinct input parameter
+  url: /v1/graphql
+  status: 200
+  query:
+    query: |
+      query {
+        get_user_by_email(email: "clarke@gmail.com"){
+          __typename
+          id
+          articles(distinct_on: name) {
+            name
+          }
+        }
+      }
+  response:
+    data:
+      get_user_by_email:
+        __typename: UserId
+        id: 1
+        articles:
+          - name: bar
+          - name: foo
+
+
+- description: Use user_by_email to get our user and test aggregate array relationship
+  url: /v1/graphql
+  status: 200
+  query:
+    query: |
+      query {
+        get_user_by_email(email: "clarke@gmail.com"){
+          __typename
+          id
+          articles_aggregate {
+            aggregate {
+              max {
+                name
+              }
+            }
+          }
+        }
+      }
+  response:
+    data:
+      get_user_by_email:
+        __typename: UserId
+        id: 1
+        articles:
+          - name: bar
+          - name: foo

server/tests-py/queries/actions/sync/create_user_relationship_fail.yaml

@@ -0,0 +1,48 @@
+- description: Run create_user sync action mutation with valid email
+  url: /v1/graphql
+  status: 200
+  query:
+    query: |
+      mutation {
+        create_user(email: "clarke@gmail.com", name: "Clarke"){
+          __typename
+          id
+          user {
+            __typename
+            name
+            email
+            is_admin
+          }
+        }
+      }
+  response:
+    data:
+      create_user:
+        __typename: UserId
+        id: 1
+        user:
+          __typename: user
+          name: Clarke
+          email: clarke@gmail.com
+          is_admin: false
+
+- description: Use user_by_email to get our user and test object relationship
+  url: /v1/graphql
+  status: 200
+  query:
+    query: |
+      query {
+        get_user_by_email(email: "clarke@gmail.com"){
+          __typename
+          id
+          user(limit: 4) {
+            name
+          }
+        }
+      }
+  response:
+    errors:
+    - extensions:
+        path: $.selectionSet.get_user_by_email.selectionSet.user
+        code: validation-failed
+      message: '"user" has no argument named "limit"'

server/tests-py/queries/actions/sync/schema_setup.yaml

@@ -10,12 +10,27 @@ args:
         email TEXT NOT NULL,
         is_admin BOOLEAN NOT NULL DEFAULT false
       );
+      CREATE TABLE "article"(
+        id SERIAL PRIMARY KEY,
+        name TEXT NOT NULL,
+        user_id INTEGER
+      );
+      INSERT INTO "article" (name, user_id) VALUES
+        ('foo', 1),
+        ('bar', 1),
+        ('bar', 1),
+        ('baz', 2);
 
 - type: track_table
   args:
     name: user
     schema: public
 
+- type: track_table
+  args:
+    name: article
+    schema: public
+
 - type: set_custom_types
   args:
     input_objects:
@@ -46,6 +61,11 @@ args:
         remote_table: user
         field_mapping:
           id: id
+      - name: articles
+        type: array
+        remote_table: article
+        field_mapping:
+          id: user_id
 
     - name: OutObject
       fields:

server/tests-py/queries/actions/sync/schema_teardown.yaml

@@ -29,3 +29,4 @@ args:
     cascade: true
     sql: |
       DROP TABLE "user";
+      DROP TABLE "article";

server/tests-py/test_actions.py

@@ -46,6 +46,12 @@ class TestActionsSyncWebsocket:
     def test_create_user_success(self, hge_ctx, transport):
         check_query_f(hge_ctx, self.dir() + '/create_user_success.yaml', transport)
 
+    def test_create_user_relationship(self, hge_ctx, transport):
+        check_query_f(hge_ctx, self.dir() + '/create_user_relationship.yaml', transport)
+
+    def test_create_user_relationship(self, hge_ctx, transport):
+        check_query_f(hge_ctx, self.dir() + '/create_user_relationship_fail.yaml', transport)
+
     def test_create_users_fail(self, hge_ctx, transport):
         check_query_f(hge_ctx, self.dir() + '/create_users_fail.yaml', transport)