From f965756c8d18ad6d2b4011f818bd15fe61a96ca4 Mon Sep 17 00:00:00 2001
From: Rajas Vanjape <rajas@dgraph.io>
Date: Tue, 23 Feb 2021 18:39:02 +0530
Subject: [PATCH] Handle upsert with multiple XIDs in case one of the XIDs does
 not exist

---
 graphql/resolve/add_mutation_test.yaml | 52 +++++++++++++++++++++++++-
 graphql/resolve/mutation_rewriter.go   | 10 ++++-
 2 files changed, 59 insertions(+), 3 deletions(-)

diff --git a/graphql/resolve/add_mutation_test.yaml b/graphql/resolve/add_mutation_test.yaml
index f4271969b87..b8a38c1eee7 100644
--- a/graphql/resolve/add_mutation_test.yaml
+++ b/graphql/resolve/add_mutation_test.yaml
@@ -915,7 +915,7 @@
       cond: "@if(gt(len(State3), 0))"
 
 -
-  name: "Multiple Upsert Mutation with multiple xids where both existence queries result exist"
+  name: "Upsert Mutation with multiple xids where both existence queries result exist"
   gqlmutation: |
     mutation addBook($input: [AddBookInput!]!) {
       addBook(input: $input, upsert: true) {
@@ -962,6 +962,56 @@
         }
       cond: "@if(gt(len(Book2), 0))"
 
+-
+  name: "Upsert Mutation with multiple xids where only one of existence queries result exist"
+  explanation: "Book1 does not exist but Book2 exists. As Book2 exists, this is an upsert.
+    Even though, Book1 does not exist, the mutation should not update ISBN as it is also an XID."
+  gqlmutation: |
+    mutation addBook($input: [AddBookInput!]!) {
+      addBook(input: $input, upsert: true) {
+        book {
+          title
+          ISBN
+        }
+      }
+    }
+  gqlvariables: |
+    { "input":
+      [
+        {
+          "title": "Sapiens",
+          "ISBN": "NSW",
+          "publisher": "penguin"
+        }
+      ]
+    }
+  dgquery: |-
+    query {
+      Book1(func: eq(Book.ISBN, "NSW")) @filter(type(Book)) {
+        uid
+      }
+      Book2(func: eq(Book.title, "Sapiens")) @filter(type(Book)) {
+        uid
+      }
+    }
+  qnametouid: |-
+    {
+      "Book2": "0x11"
+    }
+  dgquerysec: |-
+    query {
+      Book2 as addBook(func: uid(0x11)) @filter(type(Book)) {
+        uid
+      }
+    }
+  dgmutations:
+    - setjson: |
+        {
+          "uid" : "uid(Book2)",
+          "Book.publisher": "penguin"
+        }
+      cond: "@if(gt(len(Book2), 0))"
+
 -
   name: "Multiple Upsert Mutation 2"
   explanation: "The first state exists and is updated. Second is created. Country
diff --git a/graphql/resolve/mutation_rewriter.go b/graphql/resolve/mutation_rewriter.go
index bc93eb2d7e6..e7fabbcb97c 100644
--- a/graphql/resolve/mutation_rewriter.go
+++ b/graphql/resolve/mutation_rewriter.go
@@ -1413,8 +1413,6 @@ func rewriteObject(
 							// updating this node.
 							upsertVar = variable
 							srcUID = fmt.Sprintf("uid(%s)", variable)
-							// To ensure that xid is not added to the output json
-							delete(obj, xid.Name())
 						} else {
 							// We return an error as we are at top level of non-upsert mutation and the XID exists.
 							// We need to conceal the error because we might be leaking information to the user if it
@@ -1501,6 +1499,14 @@ func rewriteObject(
 					return nil, upsertVar, retErrors
 				}
 			}
+		} else {
+			// In case this is known to be an Upsert. We delete all entries of XIDs
+			// from obj. This is done to prevent any XID entries in the json which is returned
+			// by rewriteObject and ensure that no XID value gets rewritten due to upsert.
+			for _, xid := range xids {
+				// To ensure that xid is not added to the output json in case of upsert
+				delete(obj, xid.Name())
+			}
 		}
 	}