Fix(GraphQL): Fix duplicate XID error in case of interface XIDs (#7776)

* Fix second scenario * Fix scenario 1 * Add tests * Add comments
hypermodeinc · May 11, 2021 · 6fbca75 · 6fbca75
1 parent 042d35c
commit 6fbca75
Show file tree

Hide file tree

Showing 10 changed files with 279 additions and 108 deletions.
diff --git a/graphql/e2e/common/common.go b/graphql/e2e/common/common.go
@@ -901,6 +901,8 @@ func RunAll(t *testing.T) {
 	t.Run("Update language tag fields", updateLangTagFields)
 	t.Run("mutation with @id field and interface arg", mutationWithIDFieldHavingInterfaceArg)
 	t.Run("xid update and nullable tests", xidUpdateAndNullableTests)
+	t.Run("Referencing same node containing multiple XIDs",
+		referencingSameNodeWithMultipleXIds)
 
 	// error tests
 	t.Run("graphql completion on", graphQLCompletionOn)

diff --git a/graphql/e2e/common/mutation.go b/graphql/e2e/common/mutation.go
@@ -6620,3 +6620,77 @@ func xidUpdateAndNullableTests(t *testing.T) {
 	DeleteGqlType(t, "Employer", filterEmployer, 2, nil)
 	DeleteGqlType(t, "Worker", filterWorker, 4, nil)
 }
+
+func referencingSameNodeWithMultipleXIds(t *testing.T) {
+	params := &GraphQLParams{
+		Query: `mutation($input: [AddPerson1Input!]!) {
+					addPerson1(input: $input) {
+						person1 {
+							regId
+							name
+							friends {
+								regId
+								name
+							}
+							closeFriends {
+								regId
+								name
+							}
+						}
+					}
+				}`,
+		Variables: map[string]interface{}{
+			"input": []interface{}{
+				map[string]interface{}{
+					"regId": "7",
+					"name":  "7th Person",
+					"name1": "seventh Person",
+					"friends": []interface{}{
+						map[string]interface{}{
+							"regId": "8",
+							"name":  "8th Person",
+							"name1": "eighth Person",
+						},
+					},
+					"closeFriends": []interface{}{
+						map[string]interface{}{
+							"regId": "8",
+							"name":  "8th Person",
+						},
+					},
+				},
+			},
+		},
+	}
+
+	gqlResponse := postExecutor(t, GraphqlURL, params)
+	RequireNoGQLErrors(t, gqlResponse)
+
+	expected := `{
+					"addPerson1":
+						{
+							"person1": [
+								{
+									"closeFriends": [
+										{
+											"name": "8th Person",
+											"regId": "8"
+										}
+									],
+									"friends": [
+										{
+											"name": "8th Person",
+        							      	"regId": "8"
+										}
+									],
+									"name": "7th Person",
+									"regId": "7"
+								}
+							]
+						}
+					}`
+	testutil.CompareJSON(t, expected, string(gqlResponse.Data))
+
+	// cleanup
+	DeleteGqlType(t, "Person1", map[string]interface{}{}, 2, nil)
+}
diff --git a/graphql/e2e/directives/schema.graphql b/graphql/e2e/directives/schema.graphql
@@ -206,7 +206,9 @@ type post1{
 
 type Person1 {
     id: ID!
-    name: String!
+    name: String! @id
+    name1: String @id
+    regId: String @id
     closeFriends: [Person1] @hasInverse(field: closeFriends)
     friends: [Person1] @hasInverse(field: friends)
 }

diff --git a/graphql/e2e/directives/schema_response.json b/graphql/e2e/directives/schema_response.json
@@ -469,7 +469,30 @@
     },
     {
       "predicate": "Person1.name",
-      "type": "string"
+      "type": "string",
+      "upsert": true,
+      "tokenizer": [
+        "hash"
+      ],
+      "index": true
+    },
+    {
+      "predicate": "Person1.regId",
+      "type": "string",
+      "upsert": true,
+      "tokenizer": [
+        "hash"
+      ],
+      "index": true
+    },
+    {
+      "predicate": "Person1.name1",
+      "type": "string",
+      "upsert": true,
+      "tokenizer": [
+        "hash"
+      ],
+      "index": true
     },
     {
       "predicate": "Plant.breed",
@@ -1209,6 +1232,12 @@
         },
         {
           "name": "Person1.closeFriends"
+        },
+        {
+          "name": "Person1.name1"
+        },
+        {
+          "name": "Person1.regId"
         }
       ],
       "name": "Person1"

diff --git a/graphql/e2e/normal/schema.graphql b/graphql/e2e/normal/schema.graphql
@@ -219,7 +219,9 @@ type post1{
 
 type Person1 {
     id: ID!
-    name: String!
+    name: String! @id
+    name1: String @id
+    regId: String @id
     closeFriends: [Person1] @hasInverse(field: closeFriends)
     friends: [Person1] @hasInverse(field: friends)
 }

diff --git a/graphql/e2e/normal/schema_response.json b/graphql/e2e/normal/schema_response.json
@@ -523,7 +523,30 @@
     },
     {
       "predicate": "Person1.name",
-      "type": "string"
+      "index": true,
+      "type": "string",
+      "tokenizer": [
+        "hash"
+      ],
+      "upsert": true
+    },
+    {
+      "predicate": "Person1.name1",
+      "index": true,
+      "type": "string",
+      "tokenizer": [
+        "hash"
+      ],
+      "upsert": true
+    },
+    {
+      "predicate": "Person1.regId",
+      "index": true,
+      "type": "string",
+      "tokenizer": [
+        "hash"
+      ],
+      "upsert": true
     },
     {
       "predicate": "Plant.breed",
@@ -1237,6 +1260,12 @@
         {
           "name": "Person1.name"
         },
+        {
+          "name": "Person1.name1"
+        },
+        {
+          "name": "Person1.regId"
+        },
         {
           "name": "Person1.friends"
         },

diff --git a/graphql/resolve/add_mutation_test.yaml b/graphql/resolve/add_mutation_test.yaml
@@ -661,8 +661,10 @@
     }
   explanation: "A reference must be a valid UID"
   error:
-    { "message":
-        "failed to rewrite mutation payload because ID argument (HI!) was not able to be parsed" }
+    {
+      "message":
+        "failed to rewrite mutation payload because ID argument (HI!) was not able to be parsed"
+    }
 
 -
   name: "Add mutation with inverse reference"
@@ -2741,43 +2743,6 @@
     message: |-
       failed to rewrite mutation payload because duplicate XID found: S1
 
-- name: "Duplicate XIDs in single mutation for Interface"
-  gqlmutation: |
-    mutation addStudent($input: [AddStudentInput!]!) {
-      addStudent(input: $input) {
-        student {
-          xid
-          name
-          taughtBy {
-            xid
-            name
-            subject
-          }
-        }
-      }
-    }
-  gqlvariables: |
-    {
-      "input": [
-        {
-          "xid": "S1",
-          "name": "Stud1"
-        },
-        {
-          "xid": "S2",
-          "name": "Stud2",
-          "taughtBy": [
-            {"xid": "S1", "name": "Teacher1", "subject": "Sub1"}
-          ]
-        }
-      ]
-    }
-  explanation: "When duplicate XIDs are given as input for an Interface in a single mutation, it
-  should return error."
-  error:
-    message: |-
-      failed to rewrite mutation payload because duplicate XID found: S1
-
 # Additional Deletes
 #
 # If we have
@@ -5500,3 +5465,42 @@
     {
       "message": "failed to rewrite mutation payload because field id cannot be empty"
     }
+
+-
+  name: "Add Mutation referencing same XID in different types"
+  gqlmutation: |
+    mutation($input: [AddT1Input!]!) {
+      addT1(input: $input) {
+        t1 {
+          name
+          name1
+          name2
+          link {
+            name
+            name1
+            name3
+          }
+        }
+      }
+    }
+  gqlvariables: |
+    {
+      "input": [
+        {
+          "name": "Bob",
+          "name1": "Bob11",
+          "name2": "Bob2",
+          "link": {
+            "name": "Bob"
+          }
+        }
+      ]
+    }
+  explanation: "As the link and top level object contain the same XID, Bob, this should throw an error"
+  error:
+    {
+      "message":
+        "failed to rewrite mutation payload because using duplicate XID value: Bob for XID: name for two different
+         implementing types of same interfaces: T1 and T2"
+    }
+