Skip to content

Commit 03169a2

Browse files
andimarekandimarek
committed
wip
1 parent f3ca89e commit 03169a2

2 files changed

Lines changed: 40 additions & 19 deletions

File tree

‎src/main/java/graphql/validation/OverlappingFields.java‎

Lines changed: 37 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,8 @@ public void visitSelectionSetOfOperationDefinition(OperationDefinition operation
112112
Map<String, Map<GraphQLObjectType, NormalizedField>> result = new LinkedHashMap<>(rootField.getChildrenAsMap());
113113
// we need do set it before as this is accessed in visitSelectionSet
114114
rootField.setChildren(result);
115-
visitSelectionSetImpl(operationDefinition.getSelectionSet(), result, possibleObjects, level, rootField, rootType);
115+
List<String> queryPath = new ArrayList<>();
116+
visitSelectionSetImpl(operationDefinition.getSelectionSet(), result, possibleObjects, level, rootField, rootType, queryPath);
116117
}
117118

118119
// This traverses the selection set of
@@ -137,6 +138,7 @@ public void visitSelectionSetOfField(Field field) {
137138
}
138139
GraphQLCompositeType astParentType = (GraphQLCompositeType) fieldDef.getType();
139140

141+
List<String> queryPath = new ArrayList<>(validationContext.getQueryPath());
140142

