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.

Sign up for GitHub

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

Jump to bottom

release/20.07: Fix(GraphQL): fix object Linking with hasInverse #6648

Merged
merged 1 commit into from
Oct 5, 2020
Merged

release/20.07: Fix(GraphQL): fix object Linking with hasInverse #6648

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions graphql/e2e/common/common.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,7 @@ func RunAll(t *testing.T) {
t.Run("deep XID mutations", deepXIDMutations)
t.Run("three level xid", testThreeLevelXID)
t.Run("nested add mutation with @hasInverse", nestedAddMutationWithHasInverse)
t.Run("add mutation with @hasInverse overrides correctly", addMutationWithHasInverseOverridesCorrectly)
t.Run("error in multiple mutations", addMultipleMutationWithOneError)
t.Run("dgraph directive with reverse edge adds data correctly",
addMutationWithReverseDgraphEdge)
Expand Down
76 changes: 76 additions & 0 deletions graphql/e2e/common/mutation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3607,3 +3607,79 @@ func nestedAddMutationWithHasInverse(t *testing.T) {
// cleanup
deleteGqlType(t, "Person1", map[string]interface{}{}, 3, nil)
}

func addMutationWithHasInverseOverridesCorrectly(t *testing.T) {
params := &GraphQLParams{
Query: `mutation addCountry($input: [AddCountryInput!]!) {
addCountry(input: $input) {
country {
name
states{
xcode
name
country{
name
}
}
}
}
}`,

Variables: map[string]interface{}{
"input": []interface{}{
map[string]interface{}{
"name": "A country",
"states": []interface{}{
map[string]interface{}{
"xcode": "abc",
"name": "Alphabet",
},
map[string]interface{}{
"xcode": "def",
"name": "Vowel",
"country": map[string]interface{}{
"name": "B country",
},
},
},
},
},
},
}

gqlResponse := postExecutor(t, GraphqlURL, params)
RequireNoGQLErrors(t, gqlResponse)

expected := `{
"addCountry": {
"country": [
{
"name": "A country",
"states": [
{
"country": {
"name": "A country"
},
"name": "Alphabet",
"xcode": "abc"
},
{
"country": {
"name": "A country"
},
"name": "Vowel",
"xcode": "def"
}
]
}
]
}
}`
testutil.CompareJSON(t, expected, string(gqlResponse.Data))
filter := map[string]interface{}{"name": map[string]interface{}{"eq": "A country"}}
deleteCountry(t, filter, 1, nil)
filter = map[string]interface{}{"xcode": map[string]interface{}{"eq": "abc"}}
deleteState(t, filter, 1, nil)
filter = map[string]interface{}{"xcode": map[string]interface{}{"eq": "def"}}
deleteState(t, filter, 1, nil)
}
42 changes: 39 additions & 3 deletions graphql/resolve/add_mutation_test.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1600,7 +1600,7 @@
{
"auth": {
"name": "A.N. Author",
"posts": [ { "postID": "0x456" } ]
"posts": [ { "postID": "0x456" }, {"title": "New Post", "author": {"name": "Abhimanyu"}} ]
}
}
dgmutations:
Expand All @@ -1613,6 +1613,12 @@
{
"uid": "0x456",
"Post.author": { "uid": "_:Author1" }
},
{
"uid": "_:Post4",
"dgraph.type": ["Post"],
"Post.title": "New Post",
"Post.author": { "uid": "_:Author1" }
}
]
}
Expand Down Expand Up @@ -1647,7 +1653,10 @@
{
"inp": {
"name": "A Country",
"states": [ { "code": "abc", "name": "Alphabet" } ]
"states": [
{ "code": "abc", "name": "Alphabet" },
{ "code": "def", "name": "Vowel", "country": { "name": "B country" } }
]
}
}

Expand All @@ -1656,6 +1665,9 @@
State3 as State3(func: eq(State.code, "abc")) @filter(type(State)) {
uid
}
State6 as State6(func: eq(State.code, "def")) @filter(type(State)) {
uid
}
}
dgmutations:
- setjson: |
Expand All @@ -1668,6 +1680,16 @@
"uid": "_:State3"
}
cond: "@if(eq(len(State3), 0))"
- setjson: |
{
"State.code": "def",
"State.name": "Vowel",
"dgraph.type": [
"State"
],
"uid": "_:State6"
}
cond: "@if(eq(len(State6), 0))"

