diff --git a/graphql/admin/add_group.go b/graphql/admin/add_group.go index 02c77ff1a69..8b4dba87e69 100644 --- a/graphql/admin/add_group.go +++ b/graphql/admin/add_group.go @@ -21,7 +21,7 @@ func NewAddGroupRewriter() resolve.MutationRewriter { // 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) ([]*resolve.UpsertMutation, error) { addGroupInput, _ := m.ArgValue(schema.InputArgName).([]interface{}) diff --git a/graphql/admin/schema.go b/graphql/admin/schema.go index 8c0506f39ed..c430281ee05 100644 --- a/graphql/admin/schema.go +++ b/graphql/admin/schema.go @@ -63,7 +63,7 @@ type updateGQLSchemaInput struct { func (asr *updateSchemaResolver) Rewrite( ctx context.Context, - m schema.Mutation) (*resolve.UpsertMutation, error) { + m schema.Mutation) ([]*resolve.UpsertMutation, error) { glog.Info("Got updateGQLSchema request") diff --git a/graphql/admin/update_group.go b/graphql/admin/update_group.go index eb082fb71e5..8aa0b908dc4 100644 --- a/graphql/admin/update_group.go +++ b/graphql/admin/update_group.go @@ -3,6 +3,7 @@ package admin import ( "context" "fmt" + dgoapi "github.com/dgraph-io/dgo/v2/protos/api" "github.com/dgraph-io/dgraph/gql" "github.com/dgraph-io/dgraph/graphql/resolve" @@ -23,7 +24,7 @@ func NewUpdateGroupRewriter() resolve.MutationRewriter { // name as another rule. func (urw *updateGroupRewriter) Rewrite( ctx context.Context, - m schema.Mutation) (*resolve.UpsertMutation, error) { + m schema.Mutation) ([]*resolve.UpsertMutation, error) { inp := m.ArgValue(schema.InputArgName).(map[string]interface{}) setArg := inp["set"] @@ -125,10 +126,10 @@ func (urw *updateGroupRewriter) Rewrite( return nil, nil } - return &resolve.UpsertMutation{ + return []*resolve.UpsertMutation{{ Query: &gql.GraphQuery{Children: []*gql.GraphQuery{upsertQuery}}, Mutations: append(mutSet, mutDel...), - }, schema.GQLWrapf(schema.AppendGQLErrs(errSet, errDel), "failed to rewrite mutation payload") + }}, schema.GQLWrapf(schema.AppendGQLErrs(errSet, errDel), "failed to rewrite mutation payload") } // FromMutationResult rewrites the query part of a GraphQL update mutation into a Dgraph query. diff --git a/graphql/e2e/common/common.go b/graphql/e2e/common/common.go index afb86319f2b..d01e891601c 100644 --- a/graphql/e2e/common/common.go +++ b/graphql/e2e/common/common.go @@ -298,6 +298,7 @@ func RunAll(t *testing.T) { t.Run("password in mutation", passwordTest) t.Run("duplicate xid in single mutation", deepMutationDuplicateXIDsSameObjectTest) t.Run("alias works for mutations", mutationsWithAlias) + t.Run("three level deep", threeLevelDeepMutation) // error tests t.Run("graphql completion on", graphQLCompletionOn) diff --git a/graphql/e2e/common/mutation.go b/graphql/e2e/common/mutation.go index c7ddba99644..1a4fb941691 100644 --- a/graphql/e2e/common/mutation.go +++ b/graphql/e2e/common/mutation.go @@ -1292,8 +1292,8 @@ func updateMutationOnlyUpdatesRefsIfDifferent(t *testing.T, executeRequest reque set: $set } ) { - post { - postID + post { + postID text author { id } } @@ -1313,9 +1313,9 @@ func updateMutationOnlyUpdatesRefsIfDifferent(t *testing.T, executeRequest reque // The text is updated as expected // The author is unchanged expected := fmt.Sprintf(` - { "updatePost": { "post": [ - { - "postID": "%s", + { "updatePost": { "post": [ + { + "postID": "%s", "text": "The Updated Text", "author": { "id": "%s" } } @@ -2863,6 +2863,79 @@ func passwordTest(t *testing.T) { deleteUser(t, *newUser) } +func threeLevelDeepMutation(t *testing.T) { + newStudent := &student{ + Xid: "HS1", + Name: "Stud1", + TaughtBy: []*teacher{ + { + Xid: "HT0", + Name: "Teacher0", + Subject: "English", + Teaches: []*student{{ + Xid: "HS2", + Name: "Stud2", + }}, + }, + }, + } + + newStudents := []*student{newStudent} + + addStudentParams := &GraphQLParams{ + Query: `mutation addStudent($input: [AddStudentInput!]!) { + addStudent(input: $input) { + student { + xid + name + taughtBy { + id + xid + name + subject + teaches (order: {asc:xid}) { + xid + taughtBy { + name + xid + subject + } + } + } + } + } + }`, + Variables: map[string]interface{}{"input": newStudents}, + } + + gqlResponse := postExecutor(t, graphqlURL, addStudentParams) + fmt.Println(string(gqlResponse.Data)) + requireNoGQLErrors(t, gqlResponse) + + var actualResult struct { + AddStudent struct { + Student []*student + } + } + + err := json.Unmarshal(gqlResponse.Data, &actualResult) + require.NoError(t, err) + + require.Equal(t, actualResult.AddStudent.Student[0].Xid, "HS1") + require.Equal(t, actualResult.AddStudent.Student[0].TaughtBy[0].Xid, "HT0") + require.Equal(t, actualResult.AddStudent.Student[0].TaughtBy[0].Teaches[0].Xid, "HS1") + require.Equal(t, actualResult.AddStudent.Student[0].TaughtBy[0].Teaches[0].TaughtBy[0].Xid, "HT0") + require.Equal(t, actualResult.AddStudent.Student[0].TaughtBy[0].Teaches[1].Xid, "HS2") + require.Equal(t, actualResult.AddStudent.Student[0].TaughtBy[0].Teaches[1].TaughtBy[0].Xid, "HT0") + + // cleanup + filter := getXidFilter("xid", []string{"HS1", "HS2"}) + deleteGqlType(t, "Student", filter, 2, nil) + filter = getXidFilter("xid", []string{"HT0"}) + deleteGqlType(t, "Teacher", filter, 1, nil) + +} + func deepMutationDuplicateXIDsSameObjectTest(t *testing.T) { newStudents := []*student{ { diff --git a/graphql/e2e/directives/schema.graphql b/graphql/e2e/directives/schema.graphql index 20da0595555..3aaa2d8c6f9 100644 --- a/graphql/e2e/directives/schema.graphql +++ b/graphql/e2e/directives/schema.graphql @@ -136,9 +136,8 @@ type Teacher implements People { } type Student implements People { - taughtBy: [Teacher] + taughtBy: [Teacher] @hasInverse(field: "teaches") } - type Message { content: String! @dgraph(pred: "post") author: String @dgraph(pred: "<职业>") diff --git a/graphql/e2e/normal/schema.graphql b/graphql/e2e/normal/schema.graphql index 14141a87a74..ce7037b6013 100644 --- a/graphql/e2e/normal/schema.graphql +++ b/graphql/e2e/normal/schema.graphql @@ -136,6 +136,6 @@ type Teacher implements People { } type Student implements People { - taughtBy: [Teacher] + taughtBy: [Teacher] @hasInverse(field: teaches) } diff --git a/graphql/resolve/add_mutation_test.yaml b/graphql/resolve/add_mutation_test.yaml index 6c430230b6b..a1a56b17b69 100644 --- a/graphql/resolve/add_mutation_test.yaml +++ b/graphql/resolve/add_mutation_test.yaml @@ -26,6 +26,265 @@ "Author.posts":[] } +- + name: "Add deep mutation with variables" + gqlmutation: | + mutation addAuthor($auth: AddAuthorInput!) { + addAuthor(input: [$auth]) { + author { + name + } + } + } + gqlvariables: | + { "auth": + { "name": "A.N. Author", + "posts": [{ + "title": "post1", + "ps": {"title": "ps1"} + }, { + "title": "post2", + "ps": {"title": "ps2"} + }, { + "title": "post3", + "ps": {"title": "ps3"} + }, { + "title": "post4", + "ps": {"title": "ps4"} + }, { + "title": "post5", + "ps": {"title": "ps5"} + }, { + "title": "post6", + "ps": {"title": "ps6"} + }, { + "title": "post7", + "ps": {"title": "ps7"} + }, { + "title": "post8", + "ps": {"title": "ps8"} + }] + } + } + explanation: "A uid and type should get injected and all data transformed to + underlying Dgraph edge names" + dgquery: |- + query { + PostSecret4 as PostSecret4(func: eq(PostSecret.title, "ps1")) @filter(type(PostSecret)) { + uid + } + PostSecret7 as PostSecret7(func: eq(PostSecret.title, "ps2")) @filter(type(PostSecret)) { + uid + } + PostSecret10 as PostSecret10(func: eq(PostSecret.title, "ps3")) @filter(type(PostSecret)) { + uid + } + PostSecret13 as PostSecret13(func: eq(PostSecret.title, "ps4")) @filter(type(PostSecret)) { + uid + } + PostSecret16 as PostSecret16(func: eq(PostSecret.title, "ps5")) @filter(type(PostSecret)) { + uid + } + PostSecret19 as PostSecret19(func: eq(PostSecret.title, "ps6")) @filter(type(PostSecret)) { + uid + } + PostSecret22 as PostSecret22(func: eq(PostSecret.title, "ps7")) @filter(type(PostSecret)) { + uid + } + PostSecret25 as PostSecret25(func: eq(PostSecret.title, "ps8")) @filter(type(PostSecret)) { + uid + } + } + dgquerysec: |- + query { + PostSecret4 as PostSecret4(func: eq(PostSecret.title, "ps1")) @filter(type(PostSecret)) { + uid + } + PostSecret7 as PostSecret7(func: eq(PostSecret.title, "ps2")) @filter(type(PostSecret)) { + uid + } + PostSecret10 as PostSecret10(func: eq(PostSecret.title, "ps3")) @filter(type(PostSecret)) { + uid + } + PostSecret13 as PostSecret13(func: eq(PostSecret.title, "ps4")) @filter(type(PostSecret)) { + uid + } + PostSecret16 as PostSecret16(func: eq(PostSecret.title, "ps5")) @filter(type(PostSecret)) { + uid + } + PostSecret19 as PostSecret19(func: eq(PostSecret.title, "ps6")) @filter(type(PostSecret)) { + uid + } + PostSecret22 as PostSecret22(func: eq(PostSecret.title, "ps7")) @filter(type(PostSecret)) { + uid + } + PostSecret25 as PostSecret25(func: eq(PostSecret.title, "ps8")) @filter(type(PostSecret)) { + uid + } + } + dgmutations: + - setjson: | + { + "PostSecret.title": "ps1", + "dgraph.type": [ + "PostSecret" + ], + "uid": "_:PostSecret4" + } + cond: "@if(eq(len(PostSecret4), 0))" + - setjson: | + { + "PostSecret.title": "ps2", + "dgraph.type": [ + "PostSecret" + ], + "uid": "_:PostSecret7" + } + cond: "@if(eq(len(PostSecret7), 0))" + - setjson: | + { + "PostSecret.title": "ps3", + "dgraph.type": [ + "PostSecret" + ], + "uid": "_:PostSecret10" + } + cond: "@if(eq(len(PostSecret10), 0))" + - setjson: | + { + "PostSecret.title": "ps4", + "dgraph.type": [ + "PostSecret" + ], + "uid": "_:PostSecret13" + } + cond: "@if(eq(len(PostSecret13), 0))" + - setjson: | + { + "PostSecret.title": "ps5", + "dgraph.type": [ + "PostSecret" + ], + "uid": "_:PostSecret16" + } + cond: "@if(eq(len(PostSecret16), 0))" + - setjson: | + { + "PostSecret.title": "ps6", + "dgraph.type": [ + "PostSecret" + ], + "uid": "_:PostSecret19" + } + cond: "@if(eq(len(PostSecret19), 0))" + - setjson: | + { + "PostSecret.title": "ps7", + "dgraph.type": [ + "PostSecret" + ], + "uid": "_:PostSecret22" + } + cond: "@if(eq(len(PostSecret22), 0))" + - setjson: | + { + "PostSecret.title": "ps8", + "dgraph.type": [ + "PostSecret" + ], + "uid": "_:PostSecret25" + } + cond: "@if(eq(len(PostSecret25), 0))" + dgmutationssec: + - setjson: | + { + "Author.name": "A.N. Author", + "Author.posts": [{ + "Post.author": { + "uid": "_:Author1" + }, + "Post.ps": { + "uid": "uid(PostSecret4)" + }, + "Post.title": "post1", + "dgraph.type": ["Post"], + "uid": "_:Post2" + }, { + "Post.author": { + "uid": "_:Author1" + }, + "Post.ps": { + "uid": "uid(PostSecret7)" + }, + "Post.title": "post2", + "dgraph.type": ["Post"], + "uid": "_:Post5" + }, { + "Post.author": { + "uid": "_:Author1" + }, + "Post.ps": { + "uid": "uid(PostSecret10)" + }, + "Post.title": "post3", + "dgraph.type": ["Post"], + "uid": "_:Post8" + }, { + "Post.author": { + "uid": "_:Author1" + }, + "Post.ps": { + "uid": "uid(PostSecret13)" + }, + "Post.title": "post4", + "dgraph.type": ["Post"], + "uid": "_:Post11" + }, { + "Post.author": { + "uid": "_:Author1" + }, + "Post.ps": { + "uid": "uid(PostSecret16)" + }, + "Post.title": "post5", + "dgraph.type": ["Post"], + "uid": "_:Post14" + }, { + "Post.author": { + "uid": "_:Author1" + }, + "Post.ps": { + "uid": "uid(PostSecret19)" + }, + "Post.title": "post6", + "dgraph.type": ["Post"], + "uid": "_:Post17" + }, { + "Post.author": { + "uid": "_:Author1" + }, + "Post.ps": { + "uid": "uid(PostSecret22)" + }, + "Post.title": "post7", + "dgraph.type": ["Post"], + "uid": "_:Post20" + }, { + "Post.author": { + "uid": "_:Author1" + }, + "Post.ps": { + "uid": "uid(PostSecret25)" + }, + "Post.title": "post8", + "dgraph.type": ["Post"], + "uid": "_:Post23" + }], + "dgraph.type": ["Author"], + "uid": "_:Author1" + } + cond: "@if(eq(len(PostSecret4), 1) AND eq(len(PostSecret7), 1) AND eq(len(PostSecret10), 1) AND eq(len(PostSecret13), 1) AND eq(len(PostSecret16), 1) AND eq(len(PostSecret19), 1) AND eq(len(PostSecret22), 1) AND eq(len(PostSecret25), 1))" + - name: "Add mutation for predicates with special characters having @dgraph directive." gqlmutation: | @@ -857,27 +1116,31 @@ State3 as State3(func: eq(State.code, "dg")) @filter(type(State)) { uid } - var(func: uid(State3)) { - Country4 as State.country - } } + dgmutations: - setjson: | { - "uid": "_:Country1", - "dgraph.type": ["Country"], - "Country.name": "Dgraph Land", - "Country.states": [ { - "uid": "_:State3", - "dgraph.type": ["State"], - "State.code": "dg", - "State.name": "Dgraph", - "State.country": { - "uid": "_:Country1" - } - } ] + "State.code": "dg", + "State.name": "Dgraph", + "dgraph.type": [ + "State" + ], + "uid": "_:State3" } cond: "@if(eq(len(State3), 0))" + + dgquerysec: |- + query { + State3 as State3(func: eq(State.code, "dg")) @filter(type(State)) { + uid + } + var(func: uid(State3)) { + Country4 as State.country + } + } + + dgmutationssec: - setjson: | { "uid": "_:Country1", @@ -1092,21 +1355,25 @@ uid } } + dgquerysec: |- + query { + District3 as District3(func: eq(District.code, "D1")) @filter(type(District)) { + uid + } + } dgmutations: - setjson: | { - "City.name":"Bengaluru", - "City.district":{ - "District.code":"D1", - "District.cities":[{"uid":"_:City1"}], - "District.name":"Dist1", - "dgraph.type":["District"], - "uid":"_:District3" - }, - "dgraph.type":["City"], - "uid":"_:City1" + "District.code": "D1", + "District.name": "Dist1", + "dgraph.type": [ + "District" + ], + "uid": "_:District3" } cond: "@if(eq(len(District3), 0))" + + dgmutationssec: - setjson: | { "City.name":"Bengaluru", @@ -1118,18 +1385,6 @@ "uid":"_:City1" } cond: "@if(eq(len(District3), 1))" - - setjson: | - { - "City.name":"NY", - "City.district":{ - "District.cities":[{"uid":"_:City4"}], - "dgraph.type":["District"], - "uid":"_:District3" - }, - "dgraph.type":["City"], - "uid":"_:City4" - } - cond: "@if(eq(len(District3), 0))" - setjson: | { "City.name":"NY", @@ -1142,6 +1397,8 @@ } cond: "@if(eq(len(District3), 1))" + + - name: "Deep Mutation Duplicate XIDs with same object with @hasInverse Test" gqlmutation: | mutation addCountry($input: [AddCountryInput!]!) { @@ -1186,7 +1443,6 @@ does not have the @hasInverse field of List type, it should return error." error: message: |- - failed to rewrite mutation payload because duplicate XID found: S1 failed to rewrite mutation payload because duplicate XID found: S1 failed to rewrite mutation payload because duplicate XID found: S2 @@ -1393,31 +1649,43 @@ } } gqlvariables: | - { - "inp": { + { + "inp": { "name": "A Country", "states": [ { "code": "abc", "name": "Alphabet" } ] } } + + dgquery: |- + query { + State3 as State3(func: eq(State.code, "abc")) @filter(type(State)) { + uid + } + } dgmutations: - setjson: | - { - "uid" : "_:Country1", - "dgraph.type": ["Country"], - "Country.name": "A Country", - "Country.states": [ - { - "uid": "_:State3", - "dgraph.type": ["State"], - "State.code": "abc", - "State.name": "Alphabet", - "State.country": { "uid": "_:Country1" } - } - ] + { + "State.code": "abc", + "State.name": "Alphabet", + "dgraph.type": [ + "State" + ], + "uid": "_:State3" } cond: "@if(eq(len(State3), 0))" + + dgquerysec: |- + query { + State3 as State3(func: eq(State.code, "abc")) @filter(type(State)) { + uid + } + var(func: uid(State3)) { + Country4 as State.country + } + } + dgmutationssec: - setjson: | - { + { "uid" : "_:Country1", "dgraph.type": ["Country"], "Country.name": "A Country", @@ -1436,16 +1704,447 @@ } ] cond: "@if(eq(len(State3), 1))" + +- name: "Deep XID 4 level deep" + gqlmutation: | + mutation addStudent($student: AddStudentInput!) { + addStudent(input: [$student]) { + student { + name + } + } + } + gqlvariables: | + { + "student": { + "xid": "S0", + "name": "Student0", + "taughtBy": [{ + "xid": "T0", + "name": "teacher0", + "teaches": [{ + "xid": "S1", + "name": "Student1", + "taughtBy": [{ + "xid": "T1", + "name": "teacher1" + }] + }] + }] + } + } dgquery: |- query { - State3 as State3(func: eq(State.code, "abc")) @filter(type(State)) { + Teacher4 as Teacher4(func: eq(People.xid, "T0")) @filter(type(Teacher)) { uid } - var(func: uid(State3)) { - Country4 as State.country + Teacher8 as Teacher8(func: eq(People.xid, "T1")) @filter(type(Teacher)) { + uid + } + Student6 as Student6(func: eq(People.xid, "S1")) @filter(type(Student)) { + uid } } + dgmutations: + - setjson: | + { + "People.name":"teacher0", + "People.xid":"T0", + "dgraph.type":["Teacher","People"], + "uid":"_:Teacher4" + } + + cond: "@if(eq(len(Teacher4), 0))" + + - setjson: | + { + "People.name": "Student1", + "People.xid": "S1", + "Student.taughtBy": [{"uid": "_:Teacher4"}], + "dgraph.type": ["Student","People"], + "uid": "_:Student6" + } + cond: "@if(eq(len(Student6), 0) AND eq(len(Teacher4), 0))" + + - setjson: | + { + "People.name": "teacher1", + "People.xid": "T1", + "Teacher.teaches": [{"uid": "_:Student6"}], + "dgraph.type": ["Teacher","People"], + "uid": "_:Teacher8" + } + cond: "@if(eq(len(Teacher8), 0) AND eq(len(Student6), 0) AND eq(len(Teacher4), 0))" + + - setjson: | + { + "Student.taughtBy": [{"uid": "uid(Teacher8)"}], + "uid": "_:Student6" + } + cond: "@if(eq(len(Teacher8), 1) AND eq(len(Student6), 0) AND eq(len(Teacher4), 0))" + + - setjson: | + { + "Student.taughtBy": [{"uid": "_:Teacher8"}], + "uid": "_:Student6" + } + cond: "@if(eq(len(Teacher8), 0) AND eq(len(Student6), 0) AND eq(len(Teacher4), 0))" + + - setjson: | + { + "Teacher.teaches": [{"uid": "_:Student6"}], + "uid": "uid(Teacher8)" + } + cond: "@if(eq(len(Teacher8), 1) AND eq(len(Student6), 0) AND eq(len(Teacher4), 0))" + + - setjson: | + { + "Teacher.teaches": [{"uid": "uid(Student6)"}], + "uid": "_:Teacher4" + } + cond: "@if(eq(len(Student6), 1) AND eq(len(Teacher4), 0))" + + - setjson: | + { + "Teacher.teaches": [{"uid": "_:Student6"}], + "uid": "_:Teacher4" + } + cond: "@if(eq(len(Student6), 0) AND eq(len(Teacher4), 0))" + + - setjson: | + { + "Student.taughtBy": [{"uid": "_:Teacher4"}], + "uid": "uid(Student6)" + } + cond: "@if(eq(len(Student6), 1) AND eq(len(Teacher4), 0))" + + dgquerysec: |- + query { + Student2 as Student2(func: eq(People.xid, "S0")) @filter(type(Student)) { + uid + } + Teacher4 as Teacher4(func: eq(People.xid, "T0")) @filter(type(Teacher)) { + uid + } + } + + dgmutationssec: + - setjson: | + { + "People.name": "Student0", + "People.xid": "S0", + "Student.taughtBy": [{ + "Teacher.teaches": [{"uid": "_:Student2"}], + "uid": "uid(Teacher4)" + }], + "dgraph.type": ["Student","People"], + "uid": "_:Student2" + } + cond: "@if(eq(len(Student2), 0) AND eq(len(Teacher4), 1))" + +- name: "Deep XID Add top level hasInverse" + gqlmutation: | + mutation addStudent($student: AddStudentInput!) { + addStudent(input: [$student]) { + student { + name + } + } + } + gqlvariables: | + { + "student": { + "xid": "S0", + "name": "Student0", + "taughtBy": [{ + "xid": "T0", + "name": "teacher0", + "teaches": [{ + "xid": "S1", + "name": "Student1" + }] + }] + } + } + dgquery: |- + query { + Teacher4 as Teacher4(func: eq(People.xid, "T0")) @filter(type(Teacher)) { + uid + } + Student6 as Student6(func: eq(People.xid, "S1")) @filter(type(Student)) { + uid + } + } + + dgmutations: + - setjson: | + { + "People.name":"teacher0", + "People.xid":"T0", + "dgraph.type":["Teacher","People"], + "uid":"_:Teacher4" + } + + cond: "@if(eq(len(Teacher4), 0))" + + - setjson: | + { + "People.name":"Student1", + "People.xid":"S1", + "Student.taughtBy":[{"uid":"_:Teacher4"}], + "dgraph.type":["Student","People"], + "uid":"_:Student6" + } + cond: "@if(eq(len(Student6), 0) AND eq(len(Teacher4), 0))" + + - setjson: | + { + "Teacher.teaches":[{"uid":"uid(Student6)"}], + "uid":"_:Teacher4" + } + deletejson: | + cond: "@if(eq(len(Student6), 1) AND eq(len(Teacher4), 0))" + + - setjson: | + { + "Teacher.teaches":[{"uid":"_:Student6"}], + "uid":"_:Teacher4" + } + deletejson: | + cond: "@if(eq(len(Student6), 0) AND eq(len(Teacher4), 0))" + + - setjson: | + { + "Student.taughtBy":[{"uid":"_:Teacher4"}], + "uid":"uid(Student6)" + } + deletejson: | + cond: "@if(eq(len(Student6), 1) AND eq(len(Teacher4), 0))" + + dgquerysec: |- + query { + Student2 as Student2(func: eq(People.xid, "S0")) @filter(type(Student)) { + uid + } + Teacher4 as Teacher4(func: eq(People.xid, "T0")) @filter(type(Teacher)) { + uid + } + } + + dgmutationssec: + - setjson: | + { + "People.name":"Student0", + "People.xid":"S0", + "Student.taughtBy":[{"Teacher.teaches":[{"uid":"_:Student2"}],"uid":"uid(Teacher4)"}], + "dgraph.type":["Student","People"], + "uid":"_:Student2" + } + cond: "@if(eq(len(Student2), 0) AND eq(len(Teacher4), 1))" + + +- name: "Deep XID Add lower level hasInvsere" + gqlmutation: | + mutation addLab($lab: AddLabInput!) { + addLab(input: [$lab]) { + lab { + name + } + } + } + gqlvariables: | + { + "lab": { + "name": "Lab1", + "computers": [{ + "name": "computer1", + "owners": [{ + "name": "owner1" + }] + }] + } + } + dgmutations: + - setjson: | + { + "Computer.name": "computer1", + "dgraph.type": [ + "Computer" + ], + "uid": "_:Computer4" + } + cond: "@if(eq(len(Computer4), 0))" + + - setjson: | + { + "ComputerOwner.computers":{"uid":"_:Computer4"}, + "ComputerOwner.name":"owner1", + "dgraph.type":["ComputerOwner"], + "uid":"_:ComputerOwner6" + } + cond: "@if(eq(len(ComputerOwner6), 0) AND eq(len(Computer4), 0))" + + - setjson: | + { + "Computer.owners":[{"uid":"uid(ComputerOwner6)"}], + "uid":"_:Computer4" + } + cond: "@if(eq(len(ComputerOwner6), 1) AND eq(len(Computer4), 0))" + + - setjson: | + { + "Computer.owners":[{"uid":"_:ComputerOwner6"}], + "uid":"_:Computer4" + } + cond: "@if(eq(len(ComputerOwner6), 0) AND eq(len(Computer4), 0))" + + - setjson: | + { + "ComputerOwner.computers": { + "uid": "_:Computer4" + }, + "uid": "uid(ComputerOwner6)" + } + deletejson: |- + [{ + "Computer.owners": [ + { + "uid": "uid(ComputerOwner6)" + } + ], + "uid": "uid(Computer7)" + }] + cond: "@if(eq(len(ComputerOwner6), 1) AND eq(len(Computer4), 0))" + + + dgmutationssec: + - setjson: |- + { + "Lab.computers": [ + { + "uid": "uid(Computer4)" + } + ], + "Lab.name": "Lab1", + "dgraph.type": ["Lab"], + "uid": "_:Lab2" + } + cond: "@if(eq(len(Lab2), 0) AND eq(len(Computer4), 1))" + + dgquerysec: |- + query { + Lab2 as Lab2(func: eq(Lab.name, "Lab1")) @filter(type(Lab)) { + uid + } + Computer4 as Computer4(func: eq(Computer.name, "computer1")) @filter(type(Computer)) { + uid + } + } + + dgquery: |- + query { + Computer4 as Computer4(func: eq(Computer.name, "computer1")) @filter(type(Computer)) { + uid + } + ComputerOwner6 as ComputerOwner6(func: eq(ComputerOwner.name, "owner1")) @filter(type(ComputerOwner)) { + uid + } + var(func: uid(ComputerOwner6)) { + Computer7 as ComputerOwner.computers + } + } + +- name: "Deep mutation alternate id xid" + gqlmutation: | + mutation addAuthor($city: AddCityInput!) { + addCity(input: [$city]) { + city { + name + district { + code + name + cities { + name + district { + code + name + } + } + } + } + } + } + gqlvariables: | + { + "city": { + "name": "c1", + "district":{ + "name":"d1", + "code":"d1", + "cities":[{"name": "c2"}] + } + } + } + dgquery: |- + query { + District3 as District3(func: eq(District.code, "d1")) @filter(type(District)) { + uid + } + } + dgmutations: + - setjson: | + { + "District.cities": [ + { + "City.district": { + "uid": "_:District3" + }, + "City.name": "c2", + "dgraph.type": [ + "City" + ], + "uid": "_:City4" + } + ], + "District.code": "d1", + "District.name": "d1", + "dgraph.type": [ + "District" + ], + "uid": "_:District3" + } + + cond: "@if(eq(len(District3), 0))" + + dgquerysec: |- + query { + District3 as District3(func: eq(District.code, "d1")) @filter(type(District)) { + uid + } + } + + dgmutationssec: + - setjson: | + { + "City.district": { + "District.cities": [ + { + "uid": "_:City1" + } + ], + "uid": "uid(District3)" + }, + "City.name": "c1", + "dgraph.type": [ + "City" + ], + "uid": "_:City1" + } + + cond: "@if(eq(len(District3), 1))" + + - name: "Additional Deletes - deep mutation" gqlmutation: | mutation addAuthor($auth: AddAuthorInput!) { @@ -1467,26 +2166,17 @@ } dgmutations: - setjson: | - { - "uid":"_:Author1", - "dgraph.type":["Author"], - "Author.name":"A.N. Author", - "Author.country": { - "uid": "_:Country2", - "dgraph.type": ["Country"], - "Country.name": "A Country", - "Country.states": [ - { - "uid": "_:State4", - "dgraph.type": ["State"], - "State.code": "abc", - "State.name": "Alphabet", - "State.country": { "uid": "_:Country2" } - } - ] - } + { + "State.code": "abc", + "State.name": "Alphabet", + "dgraph.type": [ + "State" + ], + "uid": "_:State4" } cond: "@if(eq(len(State4), 0))" + + dgmutationssec: - setjson: | { "uid":"_:Author1", @@ -1512,7 +2202,8 @@ } ] cond: "@if(eq(len(State4), 1))" - dgquery: |- + + dgquerysec: |- query { State4 as State4(func: eq(State.code, "abc")) @filter(type(State)) { uid @@ -1520,4 +2211,10 @@ var(func: uid(State4)) { Country5 as State.country } - } \ No newline at end of file + } + dgquery: |- + query { + State4 as State4(func: eq(State.code, "abc")) @filter(type(State)) { + uid + } + } diff --git a/graphql/resolve/mutation.go b/graphql/resolve/mutation.go index 15c90586c56..575319f98ac 100644 --- a/graphql/resolve/mutation.go +++ b/graphql/resolve/mutation.go @@ -85,7 +85,7 @@ type MutationRewriter interface { // Rewrite rewrites GraphQL mutation m into a Dgraph mutation - that could // be as simple as a single DelNquads, or could be a Dgraph upsert mutation // with a query and multiple mutations guarded by conditions. - Rewrite(ctx context.Context, m schema.Mutation) (*UpsertMutation, error) + Rewrite(ctx context.Context, m schema.Mutation) ([]*UpsertMutation, error) // FromMutationResult takes a GraphQL mutation and the results of a Dgraph // mutation and constructs a Dgraph query. It's used to find the return @@ -206,32 +206,33 @@ func (mr *dgraphResolver) rewriteAndExecute( } } - upsert, err := mr.mutationRewriter.Rewrite(ctx, mutation) + upserts, err := mr.mutationRewriter.Rewrite(ctx, mutation) if err != nil { return emptyResult(schema.GQLWrapf(err, "couldn't rewrite mutation %s", mutation.Name())), resolverFailed } - req := &dgoapi.Request{ - Query: dgraph.AsString(upsert.Query), - CommitNow: true, - Mutations: upsert.Mutations, - } + result := make(map[string]interface{}) + req := &dgoapi.Request{} - mutResp, err = mr.executor.Execute(ctx, req) - if err != nil { - gqlErr := schema.GQLWrapLocationf( - err, mutation.Location(), "mutation %s failed", mutation.Name()) - return emptyResult(gqlErr), resolverFailed + for _, upsert := range upserts { + req.Query = dgraph.AsString(upsert.Query) + req.Mutations = upsert.Mutations - } + mutResp, err = mr.executor.Execute(ctx, req) + if err != nil { + gqlErr := schema.GQLWrapLocationf( + err, mutation.Location(), "mutation %s failed", mutation.Name()) + return emptyResult(gqlErr), resolverFailed - result := make(map[string]interface{}) - if req.Query != "" && len(mutResp.GetJson()) != 0 { - if err := json.Unmarshal(mutResp.GetJson(), &result); err != nil { - return emptyResult( - schema.GQLWrapf(err, "Couldn't unmarshal response from Dgraph mutation")), - resolverFailed + } + + if req.Query != "" && len(mutResp.GetJson()) != 0 { + if err := json.Unmarshal(mutResp.GetJson(), &result); err != nil { + return emptyResult( + schema.GQLWrapf(err, "Couldn't unmarshal response from Dgraph mutation")), + resolverFailed + } } } diff --git a/graphql/resolve/mutation_rewriter.go b/graphql/resolve/mutation_rewriter.go index 6655219aa97..f101b2857b2 100644 --- a/graphql/resolve/mutation_rewriter.go +++ b/graphql/resolve/mutation_rewriter.go @@ -225,54 +225,22 @@ func newXidMetadata() *xidMetadata { // } ], // "Author.friends":[ {"uid":"0x123"} ], // } -func (mrw *AddRewriter) Rewrite(ctx context.Context, m schema.Mutation) (*UpsertMutation, error) { - - mutatedType := m.MutatedType() - - if m.IsArgListType(schema.InputArgName) { - return mrw.handleMultipleMutations(m) - } - - varGen := NewVariableGenerator() - val := m.ArgValue(schema.InputArgName).(map[string]interface{}) - xidMd := newXidMetadata() - mrw.frags = [][]*mutationFragment{rewriteObject(mutatedType, nil, "", varGen, true, val, - xidMd)} - mutations, err := mutationsFromFragments( - mrw.frags[0], - func(frag *mutationFragment) ([]byte, error) { - return json.Marshal(frag.fragment) - }, - func(frag *mutationFragment) ([]byte, error) { - if len(frag.deletes) > 0 { - return json.Marshal(frag.deletes) - } - return nil, nil - }) - - upsert := &UpsertMutation{ - Query: queryFromFragments(mrw.frags[0]), - Mutations: mutations, - } - - return upsert, schema.GQLWrapf(err, "failed to rewrite mutation payload") -} - -func (mrw *AddRewriter) handleMultipleMutations(m schema.Mutation) (*UpsertMutation, error) { +func (mrw *AddRewriter) Rewrite(ctx context.Context, m schema.Mutation) ([]*UpsertMutation, error) { mutatedType := m.MutatedType() val, _ := m.ArgValue(schema.InputArgName).([]interface{}) varGen := NewVariableGenerator() xidMd := newXidMetadata() var errs error - var mutationsAll []*dgoapi.Mutation - queries := &gql.GraphQuery{} - for _, i := range val { - obj := i.(map[string]interface{}) - frag := rewriteObject(mutatedType, nil, "", varGen, true, obj, xidMd) - mrw.frags = append(mrw.frags, frag) + mutationsAllSec := []*dgoapi.Mutation{} + queriesSec := &gql.GraphQuery{} + mutationsAll := []*dgoapi.Mutation{} + queries := &gql.GraphQuery{} + + buildMutations := func(mutationsAll []*dgoapi.Mutation, queries *gql.GraphQuery, + frag []*mutationFragment) []*dgoapi.Mutation { mutations, err := mutationsFromFragments( frag, func(frag *mutationFragment) ([]byte, error) { @@ -293,18 +261,44 @@ func (mrw *AddRewriter) handleMultipleMutations(m schema.Mutation) (*UpsertMutat if qry != nil { queries.Children = append(queries.Children, qry.Children...) } + + return mutationsAll + } + + for _, i := range val { + obj := i.(map[string]interface{}) + frag := rewriteObject(mutatedType, nil, "", varGen, true, obj, 0, xidMd) + mrw.frags = append(mrw.frags, frag.secondPass) + + mutationsAll = buildMutations(mutationsAll, queries, frag.firstPass) + mutationsAllSec = buildMutations(mutationsAllSec, queriesSec, frag.secondPass) } if len(queries.Children) == 0 { queries = nil } - upsert := &UpsertMutation{ - Query: queries, - Mutations: mutationsAll, + if len(queriesSec.Children) == 0 { + queriesSec = nil } - return upsert, errs + result := []*UpsertMutation{} + + if len(mutationsAll) > 0 { + result = append(result, &UpsertMutation{ + Query: queries, + Mutations: mutationsAll, + }) + } + + if len(mutationsAllSec) > 0 { + result = append(result, &UpsertMutation{ + Query: queriesSec, + Mutations: mutationsAllSec, + }) + } + + return result, errs } // FromMutationResult rewrites the query part of a GraphQL add mutation into a Dgraph query. @@ -373,7 +367,7 @@ func (mrw *AddRewriter) FromMutationResult( // See AddRewriter for how the set and remove fragments get created. func (urw *UpdateRewriter) Rewrite( ctx context.Context, - m schema.Mutation) (*UpsertMutation, error) { + m schema.Mutation) ([]*UpsertMutation, error) { mutatedType := m.MutatedType() @@ -389,62 +383,94 @@ func (urw *UpdateRewriter) Rewrite( srcUID := MutationQueryVarUID xidMd := newXidMetadata() - var errSet, errDel error - var mutSet, mutDel []*dgoapi.Mutation + var errs error varGen := NewVariableGenerator() + buildMutation := func(setFrag, delFrag []*mutationFragment) *UpsertMutation { + var mutSet, mutDel []*dgoapi.Mutation + queries := []*gql.GraphQuery{upsertQuery} + + if setArg != nil { + addUpdateCondition(setFrag) + var errSet error + mutSet, errSet = mutationsFromFragments( + setFrag, + func(frag *mutationFragment) ([]byte, error) { + return json.Marshal(frag.fragment) + }, + func(frag *mutationFragment) ([]byte, error) { + if len(frag.deletes) > 0 { + return json.Marshal(frag.deletes) + } + return nil, nil + }) + + urw.setFrags = append(urw.setFrags, setFrag...) + errs = schema.AppendGQLErrs(errs, errSet) + + q1 := queryFromFragments(setFrag) + if q1 != nil { + queries = append(queries, q1.Children...) + } + } + + if delArg != nil { + addUpdateCondition(delFrag) + var errDel error + mutDel, errDel = mutationsFromFragments( + delFrag, + func(frag *mutationFragment) ([]byte, error) { + return nil, nil + }, + func(frag *mutationFragment) ([]byte, error) { + return json.Marshal(frag.fragment) + }) + + urw.delFrags = append(urw.delFrags, delFrag...) + errs = schema.AppendGQLErrs(errs, errDel) + + q2 := queryFromFragments(delFrag) + if q2 != nil { + queries = append(queries, q2.Children...) + } + } + + return &UpsertMutation{ + Query: &gql.GraphQuery{Children: queries}, + Mutations: append(mutSet, mutDel...), + } + } + + var setFragF, setFragS, delFragF, delFragS []*mutationFragment + if setArg != nil { - urw.setFrags = - rewriteObject(mutatedType, nil, srcUID, varGen, true, - setArg.(map[string]interface{}), xidMd) - addUpdateCondition(urw.setFrags) - mutSet, errSet = mutationsFromFragments( - urw.setFrags, - func(frag *mutationFragment) ([]byte, error) { - return json.Marshal(frag.fragment) - }, - func(frag *mutationFragment) ([]byte, error) { - if len(frag.deletes) > 0 { - return json.Marshal(frag.deletes) - } - return nil, nil - }) + setFrag := rewriteObject(mutatedType, nil, srcUID, varGen, true, + setArg.(map[string]interface{}), 0, xidMd) + + setFragF = setFrag.firstPass + setFragS = setFrag.secondPass } if delArg != nil { - urw.delFrags = - rewriteObject(mutatedType, nil, srcUID, varGen, false, - delArg.(map[string]interface{}), xidMd) - addUpdateCondition(urw.delFrags) - mutDel, errDel = mutationsFromFragments( - urw.delFrags, - func(frag *mutationFragment) ([]byte, error) { - return nil, nil - }, - func(frag *mutationFragment) ([]byte, error) { - return json.Marshal(frag.fragment) - }) + delFrag := rewriteObject(mutatedType, nil, srcUID, varGen, false, + delArg.(map[string]interface{}), 0, xidMd) + delFragF = delFrag.firstPass + delFragS = delFrag.secondPass } - queries := []*gql.GraphQuery{upsertQuery} + result := []*UpsertMutation{} - q1 := queryFromFragments(urw.setFrags) - if q1 != nil { - queries = append(queries, q1.Children...) + firstPass := buildMutation(setFragF, delFragF) + if len(firstPass.Mutations) > 0 { + result = append(result, firstPass) } - q2 := queryFromFragments(urw.delFrags) - if q2 != nil { - queries = append(queries, q2.Children...) + secondPass := buildMutation(setFragS, delFragS) + if len(secondPass.Mutations) > 0 { + result = append(result, secondPass) } - upsert := &UpsertMutation{ - Query: &gql.GraphQuery{Children: queries}, - Mutations: append(mutSet, mutDel...), - } - - return upsert, - schema.GQLWrapf(schema.AppendGQLErrs(errSet, errDel), "failed to rewrite mutation payload") + return result, schema.GQLWrapf(errs, "failed to rewrite mutation payload") } // FromMutationResult rewrites the query part of a GraphQL update mutation into a Dgraph query. @@ -567,7 +593,7 @@ func RewriteUpsertQueryFromMutation(m schema.Mutation) *gql.GraphQuery { func (drw *deleteRewriter) Rewrite( ctx context.Context, - m schema.Mutation) (*UpsertMutation, error) { + m schema.Mutation) ([]*UpsertMutation, error) { if m.MutationType() != schema.DeleteMutation { return nil, errors.Errorf( @@ -621,7 +647,7 @@ func (drw *deleteRewriter) Rewrite( Mutations: []*dgoapi.Mutation{{DeleteJson: b}}, } - return upsert, err + return []*UpsertMutation{upsert}, err } func (drw *deleteRewriter) FromMutationResult( @@ -706,6 +732,11 @@ func queryFromFragments(frags []*mutationFragment) *gql.GraphQuery { return qry } +type mutationRes struct { + firstPass []*mutationFragment + secondPass []*mutationFragment +} + // rewriteObject rewrites obj to a list of mutation fragments. See AddRewriter.Rewrite // for a description of what those fragments look like. // @@ -721,6 +752,11 @@ func queryFromFragments(frags []*mutationFragment) *gql.GraphQuery { // an author might make the `author: Author!` field of a bunch of Posts invalid. // (That might actually be helpful if you want to run one mutation to remove something // and then another to correct it.) +// +// rewriteObject returns two set of mutations, firstPass and secondPass. We start +// building mutations recursively in the secondPass. Whenever we encounter an XID object, +// we push it to firstPass. We need to make sure that the XID doesn't refer hasInverse links +// to secondPass, and then to make those links ourselves. func rewriteObject( typ schema.Type, srcField schema.FieldDefinition, @@ -728,7 +764,8 @@ func rewriteObject( varGen *VariableGenerator, withAdditionalDeletes bool, obj map[string]interface{}, - xidMetadata *xidMetadata) []*mutationFragment { + deepXID int, + xidMetadata *xidMetadata) *mutationRes { atTopLevel := srcField == nil topLevelAdd := srcUID == "" @@ -739,8 +776,9 @@ func rewriteObject( if id != nil { if idVal, ok := obj[id.Name()]; ok { if idVal != nil { - return []*mutationFragment{ - asIDReference(idVal, srcField, srcUID, variable, withAdditionalDeletes, varGen)} + return &mutationRes{secondPass: []*mutationFragment{ + asIDReference(idVal, srcField, srcUID, variable, + withAdditionalDeletes, varGen)}} } delete(obj, id.Name()) } @@ -756,7 +794,7 @@ func rewriteObject( if !ok { errFrag := newFragment(nil) errFrag.err = errors.New("encountered an XID that isn't a string") - return []*mutationFragment{errFrag} + return &mutationRes{secondPass: []*mutationFragment{errFrag}} } // if the object has an xid, the variable name will be formed from the xidValue in order // to handle duplicate object addition/updation @@ -778,7 +816,7 @@ func rewriteObject( xidObj, obj) || (invField != nil && invField.Type().ListType() == nil) { errFrag := newFragment(nil) errFrag.err = errors.Errorf("duplicate XID found: %s", xidString) - return []*mutationFragment{errFrag} + return &mutationRes{secondPass: []*mutationFragment{errFrag}} } } else { // if not encountered till now, add it to the map @@ -790,12 +828,26 @@ func rewriteObject( xidMetadata.seenAtTopLevel[variable] = atTopLevel } } + + deepXID += 1 } + var parentFrags []*mutationFragment + if !atTopLevel { // top level is never a reference - it's adding/updating if xid != nil && xidString != "" { xidFrag = asXIDReference(srcField, srcUID, typ, xid.Name(), xidString, variable, withAdditionalDeletes, varGen, xidMetadata) + if deepXID > 2 { + // We need to link the parent to the already existing child + res := make(map[string]interface{}, 1) + res["uid"] = srcUID + addInverseLink(res, srcField.Inverse(), fmt.Sprintf("uid(%s)", variable)) + + parentFrag := newFragment(res) + parentFrag.conditions = append(parentFrag.conditions, xidFrag.conditions...) + parentFrags = append(parentFrags, parentFrag) + } } else if !withAdditionalDeletes { // In case of delete, id/xid is required var name string @@ -804,8 +856,8 @@ func rewriteObject( } else { name = id.Name() } - return invalidObjectFragment(fmt.Errorf("%s is not provided", name), - xidFrag, variable, xidString) + return &mutationRes{secondPass: invalidObjectFragment(fmt.Errorf("%s is not provided", name), + xidFrag, variable, xidString)} } } @@ -821,14 +873,18 @@ func rewriteObject( if err := typ.EnsureNonNulls(obj, exclude); err != nil { // This object is either an invalid deep mutation or it's an xid reference // and asXIDReference must to apply or it's an error. - return invalidObjectFragment(err, xidFrag, variable, xidString) + return &mutationRes{secondPass: invalidObjectFragment(err, xidFrag, variable, xidString)} } } if !atTopLevel && !withAdditionalDeletes { // For remove op (!withAdditionalDeletes), we don't need to generate a new // blank node. - return []*mutationFragment{xidFrag} + if xidFrag != nil { + return &mutationRes{secondPass: []*mutationFragment{xidFrag}} + } else { + return &mutationRes{} + } } var myUID string @@ -840,15 +896,16 @@ func rewriteObject( newObj["dgraph.type"] = dgraphTypes myUID = fmt.Sprintf("_:%s", variable) - addInverseLink(newObj, srcField, srcUID) + if xid == nil || deepXID > 2 { + addInverseLink(newObj, srcField, srcUID) + } } else { myUID = srcUID } newObj["uid"] = myUID - frag := newFragment(newObj) - results := []*mutationFragment{frag} + results := &mutationRes{secondPass: []*mutationFragment{frag}} // if xidString != "", then we are adding with an xid. In which case, we have to ensure // as part of the upsert that the xid doesn't already exist. @@ -866,14 +923,38 @@ func rewriteObject( nil) } - // if this object has an xid, then we don't need to rewrite its children if we have encountered - // it earlier + if xid != nil && !atTopLevel { + if deepXID <= 2 { // elements in firstPass or not + // duplicate query in elements >= 2, as the pair firstPass element would already have the same query. + frag.queries = []*gql.GraphQuery{ + xidQuery(variable, xidString, xid.Name(), typ), + } + } else { + // We need to link the parent to the element we are just creating + res := make(map[string]interface{}, 1) + res["uid"] = srcUID + addInverseLink(res, srcField.Inverse(), fmt.Sprintf("_:%s", variable)) + + parentFrag := newFragment(res) + parentFrag.conditions = append(parentFrag.conditions, frag.conditions...) + parentFrags = append(parentFrags, parentFrag) + } + } + + var childrenFirstPass []*mutationFragment + + // we build the mutation to add object here. If XID != nil, we would then move it to + // firstPass from secondPass (frag). + + // if this object has an xid, then we don't need to + // rewrite its children if we have encountered it earlier. if xidString == "" || xidEncounteredFirstTime { for field, val := range obj { - var frags []*mutationFragment + var frags *mutationRes fieldDef := typ.Field(field) fieldName := typ.DgraphPredicate(field) + // This fixes mutation when dgraph predicate has special characters. PR #5526 if strings.HasPrefix(fieldName, "<") && strings.HasSuffix(fieldName, ">") { fieldName = fieldName[1 : len(fieldName)-1] @@ -890,7 +971,8 @@ func rewriteObject( // like here ^^ frags = rewriteObject(fieldDef.Type(), fieldDef, myUID, varGen, withAdditionalDeletes, - val, xidMetadata) + val, deepXID, xidMetadata) + case []interface{}: // This field is either: // 1) A list of objects: e.g. if the schema said `categories: [Categories]` @@ -905,7 +987,7 @@ func rewriteObject( // like here ^^ frags = rewriteList(fieldDef.Type(), fieldDef, myUID, varGen, withAdditionalDeletes, val, - xidMetadata) + deepXID, xidMetadata) default: // This field is either: // 1) a scalar value: e.g. @@ -915,15 +997,45 @@ func rewriteObject( // e.g. to remove the text or // { "friends": null, ... } // to remove all friends - frags = []*mutationFragment{newFragment(val)} + frags = &mutationRes{secondPass: []*mutationFragment{newFragment(val)}} } - results = squashFragments(squashIntoObject(fieldName), results, frags) + childrenFirstPass = appendFragments(childrenFirstPass, frags.firstPass) + results.secondPass = squashFragments(squashIntoObject(fieldName), results.secondPass, frags.secondPass) } } - if xidFrag != nil { - results = append(results, xidFrag) + // In the case of an XID, move the secondPass (creation mutation) to firstPass + if xid != nil && !atTopLevel { + results.firstPass = append(results.firstPass, results.secondPass...) + results.secondPass = []*mutationFragment{} + } + + // add current conditions to all the new fragments from children. + // childrens should only be addded when this level is true + conditions := []string{} + for _, i := range results.firstPass { + conditions = append(conditions, i.conditions...) + } + for _, i := range childrenFirstPass { + i.conditions = append(i.conditions, conditions...) + } + results.firstPass = append(results.firstPass, childrenFirstPass...) + + // parentFrags are reverse links to parents. only applicable for when deepXID > 2 + results.firstPass = append(results.firstPass, parentFrags...) + + // xidFrag contains the mutation to update object if it is present. + // add it to secondPass if deepXID <= 2, otherwise firstPass for relevant hasInverse links. + if xidFrag != nil && deepXID > 2 { + results.firstPass = append(results.firstPass, xidFrag) + } else if xidFrag != nil { + results.secondPass = append(results.secondPass, xidFrag) + } + + // if !xidEncounteredFirstTime, we have already seen the relevant fragments. + if xid != nil && !atTopLevel && !xidEncounteredFirstTime { + results.firstPass = []*mutationFragment{} } return results @@ -1258,25 +1370,36 @@ func rewriteList( varGen *VariableGenerator, withAdditionalDeletes bool, objects []interface{}, - xidMetadata *xidMetadata) []*mutationFragment { + deepXID int, + xidMetadata *xidMetadata) *mutationRes { - frags := []*mutationFragment{newFragment(make([]interface{}, 0))} + result := &mutationRes{} + result.secondPass = []*mutationFragment{newFragment(make([]interface{}, 0))} + foundSecondPass := false for _, obj := range objects { switch obj := obj.(type) { case map[string]interface{}: - frags = squashFragments(squashIntoList, frags, - rewriteObject(typ, srcField, srcUID, varGen, withAdditionalDeletes, obj, xidMetadata)) + frag := rewriteObject(typ, srcField, srcUID, varGen, withAdditionalDeletes, obj, deepXID, xidMetadata) + if len(frag.secondPass) != 0 { + foundSecondPass = true + } + result.firstPass = appendFragments(result.firstPass, frag.firstPass) + result.secondPass = squashFragments(squashIntoList, result.secondPass, frag.secondPass) default: // All objects in the list must be of the same type. GraphQL validation makes sure // of that. So this must be a list of scalar values (lists of lists aren't allowed). - return []*mutationFragment{ + return &mutationRes{secondPass: []*mutationFragment{ newFragment(objects), - } + }} } } - return frags + if len(objects) != 0 && !foundSecondPass { + result.secondPass = nil + } + + return result } func newFragment(f interface{}) *mutationFragment { @@ -1314,6 +1437,24 @@ func squashIntoObject(label string) func(interface{}, interface{}, bool) interfa } } +func appendFragments(left, right []*mutationFragment) []*mutationFragment { + if len(left) == 0 { + return right + } + + if len(right) == 0 { + return left + } + + for _, i := range right { + left[0].queries = append(left[0].queries, i.queries...) + i.queries = []*gql.GraphQuery{} + } + + left = append(left, right...) + return left +} + // squashFragments takes two lists of mutationFragments and produces a single list // that has all the right fragments squashed into the left. // @@ -1422,7 +1563,7 @@ func squashFragments( for _, r := range right { queries = append(queries, r.queries...) } - result[0].queries = queries + result[0].queries = queries return result } diff --git a/graphql/resolve/mutation_test.go b/graphql/resolve/mutation_test.go index 626d752dae5..9ee0fc300ad 100644 --- a/graphql/resolve/mutation_test.go +++ b/graphql/resolve/mutation_test.go @@ -19,10 +19,12 @@ package resolve import ( "context" "encoding/json" + "fmt" "io/ioutil" "strings" "testing" + dgoapi "github.com/dgraph-io/dgo/v2/protos/api" "github.com/dgraph-io/dgraph/graphql/dgraph" "github.com/dgraph-io/dgraph/graphql/schema" "github.com/dgraph-io/dgraph/graphql/test" @@ -45,7 +47,9 @@ type testCase struct { GQLVariables string Explanation string DGMutations []*dgraphMutation + DGMutationsSec []*dgraphMutation DGQuery string + DGQuerySec string Error *x.GqlError ValidationError *x.GqlError } @@ -102,6 +106,55 @@ func mutationValidation(t *testing.T, file string, rewriterFactory func() Mutati } } +func benchmark3LevelDeep(num int, b *testing.B) { + t := &testing.T{} + gqlSchema := test.LoadSchemaFromFile(t, "schema.graphql") + + innerTeachers := make([]interface{}, 0) + for i := 1; i <= num; i++ { + innerTeachers = append(innerTeachers, map[string]interface{}{ + "xid": fmt.Sprintf("S%d", i), + "name": fmt.Sprintf("Name%d", i), + }) + } + + vars := map[string]interface{}{ + "input": []interface{}{map[string]interface{}{ + "xid": "S0", + "name": "Name0", + "taughtBy": []interface{}{map[string]interface{}{ + "xid": "T0", + "name": "Teacher0", + "teaches": innerTeachers, + }}, + }}, + } + + op, _ := gqlSchema.Operation( + &schema.Request{ + Query: ` + mutation addStudent($input: [AddStudentInput!]!) { + addStudent(input: $input) { + student { + xid + } + } + }`, + Variables: vars, + }) + mut := test.GetMutation(t, op) + + for n := 0; n < b.N; n++ { + NewAddRewriter().Rewrite(context.Background(), mut) + } +} + +func Benchmark3LevelDeep5(b *testing.B) { benchmark3LevelDeep(5, b) } +func Benchmark3LevelDeep19(b *testing.B) { benchmark3LevelDeep(19, b) } +func Benchmark3LevelDeep100(b *testing.B) { benchmark3LevelDeep(100, b) } +func Benchmark3LevelDeep1000(b *testing.B) { benchmark3LevelDeep(1000, b) } +func Benchmark3LevelDeep10000(b *testing.B) { benchmark3LevelDeep(10000, b) } + func mutationRewriting(t *testing.T, file string, rewriterFactory func() MutationRewriter) { b, err := ioutil.ReadFile(file) require.NoError(t, err, "Unable to read test file") @@ -112,6 +165,19 @@ func mutationRewriting(t *testing.T, file string, rewriterFactory func() Mutatio gqlSchema := test.LoadSchemaFromFile(t, "schema.graphql") + compareMutations := func(t *testing.T, test []*dgraphMutation, generated []*dgoapi.Mutation) { + require.Len(t, generated, len(test)) + for i, expected := range test { + require.Equal(t, expected.Cond, generated[i].Cond) + if len(generated[i].SetJson) > 0 || expected.SetJSON != "" { + require.JSONEq(t, expected.SetJSON, string(generated[i].SetJson)) + } + if len(generated[i].DeleteJson) > 0 || expected.DeleteJSON != "" { + require.JSONEq(t, expected.DeleteJSON, string(generated[i].DeleteJson)) + } + } + } + for _, tcase := range tests { t.Run(tcase.Name, func(t *testing.T) { // -- Arrange -- @@ -132,24 +198,21 @@ func mutationRewriting(t *testing.T, file string, rewriterFactory func() Mutatio // -- Act -- upsert, err := rewriterToTest.Rewrite(context.Background(), mut) - q := upsert.Query - muts := upsert.Mutations // -- Assert -- if tcase.Error != nil || err != nil { + require.NotNil(t, err) + require.NotNil(t, tcase.Error) require.Equal(t, tcase.Error.Error(), err.Error()) - } else { - require.Equal(t, tcase.DGQuery, dgraph.AsString(q)) - require.Len(t, muts, len(tcase.DGMutations)) - for i, expected := range tcase.DGMutations { - require.Equal(t, expected.Cond, muts[i].Cond) - if len(muts[i].SetJson) > 0 || expected.SetJSON != "" { - require.JSONEq(t, expected.SetJSON, string(muts[i].SetJson)) - } - if len(muts[i].DeleteJson) > 0 || expected.DeleteJSON != "" { - require.JSONEq(t, expected.DeleteJSON, string(muts[i].DeleteJson)) - } - } + return + } + + require.Equal(t, tcase.DGQuery, dgraph.AsString(upsert[0].Query)) + compareMutations(t, tcase.DGMutations, upsert[0].Mutations) + + if len(upsert) > 1 { + require.Equal(t, tcase.DGQuerySec, dgraph.AsString(upsert[1].Query)) + compareMutations(t, tcase.DGMutationsSec, upsert[1].Mutations) } }) } diff --git a/graphql/resolve/schema.graphql b/graphql/resolve/schema.graphql index b54526f3ce9..5f3e3e08b28 100644 --- a/graphql/resolve/schema.graphql +++ b/graphql/resolve/schema.graphql @@ -39,6 +39,11 @@ type Post { postType: PostType @search author: Author! category: Category @hasInverse(field: posts) + ps: PostSecret +} + +type PostSecret { + title: String! @id } type Category { @@ -97,6 +102,11 @@ type MovieDirector { directed: [Movie] @dgraph(pred: "directed.movies") } +type Lab { + name: String! @id + computers: [Computer] +} + # just for testing XID remove in list type Computer { owners: [ComputerOwner!] @@ -139,7 +149,7 @@ type Teacher implements People { } type Student implements People { - taughtBy: [Teacher] + taughtBy: [Teacher] @hasInverse(field: "teaches") } type Message { diff --git a/graphql/resolve/update_mutation_test.yaml b/graphql/resolve/update_mutation_test.yaml index 9264df76850..a16fccadd58 100644 --- a/graphql/resolve/update_mutation_test.yaml +++ b/graphql/resolve/update_mutation_test.yaml @@ -670,23 +670,16 @@ explanation: "The update has a choice of linking to new or existing state" dgmutations: - setjson: | - { "uid" : "uid(x)", - "Author.country": { - "uid": "_:Country2", - "dgraph.type": ["Country"], - "Country.name": "New Country", - "Country.states": [ { - "uid": "_:State4", - "dgraph.type": ["State"], - "State.code": "dg", - "State.name": "Dgraph", - "State.country": { - "uid": "_:Country2" - } - } ] - } + { + "State.code": "dg", + "State.name": "Dgraph", + "dgraph.type": [ + "State" + ], + "uid": "_:State4" } cond: "@if(eq(len(State4), 0) AND gt(len(x), 0))" + dgmutationssec: - setjson: | { "uid" : "uid(x)", "Author.country": { @@ -710,6 +703,15 @@ ] cond: "@if(eq(len(State4), 1) AND gt(len(x), 0))" dgquery: |- + query { + x as updateAuthor(func: type(Author)) @filter(uid(0x123)) { + uid + } + State4 as State4(func: eq(State.code, "dg")) @filter(type(State)) { + uid + } + } + dgquerysec: |- query { x as updateAuthor(func: type(Author)) @filter(uid(0x123)) { uid @@ -964,64 +966,37 @@ uid } } + dgquerysec: |- + query { + x as updateStudent(func: type(Student)) @filter(uid(0x123)) { + uid + } + Teacher3 as Teacher3(func: eq(People.xid, "T1")) @filter(type(Teacher)) { + uid + } + } dgmutations: - setjson: | { - "Student.taughtBy": [ - { - "People.name": "Teacher1", - "People.xid": "T1", - "dgraph.type": [ "Teacher", "People" ], - "uid": "_:Teacher3" - }, - { - "dgraph.type": [ "Teacher", "People" ], - "uid": "_:Teacher3" - } + "People.name": "Teacher1", + "People.xid": "T1", + "dgraph.type": [ + "Teacher", + "People" ], - "uid": "uid(x)" + "uid": "_:Teacher3" } - cond: "@if(eq(len(Teacher3), 0) AND eq(len(Teacher3), 0) AND gt(len(x), 0))" + cond: "@if(eq(len(Teacher3), 0) AND gt(len(x), 0))" + dgmutationssec: - setjson: | { - "Student.taughtBy": [ - { - "People.name": "Teacher1", - "People.xid": "T1", - "dgraph.type": [ "Teacher", "People" ], - "uid": "_:Teacher3" - }, - { - "uid": "uid(Teacher3)" - } - ], - "uid": "uid(x)" - } - cond: "@if(eq(len(Teacher3), 0) AND eq(len(Teacher3), 1) AND gt(len(x), 0))" - - setjson: | - { - "Student.taughtBy": [ - { - "uid": "uid(Teacher3)" - }, - { - "dgraph.type": [ "Teacher", "People" ], - "uid": "_:Teacher3" - } - ], - "uid": "uid(x)" - } - cond: "@if(eq(len(Teacher3), 1) AND eq(len(Teacher3), 0) AND gt(len(x), 0))" - - setjson: | - { - "Student.taughtBy": [ - { - "uid": "uid(Teacher3)" - }, - { - "uid": "uid(Teacher3)" - } - ], + "Student.taughtBy":[{ + "Teacher.teaches":[{"uid":"uid(x)"}], + "uid":"uid(Teacher3)" + },{ + "Teacher.teaches":[{"uid":"uid(x)"}], + "uid":"uid(Teacher3)" + }], "uid": "uid(x)" } cond: "@if(eq(len(Teacher3), 1) AND eq(len(Teacher3), 1) AND gt(len(x), 0))" @@ -1061,7 +1036,6 @@ error: message: |- failed to rewrite mutation payload because duplicate XID found: S1 - failed to rewrite mutation payload because duplicate XID found: S1 - name: "Deep Mutation Duplicate XIDs with different object Test" gqlmutation: | @@ -1097,7 +1071,6 @@ error: message: |- failed to rewrite mutation payload because duplicate XID found: T1 - failed to rewrite mutation payload because duplicate XID found: T1 - name: "Duplicate XIDs in single mutation for Interface" @@ -1305,19 +1278,14 @@ } dgmutations: - setjson: | - { - "uid" : "uid(x)", - "Country.states": [ - { - "uid": "_:State3", - "dgraph.type": ["State"], - "State.code": "abc", - "State.name": "Alphabet", - "State.country": { "uid": "uid(x)" } - } - ] + { + "uid": "_:State3", + "dgraph.type": ["State"], + "State.code": "abc", + "State.name": "Alphabet" } cond: "@if(eq(len(State3), 0) AND gt(len(x), 0))" + dgmutationssec: - setjson: | { "uid" : "uid(x)", @@ -1337,6 +1305,15 @@ ] cond: "@if(eq(len(State3), 1) AND gt(len(x), 0))" dgquery: |- + query { + x as updateCountry(func: type(Country)) @filter(uid(0x123)) { + uid + } + State3 as State3(func: eq(State.code, "abc")) @filter(type(State)) { + uid + } + } + dgquerysec: |- query { x as updateCountry(func: type(Country)) @filter(uid(0x123)) { uid @@ -1368,15 +1345,13 @@ } dgmutations: - setjson: | - { "uid" : "uid(x)", - "ComputerOwner.computers": { - "uid": "_:Computer3", - "dgraph.type": ["Computer"], - "Computer.name": "Comp", - "Computer.owners": [ { "uid": "uid(x)" } ] - } + { + "uid": "_:Computer3", + "dgraph.type": ["Computer"], + "Computer.name": "Comp" } cond: "@if(eq(len(Computer3), 0) AND gt(len(x), 0))" + dgmutationssec: - setjson: | { "uid" : "uid(x)", "ComputerOwner.computers": { @@ -1393,6 +1368,15 @@ ] cond: "@if(eq(len(Computer3), 1) AND gt(len(x), 0))" dgquery: |- + query { + x as updateComputerOwner(func: type(ComputerOwner)) @filter(eq(ComputerOwner.name, "A.N. Owner")) { + uid + } + Computer3 as Computer3(func: eq(Computer.name, "Comp")) @filter(type(Computer)) { + uid + } + } + dgquerysec: |- query { x as updateComputerOwner(func: type(ComputerOwner)) @filter(eq(ComputerOwner.name, "A.N. Owner")) { uid @@ -1403,4 +1387,4 @@ var(func: uid(x)) { Computer4 as ComputerOwner.computers @filter(NOT (uid(Computer3))) } - } \ No newline at end of file + }