From 674b3984f740bea604aeb54631de2dfe178d4181 Mon Sep 17 00:00:00 2001 From: Oryan Date: Fri, 9 Oct 2026 21:03:23 -0400 Subject: [PATCH 1/3] Resolve nested input objects before directive wiring An input object that another input object only referenced through a list or non-null wrapper, like `subs: [Sub!]` or `other: Sub2!`, was still a GraphQLTypeReference when the directive wirings ran, unless the SDL happened to declare it first. determineInputType handed list and non-null types to determineType, the output path, which never builds input objects and falls back to a type reference. Wirings that walk nested input types, like graphql-java-extended-validation's DirectivesAndTypeWalker, then failed on GraphQLTypeUtil.unwrapAll with a ClassCastException or skipped the nested directives. Those branches now recurse into determineInputType, so the wrapped type gets built on the spot. A field only stays a type reference where it points back at an input object that is still being built, i.e. a cycle. That changes which field of a cycle stays a reference. Types are now built depth-first through the wrappers, so it's the field pointing back. Before, it was the list or non-null field, so in `input A { b: B! } input B { a: A }` the wiring of B.a used to see A and now sees a type reference, while A.b is now resolved. Unlike graphql-java, which wires directives on the finished schema, wirings here still see that reference, so DirectivesAndTypeWalker still fails on recursive or cyclic input types, as it did before. Directive definition arguments go through the same path. Building an input object needs the directive definitions for its own applied directives, and the definitions weren't assigned yet, so a directive argument whose input object carried any directive failed with an UninitializedPropertyAccessException. Nullable arguments already failed this way, and the change above would have made list and non-null arguments fail too. There's now a first set of definitions that only refers to input objects by type reference, and each one is replaced by the real definition the first time it's looked up. So an input object built for a directive argument sees the real definitions of the directives applied to it, and their input object arguments aren't type references at wiring time. Only a directive applied within its own argument types sees its first definition there. Input fields also took their legacy directives from the parent input object's definition instead of their own (a regression from 2a33e6b, which fixed this for arguments only). So inputField.getDirective() and env.directive inside onInputObjectField were null while the applied directive was set, and every input field carried the input object's own directives. The wiring of input fields and enum values now also records INPUT_FIELD_DEFINITION and ENUM_VALUE as their location instead of FIELD_DEFINITION. Fixes #321, fixes #385, fixes #731. Co-Authored-By: Claude Opus 5.5 --- .../graphql/kickstart/tools/SchemaParser.kt | 70 ++-- .../tools/directive/DirectiveWiringHelper.kt | 4 +- .../graphql/kickstart/tools/DirectiveTest.kt | 368 ++++++++++++++++++ .../kickstart/tools/SchemaParserTest.kt | 111 ++++++ 4 files changed, 529 insertions(+), 24 deletions(-) diff --git a/src/main/kotlin/graphql/kickstart/tools/SchemaParser.kt b/src/main/kotlin/graphql/kickstart/tools/SchemaParser.kt index 7417e54c..ad624506 100644 --- a/src/main/kotlin/graphql/kickstart/tools/SchemaParser.kt +++ b/src/main/kotlin/graphql/kickstart/tools/SchemaParser.kt @@ -62,6 +62,8 @@ class SchemaParser internal constructor( private val directiveWiringHelper = DirectiveWiringHelper(runtimeWiring, codeRegistryBuilder) private lateinit var schemaDirectives : Set + // replaces the first definition of a directive with the real one while they're being created, see createDirectives + private var finishDirective: (String) -> Unit = {} /** * Parses the given schema with respect to the given dictionary and returns GraphQL objects. @@ -183,7 +185,7 @@ class SchemaParser internal constructor( .apply { getDeprecated(fieldDefinition.directives)?.let { deprecate(it) } } .type(determineInputType(fieldDefinition.type, inputObjects, referencingInputObjects)) .withAppliedDirectives(*buildAppliedDirectives(fieldDefinition.directives)) - .withDirectives(*buildDirectives(definition.directives, INPUT_FIELD_DEFINITION)) + .withDirectives(*buildDirectives(fieldDefinition.directives, INPUT_FIELD_DEFINITION)) .build() ) } @@ -314,23 +316,24 @@ class SchemaParser internal constructor( } private fun createDirectives(inputObjects: MutableList) { + // Input objects used by directive arguments get built here, and building them needs the definitions of their own applied + // directives. So a first set of definitions only refers to input objects by type reference, and each one is replaced by the + // real definition the first time it's looked up, so that input objects see the real definitions of the directives applied + // to them. Only where a directive is applied within its own argument types, i.e. a cycle, do they see the first definition. schemaDirectives = directiveDefinitions.map { definition -> - val locations = definition.directiveLocations.map { Introspection.DirectiveLocation.valueOf(it.name) }.toTypedArray() - - GraphQLDirective.newDirective() - .name(definition.name) - .description(getDocumentation(definition, options)) - .definition(definition) - .comparatorRegistry(runtimeWiring.comparatorRegistry) - .validLocations(*locations) - .repeatable(definition.isRepeatable) - .apply { - definition.inputValueDefinitions.forEach { argumentDefinition -> - argument(createDirectiveArgument(argumentDefinition, inputObjects)) - } - } - .build() + createDirective(definition) { determineType(GraphQLInputType::class, it, permittedTypesForInputObject, inputObjects) as GraphQLInputType } }.toSet() + val unfinishedDefinitions = directiveDefinitions.associateByTo(mutableMapOf()) { it.name } + // shared by all definitions, so that an input object that's still being built isn't built again for another directive + val referencingInputObjects = mutableSetOf() + finishDirective = { name -> + unfinishedDefinitions.remove(name)?.let { definition -> + val directive = createDirective(definition) { determineInputType(it, inputObjects, referencingInputObjects) } + schemaDirectives = schemaDirectives.map { if (it.name == name) directive else it }.toSet() + } + } + directiveDefinitions.forEach { finishDirective(it.name) } + finishDirective = {} // because the arguments can have directives too, we attach them only after the directives themselves are created schemaDirectives = schemaDirectives.map { d -> val arguments = d.arguments.map { a -> a.transform { @@ -341,12 +344,35 @@ class SchemaParser internal constructor( }.toSet() } - private fun createDirectiveArgument(definition: InputValueDefinition, inputObjects: List): GraphQLArgument { + private fun createDirective(definition: DirectiveDefinition, determineArgumentType: (Type<*>) -> GraphQLInputType): GraphQLDirective { + val locations = definition.directiveLocations.map { Introspection.DirectiveLocation.valueOf(it.name) }.toTypedArray() + + return GraphQLDirective.newDirective() + .name(definition.name) + .description(getDocumentation(definition, options)) + .definition(definition) + .comparatorRegistry(runtimeWiring.comparatorRegistry) + .validLocations(*locations) + .repeatable(definition.isRepeatable) + .apply { + definition.inputValueDefinitions.forEach { argumentDefinition -> + argument(createDirectiveArgument(argumentDefinition, determineArgumentType)) + } + } + .build() + } + + private fun findDirective(name: String): GraphQLDirective? { + finishDirective(name) + return schemaDirectives.find { it.name == name } + } + + private fun createDirectiveArgument(definition: InputValueDefinition, determineArgumentType: (Type<*>) -> GraphQLInputType): GraphQLArgument { return GraphQLArgument.newArgument() .name(definition.name) .definition(definition) .description(getDocumentation(definition, options)) - .type(determineInputType(definition.type, inputObjects, mutableSetOf())) + .type(determineArgumentType(definition.type)) .apply { getDeprecated(definition.directives)?.let { deprecate(it) } } .apply { definition.defaultValue?.let { defaultValueLiteral(it) } } .build() @@ -354,7 +380,7 @@ class SchemaParser internal constructor( private fun buildAppliedDirectives(directives: List): Array { return directives.map { directive -> - val graphQLDirective = schemaDirectives.find { d -> d.name == directive.name } + val graphQLDirective = findDirective(directive.name) ?: BUILT_IN_DIRECTIVES[directive.name] ?: if (options.allowUndeclaredDirectives) return@map buildUndeclaredAppliedDirective(directive) else throw SchemaError("Found applied directive ${directive.name} without corresponding directive definition.") @@ -438,7 +464,7 @@ class SchemaParser internal constructor( val repeatable = directiveDefinitions.find { it.name.equals(directive.name) }?.isRepeatable ?: false if (repeatable || !names.contains(directive.name)) { names.add(directive.name) - val graphQLDirective = this.schemaDirectives.find { d -> d.name == directive.name } + val graphQLDirective = findDirective(directive.name) ?: BUILT_IN_DIRECTIVES[directive.name] // graphql-java rejects legacy directives without a definition, so undeclared ones are only kept as applied directives ?: if (options.allowUndeclaredDirectives) continue @@ -525,8 +551,8 @@ class SchemaParser internal constructor( inputObjects: List, referencingInputObjects: MutableSet): GraphQLInputType = when (typeDefinition) { - is ListType -> GraphQLList(determineType(expectedType, typeDefinition.type, allowedTypeReferences, inputObjects)) - is NonNullType -> GraphQLNonNull(determineType(expectedType, typeDefinition.type, allowedTypeReferences, inputObjects)) + is ListType -> GraphQLList(determineInputType(expectedType, typeDefinition.type, allowedTypeReferences, inputObjects, referencingInputObjects)) + is NonNullType -> GraphQLNonNull(determineInputType(expectedType, typeDefinition.type, allowedTypeReferences, inputObjects, referencingInputObjects)) is InputObjectTypeDefinition -> { log.info("Create input object") createInputObject(typeDefinition, inputObjects, referencingInputObjects as MutableSet) diff --git a/src/main/kotlin/graphql/kickstart/tools/directive/DirectiveWiringHelper.kt b/src/main/kotlin/graphql/kickstart/tools/directive/DirectiveWiringHelper.kt index 9300846c..cb1082eb 100644 --- a/src/main/kotlin/graphql/kickstart/tools/directive/DirectiveWiringHelper.kt +++ b/src/main/kotlin/graphql/kickstart/tools/directive/DirectiveWiringHelper.kt @@ -59,13 +59,13 @@ internal class DirectiveWiringHelper( private fun wireInputFields(fieldsContainer: GraphQLInputFieldsContainer): List { return fieldsContainer.fieldDefinitions.map { field -> - wireDirectives(WiringWrapper(field, FIELD_DEFINITION, SchemaDirectiveWiring::onInputObjectField, inputFieldsContainer = fieldsContainer)) + wireDirectives(WiringWrapper(field, INPUT_FIELD_DEFINITION, SchemaDirectiveWiring::onInputObjectField, inputFieldsContainer = fieldsContainer)) } } private fun wireEnumValues(enumType: GraphQLEnumType): List { return enumType.values.map { value -> - wireDirectives(WiringWrapper(value, FIELD_DEFINITION, SchemaDirectiveWiring::onEnumValue, enumType = enumType)) + wireDirectives(WiringWrapper(value, ENUM_VALUE, SchemaDirectiveWiring::onEnumValue, enumType = enumType)) } } diff --git a/src/test/kotlin/graphql/kickstart/tools/DirectiveTest.kt b/src/test/kotlin/graphql/kickstart/tools/DirectiveTest.kt index bb434364..7ba62157 100644 --- a/src/test/kotlin/graphql/kickstart/tools/DirectiveTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/DirectiveTest.kt @@ -419,6 +419,222 @@ class DirectiveTest { assertEquals(enumValue.getAppliedDirective("owner").getArgument("team")?.getValue(), "books-team") } + @Test + fun `should expose the directives of input fields through both the legacy and the applied view`() { + val rangeDirective = RangeDirective() + val schema = SchemaParser.newParser() + .schemaString( + """ + directive @range(min: Float!, max: Float!) on ARGUMENT_DEFINITION | INPUT_FIELD_DEFINITION + directive @tag(name: String!) on INPUT_OBJECT + + type Query { + withInput(input: InputObject): Float + } + + input InputObject @tag(name: "limits") { + value: Float @range(min: 0.00, max: 10.00) + nolimit: Float + limit: Float @range(min: 11.00, max: 15.00) + } + """) + .resolvers(WithInputQueryResolver()) + .directive("range", rangeDirective) + .build() + .makeExecutableSchema() + + val expectedRanges = mapOf("value" to listOf(0.0, 10.0), "limit" to listOf(11.0, 15.0)) + assertEquals(rangeDirective.appliedRanges, expectedRanges) + assertEquals(rangeDirective.legacyRanges, expectedRanges) + assertEquals(rangeDirective.elementRanges, expectedRanges) + val inputObject = schema.getType("InputObject") as GraphQLInputObjectType + val expectedDirectives = mapOf("value" to listOf("range"), "nolimit" to emptyList(), "limit" to listOf("range")) + assertEquals(inputObject.fields.associate { field -> field.name to field.directives.map { it.name } }, expectedDirectives) + assertEquals(inputObject.fields.associate { field -> field.name to field.appliedDirectives.map { it.name } }, expectedDirectives) + assertNotNull(inputObject.getDirective("tag")) + } + + @Test + fun `should validate directives on nested input fields reached through list and non-null types`() { + val schema = SchemaParser.newParser() + .schemaString( + """ + directive @size(min: Int = 0, max: Int = 2147483647) on ARGUMENT_DEFINITION | INPUT_FIELD_DEFINITION + directive @range(min: Float!, max: Float!) on ARGUMENT_DEFINITION | INPUT_FIELD_DEFINITION + + type Query { + changeUser(input: ChangeUserInput): Boolean + addBook(bookInput: BookInput!): Boolean + rate(score: Float @range(min: 0, max: 5)): Boolean + } + + input ChangeUserInput { + name: NameInput + aliases: [NameInput!] + guardian: GuardianInput! + } + + input GuardianInput { + name: NameInput! + } + + input NameInput { + forename: String! @size(min: 3, max: 25) + } + + input BookInput { + id: Int! @range(min: 4, max: 10) + name: String! + } + """) + .resolvers(ValidatedQueryResolver()) + .directiveWiring(ConstraintValidationWiring()) + .build() + .makeExecutableSchema() + + val gql = GraphQL.newGraphQL(schema) + .queryExecutionStrategy(AsyncExecutionStrategy()) + .build() + + val invalid = gql.execute( + """ + query { + changeUser(input: { name: { forename: "Al" }, aliases: [{ forename: "Alice" }, { forename: "Bo" }], guardian: { name: { forename: "Ed" } } }) + addBook(bookInput: { id: 11, name: "Dune" }) + rate(score: 6) + } + """) + assertEquals(invalid.errors.map { it.message }.sorted(), listOf( + "Exception while fetching data (/addBook) : bookInput.id must be between 4.0 and 10.0", + "Exception while fetching data (/changeUser) : input.name.forename, input.aliases[1].forename, input.guardian.name.forename must have a size between 3 and 25", + "Exception while fetching data (/rate) : score must be between 0.0 and 5.0" + )) + + val valid = gql.execute( + """ + query { + changeUser(input: { name: { forename: "Alice" }, aliases: [{ forename: "Bob" }], guardian: { name: { forename: "Eddie" } } }) + addBook(bookInput: { id: 5, name: "Dune" }) + rate(score: 4) + } + """) + assertEquals(valid.errors, emptyList()) + assertEquals(valid.getData(), mapOf("changeUser" to true, "addBook" to true, "rate" to true)) + } + + @Test + fun `should build input objects used by directive arguments even when they have directives themselves`() { + val wiredArgumentTypes = mutableMapOf() + val schema = SchemaParser.newParser() + .schemaString( + """ + directive @meta(info: MetaInput!, extras: [MetaInput!]) on FIELD_DEFINITION + + type Query { + name: String @meta(info: { note: "info" }, extras: [{ note: "extra" }]) + } + + input MetaInput { + note: String @deprecated + } + """) + .resolvers(object : GraphQLQueryResolver { + fun name(): String? = null + }) + .dictionary("MetaInput", MetaInput::class) + .directive("meta", object : SchemaDirectiveWiring { + override fun onField(environment: SchemaDirectiveWiringEnvironment): GraphQLFieldDefinition { + environment.appliedDirective.arguments.forEach { + wiredArgumentTypes[it.name] = GraphQLTypeUtil.unwrapAll(it.type).javaClass.simpleName + } + return environment.element + } + }) + .build() + .makeExecutableSchema() + + assertEquals(wiredArgumentTypes, mapOf("info" to "GraphQLInputObjectType", "extras" to "GraphQLInputObjectType")) + assert((schema.getType("MetaInput") as GraphQLInputObjectType).getField("note").isDeprecated) + } + + @Test + fun `should wire input objects used by directive arguments with the real definitions of the directives applied to them`() { + val metaDirective = "directive @meta(info: ConfigInput!) on FIELD_DEFINITION" + val optionDirective = "directive @option(value: OptionInput) on INPUT_OBJECT | INPUT_FIELD_DEFINITION" + listOf(listOf(metaDirective, optionDirective), listOf(optionDirective, metaDirective)).forEach { directives -> + val optionRecorder = OptionRecorder() + SchemaParser.newParser() + .schemaString( + """ + ${directives.joinToString("\n")} + + type Query { + name(config: ConfigInput): String @meta(info: { level: 1 }) + } + + input ConfigInput @option(value: { label: "type" }) { + level: Int @option(value: { label: "field" }) + } + + input OptionInput { + label: String + } + """) + .resolvers(object : GraphQLQueryResolver { + fun name(config: ConfigInput?): String? = null + }) + .dictionary("OptionInput", OptionInput::class) + .directive("option", optionRecorder) + .build() + .makeExecutableSchema() + + val expectedOptions = mapOf( + "ConfigInput" to listOf("GraphQLInputObjectType", mapOf("label" to "type"), mapOf("label" to "type")), + "level" to listOf("GraphQLInputObjectType", mapOf("label" to "field"), mapOf("label" to "field")) + ) + assert(optionRecorder.wiredOptions == expectedOptions) { "${optionRecorder.wiredOptions} with directives declared as $directives" } + } + } + + @Test + fun `should wire input objects only once when a directive is applied within its own argument types`() { + val limitDirective = "directive @limit(bound: BoundInput!) on INPUT_FIELD_DEFINITION" + val limitsDirective = "directive @limits(bounds: [BoundInput!]) on FIELD_DEFINITION" + listOf(listOf(limitsDirective, limitDirective), listOf(limitDirective, limitsDirective)).forEach { directives -> + val wiredFields = mutableListOf() + val schema = SchemaParser.newParser() + .schemaString( + """ + ${directives.joinToString("\n")} + + type Query { + name: String @limits(bounds: [{ max: 1 }]) + } + + input BoundInput { + max: Int @limit(bound: { max: 10 }) + } + """) + .resolvers(object : GraphQLQueryResolver { + fun name(): String? = null + }) + .dictionary("BoundInput", BoundInput::class) + .directive("limit", object : SchemaDirectiveWiring { + override fun onInputObjectField(environment: SchemaDirectiveWiringEnvironment): GraphQLInputObjectField { + wiredFields.add(environment.element.name) + return environment.element + } + }) + .build() + .makeExecutableSchema() + + assert(wiredFields == listOf("max")) { "$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)) + assert(GraphQLTypeUtil.unwrapAll(schema.getDirective("limit")!!.getArgument("bound").type) is GraphQLInputObjectType) + } + } + @Test fun `should apply directives on the schema and its extensions`() { val schema = SchemaParser.newParser() @@ -633,6 +849,158 @@ class DirectiveTest { } } + private class WithInputQueryResolver : GraphQLQueryResolver { + fun withInput(input: InputObject?): Double? = input?.value + } + + private data class InputObject( + val value: Double?, + val nolimit: Double?, + val limit: Double? + ) + + private class RangeDirective : SchemaDirectiveWiring { + val appliedRanges = mutableMapOf>() + val legacyRanges = mutableMapOf>() + val elementRanges = mutableMapOf>() + + override fun onInputObjectField(environment: SchemaDirectiveWiringEnvironment): GraphQLInputObjectField { + val field = environment.element + appliedRanges[field.name] = listOf("min", "max").map { environment.appliedDirective.getArgument(it)?.getValue() } + legacyRanges[field.name] = legacyRange(environment.directive) + elementRanges[field.name] = legacyRange(field.getDirective("range")) + return field + } + + private fun legacyRange(directive: GraphQLDirective?): List = + listOf("min", "max").map { name -> directive?.getArgument(name)?.let { GraphQLArgument.getArgumentValue(it) } } + } + + private data class MetaInput( + val note: String? + ) + + // not private, so that the arguments can be converted to them + data class ConfigInput( + val level: Int? + ) + + private data class OptionInput( + val label: String? + ) + + private data class BoundInput( + val max: Int? + ) + + /** + * Records the type and the applied and legacy value of the `value` argument of the directive at wiring time. + */ + private class OptionRecorder : SchemaDirectiveWiring { + val wiredOptions = mutableMapOf>() + + override fun onInputObjectType(environment: SchemaDirectiveWiringEnvironment): GraphQLInputObjectType { + wiredOptions[environment.element.name] = option(environment) + return environment.element + } + + override fun onInputObjectField(environment: SchemaDirectiveWiringEnvironment): GraphQLInputObjectField { + wiredOptions[environment.element.name] = option(environment) + return environment.element + } + + private fun option(environment: SchemaDirectiveWiringEnvironment<*>): List { + val applied = environment.appliedDirective.getArgument("value")!! + return listOf( + applied.type.javaClass.simpleName, + applied.getValue(), + GraphQLArgument.getArgumentValue(environment.directive.getArgument("value")) + ) + } + } + + private class ValidatedQueryResolver : GraphQLQueryResolver { + fun changeUser(input: ChangeUserInput?): Boolean = true + fun addBook(bookInput: BookInput): Boolean = true + fun rate(score: Double?): Boolean = true + } + + // not private, so that the arguments can be converted to them + data class ChangeUserInput( + val name: NameInput?, + val aliases: List?, + val guardian: GuardianInput + ) + + data class GuardianInput( + val name: NameInput + ) + + data class NameInput( + val forename: String + ) + + data class BookInput( + val id: Int, + val name: String + ) + + /** + * Validates @size and @range on arguments and on input fields nested in them, deciding at wiring time which fields need + * validating by walking the argument types like graphql-java-extended-validation's DirectivesAndTypeWalker does. + */ + private class ConstraintValidationWiring : SchemaDirectiveWiring { + override fun onField(environment: SchemaDirectiveWiringEnvironment): GraphQLFieldDefinition { + val field = environment.element + if (field.arguments.none { hasConstraint(it) || hasNestedConstraint(it.type) }) { + return field + } + + val coordinates = FieldCoordinates.coordinates(environment.fieldsContainer, field) + val originalDataFetcher = environment.codeRegistry.getDataFetcher(coordinates, field) + environment.codeRegistry.dataFetcher(coordinates, DataFetcher { env -> + val violations = env.fieldDefinition.arguments.flatMap { argument -> + violations(argument.name, argument, argument.type, env.getArgument(argument.name)) + } + if (violations.isNotEmpty()) { + throw IllegalArgumentException(violations.groupBy({ it.second }, { it.first }).map { (message, paths) -> "${paths.joinToString()} $message" }.joinToString("; ")) + } + originalDataFetcher.get(env) + }) + return field + } + + private fun hasConstraint(container: GraphQLDirectiveContainer) = + container.getDirective("size") != null || container.getDirective("range") != null + + private fun hasNestedConstraint(type: GraphQLInputType): Boolean { + val unwrapped = GraphQLTypeUtil.unwrapAll(type) + return unwrapped is GraphQLInputObjectType && unwrapped.fields.any { hasConstraint(it) || hasNestedConstraint(it.type) } + } + + private fun violations(path: String, container: GraphQLDirectiveContainer, type: GraphQLInputType, value: Any?): List> { + val unwrapped = GraphQLTypeUtil.unwrapNonNull(type) + return when { + value == null -> emptyList() + unwrapped is GraphQLList -> (value as List<*>).flatMapIndexed { i, item -> violations("$path[$i]", container, unwrapped.wrappedType as GraphQLInputType, item) } + unwrapped is GraphQLInputObjectType -> unwrapped.fields.flatMap { violations("$path.${it.name}", it, it.type, (value as Map<*, *>)[it.name]) } + else -> listOfNotNull(sizeViolation(container.getDirective("size"), value), rangeViolation(container.getDirective("range"), value)).map { path to it } + } + } + + private fun sizeViolation(directive: GraphQLDirective?, value: Any): String? { + val min = directive?.getArgument("min")?.let { GraphQLArgument.getArgumentValue(it) } ?: return null + val max = GraphQLArgument.getArgumentValue(directive.getArgument("max"))!! + return if ((value as String).length !in min..max) "must have a size between $min and $max" else null + } + + private fun rangeViolation(directive: GraphQLDirective?, value: Any): String? { + val min = directive?.getArgument("min")?.let { GraphQLArgument.getArgumentValue(it) } ?: return null + val max = GraphQLArgument.getArgumentValue(directive.getArgument("max"))!! + return if ((value as Number).toDouble() !in min..max) "must be between $min and $max" else null + } + } + private class UppercaseDirective : SchemaDirectiveWiring { override fun onObject(environment: SchemaDirectiveWiringEnvironment): GraphQLObjectType { val objectType = environment.element diff --git a/src/test/kotlin/graphql/kickstart/tools/SchemaParserTest.kt b/src/test/kotlin/graphql/kickstart/tools/SchemaParserTest.kt index e5e2ba30..1dc9d1ef 100644 --- a/src/test/kotlin/graphql/kickstart/tools/SchemaParserTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/SchemaParserTest.kt @@ -525,6 +525,101 @@ class SchemaParserTest { assert(testNullableArgument.type is GraphQLInputObjectType) } + @Test + fun `input objects nested in list and non-null input fields should resolve to GraphQLInputObjectType during directive wiring`() { + val wiredFieldTypes = mutableMapOf() + SchemaParser.newParser() + .schemaString( + """ + type Query { + test(filter: NestedFilter): Boolean + } + + input NestedFilter { + subs: [SubFilter!] + other: OtherFilter! + direct: DirectFilter + } + + extend input NestedFilter { + extended: [ExtendedFilter]! + } + + input SubFilter { name: String } + input OtherFilter { name: String } + input DirectFilter { name: String } + input ExtendedFilter { name: String } + """) + .resolvers(object : GraphQLQueryResolver { + fun test(filter: NestedFilter?): Boolean = false + }) + .directiveWiring(InputFieldTypeRecorder(wiredFieldTypes)) + .build() + .makeExecutableSchema() + + assertEquals(wiredFieldTypes, mapOf( + "subs" to "GraphQLInputObjectType", + "other" to "GraphQLInputObjectType", + "direct" to "GraphQLInputObjectType", + "extended" to "GraphQLInputObjectType" + )) + } + + @Test + fun `circular input objects should resolve to GraphQLInputObjectType during directive wiring except where they recurse`() { + val wiredFieldTypes = mutableMapOf() + val schema = SchemaParser.newParser() + .schemaString( + """ + type Query { + test(filter: TreeFilter!): Boolean + } + + input TreeFilter { + and: [TreeFilter!] + node: TreeNode! + } + + input TreeNode { + parent: TreeFilter + children: [TreeNode!]! + leaf: TreeLeaf! + } + + input TreeLeaf { name: String } + """) + .resolvers(object : GraphQLQueryResolver { + fun test(filter: TreeFilter): Boolean = false + }) + .directiveWiring(InputFieldTypeRecorder(wiredFieldTypes)) + .build() + .makeExecutableSchema() + + // TreeFilter is built first, so only the fields pointing back at a type that's still being built are references. This is on + // purpose: types are built depth-first through list and non-null wrappers, so in a cycle it's the edge back that stays a + // reference, wherever the wrappers are. Before, the wrapped edges were references instead, so `parent` was resolved. + assertEquals(wiredFieldTypes, mapOf( + "and" to "GraphQLTypeReference", + "node" to "GraphQLInputObjectType", + "parent" to "GraphQLTypeReference", + "children" to "GraphQLTypeReference", + "leaf" to "GraphQLInputObjectType" + )) + listOf("TreeFilter", "TreeNode").flatMap { (schema.getType(it) as GraphQLInputObjectType).fields }.forEach { field -> + assert(GraphQLTypeUtil.unwrapAll(field.type) is GraphQLInputObjectType) { "${field.name} wasn't resolved" } + } + } + + private class InputFieldTypeRecorder(private val wiredFieldTypes: MutableMap) : SchemaDirectiveWiring { + override fun onInputObjectField(environment: SchemaDirectiveWiringEnvironment): GraphQLInputObjectField { + val type = GraphQLTypeUtil.unwrapAllAs(environment.element.type) + if (type !is GraphQLScalarType) { + wiredFieldTypes[environment.element.name] = type.javaClass.simpleName + } + return environment.element + } + } + @Test fun `parser should use comments for descriptions`() { val schema = SchemaParser.newParser() @@ -690,6 +785,22 @@ class SchemaParserTest { fun filter(): String? = null } + data class NestedFilter(val subs: List?, val other: OtherFilter, val direct: DirectFilter?, val extended: List) + + data class SubFilter(val name: String?) + + data class OtherFilter(val name: String?) + + data class DirectFilter(val name: String?) + + data class ExtendedFilter(val name: String?) + + data class TreeFilter(val and: List?, val node: TreeNode) + + data class TreeNode(val parent: TreeFilter?, val children: List, val leaf: TreeLeaf) + + data class TreeLeaf(val name: String?) + class CustomGenericWrapper class Obj { From 4d59fdc0b8f5604ca4e4cdcf280a4b9007eee884 Mon Sep 17 00:00:00 2001 From: Oryan Date: Sat, 17 Oct 2026 09:33:17 -0400 Subject: [PATCH 2/3] Simplify the directive lookup and the nested input tests schemaDirectives is now a map by name, so finishing a directive replaces its entry instead of rebuilding the whole set, and findDirective is a plain lookup. The comments say in plainer words why directives are built twice. The WiringWrapper location change for input fields and enum values is reverted. directiveLocation isn't read anywhere, so it changed nothing. Tests: the #731 case is now an onInputObjectField override in the existing EmailDirective test. The two SchemaParserTest cases are one test with a list, a non-null and a back-pointing field. The two tests for input objects used by directive arguments are one, which records argument types at wiring time, in both declaration orders. Dropped the query-executing validation test: the type and legacy directive checks already cover what it walked, and the probe runs the issue's schema through extended-validation itself. None of the tests call deprecated API anymore, so the build passes with -Werror. Co-Authored-By: Claude Opus 5.5 --- .../graphql/kickstart/tools/SchemaParser.kt | 32 +- .../tools/directive/DirectiveWiringHelper.kt | 4 +- .../graphql/kickstart/tools/DirectiveTest.kt | 330 ++---------------- .../kickstart/tools/SchemaParserTest.kt | 97 +---- 4 files changed, 75 insertions(+), 388 deletions(-) diff --git a/src/main/kotlin/graphql/kickstart/tools/SchemaParser.kt b/src/main/kotlin/graphql/kickstart/tools/SchemaParser.kt index f1e063c8..9dd740ef 100644 --- a/src/main/kotlin/graphql/kickstart/tools/SchemaParser.kt +++ b/src/main/kotlin/graphql/kickstart/tools/SchemaParser.kt @@ -62,8 +62,9 @@ class SchemaParser internal constructor( private val codeRegistryBuilder = GraphQLCodeRegistry.newCodeRegistry() private val directiveWiringHelper = DirectiveWiringHelper(runtimeWiring, codeRegistryBuilder) - private lateinit var schemaDirectives : Set - // replaces the first definition of a directive with the real one while they're being created, see createDirectives + // by name, in the order they're declared + private val schemaDirectives = mutableMapOf() + // set while createDirectives runs, see there private var finishDirective: (String) -> Unit = {} /** @@ -114,7 +115,7 @@ class SchemaParser internal constructor( // Directives applied to the schema itself, e.g. `extend schema @link(...)`. Only GraphQLAppliedDirectives are built here, not // the deprecated GraphQLDirective ones: graphql-java doesn't resolve type references in those, so enum arguments would fail. val schemaAppliedDirectives = buildAppliedDirectives(rootInfo.getDirectives()).toList() - return SchemaObjects(query, mutation, subscription, types, schemaDirectives, codeRegistryBuilder, rootInfo.getDescription(), schemaAppliedDirectives) + return SchemaObjects(query, mutation, subscription, types, schemaDirectives.values.toSet(), codeRegistryBuilder, rootInfo.getDescription(), schemaAppliedDirectives) } /** @@ -325,32 +326,31 @@ class SchemaParser internal constructor( } private fun createDirectives(inputObjects: MutableList) { - // Input objects used by directive arguments get built here, and building them needs the definitions of their own applied - // directives. So a first set of definitions only refers to input objects by type reference, and each one is replaced by the - // real definition the first time it's looked up, so that input objects see the real definitions of the directives applied - // to them. Only where a directive is applied within its own argument types, i.e. a cycle, do they see the first definition. - schemaDirectives = directiveDefinitions.map { definition -> - createDirective(definition) { determineType(GraphQLInputType::class, it, permittedTypesForInputObject, inputObjects) as GraphQLInputType } - }.toSet() + // The input objects used by directive arguments are built here, and they need the directives applied to them. So every + // 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. + directiveDefinitions.forEach { definition -> + schemaDirectives[definition.name] = + createDirective(definition) { determineType(GraphQLInputType::class, it, permittedTypesForInputObject, inputObjects) as GraphQLInputType } + } val unfinishedDefinitions = directiveDefinitions.associateByTo(mutableMapOf()) { it.name } - // shared by all definitions, so that an input object that's still being built isn't built again for another directive + // shared, so that an input object that's still being built isn't built again for another directive val referencingInputObjects = mutableSetOf() finishDirective = { name -> unfinishedDefinitions.remove(name)?.let { definition -> - val directive = createDirective(definition) { determineInputType(it, inputObjects, referencingInputObjects) } - schemaDirectives = schemaDirectives.map { if (it.name == name) directive else it }.toSet() + schemaDirectives[name] = createDirective(definition) { determineInputType(it, inputObjects, referencingInputObjects) } } } directiveDefinitions.forEach { finishDirective(it.name) } finishDirective = {} // because the arguments can have directives too, we attach them only after the directives themselves are created - schemaDirectives = schemaDirectives.map { d -> + schemaDirectives.replaceAll { _, d -> val arguments = d.arguments.map { a -> a.transform { it.withAppliedDirectives(*buildAppliedDirectives(a.definition!!.directives)) .withDirectives(*buildDirectives(a.definition!!.directives, Introspection.DirectiveLocation.OBJECT)) } } d.transform { it.replaceArguments(arguments) } - }.toSet() + } } private fun createDirective(definition: DirectiveDefinition, determineArgumentType: (Type<*>) -> GraphQLInputType): GraphQLDirective { @@ -373,7 +373,7 @@ class SchemaParser internal constructor( private fun findDirective(name: String): GraphQLDirective? { finishDirective(name) - return schemaDirectives.find { it.name == name } + return schemaDirectives[name] } private fun createDirectiveArgument(definition: InputValueDefinition, determineArgumentType: (Type<*>) -> GraphQLInputType): GraphQLArgument { diff --git a/src/main/kotlin/graphql/kickstart/tools/directive/DirectiveWiringHelper.kt b/src/main/kotlin/graphql/kickstart/tools/directive/DirectiveWiringHelper.kt index e19a3dcd..4ccd45d2 100644 --- a/src/main/kotlin/graphql/kickstart/tools/directive/DirectiveWiringHelper.kt +++ b/src/main/kotlin/graphql/kickstart/tools/directive/DirectiveWiringHelper.kt @@ -59,13 +59,13 @@ internal class DirectiveWiringHelper( private fun wireInputFields(fieldsContainer: GraphQLInputFieldsContainer): List { return fieldsContainer.fieldDefinitions.map { field -> - wireDirectives(WiringWrapper(field, INPUT_FIELD_DEFINITION, SchemaDirectiveWiring::onInputObjectField, inputFieldsContainer = fieldsContainer)) + wireDirectives(WiringWrapper(field, FIELD_DEFINITION, SchemaDirectiveWiring::onInputObjectField, inputFieldsContainer = fieldsContainer)) } } private fun wireEnumValues(enumType: GraphQLEnumType): List { return enumType.values.map { value -> - wireDirectives(WiringWrapper(value, ENUM_VALUE, SchemaDirectiveWiring::onEnumValue, enumType = enumType)) + wireDirectives(WiringWrapper(value, FIELD_DEFINITION, SchemaDirectiveWiring::onEnumValue, enumType = enumType)) } } diff --git a/src/test/kotlin/graphql/kickstart/tools/DirectiveTest.kt b/src/test/kotlin/graphql/kickstart/tools/DirectiveTest.kt index 68a27f61..b6503e34 100644 --- a/src/test/kotlin/graphql/kickstart/tools/DirectiveTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/DirectiveTest.kt @@ -408,7 +408,8 @@ class DirectiveTest { val expectedMessages = mapOf( "contactEmail" to "{path} must be a valid email", "primaryEmail" to "{path} must be a valid email", - "backupEmail" to "invalid backup email" + "backupEmail" to "invalid backup email", + "email" to "{path} must be a valid email" ) assertEquals(emailDirective.appliedMessages, expectedMessages) assertEquals(emailDirective.legacyMessages, expectedMessages) @@ -420,156 +421,19 @@ class DirectiveTest { } @Test - fun `should expose the directives of input fields through both the legacy and the applied view`() { - val rangeDirective = RangeDirective() - val schema = SchemaParser.newParser() - .schemaString( - """ - directive @range(min: Float!, max: Float!) on ARGUMENT_DEFINITION | INPUT_FIELD_DEFINITION - directive @tag(name: String!) on INPUT_OBJECT - - type Query { - withInput(input: InputObject): Float - } - - input InputObject @tag(name: "limits") { - value: Float @range(min: 0.00, max: 10.00) - nolimit: Float - limit: Float @range(min: 11.00, max: 15.00) - } - """) - .resolvers(WithInputQueryResolver()) - .directive("range", rangeDirective) - .build() - .makeExecutableSchema() - - val expectedRanges = mapOf("value" to listOf(0.0, 10.0), "limit" to listOf(11.0, 15.0)) - assertEquals(rangeDirective.appliedRanges, expectedRanges) - assertEquals(rangeDirective.legacyRanges, expectedRanges) - assertEquals(rangeDirective.elementRanges, expectedRanges) - val inputObject = schema.getType("InputObject") as GraphQLInputObjectType - val expectedDirectives = mapOf("value" to listOf("range"), "nolimit" to emptyList(), "limit" to listOf("range")) - assertEquals(inputObject.fields.associate { field -> field.name to field.directives.map { it.name } }, expectedDirectives) - assertEquals(inputObject.fields.associate { field -> field.name to field.appliedDirectives.map { it.name } }, expectedDirectives) - assertNotNull(inputObject.getDirective("tag")) - } - - @Test - fun `should validate directives on nested input fields reached through list and non-null types`() { - val schema = SchemaParser.newParser() - .schemaString( - """ - directive @size(min: Int = 0, max: Int = 2147483647) on ARGUMENT_DEFINITION | INPUT_FIELD_DEFINITION - directive @range(min: Float!, max: Float!) on ARGUMENT_DEFINITION | INPUT_FIELD_DEFINITION - - type Query { - changeUser(input: ChangeUserInput): Boolean - addBook(bookInput: BookInput!): Boolean - rate(score: Float @range(min: 0, max: 5)): Boolean - } - - input ChangeUserInput { - name: NameInput - aliases: [NameInput!] - guardian: GuardianInput! - } - - input GuardianInput { - name: NameInput! - } - - input NameInput { - forename: String! @size(min: 3, max: 25) - } - - input BookInput { - id: Int! @range(min: 4, max: 10) - name: String! - } - """) - .resolvers(ValidatedQueryResolver()) - .directiveWiring(ConstraintValidationWiring()) - .build() - .makeExecutableSchema() - - val gql = GraphQL.newGraphQL(schema) - .queryExecutionStrategy(AsyncExecutionStrategy()) - .build() - - val invalid = gql.execute( - """ - query { - changeUser(input: { name: { forename: "Al" }, aliases: [{ forename: "Alice" }, { forename: "Bo" }], guardian: { name: { forename: "Ed" } } }) - addBook(bookInput: { id: 11, name: "Dune" }) - rate(score: 6) - } - """) - assertEquals(invalid.errors.map { it.message }.sorted(), listOf( - "Exception while fetching data (/addBook) : bookInput.id must be between 4.0 and 10.0", - "Exception while fetching data (/changeUser) : input.name.forename, input.aliases[1].forename, input.guardian.name.forename must have a size between 3 and 25", - "Exception while fetching data (/rate) : score must be between 0.0 and 5.0" - )) - - val valid = gql.execute( - """ - query { - changeUser(input: { name: { forename: "Alice" }, aliases: [{ forename: "Bob" }], guardian: { name: { forename: "Eddie" } } }) - addBook(bookInput: { id: 5, name: "Dune" }) - rate(score: 4) - } - """) - assertEquals(valid.errors, emptyList()) - assertEquals(valid.getData(), mapOf("changeUser" to true, "addBook" to true, "rate" to true)) - } - - @Test - fun `should build input objects used by directive arguments even when they have directives themselves`() { - val wiredArgumentTypes = mutableMapOf() - val schema = SchemaParser.newParser() - .schemaString( - """ - directive @meta(info: MetaInput!, extras: [MetaInput!]) on FIELD_DEFINITION - - type Query { - name: String @meta(info: { note: "info" }, extras: [{ note: "extra" }]) - } - - input MetaInput { - note: String @deprecated - } - """) - .resolvers(object : GraphQLQueryResolver { - fun name(): String? = null - }) - .dictionary("MetaInput", MetaInput::class) - .directive("meta", object : SchemaDirectiveWiring { - override fun onField(environment: SchemaDirectiveWiringEnvironment): GraphQLFieldDefinition { - environment.appliedDirective.arguments.forEach { - wiredArgumentTypes[it.name] = GraphQLTypeUtil.unwrapAll(it.type).javaClass.simpleName - } - return environment.element - } - }) - .build() - .makeExecutableSchema() - - assertEquals(wiredArgumentTypes, mapOf("info" to "GraphQLInputObjectType", "extras" to "GraphQLInputObjectType")) - assert((schema.getType("MetaInput") as GraphQLInputObjectType).getField("note").isDeprecated) - } - - @Test - fun `should wire input objects used by directive arguments with the real definitions of the directives applied to them`() { - val metaDirective = "directive @meta(info: ConfigInput!) on FIELD_DEFINITION" - val optionDirective = "directive @option(value: OptionInput) on INPUT_OBJECT | INPUT_FIELD_DEFINITION" - listOf(listOf(metaDirective, optionDirective), listOf(optionDirective, metaDirective)).forEach { directives -> - val optionRecorder = OptionRecorder() + fun `should build input objects used by directive arguments before wiring`() { + val meta = "directive @meta(info: ConfigInput!) on FIELD_DEFINITION" + val option = "directive @option(value: OptionInput) on INPUT_OBJECT | INPUT_FIELD_DEFINITION" + // ConfigInput is built for @meta and needs @option, whichever is declared first + listOf("$meta\n$option", "$option\n$meta").forEach { directives -> + val recorder = ArgumentTypeRecorder() SchemaParser.newParser() .schemaString( """ - ${directives.joinToString("\n")} + $directives type Query { - name(config: ConfigInput): String @meta(info: { level: 1 }) + name: String @meta(info: { level: 1 }) } input ConfigInput @option(value: { label: "type" }) { @@ -581,31 +445,30 @@ class DirectiveTest { } """) .resolvers(object : GraphQLQueryResolver { - fun name(config: ConfigInput?): String? = null + fun name(): String? = null }) - .dictionary("OptionInput", OptionInput::class) - .directive("option", optionRecorder) + .dictionary(ConfigInput::class, OptionInput::class) + .directive("meta", recorder) + .directive("option", recorder) .build() .makeExecutableSchema() - val expectedOptions = mapOf( - "ConfigInput" to listOf("GraphQLInputObjectType", mapOf("label" to "type"), mapOf("label" to "type")), - "level" to listOf("GraphQLInputObjectType", mapOf("label" to "field"), mapOf("label" to "field")) - ) - assert(optionRecorder.wiredOptions == expectedOptions) { "${optionRecorder.wiredOptions} with directives declared as $directives" } + val inputObject = listOf("GraphQLInputObjectType") + val expectedTypes = mapOf("name" to inputObject, "ConfigInput" to inputObject, "level" to inputObject) + assert(recorder.argumentTypes == expectedTypes) { "${recorder.argumentTypes} with directives declared as $directives" } } } @Test fun `should wire input objects only once when a directive is applied within its own argument types`() { - val limitDirective = "directive @limit(bound: BoundInput!) on INPUT_FIELD_DEFINITION" - val limitsDirective = "directive @limits(bounds: [BoundInput!]) on FIELD_DEFINITION" - listOf(listOf(limitsDirective, limitDirective), listOf(limitDirective, limitsDirective)).forEach { directives -> + val limit = "directive @limit(bound: BoundInput!) 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() val schema = SchemaParser.newParser() .schemaString( """ - ${directives.joinToString("\n")} + $directives type Query { name: String @limits(bounds: [{ max: 1 }]) @@ -618,7 +481,7 @@ class DirectiveTest { .resolvers(object : GraphQLQueryResolver { fun name(): String? = null }) - .dictionary("BoundInput", BoundInput::class) + .dictionary(BoundInput::class) .directive("limit", object : SchemaDirectiveWiring { override fun onInputObjectField(environment: SchemaDirectiveWiringEnvironment): GraphQLInputObjectField { wiredFields.add(environment.element.name) @@ -631,7 +494,6 @@ class DirectiveTest { assert(wiredFields == listOf("max")) { "$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)) - assert(GraphQLTypeUtil.unwrapAll(schema.getDirective("limit")!!.getArgument("bound").type) is GraphQLInputObjectType) } } @@ -842,6 +704,11 @@ class DirectiveTest { return environment.element } + override fun onInputObjectField(environment: SchemaDirectiveWiringEnvironment): GraphQLInputObjectField { + recordMessage(environment) + return environment.element + } + private fun recordMessage(environment: SchemaDirectiveWiringEnvironment<*>) { val name = environment.element.name appliedMessages[name] = environment.appliedDirective.getArgument("message")?.getValue() @@ -850,39 +717,7 @@ class DirectiveTest { } } - private class WithInputQueryResolver : GraphQLQueryResolver { - fun withInput(input: InputObject?): Double? = input?.value - } - - private data class InputObject( - val value: Double?, - val nolimit: Double?, - val limit: Double? - ) - - private class RangeDirective : SchemaDirectiveWiring { - val appliedRanges = mutableMapOf>() - val legacyRanges = mutableMapOf>() - val elementRanges = mutableMapOf>() - - override fun onInputObjectField(environment: SchemaDirectiveWiringEnvironment): GraphQLInputObjectField { - val field = environment.element - appliedRanges[field.name] = listOf("min", "max").map { environment.appliedDirective.getArgument(it)?.getValue() } - legacyRanges[field.name] = legacyRange(environment.directive) - elementRanges[field.name] = legacyRange(field.getDirective("range")) - return field - } - - private fun legacyRange(directive: GraphQLDirective?): List = - listOf("min", "max").map { name -> directive?.getArgument(name)?.let { GraphQLArgument.getArgumentValue(it) } } - } - - private data class MetaInput( - val note: String? - ) - - // not private, so that the arguments can be converted to them - data class ConfigInput( + private data class ConfigInput( val level: Int? ) @@ -895,110 +730,21 @@ class DirectiveTest { ) /** - * Records the type and the applied and legacy value of the `value` argument of the directive at wiring time. + * Records the unwrapped types of the arguments of each applied directive, by the name of the element it's applied to. */ - private class OptionRecorder : SchemaDirectiveWiring { - val wiredOptions = mutableMapOf>() + private class ArgumentTypeRecorder : SchemaDirectiveWiring { + val argumentTypes = mutableMapOf>() - override fun onInputObjectType(environment: SchemaDirectiveWiringEnvironment): GraphQLInputObjectType { - wiredOptions[environment.element.name] = option(environment) - return environment.element - } + override fun onField(environment: SchemaDirectiveWiringEnvironment) = record(environment) - override fun onInputObjectField(environment: SchemaDirectiveWiringEnvironment): GraphQLInputObjectField { - wiredOptions[environment.element.name] = option(environment) - return environment.element - } + override fun onInputObjectType(environment: SchemaDirectiveWiringEnvironment) = record(environment) - private fun option(environment: SchemaDirectiveWiringEnvironment<*>): List { - val applied = environment.appliedDirective.getArgument("value")!! - return listOf( - applied.type.javaClass.simpleName, - applied.getValue(), - GraphQLArgument.getArgumentValue(environment.directive.getArgument("value")) - ) - } - } + override fun onInputObjectField(environment: SchemaDirectiveWiringEnvironment) = record(environment) - private class ValidatedQueryResolver : GraphQLQueryResolver { - fun changeUser(input: ChangeUserInput?): Boolean = true - fun addBook(bookInput: BookInput): Boolean = true - fun rate(score: Double?): Boolean = true - } - - // not private, so that the arguments can be converted to them - data class ChangeUserInput( - val name: NameInput?, - val aliases: List?, - val guardian: GuardianInput - ) - - data class GuardianInput( - val name: NameInput - ) - - data class NameInput( - val forename: String - ) - - data class BookInput( - val id: Int, - val name: String - ) - - /** - * Validates @size and @range on arguments and on input fields nested in them, deciding at wiring time which fields need - * validating by walking the argument types like graphql-java-extended-validation's DirectivesAndTypeWalker does. - */ - private class ConstraintValidationWiring : SchemaDirectiveWiring { - override fun onField(environment: SchemaDirectiveWiringEnvironment): GraphQLFieldDefinition { - val field = environment.element - if (field.arguments.none { hasConstraint(it) || hasNestedConstraint(it.type) }) { - return field - } - - val coordinates = FieldCoordinates.coordinates(environment.fieldsContainer, field) - val originalDataFetcher = environment.codeRegistry.getDataFetcher(coordinates, field) - environment.codeRegistry.dataFetcher(coordinates, DataFetcher { env -> - val violations = env.fieldDefinition.arguments.flatMap { argument -> - violations(argument.name, argument, argument.type, env.getArgument(argument.name)) - } - if (violations.isNotEmpty()) { - throw IllegalArgumentException(violations.groupBy({ it.second }, { it.first }).map { (message, paths) -> "${paths.joinToString()} $message" }.joinToString("; ")) - } - originalDataFetcher.get(env) - }) - return field - } - - private fun hasConstraint(container: GraphQLDirectiveContainer) = - container.getDirective("size") != null || container.getDirective("range") != null - - private fun hasNestedConstraint(type: GraphQLInputType): Boolean { - val unwrapped = GraphQLTypeUtil.unwrapAll(type) - return unwrapped is GraphQLInputObjectType && unwrapped.fields.any { hasConstraint(it) || hasNestedConstraint(it.type) } - } - - private fun violations(path: String, container: GraphQLDirectiveContainer, type: GraphQLInputType, value: Any?): List> { - val unwrapped = GraphQLTypeUtil.unwrapNonNull(type) - return when { - value == null -> emptyList() - unwrapped is GraphQLList -> (value as List<*>).flatMapIndexed { i, item -> violations("$path[$i]", container, unwrapped.wrappedType as GraphQLInputType, item) } - unwrapped is GraphQLInputObjectType -> unwrapped.fields.flatMap { violations("$path.${it.name}", it, it.type, (value as Map<*, *>)[it.name]) } - else -> listOfNotNull(sizeViolation(container.getDirective("size"), value), rangeViolation(container.getDirective("range"), value)).map { path to it } - } - } - - private fun sizeViolation(directive: GraphQLDirective?, value: Any): String? { - val min = directive?.getArgument("min")?.let { GraphQLArgument.getArgumentValue(it) } ?: return null - val max = GraphQLArgument.getArgumentValue(directive.getArgument("max"))!! - return if ((value as String).length !in min..max) "must have a size between $min and $max" else null - } - - private fun rangeViolation(directive: GraphQLDirective?, value: Any): String? { - val min = directive?.getArgument("min")?.let { GraphQLArgument.getArgumentValue(it) } ?: return null - val max = GraphQLArgument.getArgumentValue(directive.getArgument("max"))!! - return if ((value as Number).toDouble() !in min..max) "must be between $min and $max" else null + private fun record(environment: SchemaDirectiveWiringEnvironment): T { + argumentTypes[environment.element.name] = + environment.appliedDirective.arguments.map { GraphQLTypeUtil.unwrapAllAs(it.type).javaClass.simpleName } + return environment.element } } diff --git a/src/test/kotlin/graphql/kickstart/tools/SchemaParserTest.kt b/src/test/kotlin/graphql/kickstart/tools/SchemaParserTest.kt index 36326dcc..cfcd88f0 100644 --- a/src/test/kotlin/graphql/kickstart/tools/SchemaParserTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/SchemaParserTest.kt @@ -592,7 +592,7 @@ class SchemaParserTest { } @Test - fun `input objects nested in list and non-null input fields should resolve to GraphQLInputObjectType during directive wiring`() { + fun `input objects behind list and non-null input fields should resolve to GraphQLInputObjectType during directive wiring`() { val wiredFieldTypes = mutableMapOf() SchemaParser.newParser() .schemaString( @@ -604,86 +604,37 @@ class SchemaParserTest { input NestedFilter { subs: [SubFilter!] other: OtherFilter! - direct: DirectFilter } - extend input NestedFilter { - extended: [ExtendedFilter]! + input SubFilter { + parent: NestedFilter } - input SubFilter { name: String } - input OtherFilter { name: String } - input DirectFilter { name: String } - input ExtendedFilter { name: String } + input OtherFilter { + name: String + } """) .resolvers(object : GraphQLQueryResolver { fun test(filter: NestedFilter?): Boolean = false }) - .directiveWiring(InputFieldTypeRecorder(wiredFieldTypes)) - .build() - .makeExecutableSchema() - - assertEquals(wiredFieldTypes, mapOf( - "subs" to "GraphQLInputObjectType", - "other" to "GraphQLInputObjectType", - "direct" to "GraphQLInputObjectType", - "extended" to "GraphQLInputObjectType" - )) - } - - @Test - fun `circular input objects should resolve to GraphQLInputObjectType during directive wiring except where they recurse`() { - val wiredFieldTypes = mutableMapOf() - val schema = SchemaParser.newParser() - .schemaString( - """ - type Query { - test(filter: TreeFilter!): Boolean - } - - input TreeFilter { - and: [TreeFilter!] - node: TreeNode! - } - - input TreeNode { - parent: TreeFilter - children: [TreeNode!]! - leaf: TreeLeaf! + .directiveWiring(object : SchemaDirectiveWiring { + override fun onInputObjectField(environment: SchemaDirectiveWiringEnvironment): GraphQLInputObjectField { + val type = GraphQLTypeUtil.unwrapAllAs(environment.element.type) + if (type !is GraphQLScalarType) { + wiredFieldTypes[environment.element.name] = type.javaClass.simpleName + } + return environment.element } - - input TreeLeaf { name: String } - """) - .resolvers(object : GraphQLQueryResolver { - fun test(filter: TreeFilter): Boolean = false }) - .directiveWiring(InputFieldTypeRecorder(wiredFieldTypes)) .build() .makeExecutableSchema() - // TreeFilter is built first, so only the fields pointing back at a type that's still being built are references. This is on - // purpose: types are built depth-first through list and non-null wrappers, so in a cycle it's the edge back that stays a - // reference, wherever the wrappers are. Before, the wrapped edges were references instead, so `parent` was resolved. + // parent points back at NestedFilter while it's still being built, so it stays a type reference until the schema is built assertEquals(wiredFieldTypes, mapOf( - "and" to "GraphQLTypeReference", - "node" to "GraphQLInputObjectType", - "parent" to "GraphQLTypeReference", - "children" to "GraphQLTypeReference", - "leaf" to "GraphQLInputObjectType" + "subs" to "GraphQLInputObjectType", + "other" to "GraphQLInputObjectType", + "parent" to "GraphQLTypeReference" )) - listOf("TreeFilter", "TreeNode").flatMap { (schema.getType(it) as GraphQLInputObjectType).fields }.forEach { field -> - assert(GraphQLTypeUtil.unwrapAll(field.type) is GraphQLInputObjectType) { "${field.name} wasn't resolved" } - } - } - - private class InputFieldTypeRecorder(private val wiredFieldTypes: MutableMap) : SchemaDirectiveWiring { - override fun onInputObjectField(environment: SchemaDirectiveWiringEnvironment): GraphQLInputObjectField { - val type = GraphQLTypeUtil.unwrapAllAs(environment.element.type) - if (type !is GraphQLScalarType) { - wiredFieldTypes[environment.element.name] = type.javaClass.simpleName - } - return environment.element - } } @Test @@ -851,22 +802,12 @@ class SchemaParserTest { fun filter(): String? = null } - data class NestedFilter(val subs: List?, val other: OtherFilter, val direct: DirectFilter?, val extended: List) + data class NestedFilter(val subs: List?, val other: OtherFilter) - data class SubFilter(val name: String?) + data class SubFilter(val parent: NestedFilter?) data class OtherFilter(val name: String?) - data class DirectFilter(val name: String?) - - data class ExtendedFilter(val name: String?) - - data class TreeFilter(val and: List?, val node: TreeNode) - - data class TreeNode(val parent: TreeFilter?, val children: List, val leaf: TreeLeaf) - - data class TreeLeaf(val name: String?) - class CustomGenericWrapper class Obj { From 75a9362f8bed0d325ddae226d84d8adf3f3aa60c Mon Sep 17 00:00:00 2001 From: Oryan Date: Sat, 17 Oct 2026 10:52:22 -0400 Subject: [PATCH 3/3] Keep the directive definitions in one object SchemaDirectives holds the directives by name, the ones not rebuilt with their input objects yet, and the input objects being built for them. find() rebuilds a directive the first time it's looked up. This replaces the finishDirective function that createDirectives had to set and reset, and a new object per parse means nothing carries over between parses. Co-Authored-By: Claude Opus 5.5 --- .../graphql/kickstart/tools/SchemaParser.kt | 63 ++++++++++--------- 1 file changed, 33 insertions(+), 30 deletions(-) diff --git a/src/main/kotlin/graphql/kickstart/tools/SchemaParser.kt b/src/main/kotlin/graphql/kickstart/tools/SchemaParser.kt index 9dd740ef..068c21d2 100644 --- a/src/main/kotlin/graphql/kickstart/tools/SchemaParser.kt +++ b/src/main/kotlin/graphql/kickstart/tools/SchemaParser.kt @@ -62,10 +62,7 @@ class SchemaParser internal constructor( private val codeRegistryBuilder = GraphQLCodeRegistry.newCodeRegistry() private val directiveWiringHelper = DirectiveWiringHelper(runtimeWiring, codeRegistryBuilder) - // by name, in the order they're declared - private val schemaDirectives = mutableMapOf() - // set while createDirectives runs, see there - private var finishDirective: (String) -> Unit = {} + private lateinit var schemaDirectives: SchemaDirectives /** * Parses the given schema with respect to the given dictionary and returns GraphQL objects. @@ -115,7 +112,7 @@ class SchemaParser internal constructor( // Directives applied to the schema itself, e.g. `extend schema @link(...)`. Only GraphQLAppliedDirectives are built here, not // the deprecated GraphQLDirective ones: graphql-java doesn't resolve type references in those, so enum arguments would fail. val schemaAppliedDirectives = buildAppliedDirectives(rootInfo.getDirectives()).toList() - return SchemaObjects(query, mutation, subscription, types, schemaDirectives.values.toSet(), codeRegistryBuilder, rootInfo.getDescription(), schemaAppliedDirectives) + return SchemaObjects(query, mutation, subscription, types, schemaDirectives.all, codeRegistryBuilder, rootInfo.getDescription(), schemaAppliedDirectives) } /** @@ -326,25 +323,10 @@ class SchemaParser internal constructor( } private fun createDirectives(inputObjects: MutableList) { - // The input objects used by directive arguments are built here, and they need the directives applied to them. So every - // 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. - directiveDefinitions.forEach { definition -> - schemaDirectives[definition.name] = - createDirective(definition) { determineType(GraphQLInputType::class, it, permittedTypesForInputObject, inputObjects) as GraphQLInputType } - } - val unfinishedDefinitions = directiveDefinitions.associateByTo(mutableMapOf()) { it.name } - // shared, so that an input object that's still being built isn't built again for another directive - val referencingInputObjects = mutableSetOf() - finishDirective = { name -> - unfinishedDefinitions.remove(name)?.let { definition -> - schemaDirectives[name] = createDirective(definition) { determineInputType(it, inputObjects, referencingInputObjects) } - } - } - directiveDefinitions.forEach { finishDirective(it.name) } - finishDirective = {} + schemaDirectives = SchemaDirectives(inputObjects) + directiveDefinitions.forEach { schemaDirectives.find(it.name) } // because the arguments can have directives too, we attach them only after the directives themselves are created - schemaDirectives.replaceAll { _, d -> + schemaDirectives.update { d -> val arguments = d.arguments.map { a -> a.transform { it.withAppliedDirectives(*buildAppliedDirectives(a.definition!!.directives)) .withDirectives(*buildDirectives(a.definition!!.directives, Introspection.DirectiveLocation.OBJECT)) @@ -353,6 +335,32 @@ 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. + */ + 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 } + } + 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 + private val referencingInputObjects = mutableSetOf() + + val all: Set get() = byName.values.toSet() + + fun find(name: String): GraphQLDirective? { + unfinished.remove(name)?.let { definition -> + byName[name] = createDirective(definition) { determineInputType(it, inputObjects, referencingInputObjects) } + } + return byName[name] + } + + fun update(transform: (GraphQLDirective) -> GraphQLDirective) = byName.replaceAll { _, directive -> transform(directive) } + } + private fun createDirective(definition: DirectiveDefinition, determineArgumentType: (Type<*>) -> GraphQLInputType): GraphQLDirective { val locations = definition.directiveLocations.map { Introspection.DirectiveLocation.valueOf(it.name) }.toTypedArray() @@ -371,11 +379,6 @@ class SchemaParser internal constructor( .build() } - private fun findDirective(name: String): GraphQLDirective? { - finishDirective(name) - return schemaDirectives[name] - } - private fun createDirectiveArgument(definition: InputValueDefinition, determineArgumentType: (Type<*>) -> GraphQLInputType): GraphQLArgument { return GraphQLArgument.newArgument() .name(definition.name) @@ -389,7 +392,7 @@ class SchemaParser internal constructor( private fun buildAppliedDirectives(directives: List): Array { return directives.map { directive -> - val graphQLDirective = findDirective(directive.name) + val graphQLDirective = schemaDirectives.find(directive.name) ?: BUILT_IN_DIRECTIVES[directive.name] ?: if (options.allowUndeclaredDirectives) return@map buildUndeclaredAppliedDirective(directive) else throw SchemaError("Found applied directive ${directive.name} without corresponding directive definition.") @@ -474,7 +477,7 @@ class SchemaParser internal constructor( val repeatable = directiveDefinitions.find { it.name.equals(directive.name) }?.isRepeatable ?: false if (repeatable || !names.contains(directive.name)) { names.add(directive.name) - val graphQLDirective = findDirective(directive.name) + val graphQLDirective = schemaDirectives.find(directive.name) ?: BUILT_IN_DIRECTIVES[directive.name] // graphql-java rejects legacy directives without a definition, so undeclared ones are only kept as applied directives ?: if (options.allowUndeclaredDirectives) continue