From 8a80a7208920a224d6ddadec059124d7666df25d Mon Sep 17 00:00:00 2001 From: Oryan Date: Fri, 9 Oct 2026 21:03:23 -0400 Subject: [PATCH 1/3] Register input field types that have no Java property When the scanner couldn't find a Java property for an input field, e.g. because the input is bound to a Map or ObjectNode, the getter name doesn't match the schema field (getFoo_Back for foo_back) or the field is private, it only logged "Cannot find definition for field ..." and never registered the field's type. Nested input objects and custom scalars only reachable that way were left out of the scan, and the parser then failed with "Expected type 'X' to be a GraphQLInputType, but it wasn't!", which pointed users at the wrong problem. The scanner now still registers what such a field needs. A nested input object is registered without a class and its fields are scanned the same way; a class found for it later is set and scanned as usual, so it doesn't clash. A custom scalar is registered like any other. An enum can't be built without its Java class, so if none is found for it anywhere, the scan fails with a message naming the enum, the input type, the field and the class, and how to fix it. The garbled warning is replaced by one that's only logged when an input class (not a Map or JsonNode) lacks a matching property. Input fields of a Map or JsonNode are no longer matched against the methods and fields it inherits from the JDK or Jackson, so fields named like values, elements or fields work with ObjectNode. Getters declared by a Map subclass are still used. A class taken from the dictionary for an input field is matched against the field's unwrapped type, so a list field whose element type is in the dictionary no longer fails with "Java class is not a List". A misspelled property on an input class whose nested input type is only reachable through it used to fail the build with the misleading error above. It now builds with the warning and, like any other misspelled property, fails when a client sends the field, or the value is dropped if the ObjectMapper ignores unknown properties. There's still no build-time check that input class properties match the schema (#422). The parser's "Expected type ... to be a GraphQLInputType" error is replaced by one that tells apart a type that isn't defined, a type of the wrong kind (an output type used as an input or vice versa) and a type that was defined but never reached while scanning, with a hint for reaching it that depends on the kind of type. It names the field or argument using the type and its source location. Schemas passed through schemaString() are still parsed as one document without a source name, so their line numbers count from the start of all strings (#392). Co-Authored-By: Claude Opus 5.5 --- .../kickstart/tools/SchemaClassScanner.kt | 122 ++++-- .../graphql/kickstart/tools/SchemaParser.kt | 65 +++- .../kickstart/tools/NestedInputTypesTest.kt | 365 ++++++++++++++++++ .../kickstart/tools/SchemaClassScannerTest.kt | 49 +++ .../kickstart/tools/SchemaParserTest.kt | 259 ++++++++++++- src/test/resources/UnreachedType.graphqls | 11 + 6 files changed, 824 insertions(+), 47 deletions(-) create mode 100644 src/test/resources/UnreachedType.graphqls diff --git a/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt b/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt index bea11ace..a741eddd 100644 --- a/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt +++ b/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt @@ -1,5 +1,6 @@ package graphql.kickstart.tools +import com.fasterxml.jackson.databind.JsonNode import graphql.kickstart.tools.resolver.FieldResolver import graphql.kickstart.tools.resolver.FieldResolverScanner import graphql.kickstart.tools.util.* @@ -8,6 +9,7 @@ import graphql.schema.GraphQLScalarType import graphql.schema.idl.ScalarInfo import org.apache.commons.lang3.ClassUtils import org.slf4j.LoggerFactory +import java.lang.reflect.Member import java.lang.reflect.Method /** @@ -45,6 +47,7 @@ internal class SchemaClassScanner( private val typeClassMatcher = TypeClassMatcher(definitionsByName) private val dictionary = mutableMapOf, DictionaryEntry>() private val unvalidatedTypes = mutableSetOf>() + private val enumsWithoutClass = mutableMapOf() private val queue = linkedSetOf() private val fieldResolversByType = mutableMapOf>() @@ -173,6 +176,11 @@ internal class SchemaClassScanner( } private fun validateAndCreateResult(rootTypeHolder: RootTypesHolder): ScannedSchemaObjects { + enumsWithoutClass + .filter { dictionary[it.key]?.javaType == null } + .values.firstOrNull() + ?.let { throw SchemaClassScannerError(it) } + initialDictionary .filter { !it.value.accessed } .forEach { @@ -384,31 +392,83 @@ internal class SchemaClassScanner( scanInterfacesOfType(graphQLType) } - is InputObjectTypeDefinition -> { - val inputObjectTypes = listOf(graphQLType) + inputExtensionDefinitions.filter { it.name == graphQLType.name } - inputObjectTypes - .flatMap { it.inputValueDefinitions } - .forEach { inputValueDefinition -> - val inputGraphQLType = inputValueDefinition.type.unwrap() - if (inputGraphQLType is TypeName && !ScalarInfo.GRAPHQL_SPECIFICATION_SCALARS_DEFINITIONS.containsKey(inputGraphQLType.name)) { - val inputValueJavaType = findInputValueType(inputValueDefinition.name, inputGraphQLType, javaType.unwrap()) - if (inputValueJavaType != null) { - handleFoundType(typeClassMatcher.match(TypeClassMatcher.PotentialMatch.parameterType( - inputValueDefinition.type, - inputValueJavaType, - GenericType(javaType, options).relativeToType(javaType), - InputObjectReference(inputValueDefinition) - ))) - } else { - var mappingAdvice = "Try adding it manually to the dictionary" - if (javaType.unwrap().name.contains("Map")) { - mappingAdvice = " or add a class to represent your input type instead of a Map." - } - log.warn("Cannot find definition for field '${inputValueDefinition.name}: ${inputGraphQLType.name}' on input type '${graphQLType.name}' -> ${javaType.unwrap().name}. $mappingAdvice") - } - } + is InputObjectTypeDefinition -> handleInputObjectFields(graphQLType, javaType) + } + } + + /** + * Finds the types of the input object's fields through the properties of its class. A null [javaType] means the input object + * has no known class, e.g. because it's only reachable through a Map. + */ + private fun handleInputObjectFields(graphQLType: InputObjectTypeDefinition, javaType: JavaType?) { + val inputObjectTypes = listOf(graphQLType) + inputExtensionDefinitions.filter { it.name == graphQLType.name } + inputObjectTypes + .flatMap { it.inputValueDefinitions } + .forEach { inputValueDefinition -> + val inputGraphQLType = inputValueDefinition.type.unwrap() + if (inputGraphQLType is TypeName && !ScalarInfo.GRAPHQL_SPECIFICATION_SCALARS_DEFINITIONS.containsKey(inputGraphQLType.name)) { + val propertyType = javaType?.unwrap()?.let { findInputValueTypeInType(inputValueDefinition.name, it) } + val inputValueJavaType = propertyType ?: initialDictionary[inputGraphQLType.name]?.get() + if (inputValueJavaType != null) { + val genericContext = javaType ?: inputValueJavaType + handleFoundType(typeClassMatcher.match(TypeClassMatcher.PotentialMatch.parameterType( + // a class from the dictionary is for the unwrapped type, not for its list or non-null wrapper + if (propertyType != null) inputValueDefinition.type else inputGraphQLType, + inputValueJavaType, + GenericType(genericContext, options).relativeToType(genericContext), + InputObjectReference(inputValueDefinition) + ))) + } else { + handleInputValueWithoutJavaType(graphQLType, inputValueDefinition, inputGraphQLType, javaType?.unwrap()) } + } } + } + + /** + * Without a Java property for an input field its type can still be built, as long as it doesn't need a class of its own. + * At runtime the value is whatever the input object's class (or the Map) deserializes it into. + */ + private fun handleInputValueWithoutJavaType(graphQLType: InputObjectTypeDefinition, inputValueDefinition: InputValueDefinition, inputGraphQLType: TypeName, clazz: Class<*>?) { + if (clazz != null && !isPropertyMap(clazz)) { + log.warn("Cannot find a public getter or field for field '${inputValueDefinition.name}: ${inputGraphQLType.name}' of input type '${graphQLType.name}' on ${clazz.name}. " + + "Unless Jackson maps it some other way, e.g. through @JsonProperty, check that the property name matches the schema, " + + "otherwise the field's value is rejected or dropped when the input is deserialized.") + } + + when (val definition = definitionsByName[inputGraphQLType.name]) { + is ScalarTypeDefinition -> handleFoundScalarType(definition) + is InputObjectTypeDefinition -> { + val isNewType = !dictionary.containsKey(definition) + // registered without a class, so a class found for it later still gets set and scanned + handleFoundType(definition, null, InputObjectReference(inputValueDefinition)) + if (isNewType) { + handleInputObjectFields(definition, null) + } + } + // the enum class may still be found through another field, so this is only reported once scanning is done + is EnumTypeDefinition -> enumsWithoutClass.putIfAbsent(definition, enumWithoutClassMessage(definition, graphQLType, inputValueDefinition, clazz)) + // undefined types and output types are reported by the SchemaParser + else -> {} + } + } + + private fun isPropertyMap(clazz: Class<*>) = + java.util.Map::class.java.isAssignableFrom(clazz) || JsonNode::class.java.isAssignableFrom(clazz) || clazz == Object::class.java + + private fun isLibraryClass(clazz: Class<*>) = + listOf("java.", "javax.", "kotlin.", "com.fasterxml.jackson.").any { clazz.name.startsWith(it) } + + private fun enumWithoutClassMessage(enum: EnumTypeDefinition, graphQLType: InputObjectTypeDefinition, inputValueDefinition: InputValueDefinition, clazz: Class<*>?): String { + val field = inputValueDefinition.name + val problem = "Enum type '${enum.name}' is used by field '$field' of input type '${graphQLType.name}', but its Java enum can't be determined" + return when { + clazz == null -> "$problem because no class was found for input type '${graphQLType.name}'. " + + "Pass the enum class for type '${enum.name}' in the parser's dictionary, or a class for '${graphQLType.name}' with a public getter or field '$field'." + isPropertyMap(clazz) && isLibraryClass(clazz) -> "$problem because '${graphQLType.name}' is bound to ${clazz.name}, which has no type information for its fields. " + + "Pass the enum class for type '${enum.name}' in the parser's dictionary, or use a class for '${graphQLType.name}' with a public getter or field '$field'." + else -> "$problem because ${clazz.name} has no public getter or field named '$field'. " + + "Add a public getter or field '$field' to the class used for '${graphQLType.name}', or pass the enum class for type '${enum.name}' in the parser's dictionary." } } @@ -425,17 +485,11 @@ internal class SchemaClassScanner( queue.add(QueueItem(graphQLType, javaType)) } - private fun findInputValueType(name: String, inputGraphQLType: TypeName, clazz: Class): JavaType? { - val inputValueType = findInputValueTypeInType(name, clazz) - if (inputValueType != null) { - return inputValueType - } - - return initialDictionary[inputGraphQLType.name]?.get() - } - private fun findInputValueTypeInType(name: String, clazz: Class<*>): JavaType? { - val methods = clazz.methods + // a Map or JsonNode has no properties for the input's fields, only methods that may share their names, e.g. ObjectNode.fields(), + // but a subclass of one may still declare typed getters or fields of its own + val isProperty = { member: Member -> !isPropertyMap(clazz) || !isLibraryClass(member.declaringClass) } + val methods = clazz.methods.filter(isProperty) val (getters, methodsWithParameters) = methods.filter { it.name == name || it.name == "get${name.replaceFirstChar(Char::titlecase)}" @@ -443,7 +497,7 @@ internal class SchemaClassScanner( return getters.find { !it.isSynthetic }?.genericReturnType ?: getters.firstOrNull()?.genericReturnType - ?: clazz.fields.find { it.name == name }?.genericType + ?: clazz.fields.find { it.name == name && isProperty(it) }?.genericType ?: methodsWithParameters.find { !it.isSynthetic }?.genericReturnType ?: methodsWithParameters.firstOrNull()?.genericReturnType } diff --git a/src/main/kotlin/graphql/kickstart/tools/SchemaParser.kt b/src/main/kotlin/graphql/kickstart/tools/SchemaParser.kt index 7417e54c..050ff2a0 100644 --- a/src/main/kotlin/graphql/kickstart/tools/SchemaParser.kt +++ b/src/main/kotlin/graphql/kickstart/tools/SchemaParser.kt @@ -506,8 +506,7 @@ class SchemaParser internal constructor( scalarType } else { if (!allowedTypeReferences.contains(typeDefinition.name)) { - throw SchemaError("Expected type '${typeDefinition.name}' to be a ${expectedType.simpleName}, but it wasn't! " + - "Was a type only permitted for object types incorrectly used as an input type, or vice-versa?") + throw unexpectedTypeError(typeDefinition, expectedType) } inputObjects.find { it.name == typeDefinition.name } ?: GraphQLTypeReference(typeDefinition.name) } @@ -538,8 +537,7 @@ class SchemaParser internal constructor( scalarType } else { if (!allowedTypeReferences.contains(typeDefinition.name)) { - throw SchemaError("Expected type '${typeDefinition.name}' to be a ${expectedType.simpleName}, but it wasn't! " + - "Was a type only permitted for object types incorrectly used as an input type, or vice-versa?") + throw unexpectedTypeError(typeDefinition, expectedType) } val found = inputObjects.filter { it.name == typeDefinition.name } if (found.size == 1) { @@ -565,6 +563,65 @@ class SchemaParser internal constructor( else -> throw SchemaError("Unknown type: $typeDefinition") } + private fun unexpectedTypeError(typeName: TypeName, expectedType: KClass<*>): SchemaError { + val name = typeName.name + val subject = "Type '$name'" + + (findUsage(typeName)?.let { " used by $it" } ?: "") + + (typeName.sourceLocation?.let { " at line ${it.line}, column ${it.column}" + (it.sourceName?.let { source -> " of $source" } ?: "") } ?: "") + // unused ones first, since the extensions of an unused type are still among the definitions + val definition = unusedDefinitions.find { it.name == name } + ?: definitions.filterIsInstance>().find { it.name == name } + val isInput = expectedType == GraphQLInputType::class + val kind = when (definition) { + is InputObjectTypeDefinition -> "an input object type" + is InterfaceTypeDefinition -> "an interface" + is UnionTypeDefinition -> "a union" + else -> "an object type" + } + + return SchemaError(when { + definition == null -> "$subject is not defined in the schema." + isInput && definition !is InputObjectTypeDefinition && definition !is EnumTypeDefinition && definition !is ScalarTypeDefinition -> + "$subject is $kind, which can't be used as an input type. Input types are input objects, enums and scalars." + !isInput && definition is InputObjectTypeDefinition -> + "$subject is $kind, which can't be used as an output type. Output types are object types, interfaces, unions, enums and scalars." + definition in unusedDefinitions -> + "$subject is defined in the schema, but was not reached while scanning the resolvers and the classes they use, so it wasn't built. " + + unreachedTypeHint(definition) + else -> "$subject can't be used as ${if (isInput) "an input" else "an output"} type." + }) + } + + // what makes the scanner reach the type depends on its kind, see SchemaClassScanner.canIncludeUnusedType + private fun unreachedTypeHint(definition: TypeDefinition<*>): String = when (definition) { + is InterfaceTypeDefinition, is UnionTypeDefinition -> + "Return it from a resolver method or one of those classes, or set SchemaParserOptions.includeUnusedTypes." + is EnumTypeDefinition -> + "Use it in a resolver method or one of those classes, or set SchemaParserOptions.includeUnusedTypes and pass its enum class in the parser's dictionary." + is InputObjectTypeDefinition -> + "Use it in an argument of a resolver method, directly or through another input type. SchemaParserOptions.includeUnusedTypes doesn't include input types." + is ScalarTypeDefinition -> + "Use it in a resolver method or one of those classes. SchemaParserOptions.includeUnusedTypes doesn't include scalars." + else -> + "Return it from a resolver method or one of those classes, or set SchemaParserOptions.includeUnusedTypes and pass a class for it in the parser's dictionary." + } + + // determineType only gets the type reference, so the definition using it is looked up for the error message + private fun findUsage(typeName: TypeName): String? = definitions.asSequence().flatMap { definition -> + when (definition) { + is ObjectTypeDefinition -> fieldUsages(definition.name, definition.fieldDefinitions) + is InterfaceTypeDefinition -> fieldUsages(definition.name, definition.fieldDefinitions) + is InputObjectTypeDefinition -> definition.inputValueDefinitions.map { "input field '${definition.name}.${it.name}'" to it.type } + is DirectiveDefinition -> definition.inputValueDefinitions.map { "argument '${it.name}' of directive '@${definition.name}'" to it.type } + else -> emptyList() + } + }.find { (_, type) -> type.unwrap() === typeName }?.first + + private fun fieldUsages(typeName: String, fields: List): List>> = fields.flatMap { field -> + listOf("field '$typeName.${field.name}'" to field.type) + + field.inputValueDefinitions.map { "argument '${it.name}' of field '$typeName.${field.name}'" to it.type } + } + /** * Returns an optional [String] describing a deprecated field/enum. * If a deprecation directive was defined using the @deprecated directive, diff --git a/src/test/kotlin/graphql/kickstart/tools/NestedInputTypesTest.kt b/src/test/kotlin/graphql/kickstart/tools/NestedInputTypesTest.kt index 2079cb91..f2b093c4 100644 --- a/src/test/kotlin/graphql/kickstart/tools/NestedInputTypesTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/NestedInputTypesTest.kt @@ -1,5 +1,7 @@ package graphql.kickstart.tools +import com.fasterxml.jackson.annotation.JsonProperty +import com.fasterxml.jackson.databind.node.ObjectNode import graphql.GraphQL import graphql.execution.AsyncExecutionStrategy import org.junit.Test @@ -103,6 +105,369 @@ class NestedInputTypesTest { assertEquals((data["materials"]), emptyList()) } + @Test + fun `nested input types are found through a Map parameter`() { + val schema = SchemaParser.newParser() + .schemaString( + """ + type Query { + discountName(filter: MaterialFilter!): String + } + + input MaterialFilter { + title: String + requestFilter: RequestFilter + } + + input RequestFilter { + and: [RequestFilter!] + or: [RequestFilter!] + discountTypeFilter: DiscountTypeFilter + } + + input DiscountTypeFilter { + name: String + } + """) + .resolvers(object : GraphQLQueryResolver { + fun discountName(filter: Map): String? { + val requestFilter = filter["requestFilter"] as Map<*, *> + val discountTypeFilter = (requestFilter["and"] as List<*>).single() as Map<*, *> + return (discountTypeFilter["discountTypeFilter"] as Map<*, *>)["name"] as String? + } + }) + .build() + .makeExecutableSchema() + val gql = GraphQL.newGraphQL(schema).build() + + val data = assertNoGraphQlErrors(gql) { + """ + query { + discountName(filter: { requestFilter: { and: [{ discountTypeFilter: { name: "discount" } }] } }) + } + """ + } + + assertEquals(data["discountName"], "discount") + } + + @Test + fun `nested input types are found through ObjectNode parameters`() { + val schema = SchemaParser.newParser() + .schemaString( + """ + type Query { + test: String + } + + type Mutation { + create(input: EntityCreateInput!): String + update(input: EntityUpdateInput!): String + } + + input EntityCreateInput { + name: String + nested: Nested + } + + input EntityUpdateInput { + id: ID! + nested: Nested + } + + input Nested { + x: Int + } + """) + .resolvers(object : GraphQLQueryResolver { + fun test(): String? = null + }, object : GraphQLMutationResolver { + fun create(input: ObjectNode): String = input.toString() + fun update(input: ObjectNode): String = input.toString() + }) + .build() + .makeExecutableSchema() + val gql = GraphQL.newGraphQL(schema).build() + + val data = assertNoGraphQlErrors(gql) { + """ + mutation { + create(input: { name: "n", nested: { x: 1 } }) + update(input: { id: "2", nested: { x: 3 } }) + } + """ + } + + assertEquals(data["create"], """{"name":"n","nested":{"x":1}}""") + assertEquals(data["update"], """{"id":"2","nested":{"x":3}}""") + } + + @Test + fun `nested input types are found when the input class has no matching property`() { + val schema = SchemaParser.newParser() + .schemaString( + """ + type Query { + test: String + } + + type Mutation { + createAgreement(data: AgreementCreateInput!): String + } + + input AgreementCreateInput { + name: String + IncludedAgreement_back: IncludedAgreementCreateManyInput + } + + input IncludedAgreementCreateManyInput { + create: [IncludedAgreementCreateInput!] + } + + input IncludedAgreementCreateInput { + name: String + } + """) + .resolvers(object : GraphQLQueryResolver { + fun test(): String? = null + }, object : GraphQLMutationResolver { + fun createAgreement(data: AgreementCreateInput): String? = + data.getIncludedAgreement_Back()?.create?.joinToString { it.name.orEmpty() } + }) + .build() + .makeExecutableSchema() + val gql = GraphQL.newGraphQL(schema).build() + + val data = assertNoGraphQlErrors(gql) { + """ + mutation { + createAgreement(data: { name: "a", IncludedAgreement_back: { create: [{ name: "b" }, { name: "c" }] } }) + } + """ + } + + assertEquals(data["createAgreement"], "b, c") + } + + @Test + fun `nested input type reached without a class is scanned again once its class is found`() { + val schema = SchemaParser.newParser() + .schemaString( + """ + type Query { + byMap(filter: ColorFilter): String + byClass(color: ColorInput): String + } + + input ColorFilter { + color: ColorInput + } + + input ColorInput { + value: Color + } + + enum Color { + RED + GREEN + } + """) + .resolvers(object : GraphQLQueryResolver { + fun byMap(filter: Map): String = (filter["color"] as Map<*, *>)["value"].toString() + fun byClass(color: ColorInput): String = color.value.toString() + }) + .build() + .makeExecutableSchema() + val gql = GraphQL.newGraphQL(schema).build() + + val data = assertNoGraphQlErrors(gql) { + """ + query { + byMap(filter: { color: { value: RED } }) + byClass(color: { value: GREEN }) + } + """ + } + + assertEquals(data["byMap"], "RED") + assertEquals(data["byClass"], "GREEN") + } + + @Test + fun `list field of a nested input type reached without a class uses the dictionary class of its element`() { + val schema = SchemaParser.newParser() + .schemaString( + """ + type Query { + byMap(filter: ShipmentFilter): String + byClass(shipment: ShipmentInput): String + } + + input ShipmentFilter { + shipment: ShipmentInput + } + + input ShipmentInput { + lines: [ShipmentLineInput!] + } + + input ShipmentLineInput { + sku: String + } + """) + .resolvers(object : GraphQLQueryResolver { + fun byMap(filter: Map): String = filter["shipment"].toString() + fun byClass(shipment: ShipmentInput): String = shipment.lines.orEmpty().joinToString { it.sku.orEmpty() } + }) + .dictionary("ShipmentLineInput", ShipmentLineInput::class.java) + .build() + .makeExecutableSchema() + val gql = GraphQL.newGraphQL(schema).build() + + val data = assertNoGraphQlErrors(gql) { + """ + query { + byMap(filter: { shipment: { lines: [{ sku: "a" }] } }) + byClass(shipment: { lines: [{ sku: "b" }, { sku: "c" }] }) + } + """ + } + + assertEquals(data["byMap"], "{lines=[{sku=a}]}") + assertEquals(data["byClass"], "b, c") + } + + @Test + fun `input fields bound to ObjectNode are not matched against its own methods`() { + val schema = SchemaParser.newParser() + .schemaString( + """ + type Query { + create(input: EntityInput!): String + } + + input EntityInput { + values: [Nested] + elements: [Nested] + fields: [Nested] + } + + input Nested { + x: Int + } + """) + .resolvers(object : GraphQLQueryResolver { + fun create(input: ObjectNode): String = input.toString() + }) + .build() + .makeExecutableSchema() + val gql = GraphQL.newGraphQL(schema).build() + + val data = assertNoGraphQlErrors(gql) { + """ + query { + create(input: { values: [{ x: 1 }], elements: [{ x: 2 }], fields: [{ x: 3 }] }) + } + """ + } + + assertEquals(data["create"], """{"values":[{"x":1}],"elements":[{"x":2}],"fields":[{"x":3}]}""") + } + + @Test + fun `input field types are found through the getters of a Map subclass`() { + val schema = SchemaParser.newParser() + .schemaString( + """ + type Query { + kind(filter: KindFilter!): String + nested(filter: NestedKindFilter!): String + } + + input KindFilter { + kind: Kind + } + + input NestedKindFilter { + inner: InnerKindFilter + } + + input InnerKindFilter { + kind: Kind + } + + enum Kind { + A + B + } + """) + .resolvers(object : GraphQLQueryResolver { + fun kind(filter: KindFilterMap): String = filter.kind.toString() + fun nested(filter: NestedKindFilterMap): String = filter["inner"].toString() + }) + .build() + .makeExecutableSchema() + val gql = GraphQL.newGraphQL(schema).build() + + val data = assertNoGraphQlErrors(gql) { + """ + query { + kind(filter: { kind: B }) + nested(filter: { inner: { kind: A } }) + } + """ + } + + assertEquals(data["kind"], "B") + assertEquals(data["nested"], "{kind=A}") + } + + class KindFilterMap : HashMap() { + val kind: Kind? get() = this["kind"]?.let { Kind.valueOf(it.toString()) } + } + + class NestedKindFilterMap : HashMap() { + val inner: InnerKindFilter? get() = null + } + + class InnerKindFilter { + var kind: Kind? = null + } + + enum class Kind { A, B } + + class AgreementCreateInput { + var name: String? = null + + // the getter doesn't follow the bean naming of the schema field, so only Jackson knows about the property + @field:JsonProperty("IncludedAgreement_back") + private var includedAgreementBack: IncludedAgreementCreateManyInput? = null + + fun getIncludedAgreement_Back(): IncludedAgreementCreateManyInput? = includedAgreementBack + } + + class IncludedAgreementCreateManyInput { + var create: List? = null + } + + class IncludedAgreementCreateInput { + var name: String? = null + } + + class ColorInput { + var value: Color? = null + } + + enum class Color { RED, GREEN } + + class ShipmentInput { + var lines: List? = null + } + + class ShipmentLineInput { + var sku: String? = null + } + class QueryResolver : GraphQLQueryResolver { fun materials(filter: MaterialFilter): List = listOf() } diff --git a/src/test/kotlin/graphql/kickstart/tools/SchemaClassScannerTest.kt b/src/test/kotlin/graphql/kickstart/tools/SchemaClassScannerTest.kt index 24f0e662..2199fd94 100644 --- a/src/test/kotlin/graphql/kickstart/tools/SchemaClassScannerTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/SchemaClassScannerTest.kt @@ -1,6 +1,8 @@ package graphql.kickstart.tools +import graphql.GraphQL import graphql.GraphQLContext +import graphql.Scalars import graphql.execution.CoercedVariables import graphql.language.Value import graphql.schema.* @@ -462,6 +464,53 @@ class SchemaClassScannerTest { var map: Map? = null } + @Test + fun `scanner handles custom scalars only used in input types bound to a Map`() { + val foo = GraphQLScalarType.newScalar(Scalars.GraphQLString) + .name("Foo") + .description("foo") + .build() + + val schema = SchemaParser.newParser() + .resolvers(object : GraphQLQueryResolver { + fun test(): String? = null + }, object : GraphQLMutationResolver { + fun createHuman(input: Map): String = "${input["name"]} ${input["foo"]}" + }) + .scalars(foo) + .schemaString( + """ + scalar Foo + + type Query { + test: String + } + + type Mutation { + createHuman(input: CreateHumanInput!): String + } + + input CreateHumanInput { + name: String + homePlanet: String + foo: Foo + } + """) + .build() + .makeExecutableSchema() + val gql = GraphQL.newGraphQL(schema).build() + + val data = assertNoGraphQlErrors(gql) { + """ + mutation { + createHuman(input: { name: "Luke", foo: "bar" }) + } + """ + } + + assertEquals(data["createHuman"], "Luke bar") + } + @Test fun `scanner allows class to be used for object type and input object type`() { val schema = SchemaParser.newParser() diff --git a/src/test/kotlin/graphql/kickstart/tools/SchemaParserTest.kt b/src/test/kotlin/graphql/kickstart/tools/SchemaParserTest.kt index e5e2ba30..4353764e 100644 --- a/src/test/kotlin/graphql/kickstart/tools/SchemaParserTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/SchemaParserTest.kt @@ -200,24 +200,146 @@ class SchemaParserTest { @Test fun `parser should throw descriptive exception when object is used as input type incorrectly`() { - assertThrows("Was a type only permitted for object types incorrectly used as an input type, or vice-versa", SchemaError::class.java) { + val error = assertThrows(SchemaError::class.java) { SchemaParser.newParser() .schemaString( """ - type Query { - name(filter: Filter): [String] - } - - type Filter { - filter: String - } - """) + |type Query { + | name(filter: Filter): [String] + |} + | + |type Filter { + | filter: String + |} + """.trimMargin()) .resolvers(object : GraphQLQueryResolver { fun name(filter: Filter): List? = null }) .build() .makeExecutableSchema() } + + assertEquals(error.message, "Type 'Filter' used by argument 'filter' of field 'Query.name' at line 2, column 18 is an object type, " + + "which can't be used as an input type. Input types are input objects, enums and scalars.") + } + + @Test + fun `parser should throw descriptive exception when input object is used as output type incorrectly`() { + val error = assertThrows(SchemaError::class.java) { + SchemaParser.newParser() + .schemaString( + """ + |type Query { + | filters: [Filter!] + |} + | + |input Filter { + | filter: String + |} + """.trimMargin()) + .resolvers(object : GraphQLQueryResolver { + fun filters(): List? = null + }) + .build() + .makeExecutableSchema() + } + + assertEquals(error.message, "Type 'Filter' used by field 'Query.filters' at line 2, column 15 is an input object type, " + + "which can't be used as an output type. Output types are object types, interfaces, unions, enums and scalars.") + } + + @Test + fun `parser should throw descriptive exception when input type is not defined`() { + val error = assertThrows(SchemaError::class.java) { + SchemaParser.newParser() + .schemaString( + """ + |type Query { + | test(input: TestInput): String + |} + | + |input TestInput { + | nested: [Nested!] + |} + """.trimMargin()) + .resolvers(object : GraphQLQueryResolver { + fun test(input: Map): String? = null + }) + .build() + .makeExecutableSchema() + } + + assertEquals(error.message, "Type 'Nested' used by input field 'TestInput.nested' at line 6, column 14 is not defined in the schema.") + } + + @Test + fun `parser should throw descriptive exception when type was never reached by the scanner`() { + val error = assertThrows(SchemaError::class.java) { + SchemaParser.newParser() + .file("UnreachedType.graphqls") + .resolvers(object : GraphQLQueryResolver { + fun pet(): Pet? = null + }) + .build() + .makeExecutableSchema() + } + + assertEquals(error.message, "Type 'Cat' used by field 'Pet.friend' at line 6, column 11 of UnreachedType.graphqls is defined in the schema, " + + "but was not reached while scanning the resolvers and the classes they use, so it wasn't built. " + + "Return it from a resolver method or one of those classes, or set SchemaParserOptions.includeUnusedTypes and pass a class for it in the parser's dictionary.") + } + + @Test + fun `parser should build type that was never reached by the scanner when including unused types`() { + SchemaParser.newParser() + .file("UnreachedType.graphqls") + .resolvers(object : GraphQLQueryResolver { + fun pet(): Pet? = null + }) + .dictionary(Cat::class) + .options(SchemaParserOptions.newOptions().includeUnusedTypes(true).build()) + .build() + .makeExecutableSchema() + } + + @Test + fun `parser should throw descriptive exception when input type is only used by an unused interface`() { + val error = assertThrows(SchemaError::class.java) { + SchemaParser.newParser() + .schemaString( + """ + |type Query { + | name: String + |} + | + |interface Searchable { + | search(criteria: Criteria): String + |} + | + |input Criteria { + | text: String + |} + """.trimMargin()) + .resolvers(object : GraphQLQueryResolver { + fun name(): String? = null + }) + .options(SchemaParserOptions.newOptions().includeUnusedTypes(true).build()) + .build() + .makeExecutableSchema() + } + + assertEquals(error.message, "Type 'Criteria' used by argument 'criteria' of field 'Searchable.search' at line 6, column 22 is defined in the schema, " + + "but was not reached while scanning the resolvers and the classes they use, so it wasn't built. " + + "Use it in an argument of a resolver method, directly or through another input type. " + + "SchemaParserOptions.includeUnusedTypes doesn't include input types.") + } + + interface Pet { + val friend: Cat? + } + + class Cat { + var name: String? = null } @Test @@ -356,6 +478,123 @@ class SchemaParserTest { .makeExecutableSchema() } + @Test + fun `support enum types used in input Map if their class is found elsewhere`() { + SchemaParser.newParser() + .schemaString( + """ + type Query { + save(input: SaveInput!): Boolean + type: EnumType + } + + input SaveInput { + type: EnumType! + } + + enum EnumType { + TEST + } + """) + .resolvers(object : GraphQLQueryResolver { + fun save(input: Map<*, *>): Boolean = false + fun type(): EnumType? = null + }) + .build() + .makeExecutableSchema() + } + + @Test + fun `parser should throw descriptive exception when enum is only used in input Map without a class`() { + val error = assertThrows(SchemaClassScannerError::class.java) { + SchemaParser.newParser() + .schemaString( + """ + type Query { + save(input: SaveInput!): Boolean + } + + input SaveInput { + type: EnumType! + } + + enum EnumType { + TEST + } + """) + .resolvers(object : GraphQLQueryResolver { + fun save(input: Map<*, *>): Boolean = false + }) + .build() + } + + assertEquals(error.message, "Enum type 'EnumType' is used by field 'type' of input type 'SaveInput', but its Java enum can't be determined " + + "because 'SaveInput' is bound to java.util.Map, which has no type information for its fields. Pass the enum class for type " + + "'EnumType' in the parser's dictionary, or use a class for 'SaveInput' with a public getter or field 'type'.") + } + + @Test + fun `parser should throw descriptive exception when enum is only used in input Map subclass without a getter for it`() { + val error = assertThrows(SchemaClassScannerError::class.java) { + SchemaParser.newParser() + .schemaString( + """ + type Query { + save(input: SaveInput!): Boolean + } + + input SaveInput { + type: EnumType! + } + + enum EnumType { + TEST + } + """) + .resolvers(object : GraphQLQueryResolver { + fun save(input: SaveInputMap): Boolean = false + }) + .build() + } + + assertEquals(error.message, "Enum type 'EnumType' is used by field 'type' of input type 'SaveInput', but its Java enum can't be determined " + + "because ${SaveInputMap::class.java.name} has no public getter or field named 'type'. Add a public getter or field 'type' to the class " + + "used for 'SaveInput', or pass the enum class for type 'EnumType' in the parser's dictionary.") + } + + @Test + fun `parser should throw descriptive exception when enum is only used in a nested input without a class`() { + val error = assertThrows(SchemaClassScannerError::class.java) { + SchemaParser.newParser() + .schemaString( + """ + type Query { + save(input: SaveInput!): Boolean + } + + input SaveInput { + nested: NestedInput + } + + input NestedInput { + type: EnumType! + } + + enum EnumType { + TEST + } + """) + .resolvers(object : GraphQLQueryResolver { + fun save(input: Map<*, *>): Boolean = false + }) + .build() + } + + assertEquals(error.message, "Enum type 'EnumType' is used by field 'type' of input type 'NestedInput', but its Java enum can't be determined " + + "because no class was found for input type 'NestedInput'. Pass the enum class for type 'EnumType' in the parser's dictionary, " + + "or a class for 'NestedInput' with a public getter or field 'type'.") + } + @Test fun `allow circular relations in input objects`() { SchemaParser.newParser() @@ -682,6 +921,8 @@ class SchemaParserTest { TEST } + class SaveInputMap : HashMap() + class QueryWithIdResolver : GraphQLQueryResolver { fun getId(): String? = null } diff --git a/src/test/resources/UnreachedType.graphqls b/src/test/resources/UnreachedType.graphqls new file mode 100644 index 00000000..d6fe977f --- /dev/null +++ b/src/test/resources/UnreachedType.graphqls @@ -0,0 +1,11 @@ +type Query { + pet: Pet +} + +interface Pet { + friend: Cat +} + +type Cat { + name: String +} From 04ccd9c4bbc96efa95c3632667c6b2c2044eacd5 Mon Sep 17 00:00:00 2001 From: Oryan Date: Mon, 12 Oct 2026 15:23:34 -0400 Subject: [PATCH 2/3] Let input types share a dictionary class SchemaParserDictionary stored its entries in a BiMap, so mapping two type names to one class, e.g. two input types to ObjectNode or Map, threw "value already present" as soon as the second was added. Input types never go into the scanner's type => class bimap, so nothing after that needs their classes to be unique. Directive input types still need a dictionary entry, and entries for nested inputs under a Map or ObjectNode are a common workaround, so both hit this. The dictionary is now a plain map. Every other kind of type still needs a class of its own, but the scanner checks that when the schema is built and names the class and the types sharing it. getDictionary() keeps its signature, since graphql-spring-boot passes its result to the builder. If several types share a class, the inverse of the returned copy only maps it back to the first of them. Names other than input types that share a class now fail in build() with a SchemaClassScannerError, instead of in dictionary() with an IllegalArgumentException. Co-Authored-By: Claude Opus 5.5 --- .../kickstart/tools/SchemaClassScanner.kt | 13 +- .../kickstart/tools/SchemaParserDictionary.kt | 9 +- .../graphql/kickstart/tools/util/BiMap.java | 12 + .../tools/SchemaParserDictionaryTest.kt | 278 ++++++++++++++++++ .../graphql/kickstart/tools/util/BiMapTest.kt | 15 + 5 files changed, 324 insertions(+), 3 deletions(-) create mode 100644 src/test/kotlin/graphql/kickstart/tools/SchemaParserDictionaryTest.kt diff --git a/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt b/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt index 585a7092..91cd8ed3 100644 --- a/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt +++ b/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt @@ -16,7 +16,7 @@ import java.lang.reflect.Method * @author Andrew Potter */ internal class SchemaClassScanner( - initialDictionary: BiMap>, + initialDictionary: Map>, allDefinitions: List>, resolvers: List>, private val scalars: CustomScalarMap, @@ -59,6 +59,17 @@ internal class SchemaClassScanner( } } + // Input types never go into the type => class bimap, so several of them can share a class, e.g. ObjectNode. + // Other types need one each. + initialDictionary + .filterKeys { definitionsByName[it] !is InputObjectTypeDefinition } + .entries.groupBy({ it.value }, { it.key }) + .filterValues { it.size > 1 } + .entries.firstOrNull() + ?.let { (clazz, names) -> + throw SchemaClassScannerError("Class in supplied dictionary '${clazz.name}' specified type names ${names.joinToString { "'$it'" }}, but only input object types can share a class!") + } + if (options.allowUnimplementedResolvers) { log.warn("Option 'allowUnimplementedResolvers' should only be set to true during development, as it can cause schema errors to be moved to query time instead of schema creation time. Make sure this is turned off in production.") } diff --git a/src/main/kotlin/graphql/kickstart/tools/SchemaParserDictionary.kt b/src/main/kotlin/graphql/kickstart/tools/SchemaParserDictionary.kt index 116fa7ca..1fc10eeb 100644 --- a/src/main/kotlin/graphql/kickstart/tools/SchemaParserDictionary.kt +++ b/src/main/kotlin/graphql/kickstart/tools/SchemaParserDictionary.kt @@ -5,9 +5,14 @@ import kotlin.reflect.KClass class SchemaParserDictionary { - private val dictionary: BiMap> = BiMap.create() + // Not a BiMap, since several input types can share a class, e.g. ObjectNode. The scanner checks that other types don't. + private val dictionary = linkedMapOf>() - fun getDictionary(): BiMap> = BiMap.unmodifiableBiMap(dictionary) + /** + * Returns a copy of the entries added so far. If several types share a class, the inverse of the copy only maps that class + * back to the first of them. + */ + fun getDictionary(): BiMap> = BiMap.unmodifiableCopyOf(dictionary) /** * Add arbitrary classes to the parser's dictionary, overriding the generated type name. diff --git a/src/main/kotlin/graphql/kickstart/tools/util/BiMap.java b/src/main/kotlin/graphql/kickstart/tools/util/BiMap.java index 230f995c..e1ec9603 100644 --- a/src/main/kotlin/graphql/kickstart/tools/util/BiMap.java +++ b/src/main/kotlin/graphql/kickstart/tools/util/BiMap.java @@ -1,6 +1,7 @@ package graphql.kickstart.tools.util; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.Map; import java.util.Objects; import java.util.Set; @@ -21,6 +22,17 @@ public static BiMap unmodifiableBiMap(BiMap biMap) { return new BiMap<>(unmodifiableMap(biMap.delegate), unmodifiableMap(biMap.inverse)); } + /** + * Returns an unmodifiable bimap with the entries of the given map. Unlike {@link #put(Object, Object)}, several keys may + * have the same value. The inverse only maps such a value back to the first of its keys, so the inverse and + * {@link #values()} can have fewer entries than the bimap. + */ + public static BiMap unmodifiableCopyOf(Map map) { + Map inverse = new HashMap<>(); + map.forEach((key, value) -> inverse.putIfAbsent(value, key)); + return new BiMap<>(unmodifiableMap(new LinkedHashMap<>(map)), unmodifiableMap(inverse)); + } + private BiMap(Map delegate, Map inverse) { this.delegate = delegate; this.inverse = inverse; diff --git a/src/test/kotlin/graphql/kickstart/tools/SchemaParserDictionaryTest.kt b/src/test/kotlin/graphql/kickstart/tools/SchemaParserDictionaryTest.kt new file mode 100644 index 00000000..9629e9c3 --- /dev/null +++ b/src/test/kotlin/graphql/kickstart/tools/SchemaParserDictionaryTest.kt @@ -0,0 +1,278 @@ +package graphql.kickstart.tools + +import com.fasterxml.jackson.databind.node.ObjectNode +import graphql.GraphQL +import org.junit.Assert.assertThrows +import org.junit.Test + +class SchemaParserDictionaryTest { + + private val entitySchema = """ + type Query { + test: String + } + + type Mutation { + create(input: EntityCreateInput!): String + update(input: EntityUpdateInput!): String + } + + input EntityCreateInput { + name: String + nested: Nested + } + + input EntityUpdateInput { + id: ID! + nested: Nested + } + + input Nested { + x: Int + } + """ + + private val directiveSchema = """ + directive @a(arg: AInput) on FIELD_DEFINITION + directive @b(arg: BInput) on FIELD_DEFINITION + + type Query { + test: String @a(arg: { x: 1 }) @b(arg: { y: "b" }) + } + + input AInput { + x: Int + } + + input BInput { + y: String + } + """ + + @Test + fun `input types can share a dictionary class`() { + val schema = SchemaParser.newParser() + .schemaString(entitySchema) + .resolvers(EntityQuery(), EntityMutation()) + .dictionary("EntityCreateInput", ObjectNode::class.java) + .dictionary("EntityUpdateInput", ObjectNode::class.java) + .build() + .makeExecutableSchema() + val gql = GraphQL.newGraphQL(schema).build() + + val data = assertNoGraphQlErrors(gql) { + """ + mutation { + create(input: { name: "n", nested: { x: 1 } }) + update(input: { id: "2", nested: { x: 3 } }) + } + """ + } + + assertEquals(data["create"], """{"name":"n","nested":{"x":1}}""") + assertEquals(data["update"], """{"id":"2","nested":{"x":3}}""") + } + + @Test + fun `nested input types can share a dictionary class`() { + val schema = SchemaParser.newParser() + .schemaString( + """ + type Query { + create(input: EntityInput!): String + } + + input EntityInput { + name: String + a: NestedA + b: [NestedB!] + } + + input NestedA { + x: Int + } + + input NestedB { + y: Int + } + """) + .resolvers(object : GraphQLQueryResolver { + fun create(input: ObjectNode): String = input.toString() + }) + .dictionary("NestedA", ObjectNode::class.java) + .dictionary("NestedB", ObjectNode::class.java) + .build() + .makeExecutableSchema() + val gql = GraphQL.newGraphQL(schema).build() + + val data = assertNoGraphQlErrors(gql) { + """ + query { + create(input: { name: "n", a: { x: 1 }, b: [{ y: 2 }] }) + } + """ + } + + assertEquals(data["create"], """{"name":"n","a":{"x":1},"b":[{"y":2}]}""") + } + + @Test + fun `directive input types can share a dictionary class`() { + val schema = SchemaParser.newParser() + .schemaString(directiveSchema) + .resolvers(DirectiveQuery()) + .dictionary("AInput", Map::class.java) + .dictionary("BInput", Map::class.java) + .build() + .makeExecutableSchema() + + val field = schema.queryType.getFieldDefinition("test") + assertEquals(field.getAppliedDirective("a").getArgument("arg")!!.getValue>(), mapOf("x" to 1)) + assertEquals(field.getAppliedDirective("b").getArgument("arg")!!.getValue>(), mapOf("y" to "b")) + + val gql = GraphQL.newGraphQL(schema).build() + val data = assertNoGraphQlErrors(gql) { + """ + query { + test + } + """ + } + + assertEquals(data["test"], "test") + } + + @Test + fun `input types sharing a class can be passed to the builder from a SchemaParserDictionary`() { + val dictionary = SchemaParserDictionary() + .add("AInput", Map::class) + .add("BInput", Map::class) + + assertEquals(dictionary.getDictionary().toMap(), mapOf("AInput" to Map::class.java, "BInput" to Map::class.java)) + + // graphql-spring-boot reads a SchemaParserDictionary bean this way + val schema = SchemaParser.newParser() + .schemaString(directiveSchema) + .resolvers(DirectiveQuery()) + .dictionary(dictionary.getDictionary()) + .build() + .makeExecutableSchema() + + val field = schema.queryType.getFieldDefinition("test") + assertEquals(field.getAppliedDirective("b").getArgument("arg")!!.getValue>(), mapOf("y" to "b")) + } + + @Test + fun `input type and object type can share a dictionary class`() { + val schema = SchemaParser.newParser() + .schemaString( + """ + type Query { + node: Node + } + + type Mutation { + save(item: ItemInput!): Node + } + + interface Node { + name: String + } + + type Item implements Node { + name: String + } + + input ItemInput { + name: String + } + """) + .resolvers(object : GraphQLQueryResolver { + fun node(): Node = Item().apply { name = "a" } + }, object : GraphQLMutationResolver { + fun save(item: Item): Node = item + }) + .dictionary("Item", Item::class.java) + .dictionary("ItemInput", Item::class.java) + .build() + .makeExecutableSchema() + val gql = GraphQL.newGraphQL(schema).build() + + val queryData = assertNoGraphQlErrors(gql) { + """ + query { + node { + __typename + name + } + } + """ + } + val mutationData = assertNoGraphQlErrors(gql) { + """ + mutation { + save(item: { name: "b" }) { + __typename + name + } + } + """ + } + + assertEquals(queryData["node"], mapOf("__typename" to "Item", "name" to "a")) + assertEquals(mutationData["save"], mapOf("__typename" to "Item", "name" to "b")) + } + + @Test + fun `object types can't share a dictionary class`() { + val error = assertThrows(SchemaClassScannerError::class.java) { + SchemaParser.newParser() + .schemaString( + """ + type Query { + a: A + b: B + } + + type A { + name: String + } + + type B { + name: String + } + """) + .resolvers(object : GraphQLQueryResolver { + fun a(): Item? = null + fun b(): Item? = null + }) + .dictionary("A", Item::class.java) + .dictionary("B", Item::class.java) + .build() + } + + assertEquals(error.message, "Class in supplied dictionary '${Item::class.java.name}' specified type names 'A', 'B', " + + "but only input object types can share a class!") + } + + class EntityQuery : GraphQLQueryResolver { + fun test(): String? = null + } + + class EntityMutation : GraphQLMutationResolver { + fun create(input: ObjectNode): String = input.toString() + fun update(input: ObjectNode): String = input.toString() + } + + class DirectiveQuery : GraphQLQueryResolver { + fun test(): String = "test" + } + + interface Node { + val name: String? + } + + class Item : Node { + override var name: String? = null + } +} diff --git a/src/test/kotlin/graphql/kickstart/tools/util/BiMapTest.kt b/src/test/kotlin/graphql/kickstart/tools/util/BiMapTest.kt index fc753990..fdc56f30 100644 --- a/src/test/kotlin/graphql/kickstart/tools/util/BiMapTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/util/BiMapTest.kt @@ -67,4 +67,19 @@ class BiMapTest { val expected = setOf(1, 2, 3) assertEquals(expected, values) } + + @Test + fun `unmodifiable copy should keep keys with the same value`() { + val copy = BiMap.unmodifiableCopyOf(linkedMapOf("foo" to 1, "bar" to 2, "baz" to 1)) + + assertEquals(mapOf("foo" to 1, "bar" to 2, "baz" to 1), copy.toMap()) + assertEquals(listOf("foo", "bar", "baz"), copy.keys.toList()) + assertEquals(setOf(1, 2), copy.values) + assertEquals(mapOf(1 to "foo", 2 to "bar"), copy.inverse().toMap()) + } + + @Test(expected = UnsupportedOperationException::class) + fun `unmodifiable copy should not permit changes`() { + BiMap.unmodifiableCopyOf(mapOf("foo" to 1))["bar"] = 2 + } } From 5a8aa0543f7018ff3a4143495e07a44d0febef3d Mon Sep 17 00:00:00 2001 From: Oryan Date: Mon, 12 Oct 2026 15:29:10 -0400 Subject: [PATCH 3/3] Tighten the shared dictionary class tests The test for an input type and an object type sharing a class bound the input type through a resolver parameter, so its dictionary entry was never read. It now only uses the input type in a directive, which needs the entry. The test for object types sharing a class also maps an input type to that class, to check it is left out of the error. The scanner comment no longer says every other kind of type needs a class of its own. Scalars don't, but they keep the old rule. Co-Authored-By: Claude Opus 5.5 --- .../kickstart/tools/SchemaClassScanner.kt | 5 ++- .../tools/SchemaParserDictionaryTest.kt | 37 ++++++++----------- 2 files changed, 19 insertions(+), 23 deletions(-) diff --git a/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt b/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt index 91cd8ed3..5b081e2d 100644 --- a/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt +++ b/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt @@ -59,8 +59,9 @@ internal class SchemaClassScanner( } } - // Input types never go into the type => class bimap, so several of them can share a class, e.g. ObjectNode. - // Other types need one each. + // Input types never go into the type => class bimap, so several of them can share a class, e.g. ObjectNode. Every + // other kind still gets one class per type, as before. Most of them end up in the bimap, which interface and union + // type resolution use to find the type of an object. initialDictionary .filterKeys { definitionsByName[it] !is InputObjectTypeDefinition } .entries.groupBy({ it.value }, { it.key }) diff --git a/src/test/kotlin/graphql/kickstart/tools/SchemaParserDictionaryTest.kt b/src/test/kotlin/graphql/kickstart/tools/SchemaParserDictionaryTest.kt index 9629e9c3..625decea 100644 --- a/src/test/kotlin/graphql/kickstart/tools/SchemaParserDictionaryTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/SchemaParserDictionaryTest.kt @@ -164,15 +164,14 @@ class SchemaParserDictionaryTest { @Test fun `input type and object type can share a dictionary class`() { + // ItemInput is only used by a directive, so its class can only come from the dictionary val schema = SchemaParser.newParser() .schemaString( """ - type Query { - node: Node - } + directive @d(arg: ItemInput) on FIELD_DEFINITION - type Mutation { - save(item: ItemInput!): Node + type Query { + node: Node @d(arg: { name: "z" }) } interface Node { @@ -189,16 +188,17 @@ class SchemaParserDictionaryTest { """) .resolvers(object : GraphQLQueryResolver { fun node(): Node = Item().apply { name = "a" } - }, object : GraphQLMutationResolver { - fun save(item: Item): Node = item }) .dictionary("Item", Item::class.java) .dictionary("ItemInput", Item::class.java) .build() .makeExecutableSchema() - val gql = GraphQL.newGraphQL(schema).build() - val queryData = assertNoGraphQlErrors(gql) { + val field = schema.queryType.getFieldDefinition("node") + assertEquals(field.getAppliedDirective("d").getArgument("arg")!!.getValue>(), mapOf("name" to "z")) + + val gql = GraphQL.newGraphQL(schema).build() + val data = assertNoGraphQlErrors(gql) { """ query { node { @@ -208,19 +208,8 @@ class SchemaParserDictionaryTest { } """ } - val mutationData = assertNoGraphQlErrors(gql) { - """ - mutation { - save(item: { name: "b" }) { - __typename - name - } - } - """ - } - assertEquals(queryData["node"], mapOf("__typename" to "Item", "name" to "a")) - assertEquals(mutationData["save"], mapOf("__typename" to "Item", "name" to "b")) + assertEquals(data["node"], mapOf("__typename" to "Item", "name" to "a")) } @Test @@ -241,16 +230,22 @@ class SchemaParserDictionaryTest { type B { name: String } + + input AInput { + name: String + } """) .resolvers(object : GraphQLQueryResolver { fun a(): Item? = null fun b(): Item? = null }) .dictionary("A", Item::class.java) + .dictionary("AInput", Item::class.java) .dictionary("B", Item::class.java) .build() } + // the input type sharing the class isn't part of the conflict assertEquals(error.message, "Class in supplied dictionary '${Item::class.java.name}' specified type names 'A', 'B', " + "but only input object types can share a class!") }