141143
List<NormalizedField> parentNormalizedFields = fieldToNormalizedField.get(field);
142144
if (parentNormalizedFields == null) {
@@ -148,7 +150,13 @@ public void visitSelectionSetOfField(Field field) {
148150
Set<GraphQLObjectType> possibleObjects = new LinkedHashSet<>(resolvePossibleObjects((GraphQLCompositeType) fieldType));
149151
Map<String, Map<GraphQLObjectType, NormalizedField>> result = new LinkedHashMap<>(parentNormalizedField.getChildrenAsMap());
150152

151-
visitSelectionSetImpl(field.getSelectionSet(), result, possibleObjects, level, parentNormalizedField, astParentType);
153+
visitSelectionSetImpl(field.getSelectionSet(),
154+
result,
155+
possibleObjects,
156+
level,
157+
parentNormalizedField,
158+
astParentType,
159+
queryPath);
152160
parentNormalizedField.replaceChildren(result);
153161
}
154162
// this means this field a top level field
@@ -161,17 +169,18 @@ private void visitSelectionSetImpl(SelectionSet selectionSet,
161169
Set<GraphQLObjectType> possibleObjects,
162170
int level,
163171
NormalizedField parentNormalizedField,
164-
GraphQLCompositeType astParentType
172+
GraphQLCompositeType astParentType,
173+
List<String> queryPath
165174
) {
166175

167176

168177
for (Selection selection : selectionSet.getSelections()) {
169178
if (selection instanceof Field) {
170-
visitField((Field) selection, result, possibleObjects, level, parentNormalizedField, astParentType);
179+
visitField((Field) selection, result, possibleObjects, level, parentNormalizedField, astParentType, queryPath);
171180
} else if (selection instanceof InlineFragment) {
172-
visitInlineFragment((InlineFragment) selection, result, possibleObjects, level, parentNormalizedField, astParentType);
181+
visitInlineFragment((InlineFragment) selection, result, possibleObjects, level, parentNormalizedField, astParentType, queryPath);
173182
} else if (selection instanceof FragmentSpread) {
174-
visitFragmentSpread((FragmentSpread) selection, result, possibleObjects, level, parentNormalizedField, astParentType);
183+
visitFragmentSpread((FragmentSpread) selection, result, possibleObjects, level, parentNormalizedField, astParentType, queryPath);
175184
}
176185
}
177186
}
@@ -181,7 +190,8 @@ private void visitFragmentSpread(FragmentSpread fragmentSpread,
181190
Set<GraphQLObjectType> possibleObjects,
182191
int level,
183192
NormalizedField parentNormalizedField,
184-
GraphQLCompositeType astParentType) {
193+
GraphQLCompositeType astParentType,
194+
List<String> queryPath) {
185195
FragmentDefinition fragmentDefinition = fragmentsByName.get(fragmentSpread.getName());
186196
if (fragmentDefinition == null) {
187197
return;
@@ -192,15 +202,17 @@ private void visitFragmentSpread(FragmentSpread fragmentSpread,
192202
return;
193203
}
194204
Set<GraphQLObjectType> newPossibleObjects = narrowDownPossibleObjects(possibleObjects, newParentType);
195-
visitSelectionSetImpl(fragmentDefinition.getSelectionSet(), result, newPossibleObjects, level, parentNormalizedField, newParentType);
205+
queryPath.add(fragmentDefinition.getName());
206+
visitSelectionSetImpl(fragmentDefinition.getSelectionSet(), result, newPossibleObjects, level, parentNormalizedField, newParentType, queryPath);
196207
}
197208

198209
private void visitInlineFragment(InlineFragment inlineFragment,
199210
Map<String, Map<GraphQLObjectType, NormalizedField>> result,
200211
Set<GraphQLObjectType> possibleObjects,
201212
int level,
202213
NormalizedField parentNormalizedField,
203-
GraphQLCompositeType astParentType) {
214+
GraphQLCompositeType astParentType,
215+
List<String> queryPath) {
204216
Set<GraphQLObjectType> newPossibleObjects = possibleObjects;
205217

206218
GraphQLCompositeType newParentType = astParentType;
@@ -213,20 +225,23 @@ private void visitInlineFragment(InlineFragment inlineFragment,
213225
newParentType = newCondition;
214226
newPossibleObjects = narrowDownPossibleObjects(possibleObjects, newCondition);
215227
}
216-
visitSelectionSetImpl(inlineFragment.getSelectionSet(), result, newPossibleObjects, level, parentNormalizedField, newParentType);
228+
visitSelectionSetImpl(inlineFragment.getSelectionSet(), result, newPossibleObjects, level, parentNormalizedField, newParentType, queryPath);
217229
}
218230

219231
private void visitField(Field field,
220232
Map<String, Map<GraphQLObjectType, NormalizedField>> result,
221233
Set<GraphQLObjectType> objectTypes,
222234
int level,
223235
NormalizedField parentNormalizedField,
224-
GraphQLCompositeType astParentType) {
236+
GraphQLCompositeType astParentType,
237+
List<String> queryPath) {
225238
GraphQLFieldDefinition fieldDef = getFieldDef(astParentType, field);
226239
if (fieldDef == null) {
227240
return;
228241
}
229242
String resultKey = field.getResultKey();
243+
queryPath = new ArrayList<>(queryPath);
244+
// queryPath.add(resultKey);
230245

231246
result.computeIfAbsent(resultKey, ignored -> new LinkedHashMap<>());
232247
Map<GraphQLObjectType, NormalizedField> objectTypeToNormalizedField = result.get(resultKey);
@@ -236,13 +251,14 @@ private void visitField(Field field,
236251
ChildOverlappingState childOverlappingState = astParentType instanceof GraphQLObjectType ? UNDECIDED_OBJECTS : SAME_FIELD;
237252
parentNormalizedField.updateChildOverlappingState(resultKey, childOverlappingState);
238253
}
254+
// we need to check here bases on the resultKey, not only matching object types!
239255
for (GraphQLObjectType objectType : objectTypes) {
240256

241257
if (objectTypeToNormalizedField.containsKey(objectType)) {
242258
NormalizedField existingChild = objectTypeToNormalizedField.get(objectType);
243259
Conflict conflict = checkIfFieldIsCompatible(field, astParentType, fieldDef.getType(), existingChild, parentNormalizedField);
244260
if (conflict != null) {
245-
addError(FieldsConflict, conflict.fields, conflict.reason);
261+
addError(FieldsConflict, conflict.fields, conflict.reason, queryPath);
246262
continue;
247263
}
248264

@@ -378,7 +394,7 @@ private Conflict checkExactlySameField(String resultKey, Field fieldA, Field fie
378394
String fieldNameB = fieldB.getName();
379395
if (!fieldNameA.equals(fieldNameB)) {
380396
String reason = format("%s: %s and %s are different fields", resultKey, fieldNameB, fieldNameA);
381-
return new Conflict(resultKey, reason, fieldA, fieldB);
397+
return new Conflict(resultKey, reason, fieldB, fieldA);
382398
}
383399
if (!sameType(typeA, typeB)) {
384400
return mkNotSameTypeError(resultKey, fieldA, fieldB, typeA, typeB);
@@ -545,15 +561,20 @@ private GraphQLFieldDefinition getFieldDef(GraphQLType parentType, Field field)
545561
return null;
546562
}
547563

548-
public void addError(ValidationErrorType validationErrorType, List<? extends Node<?>> locations, String description) {
564+
public void addError(ValidationErrorType validationErrorType,
565+
List<? extends Node<?>> locations,
566+
String description,
567+
List<String> queryPath) {
549568
List<SourceLocation> locationList = new ArrayList<>();
550569
for (Node<?> node : locations) {
551570
locationList.add(node.getSourceLocation());
552571
}
553572
addError(newValidationError()
554573
.validationErrorType(validationErrorType)
555574
.sourceLocations(locationList)
556-
.description(description));
575+
.description(description)
576+
.queryPath(queryPath)
577+
);
557578
}
558579

559580
public void addError(ValidationErrorType validationErrorType, SourceLocation location, String description) {
@@ -564,7 +585,7 @@ public void addError(ValidationErrorType validationErrorType, SourceLocation loc
564585
}
565586

566587
public void addError(ValidationError.Builder validationError) {
567-
validationErrorCollector.addError(validationError.queryPath(validationContext.getQueryPath()).build());
588+
validationErrorCollector.addError(validationError.build());
568589
}
569590

570591

‎src/test/groovy/graphql/validation/MainValidationTraversalTestMergedFieldValidation.groovy‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,13 +70,13 @@ class MainValidationTraversalTestMergedFieldValidation extends Specification {
7070
def "two aliases for different fields in fragment"() {
7171
given:
7272
def query = """
73-
{
74-
...f
75-
}
7673
fragment f on Test{
7774
myName : name
7875
myName : nickname
7976
}
77+
{
78+
...f
79+
}
8080
"""
8181
when:
8282
traverse(query, null)

0 commit comments

Comments
 (0)