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.

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

feat(GraphQL): allow duplicate XIDs if only XID value is repeated #6762

Merged
merged 2 commits into from Oct 22, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
17 changes: 16 additions & 1 deletion graphql/resolve/add_mutation_test.yaml
Expand Up @@ -1523,11 +1523,15 @@
{
"name": "NY",
"district": {"code": "D1", "name": "Dist1"}
},
{
"name": "Sydney",
"district": {"code": "D1"}
}
]
}
explanation: "When duplicate XIDs are given as input to deep mutation but the object structure
is same, it should not return error."
is same or contains just xid, it should not return error."
dgquery: |-
query {
District3 as District3(func: eq(District.code, "D1")) @filter(type(District)) {
Expand Down Expand Up @@ -1575,6 +1579,17 @@
"uid":"_:City4"
}
cond: "@if(eq(len(District3), 1))"
- setjson: |
{
"City.name":"Sydney",
"City.district":{
"District.cities":[{"uid":"_:City6"}],
"uid":"uid(District3)"
},
"dgraph.type":["City"],
"uid":"_:City6"
}
cond: "@if(eq(len(District3), 1))"



Expand Down
49 changes: 33 additions & 16 deletions graphql/resolve/mutation_rewriter.go
Expand Up @@ -73,7 +73,7 @@ type mutationFragment struct {
// single mutation
type xidMetadata struct {
// variableObjMap stores the mapping of xidVariable -> the input object which contains that xid
variableObjMap map[string]interface{}
variableObjMap map[string]map[string]interface{}
// seenAtTopLevel tells whether the xidVariable has been previously seen at top level or not
seenAtTopLevel map[string]bool
// queryExists tells whether the query part in upsert has already been created for xidVariable
Expand Down Expand Up @@ -151,12 +151,39 @@ func NewDeleteRewriter() MutationRewriter {
// newXidMetadata returns a new empty *xidMetadata for storing the metadata.
func newXidMetadata() *xidMetadata {
return &xidMetadata{
variableObjMap: make(map[string]interface{}),
variableObjMap: make(map[string]map[string]interface{}),
seenAtTopLevel: make(map[string]bool),
queryExists: make(map[string]bool),
}
}

// isDuplicateXid returns true if:
// 1. we are at top level and this xid has already been seen at top level, OR
// 2. we are in a deep mutation and:
// a. this newXidObj has a field which is inverse of srcField and that
// invField is not of List type, OR
// b. newXidObj has some values other than xid and isn't equal to existingXidObject
// It is used in places where we don't want to allow duplicates.
func (xidMetadata *xidMetadata) isDuplicateXid(atTopLevel bool, xidVar string,
newXidObj map[string]interface{}, srcField schema.FieldDefinition) bool {
if atTopLevel && xidMetadata.seenAtTopLevel[xidVar] {
return true
}

if srcField != nil {
invField := srcField.Inverse()
if invField != nil && invField.Type().ListType() == nil {
return true
}
}

if len(newXidObj) > 1 && !reflect.DeepEqual(xidMetadata.variableObjMap[xidVar], newXidObj) {
return true
}

return false
}

// Rewrite takes a GraphQL schema.Mutation add and builds a Dgraph upsert mutation.
// m must have a single argument called 'input' that carries the mutation data.
//
Expand Down Expand Up @@ -957,20 +984,10 @@ func rewriteObject(
// to handle duplicate object addition/updation
variable = varGen.Next(typ, xid.Name(), xidString, false)
// check if an object with same xid has been encountered earlier
if xidObj := xidMetadata.variableObjMap[variable]; xidObj != nil {
// if we already encountered an object with same xid earlier, then we give error if:
// 1. We are at top level and this object has already been seen at top level, as no
// duplicates are allowed for top level
// 2. OR, we are in a deep mutation and:
// a. this obj is different from its first encounter
// b. OR, this object has a field which is inverse of srcField and that
// invField is not of List type
var invField schema.FieldDefinition
if srcField != nil {
invField = srcField.Inverse()
}
if (atTopLevel && xidMetadata.seenAtTopLevel[variable]) || !reflect.DeepEqual(
xidObj, obj) || (invField != nil && invField.Type().ListType() == nil) {
if xidMetadata.variableObjMap[variable] != nil {
// if we already encountered an object with same xid earlier, and this object is
// considered a duplicate of the existing object, then return error.
if xidMetadata.isDuplicateXid(atTopLevel, variable, obj, srcField) {
errFrag := newFragment(nil)
errFrag.err = errors.Errorf("duplicate XID found: %s", xidString)
return &mutationRes{secondPass: []*mutationFragment{errFrag}}
Expand Down