Skip to content

Commit

Permalink
Merge pull request #1284 from dgraph-io/jatin/sameFieldSameTypeGettin…
Browse files Browse the repository at this point in the history
…gIgnored

fix same field name in two different fragments
  • Loading branch information
lwc authored Aug 18, 2020
2 parents eb424a2 + 8a7f3e6 commit 108975c
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 5 deletions.
28 changes: 28 additions & 0 deletions graphql/context_operation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,4 +78,32 @@ func TestCollectAllFields(t *testing.T) {
s := CollectAllFields(ctx)
require.Equal(t, []string{"fieldA", "fieldB"}, s)
})

t.Run("collect fragments with same field name on different types", func(t *testing.T) {
ctx := testContext(ast.SelectionSet{
&ast.InlineFragment{
TypeCondition: "ExampleTypeA",
SelectionSet: ast.SelectionSet{
&ast.Field{
Name: "fieldA",
ObjectDefinition: &ast.Definition{Name: "ExampleTypeA"},
},
},
},
&ast.InlineFragment{
TypeCondition: "ExampleTypeB",
SelectionSet: ast.SelectionSet{
&ast.Field{
Name: "fieldA",
ObjectDefinition: &ast.Definition{Name: "ExampleTypeB"},
},
},
},
})
resCtx := GetFieldContext(ctx)
collected := CollectFields(GetOperationContext(ctx), resCtx.Field.Selections, nil)
require.Len(t, collected, 2)
require.NotEqual(t, collected[0], collected[1])
require.Equal(t, collected[0].Name, collected[1].Name)
})
}
10 changes: 5 additions & 5 deletions graphql/executable_schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func collectFields(reqCtx *OperationContext, selSet ast.SelectionSet, satisfies
if !shouldIncludeNode(sel.Directives, reqCtx.Variables) {
continue
}
f := getOrCreateAndAppendField(&groupedFields, sel.Alias, func() CollectedField {
f := getOrCreateAndAppendField(&groupedFields, sel.Alias, sel.ObjectDefinition, func() CollectedField {
return CollectedField{Field: sel}
})

Expand All @@ -45,7 +45,7 @@ func collectFields(reqCtx *OperationContext, selSet ast.SelectionSet, satisfies
continue
}
for _, childField := range collectFields(reqCtx, sel.SelectionSet, satisfies, visited) {
f := getOrCreateAndAppendField(&groupedFields, childField.Name, func() CollectedField { return childField })
f := getOrCreateAndAppendField(&groupedFields, childField.Name, childField.ObjectDefinition, func() CollectedField { return childField })
f.Selections = append(f.Selections, childField.Selections...)
}

Expand All @@ -70,7 +70,7 @@ func collectFields(reqCtx *OperationContext, selSet ast.SelectionSet, satisfies
}

for _, childField := range collectFields(reqCtx, fragment.SelectionSet, satisfies, visited) {
f := getOrCreateAndAppendField(&groupedFields, childField.Name, func() CollectedField { return childField })
f := getOrCreateAndAppendField(&groupedFields, childField.Name, childField.ObjectDefinition, func() CollectedField { return childField })
f.Selections = append(f.Selections, childField.Selections...)
}
default:
Expand All @@ -96,9 +96,9 @@ func instanceOf(val string, satisfies []string) bool {
return false
}

func getOrCreateAndAppendField(c *[]CollectedField, name string, creator func() CollectedField) *CollectedField {
func getOrCreateAndAppendField(c *[]CollectedField, name string, objectDefinition *ast.Definition, creator func() CollectedField) *CollectedField {
for i, cf := range *c {
if cf.Alias == name {
if cf.Alias == name && (cf.ObjectDefinition == objectDefinition || (cf.ObjectDefinition != nil && objectDefinition != nil && cf.ObjectDefinition.Name == objectDefinition.Name)) {
return &(*c)[i]
}
}
Expand Down

0 comments on commit 108975c

Please sign in to comment.