From 43c1e50eb8372f43304b4db7708dc220ccaf4298 Mon Sep 17 00:00:00 2001 From: Oryan Date: Mon, 12 Oct 2026 15:52:04 -0400 Subject: [PATCH 1/8] 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 | 6 +- .../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(+), 10 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 5459c470..75963704 100644 --- a/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt +++ b/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt @@ -650,8 +650,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}. " + - "If Jackson doesn't map it some other way, e.g. through @JsonProperty, 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 ce1d4d3b..a346c41f 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.extensionsOf import graphql.kickstart.tools.util.getDocumentation import graphql.kickstart.tools.util.unwrap @@ -85,7 +86,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) } @@ -125,7 +129,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 extensions = typeExtensions.extensionsOf(name) val directives = directivesOf(objectDefinition, extensions) @@ -147,11 +156,15 @@ class SchemaParser internal constructor( objectDefinition.withExtensions(typeExtensions).flatMap { it.fieldDefinitions }.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 d1c6a5e8..3f9afa5f 100644 --- a/src/main/kotlin/graphql/kickstart/tools/SchemaParserOptions.kt +++ b/src/main/kotlin/graphql/kickstart/tools/SchemaParserOptions.kt @@ -37,7 +37,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 @@ -68,6 +69,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 @@ -162,6 +164,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 + } + fun build(): SchemaParserOptions { val coroutineContextProvider = coroutineContextProvider ?: DefaultCoroutineContextProvider(Dispatchers.Default) @@ -203,7 +223,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 c9a1046f..50d296d9 100644 --- a/src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt +++ b/src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt @@ -1,5 +1,6 @@ package graphql.kickstart.tools.resolver +import com.fasterxml.jackson.databind.ObjectMapper import graphql.GraphQLContext import graphql.kickstart.tools.* import graphql.kickstart.tools.SchemaParserOptions.GenericWrapper @@ -48,9 +49,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) { @@ -198,7 +202,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 1450ee6c9d0deb5fd087e9bbfec3d8768c3c1bcc Mon Sep 17 00:00:00 2001 From: Oryan Date: Mon, 12 Oct 2026 16:07:37 -0400 Subject: [PATCH 2/8] 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() } From f8821a197b253c318d9220f1a562fd78cdd03e17 Mon Sep 17 00:00:00 2001 From: Oryan Date: Sun, 18 Oct 2026 12:34:59 -0400 Subject: [PATCH 3/8] Apply an Optional property's annotations in the unmapped input field check --- .../kickstart/tools/UnmappedInputFieldChecker.kt | 15 +++++++++------ .../kickstart/tools/UnmappedInputFieldsTest.kt | 10 +++++++--- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/src/main/kotlin/graphql/kickstart/tools/UnmappedInputFieldChecker.kt b/src/main/kotlin/graphql/kickstart/tools/UnmappedInputFieldChecker.kt index 2378c362..1980bdac 100644 --- a/src/main/kotlin/graphql/kickstart/tools/UnmappedInputFieldChecker.kt +++ b/src/main/kotlin/graphql/kickstart/tools/UnmappedInputFieldChecker.kt @@ -1,5 +1,6 @@ package graphql.kickstart.tools +import com.fasterxml.jackson.databind.BeanProperty import com.fasterxml.jackson.databind.DeserializationContext import com.fasterxml.jackson.databind.DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES import com.fasterxml.jackson.databind.JavaType @@ -51,7 +52,7 @@ internal class UnmappedInputFieldChecker( ?.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) + checkFields(mapper, context, deserializer, null, inputObject, usage) } catch (e: VirtualMachineError) { throw e } catch (e: Throwable) { @@ -77,10 +78,11 @@ internal class UnmappedInputFieldChecker( mapper: ObjectMapper, context: DefaultDeserializationContext, deserializer: JsonDeserializer<*>?, + property: BeanProperty?, inputObject: GraphQLInputObjectType, usage: String ) { - val beanDeserializer = beanDeserializer(context, deserializer) ?: return + val beanDeserializer = beanDeserializer(context, deserializer, property) ?: return if (!checked.add(beanDeserializer to inputObject.name)) { return } @@ -94,24 +96,25 @@ internal class UnmappedInputFieldChecker( } } 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) } + inputObjectOf(field.type)?.let { checkFields(mapper, context, property.valueDeserializer, property, 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? = + private tailrec fun beanDeserializer(context: DeserializationContext, deserializer: JsonDeserializer<*>?, property: BeanProperty?): 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 || isPolymorphic(context, deserializer.contentType)) null - else beanDeserializer(context, deserializer.contentDeserializer) + else beanDeserializer(context, deserializer.contentDeserializer, property) is ReferenceTypeDeserializer<*> -> { val referencedType = deserializer.valueType.referencedType + // looked up with the property, like Jackson does, so that its annotations apply, e.g. @JsonIgnoreProperties if (isPolymorphic(context, referencedType)) null - else beanDeserializer(context, context.findContextualValueDeserializer(referencedType, null)) + else beanDeserializer(context, context.findContextualValueDeserializer(referencedType, property), property) } else -> null } diff --git a/src/test/kotlin/graphql/kickstart/tools/UnmappedInputFieldsTest.kt b/src/test/kotlin/graphql/kickstart/tools/UnmappedInputFieldsTest.kt index d3f63e5a..b20152b6 100644 --- a/src/test/kotlin/graphql/kickstart/tools/UnmappedInputFieldsTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/UnmappedInputFieldsTest.kt @@ -577,6 +577,7 @@ class UnmappedInputFieldsTest { input PropertyIgnoralsInput { inner: InnerInput + optionalInner: InnerInput } input InnerInput { @@ -609,7 +610,7 @@ class UnmappedInputFieldsTest { 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 propertyIgnorals(input: PropertyIgnoralsInput): String = "${input.inner?.name}, ${input.optionalInner?.get()?.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 @@ -627,7 +628,7 @@ class UnmappedInputFieldsTest { anySetter(input: { a: 1, b: "c" }) snakeCase(input: { first_name: "d" }) caseInsensitive(input: { NAME: "e" }) - propertyIgnorals(input: { inner: { name: "f", legacy: "x" } }) + propertyIgnorals(input: { inner: { name: "f", legacy: "x" }, optionalInner: { name: "m", legacy: "y" } }) filter(input: { and: [{ or: [{ name: "g" }] }] }) map(input: { anything: "h", filter: { name: "i" } }) beanMap(input: { first: { name: "k" }, second: { name: "l" } }) @@ -641,7 +642,7 @@ class UnmappedInputFieldsTest { "anySetter" to "{a=1, b=c}", "snakeCase" to "d", "caseInsensitive" to "e", - "propertyIgnorals" to "f", + "propertyIgnorals" to "f, m", "filter" to "g", "map" to "{anything=h, filter={name=i}}", "beanMap" to "{first=k, second=l}", @@ -992,6 +993,9 @@ class UnmappedInputFieldsTest { class PropertyIgnoralsInput { @field:JsonIgnoreProperties("legacy") var inner: InnerInput? = null + + @field:JsonIgnoreProperties("legacy") + var optionalInner: Optional? = null } class InnerInput { From 49592d7aa92ee0a417507398b7569594ec29dc3d Mon Sep 17 00:00:00 2001 From: Oryan Date: Sun, 18 Oct 2026 12:50:01 -0400 Subject: [PATCH 4/8] Compile the Java tests on the class path so they build on Java 11 --- pom.xml | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/pom.xml b/pom.xml index 69df3183..a553ddb4 100644 --- a/pom.xml +++ b/pom.xml @@ -294,6 +294,16 @@ 11 true + + + + default-testCompile + + false + + + From 65d8152840b82ba2d2b08cb1a4a061f5f605cb14 Mon Sep 17 00:00:00 2001 From: Oryan Date: Sun, 18 Oct 2026 13:01:39 -0400 Subject: [PATCH 5/8] Explain why the Java tests compile on the class path --- pom.xml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/pom.xml b/pom.xml index a553ddb4..f9e50d44 100644 --- a/pom.xml +++ b/pom.xml @@ -296,8 +296,10 @@ - + + default-testCompile false From ae7449e061dca648ad26101c16d3ceaa8e05f5e3 Mon Sep 17 00:00:00 2001 From: Oryan Date: Sun, 18 Oct 2026 13:38:01 -0400 Subject: [PATCH 6/8] Drop unmapped input field test cases that repeat another case --- .../tools/UnmappedInputFieldsFixtures.java | 25 --- .../tools/UnmappedInputFieldsTest.kt | 143 ++++-------------- 2 files changed, 27 insertions(+), 141 deletions(-) diff --git a/src/test/kotlin/graphql/kickstart/tools/UnmappedInputFieldsFixtures.java b/src/test/kotlin/graphql/kickstart/tools/UnmappedInputFieldsFixtures.java index 9180fd2b..3ba4faf8 100644 --- a/src/test/kotlin/graphql/kickstart/tools/UnmappedInputFieldsFixtures.java +++ b/src/test/kotlin/graphql/kickstart/tools/UnmappedInputFieldsFixtures.java @@ -115,31 +115,6 @@ public String describe() { } } - 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; diff --git a/src/test/kotlin/graphql/kickstart/tools/UnmappedInputFieldsTest.kt b/src/test/kotlin/graphql/kickstart/tools/UnmappedInputFieldsTest.kt index b20152b6..7bb65261 100644 --- a/src/test/kotlin/graphql/kickstart/tools/UnmappedInputFieldsTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/UnmappedInputFieldsTest.kt @@ -5,7 +5,6 @@ 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 @@ -28,7 +27,6 @@ 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 import graphql.kickstart.tools.UnmappedInputFieldsFixtures.* import graphql.schema.DataFetchingEnvironment @@ -65,49 +63,26 @@ class UnmappedInputFieldsTest { } @Test - fun `misspelled scalar property of an input class is reported`() { - val (_, warnings) = withWarnings { + fun `misspelled scalar property and Kotlin property named like a boolean getter are reported`() { + val (schema, warnings) = withWarnings { SchemaParser.newParser() .schemaString( """ type Query { greet(input: GreetingInput!): String + active(input: ActiveInput!): Boolean } 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 greet(input: GreetingInput): String? = input.nmae fun active(input: ActiveInput): Boolean = input.isActive }) .build() @@ -115,7 +90,10 @@ class UnmappedInputFieldsTest { } // 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"))) + assertEquals(warnings, listOf( + unmapped("GreetingInput.name", GreetingInput::class.java, "argument 'input' of field 'Query.greet'", "nmae"), + 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 }) }") assertContains(result.errors.single().message, "Unrecognized field \"active\"") @@ -416,8 +394,6 @@ class UnmappedInputFieldsTest { """ 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 @@ -437,14 +413,6 @@ class UnmappedInputFieldsTest { value: String } - input BooleanGetterInput { - active: Boolean! - } - - input JsonCreatorInput { - name: String - } - input ConstructorPropertiesInput { name: String } @@ -484,8 +452,6 @@ class UnmappedInputFieldsTest { """) .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() @@ -507,8 +473,6 @@ class UnmappedInputFieldsTest { """ 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" }) @@ -523,8 +487,6 @@ class UnmappedInputFieldsTest { assertEquals(data, mapOf( "setterOnly" to "a b", - "booleanGetter" to true, - "jsonCreator" to "c", "constructorProperties" to "d", "privateField" to "e", "alias" to "f", @@ -543,14 +505,11 @@ class UnmappedInputFieldsTest { """ 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 beanMap(input: BeanMapInput!): String - objectNode(input: ObjectNodeInput!): String } input DataClassInput { @@ -558,15 +517,6 @@ class UnmappedInputFieldsTest { count: Int } - extend input DataClassInput { - label: String - } - - input KotlinAnySetterInput { - a: Int - b: String - } - input SnakeCaseInput { first_name: String } @@ -591,31 +541,19 @@ class UnmappedInputFieldsTest { name: String } - input MapInput { - anything: String - filter: RequestFilter - } - input BeanMapInput { first: RequestFilter second: 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}, ${input.optionalInner?.get()?.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()) .build() @@ -624,29 +562,23 @@ class UnmappedInputFieldsTest { val data = assertNoGraphQlErrors(GraphQL.newGraphQL(schema).build()) { """ query { - dataClass(input: { name: "a", label: "b" }) - anySetter(input: { a: 1, b: "c" }) + dataClass(input: { name: "a" }) snakeCase(input: { first_name: "d" }) caseInsensitive(input: { NAME: "e" }) propertyIgnorals(input: { inner: { name: "f", legacy: "x" }, optionalInner: { name: "m", legacy: "y" } }) 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" }) } """ } assertEquals(data, mapOf( - "dataClass" to "DataClassInput(name=a, count=1, label=b)", - "anySetter" to "{a=1, b=c}", + "dataClass" to "DataClassInput(name=a, count=1)", "snakeCase" to "d", "caseInsensitive" to "e", "propertyIgnorals" to "f, m", "filter" to "g", - "map" to "{anything=h, filter={name=i}}", - "beanMap" to "{first=k, second=l}", - "objectNode" to """{"anything":"j"}""" + "beanMap" to "{first=k, second=l}" )) } @@ -808,58 +740,42 @@ class UnmappedInputFieldsTest { } @Test - fun `input class that Jackson can't deserialize doesn't fail the build`() { + fun `input classes whose deserializer Jackson can't look up don't fail the build`() { val schema = SchemaParser.newParser() .schemaString( """ type Query { broken(input: BrokenInput!): String + missingClass(input: GreetingInput!): String } input BrokenInput { name: String } + + input GreetingInput { + name: String! + } """) .resolvers(object : GraphQLQueryResolver { fun broken(input: ConflictingCreatorsInput): String = input.toString() + fun missingClass(input: GreetingInput): String? = input.nmae }) - .options(failOptions().build()) + .options(failOptions() + .objectMapperConfigurer { mapper, context -> + if (context.field.name == "missingClass") { + mapper.registerModule(SimpleModule().apply { setDeserializers(MissingClassDeserializers()) }) + } + } + .build()) .build() .makeExecutableSchema() - // it only fails once the argument is converted, as it did before the check + // the lookups only fail once the arguments are converted, as they did before the check val result = GraphQL.newGraphQL(schema).build().execute("query { broken(input: { name: \"a\" }) }") assertContains(result.errors.single().message, "Conflicting property-based creators") } - @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>" } @@ -973,12 +889,7 @@ class UnmappedInputFieldsTest { 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() - } + data class DataClassInput(val name: String, val count: Int = 1) @JsonNaming(PropertyNamingStrategies.SnakeCaseStrategy::class) class SnakeCaseInput { From af2df65f8a2bbc77664c24c53ffa578e242e1eaf Mon Sep 17 00:00:00 2001 From: Oryan Date: Sun, 18 Oct 2026 13:51:41 -0400 Subject: [PATCH 7/8] Check the input classes in one pass after the objects are built --- .../graphql/kickstart/tools/SchemaParser.kt | 27 ++++++++----------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/src/main/kotlin/graphql/kickstart/tools/SchemaParser.kt b/src/main/kotlin/graphql/kickstart/tools/SchemaParser.kt index a346c41f..9253cc9f 100644 --- a/src/main/kotlin/graphql/kickstart/tools/SchemaParser.kt +++ b/src/main/kotlin/graphql/kickstart/tools/SchemaParser.kt @@ -86,10 +86,14 @@ class SchemaParser internal constructor( } } val interfaces = interfaceDefinitions.map { createInterfaceObject(it, 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 objects = objectDefinitions.map { createObject(it, interfaces, inputObjects) } + if (options.unmappedInputFields != UnmappedInputFields.IGNORE) { + val checker = UnmappedInputFieldChecker(inputObjects.associateBy { it.name }, options.unmappedInputFields) + objectDefinitions.forEach { type -> + fieldResolversByType[type].orEmpty().values.filterIsInstance().forEach { checker.check(type.name, it) } + } + checker.report() + } val unions = unionDefinitions.map { createUnionObject(it, objects) } val enums = enumDefinitions.map { createEnumObject(it) } @@ -129,12 +133,7 @@ class SchemaParser internal constructor( @Suppress("unused") fun getUnusedDefinitions(): Set> = unusedDefinitions - private fun createObject( - objectDefinition: ObjectTypeDefinition, - interfaces: List, - inputObjects: List, - unmappedInputFieldChecker: UnmappedInputFieldChecker? - ): GraphQLObjectType { + private fun createObject(objectDefinition: ObjectTypeDefinition, interfaces: List, inputObjects: List): GraphQLObjectType { val name = objectDefinition.name val extensions = typeExtensions.extensionsOf(name) val directives = directivesOf(objectDefinition, extensions) @@ -156,15 +155,11 @@ class SchemaParser internal constructor( objectDefinition.withExtensions(typeExtensions).flatMap { it.fieldDefinitions }.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), - fieldResolver.createDataFetcher() + 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") ) - if (fieldResolver is MethodFieldResolver) { - unmappedInputFieldChecker?.check(objectDefinition.name, fieldResolver) - } val wiredField = field.build() GraphQLFieldDefinition.Builder(wiredField) From e7f8e2caa9a6d9bad3300b5fd26edaa0317a9956 Mon Sep 17 00:00:00 2001 From: Oryan Date: Sun, 18 Oct 2026 14:13:41 -0400 Subject: [PATCH 8/8] Remove the scanner's log for input fields without a property --- .../kotlin/graphql/kickstart/tools/SchemaClassScanner.kt | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt b/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt index 75963704..fe903b99 100644 --- a/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt +++ b/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt @@ -649,13 +649,6 @@ internal class SchemaClassScanner( * 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)) { - // 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]) { is ScalarTypeDefinition -> handleFoundScalarType(definition) is InputObjectTypeDefinition -> {