Skip to content

Commit fac552d

Browse files
andimarekandimarek
committed
fields which are not part of the same hierarchy should not be merged
1 parent f0c52ac commit fac552d

File tree

4 files changed

+65
-3
lines changed

4 files changed

+65
-3
lines changed

‎src/main/java/graphql/normalized/ENFMerger.java‎

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,9 @@
33
import graphql.Internal;
44
import graphql.language.Argument;
55
import graphql.language.AstComparator;
6+
import graphql.schema.GraphQLNamedOutputType;
7+
import graphql.schema.GraphQLObjectType;
8+
import graphql.schema.GraphQLSchema;
69

710
import java.util.ArrayList;
811
import java.util.Iterator;
@@ -14,7 +17,7 @@
1417
@Internal
1518
public class ENFMerger {
1619

17-
public static void merge(ExecutableNormalizedField parent, List<ExecutableNormalizedField> childrenWithSameResultKey) {
20+
public static void merge(ExecutableNormalizedField parent, List<ExecutableNormalizedField> childrenWithSameResultKey, GraphQLSchema schema) {
1821
// they have all the same result key
1922
// we can only merge the fields if they have the same field name + arguments + all children are the same
2023
List<Set<ExecutableNormalizedField>> possibleGroupsToMerge = new ArrayList<>();
@@ -23,7 +26,10 @@ public static void merge(ExecutableNormalizedField parent, List<ExecutableNormal
2326
overPossibleGroups:
2427
for (Set<ExecutableNormalizedField> group : possibleGroupsToMerge) {
2528
for (ExecutableNormalizedField fieldInGroup : group) {
26-
if (field.getFieldName().equals(fieldInGroup.getFieldName()) && sameArguments(field.getAstArguments(), fieldInGroup.getAstArguments())) {
29+
if (field.getFieldName().equals(fieldInGroup.getFieldName()) &&
30+
sameArguments(field.getAstArguments(), fieldInGroup.getAstArguments())
31+
&& isPartOfTheSameHierarchy(field, fieldInGroup, schema)
32+
) {
2733
addToGroup = true;
2834
group.add(field);
2935
continue overPossibleGroups;
@@ -58,6 +64,18 @@ public static void merge(ExecutableNormalizedField parent, List<ExecutableNormal
5864
}
5965
}
6066

67+
private static boolean isPartOfTheSameHierarchy(ExecutableNormalizedField fieldOne, ExecutableNormalizedField fieldTwo, GraphQLSchema schema) {
68+
// we can get away with only checking one of the object names, because all object names in one ENF are guaranteed to be part of
69+
// the same hierarchy
70+
String firstObject = fieldOne.getSingleObjectTypeName();
71+
String secondObject = fieldTwo.getSingleObjectTypeName();
72+
GraphQLObjectType objectTypeOne = schema.getObjectType(firstObject);
73+
GraphQLObjectType objectTypeTwo = schema.getObjectType(firstObject);
74+
List<GraphQLNamedOutputType> interfacesOne = objectTypeOne.getInterfaces();
75+
List<GraphQLNamedOutputType> interfacesTwo = objectTypeTwo.getInterfaces();
76+
return interfacesOne.stream().anyMatch(interfacesTwo::contains);
77+
}
78+
6179

6280
private static boolean areFieldSetsTheSame(List<Set<ExecutableNormalizedField>> listOfSets) {
6381
if (listOfSets.size() == 0 || listOfSets.size() == 1) {

‎src/main/java/graphql/normalized/ExecutableNormalizedField.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,10 @@ public Set<String> getObjectTypeNames() {
195195
return objectTypeNames;
196196
}
197197

198+
public String getSingleObjectTypeName() {
199+
return objectTypeNames.iterator().next();
200+
}
201+
198202
public GraphQLObjectType getOneObjectType(GraphQLSchema schema) {
199203
return (GraphQLObjectType) schema.getType(objectTypeNames.iterator().next());
200204
}

‎src/main/java/graphql/normalized/ExecutableNormalizedOperationFactory.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ private ExecutableNormalizedOperation createNormalizedQueryImpl(GraphQLSchema gr
137137
}
138138
for (FieldCollectorNormalizedQueryParams.PossibleMerger possibleMerger : parameters.possibleMergerList) {
139139
List<ExecutableNormalizedField> childrenWithSameResultKey = possibleMerger.parent.getChildrenWithSameResultKey(possibleMerger.resultKey);
140-
ENFMerger.merge(possibleMerger.parent, childrenWithSameResultKey);
140+
ENFMerger.merge(possibleMerger.parent, childrenWithSameResultKey, graphQLSchema);
141141
}
142142
return new ExecutableNormalizedOperation(new ArrayList<>(collectFromOperationResult.children), fieldToNormalizedField.build(), normalizedFieldToMergedField.build(), coordinatesToNormalizedFields.build());
143143
}

‎src/test/groovy/graphql/normalized/ExecutableNormalizedOperationFactoryTest.groovy‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1852,6 +1852,46 @@ schema {
18521852
]
18531853
}
18541854

1855+
def "union fields are not merged"() {
1856+
given:
1857+
def schema = schema('''
1858+
type Query {
1859+
pets: [DogOrCat]
1860+
}
1861+
type Dog {
1862+
name: String
1863+
}
1864+
type Cat {
1865+
name: String
1866+
}
1867+
union DogOrCat = Dog | Cat
1868+
''')
1869+
def query = '''
1870+
{
1871+
pets {
1872+
... on Dog {
1873+
name
1874+
}
1875+
... on Cat {
1876+
name
1877+
}
1878+
}
1879+
}
1880+
'''
1881+
assertValidQuery(schema, query)
1882+
Document document = TestUtil.parseQuery(query)
1883+
ExecutableNormalizedOperationFactory dependencyGraph = new ExecutableNormalizedOperationFactory();
1884+
when:
1885+
def tree = dependencyGraph.createExecutableNormalizedOperationWithRawVariables(schema, document, null, [:])
1886+
def printedTree = printTreeWithLevelInfo(tree, schema)
1887+
1888+
then:
1889+
printedTree == ['-Query.pets: [DogOrCat]',
1890+
'--Dog.name: String',
1891+
'--Cat.name: String'
1892+
]
1893+
}
1894+
18551895
def "fields are merged together on multiple level"() {
18561896
given:
18571897
def schema = schema('''

0 commit comments

Comments
 (0)