Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 3 commits into from
Feb 9, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 3 additions & 5 deletions ee/acl/acl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
17 changes: 15 additions & 2 deletions graphql/admin/add_group.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{})

Expand All @@ -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.
Expand Down
21 changes: 17 additions & 4 deletions graphql/admin/update_group.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Expand All @@ -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 {
Expand All @@ -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"]

Expand Down Expand Up @@ -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(`[
Expand Down
16 changes: 16 additions & 0 deletions graphql/e2e/common/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down
13 changes: 7 additions & 6 deletions graphql/e2e/common/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"}}},
Expand All @@ -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",
},
}

Expand Down
2 changes: 1 addition & 1 deletion graphql/e2e/common/lambda.go
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ func lambdaInMutationWithDuplicateId(t *testing.T) {

testutil.CompareJSON(t, `{
"addChapter": {
"numUids": 4,
"numUids": 6,
"chapter": [
{
"chapterId": 4,
Expand Down
Loading