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 82b19f5acb42ffce106becfc322e8e0b42db7523 Mon Sep 17 00:00:00 2001 From: Oryan Date: Mon, 12 Oct 2026 15:52:04 -0400 Subject: [PATCH 2/3] Check input object classes against the schema with Jackson A misspelled property on an input class, like `technolgies` for `technologies` in #77, only showed up once a client sent the field: Jackson then failed to convert the argument, or dropped the value if the ObjectMapper ignores unknown properties. The scanner's getter lookup can't tell, since Jackson also maps setters, creators, naming strategies, aliases and any setters. Now the SchemaParser asks Jackson. For each resolver argument whose type is an input object, it looks up the deserializer that the field's own ObjectMapper converts the argument with, and checks that it has a property for every field of the built input object type, through lists, arrays, Optionals and nested input objects. A field it doesn't have is read into a placeholder object with unknown properties made to fail, so ignored names, any setters and ignoreUnknown count as mapped without running the class's constructors or setters. - New SchemaParserOptions.unmappedInputFields: WARN by default, FAIL throws one SchemaError listing every field, IGNORE skips the check - MethodFieldResolver keeps its ObjectMapper, so the check and the conversion use the same one - The scanner's "Cannot find a public getter or field" warning is now debug, since it fired for setter-only classes and similar Co-Authored-By: Claude Opus 5.5 --- .../kickstart/tools/SchemaClassScanner.kt | 7 +- .../graphql/kickstart/tools/SchemaParser.kt | 21 +- .../kickstart/tools/SchemaParserOptions.kt | 25 +- .../tools/UnmappedInputFieldChecker.kt | 162 +++ .../kickstart/tools/UnmappedInputFields.kt | 16 + .../tools/resolver/MethodFieldResolver.kt | 9 +- .../tools/UnmappedInputFieldsFixtures.java | 228 +++++ .../tools/UnmappedInputFieldsTest.kt | 965 ++++++++++++++++++ 8 files changed, 1422 insertions(+), 11 deletions(-) create mode 100644 src/main/kotlin/graphql/kickstart/tools/UnmappedInputFieldChecker.kt create mode 100644 src/main/kotlin/graphql/kickstart/tools/UnmappedInputFields.kt create mode 100644 src/test/kotlin/graphql/kickstart/tools/UnmappedInputFieldsFixtures.java create mode 100644 src/test/kotlin/graphql/kickstart/tools/UnmappedInputFieldsTest.kt diff --git a/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt b/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt index 585a7092..e4b3a2b9 100644 --- a/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt +++ b/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt @@ -459,9 +459,10 @@ internal class SchemaClassScanner( */ 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.") + // not a warning, since Jackson may map the field some other way, e.g. through a setter. The SchemaParser checks that with + // Jackson itself, see UnmappedInputFields + log.debug("Cannot find a public getter or field for field '${inputValueDefinition.name}: ${inputGraphQLType.name}' of input type '${graphQLType.name}' on ${clazz.name}, " + + "so its type is taken from the schema.") } when (val definition = definitionsByName[inputGraphQLType.name]) { diff --git a/src/main/kotlin/graphql/kickstart/tools/SchemaParser.kt b/src/main/kotlin/graphql/kickstart/tools/SchemaParser.kt index 050ff2a0..674f206a 100644 --- a/src/main/kotlin/graphql/kickstart/tools/SchemaParser.kt +++ b/src/main/kotlin/graphql/kickstart/tools/SchemaParser.kt @@ -5,6 +5,7 @@ import graphql.Scalars import graphql.introspection.Introspection import graphql.introspection.Introspection.DirectiveLocation.INPUT_FIELD_DEFINITION import graphql.kickstart.tools.directive.DirectiveWiringHelper +import graphql.kickstart.tools.resolver.MethodFieldResolver import graphql.kickstart.tools.util.getDocumentation import graphql.kickstart.tools.util.getExtendedFieldDefinitions import graphql.kickstart.tools.util.unwrap @@ -82,7 +83,10 @@ class SchemaParser internal constructor( } } val interfaces = interfaceDefinitions.map { createInterfaceObject(it, inputObjects) } - val objects = objectDefinitions.map { createObject(it, interfaces, inputObjects) } + val unmappedInputFieldChecker = if (options.unmappedInputFields == UnmappedInputFields.IGNORE) null + else UnmappedInputFieldChecker(inputObjects.associateBy { it.name }, options.unmappedInputFields) + val objects = objectDefinitions.map { createObject(it, interfaces, inputObjects, unmappedInputFieldChecker) } + unmappedInputFieldChecker?.report() val unions = unionDefinitions.map { createUnionObject(it, objects) } val enums = enumDefinitions.map { createEnumObject(it) } @@ -122,7 +126,12 @@ class SchemaParser internal constructor( @Suppress("unused") fun getUnusedDefinitions(): Set> = unusedDefinitions - private fun createObject(objectDefinition: ObjectTypeDefinition, interfaces: List, inputObjects: List): GraphQLObjectType { + private fun createObject( + objectDefinition: ObjectTypeDefinition, + interfaces: List, + inputObjects: List, + unmappedInputFieldChecker: UnmappedInputFieldChecker? + ): GraphQLObjectType { val name = objectDefinition.name val builder = GraphQLObjectType.newObject() .name(name) @@ -141,11 +150,15 @@ class SchemaParser internal constructor( objectDefinition.getExtendedFieldDefinitions(extensionDefinitions).forEach { fieldDefinition -> field { field -> createField(field, fieldDefinition, inputObjects) + val fieldResolver = fieldResolversByType[objectDefinition]?.get(fieldDefinition) + ?: throw SchemaError("No resolver method found for object type '${objectDefinition.name}' and field '${fieldDefinition.name}', this is most likely a bug with graphql-java-tools") codeRegistryBuilder.dataFetcher( FieldCoordinates.coordinates(objectDefinition.name, fieldDefinition.name), - fieldResolversByType[objectDefinition]?.get(fieldDefinition)?.createDataFetcher() - ?: throw SchemaError("No resolver method found for object type '${objectDefinition.name}' and field '${fieldDefinition.name}', this is most likely a bug with graphql-java-tools") + fieldResolver.createDataFetcher() ) + if (fieldResolver is MethodFieldResolver) { + unmappedInputFieldChecker?.check(objectDefinition.name, fieldResolver) + } val wiredField = field.build() GraphQLFieldDefinition.Builder(wiredField) diff --git a/src/main/kotlin/graphql/kickstart/tools/SchemaParserOptions.kt b/src/main/kotlin/graphql/kickstart/tools/SchemaParserOptions.kt index 8b0e0c60..20cb6fed 100644 --- a/src/main/kotlin/graphql/kickstart/tools/SchemaParserOptions.kt +++ b/src/main/kotlin/graphql/kickstart/tools/SchemaParserOptions.kt @@ -38,7 +38,8 @@ data class SchemaParserOptions internal constructor( val fieldVisibility: GraphqlFieldVisibility?, val includeUnusedTypes: Boolean, val useCommentsForDescriptions: Boolean, - val allowUndeclaredDirectives: Boolean + val allowUndeclaredDirectives: Boolean, + val unmappedInputFields: UnmappedInputFields ) { companion object { @JvmStatic @@ -70,6 +71,7 @@ data class SchemaParserOptions internal constructor( private var includeUnusedTypes = false private var useCommentsForDescriptions = true private var allowUndeclaredDirectives = false + private var unmappedInputFields = UnmappedInputFields.WARN fun contextClass(contextClass: Class<*>) = this.apply { this.contextClass = contextClass @@ -164,6 +166,24 @@ data class SchemaParserOptions internal constructor( this.allowUndeclaredDirectives = allowUndeclaredDirectives } + /** + * What to do about input object fields that the class of a resolver argument has no property for. Jackson fails to convert + * the argument when such a field is sent, or drops the field's value if the ObjectMapper ignores unknown properties. + * Defaults to [UnmappedInputFields.WARN]. + * + * The classes are checked when the schema is built, with the deserializer of each resolver field's own ObjectMapper, so + * anything that maps a property for Jackson counts, e.g. setters, creators, naming strategies, aliases, ignored properties + * and any setters. A field is still reported if the ObjectMapper ignores unknown properties, since that's usually set for + * every class at once, but not if its class has `@JsonIgnoreProperties(ignoreUnknown = true)`. + * + * Classes that Jackson doesn't deserialize property by property aren't checked: those with a custom deserializer, a + * delegating creator, a builder or type info, and those with injected values. Neither are classes with `@JsonUnwrapped` + * properties, whose unknown fields Jackson drops, nor arguments bound to a Map, a JsonNode or a type variable. + */ + fun unmappedInputFields(unmappedInputFields: UnmappedInputFields) = this.apply { + this.unmappedInputFields = unmappedInputFields + } + @ExperimentalCoroutinesApi fun build(): SchemaParserOptions { val coroutineContextProvider = coroutineContextProvider @@ -206,7 +226,8 @@ data class SchemaParserOptions internal constructor( fieldVisibility, includeUnusedTypes, useCommentsForDescriptions, - allowUndeclaredDirectives + allowUndeclaredDirectives, + unmappedInputFields ) } } diff --git a/src/main/kotlin/graphql/kickstart/tools/UnmappedInputFieldChecker.kt b/src/main/kotlin/graphql/kickstart/tools/UnmappedInputFieldChecker.kt new file mode 100644 index 00000000..dce46d16 --- /dev/null +++ b/src/main/kotlin/graphql/kickstart/tools/UnmappedInputFieldChecker.kt @@ -0,0 +1,162 @@ +package graphql.kickstart.tools + +import com.fasterxml.jackson.databind.DeserializationContext +import com.fasterxml.jackson.databind.DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES +import com.fasterxml.jackson.databind.JsonDeserializer +import com.fasterxml.jackson.databind.ObjectMapper +import com.fasterxml.jackson.databind.deser.BeanDeserializerBase +import com.fasterxml.jackson.databind.deser.DefaultDeserializationContext +import com.fasterxml.jackson.databind.deser.std.ContainerDeserializerBase +import com.fasterxml.jackson.databind.deser.std.ReferenceTypeDeserializer +import com.fasterxml.jackson.databind.exc.UnrecognizedPropertyException +import graphql.kickstart.tools.resolver.MethodFieldResolver +import graphql.kickstart.tools.util.unwrap +import graphql.language.TypeName +import graphql.schema.GraphQLInputObjectField +import graphql.schema.GraphQLInputObjectType +import graphql.schema.GraphQLNamedType +import graphql.schema.GraphQLType +import graphql.schema.GraphQLTypeUtil +import org.slf4j.LoggerFactory + +/** + * Checks that the classes of resolver arguments have a property for each field of their input objects, see [UnmappedInputFields]. + * + * The fields are looked up in the deserializer that the resolver's ObjectMapper converts the argument with, not in the class + * itself, so that a field counts as mapped whenever Jackson maps it. An input type can be used by fields whose mappers are + * configured differently, so it's checked once for each mapper. + */ +internal class UnmappedInputFieldChecker( + private val inputObjects: Map, + private val unmappedInputFields: UnmappedInputFields +) { + private val log = LoggerFactory.getLogger(javaClass) + + // a deserializer is cached by its mapper, so this is per mapper, class and input type, and ends recursive input types + private val checked = mutableSetOf, String>>() + + // reported once, for the first argument found using it + private val messages = linkedMapOf>, String>() + + fun check(objectType: String, resolver: MethodFieldResolver) { + val mapper = resolver.objectMapper + val context = (mapper.deserializationContext as? DefaultDeserializationContext)?.createDummyInstance(mapper.deserializationConfig) ?: return + + resolver.field.inputValueDefinitions.forEachIndexed { index, argument -> + val inputObject = (argument.type.unwrap() as? TypeName)?.let { inputObjects[it.name] } ?: return@forEachIndexed + val parameterType = resolver.getMethodParameterType(index) ?: return@forEachIndexed + val usage = "argument '${argument.name}' of field '$objectType.${resolver.field.name}'" + + try { + // the same lookup as convertValue's when it converts the argument + val deserializer = context.findRootValueDeserializer(mapper.typeFactory.constructType(parameterType)) + checkFields(mapper, context, deserializer, inputObject, usage) + } catch (e: Exception) { + // e.g. a class Jackson can't deserialize, which only fails once the argument is converted, as it did before this check + log.debug("Can't check the input fields of $usage with Jackson's deserializer for $parameterType", e) + } + } + } + + fun report() { + if (messages.isEmpty()) { + return + } + + if (unmappedInputFields == UnmappedInputFields.FAIL) { + throw SchemaError(messages.values.joinToString("\n")) + } + messages.values.forEach { log.warn(it) } + } + + private fun checkFields( + mapper: ObjectMapper, + context: DefaultDeserializationContext, + deserializer: JsonDeserializer<*>?, + inputObject: GraphQLInputObjectType, + usage: String + ) { + val beanDeserializer = beanDeserializer(context, deserializer) ?: return + if (!checked.add(beanDeserializer to inputObject.name)) { + return + } + + inputObject.fields.forEach { field -> + val property = beanDeserializer.findProperty(field.name) + if (property == null) { + knownPropertiesIfUnmapped(mapper, context, beanDeserializer, field.name)?.let { knownProperties -> + messages.getOrPut(Triple(inputObject.name, field.name, beanDeserializer.handledType())) { + message(mapper, inputObject, field, beanDeserializer.handledType(), usage, knownProperties) + } + } + } else if (property.valueTypeDeserializer == null) { + // a polymorphic property's class is only known once its type id is read + inputObjectOf(field.type)?.let { checkFields(mapper, context, property.valueDeserializer, it, usage) } + } + } + } + + // the deserializer of the class an input object is converted to, past the lists, arrays and Optionals around it + private tailrec fun beanDeserializer(context: DeserializationContext, deserializer: JsonDeserializer<*>?): BeanDeserializerBase? = + when (deserializer) { + // a delegating creator gets the whole input object, e.g. as a Map + is BeanDeserializerBase -> deserializer.takeUnless { it.valueInstantiator.canCreateUsingDelegate() } + // a Map gets the fields as its keys + is ContainerDeserializerBase<*> -> + if (deserializer.valueType.isMapLikeType) null else beanDeserializer(context, deserializer.contentDeserializer) + is ReferenceTypeDeserializer<*> -> + beanDeserializer(context, context.findContextualValueDeserializer(deserializer.valueType.referencedType, null)) + else -> null + } + + /** + * Returns the properties Jackson knows if it fails on the field, or null if it maps or ignores it. + * + * `{"": null}` is read into a placeholder object rather than an instance of the class, so neither its constructors nor + * its setters are called, but the field still goes through Jackson's handling of a property the class doesn't have: ignored + * names, any setters, ignoreUnknown and problem handlers. Unknown properties are made to fail even if the mapper ignores them. + * Any other exception, e.g. from a deserializer that can't read into an existing object, means the field can't be checked. + */ + private fun knownPropertiesIfUnmapped( + mapper: ObjectMapper, + context: DefaultDeserializationContext, + deserializer: BeanDeserializerBase, + field: String + ): Collection? { + val config = mapper.deserializationConfig.with(FAIL_ON_UNKNOWN_PROPERTIES) + return try { + mapper.treeAsTokens(mapper.createObjectNode().putNull(field)).use { parser -> + parser.nextToken() + deserializer.deserialize(parser, context.createInstance(config, parser, mapper.injectableValues), Any()) + } + null + } catch (e: UnrecognizedPropertyException) { + if (e.propertyName == field) e.knownPropertyIds.orEmpty() else null + } catch (e: Exception) { + null + } + } + + private fun inputObjectOf(type: GraphQLType): GraphQLInputObjectType? = + (GraphQLTypeUtil.unwrapAllAs(type) as? GraphQLNamedType)?.let { inputObjects[it.name] } + + private fun message( + mapper: ObjectMapper, + inputObject: GraphQLInputObjectType, + field: GraphQLInputObjectField, + clazz: Class<*>, + usage: String, + knownProperties: Collection + ): String { + val consequence = when { + !mapper.deserializationConfig.isEnabled(FAIL_ON_UNKNOWN_PROPERTIES) -> "drops the field's value, since the ObjectMapper ignores unknown properties" + field.hasSetDefaultValue() -> "fails for every '${inputObject.name}' value sent, since the field has a default value" + else -> "fails whenever the field is sent" + } + val properties = knownProperties.map { it.toString() }.sorted().ifEmpty { listOf("none") } + + return "Input field '${inputObject.name}.${field.name}' has no matching property in ${clazz.name}. Converting $usage $consequence. " + + "Known properties: ${properties.joinToString(", ")}. " + + "Rename the property or map it with @JsonProperty(\"${field.name}\"). If it's left unmapped on purpose, list it in @JsonIgnoreProperties on the class." + } +} diff --git a/src/main/kotlin/graphql/kickstart/tools/UnmappedInputFields.kt b/src/main/kotlin/graphql/kickstart/tools/UnmappedInputFields.kt new file mode 100644 index 00000000..80a4e296 --- /dev/null +++ b/src/main/kotlin/graphql/kickstart/tools/UnmappedInputFields.kt @@ -0,0 +1,16 @@ +package graphql.kickstart.tools + +/** + * What the [SchemaParser] does about input object fields that the class of a resolver argument has no property for, + * see [SchemaParserOptions.Builder.unmappedInputFields]. + */ +enum class UnmappedInputFields { + /** Doesn't check the classes. */ + IGNORE, + + /** Logs a warning for each field. */ + WARN, + + /** Fails with a [SchemaError] that lists every field. */ + FAIL +} diff --git a/src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt b/src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt index 0a9e65d6..14a01d14 100644 --- a/src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt +++ b/src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt @@ -1,6 +1,7 @@ package graphql.kickstart.tools.resolver import com.fasterxml.jackson.core.type.TypeReference +import com.fasterxml.jackson.databind.ObjectMapper import graphql.GraphQLContext import graphql.kickstart.tools.* import graphql.kickstart.tools.SchemaParserOptions.GenericWrapper @@ -49,9 +50,12 @@ internal class MethodFieldResolver( private val numberOfParameters = method.parameterCountWithoutContinuation() private val hasAdditionalParameter = numberOfParameters == (field.inputValueDefinitions.size + getIndexOffset() + 1) + // the arguments are converted with the same ObjectMapper that the UnmappedInputFieldChecker checks their classes with + internal val objectMapper: ObjectMapper by lazy { options.objectMapperProvider.provide(field) } + override fun createDataFetcher(): DataFetcher<*> { val args = mutableListOf() - val mapper = options.objectMapperProvider.provide(field) + val mapper = objectMapper // Add source argument if this is a resolver (but not a root resolver) if (this.search.requiredFirstParameterType != null) { @@ -200,7 +204,8 @@ internal class MethodFieldResolver( private fun getJavaMethodParameterIndex(index: Int) = index + getIndexOffset() - private fun getMethodParameterType(index: Int): JavaType? { + // the type that the value of the argument at the index is converted to + internal fun getMethodParameterType(index: Int): JavaType? { val methodIndex = getJavaMethodParameterIndex(index) val parameters = method.parameterTypes diff --git a/src/test/kotlin/graphql/kickstart/tools/UnmappedInputFieldsFixtures.java b/src/test/kotlin/graphql/kickstart/tools/UnmappedInputFieldsFixtures.java new file mode 100644 index 00000000..9180fd2b --- /dev/null +++ b/src/test/kotlin/graphql/kickstart/tools/UnmappedInputFieldsFixtures.java @@ -0,0 +1,228 @@ +package graphql.kickstart.tools; + +import com.fasterxml.jackson.annotation.JsonAlias; +import com.fasterxml.jackson.annotation.JsonAnySetter; +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonIgnore; +import com.fasterxml.jackson.annotation.JsonIgnoreProperties; +import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.annotation.JsonUnwrapped; + +import java.beans.ConstructorProperties; +import java.util.List; +import java.util.Map; +import java.util.TreeMap; + +// Input classes as they're written in Java, by hand or with Lombok, so their properties aren't named by the Kotlin module. +public class UnmappedInputFieldsFixtures { + + // the class from https://github.com/graphql-java-kickstart/graphql-java-tools/issues/77#issuecomment-662855626 + public static class ProjectInput { + private Integer id; + private String name; + private String description; + private List technolgies; + private List profiles; + + public Integer getId() { + return id; + } + + public void setId(Integer id) { + this.id = id; + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + + public String getDescription() { + return description; + } + + public void setDescription(String description) { + this.description = description; + } + + public List getTechnolgies() { + return technolgies; + } + + public void setTechnolgies(List technolgies) { + this.technolgies = technolgies; + } + + public List getProfiles() { + return profiles; + } + + public void setProfiles(List profiles) { + this.profiles = profiles; + } + } + + public static class TechnologyInput { + private Integer id; + private String name; + + public Integer getId() { + return id; + } + + public void setId(Integer id) { + this.id = id; + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + } + + public static class SetterOnlyInput { + private String name; + private NestedInput nested; + + public void setName(String name) { + this.name = name; + } + + public void setNested(NestedInput nested) { + this.nested = nested; + } + + public String describe() { + return name + " " + nested.describe(); + } + } + + public static class NestedInput { + private String value; + + public void setValue(String value) { + this.value = value; + } + + public String describe() { + return value; + } + } + + public static class BooleanGetterInput { + private boolean active; + + public boolean isActive() { + return active; + } + + public void setActive(boolean active) { + this.active = active; + } + } + + public static class JsonCreatorInput { + private final String name; + + @JsonCreator + public JsonCreatorInput(@JsonProperty("name") String name) { + this.name = name; + } + + public String describe() { + return name; + } + } + + public static class ConstructorPropertiesInput { + private final String name; + + @ConstructorProperties({"name"}) + public ConstructorPropertiesInput(String name) { + this.name = name; + } + + public String describe() { + return name; + } + } + + public static class PrivateFieldInput { + @JsonProperty("name") + private String value; + + public String describe() { + return value; + } + } + + public static class AliasInput { + private String title; + + @JsonAlias("name") + public void setTitle(String title) { + this.title = title; + } + + public String describe() { + return title; + } + } + + public static class AnySetterInput { + private final Map values = new TreeMap<>(); + + @JsonAnySetter + public void set(String name, Object value) { + values.put(name, value); + } + + public String describe() { + return values.toString(); + } + } + + @JsonIgnoreProperties(ignoreUnknown = true) + public static class IgnoreUnknownInput { + public String name; + } + + @JsonIgnoreProperties({"legacy"}) + public static class IgnoredNameInput { + public String name; + } + + public static class JsonIgnoreInput { + public String name; + + @JsonIgnore + public String secret; + } + + public static class UnwrappedInput { + public String name; + + @JsonUnwrapped + public Address address; + } + + public static class Address { + public String street; + } + + public static class ConflictingCreatorsInput { + @JsonCreator + public ConflictingCreatorsInput(@JsonProperty("name") String name) { + } + + @JsonCreator + public ConflictingCreatorsInput(@JsonProperty("name") Integer name) { + } + } +} diff --git a/src/test/kotlin/graphql/kickstart/tools/UnmappedInputFieldsTest.kt b/src/test/kotlin/graphql/kickstart/tools/UnmappedInputFieldsTest.kt new file mode 100644 index 00000000..379897a6 --- /dev/null +++ b/src/test/kotlin/graphql/kickstart/tools/UnmappedInputFieldsTest.kt @@ -0,0 +1,965 @@ +package graphql.kickstart.tools + +import ch.qos.logback.classic.Level +import ch.qos.logback.classic.Logger +import ch.qos.logback.classic.spi.ILoggingEvent +import ch.qos.logback.core.read.ListAppender +import com.fasterxml.jackson.annotation.JacksonInject +import com.fasterxml.jackson.annotation.JsonAnySetter +import com.fasterxml.jackson.annotation.JsonCreator +import com.fasterxml.jackson.annotation.JsonFormat +import com.fasterxml.jackson.annotation.JsonIgnoreProperties +import com.fasterxml.jackson.annotation.JsonSubTypes +import com.fasterxml.jackson.annotation.JsonTypeInfo +import com.fasterxml.jackson.annotation.JsonUnwrapped +import com.fasterxml.jackson.core.JsonParser +import com.fasterxml.jackson.databind.DeserializationContext +import com.fasterxml.jackson.databind.DeserializationFeature +import com.fasterxml.jackson.databind.InjectableValues +import com.fasterxml.jackson.databind.JsonNode +import com.fasterxml.jackson.databind.PropertyNamingStrategies +import com.fasterxml.jackson.databind.annotation.JsonDeserialize +import com.fasterxml.jackson.databind.annotation.JsonNaming +import com.fasterxml.jackson.databind.annotation.JsonPOJOBuilder +import com.fasterxml.jackson.databind.deser.std.StdDeserializer +import com.fasterxml.jackson.databind.module.SimpleModule +import com.fasterxml.jackson.databind.node.ObjectNode +import graphql.GraphQL +import graphql.kickstart.tools.UnmappedInputFieldsFixtures.* +import graphql.schema.DataFetchingEnvironment +import org.junit.Assert.assertThrows +import org.junit.Test +import org.slf4j.LoggerFactory +import java.util.* + +class UnmappedInputFieldsTest { + + @Test + fun `misspelled property of an input class is reported`() { + val (schema, warnings) = withWarnings { projectParser().makeExecutableSchema() } + + assertEquals(warnings, listOf(TECHNOLOGIES_MESSAGE)) + + // the warning predicts what happens when a client sends the field + val result = GraphQL.newGraphQL(schema).build().execute( + """ + mutation { + createProject(input: { name: "tools", technologies: [{ name: "Kotlin" }] }) + } + """) + assertEquals(result.errors.single().message.contains("Unrecognized field \"technologies\""), true) + } + + @Test + fun `misspelled property of an input class fails the build if asked to`() { + val error = assertThrows(SchemaError::class.java) { + projectParser(failOptions().build()).makeExecutableSchema() + } + + assertEquals(error.message, TECHNOLOGIES_MESSAGE) + } + + @Test + fun `misspelled scalar property of an input class is reported`() { + val (_, warnings) = withWarnings { + SchemaParser.newParser() + .schemaString( + """ + type Query { + greet(input: GreetingInput!): String + } + + input GreetingInput { + name: String! + } + """) + .resolvers(object : GraphQLQueryResolver { + fun greet(input: GreetingInput): String? = input.nmae + }) + .build() + .makeExecutableSchema() + } + + assertEquals(warnings, listOf( + "Input field 'GreetingInput.name' has no matching property in ${GreetingInput::class.java.name}. " + + "Converting argument 'input' of field 'Query.greet' fails whenever the field is sent. " + + "Known properties: nmae. " + + "Rename the property or map it with @JsonProperty(\"name\"). If it's left unmapped on purpose, list it in @JsonIgnoreProperties on the class." + )) + } + + @Test + fun `Kotlin property named like a boolean getter is reported`() { + val (schema, warnings) = withWarnings { + SchemaParser.newParser() + .schemaString( + """ + type Query { + active(input: ActiveInput!): Boolean + } + + input ActiveInput { + active: Boolean + } + """) + .resolvers(object : GraphQLQueryResolver { + fun active(input: ActiveInput): Boolean = input.isActive + }) + .build() + .makeExecutableSchema() + } + + // the Kotlin module keeps the "is" of a property, unlike the getter name of a Java boolean + assertEquals(warnings, listOf(unmapped("ActiveInput.active", ActiveInput::class.java, "argument 'input' of field 'Query.active'", "isActive"))) + + val result = GraphQL.newGraphQL(schema).build().execute("query { active(input: { active: true }) }") + assertEquals(result.errors.single().message.contains("Unrecognized field \"active\""), true) + } + + @Test + fun `misspelled properties of nested input classes are all reported through lists, arrays and Optionals`() { + val error = assertThrows(SchemaError::class.java) { + SchemaParser.newParser() + .schemaString( + """ + type Query { + outer(input: OuterInput): String + } + + input OuterInput { + middle: MiddleInput + } + + input MiddleInput { + list: [ListItemInput] + array: [ArrayItemInput] + optional: OptionalItemInput + } + + input ListItemInput { + name: String + } + + input ArrayItemInput { + name: String + } + + input OptionalItemInput { + name: String + } + """) + .resolvers(object : GraphQLQueryResolver { + fun outer(input: Optional): String? = input.toString() + }) + .options(failOptions().build()) + .build() + .makeExecutableSchema() + } + + val usage = "argument 'input' of field 'Query.outer'" + assertEquals(error.message, listOf( + unmapped("ListItemInput.name", ListItemInput::class.java, usage, "nmae"), + unmapped("ArrayItemInput.name", ArrayItemInput::class.java, usage, "nmae"), + unmapped("OptionalItemInput.name", OptionalItemInput::class.java, usage, "nmae") + ).joinToString("\n")) + } + + @Test + fun `input class used by several fields is reported once, including fields from extensions`() { + val error = assertThrows(SchemaError::class.java) { + SchemaParser.newParser() + .schemaString( + """ + type Query { + test: String + } + + type Mutation { + create(input: ItemInput!): String + update(input: ItemInput!): String + } + + input ItemInput { + name: String + } + + extend input ItemInput { + label: String + } + """) + .resolvers(object : GraphQLQueryResolver { + fun test(): String? = null + }, object : GraphQLMutationResolver { + fun create(input: ItemInput): String? = input.name + fun update(input: ItemInput): String? = input.name + }) + .options(failOptions().build()) + .build() + .makeExecutableSchema() + } + + assertEquals(error.message, unmapped("ItemInput.label", ItemInput::class.java, "argument 'input' of field 'Mutation.create'", "lable, name")) + } + + @Test + fun `misspelled property is reported if the ObjectMapper ignores unknown properties`() { + val (schema, warnings) = withWarnings { + SchemaParser.newParser() + .schemaString( + """ + type Query { + greet(input: GreetingInput!): String + } + + input GreetingInput { + name: String! + } + """) + .resolvers(object : GraphQLQueryResolver { + fun greet(input: GreetingInput): String? = input.nmae + }) + .options(SchemaParserOptions.newOptions() + .objectMapperConfigurer { mapper, _ -> mapper.disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES) } + .build()) + .build() + .makeExecutableSchema() + } + + assertEquals(warnings, listOf(unmapped("GreetingInput.name", GreetingInput::class.java, "argument 'input' of field 'Query.greet'", "nmae", + "drops the field's value, since the ObjectMapper ignores unknown properties"))) + + val data = assertNoGraphQlErrors(GraphQL.newGraphQL(schema).build()) { + """ + query { + greet(input: { name: "Ada" }) + } + """ + } + assertEquals(data["greet"], null) + } + + @Test + fun `input class is checked with the ObjectMapper of each field using it`() { + val parser = { options: SchemaParserOptions.Builder -> + SchemaParser.newParser() + .schemaString( + """ + type Query { + snake(input: NameInput!): String + camel(input: NameInput!): String + } + + input NameInput { + firstName: String + } + """) + .resolvers(object : GraphQLQueryResolver { + fun snake(input: NameInput): String? = input.firstName + fun camel(input: NameInput): String? = input.firstName + }) + .options(options + .objectMapperConfigurer { mapper, context -> + if (context.field.name == "snake") { + mapper.setPropertyNamingStrategy(PropertyNamingStrategies.SNAKE_CASE) + } + } + .build()) + .build() + } + + val error = assertThrows(SchemaError::class.java) { + parser(failOptions()).makeExecutableSchema() + } + assertEquals(error.message, unmapped("NameInput.firstName", NameInput::class.java, "argument 'input' of field 'Query.snake'", "first_name")) + + val gql = GraphQL.newGraphQL(parser(SchemaParserOptions.newOptions()).makeExecutableSchema()).build() + val result = gql.execute("query { snake(input: { firstName: \"Ada\" }) camel(input: { firstName: \"Ada\" }) }") + assertEquals(result.errors.single().message.contains("Unrecognized field \"firstName\""), true) + assertEquals(result.getData>()!!["camel"], "Ada") + } + + @Test + fun `misspelled property of a field with a default value is reported as failing every time`() { + val (schema, warnings) = withWarnings { + SchemaParser.newParser() + .schemaString( + """ + type Query { + search(filter: SearchFilter): String + } + + input SearchFilter { + query: String + limit: Int = 10 + } + """) + .resolvers(object : GraphQLQueryResolver { + fun search(filter: SearchFilter?): String? = filter?.query + }) + .build() + .makeExecutableSchema() + } + + assertEquals(warnings, listOf(unmapped("SearchFilter.limit", SearchFilter::class.java, "argument 'filter' of field 'Query.search'", "limt, query", + "fails for every 'SearchFilter' value sent, since the field has a default value"))) + + val result = GraphQL.newGraphQL(schema).build().execute("query { search(filter: { query: \"a\" }) }") + assertEquals(result.errors.single().message.contains("Unrecognized field \"limit\""), true) + } + + @Test + fun `arguments are matched to their parameters past sources, environments and continuations`() { + val error = assertThrows(SchemaError::class.java) { + SchemaParser.newParser() + .schemaString( + """ + type Query { + owner: Owner + withEnvironment(input: EnvironmentInput!): String + suspending(input: SuspendInput!): String + } + + type Owner { + child(id: ID!, input: ChildInput!): String + } + + input EnvironmentInput { + name: String + } + + input SuspendInput { + name: String + } + + input ChildInput { + name: String + } + """) + .resolvers(object : GraphQLQueryResolver { + fun owner(): Owner = Owner() + fun withEnvironment(input: EnvironmentInput, environment: DataFetchingEnvironment): String? = input.nmae + @Suppress("RedundantSuspendModifier") + suspend fun suspending(input: SuspendInput): String? = input.nmae + }, object : GraphQLResolver { + fun child(owner: Owner, id: String, input: ChildInput): String? = input.nmae + }) + .options(failOptions().build()) + .build() + .makeExecutableSchema() + } + + assertEquals(error.message, listOf( + unmapped("ChildInput.name", ChildInput::class.java, "argument 'input' of field 'Owner.child'", "nmae"), + unmapped("EnvironmentInput.name", EnvironmentInput::class.java, "argument 'input' of field 'Query.withEnvironment'", "nmae"), + unmapped("SuspendInput.name", SuspendInput::class.java, "argument 'input' of field 'Query.suspending'", "nmae") + ).joinToString("\n")) + } + + @Test + fun `input classes are not checked if asked not to`() { + val (_, warnings) = withWarnings { + projectParser(SchemaParserOptions.newOptions().unmappedInputFields(UnmappedInputFields.IGNORE).build()).makeExecutableSchema() + } + + assertEquals(warnings, emptyList()) + } + + @Test + fun `Java input classes that Jackson maps are not reported`() { + val (schema, warnings) = withWarnings { + SchemaParser.newParser() + .schemaString( + """ + type Query { + setterOnly(input: SetterOnlyInput!): String + booleanGetter(input: BooleanGetterInput!): Boolean + jsonCreator(input: JsonCreatorInput!): String + constructorProperties(input: ConstructorPropertiesInput!): String + privateField(input: PrivateFieldInput!): String + alias(input: AliasInput!): String + anySetter(input: AnySetterInput!): String + ignoreUnknown(input: IgnoreUnknownInput!): String + ignoredName(input: IgnoredNameInput!): String + jsonIgnore(input: JsonIgnoreInput!): String + unwrapped(input: UnwrappedInput!): String + } + + input SetterOnlyInput { + name: String + nested: NestedInput + } + + input NestedInput { + value: String + } + + input BooleanGetterInput { + active: Boolean! + } + + input JsonCreatorInput { + name: String + } + + input ConstructorPropertiesInput { + name: String + } + + input PrivateFieldInput { + name: String + } + + input AliasInput { + name: String + } + + input AnySetterInput { + a: Int + b: String + } + + input IgnoreUnknownInput { + name: String + other: String + } + + input IgnoredNameInput { + name: String + legacy: String + } + + input JsonIgnoreInput { + name: String + secret: String + } + + input UnwrappedInput { + name: String + street: String + } + """) + .resolvers(object : GraphQLQueryResolver { + fun setterOnly(input: SetterOnlyInput): String = input.describe() + fun booleanGetter(input: BooleanGetterInput): Boolean = input.isActive + fun jsonCreator(input: JsonCreatorInput): String = input.describe() + fun constructorProperties(input: ConstructorPropertiesInput): String = input.describe() + fun privateField(input: PrivateFieldInput): String = input.describe() + fun alias(input: AliasInput): String = input.describe() + fun anySetter(input: AnySetterInput): String = input.describe() + fun ignoreUnknown(input: IgnoreUnknownInput): String = input.name + fun ignoredName(input: IgnoredNameInput): String = input.name + fun jsonIgnore(input: JsonIgnoreInput): String = "${input.name} ${input.secret}" + fun unwrapped(input: UnwrappedInput): String = "${input.name} ${input.address.street}" + }) + .options(failOptions().build()) + .build() + .makeExecutableSchema() + } + + // the scanner can't find a getter for SetterOnlyInput.nested either, but that's no reason to warn + assertEquals(warnings, emptyList()) + + val data = assertNoGraphQlErrors(GraphQL.newGraphQL(schema).build()) { + """ + query { + setterOnly(input: { name: "a", nested: { value: "b" } }) + booleanGetter(input: { active: true }) + jsonCreator(input: { name: "c" }) + constructorProperties(input: { name: "d" }) + privateField(input: { name: "e" }) + alias(input: { name: "f" }) + anySetter(input: { a: 1, b: "g" }) + ignoreUnknown(input: { name: "h", other: "x" }) + ignoredName(input: { name: "i", legacy: "x" }) + jsonIgnore(input: { name: "j", secret: "x" }) + unwrapped(input: { name: "k", street: "l" }) + } + """ + } + + assertEquals(data, mapOf( + "setterOnly" to "a b", + "booleanGetter" to true, + "jsonCreator" to "c", + "constructorProperties" to "d", + "privateField" to "e", + "alias" to "f", + "anySetter" to "{a=1, b=g}", + "ignoreUnknown" to "h", + "ignoredName" to "i", + "jsonIgnore" to "j null", + "unwrapped" to "k l" + )) + } + + @Test + fun `Kotlin input classes and other arguments that Jackson maps are not reported`() { + val schema = SchemaParser.newParser() + .schemaString( + """ + type Query { + dataClass(input: DataClassInput!): String + anySetter(input: KotlinAnySetterInput!): String + snakeCase(input: SnakeCaseInput!): String + caseInsensitive(input: CaseInsensitiveInput!): String + propertyIgnorals(input: PropertyIgnoralsInput!): String + filter(input: RequestFilter!): String + map(input: MapInput!): String + objectNode(input: ObjectNodeInput!): String + } + + input DataClassInput { + name: String! + count: Int + } + + extend input DataClassInput { + label: String + } + + input KotlinAnySetterInput { + a: Int + b: String + } + + input SnakeCaseInput { + first_name: String + } + + input CaseInsensitiveInput { + NAME: String + } + + input PropertyIgnoralsInput { + inner: InnerInput + } + + input InnerInput { + name: String + legacy: String + } + + input RequestFilter { + and: [RequestFilter!] + or: [RequestFilter!] + name: String + } + + input MapInput { + anything: String + filter: RequestFilter + } + + input ObjectNodeInput { + anything: String + } + """) + .resolvers(object : GraphQLQueryResolver { + fun dataClass(input: DataClassInput): String = input.toString() + fun anySetter(input: KotlinAnySetterInput): String = input.values.toString() + fun snakeCase(input: SnakeCaseInput): String? = input.firstName + fun caseInsensitive(input: CaseInsensitiveInput): String? = input.name + fun propertyIgnorals(input: PropertyIgnoralsInput): String? = input.inner?.name + fun filter(input: RequestFilter): String? = input.and?.single()?.or?.single()?.name + fun map(input: Map): String = input.toString() + fun objectNode(input: ObjectNode): String = input.toString() + }) + .options(failOptions().build()) + .build() + .makeExecutableSchema() + + val data = assertNoGraphQlErrors(GraphQL.newGraphQL(schema).build()) { + """ + query { + dataClass(input: { name: "a", label: "b" }) + anySetter(input: { a: 1, b: "c" }) + snakeCase(input: { first_name: "d" }) + caseInsensitive(input: { NAME: "e" }) + propertyIgnorals(input: { inner: { name: "f", legacy: "x" } }) + filter(input: { and: [{ or: [{ name: "g" }] }] }) + map(input: { anything: "h", filter: { name: "i" } }) + objectNode(input: { anything: "j" }) + } + """ + } + + assertEquals(data, mapOf( + "dataClass" to "DataClassInput(name=a, count=1, label=b)", + "anySetter" to "{a=1, b=c}", + "snakeCase" to "d", + "caseInsensitive" to "e", + "propertyIgnorals" to "f", + "filter" to "g", + "map" to "{anything=h, filter={name=i}}", + "objectNode" to """{"anything":"j"}""" + )) + } + + @Test + fun `input classes that Jackson doesn't deserialize property by property are not reported`() { + val schema = SchemaParser.newParser() + .schemaString( + """ + type Query { + delegating(input: DelegatingInput!): String + builder(input: BuilderInput!): String + shape(input: ShapeInput!): String + money(input: MoneyInput!): String + } + + input DelegatingInput { + name: String + } + + input BuilderInput { + name: String + } + + input ShapeInput { + kind: String! + radius: Float + side: Float + } + + input MoneyInput { + cents: Int + } + """) + .resolvers(object : GraphQLQueryResolver { + fun delegating(input: DelegatingInput): String = input.values.toString() + fun builder(input: BuilderInput): String? = input.name + fun shape(input: ShapeInput): String = when (input) { + is Circle -> "circle ${input.radius}" + is Square -> "square ${input.side}" + } + fun money(input: Money): String = input.amount.toString() + }) + .options(failOptions() + .objectMapperConfigurer { mapper, _ -> mapper.registerModule(SimpleModule().addDeserializer(Money::class.java, MoneyDeserializer())) } + .build()) + .build() + .makeExecutableSchema() + + val data = assertNoGraphQlErrors(GraphQL.newGraphQL(schema).build()) { + """ + query { + delegating(input: { name: "a" }) + builder(input: { name: "b" }) + shape(input: { kind: "circle", radius: 1.5 }) + money(input: { cents: 250 }) + } + """ + } + + assertEquals(data, mapOf( + "delegating" to "{name=a}", + "builder" to "b", + "shape" to "circle 1.5", + "money" to "250" + )) + } + + @Test + fun `unmapped fields that can't be checked are not reported`() { + val schema = SchemaParser.newParser() + .schemaString( + """ + type Query { + injected(input: InjectedInput!): String + builder(input: BuilderTypoInput!): String + unwrapped(input: KotlinUnwrappedInput!): String + generic(input: GenericInput!): String + } + + input InjectedInput { + name: String + } + + input BuilderTypoInput { + name: String + } + + input KotlinUnwrappedInput { + name: String + street: String + } + + input GenericInput { + name: String + } + """) + .resolvers(object : GraphQLQueryResolver { + fun injected(input: InjectedInput): String? = input.nmae + fun builder(input: BuilderTypoInput): String? = input.name + fun unwrapped(input: KotlinUnwrappedInput): String = "${input.name} ${input.address?.stret}" + }, GenericInputResolver()) + .options(failOptions() + .objectMapperConfigurer { mapper, _ -> mapper.setInjectableValues(InjectableValues.Std().addValue("tenant", "acme")) } + .build()) + .build() + // the parameter of 'generic' is a type variable, which isn't resolved to its class here, nor when the argument is converted + .makeExecutableSchema() + + val result = GraphQL.newGraphQL(schema).build().execute( + """ + query { + injected(input: { name: "a" }) + builder(input: { name: "b" }) + unwrapped(input: { name: "c", street: "d" }) + } + """) + + // neither injecting values nor a builder works with the placeholder object, so the fields are never read + assertEquals(result.errors.map { it.path }, listOf(listOf("injected"), listOf("builder"))) + assertEquals(result.errors.all { it.message.contains("Unrecognized field \"name\"") }, true) + // Jackson drops what none of the unwrapped properties know + assertEquals(result.getData>()!!["unwrapped"], "c null") + } + + @Test + fun `input class that Jackson can't deserialize doesn't fail the build`() { + val schema = SchemaParser.newParser() + .schemaString( + """ + type Query { + broken(input: BrokenInput!): String + } + + input BrokenInput { + name: String + } + """) + .resolvers(object : GraphQLQueryResolver { + fun broken(input: ConflictingCreatorsInput): String = input.toString() + }) + .options(failOptions().build()) + .build() + .makeExecutableSchema() + + // it only fails once the argument is converted, as it did before the check + val result = GraphQL.newGraphQL(schema).build().execute("query { broken(input: { name: \"a\" }) }") + assertEquals(result.errors.single().message.contains("Conflicting property-based creators"), true) + } + + private fun failOptions() =SchemaParserOptions.newOptions().unmappedInputFields(UnmappedInputFields.FAIL) + + private fun projectParser(options: SchemaParserOptions = SchemaParserOptions.newOptions().build()) = SchemaParser.newParser() + .schemaString( + """ + type Query { + test: String + } + + type Mutation { + createProject(input: ProjectInput!): String + } + + input ProjectInput { + id: Int + name: String + description: String + technologies: [TechnologyInput!] + profiles: [ID!] + } + + input TechnologyInput { + id: Int + name: String! + } + """) + .resolvers(object : GraphQLQueryResolver { + fun test(): String? = null + }, object : GraphQLMutationResolver { + fun createProject(input: ProjectInput): String? = input.name + }) + .options(options) + .build() + + private fun unmapped(field: String, clazz: Class<*>, usage: String, knownProperties: String, consequence: String = "fails whenever the field is sent") = + "Input field '$field' has no matching property in ${clazz.name}. Converting $usage $consequence. Known properties: $knownProperties. " + + "Rename the property or map it with @JsonProperty(\"${field.substringAfter('.')}\"). If it's left unmapped on purpose, list it in @JsonIgnoreProperties on the class." + + // the warnings logged by any logger while building + private fun withWarnings(build: () -> T): Pair> { + // the tests run in the library's module, which doesn't read logback's + listOf("ch.qos.logback.classic", "ch.qos.logback.core").forEach { name -> + ModuleLayer.boot().findModule(name).ifPresent { javaClass.module.addReads(it) } + } + val logger = LoggerFactory.getLogger(org.slf4j.Logger.ROOT_LOGGER_NAME) as Logger + val appender = ListAppender().apply { start() } + logger.addAppender(appender) + try { + val result = build() + return result to appender.list.filter { it.level.isGreaterOrEqual(Level.WARN) }.map { it.formattedMessage } + } finally { + logger.detachAppender(appender) + } + } + + class GreetingInput { + var nmae: String? = null + } + + data class ActiveInput(val isActive: Boolean = false) + + class OuterInput { + var middle: MiddleInput? = null + } + + class MiddleInput { + var list: List? = null + var array: Array? = null + var optional: Optional? = null + } + + class ListItemInput { + var nmae: String? = null + } + + class ArrayItemInput { + var nmae: String? = null + } + + class OptionalItemInput { + var nmae: String? = null + } + + class ItemInput { + var name: String? = null + var lable: String? = null + } + + class NameInput { + var firstName: String? = null + } + + class SearchFilter { + var query: String? = null + var limt: Int? = null + } + + class Owner + + class EnvironmentInput { + var nmae: String? = null + } + + class SuspendInput { + var nmae: String? = null + } + + class ChildInput { + var nmae: String? = null + } + + data class DataClassInput(val name: String, val count: Int = 1, val label: String? = null) + + class KotlinAnySetterInput { + @field:JsonAnySetter + val values = sortedMapOf() + } + + @JsonNaming(PropertyNamingStrategies.SnakeCaseStrategy::class) + class SnakeCaseInput { + var firstName: String? = null + } + + @JsonFormat(with = [JsonFormat.Feature.ACCEPT_CASE_INSENSITIVE_PROPERTIES]) + class CaseInsensitiveInput { + var name: String? = null + } + + class PropertyIgnoralsInput { + @field:JsonIgnoreProperties("legacy") + var inner: InnerInput? = null + } + + class InnerInput { + var name: String? = null + } + + class RequestFilter { + var and: List? = null + var or: List? = null + var name: String? = null + } + + class DelegatingInput @JsonCreator(mode = JsonCreator.Mode.DELEGATING) constructor(val values: Map) + + @JsonDeserialize(builder = BuilderInput.Builder::class) + class BuilderInput private constructor(val name: String?) { + @JsonPOJOBuilder(withPrefix = "") + class Builder { + private var name: String? = null + + fun name(name: String?) = apply { this.name = name } + + fun build() = BuilderInput(name) + } + } + + @JsonTypeInfo(use = JsonTypeInfo.Id.NAME, property = "kind") + @JsonSubTypes(JsonSubTypes.Type(Circle::class, name = "circle"), JsonSubTypes.Type(Square::class, name = "square")) + sealed class ShapeInput + + class Circle : ShapeInput() { + var radius: Double? = null + } + + class Square : ShapeInput() { + var side: Double? = null + } + + // the property doesn't match the schema, only the deserializer does + class Money(val amount: Int) + + class MoneyDeserializer : StdDeserializer(Money::class.java) { + override fun deserialize(p: JsonParser, ctxt: DeserializationContext): Money = Money(p.readValueAsTree()["cents"].asInt()) + } + + class InjectedInput { + @field:JacksonInject("tenant") + var tenant: String? = null + var nmae: String? = null + } + + @JsonDeserialize(builder = BuilderTypoInput.Builder::class) + class BuilderTypoInput private constructor(val name: String?) { + @JsonPOJOBuilder(withPrefix = "") + class Builder { + private var nmae: String? = null + + fun nmae(nmae: String?) = apply { this.nmae = nmae } + + fun build() = BuilderTypoInput(nmae) + } + } + + class KotlinUnwrappedInput { + var name: String? = null + + @field:JsonUnwrapped + var address: KotlinAddress? = null + } + + class KotlinAddress { + var stret: String? = null + } + + abstract class InputResolver : GraphQLQueryResolver { + fun generic(input: I): String = input.toString() + } + + class GenericInputResolver : InputResolver() + + class GenericInput { + var nmae: String? = null + } + + companion object { + private val TECHNOLOGIES_MESSAGE = "Input field 'ProjectInput.technologies' has no matching property in ${ProjectInput::class.java.name}. " + + "Converting argument 'input' of field 'Mutation.createProject' fails whenever the field is sent. " + + "Known properties: description, id, name, profiles, technolgies. " + + "Rename the property or map it with @JsonProperty(\"technologies\"). If it's left unmapped on purpose, list it in @JsonIgnoreProperties on the class." + } +} From 108387f255c7b5e5b9a35bcbd6e11e92a28a5535 Mon Sep 17 00:00:00 2001 From: Oryan Date: Mon, 12 Oct 2026 16:07:37 -0400 Subject: [PATCH 3/3] Skip polymorphic elements in the unmapped input field check A List, array or Optional of a class with @JsonTypeInfo got the base class's deserializer from the container, so the type id and every field that only a subclass has were reported as unmapped, which failed the build in FAIL mode for inputs that convert fine. The check now looks up the elements' type info the way Jackson does when it creates the container's deserializer, from the property's annotation or else from the class or default typing, and skips them like a polymorphic property. - An input type used by fields whose ObjectMappers handle unknown properties differently is reported once for each consequence, so a field that fails isn't hidden behind one that drops the value - Errors such as NoClassDefFoundError while looking up a deserializer are logged at debug like exceptions, since the lookup used to run on the first query and mustn't fail a build that used to succeed - Tests for Map arguments and polymorphic properties, and contains assertions that show the actual message Co-Authored-By: Claude Opus 5.5 --- .../tools/UnmappedInputFieldChecker.kt | 65 ++++--- .../tools/UnmappedInputFieldsTest.kt | 176 +++++++++++++++++- 2 files changed, 208 insertions(+), 33 deletions(-) diff --git a/src/main/kotlin/graphql/kickstart/tools/UnmappedInputFieldChecker.kt b/src/main/kotlin/graphql/kickstart/tools/UnmappedInputFieldChecker.kt index dce46d16..2378c362 100644 --- a/src/main/kotlin/graphql/kickstart/tools/UnmappedInputFieldChecker.kt +++ b/src/main/kotlin/graphql/kickstart/tools/UnmappedInputFieldChecker.kt @@ -2,6 +2,7 @@ package graphql.kickstart.tools import com.fasterxml.jackson.databind.DeserializationContext import com.fasterxml.jackson.databind.DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES +import com.fasterxml.jackson.databind.JavaType import com.fasterxml.jackson.databind.JsonDeserializer import com.fasterxml.jackson.databind.ObjectMapper import com.fasterxml.jackson.databind.deser.BeanDeserializerBase @@ -35,25 +36,28 @@ internal class UnmappedInputFieldChecker( // a deserializer is cached by its mapper, so this is per mapper, class and input type, and ends recursive input types private val checked = mutableSetOf, String>>() - // reported once, for the first argument found using it - private val messages = linkedMapOf>, String>() + // reported once for each consequence, with the first argument found using it, since mappers differ in ignoring unknown properties + private val messages = linkedMapOf() fun check(objectType: String, resolver: MethodFieldResolver) { - val mapper = resolver.objectMapper - val context = (mapper.deserializationContext as? DefaultDeserializationContext)?.createDummyInstance(mapper.deserializationConfig) ?: return - resolver.field.inputValueDefinitions.forEachIndexed { index, argument -> val inputObject = (argument.type.unwrap() as? TypeName)?.let { inputObjects[it.name] } ?: return@forEachIndexed - val parameterType = resolver.getMethodParameterType(index) ?: return@forEachIndexed val usage = "argument '${argument.name}' of field '$objectType.${resolver.field.name}'" try { + val parameterType = resolver.getMethodParameterType(index) ?: return@forEachIndexed + val mapper = resolver.objectMapper + val context = (mapper.deserializationContext as? DefaultDeserializationContext) + ?.createDummyInstance(mapper.deserializationConfig) ?: return@forEachIndexed // the same lookup as convertValue's when it converts the argument val deserializer = context.findRootValueDeserializer(mapper.typeFactory.constructType(parameterType)) checkFields(mapper, context, deserializer, inputObject, usage) - } catch (e: Exception) { - // e.g. a class Jackson can't deserialize, which only fails once the argument is converted, as it did before this check - log.debug("Can't check the input fields of $usage with Jackson's deserializer for $parameterType", e) + } catch (e: VirtualMachineError) { + throw e + } catch (e: Throwable) { + // e.g. a class Jackson can't deserialize, or one that refers to a missing class, which only fails once the argument is + // converted, as it did before this check. Errors are caught too, since the lookup used to run on the first query + log.debug("Can't check the input fields of $usage with Jackson", e) } } } @@ -85,9 +89,8 @@ internal class UnmappedInputFieldChecker( val property = beanDeserializer.findProperty(field.name) if (property == null) { knownPropertiesIfUnmapped(mapper, context, beanDeserializer, field.name)?.let { knownProperties -> - messages.getOrPut(Triple(inputObject.name, field.name, beanDeserializer.handledType())) { - message(mapper, inputObject, field, beanDeserializer.handledType(), usage, knownProperties) - } + val unmapped = Unmapped(inputObject.name, field.name, beanDeserializer.handledType(), consequence(mapper, inputObject, field)) + messages.getOrPut(unmapped) { message(unmapped, usage, knownProperties) } } } else if (property.valueTypeDeserializer == null) { // a polymorphic property's class is only known once its type id is read @@ -103,12 +106,24 @@ internal class UnmappedInputFieldChecker( is BeanDeserializerBase -> deserializer.takeUnless { it.valueInstantiator.canCreateUsingDelegate() } // a Map gets the fields as its keys is ContainerDeserializerBase<*> -> - if (deserializer.valueType.isMapLikeType) null else beanDeserializer(context, deserializer.contentDeserializer) - is ReferenceTypeDeserializer<*> -> - beanDeserializer(context, context.findContextualValueDeserializer(deserializer.valueType.referencedType, null)) + if (deserializer.valueType.isMapLikeType || isPolymorphic(context, deserializer.contentType)) null + else beanDeserializer(context, deserializer.contentDeserializer) + is ReferenceTypeDeserializer<*> -> { + val referencedType = deserializer.valueType.referencedType + if (isPolymorphic(context, referencedType)) null + else beanDeserializer(context, context.findContextualValueDeserializer(referencedType, null)) + } else -> null } + /** + * Whether the elements of a list, an array or an Optional are read with type info, so that their class is only known once their + * type id is read. Their deserializer is the base class's, which doesn't know the subclasses' properties. The same lookup as + * Jackson's when it creates the deserializer: type info from a property's annotation, or else from the class or default typing. + */ + private fun isPolymorphic(context: DeserializationContext, contentType: JavaType): Boolean = + contentType.getTypeHandler() != null || context.factory.findTypeDeserializer(context.config, contentType) != null + /** * Returns the properties Jackson knows if it fails on the field, or null if it maps or ignores it. * @@ -140,23 +155,21 @@ internal class UnmappedInputFieldChecker( private fun inputObjectOf(type: GraphQLType): GraphQLInputObjectType? = (GraphQLTypeUtil.unwrapAllAs(type) as? GraphQLNamedType)?.let { inputObjects[it.name] } - private fun message( - mapper: ObjectMapper, - inputObject: GraphQLInputObjectType, - field: GraphQLInputObjectField, - clazz: Class<*>, - usage: String, - knownProperties: Collection - ): String { - val consequence = when { + private fun consequence(mapper: ObjectMapper, inputObject: GraphQLInputObjectType, field: GraphQLInputObjectField): String = + when { !mapper.deserializationConfig.isEnabled(FAIL_ON_UNKNOWN_PROPERTIES) -> "drops the field's value, since the ObjectMapper ignores unknown properties" field.hasSetDefaultValue() -> "fails for every '${inputObject.name}' value sent, since the field has a default value" else -> "fails whenever the field is sent" } + + private fun message(unmapped: Unmapped, usage: String, knownProperties: Collection): String { val properties = knownProperties.map { it.toString() }.sorted().ifEmpty { listOf("none") } - return "Input field '${inputObject.name}.${field.name}' has no matching property in ${clazz.name}. Converting $usage $consequence. " + + return "Input field '${unmapped.inputObject}.${unmapped.field}' has no matching property in ${unmapped.clazz.name}. " + + "Converting $usage ${unmapped.consequence}. " + "Known properties: ${properties.joinToString(", ")}. " + - "Rename the property or map it with @JsonProperty(\"${field.name}\"). If it's left unmapped on purpose, list it in @JsonIgnoreProperties on the class." + "Rename the property or map it with @JsonProperty(\"${unmapped.field}\"). If it's left unmapped on purpose, list it in @JsonIgnoreProperties on the class." } + + private data class Unmapped(val inputObject: String, val field: String, val clazz: Class<*>, val consequence: String) } diff --git a/src/test/kotlin/graphql/kickstart/tools/UnmappedInputFieldsTest.kt b/src/test/kotlin/graphql/kickstart/tools/UnmappedInputFieldsTest.kt index 379897a6..d3f63e5a 100644 --- a/src/test/kotlin/graphql/kickstart/tools/UnmappedInputFieldsTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/UnmappedInputFieldsTest.kt @@ -13,15 +13,20 @@ import com.fasterxml.jackson.annotation.JsonSubTypes import com.fasterxml.jackson.annotation.JsonTypeInfo import com.fasterxml.jackson.annotation.JsonUnwrapped import com.fasterxml.jackson.core.JsonParser +import com.fasterxml.jackson.databind.BeanDescription +import com.fasterxml.jackson.databind.DeserializationConfig import com.fasterxml.jackson.databind.DeserializationContext import com.fasterxml.jackson.databind.DeserializationFeature import com.fasterxml.jackson.databind.InjectableValues +import com.fasterxml.jackson.databind.JavaType +import com.fasterxml.jackson.databind.JsonDeserializer import com.fasterxml.jackson.databind.JsonNode import com.fasterxml.jackson.databind.PropertyNamingStrategies import com.fasterxml.jackson.databind.annotation.JsonDeserialize import com.fasterxml.jackson.databind.annotation.JsonNaming import com.fasterxml.jackson.databind.annotation.JsonPOJOBuilder import com.fasterxml.jackson.databind.deser.std.StdDeserializer +import com.fasterxml.jackson.databind.module.SimpleDeserializers import com.fasterxml.jackson.databind.module.SimpleModule import com.fasterxml.jackson.databind.node.ObjectNode import graphql.GraphQL @@ -47,7 +52,7 @@ class UnmappedInputFieldsTest { createProject(input: { name: "tools", technologies: [{ name: "Kotlin" }] }) } """) - assertEquals(result.errors.single().message.contains("Unrecognized field \"technologies\""), true) + assertContains(result.errors.single().message, "Unrecognized field \"technologies\"") } @Test @@ -113,7 +118,7 @@ class UnmappedInputFieldsTest { assertEquals(warnings, listOf(unmapped("ActiveInput.active", ActiveInput::class.java, "argument 'input' of field 'Query.active'", "isActive"))) val result = GraphQL.newGraphQL(schema).build().execute("query { active(input: { active: true }) }") - assertEquals(result.errors.single().message.contains("Unrecognized field \"active\""), true) + assertContains(result.errors.single().message, "Unrecognized field \"active\"") } @Test @@ -274,10 +279,49 @@ class UnmappedInputFieldsTest { val gql = GraphQL.newGraphQL(parser(SchemaParserOptions.newOptions()).makeExecutableSchema()).build() val result = gql.execute("query { snake(input: { firstName: \"Ada\" }) camel(input: { firstName: \"Ada\" }) }") - assertEquals(result.errors.single().message.contains("Unrecognized field \"firstName\""), true) + assertContains(result.errors.single().message, "Unrecognized field \"firstName\"") assertEquals(result.getData>()!!["camel"], "Ada") } + @Test + fun `input class is reported once for each consequence of the ObjectMappers using it`() { + val error = assertThrows(SchemaError::class.java) { + SchemaParser.newParser() + .schemaString( + """ + type Query { + lenient(input: GreetingInput!): String + strict(input: GreetingInput!): String + alsoStrict(input: GreetingInput!): String + } + + input GreetingInput { + name: String! + } + """) + .resolvers(object : GraphQLQueryResolver { + fun lenient(input: GreetingInput): String? = input.nmae + fun strict(input: GreetingInput): String? = input.nmae + fun alsoStrict(input: GreetingInput): String? = input.nmae + }) + .options(failOptions() + .objectMapperConfigurer { mapper, context -> + if (context.field.name == "lenient") { + mapper.disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES) + } + } + .build()) + .build() + .makeExecutableSchema() + } + + assertEquals(error.message, listOf( + unmapped("GreetingInput.name", GreetingInput::class.java, "argument 'input' of field 'Query.lenient'", "nmae", + "drops the field's value, since the ObjectMapper ignores unknown properties"), + unmapped("GreetingInput.name", GreetingInput::class.java, "argument 'input' of field 'Query.strict'", "nmae") + ).joinToString("\n")) + } + @Test fun `misspelled property of a field with a default value is reported as failing every time`() { val (schema, warnings) = withWarnings { @@ -304,7 +348,7 @@ class UnmappedInputFieldsTest { "fails for every 'SearchFilter' value sent, since the field has a default value"))) val result = GraphQL.newGraphQL(schema).build().execute("query { search(filter: { query: \"a\" }) }") - assertEquals(result.errors.single().message.contains("Unrecognized field \"limit\""), true) + assertContains(result.errors.single().message, "Unrecognized field \"limit\"") } @Test @@ -505,6 +549,7 @@ class UnmappedInputFieldsTest { propertyIgnorals(input: PropertyIgnoralsInput!): String filter(input: RequestFilter!): String map(input: MapInput!): String + beanMap(input: BeanMapInput!): String objectNode(input: ObjectNodeInput!): String } @@ -550,6 +595,11 @@ class UnmappedInputFieldsTest { filter: RequestFilter } + input BeanMapInput { + first: RequestFilter + second: RequestFilter + } + input ObjectNodeInput { anything: String } @@ -562,6 +612,8 @@ class UnmappedInputFieldsTest { fun propertyIgnorals(input: PropertyIgnoralsInput): String? = input.inner?.name fun filter(input: RequestFilter): String? = input.and?.single()?.or?.single()?.name fun map(input: Map): String = input.toString() + // the fields are the map's keys, not properties of its values + fun beanMap(input: Map): String = input.mapValues { it.value.name }.toString() fun objectNode(input: ObjectNode): String = input.toString() }) .options(failOptions().build()) @@ -578,6 +630,7 @@ class UnmappedInputFieldsTest { propertyIgnorals(input: { inner: { name: "f", legacy: "x" } }) filter(input: { and: [{ or: [{ name: "g" }] }] }) map(input: { anything: "h", filter: { name: "i" } }) + beanMap(input: { first: { name: "k" }, second: { name: "l" } }) objectNode(input: { anything: "j" }) } """ @@ -591,6 +644,7 @@ class UnmappedInputFieldsTest { "propertyIgnorals" to "f", "filter" to "g", "map" to "{anything=h, filter={name=i}}", + "beanMap" to "{first=k, second=l}", "objectNode" to """{"anything":"j"}""" )) } @@ -604,6 +658,9 @@ class UnmappedInputFieldsTest { delegating(input: DelegatingInput!): String builder(input: BuilderInput!): String shape(input: ShapeInput!): String + animals(input: [AnimalInput!]!): String + animal(input: AnimalInput): String + zoo(input: ZooInput!): String money(input: MoneyInput!): String } @@ -621,6 +678,24 @@ class UnmappedInputFieldsTest { side: Float } + input AnimalInput { + kind: String! + name: String + barks: Boolean + } + + input PetInput { + kind: String! + name: String + meows: Boolean + } + + input ZooInput { + animal: AnimalInput + animals: [AnimalInput!] + pets: [PetInput!] + } + input MoneyInput { cents: Int } @@ -632,6 +707,11 @@ class UnmappedInputFieldsTest { is Circle -> "circle ${input.radius}" is Square -> "square ${input.side}" } + // the base classes are concrete, so their properties are found, but not their subclasses' + fun animals(input: List): String = input.joinToString { it.describe() } + fun animal(input: Optional): String = input.map { it.describe() }.orElse("none") + fun zoo(input: Zoo): String = + listOfNotNull(input.animal?.describe(), input.animals?.single()?.describe(), input.pets?.single()?.describe()).joinToString() fun money(input: Money): String = input.amount.toString() }) .options(failOptions() @@ -646,6 +726,13 @@ class UnmappedInputFieldsTest { delegating(input: { name: "a" }) builder(input: { name: "b" }) shape(input: { kind: "circle", radius: 1.5 }) + animals(input: [{ kind: "dog", name: "c", barks: true }, { kind: "animal", name: "d" }]) + animal(input: { kind: "dog", name: "e", barks: false }) + zoo(input: { + animal: { kind: "dog", name: "f", barks: true } + animals: [{ kind: "dog", name: "g", barks: true }] + pets: [{ kind: "cat", name: "h", meows: true }] + }) money(input: { cents: 250 }) } """ @@ -655,6 +742,9 @@ class UnmappedInputFieldsTest { "delegating" to "{name=a}", "builder" to "b", "shape" to "circle 1.5", + "animals" to "dog c barks=true, animal d", + "animal" to "dog e barks=false", + "zoo" to "dog f barks=true, dog g barks=true, cat h meows=true", "money" to "250" )) } @@ -711,7 +801,7 @@ class UnmappedInputFieldsTest { // neither injecting values nor a builder works with the placeholder object, so the fields are never read assertEquals(result.errors.map { it.path }, listOf(listOf("injected"), listOf("builder"))) - assertEquals(result.errors.all { it.message.contains("Unrecognized field \"name\"") }, true) + result.errors.forEach { assertContains(it.message, "Unrecognized field \"name\"") } // Jackson drops what none of the unwrapped properties know assertEquals(result.getData>()!!["unwrapped"], "c null") } @@ -738,10 +828,40 @@ class UnmappedInputFieldsTest { // it only fails once the argument is converted, as it did before the check val result = GraphQL.newGraphQL(schema).build().execute("query { broken(input: { name: \"a\" }) }") - assertEquals(result.errors.single().message.contains("Conflicting property-based creators"), true) + assertContains(result.errors.single().message, "Conflicting property-based creators") } - private fun failOptions() =SchemaParserOptions.newOptions().unmappedInputFields(UnmappedInputFields.FAIL) + @Test + fun `error while Jackson looks up a deserializer doesn't fail the build`() { + val (_, warnings) = withWarnings { + SchemaParser.newParser() + .schemaString( + """ + type Query { + greet(input: GreetingInput!): String + } + + input GreetingInput { + name: String! + } + """) + .resolvers(object : GraphQLQueryResolver { + fun greet(input: GreetingInput): String? = input.nmae + }) + .options(SchemaParserOptions.newOptions() + .objectMapperConfigurer { mapper, _ -> mapper.registerModule(SimpleModule().apply { setDeserializers(MissingClassDeserializers()) }) } + .build()) + .build() + .makeExecutableSchema() + } + + // the lookup only fails once the argument is converted, as it did before the check + assertEquals(warnings, emptyList()) + } + + private fun failOptions() = SchemaParserOptions.newOptions().unmappedInputFields(UnmappedInputFields.FAIL) + + private fun assertContains(actual: String, expected: String) = assert(actual.contains(expected)) { "expected:<$actual> to contain:<$expected>" } private fun projectParser(options: SchemaParserOptions = SchemaParserOptions.newOptions().build()) = SchemaParser.newParser() .schemaString( @@ -910,6 +1030,42 @@ class UnmappedInputFieldsTest { var side: Double? = null } + @JsonTypeInfo(use = JsonTypeInfo.Id.NAME, property = "kind") + @JsonSubTypes(JsonSubTypes.Type(Animal::class, name = "animal"), JsonSubTypes.Type(Dog::class, name = "dog")) + open class Animal { + var name: String? = null + + open fun describe() = "animal $name" + } + + class Dog : Animal() { + var barks: Boolean? = null + + override fun describe() = "dog $name barks=$barks" + } + + // only polymorphic where a property says so + open class Pet { + var name: String? = null + + open fun describe() = "pet $name" + } + + class Cat : Pet() { + var meows: Boolean? = null + + override fun describe() = "cat $name meows=$meows" + } + + class Zoo { + var animal: Animal? = null + var animals: List? = null + + @field:JsonTypeInfo(use = JsonTypeInfo.Id.NAME, property = "kind") + @field:JsonSubTypes(JsonSubTypes.Type(Cat::class, name = "cat")) + var pets: List? = null + } + // the property doesn't match the schema, only the deserializer does class Money(val amount: Int) @@ -946,6 +1102,12 @@ class UnmappedInputFieldsTest { var stret: String? = null } + // like a class that refers to one missing from the classpath + class MissingClassDeserializers : SimpleDeserializers() { + override fun findBeanDeserializer(type: JavaType, config: DeserializationConfig, beanDesc: BeanDescription): JsonDeserializer<*> = + throw NoClassDefFoundError("com/example/Missing") + } + abstract class InputResolver : GraphQLQueryResolver { fun generic(input: I): String = input.toString() }