Skip to content

Commit dd5d4d8

Browse files
andimarekandimarek
authored
Merge pull request #2018 from graphql-java/deprecated-directive-is-checked-for-location-correctedness
Now checking for @deprecated location correctness
2 parents 9ba7606 + d2b8500 commit dd5d4d8

8 files changed

Lines changed: 78 additions & 120 deletions

File tree

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

Lines changed: 53 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
package graphql;
22

33

4+
import graphql.language.Description;
5+
import graphql.language.DirectiveDefinition;
6+
import graphql.language.InputValueDefinition;
7+
import graphql.language.StringValue;
48
import graphql.schema.GraphQLDirective;
59

610
import static graphql.Scalars.GraphQLBoolean;
@@ -11,6 +15,10 @@
1115
import static graphql.introspection.Introspection.DirectiveLocation.FRAGMENT_SPREAD;
1216
import static graphql.introspection.Introspection.DirectiveLocation.INLINE_FRAGMENT;
1317
import static graphql.introspection.Introspection.DirectiveLocation.SCALAR;
18+
import static graphql.language.DirectiveLocation.newDirectiveLocation;
19+
import static graphql.language.InputValueDefinition.*;
20+
import static graphql.language.NonNullType.newNonNullType;
21+
import static graphql.language.TypeName.newTypeName;
1422
import static graphql.schema.GraphQLArgument.newArgument;
1523
import static graphql.schema.GraphQLNonNull.nonNull;
1624

