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

Fix(GraphQL): Fix duplicate XID error in case of interface XIDs #7776

Merged
merged 4 commits into from
May 11, 2021
Merged

Fix(GraphQL): Fix duplicate XID error in case of interface XIDs #7776

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
33c1c42
Fix second scenario
vmrajas May 4, 2021
79c7a34
Fix scenario 1
vmrajas May 6, 2021
d9b4e32
Add tests
vmrajas May 6, 2021
4d5f23c
Add comments
vmrajas May 6, 2021
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
2 changes: 2 additions & 0 deletions graphql/e2e/common/common.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -900,6 +900,8 @@ func RunAll(t *testing.T) {
t.Run("Update language tag fields", updateLangTagFields)
t.Run("mutation with @id field and interface arg", mutationWithIDFieldHavingInterfaceArg)
t.Run("xid update and nullable tests", xidUpdateAndNullableTests)
t.Run("Referencing same node containing multiple XIDs",
referencingSameNodeWithMultipleXIds)

// error tests
t.Run("graphql completion on", graphQLCompletionOn)
Expand Down
74 changes: 74 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 @@ -6620,3 +6620,77 @@ func xidUpdateAndNullableTests(t *testing.T) {
DeleteGqlType(t, "Employer", filterEmployer, 2, nil)
DeleteGqlType(t, "Worker", filterWorker, 4, nil)
}

func referencingSameNodeWithMultipleXIds(t *testing.T) {
params := &GraphQLParams{
Query: `mutation($input: [AddPerson1Input!]!) {
addPerson1(input: $input) {
person1 {
regId
name
friends {
regId
name
}
closeFriends {
regId
name
}
}
}
}`,
Variables: map[string]interface{}{
"input": []interface{}{
map[string]interface{}{
"regId": "7",
"name": "7th Person",
"name1": "seventh Person",
"friends": []interface{}{
map[string]interface{}{
"regId": "8",
"name": "8th Person",
"name1": "eighth Person",
},
},
"closeFriends": []interface{}{
map[string]interface{}{
"regId": "8",
"name": "8th Person",
},
},
},
},
},
}

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

expected := `{
"addPerson1":
{
"person1": [
{
"closeFriends": [
{
"name": "8th Person",
"regId": "8"
}
],
"friends": [
{
"name": "8th Person",
"regId": "8"
}
],
"name": "7th Person",
"regId": "7"
}
]
}
}`
testutil.CompareJSON(t, expected, string(gqlResponse.Data))

// cleanup
DeleteGqlType(t, "Person1", map[string]interface{}{}, 2, nil)
}
4 changes: 3 additions & 1 deletion graphql/e2e/directives/schema.graphql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,9 @@ type post1{

type Person1 {
id: ID!
name: String!
name: String! @id
name1: String @id
regId: String @id
closeFriends: [Person1] @hasInverse(field: closeFriends)
friends: [Person1] @hasInverse(field: friends)
}
Expand Down
31 changes: 30 additions & 1 deletion graphql/e2e/directives/schema_response.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -469,7 +469,30 @@
},
{
"predicate": "Person1.name",
"type": "string"
"type": "string",
"upsert": true,
"tokenizer": [
"hash"
],
"index": true
},
{
"predicate": "Person1.regId",
"type": "string",
"upsert": true,
"tokenizer": [
"hash"
],
"index": true
},
{
"predicate": "Person1.name1",
"type": "string",
"upsert": true,
"tokenizer": [
"hash"
],
"index": true
},
{
"predicate": "Plant.breed",
Expand Down Expand Up @@ -1209,6 +1232,12 @@
},
{
"name": "Person1.closeFriends"
},
{
"name": "Person1.name1"
},
{
"name": "Person1.regId"
}
],
"name": "Person1"
Expand Down
4 changes: 3 additions & 1 deletion graphql/e2e/normal/schema.graphql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,9 @@ type post1{

type Person1 {
id: ID!
name: String!
name: String! @id
name1: String @id
regId: String @id
closeFriends: [Person1] @hasInverse(field: closeFriends)
friends: [Person1] @hasInverse(field: friends)
}
Expand Down
31 changes: 30 additions & 1 deletion graphql/e2e/normal/schema_response.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -523,7 +523,30 @@
},
{
"predicate": "Person1.name",
"type": "string"
"index": true,
"type": "string",
"tokenizer": [
"hash"
],
"upsert": true
},
{
"predicate": "Person1.name1",
"index": true,
"type": "string",
"tokenizer": [
"hash"
],
"upsert": true
},
{
"predicate": "Person1.regId",
"index": true,
"type": "string",
"tokenizer": [
"hash"
],
"upsert": true
},
{
"predicate": "Plant.breed",
Expand Down Expand Up @@ -1237,6 +1260,12 @@
{
"name": "Person1.name"
},
{
"name": "Person1.name1"
},
{
"name": "Person1.regId"
},
{
"name": "Person1.friends"
},
Expand Down
82 changes: 43 additions & 39 deletions graphql/resolve/add_mutation_test.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -661,8 +661,10 @@
}
explanation: "A reference must be a valid UID"
error:
{ "message":
"failed to rewrite mutation payload because ID argument (HI!) was not able to be parsed" }
{
"message":
"failed to rewrite mutation payload because ID argument (HI!) was not able to be parsed"
}

-
name: "Add mutation with inverse reference"
Expand Down Expand Up @@ -2741,43 +2743,6 @@
message: |-
failed to rewrite mutation payload because duplicate XID found: S1

- name: "Duplicate XIDs in single mutation for Interface"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed this test as it was wrong. As xid does not have interface=true flag. It is perfectly fine to have Student and Teacher with same xid.

gqlmutation: |
mutation addStudent($input: [AddStudentInput!]!) {
addStudent(input: $input) {
student {
xid
name
taughtBy {
xid
name
subject
}
}
}
}
gqlvariables: |
{
"input": [
{
"xid": "S1",
"name": "Stud1"
},
{
"xid": "S2",
"name": "Stud2",
"taughtBy": [
{"xid": "S1", "name": "Teacher1", "subject": "Sub1"}
]
}
]
}
explanation: "When duplicate XIDs are given as input for an Interface in a single mutation, it
should return error."
error:
message: |-
failed to rewrite mutation payload because duplicate XID found: S1

# Additional Deletes
#
# If we have
Expand Down Expand Up @@ -5500,3 +5465,42 @@
{
"message": "failed to rewrite mutation payload because field id cannot be empty"
}

-
name: "Add Mutation referencing same XID in different types"
gqlmutation: |
mutation($input: [AddT1Input!]!) {
addT1(input: $input) {
t1 {
name
name1
name2
link {
name
name1
name3
}
}
}
}
gqlvariables: |
{
"input": [
{
"name": "Bob",
"name1": "Bob11",
"name2": "Bob2",
"link": {
"name": "Bob"
}
}
]
}
explanation: "As the link and top level object contain the same XID, Bob, this should throw an error"
error:
{
"message":
"failed to rewrite mutation payload because using duplicate XID value: Bob for XID: name for two different
implementing types of same interfaces: T1 and T2"
}

Loading