Skip to content

Commit

Permalink
fix(GraphQL): Fix squashIntoObject so that results are correctly merg…
Browse files Browse the repository at this point in the history
…ed (#6416)

While creating the add mutation, we weren't squashing different bits correctly which led to the incorrect mutation being sent to Dgraph. This change modifies the set to append for []interface{} to fix that.

Fixes GRAPHQL-679

(cherry picked from commit 816a08f)
  • Loading branch information
pawanrawal committed Sep 18, 2020
1 parent b72e296 commit 201459f
Show file tree
Hide file tree
Showing 9 changed files with 239 additions and 3 deletions.
1 change: 1 addition & 0 deletions graphql/e2e/common/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,7 @@ func RunAll(t *testing.T) {
t.Run("add multiple mutations", testMultipleMutations)
t.Run("deep XID mutations", deepXIDMutations)
t.Run("three level xid", testThreeLevelXID)
t.Run("nested add mutation with @hasInverse", nestedAddMutationWithHasInverse)
t.Run("error in multiple mutations", addMultipleMutationWithOneError)
t.Run("dgraph directive with reverse edge adds data correctly",
addMutationWithReverseDgraphEdge)
Expand Down
99 changes: 99 additions & 0 deletions graphql/e2e/common/mutation.go
Original file line number Diff line number Diff line change
Expand Up @@ -3542,3 +3542,102 @@ func updateMutationWithoutSetRemove(t *testing.T) {
// cleanup
deleteCountry(t, map[string]interface{}{"id": []string{country.ID}}, 1, nil)
}

func int64BoundaryTesting(t *testing.T) {
//This test checks the range of Int64
//(2^63)=9223372036854775808
addPost1Params := &GraphQLParams{
Query: `mutation {
addpost1(input: [{title: "Dgraph", numLikes: 9223372036854775807 },{title: "Dgraph1", numLikes: -9223372036854775808 }]) {
post1 {
title
numLikes
}
}
}`,
}

gqlResponse := addPost1Params.ExecuteAsPost(t, graphqlURL)
RequireNoGQLErrors(t, gqlResponse)

addPost1Expected := `{
"addpost1": {
"post1": [{
"title": "Dgraph",
"numLikes": 9223372036854775807
},{
"title": "Dgraph1",
"numLikes": -9223372036854775808
}]
}
}`
testutil.CompareJSON(t, addPost1Expected, string(gqlResponse.Data))
filter := map[string]interface{}{"title": map[string]interface{}{"regexp": "/Dgraph.*/"}}
deleteGqlType(t, "post1", filter, 2, nil)
}

func nestedAddMutationWithHasInverse(t *testing.T) {
params := &GraphQLParams{
Query: `mutation addPerson1($input: [AddPerson1Input!]!) {
addPerson1(input: $input) {
person1 {
name
friends {
name
friends {
name
}
}
}
}
}`,
Variables: map[string]interface{}{
"input": []interface{}{
map[string]interface{}{
"name": "Or",
"friends": []interface{}{
map[string]interface{}{
"name": "Michal",
"friends": []interface{}{
map[string]interface{}{
"name": "Justin",
},
},
},
},
},
},
},
}

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

expected := `{
"addPerson1": {
"person1": [
{
"friends": [
{
"friends": [
{
"name": "Or"
},
{
"name": "Justin"
}
],
"name": "Michal"
}
],
"name": "Or"
}
]
}
}`
testutil.CompareJSON(t, expected, string(gqlResponse.Data))

// cleanup
deleteGqlType(t, "Person1", map[string]interface{}{}, 3, nil)
}
8 changes: 7 additions & 1 deletion graphql/e2e/directives/schema.graphql
Original file line number Diff line number Diff line change
Expand Up @@ -171,4 +171,10 @@ type Post1 {
type Comment1 {
id: String! @id
replies: [Comment1]
}
}

type Person1 {
id: ID!
name: String!
friends: [Person1] @hasInverse(field: friends)
}
20 changes: 20 additions & 0 deletions graphql/e2e/directives/schema_response.json
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,15 @@
],
"upsert": true
},
{
"predicate": "Person1.name",
"type": "string"
},
{
"predicate": "Person1.friends",
"type": "uid",
"list": true
},
{
"predicate": "Post1.comments",
"type": "uid",
Expand Down Expand Up @@ -430,6 +439,17 @@
],
"name": "People"
},
{
"fields": [
{
"name": "Person1.name"
},
{
"name": "Person1.friends"
}
],
"name": "Person1"
},
{
"fields": [
{
Expand Down
8 changes: 7 additions & 1 deletion graphql/e2e/normal/schema.graphql
Original file line number Diff line number Diff line change
Expand Up @@ -171,4 +171,10 @@ type Post1 {
type Comment1 {
id: String! @id
replies: [Comment1]
}
}

type Person1 {
id: ID!
name: String!
friends: [Person1] @hasInverse(field: friends)
}
20 changes: 20 additions & 0 deletions graphql/e2e/normal/schema_response.json
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,15 @@
"predicate": "Person.name",
"type": "string"
},
{
"predicate": "Person1.name",
"type": "string"
},
{
"predicate": "Person1.friends",
"type": "uid",
"list": true
},
{
"predicate": "Post.author",
"type": "uid"
Expand Down Expand Up @@ -483,6 +492,17 @@
],
"name": "Person"
},
{
"fields": [
{
"name": "Person1.name"
},
{
"name": "Person1.friends"
}
],
"name": "Person1"
},
{
"fields": [
{
Expand Down
58 changes: 58 additions & 0 deletions graphql/resolve/add_mutation_test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2413,3 +2413,61 @@
explanation: "The add mutation should not be allowed since value of @id field is empty."
error:
{ "message": "failed to rewrite mutation payload because encountered an empty value for @id field `State.code`" }

-
name: "Add mutation for person with @hasInverse"
gqlmutation: |
mutation($input: [AddPersonInput!]!) {
addPerson(input: $input) {
person {
name
}
}
}
gqlvariables: |
{
"input": [
{
"name": "Or",
"friends": [
{ "name": "Michal", "friends": [{ "name": "Justin" }] }
]
}
]
}
dgmutations:
- setjson: |
{
"Person.friends": [
{
"Person.friends": [
{
"uid": "_:Person1"
},
{
"Person.friends": [
{
"uid": "_:Person2"
}
],
"Person.name": "Justin",
"dgraph.type": [
"Person"
],
"uid": "_:Person3"
}
],
"Person.name": "Michal",
"dgraph.type": [
"Person"
],
"uid": "_:Person2"
}
],
"Person.name": "Or",
"dgraph.type": [
"Person"
],
"uid": "_:Person1"
}
22 changes: 21 additions & 1 deletion graphql/resolve/mutation_rewriter.go
Original file line number Diff line number Diff line change
Expand Up @@ -1720,7 +1720,27 @@ func squashIntoObject(label string) func(interface{}, interface{}, bool) interfa
}
asObject = cpy
}
asObject[label] = v

val := v

// If there is an existing value for the label in the object, then we should append to it
// instead of overwriting it if the existing value is a list. This can happen when there
// is @hasInverse and we are doing nested adds.
existing := asObject[label]
switch ev := existing.(type) {
case []interface{}:
switch vv := v.(type) {
case []interface{}:
ev = append(ev, vv...)
val = ev
case interface{}:
ev = append(ev, vv)
val = ev
default:
}
default:
}
asObject[label] = val
return asObject
}
}
Expand Down
6 changes: 6 additions & 0 deletions graphql/resolve/schema.graphql
Original file line number Diff line number Diff line change
Expand Up @@ -282,3 +282,9 @@ type ThingTwo implements Thing {
prop: String @dgraph(pred: "prop")
owner: String
}

type Person {
id: ID!
name: String @search(by: [hash])
friends: [Person] @hasInverse(field: friends)
}

0 comments on commit 201459f

Please sign in to comment.