Skip to content

Commit

Permalink
adds experimental fast overlapping-fields-can-be-merged algorithm
Browse files Browse the repository at this point in the history 
fixes graphql-java#1786
  • Loading branch information
Jezdik, Radek authored and Jezdik, Radek committed Apr 8, 2020
1 parent c98ebf7 commit ffc0f10
Show file tree
Hide file tree
Showing 12 changed files with 1,688 additions and 27 deletions.
14 changes: 10 additions & 4 deletions src/main/java/graphql/validation/Validator.java
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package graphql.validation;


import graphql.Internal;
import graphql.language.Document;
import graphql.schema.GraphQLSchema;
Expand Down Expand Up @@ -30,6 +29,7 @@
import graphql.validation.rules.VariableDefaultValuesOfCorrectType;
import graphql.validation.rules.VariableTypesMatchRule;
import graphql.validation.rules.VariablesAreInputTypes;
import graphql.validation.rules.experimental.overlappingfieldsmerge.OverlappingFieldsCanBeMergedEx;

import java.util.ArrayList;
import java.util.List;
Expand All @@ -40,7 +40,6 @@ public class Validator {
public List<ValidationError> validateDocument(GraphQLSchema schema, Document document) {
ValidationContext validationContext = new ValidationContext(schema, document);


ValidationErrorCollector validationErrorCollector = new ValidationErrorCollector();
List<AbstractRule> rules = createRules(validationContext, validationErrorCollector);
LanguageTraversal languageTraversal = new LanguageTraversal();
Expand Down Expand Up @@ -81,8 +80,15 @@ private List<AbstractRule> createRules(ValidationContext validationContext, Vali
NoUnusedVariables noUnusedVariables = new NoUnusedVariables(validationContext, validationErrorCollector);
rules.add(noUnusedVariables);

OverlappingFieldsCanBeMerged overlappingFieldsCanBeMerged = new OverlappingFieldsCanBeMerged(validationContext, validationErrorCollector);
rules.add(overlappingFieldsCanBeMerged);
if (System.getProperty("graphql.experimental.validation.overlappingFieldsCanBeMerged", Boolean.FALSE.toString()).equals(Boolean.TRUE.toString())) {
OverlappingFieldsCanBeMergedEx overlappingFieldsCanBeMerged =
new OverlappingFieldsCanBeMergedEx(validationContext, validationErrorCollector);
rules.add(overlappingFieldsCanBeMerged);
} else {
OverlappingFieldsCanBeMerged overlappingFieldsCanBeMerged =
new OverlappingFieldsCanBeMerged(validationContext, validationErrorCollector);
rules.add(overlappingFieldsCanBeMerged);
}

PossibleFragmentSpreads possibleFragmentSpreads = new PossibleFragmentSpreads(validationContext, validationErrorCollector);
rules.add(possibleFragmentSpreads);
Expand Down
68 changes: 45 additions & 23 deletions src/main/java/graphql/validation/rules/OverlappingFieldsCanBeMerged.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.function.BiFunction;

import static graphql.schema.GraphQLTypeUtil.isEnum;
import static graphql.schema.GraphQLTypeUtil.isList;
Expand All @@ -45,9 +46,16 @@
*/
public class OverlappingFieldsCanBeMerged extends AbstractRule {


private final List<FieldPair> alreadyChecked = new ArrayList<>();

private final BiFunction<Field, Field, Boolean> alreadyCheckedOtherwiseAdd = (fieldA, fieldB) -> {
boolean checked = isAlreadyChecked(fieldA, fieldB);
if (!checked) {
alreadyChecked.add(new FieldPair(fieldA, fieldB));
}
return checked;
};

public OverlappingFieldsCanBeMerged(ValidationContext validationContext, ValidationErrorCollector validationErrorCollector) {
super(validationContext, validationErrorCollector);
}
Expand All @@ -69,7 +77,10 @@ private List<Conflict> findConflicts(Map<String, List<FieldAndType>> fieldMap) {
List<FieldAndType> fieldAndTypes = fieldMap.get(name);
for (int i = 0; i < fieldAndTypes.size(); i++) {
for (int j = i + 1; j < fieldAndTypes.size(); j++) {
Conflict conflict = findConflict(name, fieldAndTypes.get(i), fieldAndTypes.get(j));
Conflict conflict = findConflict(alreadyCheckedOtherwiseAdd, name, fieldAndTypes.get(i), fieldAndTypes.get(j));
if (conflict == null) {
conflict = findConflictInChildSelections(name, fieldAndTypes.get(i), fieldAndTypes.get(j));
}
if (conflict != null) {
result.add(conflict);
}
Expand All @@ -92,23 +103,22 @@ private boolean isAlreadyChecked(Field field1, Field field2) {
}

@SuppressWarnings("ConstantConditions")
private Conflict findConflict(String responseName, FieldAndType fieldAndTypeA, FieldAndType fieldAndTypeB) {
public static Conflict findConflict(BiFunction<Field, Field, Boolean> alreadyCheckedFunction, String responseName, FieldAndType fieldAndTypeA, FieldAndType fieldAndTypeB) {

Field fieldA = fieldAndTypeA.field;
Field fieldB = fieldAndTypeB.field;

if (isAlreadyChecked(fieldA, fieldB)) {
if (alreadyCheckedFunction.apply(fieldA, fieldB)) {
return null;
}
alreadyChecked.add(new FieldPair(fieldA, fieldB));

String fieldNameA = fieldA.getName();
String fieldNameB = fieldB.getName();

GraphQLType typeA = fieldAndTypeA.graphQLType;
GraphQLType typeB = fieldAndTypeB.graphQLType;

Conflict conflict = checkListAndNonNullConflict(responseName,fieldAndTypeA,fieldAndTypeB);
Conflict conflict = checkListAndNonNullConflict(responseName, fieldAndTypeA, fieldAndTypeB);
if (conflict != null) {
return conflict;
}
Expand Down Expand Up @@ -147,6 +157,18 @@ private Conflict findConflict(String responseName, FieldAndType fieldAndTypeA, F
String reason = format("%s: they have differing arguments", responseName);
return new Conflict(responseName, reason, fieldA, fieldB);
}
return null;
}

private Conflict findConflictInChildSelections(
String responseName, FieldAndType fieldAndTypeA, FieldAndType fieldAndTypeB
) {
Field fieldA = fieldAndTypeA.field;
Field fieldB = fieldAndTypeB.field;

GraphQLType typeA = unwrapAll(fieldAndTypeA.graphQLType);
GraphQLType typeB = unwrapAll(fieldAndTypeB.graphQLType);

SelectionSet selectionSet1 = fieldA.getSelectionSet();
SelectionSet selectionSet2 = fieldB.getSelectionSet();
if (selectionSet1 != null && selectionSet2 != null) {
Expand All @@ -167,7 +189,7 @@ private Conflict findConflict(String responseName, FieldAndType fieldAndTypeA, F
return null;
}

private Conflict checkListAndNonNullConflict(String responseName, FieldAndType fieldAndTypeA, FieldAndType fieldAndTypeB) {
private static Conflict checkListAndNonNullConflict(String responseName, FieldAndType fieldAndTypeA, FieldAndType fieldAndTypeB) {

GraphQLType typeA = fieldAndTypeA.graphQLType;
GraphQLType typeB = fieldAndTypeB.graphQLType;
Expand All @@ -194,7 +216,7 @@ private Conflict checkListAndNonNullConflict(String responseName, FieldAndType f
return null;
}

private boolean checkScalarAndEnumConflict(GraphQLType typeA, GraphQLType typeB) {
private static boolean checkScalarAndEnumConflict(GraphQLType typeA, GraphQLType typeB) {
if (isScalar(typeA) || isScalar(typeB)) {
if (!sameType(typeA, typeB)) {
return true;
Expand All @@ -208,22 +230,22 @@ private boolean checkScalarAndEnumConflict(GraphQLType typeA, GraphQLType typeB)
return false;
}

private Conflict mkNotSameTypeError(String responseName, Field fieldA, Field fieldB, GraphQLType typeA, GraphQLType typeB) {
private static Conflict mkNotSameTypeError(String responseName, Field fieldA, Field fieldB, GraphQLType typeA, GraphQLType typeB) {
String name1 = typeA != null ? simplePrint(typeA) : "null";
String name2 = typeB != null ? simplePrint(typeB) : "null";
String reason = format("%s: they return differing types %s and %s", responseName, name1, name2);
return new Conflict(responseName, reason, fieldA, fieldB);
}

private List<Field> collectFields(List<Conflict> conflicts) {
private static List<Field> collectFields(List<Conflict> conflicts) {
List<Field> result = new ArrayList<>();
for (Conflict conflict : conflicts) {
result.addAll(conflict.fields);
}
return result;
}

private String joinReasons(List<Conflict> conflicts) {
private static String joinReasons(List<Conflict> conflicts) {
StringBuilder result = new StringBuilder();
result.append("(");
for (Conflict conflict : conflicts) {
Expand All @@ -236,15 +258,15 @@ private String joinReasons(List<Conflict> conflicts) {
}

@SuppressWarnings("SimplifiableIfStatement")
private boolean sameType(GraphQLType type1, GraphQLType type2) {
private static boolean sameType(GraphQLType type1, GraphQLType type2) {
if (type1 == null || type2 == null) {
return true;
}
return type1.equals(type2);
}

@SuppressWarnings("SimplifiableIfStatement")
private boolean sameValue(Value value1, Value value2) {
private static boolean sameValue(Value value1, Value value2) {
if (value1 == null && value2 == null) {
return true;
}
Expand All @@ -257,7 +279,7 @@ private boolean sameValue(Value value1, Value value2) {
return new AstComparator().isEqual(value1, value2);
}

private boolean sameArguments(List<Argument> arguments1, List<Argument> arguments2) {
private static boolean sameArguments(List<Argument> arguments1, List<Argument> arguments2) {
if (arguments1.size() != arguments2.size()) {
return false;
}
Expand All @@ -273,7 +295,7 @@ private boolean sameArguments(List<Argument> arguments1, List<Argument> argument
return true;
}

private Argument findArgumentByName(String name, List<Argument> arguments) {
private static Argument findArgumentByName(String name, List<Argument> arguments) {
for (Argument argument : arguments) {
if (argument.getName().equals(name)) {
return argument;
Expand Down Expand Up @@ -347,10 +369,10 @@ public FieldPair(Field field1, Field field2) {
}
}

private static class Conflict {
final String responseName;
final String reason;
final List<Field> fields = new ArrayList<>();
public static class Conflict {
public final String responseName;
public final String reason;
public final List<Field> fields = new ArrayList<>();

public Conflict(String responseName, String reason, Field field1, Field field2) {
this.responseName = responseName;
Expand All @@ -368,10 +390,10 @@ public Conflict(String responseName, String reason, List<Field> fields) {
}


private static class FieldAndType {
final Field field;
final GraphQLType graphQLType;
final GraphQLType parentType;
public static class FieldAndType {
public final Field field;
public final GraphQLType graphQLType;
public final GraphQLType parentType;

public FieldAndType(Field field, GraphQLType graphQLType, GraphQLType parentType) {
this.field = field;
Expand Down
Loading

0 comments on commit ffc0f10

Please sign in to comment.