dgquerysec: |-
query {
Expand All @@ -1677,6 +1699,12 @@
var(func: uid(State3)) {
Country4 as State.country
}
State6 as State6(func: eq(State.code, "def")) @filter(type(State)) {
uid
}
var(func: uid(State6)) {
Country7 as State.country
}
}
dgmutationssec:
- setjson: |
Expand All @@ -1688,6 +1716,10 @@
{
"uid": "uid(State3)",
"State.country": { "uid": "_:Country1" }
},
{
"uid": "uid(State6)",
"State.country": { "uid": "_:Country1" }
}
]
}
Expand All @@ -1696,9 +1728,13 @@
{
"uid": "uid(Country4)",
"Country.states": [ { "uid": "uid(State3)" } ]
},
{
"uid": "uid(Country7)",
"Country.states": [ { "uid": "uid(State6)" } ]
}
]
cond: "@if(eq(len(State3), 1))"
cond: "@if(eq(len(State3), 1) AND eq(len(State6), 1))"

- name: "Deep XID 4 level deep"
gqlmutation: |
Expand Down
55 changes: 55 additions & 0 deletions graphql/resolve/mutation_rewriter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -996,6 +996,30 @@ func rewriteObject(
if xid != nil && xidString != "" {
xidFrag = asXIDReference(ctx, srcField, srcUID, typ, xid.Name(), xidString,
variable, withAdditionalDeletes, varGen, xidMetadata)

// Inverse Link is added as a Part of asXIDReference so we delete any provided
// Link to the object.
// Example: for this mutation
// mutation addCountry($inp: AddCountryInput!) {
// addCountry(input: [$inp]) {
// country {
// id
// }
// }
// }
// with the input:
//{
// "inp": {
// "name": "A Country",
// "states": [
// { "code": "abc", "name": "Alphabet" },
// { "code": "def", "name": "Vowel", "country": { "name": "B country" } }
// ]
// }
// }
// we delete the link of Second state to "B Country"
deleteInverseObject(obj, srcField)

if deepXID > 2 {
// Here we link the already existing node with an xid to the parent whose id is
// passed in srcUID. We do this linking only if there is a hasInverse relationship
Expand Down Expand Up @@ -1059,11 +1083,33 @@ func rewriteObject(
myUID = fmt.Sprintf("_:%s", variable)

if xid == nil || deepXID > 2 {
// If this object had an overwritten value for the inverse field, then we don't want to
// use that value as we will add the link to the inverse field in the below
// function call with the parent of this object
// for example, for this mutation:
// mutation addAuthor($auth: AddAuthorInput!) {
// addAuthor(input: [$auth]) {
// author {
// id
// }
// }
// }
// with the following input
// {
// "auth": {
// "name": "A.N. Author",
// "posts": [ { "postID": "0x456" }, {"title": "New Post", "author": {"name": "Abhimanyu"}} ]
// }
// }
// We delete the link of second input post with Author "name" : "Abhimanyu".
deleteInverseObject(obj, srcField)

// Lets link the new node that we are creating with the parent if a @hasInverse
// exists between the two.
// So for example if we had the addAuthor mutation which is also adding nested
// posts, then we add the link _:Post Post.author AuthorUID(srcUID) here.
addInverseLink(newObj, srcField, srcUID)

}
} else {
myUID = srcUID
Expand Down Expand Up @@ -1636,6 +1682,15 @@ func attachChild(res map[string]interface{}, parent schema.Type, child schema.Fi
}
}

func deleteInverseObject(obj map[string]interface{}, srcField schema.FieldDefinition) {
if srcField != nil {
invField := srcField.Inverse()
if invField != nil && invField.Type().ListType() == nil {
delete(obj, invField.Name())
}
}
}

func addInverseLink(obj map[string]interface{}, srcField schema.FieldDefinition, srcUID string) {
if srcField != nil {
invField := srcField.Inverse()
Expand Down