Skip to content

Commit a15baf6

Browse files
author
Raymie Stata
committed
Refactor built-in directive handling
A reviewer noted that the dispirate ways that the classic builder and the new "fast" builder handled built-in directives created two parallel lists, which is a maintenance burden. This commit consolidates the lists.
1 parent d430e66 commit a15baf6

2 files changed

Lines changed: 43 additions & 22 deletions

File tree

‎src/main/java/graphql/Directives.java‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import graphql.language.StringValue;
88
import graphql.schema.GraphQLDirective;
99

10+
import java.util.List;
1011
import java.util.concurrent.atomic.AtomicBoolean;
1112

1213
import static graphql.Scalars.GraphQLBoolean;
@@ -249,6 +250,35 @@ public class Directives {
249250
.definition(EXPERIMENTAL_DISABLE_ERROR_PROPAGATION_DIRECTIVE_DEFINITION)
250251
.build();
251252

253+
/**
254+
* Returns the directives that are included in a schema by default but can be removed
255+
* by calling {@code clearDirectives()} on the builder.
256+
*
257+
* @return an unmodifiable list of default directives (include, skip)
258+
*/
259+
@Internal
260+
public static List<GraphQLDirective> getDefaultDirectives() {
261+
return List.of(IncludeDirective, SkipDirective);
262+
}
263+
264+
/**
265+
* Returns the directives that are mandatory and will always be added to a schema,
266+
* even after {@code clearDirectives()} is called on the builder.
267+
* These are inherently part of the GraphQL spec.
268+
*
269+
* @return an unmodifiable list of mandatory directives
270+
*/
271+
@Internal
272+
public static List<GraphQLDirective> getMandatoryDirectives() {
273+
return List.of(
274+
DeprecatedDirective,
275+
SpecifiedByDirective,
276+
OneOfDirective,
277+
DeferDirective,
278+
ExperimentalDisableErrorPropagationDirective
279+
);
280+
}
281+
252282
private static Description createDescription(String s) {
253283
return new Description(s, null, false);
254284
}

‎src/main/java/graphql/schema/GraphQLSchema.java‎

Lines changed: 13 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,6 @@
4242
import static graphql.collect.ImmutableKit.nonNullCopyOf;
4343
import static graphql.schema.GraphqlTypeComparators.byNameAsc;
4444
import static graphql.schema.GraphqlTypeComparators.sortTypes;
45-
import static java.util.Arrays.asList;
4645
import static java.util.Collections.singletonList;
4746

4847
/**
@@ -812,9 +811,10 @@ public static class Builder {
812811
private List<SchemaExtensionDefinition> extensionDefinitions;
813812
private String description;
814813

815-
// we default these in
814+
// We initially add these default directives (e.g., include and skip), but these can be
815+
// cleared by the user (unlike mandatory ones which are always re-added in buildImpl)
816816
private final Set<GraphQLDirective> additionalDirectives = new LinkedHashSet<>(
817-
asList(Directives.IncludeDirective, Directives.SkipDirective)
817+
Directives.getDefaultDirectives()
818818
);
819819
private final Set<GraphQLNamedType> additionalTypes = new LinkedHashSet<>();
820820
private final List<GraphQLDirective> schemaDirectives = new ArrayList<>();
@@ -1031,13 +1031,8 @@ private GraphQLSchema buildImpl() {
10311031
assertNotNull(additionalTypes, "additionalTypes can't be null");
10321032
assertNotNull(additionalDirectives, "additionalDirectives can't be null");
10331033

1034-
// schemas built via the schema generator have the deprecated directive BUT we want it present for hand built
1035-
// schemas - it's inherently part of the spec!
1036-
addBuiltInDirective(Directives.DeprecatedDirective, additionalDirectives);
1037-
addBuiltInDirective(Directives.SpecifiedByDirective, additionalDirectives);
1038-
addBuiltInDirective(Directives.OneOfDirective, additionalDirectives);
1039-
addBuiltInDirective(Directives.DeferDirective, additionalDirectives);
1040-
addBuiltInDirective(Directives.ExperimentalDisableErrorPropagationDirective, additionalDirectives);
1034+
// Mandatory directives are always added, even after clearDirectives() - they're part of the spec
1035+
Directives.getMandatoryDirectives().forEach(d -> addBuiltInDirective(d, additionalDirectives));
10411036

10421037
// quick build - no traversing
10431038
final GraphQLSchema partiallyBuiltSchema = new GraphQLSchema(this);
@@ -1060,19 +1055,19 @@ private GraphQLSchema buildImpl() {
10601055
return validateSchema(finalSchema);
10611056
}
10621057

1063-
private void addBuiltInDirective(GraphQLDirective qlDirective, Set<GraphQLDirective> additionalDirectives1) {
1064-
if (additionalDirectives1.stream().noneMatch(d -> d.getName().equals(qlDirective.getName()))) {
1065-
additionalDirectives1.add(qlDirective);
1066-
}
1067-
}
1068-
10691058
private GraphQLSchema validateSchema(GraphQLSchema graphQLSchema) {
10701059
Collection<SchemaValidationError> errors = new SchemaValidator().validateSchema(graphQLSchema);
10711060
if (!errors.isEmpty()) {
10721061
throw new InvalidSchemaException(errors);
10731062
}
10741063
return graphQLSchema;
10751064
}
1065+
1066+
private void addBuiltInDirective(GraphQLDirective qlDirective, Set<GraphQLDirective> additionalDirectives1) {
1067+
if (additionalDirectives1.stream().noneMatch(d -> d.getName().equals(qlDirective.getName()))) {
1068+
additionalDirectives1.add(qlDirective);
1069+
}
1070+
}
10761071
}
10771072

10781073
/**
@@ -1383,12 +1378,8 @@ public GraphQLSchema build() {
13831378
}
13841379

13851380
private void addBuiltInDirectivesIfMissing() {
1386-
addDirectiveIfMissing(Directives.IncludeDirective);
1387-
addDirectiveIfMissing(Directives.SkipDirective);
1388-
addDirectiveIfMissing(Directives.DeprecatedDirective);
1389-
addDirectiveIfMissing(Directives.SpecifiedByDirective);
1390-
addDirectiveIfMissing(Directives.OneOfDirective);
1391-
addDirectiveIfMissing(Directives.DeferDirective);
1381+
Directives.getDefaultDirectives().forEach(this::addDirectiveIfMissing);
1382+
Directives.getMandatoryDirectives().forEach(this::addDirectiveIfMissing);
13921383
}
13931384

13941385
private void addDirectiveIfMissing(GraphQLDirective directive) {

0 commit comments

Comments
 (0)