Skip to content

Commit d601754

Browse files
andimarekandimarek
committed
remove the option to have undeclared directives in your SDL
1 parent 578985f commit d601754

File tree

9 files changed

+61
-154
lines changed

9 files changed

+61
-154
lines changed

‎src/main/java/graphql/schema/idl/SchemaGenerator.java

Lines changed: 3 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -95,35 +95,12 @@ public class SchemaGenerator {
9595
* These options control how the schema generation works
9696
*/
9797
public static class Options {
98-
private final boolean enforceSchemaDirectives;
9998

100-
Options(boolean enforceSchemaDirectives) {
101-
this.enforceSchemaDirectives = enforceSchemaDirectives;
102-
}
103-
104-
/**
105-
* This controls whether schema directives MUST be declared using
106-
* directive definition syntax before use.
107-
*
108-
* @return true if directives must be fully declared; the default is true
109-
*/
110-
public boolean isEnforceSchemaDirectives() {
111-
return enforceSchemaDirectives;
99+
Options() {
112100
}
113101

114102
public static Options defaultOptions() {
115-
return new Options(true);
116-
}
117-
118-
/**
119-
* This controls whether schema directives MUST be declared using
120-
* directive definition syntax before use.
121-
*
122-
* @param flag the value to use
123-
* @return the new options
124-
*/
125-
public Options enforceSchemaDirectives(boolean flag) {
126-
return new Options(flag);
103+
return new Options();
127104
}
128105

129106
}
@@ -260,7 +237,7 @@ public GraphQLSchema makeExecutableSchema(Options options, TypeDefinitionRegistr
260237

261238
schemaGeneratorHelper.addDeprecatedDirectiveDefinition(typeRegistryCopy);
262239

263-
List<GraphQLError> errors = typeChecker.checkTypeRegistry(typeRegistryCopy, wiring, options.enforceSchemaDirectives);
240+
List<GraphQLError> errors = typeChecker.checkTypeRegistry(typeRegistryCopy, wiring);
264241
if (!errors.isEmpty()) {
265242
throw new SchemaProblem(errors);
266243
}

‎src/main/java/graphql/schema/idl/SchemaGeneratorHelper.java

Lines changed: 6 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,15 @@
11
package graphql.schema.idl;
22

33
import graphql.Internal;
4-
import graphql.Scalars;
54
import graphql.introspection.Introspection.DirectiveLocation;
65
import graphql.language.Argument;
76
import graphql.language.ArrayValue;
8-
import graphql.language.BooleanValue;
97
import graphql.language.Comment;
108
import graphql.language.Description;
119
import graphql.language.Directive;
1210
import graphql.language.DirectiveDefinition;
1311
import graphql.language.EnumValue;
14-
import graphql.language.FloatValue;
1512
import graphql.language.InputValueDefinition;
16-
import graphql.language.IntValue;
1713
import graphql.language.Node;
1814
import graphql.language.NullValue;
1915
import graphql.language.ObjectValue;
@@ -39,15 +35,12 @@
3935
import java.util.Optional;
4036
import java.util.Set;
4137
import java.util.function.Function;
42-
import java.util.stream.Collectors;
4338

4439
import static graphql.Assert.assertShouldNeverHappen;
45-
import static graphql.Assert.assertTrue;
4640
import static graphql.introspection.Introspection.DirectiveLocation.ENUM_VALUE;
4741
import static graphql.introspection.Introspection.DirectiveLocation.FIELD_DEFINITION;
4842
import static graphql.introspection.Introspection.DirectiveLocation.valueOf;
4943
import static graphql.language.DirectiveLocation.newDirectiveLocation;
50-
import static graphql.schema.GraphQLList.list;
5144
import static graphql.schema.GraphQLTypeUtil.isList;
5245
import static graphql.schema.GraphQLTypeUtil.simplePrint;
5346
import static graphql.schema.GraphQLTypeUtil.unwrapOne;
@@ -173,74 +166,10 @@ public void addDeprecatedDirectiveDefinition(TypeDefinitionRegistry typeRegistry
173166
typeRegistry.add(DEPRECATED_DIRECTIVE_DEFINITION);
174167
}
175168

176-
/**
177-
* We support the basic types as directive types
178-
*
179-
* @param value the value to use
180-
*
181-
* @return a graphql input type
182-
*/
183-
public GraphQLInputType buildDirectiveInputType(Value value) {
184-
if (value instanceof NullValue) {
185-
return Scalars.GraphQLString;
186-
}
187-
if (value instanceof FloatValue) {
188-
return Scalars.GraphQLFloat;
189-
}
190-
if (value instanceof StringValue) {
191-
return Scalars.GraphQLString;
192-
}
193-
if (value instanceof IntValue) {
194-
return Scalars.GraphQLInt;
195-
}
196-
if (value instanceof BooleanValue) {
197-
return Scalars.GraphQLBoolean;
198-
}
199-
if (value instanceof ArrayValue) {
200-
ArrayValue arrayValue = (ArrayValue) value;
201-
return list(buildDirectiveInputType(getArrayValueWrappedType(arrayValue)));
202-
}
203-
return assertShouldNeverHappen("Directive values of type '%s' are not supported yet", value.getClass().getSimpleName());
204-
}
205-
206-
private Value getArrayValueWrappedType(ArrayValue value) {
207-
// empty array [] is equivalent to [null]
208-
if (value.getValues().isEmpty()) {
209-
return NullValue.Null;
210-
}
211-
212-
// get rid of null values
213-
List<Value> nonNullValueList = value.getValues().stream()
214-
.filter(v -> !(v instanceof NullValue))
215-
.collect(toList());
216-
217-
// [null, null, ...] unwrapped is null
218-
if (nonNullValueList.isEmpty()) {
219-
return NullValue.Null;
220-
}
221-
222-
// make sure the array isn't polymorphic
223-
Set<Class<? extends Value>> distinctTypes = nonNullValueList.stream()
224-
.map(Value::getClass)
225-
.distinct()
226-
.collect(Collectors.toSet());
227-
228-
assertTrue(distinctTypes.size() == 1,
229-
"Arrays containing multiple types of values are not supported yet. Detected the following types [%s]",
230-
nonNullValueList.stream()
231-
.map(Value::getClass)
232-
.map(Class::getSimpleName)
233-
.distinct()
234-
.sorted()
235-
.collect(Collectors.joining(",")));
236-
237-
// peek at first value, value exists and is assured to be non-null
238-
return nonNullValueList.get(0);
239-
}
240169

241170
// builds directives from a type and its extensions
242171
public GraphQLDirective buildDirective(Directive directive, Set<GraphQLDirective> directiveDefinitions, DirectiveLocation directiveLocation, GraphqlTypeComparatorRegistry comparatorRegistry) {
243-
Optional<GraphQLDirective> directiveDefinition = directiveDefinitions.stream().filter(dd -> dd.getName().equals(directive.getName())).findFirst();
172+
GraphQLDirective directiveDefinition = FpKit.findOne(directiveDefinitions, dd -> dd.getName().equals(directive.getName())).get();
244173
GraphQLDirective.Builder builder = GraphQLDirective.newDirective()
245174
.name(directive.getName())
246175
.description(buildDescription(directive, null))
@@ -251,26 +180,20 @@ public GraphQLDirective buildDirective(Directive directive, Set<GraphQLDirective
251180
.map(arg -> buildDirectiveArgument(arg, directiveDefinition))
252181
.collect(toList());
253182

254-
if (directiveDefinition.isPresent()) {
255-
arguments = transferMissingArguments(arguments, directiveDefinition.get());
256-
}
183+
arguments = transferMissingArguments(arguments, directiveDefinition);
257184
arguments.forEach(builder::argument);
258185

259186
return builder.build();
260187
}
261188

262-
private GraphQLArgument buildDirectiveArgument(Argument arg, Optional<GraphQLDirective> directiveDefinition) {
263-
Optional<GraphQLArgument> directiveDefArgument = directiveDefinition.map(dd -> dd.getArgument(arg.getName()));
189+
private GraphQLArgument buildDirectiveArgument(Argument arg, GraphQLDirective directiveDefinition) {
190+
GraphQLArgument directiveDefArgument = directiveDefinition.getArgument(arg.getName());
264191
GraphQLArgument.Builder builder = GraphQLArgument.newArgument();
265192
builder.name(arg.getName());
266193
GraphQLInputType inputType;
267194
Object defaultValue = null;
268-
if (directiveDefArgument.isPresent()) {
269-
inputType = directiveDefArgument.get().getType();
270-
defaultValue = directiveDefArgument.get().getDefaultValue();
271-
} else {
272-
inputType = buildDirectiveInputType(arg.getValue());
273-
}
195+
inputType = directiveDefArgument.getType();
196+
defaultValue = directiveDefArgument.getDefaultValue();
274197
builder.type(inputType);
275198
builder.defaultValue(defaultValue);
276199

‎src/main/java/graphql/schema/idl/SchemaTypeChecker.java

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,6 @@
1717
import graphql.language.NonNullType;
1818
import graphql.language.ObjectTypeDefinition;
1919
import graphql.language.ObjectTypeExtensionDefinition;
20-
import graphql.language.OperationTypeDefinition;
21-
import graphql.language.SchemaDefinition;
2220
import graphql.language.StringValue;
2321
import graphql.language.Type;
2422
import graphql.language.TypeDefinition;
@@ -38,8 +36,6 @@
3836
import graphql.schema.idl.errors.NonUniqueArgumentError;
3937
import graphql.schema.idl.errors.NonUniqueDirectiveError;
4038
import graphql.schema.idl.errors.NonUniqueNameError;
41-
import graphql.schema.idl.errors.OperationTypesMustBeObjects;
42-
import graphql.schema.idl.errors.QueryOperationMissingError;
4339
import graphql.schema.idl.errors.SchemaProblem;
4440

4541
import java.util.ArrayList;
@@ -66,7 +62,7 @@
6662
@Internal
6763
public class SchemaTypeChecker {
6864

69-
public List<GraphQLError> checkTypeRegistry(TypeDefinitionRegistry typeRegistry, RuntimeWiring wiring, boolean enforceSchemaDirectives) throws SchemaProblem {
65+
public List<GraphQLError> checkTypeRegistry(TypeDefinitionRegistry typeRegistry, RuntimeWiring wiring) throws SchemaProblem {
7066
List<GraphQLError> errors = new ArrayList<>();
7167
checkForMissingTypes(errors, typeRegistry);
7268

@@ -86,10 +82,8 @@ public List<GraphQLError> checkTypeRegistry(TypeDefinitionRegistry typeRegistry,
8682
//check directive definitions before checking directive usages
8783
checkDirectiveDefinitions(typeRegistry, errors);
8884

89-
if (enforceSchemaDirectives) {
90-
SchemaTypeDirectivesChecker directivesChecker = new SchemaTypeDirectivesChecker(typeRegistry, wiring);
91-
directivesChecker.checkTypeDirectives(errors);
92-
}
85+
SchemaTypeDirectivesChecker directivesChecker = new SchemaTypeDirectivesChecker(typeRegistry, wiring);
86+
directivesChecker.checkTypeDirectives(errors);
9387

9488
return errors;
9589
}

‎src/main/java/graphql/util/FpKit.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,6 @@ private static <T> BinaryOperator<T> throwingMerger() {
5858
}
5959

6060

61-
6261
//
6362
// From a list of named things, get a map of them by name, merging them first one added
6463
public static <T> Map<String, T> getByName(List<T> namedObjects, Function<T, String> nameFn) {
@@ -170,7 +169,7 @@ public static <T> List<T> flatList(List<List<T>> listLists) {
170169
.collect(Collectors.toList());
171170
}
172171

173-
public static <T> Optional<T> findOne(List<T> list, Predicate<T> filter) {
172+
public static <T> Optional<T> findOne(Collection<T> list, Predicate<T> filter) {
174173
return list
175174
.stream()
176175
.filter(filter)

‎src/test/groovy/graphql/TestUtil.groovy

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ class TestUtil {
6969
def stream = TestUtil.class.getClassLoader().getResourceAsStream(fileName)
7070

7171
def typeRegistry = new SchemaParser().parse(new InputStreamReader(stream))
72-
def options = SchemaGenerator.Options.defaultOptions().enforceSchemaDirectives(false)
72+
def options = SchemaGenerator.Options.defaultOptions()
7373
def schema = new SchemaGenerator().makeExecutableSchema(options, typeRegistry, wiring)
7474
schema
7575
}
@@ -106,7 +106,7 @@ class TestUtil {
106106
static GraphQLSchema schema(Reader specReader, RuntimeWiring runtimeWiring) {
107107
try {
108108
def registry = new SchemaParser().parse(specReader)
109-
def options = SchemaGenerator.Options.defaultOptions().enforceSchemaDirectives(false)
109+
def options = SchemaGenerator.Options.defaultOptions()
110110
return new SchemaGenerator().makeExecutableSchema(options, registry, runtimeWiring)
111111
} catch (SchemaProblem e) {
112112
assert false: "The schema could not be compiled : ${e}"

‎src/test/groovy/graphql/schema/idl/SchemaGeneratorDirectiveHelperTest.groovy

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,16 @@ class SchemaGeneratorDirectiveHelperTest extends Specification {
6060
def "will trace down into each directive callback"() {
6161

6262
def sdl = '''
63+
directive @fieldDirective(target: String) on FIELD_DEFINITION
64+
directive @argumentDirective(target: String) on ARGUMENT_DEFINITION
65+
directive @objectDirective(target: String) on OBJECT
66+
directive @interfaceDirective(target: String) on INTERFACE
67+
directive @unionDirective(target: String) on UNION
68+
directive @enumDirective(target: String) on ENUM
69+
directive @enumValueDirective(target: String) on ENUM_VALUE
70+
directive @inputDirective(target: String) on INPUT_OBJECT
71+
directive @inputFieldDirective(target: String) on INPUT_FIELD_DEFINITION
72+
directive @scalarDirective(target: String) on SCALAR
6373
type Query {
6474
f : ObjectType
6575
s : ScalarType
@@ -275,6 +285,11 @@ class SchemaGeneratorDirectiveHelperTest extends Specification {
275285

276286
def "can modify the existing behaviour"() {
277287
def sdl = '''
288+
directive @uppercase on FIELD_DEFINITION
289+
directive @lowercase on FIELD_DEFINITION
290+
directive @mixedcase on FIELD_DEFINITION
291+
directive @echoFieldName on FIELD_DEFINITION
292+
directive @reverse on FIELD_DEFINITION
278293
type Query {
279294
lowerCaseValue : String @uppercase
280295
upperCaseValue : String @lowercase
@@ -384,6 +399,7 @@ class SchemaGeneratorDirectiveHelperTest extends Specification {
384399
def "ensure the readme examples work"() {
385400

386401
def sdl = '''
402+
directive @dateFormat on FIELD_DEFINITION
387403
type Query {
388404
dateField : String @dateFormat
389405
}
@@ -421,6 +437,7 @@ class SchemaGeneratorDirectiveHelperTest extends Specification {
421437

422438
def "can state-fully track wrapped elements"() {
423439
def sdl = '''
440+
directive @secret on FIELD_DEFINITION | OBJECT
424441
type Query {
425442
secret : Secret
426443
nonSecret : NonSecret
@@ -541,6 +558,10 @@ class SchemaGeneratorDirectiveHelperTest extends Specification {
541558

542559
def "ordering of directive wiring is locked in place"() {
543560
def sdl = '''
561+
directive @generalDirective on FIELD_DEFINITION
562+
directive @factoryDirective on FIELD_DEFINITION
563+
directive @namedDirective1 on FIELD_DEFINITION
564+
directive @namedDirective2 on FIELD_DEFINITION
544565
type Query {
545566
field : String @generalDirective @factoryDirective @namedDirective1 @namedDirective2
546567
}
@@ -634,6 +655,10 @@ class SchemaGeneratorDirectiveHelperTest extends Specification {
634655

635656
def "all directives are available to all callbacks"() {
636657
def sdl = '''
658+
directive @generalDirective on FIELD_DEFINITION
659+
directive @factoryDirective on FIELD_DEFINITION
660+
directive @namedDirective1 on FIELD_DEFINITION
661+
directive @namedDirective2 on FIELD_DEFINITION
637662
type Query {
638663
field : String @generalDirective @factoryDirective @namedDirective1 @namedDirective2
639664
}
@@ -721,6 +746,10 @@ class SchemaGeneratorDirectiveHelperTest extends Specification {
721746

722747
def "parent and child element directives can be accessed"() {
723748
def sdl = '''
749+
directive @argDirective1 on ARGUMENT_DEFINITION
750+
directive @argDirective2 on ARGUMENT_DEFINITION
751+
directive @argDirective3 on ARGUMENT_DEFINITION
752+
directive @fieldDirective on FIELD_DEFINITION
724753
type Query {
725754
field(arg1 : String @argDirective1 @argDirective2, arg2 : String @argDirective3) : String @fieldDirective
726755
}
@@ -784,6 +813,10 @@ class SchemaGeneratorDirectiveHelperTest extends Specification {
784813

785814
def "data fetchers can be changed in argument and object callbacks"() {
786815
def sdl = '''
816+
directive @argDirective1 on ARGUMENT_DEFINITION
817+
directive @argDirective2 on ARGUMENT_DEFINITION
818+
directive @argDirective3 on ARGUMENT_DEFINITION
819+
directive @fieldDirective on FIELD_DEFINITION
787820
type Query {
788821
field1(arg1 : String @argDirective1 @argDirective2, arg2 : String @argDirective3) : String @fieldDirective
789822
field2 : String

0 commit comments

Comments
 (0)