Fix(GraphQL): Refactor Mutation Rewriter for Add and Update Mutations (…

…#7409)

* Feat(GraphQL): Refactor Mutation Rewriting for AddRewriter (#7257)

* Add Existence Queries for Mutation Rewriting

* Add tests

* Add Error handling to getExistenceQueries

* Use new code for AddRewriter

* Fix Geo

* Fix tests

* Fix auth tests

* Fix resolve tests

* Fix add_mutation_test.yaml

* Add bug related tests

* Address more comments

* Rebase

* Final changes

* Fix(GraphQL): Refactor Mutation Rewriting for Update Rewriter (#7383)

* Fix auth tests

* Refactor Update Mutation Rewriting

* Fix update tests

* Add comments and remove unused code

* Address comments

* Address more comments

* Fix ACL test

ee/acl/acl_test.go

@@ -229,11 +229,9 @@ func TestCreateAndDeleteUsers(t *testing.T) {
 	// adding the user again should fail
 	token := testutil.GrootHttpLogin(adminEndpoint)
 	resp := createUser(t, token, userid, userpassword)
-	require.Equal(t, x.GqlErrorList{{
-		Message: "couldn't rewrite query for mutation addUser because id alice already exists" +
-			" for type User",
-		Path: []interface{}{"addUser"},
-	}}, resp.Errors)
+	require.Equal(t, 1, len(resp.Errors))
+	require.Equal(t, "couldn't rewrite mutation addUser because failed to rewrite mutation payload because id"+
+		" alice already exists for type User", resp.Errors[0].Message)
 	checkUserCount(t, resp.Data, 0)
 
 	// delete the user

graphql/admin/add_group.go

@@ -16,12 +16,25 @@ func NewAddGroupRewriter() resolve.MutationRewriter {
 	return &addGroupRewriter{}
 }
 
+// RewriteQueries generates and rewrites queries for schema.Mutation
+// into dql queries. These queries are used to check if there exist any
+// nodes with the ID or XID which we are going to be adding.
+// RewriteQueries on addGroupRewriter calls the corresponding function for
+// AddRewriter.
+func (mrw *addGroupRewriter) RewriteQueries(
+	ctx context.Context,
+	m schema.Mutation) ([]*gql.GraphQuery, error) {
+
+	return ((*resolve.AddRewriter)(mrw)).RewriteQueries(ctx, m)
+}
+
 // Rewrite rewrites schema.Mutation into dql upsert mutations only for Group type.
 // It ensures that only the last rule out of all duplicate rules in input is preserved.
 // A rule is duplicate if it has same predicate name as another rule.
 func (mrw *addGroupRewriter) Rewrite(
 	ctx context.Context,
-	m schema.Mutation) ([]*resolve.UpsertMutation, error) {
+	m schema.Mutation,
+	idExistence map[string]string) ([]*resolve.UpsertMutation, error) {
 
 	addGroupInput, _ := m.ArgValue(schema.InputArgName).([]interface{})
 
@@ -34,7 +47,7 @@ func (mrw *addGroupRewriter) Rewrite(
 
 	m.SetArgTo(schema.InputArgName, addGroupInput)
 
-	return ((*resolve.AddRewriter)(mrw)).Rewrite(ctx, m)
+	return ((*resolve.AddRewriter)(mrw)).Rewrite(ctx, m, idExistence)
 }
 
 // FromMutationResult rewrites the query part of a GraphQL add mutation into a Dgraph query.

graphql/admin/update_group.go

@@ -17,14 +17,28 @@ func NewUpdateGroupRewriter() resolve.MutationRewriter {
 	return &updateGroupRewriter{}
 }
 
+// RewriteQueries on updateGroupRewriter initializes  urw.VarGen and
+// urw.XidMetadata. As there is no need to rewrite queries to check for existing
+// nodes. It does not rewrite any queries.
+func (urw *updateGroupRewriter) RewriteQueries(
+	ctx context.Context,
+	m schema.Mutation) ([]*gql.GraphQuery, error) {
+
+	urw.VarGen = resolve.NewVariableGenerator()
+	urw.XidMetadata = resolve.NewXidMetadata()
+
+	return []*gql.GraphQuery{}, nil
+}
+
 // Rewrite rewrites set and remove update patches into dql upsert mutations
 // only for Group type. It ensures that if a rule already exists in db, it is updated;
 // otherwise, it is created. It also ensures that only the last rule out of all
 // duplicate rules in input is preserved. A rule is duplicate if it has same predicate
 // name as another rule.
 func (urw *updateGroupRewriter) Rewrite(
 	ctx context.Context,
-	m schema.Mutation) ([]*resolve.UpsertMutation, error) {
+	m schema.Mutation,
+	idExistence map[string]string) ([]*resolve.UpsertMutation, error) {
 
 	inp := m.ArgValue(schema.InputArgName).(map[string]interface{})
 	setArg := inp["set"]
@@ -39,7 +53,6 @@ func (urw *updateGroupRewriter) Rewrite(
 
 	var errSet, errDel error
 	var mutSet, mutDel []*dgoapi.Mutation
-	varGen := resolve.NewVariableGenerator()
 	ruleType := m.MutatedType().Field("rules").Type()
 
 	if setArg != nil {
@@ -50,7 +63,7 @@ func (urw *updateGroupRewriter) Rewrite(
 		}
 		for _, ruleI := range rules {
 			rule := ruleI.(map[string]interface{})
-			variable := varGen.Next(ruleType, "", "", false)
+			variable := urw.VarGen.Next(ruleType, "", "", false)
 			predicate := rule["predicate"]
 			permission := rule["permission"]
 
@@ -96,7 +109,7 @@ func (urw *updateGroupRewriter) Rewrite(
 				continue
 			}
 
-			variable := varGen.Next(ruleType, "", "", false)
+			variable := urw.VarGen.Next(ruleType, "", "", false)
 			addAclRuleQuery(upsertQuery, predicate.(string), variable)
 
 			deleteJson := []byte(fmt.Sprintf(`[

graphql/e2e/common/common.go

@@ -183,6 +183,18 @@ type state struct {
 	Code    string   `json:"xcode,omitempty"`
 	Capital string   `json:"capital,omitempty"`
 	Country *country `json:"country,omitempty"`
+	Region  *region  `json:"region,omitempty"`
+}
+
+type region struct {
+	ID       string    `json:"id,omitempty"`
+	Name     string    `json:"name,omitempty"`
+	District *district `json:"district,omitempty"`
+}
+
+type district struct {
+	ID   string `json:"id,omitempty"`
+	Name string `json:"name,omitempty"`
 }
 
 type movie struct {
@@ -759,6 +771,10 @@ func RunAll(t *testing.T) {
 	t.Run("mutation id directive with float", idDirectiveWithFloatMutation)
 	t.Run("add mutation on extended type with field of ID type as key field", addMutationOnExtendedTypeWithIDasKeyField)
 	t.Run("add mutation with deep extended type objects", addMutationWithDeepExtendedTypeObjects)
+	t.Run("three level double XID mutation", threeLevelDoubleXID)
+	t.Run("two levels linked to one XID", twoLevelsLinkedToXID)
+	t.Run("cyclically linked mutation", cyclicMutation)
+	t.Run("parallel mutations", parallelMutations)
 
 	// error tests
 	t.Run("graphql completion on", graphQLCompletionOn)

graphql/e2e/common/error.go

@@ -134,8 +134,8 @@ func deepMutationErrors(t *testing.T) {
 	}{
 		"missing ID and XID": {
 			set: &country{States: []*state{{Name: "NOT A VALID STATE"}}},
-			exp: "couldn't rewrite mutation updateCountry because failed to rewrite mutation " +
-				"payload because type State requires a value for field xcode, but no value present",
+			exp: "couldn't rewrite mutation updateCountry because failed to rewrite" +
+				" mutation payload because field xcode cannot be empty",
 		},
 		"ID not valid": {
 			set: &country{States: []*state{{ID: "HI"}}},
@@ -144,13 +144,14 @@ func deepMutationErrors(t *testing.T) {
 		},
 		"ID not found": {
 			set: &country{States: []*state{{ID: "0x1"}}},
-			exp: "couldn't rewrite query for mutation updateCountry because ID \"0x1\" isn't a State",
+			exp: "couldn't rewrite mutation updateCountry because failed to rewrite mutation" +
+				" payload because ID \"0x1\" isn't a State",
 		},
 		"XID not found": {
 			set: &country{States: []*state{{Code: "NOT A VALID CODE"}}},
-			exp: "couldn't rewrite query for mutation updateCountry because xid " +
-				"\"NOT A VALID CODE\" doesn't exist and input object not well formed because type " +
-				"State requires a value for field name, but no value present",
+			exp: "couldn't rewrite mutation updateCountry because failed to rewrite mutation" +
+				" payload because type State requires a value for field name, but no value" +
+				" present",
 		},
 	}
 

graphql/e2e/common/lambda.go

@@ -220,7 +220,7 @@ func lambdaInMutationWithDuplicateId(t *testing.T) {
 
 	testutil.CompareJSON(t, `{
 		"addChapter": {
-		  "numUids": 4,
+		  "numUids": 6,
 		  "chapter": [
 			{
 			  "chapterId": 4,