From 201459f0858e1e7ee35c39d5329928219c3704d4 Mon Sep 17 00:00:00 2001 From: Pawan Rawal Date: Sun, 13 Sep 2020 19:50:31 +0530 Subject: [PATCH] fix(GraphQL): Fix squashIntoObject so that results are correctly merged (#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 816a08f2749a4298651c26cb7b69ef5e79e1a53b) --- graphql/e2e/common/common.go | 1 + graphql/e2e/common/mutation.go | 99 +++++++++++++++++++++ graphql/e2e/directives/schema.graphql | 8 +- graphql/e2e/directives/schema_response.json | 20 +++++ graphql/e2e/normal/schema.graphql | 8 +- graphql/e2e/normal/schema_response.json | 20 +++++ graphql/resolve/add_mutation_test.yaml | 58 ++++++++++++ graphql/resolve/mutation_rewriter.go | 22 ++++- graphql/resolve/schema.graphql | 6 ++ 9 files changed, 239 insertions(+), 3 deletions(-) diff --git a/graphql/e2e/common/common.go b/graphql/e2e/common/common.go index 5a8d2e21ad3..c49c393f7a0 100644 --- a/graphql/e2e/common/common.go +++ b/graphql/e2e/common/common.go @@ -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) diff --git a/graphql/e2e/common/mutation.go b/graphql/e2e/common/mutation.go index 24ed5f04bc5..e08ef6db22d 100644 --- a/graphql/e2e/common/mutation.go +++ b/graphql/e2e/common/mutation.go @@ -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) +} diff --git a/graphql/e2e/directives/schema.graphql b/graphql/e2e/directives/schema.graphql index eff992d8fb4..f04ab522c58 100644 --- a/graphql/e2e/directives/schema.graphql +++ b/graphql/e2e/directives/schema.graphql @@ -171,4 +171,10 @@ type Post1 { type Comment1 { id: String! @id replies: [Comment1] -} \ No newline at end of file +} + +type Person1 { + id: ID! + name: String! + friends: [Person1] @hasInverse(field: friends) +} diff --git a/graphql/e2e/directives/schema_response.json b/graphql/e2e/directives/schema_response.json index 04c66f67513..bd4b27791b2 100644 --- a/graphql/e2e/directives/schema_response.json +++ b/graphql/e2e/directives/schema_response.json @@ -58,6 +58,15 @@ ], "upsert": true }, + { + "predicate": "Person1.name", + "type": "string" + }, + { + "predicate": "Person1.friends", + "type": "uid", + "list": true + }, { "predicate": "Post1.comments", "type": "uid", @@ -430,6 +439,17 @@ ], "name": "People" }, + { + "fields": [ + { + "name": "Person1.name" + }, + { + "name": "Person1.friends" + } + ], + "name": "Person1" + }, { "fields": [ { diff --git a/graphql/e2e/normal/schema.graphql b/graphql/e2e/normal/schema.graphql index 41ac10b39e0..59634dc6e0b 100644 --- a/graphql/e2e/normal/schema.graphql +++ b/graphql/e2e/normal/schema.graphql @@ -171,4 +171,10 @@ type Post1 { type Comment1 { id: String! @id replies: [Comment1] -} \ No newline at end of file +} + +type Person1 { + id: ID! + name: String! + friends: [Person1] @hasInverse(field: friends) +} diff --git a/graphql/e2e/normal/schema_response.json b/graphql/e2e/normal/schema_response.json index ac1fc5fa81f..0b822d2013a 100644 --- a/graphql/e2e/normal/schema_response.json +++ b/graphql/e2e/normal/schema_response.json @@ -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" @@ -483,6 +492,17 @@ ], "name": "Person" }, + { + "fields": [ + { + "name": "Person1.name" + }, + { + "name": "Person1.friends" + } + ], + "name": "Person1" + }, { "fields": [ { diff --git a/graphql/resolve/add_mutation_test.yaml b/graphql/resolve/add_mutation_test.yaml index a780c466024..91a80162e4f 100644 --- a/graphql/resolve/add_mutation_test.yaml +++ b/graphql/resolve/add_mutation_test.yaml @@ -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" + } + diff --git a/graphql/resolve/mutation_rewriter.go b/graphql/resolve/mutation_rewriter.go index 0106948a916..75633058444 100644 --- a/graphql/resolve/mutation_rewriter.go +++ b/graphql/resolve/mutation_rewriter.go @@ -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 } } diff --git a/graphql/resolve/schema.graphql b/graphql/resolve/schema.graphql index eef8ed55b4e..e2d299dcf50 100644 --- a/graphql/resolve/schema.graphql +++ b/graphql/resolve/schema.graphql @@ -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) +} \ No newline at end of file