Skip to content

Commit 573a61d

Browse files
bbakermanbbakerman
committed
Now checking for @deprecated location correctness and moved around the implicit types
1 parent 9f3cab8 commit 573a61d

File tree

5 files changed

+65
-61
lines changed

5 files changed

+65
-61
lines changed

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

Lines changed: 47 additions & 2 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,9 @@
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.NonNullType.newNonNullType;
20+
import static graphql.language.TypeName.newTypeName;
1421
import static graphql.schema.GraphQLArgument.newArgument;
1522
import static graphql.schema.GraphQLNonNull.nonNull;
1623

@@ -20,6 +27,38 @@
2027
@PublicApi
2128
public class Directives {
2229

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

4483
/**
4584
* The "deprecated" directive is special and is always available in a graphql schema
46-
*
85+
* <p>
4786
* See https://graphql.github.io/graphql-spec/June2018/#sec--deprecated
4887
*/
4988
public static final GraphQLDirective DeprecatedDirective = GraphQLDirective.newDirective()
@@ -52,9 +91,10 @@ public class Directives {
5291
.argument(newArgument()
5392
.name("reason")
5493
.type(GraphQLString)
55-
.defaultValue("No longer supported")
94+
.defaultValue(NO_LONGER_SUPPORTED)
5695
.description("The reason for the deprecation"))
5796
.validLocations(FIELD_DEFINITION, ENUM_VALUE)
97+
.definition(DEPRECATED_DIRECTIVE_DEFINITION)
5898
.build();
5999

60100
/**
@@ -68,6 +108,11 @@ public class Directives {
68108
.type(nonNull(GraphQLString))
69109
.description("The URL that specifies the behaviour of this scalar."))
70110
.validLocations(SCALAR)
111+
.definition(SPECIFIED_BY_DIRECTIVE_DEFINITION)
71112
.build();
72113

114+
private static Description createDescription(String s) {
115+
return new Description(s, null, false);
116+
}
117+
73118
}

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

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

3-
import graphql.Directives;
43
import graphql.Internal;
54
import graphql.introspection.Introspection.DirectiveLocation;
65
import graphql.language.Argument;
@@ -22,7 +21,6 @@
2221
import graphql.schema.GraphQLEnumType;
2322
import graphql.schema.GraphQLInputObjectType;
2423
import graphql.schema.GraphQLInputType;
25-
import graphql.schema.GraphQLList;
2624
import graphql.schema.GraphQLScalarType;
2725
import graphql.schema.GraphQLType;
2826
import graphql.schema.GraphQLTypeUtil;
@@ -38,19 +36,15 @@
3836
import java.util.function.Function;
3937

4038
import static graphql.Assert.assertShouldNeverHappen;
41-
import static graphql.introspection.Introspection.DirectiveLocation.ENUM_VALUE;
42-
import static graphql.introspection.Introspection.DirectiveLocation.FIELD_DEFINITION;
43-
import static graphql.introspection.Introspection.DirectiveLocation.SCALAR;
39+
import static graphql.Directives.DEPRECATED_DIRECTIVE_DEFINITION;
40+
import static graphql.Directives.NO_LONGER_SUPPORTED;
41+
import static graphql.Directives.SPECIFIED_BY_DIRECTIVE_DEFINITION;
4442
import static graphql.introspection.Introspection.DirectiveLocation.valueOf;
45-
import static graphql.language.DirectiveLocation.newDirectiveLocation;
46-
import static graphql.language.NonNullType.newNonNullType;
47-
import static graphql.language.TypeName.newTypeName;
4843
import static graphql.schema.GraphQLTypeUtil.isList;
4944
import static graphql.schema.GraphQLTypeUtil.simplePrint;
5045
import static graphql.schema.GraphQLTypeUtil.unwrapOne;
5146
import static java.util.Collections.emptyList;
5247
import static java.util.stream.Collectors.joining;
53-
import static java.util.stream.Collectors.reducing;
5448
import static java.util.stream.Collectors.toList;
5549
import static java.util.stream.Collectors.toMap;
5650

@@ -60,42 +54,6 @@
6054
@Internal
6155
public class SchemaGeneratorHelper {
6256

63-
static final String NO_LONGER_SUPPORTED = "No longer supported";
64-
static final DirectiveDefinition DEPRECATED_DIRECTIVE_DEFINITION;
65-
static final DirectiveDefinition SPECIFIED_BY_DIRECTIVE_DEFINITION;
66-
67-
static {
68-
DEPRECATED_DIRECTIVE_DEFINITION = DirectiveDefinition.newDirectiveDefinition()
69-
.name(Directives.DeprecatedDirective.getName())
70-
.directiveLocation(newDirectiveLocation().name(FIELD_DEFINITION.name()).build())
71-
.directiveLocation(newDirectiveLocation().name(ENUM_VALUE.name()).build())
72-
.description(createDescription("Marks the field or enum value as deprecated"))
73-
.inputValueDefinition(
74-
InputValueDefinition.newInputValueDefinition()
75-
.name("reason")
76-
.description(createDescription("The reason for the deprecation"))
77-
.type(newTypeName().name("String").build())
78-
.defaultValue(StringValue.newStringValue().value(NO_LONGER_SUPPORTED).build())
79-
.build())
80-
.build();
81-
82-
SPECIFIED_BY_DIRECTIVE_DEFINITION = DirectiveDefinition.newDirectiveDefinition()
83-
.name(Directives.SpecifiedByDirective.getName())
84-
.directiveLocation(newDirectiveLocation().name(SCALAR.name()).build())
85-
.description(createDescription("Exposes a URL that specifies the behaviour of this scalar."))
86-
.inputValueDefinition(
87-
InputValueDefinition.newInputValueDefinition()
88-
.name("url")
89-
.description(createDescription("The URL that specifies the behaviour of this scalar."))
90-
.type(newNonNullType(newTypeName().name("String").build()).build())
91-
.build())
92-
.build();
93-
}
94-
95-
private static Description createDescription(String s) {
96-
return new Description(s, null, false);
97-
}
98-
9957
public Object buildValue(Value value, GraphQLType requiredType) {
10058
Object result = null;
10159
if (GraphQLTypeUtil.isNonNull(requiredType)) {

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

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,6 @@
4848
import static graphql.introspection.Introspection.DirectiveLocation.ENUM_VALUE;
4949
import static graphql.introspection.Introspection.DirectiveLocation.FIELD_DEFINITION;
5050
import static graphql.introspection.Introspection.DirectiveLocation.INPUT_FIELD_DEFINITION;
51-
import static graphql.introspection.Introspection.DirectiveLocation.valueOf;
5251
import static java.util.stream.Collectors.toList;
5352

5453
/**
@@ -317,23 +316,26 @@ private void checkInputValues(List<GraphQLError> errors, InputObjectTypeDefiniti
317316
*/
318317
static void checkDeprecatedDirective(List<GraphQLError> errors, Directive directive, Introspection.DirectiveLocation actualLocation, Supplier<InvalidDeprecationDirectiveError> errorSupplier) {
319318
if ("deprecated".equals(directive.getName())) {
319+
boolean ok = false;
320320
// it can have zero args
321321
List<Argument> arguments = directive.getArguments();
322322
if (arguments.size() == 0) {
323-
return;
323+
ok = true;
324324
}
325325
// but if has more than it must have 1 called "reason" of type StringValue
326-
if (arguments.size() == 1) {
326+
else if (arguments.size() == 1) {
327327
Argument arg = arguments.get(0);
328-
if ("reason".equals(arg.getName()) && arg.getValue() instanceof StringValue) {
329-
return;
328+
if (("reason".equals(arg.getName()) && arg.getValue() instanceof StringValue)) {
329+
ok = true;
330330
}
331331
}
332-
if (DEPRECATED_ALLOWED_LOCATIONS.contains(actualLocation)) {
333-
return;
332+
if (ok && !DEPRECATED_ALLOWED_LOCATIONS.contains(actualLocation)) {
333+
ok = false;
334334
}
335335
// not valid
336-
errors.add(errorSupplier.get());
336+
if (!ok) {
337+
errors.add(errorSupplier.get());
338+
}
337339
}
338340
}
339341

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

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

127127
private void checkDirectives(DirectiveLocation expectedLocation, List<GraphQLError> errors, TypeDefinitionRegistry typeRegistry, Node<?> element, String elementName, List<Directive> directives) {
128128
directives.forEach(directive -> {
129-
// we have special code for the deprecation directive in SchemaTypeChecker.checkDeprecatedDirective
130-
if (Directives.DeprecatedDirective.getName().equals(directive.getName())) {
131-
return;
132-
}
133129
Optional<DirectiveDefinition> directiveDefinition = typeRegistry.getDirectiveDefinition(directive.getName());
134130
if (!directiveDefinition.isPresent()) {
135131
errors.add(new DirectiveUndeclaredError(element, elementName, directive.getName()));

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -945,19 +945,22 @@ class SchemaTypeCheckerTest extends Specification {
945945
}
946946
947947
enum EnumType {
948-
949948
enumA @deprecated(badName : "must be called reason"),
950949
enumB @deprecated(reason : "it must have", one : "argument value")
951950
}
951+
952952
# deprecation is no allowed on input field definitions and args atm, see: https://github.com/graphql-java/graphql-java/issues/1770
953+
input InputType {
954+
inputField : String @deprecated
955+
}
953956
"""
954957

955958
def result = check(spec)
956959

957960
expect:
958961

959962
!result.isEmpty()
960-
result.size() == 6
963+
result.size() == 7
961964
}
962965

963966
def "test that directives are valid"() {

0 commit comments

Comments
 (0)