From ef33a61266f1837393d6c13905b58702f46f8880 Mon Sep 17 00:00:00 2001
From: Karthikeyan Chinnakonda <karthikeyan@hasura.io>
Date: Thu, 24 Dec 2020 15:41:05 +0530
Subject: [PATCH] server: fix handling of empty array values in relationships
 of set_custom_types API (#219)

* server: fix handling of empty array values in relationships of set_custom_types

* add a test for dropping the relationship

GitOrigin-RevId: abff138b0021647a1cb6c6c041c2ba53c1ff9b77
---
 CHANGELOG.md                                  |  1 +
 .../src-lib/Hasura/RQL/Types/CustomTypes.hs   | 16 ++++++++-
 .../custom-types/drop_relationship.yaml       | 34 +++++++++++++++++++
 server/tests-py/test_actions.py               |  3 ++
 4 files changed, 53 insertions(+), 1 deletion(-)
 create mode 100644 server/tests-py/queries/actions/custom-types/drop_relationship.yaml

diff --git a/CHANGELOG.md b/CHANGELOG.md
index d0d01114fc5..a3d4dcca571 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -86,6 +86,7 @@ and be accessible according to the permissions that were configured for the role
 - server: do not block catalog migration on inconsistent metadata
 - server: update `forkImmortal` function to log more information, i.e log starting of threads and log asynchronous and synchronous exception.
 - 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)
 - console: allow user to cascade Postgres dependencies when dropping Postgres objects (close #5109) (#5248)
 - console: mark inconsistent remote schemas in the UI (close #5093) (#5181)
 - console: remove ONLY as default for ALTER TABLE in column alter operations (close #5512) #5706
diff --git a/server/src-lib/Hasura/RQL/Types/CustomTypes.hs b/server/src-lib/Hasura/RQL/Types/CustomTypes.hs
index 56917f00c57..917213b410a 100644
--- a/server/src-lib/Hasura/RQL/Types/CustomTypes.hs
+++ b/server/src-lib/Hasura/RQL/Types/CustomTypes.hs
@@ -150,7 +150,21 @@ data ObjectTypeDefinition a b c
   } deriving (Show, Eq, Generic)
 instance (NFData a, NFData b, NFData c) => NFData (ObjectTypeDefinition a b c)
 instance (Cacheable a, Cacheable b, Cacheable c) => Cacheable (ObjectTypeDefinition a b c)
-$(J.deriveJSON (J.aesonDrop 4 J.snakeCase) ''ObjectTypeDefinition)
+$(J.deriveToJSON (J.aesonDrop 4 J.snakeCase) ''ObjectTypeDefinition)
+
+instance (J.FromJSON a, J.FromJSON b, J.FromJSON c) => J.FromJSON (ObjectTypeDefinition a b c) where
+  parseJSON = J.withObject "ObjectTypeDefinition" \obj -> do
+    name <- obj J..: "name"
+    desc <- obj J..:? "description"
+    fields <- obj J..: "fields"
+    relationships <- obj J..:? "relationships"
+    -- We need to do the below because pre-PDV, '[]' was a legal value
+    -- for relationships because the type was `(Maybe [TypeRelationshipDefinition])`,
+    -- In PDV, the type was changed to `(Maybe (NonEmpty (TypeRelationship b c)))`
+    -- which breaks on `[]` for the `relationships` field, to be backwards compatible
+    -- this `FromJSON` instance is written by hand and `[]` sets `_otdRelationships`
+    -- to `Nothing`
+    return $ ObjectTypeDefinition name desc fields (nonEmpty =<< relationships)
 
 data ScalarTypeDefinition
   = ScalarTypeDefinition
diff --git a/server/tests-py/queries/actions/custom-types/drop_relationship.yaml b/server/tests-py/queries/actions/custom-types/drop_relationship.yaml
new file mode 100644
index 00000000000..707f1c79be1
--- /dev/null
+++ b/server/tests-py/queries/actions/custom-types/drop_relationship.yaml
@@ -0,0 +1,34 @@
+- description: Set custom types with an object relationship
+  url: /v1/query
+  status: 200
+  query:
+    type: set_custom_types
+    args:
+      objects:
+      - name: User
+        fields:
+        - name: user_id
+          type: uuid!
+        - name: name
+          type: 'String!'
+        relationships:
+        - name: Names
+          type: array
+          remote_table: user
+          field_mapping:
+            name: name
+
+- description: drop the custom type relationship
+  url: /v1/query
+  status: 200
+  query:
+    type: set_custom_types
+    args:
+      objects:
+      - name: User
+        fields:
+        - name: user_id
+          type: uuid!
+        - name: name
+          type: 'String!'
+        relationships: []
diff --git a/server/tests-py/test_actions.py b/server/tests-py/test_actions.py
index 8113d5ace93..426bc94742a 100644
--- a/server/tests-py/test_actions.py
+++ b/server/tests-py/test_actions.py
@@ -445,6 +445,9 @@ class TestSetCustomTypes:
     def test_list_type_relationship(self, hge_ctx):
         check_query_f(hge_ctx, self.dir() + '/list_type_relationship.yaml')
 
+    def test_drop_relationship(self, hge_ctx):
+        check_query_f(hge_ctx, self.dir() + '/drop_relationship.yaml')
+
 @pytest.mark.usefixtures('per_class_tests_db_state')
 class TestActionsMetadata: