Skip to content

Commit 897d8d4

Browse files
jbellengerjbellenger
committed
require non-empty directive locations
1 parent 41c9ba2 commit 897d8d4

12 files changed

Lines changed: 104 additions & 99 deletions

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,7 @@
1515
import java.util.function.Consumer;
1616
import java.util.function.UnaryOperator;
1717

18-
import static graphql.Assert.assertNotNull;
19-
import static graphql.Assert.assertValidName;
18+
import static graphql.Assert.*;
2019
import static graphql.introspection.Introspection.DirectiveLocation;
2120
import static graphql.util.FpKit.getByName;
2221

@@ -52,6 +51,7 @@ private GraphQLDirective(String name,
5251
DirectiveDefinition definition) {
5352
assertValidName(name);
5453
assertNotNull(arguments, () -> "arguments can't be null");
54+
assertNotEmpty(locations, () -> "locations can't be empty");
5555
this.name = name;
5656
this.description = description;
5757
this.repeatable = repeatable;

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

Lines changed: 13 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -2,32 +2,11 @@ package graphql
22

33
import graphql.execution.MergedField
44
import graphql.execution.MergedSelectionSet
5-
import graphql.language.Document
6-
import graphql.language.Field
7-
import graphql.language.NullValue
8-
import graphql.language.ObjectTypeDefinition
9-
import graphql.language.OperationDefinition
10-
import graphql.language.ScalarTypeDefinition
11-
import graphql.language.Type
5+
import graphql.introspection.Introspection.DirectiveLocation
6+
import graphql.language.*
127
import graphql.parser.Parser
13-
import graphql.schema.Coercing
14-
import graphql.schema.DataFetcher
15-
import graphql.schema.GraphQLAppliedDirectiveArgument
16-
import graphql.schema.GraphQLAppliedDirective
17-
import graphql.schema.GraphQLArgument
18-
import graphql.schema.GraphQLDirective
19-
import graphql.schema.GraphQLInputType
20-
import graphql.schema.GraphQLObjectType
21-
import graphql.schema.GraphQLScalarType
22-
import graphql.schema.GraphQLSchema
23-
import graphql.schema.GraphQLType
24-
import graphql.schema.TypeResolver
25-
import graphql.schema.idl.RuntimeWiring
26-
import graphql.schema.idl.SchemaGenerator
27-
import graphql.schema.idl.SchemaParser
28-
import graphql.schema.idl.TestMockedWiringFactory
29-
import graphql.schema.idl.TypeRuntimeWiring
30-
import graphql.schema.idl.WiringFactory
8+
import graphql.schema.*
9+
import graphql.schema.idl.*
3110
import graphql.schema.idl.errors.SchemaProblem
3211
import groovy.json.JsonOutput
3312

@@ -194,13 +173,19 @@ class TestUtil {
194173
.name(definition.getName())
195174
.description(definition.getDescription() == null ? null : definition.getDescription().getContent())
196175
.coercing(mockCoercing())
197-
.replaceDirectives(definition.getDirectives().stream().map({ mockDirective(it.getName()) }).collect(Collectors.toList()))
176+
.replaceDirectives(
177+
definition.getDirectives()
178+
.stream()
179+
.map({ mockDirective(it.getName(), DirectiveLocation.SCALAR) })
180+
.collect(Collectors.toList()))
198181
.definition(definition)
199182
.build()
200183
}
201184

202-
static GraphQLDirective mockDirective(String name) {
203-
newDirective().name(name).description(name).build()
185+
static GraphQLDirective mockDirective(String name, DirectiveLocation location, GraphQLArgument arg = null) {
186+
def b = newDirective().name(name).description(name).validLocation(location)
187+
if (arg != null) b.argument(arg)
188+
b.build()
204189
}
205190

206191
static TypeRuntimeWiring mockTypeRuntimeWiring(String typeName, boolean withResolver) {

‎src/test/groovy/graphql/schema/GraphQLArgumentTest.groovy‎

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package graphql.schema
22

33
import graphql.collect.ImmutableKit
4+
import static graphql.introspection.Introspection.DirectiveLocation.ARGUMENT_DEFINITION
45
import graphql.language.FloatValue
56
import graphql.schema.validation.InvalidSchemaException
67
import spock.lang.Specification
@@ -9,10 +10,10 @@ import static graphql.Scalars.GraphQLFloat
910
import static graphql.Scalars.GraphQLInt
1011
import static graphql.Scalars.GraphQLString
1112
import static graphql.schema.GraphQLArgument.newArgument
12-
import static graphql.schema.GraphQLDirective.newDirective
1313
import static graphql.schema.GraphQLFieldDefinition.newFieldDefinition
1414
import static graphql.schema.GraphQLObjectType.newObject
1515
import static graphql.schema.GraphQLSchema.newSchema
16+
import static graphql.TestUtil.mockDirective
1617

1718
class GraphQLArgumentTest extends Specification {
1819

@@ -22,15 +23,15 @@ class GraphQLArgumentTest extends Specification {
2223
.description("A1_description")
2324
.type(GraphQLInt)
2425
.deprecate("custom reason")
25-
.withDirective(newDirective().name("directive1"))
26+
.withDirective(mockDirective("directive1", ARGUMENT_DEFINITION))
2627
.build()
2728
when:
2829
def transformedArgument = startingArgument.transform({
2930
it
3031
.name("A2")
3132
.description("A2_description")
3233
.type(GraphQLString)
33-
.withDirective(newDirective().name("directive3"))
34+
.withDirective(mockDirective("directive3", ARGUMENT_DEFINITION))
3435
.value("VALUE") // Retain deprecated for test coverage
3536
.deprecate(null)
3637
.defaultValue("DEFAULT") // Retain deprecated for test coverage
@@ -79,9 +80,9 @@ class GraphQLArgumentTest extends Specification {
7980
def argument
8081

8182
given:
82-
def builder = GraphQLArgument.newArgument().name("A1")
83+
def builder = newArgument().name("A1")
8384
.type(GraphQLInt)
84-
.withDirective(newDirective().name("directive1"))
85+
.withDirective(mockDirective("directive1", ARGUMENT_DEFINITION))
8586

8687
when:
8788
argument = builder.build()
@@ -96,8 +97,8 @@ class GraphQLArgumentTest extends Specification {
9697
when:
9798
argument = builder
9899
.clearDirectives()
99-
.withDirective(newDirective().name("directive2"))
100-
.withDirective(newDirective().name("directive3"))
100+
.withDirective(mockDirective("directive2", ARGUMENT_DEFINITION))
101+
.withDirective(mockDirective("directive3", ARGUMENT_DEFINITION))
101102
.build()
102103

103104
then:
@@ -109,9 +110,9 @@ class GraphQLArgumentTest extends Specification {
109110
when:
110111
argument = builder
111112
.replaceDirectives([
112-
newDirective().name("directive1").build(),
113-
newDirective().name("directive2").build(),
114-
newDirective().name("directive3").build()]) // overwrite
113+
mockDirective("directive1", ARGUMENT_DEFINITION),
114+
mockDirective("directive2", ARGUMENT_DEFINITION),
115+
mockDirective("directive3", ARGUMENT_DEFINITION)]) // overwrite
115116
.build()
116117

117118
then:
@@ -198,7 +199,7 @@ class GraphQLArgumentTest extends Specification {
198199
def "Applied schema directives arguments are validated for programmatic schemas"() {
199200
given:
200201
def arg = newArgument().name("arg").type(GraphQLInt).valueProgrammatic(ImmutableKit.emptyMap()).build() // Retain for test coverage
201-
def directive = GraphQLDirective.newDirective().name("cached").argument(arg).build()
202+
def directive = mockDirective("cached", ARGUMENT_DEFINITION, arg)
202203
def field = newFieldDefinition()
203204
.name("hello")
204205
.type(GraphQLString)

‎src/test/groovy/graphql/schema/GraphQLDirectiveTest.groovy‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package graphql.schema
22

3+
import graphql.AssertException
34
import graphql.TestUtil
5+
import graphql.introspection.Introspection
46
import graphql.language.Node
57
import spock.lang.Specification
68

@@ -168,9 +170,38 @@ class GraphQLDirectiveTest extends Specification {
168170

169171
then:
170172
assertDirectiveContainer(scalarType)
173+
}
174+
175+
def "throws an error on missing required properties"() {
176+
given:
177+
def validDirective = GraphQLDirective.newDirective()
178+
.name("dir")
179+
.validLocation(Introspection.DirectiveLocation.SCALAR)
180+
.build()
181+
182+
when:
183+
validDirective.transform { it.name(null) }
184+
185+
then:
186+
def e = thrown(AssertException)
187+
e.message.contains("Name must be non-null, non-empty")
171188

189+
when:
190+
validDirective.transform { it.replaceArguments(null) }
191+
192+
then:
193+
def e2 = thrown(AssertException)
194+
e2.message.contains("arguments must not be null")
195+
196+
when:
197+
validDirective.transform { it.clearValidLocations() }
198+
199+
then:
200+
def e3 = thrown(AssertException)
201+
e3.message.contains("locations can't be empty")
172202
}
173203

204+
174205
static boolean assertDirectiveContainer(GraphQLDirectiveContainer container) {
175206
assert container.hasDirective("d1") // Retain for test coverage
176207
assert container.hasAppliedDirective("d1")

‎src/test/groovy/graphql/schema/GraphQLEnumValueDefinitionTest.groovy‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,25 @@
11
package graphql.schema
22

3+
import static graphql.introspection.Introspection.DirectiveLocation
34
import spock.lang.Specification
45

5-
import static graphql.schema.GraphQLDirective.newDirective
66
import static graphql.schema.GraphQLEnumValueDefinition.newEnumValueDefinition
7+
import static graphql.TestUtil.mockDirective
78

89
class GraphQLEnumValueDefinitionTest extends Specification {
910
def "object can be transformed"() {
1011
given:
1112
def startEnumValue = newEnumValueDefinition().name("EV1")
1213
.description("EV1_description")
1314
.value("A")
14-
.withDirective(newDirective().name("directive1"))
15+
.withDirective(mockDirective("directive1", DirectiveLocation.ENUM_VALUE))
1516
.build()
1617
when:
1718
def transformedEnumValue = startEnumValue.transform({
1819
it
1920
.name("EV2")
2021
.value("X")
21-
.withDirective(newDirective().name("directive2"))
22-
22+
.withDirective(mockDirective("directive2", DirectiveLocation.ENUM_VALUE))
2323
})
2424

2525
then:

‎src/test/groovy/graphql/schema/GraphQLFieldDefinitionTest.groovy‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package graphql.schema
22

33
import graphql.AssertException
44
import graphql.TestUtil
5+
import graphql.introspection.Introspection
56
import graphql.schema.idl.SchemaPrinter
67
import spock.lang.Specification
78

@@ -10,9 +11,9 @@ import static graphql.Scalars.GraphQLFloat
1011
import static graphql.Scalars.GraphQLInt
1112
import static graphql.Scalars.GraphQLString
1213
import static graphql.TestUtil.mockArguments
14+
import static graphql.TestUtil.mockDirective
1315
import static graphql.schema.DefaultGraphqlTypeComparatorRegistry.newComparators
1416
import static graphql.schema.GraphQLArgument.newArgument
15-
import static graphql.schema.GraphQLDirective.newDirective
1617
import static graphql.schema.GraphQLFieldDefinition.newFieldDefinition
1718
import static graphql.schema.idl.SchemaPrinter.Options.defaultOptions
1819

@@ -35,8 +36,8 @@ class GraphQLFieldDefinitionTest extends Specification {
3536
.deprecate("F1_deprecated")
3637
.argument(newArgument().name("argStr").type(GraphQLString))
3738
.argument(newArgument().name("argInt").type(GraphQLInt))
38-
.withDirective(newDirective().name("directive1"))
39-
.withDirective(newDirective().name("directive2"))
39+
.withDirective(mockDirective("directive1", Introspection.DirectiveLocation.FIELD_DEFINITION))
40+
.withDirective(mockDirective("directive2", Introspection.DirectiveLocation.FIELD_DEFINITION))
4041
.build()
4142

4243
when:
@@ -47,13 +48,10 @@ class GraphQLFieldDefinitionTest extends Specification {
4748
.argument(newArgument().name("argStr").type(GraphQLString))
4849
.argument(newArgument().name("argInt").type(GraphQLBoolean))
4950
.argument(newArgument().name("argIntAdded").type(GraphQLInt))
50-
.withDirective(newDirective().name("directive3"))
51-
51+
.withDirective(mockDirective("directive3", Introspection.DirectiveLocation.FIELD_DEFINITION))
5252
})
5353

54-
5554
then:
56-
5755
startingField.name == "F1"
5856
startingField.type == GraphQLFloat
5957
startingField.description == "F1_description"

‎src/test/groovy/graphql/schema/GraphQLInputObjectFieldTest.groovy‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
11
package graphql.schema
22

3+
import graphql.introspection.Introspection
34
import graphql.language.FloatValue
45
import spock.lang.Specification
56

67
import static graphql.Scalars.GraphQLFloat
78
import static graphql.Scalars.GraphQLInt
8-
import static graphql.schema.GraphQLDirective.newDirective
99
import static graphql.schema.GraphQLInputObjectField.newInputObjectField
10+
import static graphql.TestUtil.mockDirective
1011

1112
class GraphQLInputObjectFieldTest extends Specification {
1213

@@ -16,8 +17,8 @@ class GraphQLInputObjectFieldTest extends Specification {
1617
.name("F1")
1718
.type(GraphQLFloat)
1819
.description("F1_description")
19-
.withDirective(newDirective().name("directive1"))
20-
.withDirective(newDirective().name("directive2"))
20+
.withDirective(mockDirective("directive1", Introspection.DirectiveLocation.INPUT_FIELD_DEFINITION))
21+
.withDirective(mockDirective("directive2", Introspection.DirectiveLocation.INPUT_FIELD_DEFINITION))
2122
.deprecate("No longer useful")
2223
.build()
2324

@@ -26,13 +27,10 @@ class GraphQLInputObjectFieldTest extends Specification {
2627
builder.name("F2")
2728
.type(GraphQLInt)
2829
.deprecate(null)
29-
.withDirective(newDirective().name("directive3"))
30-
30+
.withDirective(mockDirective("directive3", Introspection.DirectiveLocation.INPUT_FIELD_DEFINITION))
3131
})
3232

33-
3433
then:
35-
3634
startingField.name == "F1"
3735
startingField.type == GraphQLFloat
3836
startingField.description == "F1_description"

‎src/test/groovy/graphql/schema/GraphQLScalarTypeTest.groovy‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
package graphql.schema
22

3+
import graphql.introspection.Introspection
34
import spock.lang.Specification
45

5-
import static graphql.schema.GraphQLDirective.newDirective
6+
import static graphql.TestUtil.mockDirective
67

78
class GraphQLScalarTypeTest extends Specification {
89
Coercing<String, String> coercing = new Coercing<String, String>() {
@@ -28,14 +29,14 @@ class GraphQLScalarTypeTest extends Specification {
2829
.name("S1")
2930
.description("S1_description")
3031
.coercing(coercing)
31-
.withDirective(newDirective().name("directive1"))
32-
.withDirective(newDirective().name("directive2"))
32+
.withDirective(mockDirective("directive1", Introspection.DirectiveLocation.SCALAR))
33+
.withDirective(mockDirective("directive2", Introspection.DirectiveLocation.SCALAR))
3334
.build()
3435
when:
3536
def transformedScalar = startingScalar.transform({ builder ->
3637
builder.name("S2")
3738
.description("S2_description")
38-
.withDirective(newDirective().name("directive3"))
39+
.withDirective(mockDirective("directive3", Introspection.DirectiveLocation.SCALAR))
3940
})
4041

4142
then:
@@ -55,6 +56,5 @@ class GraphQLScalarTypeTest extends Specification {
5556
transformedScalar.getDirective("directive1") != null
5657
transformedScalar.getDirective("directive2") != null
5758
transformedScalar.getDirective("directive3") != null
58-
5959
}
6060
}

0 commit comments

Comments
 (0)