From bed8c8ddd2d9033f6541863ab60703606a906076 Mon Sep 17 00:00:00 2001 From: Oryan Date: Mon, 19 Oct 2026 21:01:51 -0400 Subject: [PATCH] Build scalars in SchemaParser so they get their directives Scalars were rebuilt in SchemaClassScanner from the provided GraphQLScalarType with only its name, description and coercing, so directives from the SDL, @specifiedBy, the provided specifiedByUrl and onScalar wiring were all lost. Co-Authored-By: Claude --- .../kickstart/tools/SchemaClassScanner.kt | 22 +-- .../graphql/kickstart/tools/SchemaParser.kt | 68 ++++++++- .../tools/directive/DirectiveWiringHelper.kt | 4 + .../graphql/kickstart/tools/DirectiveTest.kt | 143 +++++++++++++++++- .../kickstart/tools/TypeExtensionTest.kt | 16 +- 5 files changed, 223 insertions(+), 30 deletions(-) diff --git a/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt b/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt index 83196e79..fa2d1a77 100644 --- a/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt +++ b/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt @@ -7,7 +7,6 @@ import graphql.kickstart.tools.resolver.MethodFieldResolver import graphql.kickstart.tools.resolver.PropertyFieldResolver import graphql.kickstart.tools.util.* import graphql.language.* -import graphql.schema.GraphQLScalarType import graphql.schema.idl.ScalarInfo import graphql.schema.idl.TypeUtil import org.apache.commons.lang3.ClassUtils @@ -282,24 +281,15 @@ internal class SchemaClassScanner( } catch (t: Throwable) { throw SchemaClassScannerError("Error creating bimap of type => class", t) } - val scalarDefinitions = observedDefinitions.filterIsInstance() - - // Ensure all scalar definitions have implementations and add the definition to those. - val scalars = scalarDefinitions + // SchemaParser builds the scalars, this only checks that each one was provided + val scalars = observedDefinitions.filterIsInstance() .filter { // Filter for any defined scalars OR scalars that aren't defined but also aren't standard scalars.containsKey(it.name) || !ScalarInfo.GRAPHQL_SPECIFICATION_SCALARS_DEFINITIONS.containsKey(it.name) - }.map { definition -> - val provided = scalars[definition.name] - ?: throw SchemaClassScannerError("Expected a user-defined GraphQL scalar type with name '${definition.name}' but found none!") - GraphQLScalarType.newScalar() - .name(provided.name) - .description(getDocumentation(definition, options) ?: provided.description) - .coercing(provided.coercing) - .definition(definition) - .extensionDefinitions(typeExtensions.extensionsOf(definition.name)) - .build() - }.associateBy { it.name } + }.associate { definition -> + definition.name to (scalars[definition.name] + ?: throw SchemaClassScannerError("Expected a user-defined GraphQL scalar type with name '${definition.name}' but found none!")) + } val unusedDefinitions = (definitionsByName.values - observedDefinitions).toSet() unusedDefinitions diff --git a/src/main/kotlin/graphql/kickstart/tools/SchemaParser.kt b/src/main/kotlin/graphql/kickstart/tools/SchemaParser.kt index 002eb3cc..545775e6 100644 --- a/src/main/kotlin/graphql/kickstart/tools/SchemaParser.kt +++ b/src/main/kotlin/graphql/kickstart/tools/SchemaParser.kt @@ -11,6 +11,7 @@ import graphql.introspection.Introspection.DirectiveLocation.INPUT_FIELD_DEFINIT import graphql.introspection.Introspection.DirectiveLocation.INPUT_OBJECT import graphql.introspection.Introspection.DirectiveLocation.INTERFACE import graphql.introspection.Introspection.DirectiveLocation.OBJECT +import graphql.introspection.Introspection.DirectiveLocation.SCALAR import graphql.introspection.Introspection.DirectiveLocation.SCHEMA import graphql.introspection.Introspection.DirectiveLocation.UNION import graphql.kickstart.tools.directive.DirectiveWiringHelper @@ -60,6 +61,7 @@ class SchemaParser internal constructor( private val directiveDefinitions = definitions.filterIsInstance() private val unionDefinitions = definitions.filterIsInstance() + private val scalarDefinitions = definitions.filterIsInstance().filter { it.name in customScalars }.associateBy { it.name } private val permittedTypesForObject: Set = (objectDefinitions.map { it.name } + enumDefinitions.map { it.name } + @@ -72,6 +74,7 @@ class SchemaParser internal constructor( private val directiveWiringHelper = DirectiveWiringHelper(runtimeWiring, codeRegistryBuilder) private lateinit var schemaDirectives: SchemaDirectives + private lateinit var scalars: MutableMap /** * Parses the given schema with respect to the given dictionary and returns GraphQL objects. @@ -88,6 +91,7 @@ class SchemaParser internal constructor( // Create GraphQL objects val inputObjects: MutableList = mutableListOf() + scalars = mutableMapOf() createDirectives(inputObjects) inputObjectDefinitions.forEach { if (inputObjects.none { io -> io.name == it.name }) { @@ -262,6 +266,44 @@ class SchemaParser internal constructor( return directiveWiringHelper.wireEnum(builder.build()) } + /** + * The scalar with this name, or null if there's none. A custom scalar is built the first time it's used, so that everything + * using it holds the same instance. The directives it applies are built first, so their definitions don't keep a type + * reference to it. The directives applied to scalars can still hold one to a scalar that's being built, which graphql-java + * replaces when it builds the schema. + */ + private fun scalarType(name: String?): GraphQLScalarType? { + val definition = scalarDefinitions[name] ?: return GRAPHQL_SCALARS[name] + scalars[definition.name]?.let { return it } + definition.withExtensions(typeExtensions).flatMap { it.directives }.forEach { schemaDirectives.find(it.name) } + return scalars.getOrPut(definition.name) { createScalar(definition) } + } + + private fun createScalar(definition: ScalarTypeDefinition): GraphQLScalarType { + val name = definition.name + val provided = customScalars.getValue(name) + val extensions = typeExtensions.extensionsOf(name) + val directives = directivesOf(definition, extensions) + val (appliedDirectives, legacyDirectives) = try { + buildAppliedDirectives(directives, SCALAR) to buildDirectives(directives, SCALAR) + } catch (e: SchemaError) { + // TODO throw instead in 15.0.0 (#893), like for the other types + log.warn("Scalar '$name' is built without its directives from the schema: ${e.message}") + emptyArray() to emptyArray() + } + val builder = GraphQLScalarType.newScalar(provided) + .definition(definition) + .extensionDefinitions(extensions) + .description(getDocumentation(definition, options) ?: provided.description) + .specifiedByUrl(getSpecifiedByUrl(directives) ?: provided.specifiedByUrl) + // like graphql-java, only the schema's directives are applied + .clearDirectives() + .withAppliedDirectives(*appliedDirectives) + .withDirectives(*legacyDirectives) + + return directiveWiringHelper.wireScalar(builder.build()) + } + private fun createInterfaceObject(interfaceDefinition: InterfaceTypeDefinition, inputObjects: List): GraphQLInterfaceType { val extensions = typeExtensions.extensionsOf(interfaceDefinition.name) val directives = directivesOf(interfaceDefinition, extensions) @@ -391,13 +433,13 @@ class SchemaParser internal constructor( /** * The directive definitions. The input objects used by their arguments need the directives applied to them, so each directive - * is first built with type references to input objects, then rebuilt with the input objects the first time it's looked up. - * Only a directive applied within its own argument types gets its first build there. + * is first built with type references to the types it can't build yet, then rebuilt with the built types the first time it's + * looked up. Only a directive applied within its own argument types gets its first build there. */ private inner class SchemaDirectives(private val inputObjects: MutableList) { // by name, in the order they're declared private val byName = directiveDefinitions.associateTo(linkedMapOf()) { definition -> - definition.name to createDirective(definition) { determineType(GraphQLInputType::class, it, permittedTypesForInputObject, inputObjects) as GraphQLInputType } + definition.name to createDirective(definition, ::typeReferenceOf) } private val unfinished = directiveDefinitions.associateByTo(mutableMapOf()) { it.name } // shared, so that an input object that's still being built isn't built again for another directive @@ -597,6 +639,20 @@ class SchemaParser internal constructor( private fun missingArgumentsWithDefault(directive: Directive, graphQLDirective: GraphQLDirective): List = graphQLDirective.arguments.filter { it.hasSetDefaultValue() && directive.getArgument(it.name) == null } + private fun getSpecifiedByUrl(directives: List): String? = + (directives.find { it.name == Directives.SpecifiedByDirective.name }?.getArgument("url")?.value as? StringValue)?.value + + // a directive's first build, see SchemaDirectives: no directive exists yet, so only scalars that apply none can be built + private fun typeReferenceOf(type: Type<*>): GraphQLInputType = when (type) { + is ListType -> GraphQLList(typeReferenceOf(type.type)) + is NonNullType -> GraphQLNonNull(typeReferenceOf(type.type)) + is TypeName -> { + val appliesDirectives = scalarDefinitions[type.name]?.withExtensions(typeExtensions)?.any { it.directives.isNotEmpty() } == true + (if (appliesDirectives) null else scalarType(type.name)) ?: GraphQLTypeReference(type.name) + } + else -> throw SchemaError("Unknown type: $type") + } + private fun determineOutputType(typeDefinition: Type<*>, inputObjects: List) = determineType(GraphQLOutputType::class, typeDefinition, permittedTypesForObject, inputObjects) as GraphQLOutputType @@ -614,8 +670,7 @@ class SchemaParser internal constructor( createInputObject(typeDefinition, inputObjects, mutableSetOf()) } is TypeName -> { - val scalarType = customScalars[typeDefinition.name] - ?: GRAPHQL_SCALARS[typeDefinition.name] + val scalarType = scalarType(typeDefinition.name) if (scalarType != null) { scalarType } else { @@ -645,8 +700,7 @@ class SchemaParser internal constructor( createInputObject(typeDefinition, inputObjects, referencingInputObjects) } is TypeName -> { - val scalarType = customScalars[typeDefinition.name] - ?: GRAPHQL_SCALARS[typeDefinition.name] + val scalarType = scalarType(typeDefinition.name) if (scalarType != null) { scalarType } else { diff --git a/src/main/kotlin/graphql/kickstart/tools/directive/DirectiveWiringHelper.kt b/src/main/kotlin/graphql/kickstart/tools/directive/DirectiveWiringHelper.kt index 4ccd45d2..8e84b56a 100644 --- a/src/main/kotlin/graphql/kickstart/tools/directive/DirectiveWiringHelper.kt +++ b/src/main/kotlin/graphql/kickstart/tools/directive/DirectiveWiringHelper.kt @@ -44,6 +44,10 @@ internal class DirectiveWiringHelper( return wireDirectives(WiringWrapper(unionType, UNION, SchemaDirectiveWiring::onUnion)) } + fun wireScalar(scalarType: GraphQLScalarType): GraphQLScalarType { + return wireDirectives(WiringWrapper(scalarType, SCALAR, SchemaDirectiveWiring::onScalar)) + } + private fun wireFields(fieldsContainer: GraphQLFieldsContainer): List { return fieldsContainer.fields.map { field -> // wire arguments diff --git a/src/test/kotlin/graphql/kickstart/tools/DirectiveTest.kt b/src/test/kotlin/graphql/kickstart/tools/DirectiveTest.kt index a3367c0d..06d3d973 100644 --- a/src/test/kotlin/graphql/kickstart/tools/DirectiveTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/DirectiveTest.kt @@ -1,6 +1,7 @@ package graphql.kickstart.tools import graphql.GraphQL +import graphql.Scalars import graphql.execution.AsyncExecutionStrategy import graphql.relay.Connection import graphql.relay.SimpleListConnection @@ -522,7 +523,7 @@ class DirectiveTest { @Test fun `should wire input objects only once when a directive is applied within its own argument types`() { - val limit = "directive @limit(bound: BoundInput!) on INPUT_FIELD_DEFINITION" + val limit = "directive @limit(bound: BoundInput!, since: Date) on INPUT_FIELD_DEFINITION" val limits = "directive @limits(bounds: [BoundInput!]) on FIELD_DEFINITION" listOf("$limits\n$limit", "$limit\n$limits").forEach { directives -> val wiredFields = mutableListOf() @@ -536,16 +537,19 @@ class DirectiveTest { } input BoundInput { - max: Int @limit(bound: { max: 10 }) + max: Int @limit(bound: { max: 10 }, since: "2020-01-01") } + + scalar Date """) .resolvers(object : GraphQLQueryResolver { fun name(): String? = null }) .dictionary(BoundInput::class) + .scalars(GraphQLScalarType.newScalar().name("Date").coercing(Scalars.GraphQLString.coercing).build()) .directive("limit", object : SchemaDirectiveWiring { override fun onInputObjectField(environment: SchemaDirectiveWiringEnvironment): GraphQLInputObjectField { - wiredFields.add(environment.element.name) + wiredFields.add("${environment.element.name} since ${environment.appliedDirective.getArgument("since")?.getValue()}") return environment.element } }) @@ -553,12 +557,137 @@ class DirectiveTest { .makeExecutableSchema() .assertEveryFieldResolves() - assert(wiredFields == listOf("max")) { "$wiredFields with directives declared as $directives" } + assert(wiredFields == listOf("max since 2020-01-01")) { "$wiredFields with directives declared as $directives" } val limit = (schema.getType("BoundInput") as GraphQLInputObjectType).getField("max").getAppliedDirective("limit") assertEquals(limit.getArgument("bound")?.getValue>(), mapOf("max" to 10)) } } + @Test + fun `should apply the schema's directives and the provided specifiedBy URL to scalars`() { + val wired = mutableListOf() + val schema = SchemaParser.newParser() + .schemaString( + """ + directive @unit(name: String!) on SCALAR + + "A calendar date" + scalar Date @specifiedBy(url: "https://example.com/date") @unit(name: "day") + scalar Time + + type Query { + today: Date + now: Time + } + """) + .resolvers(object : GraphQLQueryResolver { + fun today(): String = "2026-10-19" + fun now(): String = "12:00" + }) + .scalars( + GraphQLScalarType.newScalar().name("Date").description("A date").coercing(Scalars.GraphQLString.coercing) + .specifiedByUrl("https://provided.example.com/date").build(), + GraphQLScalarType.newScalar().name("Time").description("A time").coercing(Scalars.GraphQLString.coercing) + .specifiedByUrl("https://example.com/time").build() + ) + .directive("unit", object : SchemaDirectiveWiring { + override fun onScalar(environment: SchemaDirectiveWiringEnvironment): GraphQLScalarType { + wired.add("${environment.element.name} ${environment.appliedDirective.getArgument("name")?.getValue()}") + return environment.element + } + }) + .build() + .makeExecutableSchema() + .assertEveryFieldResolves() + + val data = assertNoGraphQlErrors(GraphQL.newGraphQL(schema).build()) { + """ + query { + __schema { types { name description specifiedByURL } } + } + """ + } + @Suppress("UNCHECKED_CAST") + val types = (data["__schema"] as Map>>).getValue("types") + assertEquals(types.filter { it["name"] in listOf("Date", "Time") }, listOf( + mapOf("name" to "Date", "description" to "A calendar date", "specifiedByURL" to "https://example.com/date"), + mapOf("name" to "Time", "description" to "A time", "specifiedByURL" to "https://example.com/time") + )) + assertEquals(wired, listOf("Date day")) + val date = schema.getType("Date") as GraphQLScalarType + assertEquals(date.appliedDirectives.map { it.name }, listOf("specifiedBy", "unit")) + val printed = SchemaPrinter().print(schema) + assert(printed.contains("""@specifiedBy(url : "https://example.com/date")""")) { printed } + } + + @Test + fun `should build scalars used by the arguments of their own directives`() { + val until = "directive @until(date: Date) on FIELD_DEFINITION" + val since = "directive @since(date: Date) on SCALAR | FIELD_DEFINITION | INPUT_FIELD_DEFINITION" + // building @until builds Date, which needs @since, whichever is declared first + listOf("$until\n$since", "$since\n$until").forEach { directives -> + val recorder = ArgumentTypeRecorder() + val schema = SchemaParser.newParser() + .schemaString( + """ + $directives + + scalar Date @since(date: "2020-01-01") + + input Range { + from: Date @since(date: "2021-01-01") + } + + type Query { + name(after: Date, range: Range): String @since(date: "2022-01-01") @until(date: "2030-01-01") + } + """) + .resolvers(object : GraphQLQueryResolver { + fun name(after: String?, range: Range?): String? = null + }) + .dictionary(Range::class) + .scalars(GraphQLScalarType.newScalar().name("Date").coercing(Scalars.GraphQLString.coercing).build()) + .directive("since", recorder) + .directive("until", recorder) + .build() + .makeExecutableSchema() + .assertEveryFieldResolves() + + // only Date's own @since can't hold Date yet, graphql-java replaces the reference when it builds the schema + val expectedTypes = mapOf("name" to listOf("GraphQLScalarType"), "from" to listOf("GraphQLScalarType"), "Date" to listOf("GraphQLTypeReference")) + assert(recorder.argumentTypes == expectedTypes) { "${recorder.argumentTypes} with directives declared as $directives" } + val since = (schema.getType("Date") as GraphQLScalarType).getAppliedDirective("since") + assertEquals(since.getArgument("date")?.getValue(), "2020-01-01") + } + } + + @Test + fun `should warn about undeclared directives on scalars and build them without the schema's directives`() { + val (schema, warnings) = withWarnings { + SchemaParser.newParser() + .schemaString( + """ + scalar Date @specifiedBy(url: "https://example.com/date") @format(pattern: "yyyy-MM-dd") + + type Query { + today: Date + } + """) + .resolvers(object : GraphQLQueryResolver { + fun today(): String = "2026-10-19" + }) + .scalars(GraphQLScalarType.newScalar().name("Date").coercing(Scalars.GraphQLString.coercing).build()) + .build() + .makeExecutableSchema() + .assertEveryFieldResolves() + } + + assertEquals(warnings, listOf("Scalar 'Date' is built without its directives from the schema: Found applied directive format without corresponding directive definition.")) + val date = schema.getType("Date") as GraphQLScalarType + assertEquals(date.appliedDirectives, emptyList()) + assertEquals(date.specifiedByUrl, "https://example.com/date") + } + @Test fun `should apply directives on the schema and its extensions`() { val schema = SchemaParser.newParser() @@ -793,6 +922,10 @@ class DirectiveTest { val max: Int? ) + data class Range( + val from: String? + ) + /** * Records the unwrapped types of the arguments of each applied directive, by the name of the element it's applied to. */ @@ -805,6 +938,8 @@ class DirectiveTest { override fun onInputObjectField(environment: SchemaDirectiveWiringEnvironment) = record(environment) + override fun onScalar(environment: SchemaDirectiveWiringEnvironment) = record(environment) + private fun record(environment: SchemaDirectiveWiringEnvironment): T { argumentTypes[environment.element.name] = environment.appliedDirective.arguments.map { GraphQLTypeUtil.unwrapAllAs(it.type).javaClass.simpleName } diff --git a/src/test/kotlin/graphql/kickstart/tools/TypeExtensionTest.kt b/src/test/kotlin/graphql/kickstart/tools/TypeExtensionTest.kt index e98dc9a7..9012e67f 100644 --- a/src/test/kotlin/graphql/kickstart/tools/TypeExtensionTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/TypeExtensionTest.kt @@ -66,6 +66,7 @@ class TypeExtensionTest { assertEquals(item.interfaces.map { it.name }, listOf("Node", "Entity")) assertEquals(schema.getImplementations(schema.getType("Entity") as GraphQLInterfaceType)?.map { it.name }, listOf("Item")) assertEquals(date.description, "A date") + assertEquals(date.appliedDirectives.map { it.name }, listOf("format")) // through GraphQLNamedSchemaElement, since graphql-java 25 and 26 disagree on which types have a nullable definition assertEquals(listOf(fooBar, color, node, item, date).map { it.definition?.javaClass }, listOf( @@ -217,7 +218,7 @@ class TypeExtensionTest { val (schema, warnings) = withWarnings { parser( """ - directive @tag(name: String!) repeatable on OBJECT | INTERFACE | UNION | ENUM | INPUT_OBJECT + directive @tag(name: String!) repeatable on OBJECT | INTERFACE | UNION | ENUM | INPUT_OBJECT | SCALAR directive @once(name: String!) on OBJECT | INPUT_OBJECT type Query @once(name: "Query") { @@ -225,6 +226,7 @@ class TypeExtensionTest { fooBar: FooBar color: Color search(filter: Filter): String + today: Date } extend type Query @once(name: "Query extension") @@ -244,9 +246,13 @@ class TypeExtensionTest { input Filter @tag(name: "Filter") { id: ID } extend input Filter @tag(name: "Filter extension") @once(name: "Filter extension") + scalar Date @tag(name: "Date") + extend scalar Date @tag(name: "Date extension") + type Baz { baz: String } """) { dictionary(Item::class, Baz::class) + scalars(GraphQLScalarType.newScalar().name("Date").coercing(Scalars.GraphQLString.coercing).build()) directive("tag", wiring) directive("once", wiring) }.makeExecutableSchema() @@ -255,7 +261,7 @@ class TypeExtensionTest { assertEquals(warnings, listOf("Directive @once isn't repeatable but an extension of type 'Query' applies it again, only the first one is kept.")) - val applied = listOf("Query", "Node", "Item", "FooBar", "Color", "Filter").associateWith { name -> + val applied = listOf("Query", "Node", "Item", "FooBar", "Color", "Filter", "Date").associateWith { name -> (schema.getType(name) as GraphQLDirectiveContainer).appliedDirectives.map { it.describe() } } assertEquals(applied, mapOf( @@ -264,7 +270,8 @@ class TypeExtensionTest { "Item" to listOf("@tag(Item)", "@tag(Item extension)"), "FooBar" to listOf("@tag(FooBar)", "@tag(FooBar extension)"), "Color" to listOf("@tag(Color)", "@tag(Color extension)"), - "Filter" to listOf("@tag(Filter)", "@tag(Filter extension)", "@once(Filter extension)") + "Filter" to listOf("@tag(Filter)", "@tag(Filter extension)", "@once(Filter extension)"), + "Date" to listOf("@tag(Date)", "@tag(Date extension)") )) assertEquals(wiring.wired.sorted(), applied.values.flatten().sorted()) @@ -273,6 +280,7 @@ class TypeExtensionTest { assertEquals(item.directives.map { it.name }, listOf("tag", "tag")) assertEquals(filter.directives.map { it.name }, listOf("tag", "tag", "once")) assertEquals(schema.queryType.directives.map { it.name }, listOf("once")) + assertEquals((schema.getType("Date") as GraphQLScalarType).directives.map { it.name }, listOf("tag", "tag")) assertEquals(item.fieldDefinitions.map { it.name }, listOf("id", "name")) assertEquals(filter.extensionDefinitions.size, 1) @@ -511,6 +519,8 @@ class TypeExtensionTest { override fun onInputObjectType(environment: SchemaDirectiveWiringEnvironment) = record(environment) + override fun onScalar(environment: SchemaDirectiveWiringEnvironment) = record(environment) + private fun record(environment: SchemaDirectiveWiringEnvironment): T { wired.add(environment.appliedDirective.describe()) return environment.element