Skip to content

Commit

Permalink
Fix(GraphQL): fix object Linking with hasInverse (#6557)
Browse files Browse the repository at this point in the history 
This PR fixes incorrect linking of objects with `hasInverse`. For example
For this schema, `type Country` has field `state` with `hasInverse` predicate.
```
type Country {
        id: ID!
        name: String! @search(by: [trigram, hash])
        states: [State] @hasInverse(field: country)
}

type State {
        id: ID!
        xcode: String! @id @search(by: [regexp])
        name: String!
	capital: String
	country: Country
}
```
And for the following mutation
```
mutation addCountry($input: [AddCountryInput!]!) {
			addCountry(input: $input) {
			  country {
				name
				states{
				  xcode
				  name
				  country{
					name
				  }
				}
			  }
			}
		  }
```
with `input`:
```
{
	"input": {
		"name": "A Country",
		"states": [
			{
				"xcode": "abc",
				"name": "Alphabet"
			},
			{
				"xcode": "def",
				"name": "Vowel",
				"country": {
					"name": "B country"
				}
			}
		]
	}
}
``` 
should result in 
```
{
		"addCountry": {
		  "country": [
			{
			  "name": "A country",
			  "states": [
				{
				  "country": {
					"name": "A country"
				  },
				  "name": "Alphabet",
				  "xcode": "abc"
				},
				{
				  "country": {
					"name": "A country"
				  },
				  "name": "Vowel",
				  "xcode": "def"
				}
			  ]
			}
		  ]
		}
	  }`
```
whereas the incorrect output was 
```
{
		"addCountry": {
		  "country": [
			{
			  "name": "A country",
			  "states": [
				{
				  "country": {
					"name": "A country"
				  },
				  "name": "Alphabet",
				  "xcode": "abc"
				},
				{
				  "country": {
					"name": "B country"
				  },
				  "name": "Vowel",
				  "xcode": "def"
				}
			  ]
			}
		  ]
		}
	  }`
```
This PR fixes this issue.
  • Loading branch information
@minhaj-shakeel
minhaj-shakeel authored Sep 28, 2020
1 parent b543f75 commit e496467
Show file tree
Hide file tree
Showing 4 changed files with 171 additions and 3 deletions.
1 change: 1 addition & 0 deletions graphql/e2e/common/common.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,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
Original file line number Diff line number Diff line change
Expand Up @@ -3746,3 +3746,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
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
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

0 comments on commit e496467

Please sign in to comment.