Skip to content

Commit 5be7647

Browse files
bbakermanbbakerman
committed
PR feedback
1 parent 1c70957 commit 5be7647

File tree

6 files changed

+47
-40
lines changed

6 files changed

+47
-40
lines changed

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,11 +58,11 @@ public void addError(ValidationErrorType validationErrorType, List<? extends Nod
5858
for (Node node : locations) {
5959
locationList.add(node.getSourceLocation());
6060
}
61-
validationErrorCollector.addError(new ValidationError(validationErrorType, locationList, description, getPath()));
61+
validationErrorCollector.addError(new ValidationError(validationErrorType, locationList, description, getQueryPath()));
6262
}
6363

6464
public void addError(ValidationErrorType validationErrorType, SourceLocation location, String description) {
65-
validationErrorCollector.addError(new ValidationError(validationErrorType, location, description, getPath()));
65+
validationErrorCollector.addError(new ValidationError(validationErrorType, location, description, getQueryPath()));
6666
}
6767

6868
public List<ValidationError> getErrors() {
@@ -74,8 +74,8 @@ public ValidationContext getValidationContext() {
7474
return validationContext;
7575
}
7676

77-
protected List<Object> getPath() {
78-
return validationContext.getPath();
77+
protected List<String> getQueryPath() {
78+
return validationContext.getQueryPath();
7979
}
8080

8181
public void checkArgument(Argument argument) {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -276,7 +276,7 @@ public GraphQLFieldDefinition getFieldDef() {
276276
return lastElement(fieldDefStack);
277277
}
278278

279-
public List<Object> getPath() {
279+
public List<String> getQueryPath() {
280280
if (nameStack.isEmpty()) {
281281
return null;
282282
}

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -83,12 +83,12 @@ public GraphQLOutputType getOutputType() {
8383
}
8484

8585

86-
public List<Object> getPath() {
87-
return traversalContext.getPath();
86+
public List<String> getQueryPath() {
87+
return traversalContext.getQueryPath();
8888
}
8989

9090
@Override
9191
public String toString() {
92-
return "ValidationContext{" + getPath() + "}";
92+
return "ValidationContext{" + getQueryPath() + "}";
9393
}
9494
}

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

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,15 @@
99
import java.util.ArrayList;
1010
import java.util.Collections;
1111
import java.util.List;
12+
import java.util.stream.Collectors;
1213

1314
public class ValidationError implements GraphQLError {
1415

1516
private final String message;
1617
private final List<SourceLocation> locations = new ArrayList<>();
1718
private final String description;
1819
private final ValidationErrorType validationErrorType;
19-
private final List<Object> path;
20+
private final List<String> queryPath;
2021

2122
public ValidationError(ValidationErrorType validationErrorType) {
2223
this(validationErrorType, (SourceLocation) null, null);
@@ -26,29 +27,36 @@ public ValidationError(ValidationErrorType validationErrorType, SourceLocation s
2627
this(validationErrorType, nullOrList(sourceLocation), description, null);
2728
}
2829

29-
public ValidationError(ValidationErrorType validationErrorType, SourceLocation sourceLocation, String description, List<Object> path) {
30-
this(validationErrorType, nullOrList(sourceLocation), description, path);
30+
public ValidationError(ValidationErrorType validationErrorType, SourceLocation sourceLocation, String description, List<String> queryPath) {
31+
this(validationErrorType, nullOrList(sourceLocation), description, queryPath);
3132
}
3233

3334
public ValidationError(ValidationErrorType validationErrorType, List<SourceLocation> sourceLocations, String description) {
3435
this(validationErrorType, sourceLocations, description, null);
3536
}
3637

37-
public ValidationError(ValidationErrorType validationErrorType, List<SourceLocation> sourceLocations, String description, List<Object> path) {
38+
public ValidationError(ValidationErrorType validationErrorType, List<SourceLocation> sourceLocations, String description, List<String> queryPath) {
3839
this.validationErrorType = validationErrorType;
3940
if (sourceLocations != null)
4041
this.locations.addAll(sourceLocations);
4142
this.description = description;
42-
this.message = mkMessage(validationErrorType, description);
43-
this.path = path;
43+
this.message = mkMessage(validationErrorType, description, queryPath);
44+
this.queryPath = queryPath;
4445
}
4546

4647
private static List<SourceLocation> nullOrList(SourceLocation sourceLocation) {
4748
return sourceLocation == null ? null : Collections.singletonList(sourceLocation);
4849
}
4950

50-
private String mkMessage(ValidationErrorType validationErrorType, String description) {
51-
return String.format("Validation error of type %s: %s", validationErrorType, description);
51+
private String mkMessage(ValidationErrorType validationErrorType, String description, List<String> queryPath) {
52+
return String.format("Validation error of type %s: %s%s", validationErrorType, description, toPath(queryPath));
53+
}
54+
55+
private String toPath(List<String> queryPath) {
56+
if (queryPath == null) {
57+
return "";
58+
}
59+
return String.format(" @ '%s'", queryPath.stream().collect(Collectors.joining("/")));
5260
}
5361

5462
public ValidationErrorType getValidationErrorType() {
@@ -69,22 +77,21 @@ public List<SourceLocation> getLocations() {
6977
return locations;
7078
}
7179

72-
@Override
73-
public List<Object> getPath() {
74-
return path;
75-
}
76-
7780
@Override
7881
public ErrorType getErrorType() {
7982
return ErrorType.ValidationError;
8083
}
8184

85+
public List<String> getQueryPath() {
86+
return queryPath;
87+
}
88+
8289

8390
@Override
8491
public String toString() {
8592
return "ValidationError{" +
8693
"validationErrorType=" + validationErrorType +
87-
", path=" + path +
94+
", queryPath=" + queryPath +
8895
", message=" + message +
8996
", locations=" + locations +
9097
", description='" + description + '\'' +

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

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,10 @@ class ValidationPathTest extends SpecValidationBase {
2121

2222
then:
2323
validationErrors.size() == 4
24-
validationErrors[0].path == ["dog", "doesKnowCommand"]
25-
validationErrors[1].path == ["dog", "owner", "name"]
26-
validationErrors[2].path == ["dog", "owner", "badField"]
27-
validationErrors[3].path == ["elephant"]
24+
validationErrors[0].queryPath == ["dog", "doesKnowCommand"]
25+
validationErrors[1].queryPath == ["dog", "owner", "name"]
26+
validationErrors[2].queryPath == ["dog", "owner", "badField"]
27+
validationErrors[3].queryPath == ["elephant"]
2828
}
2929

3030
def "fragments validation errors have paths"() {
@@ -55,11 +55,11 @@ class ValidationPathTest extends SpecValidationBase {
5555

5656
then:
5757
validationErrors.size() == 5
58-
validationErrors[0].path == ["dog", "owner"]
59-
validationErrors[1].path == ["dog", "owner", "homePlanet"]
60-
validationErrors[2].path == ["namedFragment", "doesKnowCommand"]
61-
validationErrors[3].path == ["namedFragment", "owner", "name"]
62-
validationErrors[4].path == ["namedFragment", "owner", "badField"]
58+
validationErrors[0].queryPath == ["dog", "owner"]
59+
validationErrors[1].queryPath == ["dog", "owner", "homePlanet"]
60+
validationErrors[2].queryPath == ["namedFragment", "doesKnowCommand"]
61+
validationErrors[3].queryPath == ["namedFragment", "owner", "name"]
62+
validationErrors[4].queryPath == ["namedFragment", "owner", "badField"]
6363

6464
}
6565
}

‎src/test/groovy/graphql/validation/rules/OverlappingFieldsCanBeMergedTest.groovy‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ class OverlappingFieldsCanBeMergedTest extends Specification {
7171

7272
then:
7373
errorCollector.getErrors().size() == 1
74-
errorCollector.getErrors()[0].message == "Validation error of type FieldsConflict: myName: name and nickname are different fields"
74+
errorCollector.getErrors()[0].message == "Validation error of type FieldsConflict: myName: name and nickname are different fields @ 'f'"
7575
errorCollector.getErrors()[0].locations == [new SourceLocation(3, 17), new SourceLocation(4, 17)]
7676
}
7777

@@ -128,7 +128,7 @@ class OverlappingFieldsCanBeMergedTest extends Specification {
128128

129129
then:
130130
errorCollector.getErrors().size() == 1
131-
errorCollector.getErrors()[0].message == "Validation error of type FieldsConflict: scalar: they return differing types Int and String"
131+
errorCollector.getErrors()[0].message == "Validation error of type FieldsConflict: scalar: they return differing types Int and String @ 'boxUnion'"
132132
}
133133

134134

@@ -259,7 +259,7 @@ class OverlappingFieldsCanBeMergedTest extends Specification {
259259

260260
then:
261261
errorCollector.getErrors().size() == 1
262-
errorCollector.getErrors()[0].message == "Validation error of type FieldsConflict: fido: name and nickname are different fields"
262+
errorCollector.getErrors()[0].message == "Validation error of type FieldsConflict: fido: name and nickname are different fields @ 'sameAliasesWithDifferentFieldTargets'"
263263
errorCollector.getErrors()[0].locations == [new SourceLocation(3, 13), new SourceLocation(4, 13)]
264264
}
265265

@@ -276,7 +276,7 @@ class OverlappingFieldsCanBeMergedTest extends Specification {
276276

277277
then:
278278
errorCollector.getErrors().size() == 1
279-
errorCollector.getErrors()[0].message == "Validation error of type FieldsConflict: name: nickname and name are different fields"
279+
errorCollector.getErrors()[0].message == "Validation error of type FieldsConflict: name: nickname and name are different fields @ 'aliasMaskingDirectFieldAccess'"
280280
errorCollector.getErrors()[0].locations == [new SourceLocation(3, 13), new SourceLocation(4, 13)]
281281
}
282282

@@ -293,7 +293,7 @@ class OverlappingFieldsCanBeMergedTest extends Specification {
293293

294294
then:
295295
errorCollector.getErrors().size() == 1
296-
errorCollector.getErrors()[0].message == "Validation error of type FieldsConflict: doesKnowCommand: they have differing arguments"
296+
errorCollector.getErrors()[0].message == "Validation error of type FieldsConflict: doesKnowCommand: they have differing arguments @ 'conflictingArgs'"
297297
errorCollector.getErrors()[0].locations == [new SourceLocation(3, 13), new SourceLocation(4, 13)]
298298
}
299299

@@ -370,13 +370,13 @@ class OverlappingFieldsCanBeMergedTest extends Specification {
370370
then:
371371
errorCollector.getErrors().size() == 3
372372

373-
errorCollector.getErrors()[0].message == "Validation error of type FieldsConflict: x: a and b are different fields"
373+
errorCollector.getErrors()[0].message == "Validation error of type FieldsConflict: x: a and b are different fields @ 'f1'"
374374
errorCollector.getErrors()[0].locations == [new SourceLocation(18, 13), new SourceLocation(21, 13)]
375375

376-
errorCollector.getErrors()[1].message == "Validation error of type FieldsConflict: x: a and c are different fields"
376+
errorCollector.getErrors()[1].message == "Validation error of type FieldsConflict: x: a and c are different fields @ 'f3'"
377377
errorCollector.getErrors()[1].locations == [new SourceLocation(18, 13), new SourceLocation(14, 17)]
378378

379-
errorCollector.getErrors()[2].message == "Validation error of type FieldsConflict: x: b and c are different fields"
379+
errorCollector.getErrors()[2].message == "Validation error of type FieldsConflict: x: b and c are different fields @ 'f3'"
380380
errorCollector.getErrors()[2].locations == [new SourceLocation(21, 13), new SourceLocation(14, 17)]
381381
}
382382

@@ -471,7 +471,7 @@ class OverlappingFieldsCanBeMergedTest extends Specification {
471471

472472
then:
473473
errorCollector.getErrors().size() == 1
474-
errorCollector.getErrors()[0].message == "Validation error of type FieldsConflict: deepField: (x: a and b are different fields)"
474+
errorCollector.getErrors()[0].message == "Validation error of type FieldsConflict: deepField: (x: a and b are different fields) @ 'field'"
475475
errorCollector.getErrors()[0].locations.size() == 4
476476
}
477477

0 commit comments

Comments
 (0)