graphql: ensure @id uniqueness within a mutation

graphql/admin/update_group.go

@@ -35,7 +35,7 @@ func (urw *updateGroupRewriter) Rewrite(m schema.Mutation) (*gql.GraphQuery,
 
 	var errSet, errDel error
 	var mutSet, mutDel []*dgoapi.Mutation
-	varGen := resolve.VariableGenerator(0)
+	varGen := resolve.NewVariableGenerator()
 	ruleType := m.MutatedType().Field("rules").Type()
 
 	if setArg != nil {
@@ -46,7 +46,7 @@ func (urw *updateGroupRewriter) Rewrite(m schema.Mutation) (*gql.GraphQuery,
 		}
 		for _, ruleI := range rules {
 			rule := ruleI.(map[string]interface{})
-			variable := varGen.Next(ruleType)
+			variable := varGen.Next(ruleType, "", "")
 			predicate := rule["predicate"]
 			permission := rule["permission"]
 
@@ -92,7 +92,7 @@ func (urw *updateGroupRewriter) Rewrite(m schema.Mutation) (*gql.GraphQuery,
 				continue
 			}
 
-			variable := varGen.Next(ruleType)
+			variable := varGen.Next(ruleType, "", "")
 			addAclRuleQuery(upsertQuery, predicate.(string), variable)
 
 			deleteJson := []byte(fmt.Sprintf(`[

graphql/e2e/common/common.go

@@ -140,6 +140,7 @@ type state struct {
 	ID      string   `json:"id,omitempty"`
 	Name    string   `json:"name,omitempty"`
 	Code    string   `json:"xcode,omitempty"`
+	Capital string   `json:"capital,omitempty"`
 	Country *country `json:"country,omitempty"`
 }
 
@@ -154,6 +155,21 @@ type director struct {
 	Name string `json:"name,omitempty"`
 }
 
+type teacher struct {
+	ID      string     `json:"id,omitempty"`
+	Xid     string     `json:"xid,omitempty"`
+	Name    string     `json:"name,omitempty"`
+	Subject string     `json:"subject,omitempty"`
+	Teaches []*student `json:"teaches,omitempty"`
+}
+
+type student struct {
+	ID       string     `json:"id,omitempty"`
+	Xid      string     `json:"xid,omitempty"`
+	Name     string     `json:"name,omitempty"`
+	TaughtBy []*teacher `json:"taughtBy,omitempty"`
+}
+
 func BootstrapServer(schema, data []byte) {
 	err := checkGraphQLStarted(graphqlAdminURL)
 	if err != nil {
@@ -278,6 +294,7 @@ func RunAll(t *testing.T) {
 	t.Run("numUids test", testNumUids)
 	t.Run("empty delete", mutationEmptyDelete)
 	t.Run("password in mutation", passwordTest)
+	t.Run("duplicate xid in single mutation", deepMutationDuplicateXIDsSameObjectTest)
 
 	// error tests
 	t.Run("graphql completion on", graphQLCompletionOn)

graphql/e2e/common/mutation.go

@@ -305,6 +305,8 @@ func ignoreOpts() []cmp.Option {
 		cmpopts.IgnoreFields(post{}, "PostID"),
 		cmpopts.IgnoreFields(state{}, "ID"),
 		cmpopts.IgnoreFields(category{}, "ID"),
+		cmpopts.IgnoreFields(teacher{}, "ID"),
+		cmpopts.IgnoreFields(student{}, "ID"),
 	}
 }
 
@@ -2370,21 +2372,41 @@ func addState(t *testing.T, name string, executeRequest requestExecutor) *state
 func deleteState(
 	t *testing.T,
 	filter map[string]interface{},
-	deleteStateExpected string,
+	expectedNumUids int,
 	expectedErrors x.GqlErrorList) {
+	deleteGqlType(t, "State", filter, expectedNumUids, expectedErrors)
+}
 
-	deleteStateParams := &GraphQLParams{
-		Query: `mutation deleteState($filter: StateFilter!) {
-			deleteState(filter: $filter) { msg }
-		}`,
+func deleteGqlType(
+	t *testing.T,
+	typeName string,
+	filter map[string]interface{},
+	expectedNumUids int,
+	expectedErrors x.GqlErrorList) {
+
+	deleteTypeParams := &GraphQLParams{
+		Query: fmt.Sprintf(`mutation delete%s($filter: %sFilter!) {
+			delete%s(filter: $filter) { msg numUids }
+		}`, typeName, typeName, typeName),
 		Variables: map[string]interface{}{"filter": filter},
 	}
 
-	gqlResponse := deleteStateParams.ExecuteAsPost(t, graphqlURL)
-	require.JSONEq(t, deleteStateExpected, string(gqlResponse.Data))
+	gqlResponse := deleteTypeParams.ExecuteAsPost(t, graphqlURL)
+	if len(expectedErrors) == 0 {
+		requireNoGQLErrors(t, gqlResponse)
+
+		var result map[string]interface{}
+		err := json.Unmarshal(gqlResponse.Data, &result)
+		require.NoError(t, err)
 
-	if diff := cmp.Diff(expectedErrors, gqlResponse.Errors); diff != "" {
-		t.Errorf("errors mismatch (-want +got):\n%s", diff)
+		deleteField := fmt.Sprintf(`delete%s`, typeName)
+		deleteType := result[deleteField].(map[string]interface{})
+		require.Equal(t, "Deleted", deleteType["msg"])
+		require.Equal(t, expectedNumUids, int(deleteType["numUids"].(float64)))
+	} else {
+		if diff := cmp.Diff(expectedErrors, gqlResponse.Errors); diff != "" {
+			t.Errorf("errors mismatch (-want +got):\n%s", diff)
+		}
 	}
 }
 
@@ -2412,9 +2434,8 @@ func addMutationWithXid(t *testing.T, executeRequest requestExecutor) {
 	require.Contains(t, gqlResponse.Errors[0].Error(),
 		"because id cal already exists for type State")
 
-	deleteStateExpected := `{"deleteState" : { "msg": "Deleted" } }`
 	filter := map[string]interface{}{"xcode": map[string]interface{}{"eq": "cal"}}
-	deleteState(t, filter, deleteStateExpected, nil)
+	deleteState(t, filter, 1, nil)
 }
 
 func addMutationWithXID(t *testing.T) {
@@ -2881,3 +2902,125 @@ func passwordTest(t *testing.T) {
 
 	deleteUser(t, *newUser)
 }
+
+func deepMutationDuplicateXIDsSameObjectTest(t *testing.T) {
+	newStudents := []*student{
+		{
+			Xid:  "S0",
+			Name: "Stud0",
+			TaughtBy: []*teacher{
+				{
+					Xid:     "T0",
+					Name:    "Teacher0",
+					Subject: "English",
+				},
+			},
+		},
+		{
+			Xid:  "S1",
+			Name: "Stud1",
+			TaughtBy: []*teacher{
+				{
+					Xid:     "T0",
+					Name:    "Teacher0",
+					Subject: "English",
+				},
+				{
+					Xid:     "T0",
+					Name:    "Teacher0",
+					Subject: "English",
+				},
+			},
+		},
+	}
+
+	addStudentParams := &GraphQLParams{
+		Query: `mutation addStudent($input: [AddStudentInput!]!) {
+			addStudent(input: $input) {
+				student {
+					xid
+					name
+					taughtBy {
+						id
+						xid
+						name
+						subject
+					}
+				}
+			}
+		}`,
+		Variables: map[string]interface{}{"input": newStudents},
+	}
+
+	gqlResponse := postExecutor(t, graphqlURL, addStudentParams)
+	requireNoGQLErrors(t, gqlResponse)
+
+	var actualResult struct {
+		AddStudent struct {
+			Student []*student
+		}
+	}
+	err := json.Unmarshal(gqlResponse.Data, &actualResult)
+	require.NoError(t, err)
+
+	ignoreOpts := append(ignoreOpts(), sliceSorter())
+	if diff := cmp.Diff(actualResult.AddStudent.Student, []*student{
+		newStudents[0],
+		{
+			Xid:      newStudents[1].Xid,
+			Name:     newStudents[1].Name,
+			TaughtBy: []*teacher{newStudents[1].TaughtBy[0]},
+		},
+	}, ignoreOpts...); diff != "" {
+		t.Errorf("result mismatch (-want +got):\n%s", diff)
+	}
+	require.Equal(t, actualResult.AddStudent.Student[0].TaughtBy[0].ID,
+		actualResult.AddStudent.Student[1].TaughtBy[0].ID)
+
+	// cleanup
+	filter := getXidFilter("xid", []string{newStudents[0].Xid, newStudents[1].Xid})
+	deleteGqlType(t, "Student", filter, 2, nil)
+	filter = getXidFilter("xid", []string{newStudents[0].TaughtBy[0].Xid})
+	deleteGqlType(t, "Teacher", filter, 1, nil)
+}
+
+func sliceSorter() cmp.Option {
+	return cmpopts.SortSlices(func(v1, v2 interface{}) bool {
+		switch t1 := v1.(type) {
+		case *country:
+			t2 := v2.(*country)
+			return t1.Name < t2.Name
+		case *state:
+			t2 := v2.(*state)
+			return t1.Name < t2.Name
+		case *teacher:
+			t2 := v2.(*teacher)
+			return t1.Xid < t2.Xid
+		case *student:
+			t2 := v2.(*student)
+			return t1.Xid < t2.Xid
+		}
+		return v1.(string) < v2.(string)
+	})
+}
+
+func getXidFilter(xidKey string, xidVals []string) map[string]interface{} {
+	if len(xidVals) == 0 || xidKey == "" {
+		return nil
+	}
+
+	filter := map[string]interface{}{
+		xidKey: map[string]interface{}{"eq": xidVals[0]},
+	}
+
+	var currLevel = filter
+
+	for i := 1; i < len(xidVals); i++ {
+		currLevel["or"] = map[string]interface{}{
+			xidKey: map[string]interface{}{"eq": xidVals[i]},
+		}
+		currLevel = currLevel["or"].(map[string]interface{})
+	}
+
+	return filter
+}

graphql/e2e/directives/dgraph_directives_test.go

@@ -54,6 +54,9 @@ func TestSchema_WithDgraphDirectives(t *testing.T) {
 	}, {
 		"predicate": "MovieDirector.name",
 		"type": "string"
+	}, {
+		"predicate": "State.capital",
+		"type": "string"
 	}, {
 		"predicate": "State.name",
 		"type": "string"
@@ -185,6 +188,26 @@ func TestSchema_WithDgraphDirectives(t *testing.T) {
 		"type": "string",
 		"index": true,
 		"tokenizer": ["fulltext"]
+	}, {
+		"predicate": "People.xid",
+		"type": "string",
+		"index": true,
+		"tokenizer": ["hash"],
+		"upsert": true
+	}, {
+		"predicate": "People.name",
+		"type": "string"
+	}, {
+		"predicate": "Teacher.subject",
+		"type": "string"
+	}, {
+		"predicate": "Teacher.teaches",
+		"type": "uid",
+		"list": true
+	}, {
+		"predicate": "Student.taughtBy",
+		"type": "uid",
+		"list": true
 	}],
 	"types": [{
 		"fields": [{
@@ -230,6 +253,8 @@ func TestSchema_WithDgraphDirectives(t *testing.T) {
 			"name": "State.xcode"
 		}, {
 			"name": "State.name"
+		}, {
+			"name": "State.capital"
 		}, {
 			"name": "inCountry"
 		}],
@@ -308,6 +333,33 @@ func TestSchema_WithDgraphDirectives(t *testing.T) {
 			"name": "star.ship.length"
 		}],
 		"name": "star.ship"
+	}, {
+		"fields": [{
+			"name": "People.xid"
+		}, {
+			"name": "People.name"
+		}],
+		"name": "People"
+	}, {
+		"fields": [{
+			"name": "People.xid"
+		}, {
+			"name": "People.name"
+		}, {
+			"name": "Teacher.subject"
+		}, {
+			"name": "Teacher.teaches"
+		}],
+		"name": "Teacher"
+	}, {
+		"fields": [{
+			"name": "People.xid"
+		}, {
+			"name": "People.name"
+		}, {
+			"name": "Student.taughtBy"
+		}],
+		"name": "Student"
 	}]
 }
 	`

graphql/e2e/directives/schema.graphql

@@ -12,6 +12,7 @@ type State {
         id: ID!
         xcode: String! @id @search(by: [regexp])
         name: String!
+        capital: String
         country: Country @dgraph(pred: "inCountry")
 }
 
@@ -121,4 +122,19 @@ type MovieDirector {
         id: ID!
         name: String!
         directed: [Movie] @dgraph(pred: "directed.movies")
-}
+}
+
+interface People {
+        id: ID!
+        xid: String! @id
+        name: String!
+}
+
+type Teacher implements People {
+        subject: String
+        teaches: [Student]
+}
+
+type Student implements People {
+        taughtBy: [Teacher]
+}