Skip to content

Commit

Permalink
require non-empty directive locations
Browse files Browse the repository at this point in the history
  • Loading branch information
@jbellenger
jbellenger committed Mar 31, 2024
1 parent 41c9ba2 commit 897d8d4
Show file tree
Hide file tree
Showing 12 changed files with 104 additions and 99 deletions.
4 changes: 2 additions & 2 deletions src/main/java/graphql/schema/GraphQLDirective.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,7 @@
import java.util.function.Consumer;
import java.util.function.UnaryOperator;

import static graphql.Assert.assertNotNull;
import static graphql.Assert.assertValidName;
import static graphql.Assert.*;
import static graphql.introspection.Introspection.DirectiveLocation;
import static graphql.util.FpKit.getByName;

Expand Down Expand Up @@ -52,6 +51,7 @@ private GraphQLDirective(String name,
DirectiveDefinition definition) {
assertValidName(name);
assertNotNull(arguments, () -> "arguments can't be null");
assertNotEmpty(locations, () -> "locations can't be empty");
this.name = name;
this.description = description;
this.repeatable = repeatable;
Expand Down
41 changes: 13 additions & 28 deletions src/test/groovy/graphql/TestUtil.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,32 +2,11 @@ package graphql

import graphql.execution.MergedField
import graphql.execution.MergedSelectionSet
import graphql.language.Document
import graphql.language.Field
import graphql.language.NullValue
import graphql.language.ObjectTypeDefinition
import graphql.language.OperationDefinition
import graphql.language.ScalarTypeDefinition
import graphql.language.Type
import graphql.introspection.Introspection.DirectiveLocation
import graphql.language.*
import graphql.parser.Parser
import graphql.schema.Coercing
import graphql.schema.DataFetcher
import graphql.schema.GraphQLAppliedDirectiveArgument
import graphql.schema.GraphQLAppliedDirective
import graphql.schema.GraphQLArgument
import graphql.schema.GraphQLDirective
import graphql.schema.GraphQLInputType
import graphql.schema.GraphQLObjectType
import graphql.schema.GraphQLScalarType
import graphql.schema.GraphQLSchema
import graphql.schema.GraphQLType
import graphql.schema.TypeResolver
import graphql.schema.idl.RuntimeWiring
import graphql.schema.idl.SchemaGenerator
import graphql.schema.idl.SchemaParser
import graphql.schema.idl.TestMockedWiringFactory
import graphql.schema.idl.TypeRuntimeWiring
import graphql.schema.idl.WiringFactory
import graphql.schema.*
import graphql.schema.idl.*
import graphql.schema.idl.errors.SchemaProblem
import groovy.json.JsonOutput

Expand Down Expand Up @@ -194,13 +173,19 @@ class TestUtil {
.name(definition.getName())
.description(definition.getDescription() == null ? null : definition.getDescription().getContent())
.coercing(mockCoercing())
.replaceDirectives(definition.getDirectives().stream().map({ mockDirective(it.getName()) }).collect(Collectors.toList()))
.replaceDirectives(
definition.getDirectives()
.stream()
.map({ mockDirective(it.getName(), DirectiveLocation.SCALAR) })
.collect(Collectors.toList()))
.definition(definition)
.build()
}

static GraphQLDirective mockDirective(String name) {
newDirective().name(name).description(name).build()
static GraphQLDirective mockDirective(String name, DirectiveLocation location, GraphQLArgument arg = null) {
def b = newDirective().name(name).description(name).validLocation(location)
if (arg != null) b.argument(arg)
b.build()
}

static TypeRuntimeWiring mockTypeRuntimeWiring(String typeName, boolean withResolver) {
Expand Down
23 changes: 12 additions & 11 deletions src/test/groovy/graphql/schema/GraphQLArgumentTest.groovy
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package graphql.schema

import graphql.collect.ImmutableKit
import static graphql.introspection.Introspection.DirectiveLocation.ARGUMENT_DEFINITION
import graphql.language.FloatValue
import graphql.schema.validation.InvalidSchemaException
import spock.lang.Specification
Expand All @@ -9,10 +10,10 @@ import static graphql.Scalars.GraphQLFloat
import static graphql.Scalars.GraphQLInt
import static graphql.Scalars.GraphQLString
import static graphql.schema.GraphQLArgument.newArgument
import static graphql.schema.GraphQLDirective.newDirective
import static graphql.schema.GraphQLFieldDefinition.newFieldDefinition
import static graphql.schema.GraphQLObjectType.newObject
import static graphql.schema.GraphQLSchema.newSchema
import static graphql.TestUtil.mockDirective

class GraphQLArgumentTest extends Specification {

Expand All @@ -22,15 +23,15 @@ class GraphQLArgumentTest extends Specification {
.description("A1_description")
.type(GraphQLInt)
.deprecate("custom reason")
.withDirective(newDirective().name("directive1"))
.withDirective(mockDirective("directive1", ARGUMENT_DEFINITION))
.build()
when:
def transformedArgument = startingArgument.transform({
it
.name("A2")
.description("A2_description")
.type(GraphQLString)
.withDirective(newDirective().name("directive3"))
.withDirective(mockDirective("directive3", ARGUMENT_DEFINITION))
.value("VALUE") // Retain deprecated for test coverage
.deprecate(null)
.defaultValue("DEFAULT") // Retain deprecated for test coverage
Expand Down Expand Up @@ -79,9 +80,9 @@ class GraphQLArgumentTest extends Specification {
def argument

given:
def builder = GraphQLArgument.newArgument().name("A1")
def builder = newArgument().name("A1")
.type(GraphQLInt)
.withDirective(newDirective().name("directive1"))
.withDirective(mockDirective("directive1", ARGUMENT_DEFINITION))

when:
argument = builder.build()
Expand All @@ -96,8 +97,8 @@ class GraphQLArgumentTest extends Specification {
when:
argument = builder
.clearDirectives()
.withDirective(newDirective().name("directive2"))
.withDirective(newDirective().name("directive3"))
.withDirective(mockDirective("directive2", ARGUMENT_DEFINITION))
.withDirective(mockDirective("directive3", ARGUMENT_DEFINITION))
.build()

then:
Expand All @@ -109,9 +110,9 @@ class GraphQLArgumentTest extends Specification {
when:
argument = builder
.replaceDirectives([
newDirective().name("directive1").build(),
newDirective().name("directive2").build(),
newDirective().name("directive3").build()]) // overwrite
mockDirective("directive1", ARGUMENT_DEFINITION),
mockDirective("directive2", ARGUMENT_DEFINITION),
mockDirective("directive3", ARGUMENT_DEFINITION)]) // overwrite
.build()

then:
Expand Down Expand Up @@ -198,7 +199,7 @@ class GraphQLArgumentTest extends Specification {
def "Applied schema directives arguments are validated for programmatic schemas"() {
given:
def arg = newArgument().name("arg").type(GraphQLInt).valueProgrammatic(ImmutableKit.emptyMap()).build() // Retain for test coverage
def directive = GraphQLDirective.newDirective().name("cached").argument(arg).build()
def directive = mockDirective("cached", ARGUMENT_DEFINITION, arg)
def field = newFieldDefinition()
.name("hello")
.type(GraphQLString)
Expand Down
31 changes: 31 additions & 0 deletions src/test/groovy/graphql/schema/GraphQLDirectiveTest.groovy
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package graphql.schema

import graphql.AssertException
import graphql.TestUtil
import graphql.introspection.Introspection
import graphql.language.Node
import spock.lang.Specification

Expand Down Expand Up @@ -168,9 +170,38 @@ class GraphQLDirectiveTest extends Specification {

then:
assertDirectiveContainer(scalarType)
}

def "throws an error on missing required properties"() {
given:
def validDirective = GraphQLDirective.newDirective()
.name("dir")
.validLocation(Introspection.DirectiveLocation.SCALAR)
.build()

when:
validDirective.transform { it.name(null) }

then:
def e = thrown(AssertException)
e.message.contains("Name must be non-null, non-empty")

when:
validDirective.transform { it.replaceArguments(null) }

then:
def e2 = thrown(AssertException)
e2.message.contains("arguments must not be null")

when:
validDirective.transform { it.clearValidLocations() }

then:
def e3 = thrown(AssertException)
e3.message.contains("locations can't be empty")
}


static boolean assertDirectiveContainer(GraphQLDirectiveContainer container) {
assert container.hasDirective("d1") // Retain for test coverage
assert container.hasAppliedDirective("d1")
Expand Down
8 changes: 4 additions & 4 deletions src/test/groovy/graphql/schema/GraphQLEnumValueDefinitionTest.groovy
View file
Original file line number Diff line number Diff line change
@@ -1,25 +1,25 @@
package graphql.schema

import static graphql.introspection.Introspection.DirectiveLocation
import spock.lang.Specification

import static graphql.schema.GraphQLDirective.newDirective
import static graphql.schema.GraphQLEnumValueDefinition.newEnumValueDefinition
import static graphql.TestUtil.mockDirective

class GraphQLEnumValueDefinitionTest extends Specification {
def "object can be transformed"() {
given:
def startEnumValue = newEnumValueDefinition().name("EV1")
.description("EV1_description")
.value("A")
.withDirective(newDirective().name("directive1"))
.withDirective(mockDirective("directive1", DirectiveLocation.ENUM_VALUE))
.build()
when:
def transformedEnumValue = startEnumValue.transform({
it
.name("EV2")
.value("X")
.withDirective(newDirective().name("directive2"))

.withDirective(mockDirective("directive2", DirectiveLocation.ENUM_VALUE))
})

then:
Expand Down
12 changes: 5 additions & 7 deletions src/test/groovy/graphql/schema/GraphQLFieldDefinitionTest.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package graphql.schema

import graphql.AssertException
import graphql.TestUtil
import graphql.introspection.Introspection
import graphql.schema.idl.SchemaPrinter
import spock.lang.Specification

Expand All @@ -10,9 +11,9 @@ import static graphql.Scalars.GraphQLFloat
import static graphql.Scalars.GraphQLInt
import static graphql.Scalars.GraphQLString
import static graphql.TestUtil.mockArguments
import static graphql.TestUtil.mockDirective
import static graphql.schema.DefaultGraphqlTypeComparatorRegistry.newComparators
import static graphql.schema.GraphQLArgument.newArgument
import static graphql.schema.GraphQLDirective.newDirective
import static graphql.schema.GraphQLFieldDefinition.newFieldDefinition
import static graphql.schema.idl.SchemaPrinter.Options.defaultOptions

Expand All @@ -35,8 +36,8 @@ class GraphQLFieldDefinitionTest extends Specification {
.deprecate("F1_deprecated")
.argument(newArgument().name("argStr").type(GraphQLString))
.argument(newArgument().name("argInt").type(GraphQLInt))
.withDirective(newDirective().name("directive1"))
.withDirective(newDirective().name("directive2"))
.withDirective(mockDirective("directive1", Introspection.DirectiveLocation.FIELD_DEFINITION))
.withDirective(mockDirective("directive2", Introspection.DirectiveLocation.FIELD_DEFINITION))
.build()

when:
Expand All @@ -47,13 +48,10 @@ class GraphQLFieldDefinitionTest extends Specification {
.argument(newArgument().name("argStr").type(GraphQLString))
.argument(newArgument().name("argInt").type(GraphQLBoolean))
.argument(newArgument().name("argIntAdded").type(GraphQLInt))
.withDirective(newDirective().name("directive3"))

.withDirective(mockDirective("directive3", Introspection.DirectiveLocation.FIELD_DEFINITION))
})


then:

startingField.name == "F1"
startingField.type == GraphQLFloat
startingField.description == "F1_description"
Expand Down
12 changes: 5 additions & 7 deletions src/test/groovy/graphql/schema/GraphQLInputObjectFieldTest.groovy
View file
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
package graphql.schema

import graphql.introspection.Introspection
import graphql.language.FloatValue
import spock.lang.Specification

import static graphql.Scalars.GraphQLFloat
import static graphql.Scalars.GraphQLInt
import static graphql.schema.GraphQLDirective.newDirective
import static graphql.schema.GraphQLInputObjectField.newInputObjectField
import static graphql.TestUtil.mockDirective

class GraphQLInputObjectFieldTest extends Specification {

Expand All @@ -16,8 +17,8 @@ class GraphQLInputObjectFieldTest extends Specification {
.name("F1")
.type(GraphQLFloat)
.description("F1_description")
.withDirective(newDirective().name("directive1"))
.withDirective(newDirective().name("directive2"))
.withDirective(mockDirective("directive1", Introspection.DirectiveLocation.INPUT_FIELD_DEFINITION))
.withDirective(mockDirective("directive2", Introspection.DirectiveLocation.INPUT_FIELD_DEFINITION))
.deprecate("No longer useful")
.build()

Expand All @@ -26,13 +27,10 @@ class GraphQLInputObjectFieldTest extends Specification {
builder.name("F2")
.type(GraphQLInt)
.deprecate(null)
.withDirective(newDirective().name("directive3"))

.withDirective(mockDirective("directive3", Introspection.DirectiveLocation.INPUT_FIELD_DEFINITION))
})


then:

startingField.name == "F1"
startingField.type == GraphQLFloat
startingField.description == "F1_description"
Expand Down
10 changes: 5 additions & 5 deletions src/test/groovy/graphql/schema/GraphQLScalarTypeTest.groovy
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
package graphql.schema

import graphql.introspection.Introspection
import spock.lang.Specification

import static graphql.schema.GraphQLDirective.newDirective
import static graphql.TestUtil.mockDirective

class GraphQLScalarTypeTest extends Specification {
Coercing<String, String> coercing = new Coercing<String, String>() {
Expand All @@ -28,14 +29,14 @@ class GraphQLScalarTypeTest extends Specification {
.name("S1")
.description("S1_description")
.coercing(coercing)
.withDirective(newDirective().name("directive1"))
.withDirective(newDirective().name("directive2"))
.withDirective(mockDirective("directive1", Introspection.DirectiveLocation.SCALAR))
.withDirective(mockDirective("directive2", Introspection.DirectiveLocation.SCALAR))
.build()
when:
def transformedScalar = startingScalar.transform({ builder ->
builder.name("S2")
.description("S2_description")
.withDirective(newDirective().name("directive3"))
.withDirective(mockDirective("directive3", Introspection.DirectiveLocation.SCALAR))
})

then:
Expand All @@ -55,6 +56,5 @@ class GraphQLScalarTypeTest extends Specification {
transformedScalar.getDirective("directive1") != null
transformedScalar.getDirective("directive2") != null
transformedScalar.getDirective("directive3") != null

}
}

0 comments on commit 897d8d4

Please sign in to comment.