@@ -20,6 +28,41 @@
2028
@PublicApi
2129
public class Directives {
2230

31+
private static final String SPECIFIED_BY = "specifiedBy";
32+
private static final String DEPRECATED = "deprecated";
33+
34+
public static final String NO_LONGER_SUPPORTED = "No longer supported";
35+
public static final DirectiveDefinition DEPRECATED_DIRECTIVE_DEFINITION;
36+
public static final DirectiveDefinition SPECIFIED_BY_DIRECTIVE_DEFINITION;
37+
38+
39+
static {
40+
DEPRECATED_DIRECTIVE_DEFINITION = DirectiveDefinition.newDirectiveDefinition()
41+
.name(DEPRECATED)
42+
.directiveLocation(newDirectiveLocation().name(FIELD_DEFINITION.name()).build())
43+
.directiveLocation(newDirectiveLocation().name(ENUM_VALUE.name()).build())
44+
.description(createDescription("Marks the field or enum value as deprecated"))
45+
.inputValueDefinition(
46+
newInputValueDefinition()
47+
.name("reason")
48+
.description(createDescription("The reason for the deprecation"))
49+
.type(newTypeName().name("String").build())
50+
.defaultValue(StringValue.newStringValue().value(NO_LONGER_SUPPORTED).build())
51+
.build())
52+
.build();
53+
54+
SPECIFIED_BY_DIRECTIVE_DEFINITION = DirectiveDefinition.newDirectiveDefinition()
55+
.name(SPECIFIED_BY)
56+
.directiveLocation(newDirectiveLocation().name(SCALAR.name()).build())
57+
.description(createDescription("Exposes a URL that specifies the behaviour of this scalar."))
58+
.inputValueDefinition(
59+
newInputValueDefinition()
60+
.name("url")
61+
.description(createDescription("The URL that specifies the behaviour of this scalar."))
62+
.type(newNonNullType(newTypeName().name("String").build()).build())
63+
.build())
64+
.build();
65+
}
2366
public static final GraphQLDirective IncludeDirective = GraphQLDirective.newDirective()
2467
.name("include")
2568
.description("Directs the executor to include this field or fragment only when the `if` argument is true")
@@ -43,31 +86,37 @@ public class Directives {
4386

4487
/**
4588
* The "deprecated" directive is special and is always available in a graphql schema
46-
*
89+
* <p>
4790
* See https://graphql.github.io/graphql-spec/June2018/#sec--deprecated
4891
*/
4992
public static final GraphQLDirective DeprecatedDirective = GraphQLDirective.newDirective()
50-
.name("deprecated")
93+
.name(DEPRECATED)
5194
.description("Marks the field or enum value as deprecated")
5295
.argument(newArgument()
5396
.name("reason")
5497
.type(GraphQLString)
55-
.defaultValue("No longer supported")
98+
.defaultValue(NO_LONGER_SUPPORTED)
5699
.description("The reason for the deprecation"))
57100
.validLocations(FIELD_DEFINITION, ENUM_VALUE)
101+
.definition(DEPRECATED_DIRECTIVE_DEFINITION)
58102
.build();
59103

60104
/**
61105
* The "specifiedBy" directive allows to provide a specification URL for a Scalar
62106
*/
63107
public static final GraphQLDirective SpecifiedByDirective = GraphQLDirective.newDirective()
64-
.name("specifiedBy")
108+
.name(SPECIFIED_BY)
65109
.description("Exposes a URL that specifies the behaviour of this scalar.")
66110
.argument(newArgument()
67111
.name("url")
68112
.type(nonNull(GraphQLString))
69113
.description("The URL that specifies the behaviour of this scalar."))
70114
.validLocations(SCALAR)
115+
.definition(SPECIFIED_BY_DIRECTIVE_DEFINITION)
71116
.build();
72117

118+
private static Description createDescription(String s) {
119+
return new Description(s, null, false);
120+
}
121+
73122
}

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -724,7 +724,7 @@ GraphQLDirective[] buildDirectives(BuildContext buildCtx,
724724
for (Directive directive : directives) {
725725
if (!names.contains(directive.getName())) {
726726
names.add(directive.getName());
727-
output.add(buildDirective(buildCtx,directive, directiveDefinitions, directiveLocation, comparatorRegistry));
727+
output.add(buildDirective(buildCtx, directive, directiveDefinitions, directiveLocation, comparatorRegistry));
728728
}
729729
}
730730
for (Directive directive : extensionDirectives) {
@@ -931,7 +931,6 @@ GraphQLUnionType buildUnionType(BuildContext buildCtx, UnionTypeDefinition typeD
931931
*
932932
* @param buildCtx the context we need to work out what we are doing
933933
* @param rawType the type to be built
934-
*
935934
* @return an output type
936935
*/
937936
@SuppressWarnings({"unchecked", "TypeParameterUnusedInFormals"})
@@ -1160,7 +1159,6 @@ List<UnionTypeExtensionDefinition> unionTypeExtensions(UnionTypeDefinition typeD
11601159
* but then we build the rest of the types specified and put them in as additional types
11611160
*
11621161
* @param buildCtx the context we need to work out what we are doing
1163-
*
11641162
* @return the additional types not referenced from the top level operations
11651163
*/
11661164
Set<GraphQLType> buildAdditionalTypes(BuildContext buildCtx) {

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

Lines changed: 9 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,11 @@
1515
import graphql.language.Node;
1616
import graphql.language.ObjectTypeDefinition;
1717
import graphql.language.ObjectTypeExtensionDefinition;
18-
import graphql.language.StringValue;
1918
import graphql.language.Type;
2019
import graphql.language.TypeDefinition;
2120
import graphql.language.TypeName;
2221
import graphql.language.UnionTypeDefinition;
2322
import graphql.schema.idl.errors.DirectiveIllegalLocationError;
24-
import graphql.schema.idl.errors.InvalidDeprecationDirectiveError;
2523
import graphql.schema.idl.errors.MissingInterfaceTypeError;
2624
import graphql.schema.idl.errors.MissingScalarImplementationError;
2725
import graphql.schema.idl.errors.MissingTypeError;
@@ -43,8 +41,8 @@
4341
import java.util.function.Consumer;
4442
import java.util.function.Function;
4543
import java.util.function.Predicate;
46-
import java.util.function.Supplier;
4744

45+
import static graphql.introspection.Introspection.DirectiveLocation.INPUT_FIELD_DEFINITION;
4846
import static java.util.stream.Collectors.toList;
4947

5048
/**
@@ -198,7 +196,7 @@ private void checkFieldsAreSensible(List<GraphQLError> errors, TypeDefinitionReg
198196

199197
// input types
200198
List<InputObjectTypeDefinition> inputTypes = filterTo(typesMap, InputObjectTypeDefinition.class);
201-
inputTypes.forEach(inputType -> checkInputValues(errors, inputType, inputType.getInputValueDefinitions()));
199+
inputTypes.forEach(inputType -> checkInputValues(errors, inputType, inputType.getInputValueDefinitions(), INPUT_FIELD_DEFINITION));
202200
}
203201

204202

@@ -216,8 +214,6 @@ private void checkObjTypeFields(List<GraphQLError> errors, ObjectTypeDefinition
216214
(directiveName, directive) -> new NonUniqueDirectiveError(typeDefinition, fld, directiveName)));
217215

218216
fieldDefinitions.forEach(fld -> fld.getDirectives().forEach(directive -> {
219-
checkDeprecatedDirective(errors, directive,
220-
() -> new InvalidDeprecationDirectiveError(typeDefinition, fld));
221217

222218
checkNamedUniqueness(errors, directive.getArguments(), Argument::getName,
223219
(argumentName, argument) -> new NonUniqueArgumentError(typeDefinition, fld, argumentName));
@@ -238,14 +234,9 @@ private void checkInterfaceFields(List<GraphQLError> errors, InterfaceTypeDefini
238234
fieldDefinitions.forEach(fld -> checkNamedUniqueness(errors, fld.getDirectives(), Directive::getName,
239235
(directiveName, directive) -> new NonUniqueDirectiveError(interfaceType, fld, directiveName)));
240236

241-
fieldDefinitions.forEach(fld -> fld.getDirectives().forEach(directive -> {
242-
checkDeprecatedDirective(errors, directive,
243-
() -> new InvalidDeprecationDirectiveError(interfaceType, fld));
244-
245-
checkNamedUniqueness(errors, directive.getArguments(), Argument::getName,
246-
(argumentName, argument) -> new NonUniqueArgumentError(interfaceType, fld, argumentName));
247-
248-
}));
237+
fieldDefinitions.forEach(fld -> fld.getDirectives().forEach(directive ->
238+
checkNamedUniqueness(errors, directive.getArguments(), Argument::getName,
239+
(argumentName, argument) -> new NonUniqueArgumentError(interfaceType, fld, argumentName))));
249240
}
250241

251242
private void checkEnumValues(List<GraphQLError> errors, EnumTypeDefinition enumType, List<EnumValueDefinition> enumValueDefinitions) {
@@ -262,16 +253,14 @@ private void checkEnumValues(List<GraphQLError> errors, EnumTypeDefinition enumT
262253
});
263254

264255
enumValueDefinitions.forEach(enumValue -> enumValue.getDirectives().forEach(directive -> {
265-
checkDeprecatedDirective(errors, directive,
266-
() -> new InvalidDeprecationDirectiveError(enumType, enumValue));
267256

268257
BiFunction<String, Argument, NonUniqueArgumentError> errorFunction = (argumentName, argument) -> new NonUniqueArgumentError(enumType, enumValue, argumentName);
269258
checkNamedUniqueness(errors, directive.getArguments(), Argument::getName, errorFunction);
270259

271260
}));
272261
}
273262

274-
private void checkInputValues(List<GraphQLError> errors, InputObjectTypeDefinition inputType, List<InputValueDefinition> inputValueDefinitions) {
263+
private void checkInputValues(List<GraphQLError> errors, InputObjectTypeDefinition inputType, List<InputValueDefinition> inputValueDefinitions, Introspection.DirectiveLocation directiveLocation) {
275264

276265
// field unique ness
277266
checkNamedUniqueness(errors, inputValueDefinitions, InputValueDefinition::getName,
@@ -287,40 +276,9 @@ private void checkInputValues(List<GraphQLError> errors, InputObjectTypeDefiniti
287276
inputValueDefinitions.forEach(inputValueDef -> checkNamedUniqueness(errors, inputValueDef.getDirectives(), Directive::getName,
288277
(directiveName, directive) -> new NonUniqueDirectiveError(inputType, inputValueDef, directiveName)));
289278

290-
inputValueDefinitions.forEach(inputValueDef -> inputValueDef.getDirectives().forEach(directive -> {
291-
checkDeprecatedDirective(errors, directive,
292-
() -> new InvalidDeprecationDirectiveError(inputType, inputValueDef));
293-
294-
checkNamedUniqueness(errors, directive.getArguments(), Argument::getName,
295-
(argumentName, argument) -> new NonUniqueArgumentError(inputType, inputValueDef, argumentName));
296-
}));
297-
}
298-
299-
300-
/**
301-
* A special check for the magic @deprecated directive
302-
*
303-
* @param errors the list of errors
304-
* @param directive the directive to check
305-
* @param errorSupplier the error supplier function
306-
*/
307-
static void checkDeprecatedDirective(List<GraphQLError> errors, Directive directive, Supplier<InvalidDeprecationDirectiveError> errorSupplier) {
308-
if ("deprecated".equals(directive.getName())) {
309-
// it can have zero args
310-
List<Argument> arguments = directive.getArguments();
311-
if (arguments.size() == 0) {
312-
return;
313-
}
314-
// but if has more than it must have 1 called "reason" of type StringValue
315-
if (arguments.size() == 1) {
316-
Argument arg = arguments.get(0);
317-
if ("reason".equals(arg.getName()) && arg.getValue() instanceof StringValue) {
318-
return;
319-
}
320-
}
321-
// not valid
322-
errors.add(errorSupplier.get());
323-
}
279+
inputValueDefinitions.forEach(inputValueDef -> inputValueDef.getDirectives().forEach(directive ->
280+
checkNamedUniqueness(errors, directive.getArguments(), Argument::getName,
281+
(argumentName, argument) -> new NonUniqueArgumentError(inputType, inputValueDef, argumentName))));
324282
}
325283

326284
/**

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

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -137,10 +137,6 @@ private void checkFieldsDirectives(List<GraphQLError> errors, TypeDefinitionRegi
137137

138138
private void checkDirectives(DirectiveLocation expectedLocation, List<GraphQLError> errors, TypeDefinitionRegistry typeRegistry, Node<?> element, String elementName, List<Directive> directives) {
139139
directives.forEach(directive -> {
140-
// we have special code for the deprecation directive in SchemaTypeChecker.checkDeprecatedDirective
141-
if (Directives.DeprecatedDirective.getName().equals(directive.getName())) {
142-
return;
143-
}
144140
Optional<DirectiveDefinition> directiveDefinition = typeRegistry.getDirectiveDefinition(directive.getName());
145141
if (!directiveDefinition.isPresent()) {
146142
errors.add(new DirectiveUndeclaredError(element, elementName, directive.getName()));

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

Lines changed: 9 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
import graphql.language.TypeDefinition;
1717
import graphql.language.TypeName;
1818
import graphql.language.UnionTypeDefinition;
19-
import graphql.schema.idl.errors.InvalidDeprecationDirectiveError;
2019
import graphql.schema.idl.errors.MissingTypeError;
2120
import graphql.schema.idl.errors.NonUniqueArgumentError;
2221
import graphql.schema.idl.errors.NonUniqueDirectiveError;
@@ -33,7 +32,6 @@
3332
import java.util.function.Consumer;
3433
import java.util.stream.Collectors;
3534

36-
import static graphql.schema.idl.SchemaTypeChecker.checkDeprecatedDirective;
3735
import static graphql.schema.idl.SchemaTypeChecker.checkNamedUniqueness;
3836
import static graphql.util.FpKit.mergeFirst;
3937

@@ -84,14 +82,9 @@ private void checkObjectTypeExtensions(List<GraphQLError> errors, TypeDefinition
8482
extension.getFieldDefinitions().forEach(fld -> checkNamedUniqueness(errors, fld.getDirectives(), Directive::getName,
8583
(directiveName, directive) -> new NonUniqueDirectiveError(extension, fld, directiveName)));
8684

87-
fieldDefinitions.forEach(fld -> fld.getDirectives().forEach(directive -> {
88-
checkDeprecatedDirective(errors, directive,
89-
() -> new InvalidDeprecationDirectiveError(extension, fld));
90-
91-
checkNamedUniqueness(errors, directive.getArguments(), Argument::getName,
92-
(argumentName, argument) -> new NonUniqueArgumentError(extension, fld, argumentName));
93-
94-
}));
85+
fieldDefinitions.forEach(fld -> fld.getDirectives().forEach(directive ->
86+
checkNamedUniqueness(errors, directive.getArguments(), Argument::getName,
87+
(argumentName, argument) -> new NonUniqueArgumentError(extension, fld, argumentName))));
9588

9689
//
9790
// fields must be unique within a type extension
@@ -138,14 +131,9 @@ private void checkInterfaceTypeExtensions(List<GraphQLError> errors, TypeDefinit
138131
extension.getFieldDefinitions().forEach(fld -> checkNamedUniqueness(errors, fld.getDirectives(), Directive::getName,
139132
(directiveName, directive) -> new NonUniqueDirectiveError(extension, fld, directiveName)));
140133

141-
fieldDefinitions.forEach(fld -> fld.getDirectives().forEach(directive -> {
142-
checkDeprecatedDirective(errors, directive,
143-
() -> new InvalidDeprecationDirectiveError(extension, fld));
144-
145-
checkNamedUniqueness(errors, directive.getArguments(), Argument::getName,
146-
(argumentName, argument) -> new NonUniqueArgumentError(extension, fld, argumentName));
147-
148-
}));
134+
fieldDefinitions.forEach(fld -> fld.getDirectives().forEach(directive ->
135+
checkNamedUniqueness(errors, directive.getArguments(), Argument::getName,
136+
(argumentName, argument) -> new NonUniqueArgumentError(extension, fld, argumentName))));
149137

150138
//
151139
// fields must be unique within a type extension
@@ -270,14 +258,9 @@ private void checkInputObjectTypeExtensions(List<GraphQLError> errors, TypeDefin
270258
inputValueDefinitions.forEach(fld -> checkNamedUniqueness(errors, fld.getDirectives(), Directive::getName,
271259
(directiveName, directive) -> new NonUniqueDirectiveError(extension, fld, directiveName)));
272260

273-
inputValueDefinitions.forEach(fld -> fld.getDirectives().forEach(directive -> {
274-
checkDeprecatedDirective(errors, directive,
275-
() -> new InvalidDeprecationDirectiveError(extension, fld));
276-
277-
checkNamedUniqueness(errors, directive.getArguments(), Argument::getName,
278-
(argumentName, argument) -> new NonUniqueArgumentError(extension, fld, argumentName));
279-
280-
}));
261+
inputValueDefinitions.forEach(fld -> fld.getDirectives().forEach(directive ->
262+
checkNamedUniqueness(errors, directive.getArguments(), Argument::getName,
263+
(argumentName, argument) -> new NonUniqueArgumentError(extension, fld, argumentName))));
281264
//
282265
// fields must be unique within a type extension
283266
forEachBut(extension, extensions,

‎src/main/java/graphql/schema/idl/errors/InvalidDeprecationDirectiveError.java‎

Lines changed: 0 additions & 29 deletions
This file was deleted.

0 commit comments

Comments
 (0)