Skip to content

Commit 1450ee6

Browse files
oryan-blockclaude
andcommitted
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<String, bean> arguments and polymorphic properties, and contains assertions that show the actual message Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 43c1e50 commit 1450ee6

2 files changed

Lines changed: 208 additions & 33 deletions

File tree

‎src/main/kotlin/graphql/kickstart/tools/UnmappedInputFieldChecker.kt‎

Lines changed: 39 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package graphql.kickstart.tools
22

33
import com.fasterxml.jackson.databind.DeserializationContext
44
import com.fasterxml.jackson.databind.DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES
5+
import com.fasterxml.jackson.databind.JavaType
56
import com.fasterxml.jackson.databind.JsonDeserializer
67
import com.fasterxml.jackson.databind.ObjectMapper
78
import com.fasterxml.jackson.databind.deser.BeanDeserializerBase
@@ -35,25 +36,28 @@ internal class UnmappedInputFieldChecker(
3536
// a deserializer is cached by its mapper, so this is per mapper, class and input type, and ends recursive input types
3637
private val checked = mutableSetOf<Pair<JsonDeserializer<*>, String>>()
3738

38-
// reported once, for the first argument found using it
39-
private val messages = linkedMapOf<Triple<String, String, Class<*>>, String>()
39+
// reported once for each consequence, with the first argument found using it, since mappers differ in ignoring unknown properties
40+
private val messages = linkedMapOf<Unmapped, String>()
4041

4142
fun check(objectType: String, resolver: MethodFieldResolver) {
42-
val mapper = resolver.objectMapper
43-
val context = (mapper.deserializationContext as? DefaultDeserializationContext)?.createDummyInstance(mapper.deserializationConfig) ?: return
44-
4543
resolver.field.inputValueDefinitions.forEachIndexed { index, argument ->
4644
val inputObject = (argument.type.unwrap() as? TypeName)?.let { inputObjects[it.name] } ?: return@forEachIndexed
47-
val parameterType = resolver.getMethodParameterType(index) ?: return@forEachIndexed
4845
val usage = "argument '${argument.name}' of field '$objectType.${resolver.field.name}'"
4946

5047
try {
48+
val parameterType = resolver.getMethodParameterType(index) ?: return@forEachIndexed
49+
val mapper = resolver.objectMapper
50+
val context = (mapper.deserializationContext as? DefaultDeserializationContext)
51+
?.createDummyInstance(mapper.deserializationConfig) ?: return@forEachIndexed
5152
// the same lookup as convertValue's when it converts the argument
5253
val deserializer = context.findRootValueDeserializer(mapper.typeFactory.constructType(parameterType))
5354
checkFields(mapper, context, deserializer, inputObject, usage)
54-
} catch (e: Exception) {
55-
// e.g. a class Jackson can't deserialize, which only fails once the argument is converted, as it did before this check
56-
log.debug("Can't check the input fields of $usage with Jackson's deserializer for $parameterType", e)
55+
} catch (e: VirtualMachineError) {
56+
throw e
57+
} catch (e: Throwable) {
58+
// e.g. a class Jackson can't deserialize, or one that refers to a missing class, which only fails once the argument is
59+
// converted, as it did before this check. Errors are caught too, since the lookup used to run on the first query
60+
log.debug("Can't check the input fields of $usage with Jackson", e)
5761
}
5862
}
5963
}
@@ -85,9 +89,8 @@ internal class UnmappedInputFieldChecker(
8589
val property = beanDeserializer.findProperty(field.name)
8690
if (property == null) {
8791
knownPropertiesIfUnmapped(mapper, context, beanDeserializer, field.name)?.let { knownProperties ->
88-
messages.getOrPut(Triple(inputObject.name, field.name, beanDeserializer.handledType())) {
89-
message(mapper, inputObject, field, beanDeserializer.handledType(), usage, knownProperties)
90-
}
92+
val unmapped = Unmapped(inputObject.name, field.name, beanDeserializer.handledType(), consequence(mapper, inputObject, field))
93+
messages.getOrPut(unmapped) { message(unmapped, usage, knownProperties) }
9194
}
9295
} else if (property.valueTypeDeserializer == null) {
9396
// a polymorphic property's class is only known once its type id is read
@@ -103,12 +106,24 @@ internal class UnmappedInputFieldChecker(
103106
is BeanDeserializerBase -> deserializer.takeUnless { it.valueInstantiator.canCreateUsingDelegate() }
104107
// a Map gets the fields as its keys
105108
is ContainerDeserializerBase<*> ->
106-
if (deserializer.valueType.isMapLikeType) null else beanDeserializer(context, deserializer.contentDeserializer)
107-
is ReferenceTypeDeserializer<*> ->
108-
beanDeserializer(context, context.findContextualValueDeserializer(deserializer.valueType.referencedType, null))
109+
if (deserializer.valueType.isMapLikeType || isPolymorphic(context, deserializer.contentType)) null
110+
else beanDeserializer(context, deserializer.contentDeserializer)
111+
is ReferenceTypeDeserializer<*> -> {
112+
val referencedType = deserializer.valueType.referencedType
113+
if (isPolymorphic(context, referencedType)) null
114+
else beanDeserializer(context, context.findContextualValueDeserializer(referencedType, null))
115+
}
109116
else -> null
110117
}
111118

119+
/**
120+
* 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
121+
* type id is read. Their deserializer is the base class's, which doesn't know the subclasses' properties. The same lookup as
122+
* Jackson's when it creates the deserializer: type info from a property's annotation, or else from the class or default typing.
123+
*/
124+
private fun isPolymorphic(context: DeserializationContext, contentType: JavaType): Boolean =
125+
contentType.getTypeHandler<Any?>() != null || context.factory.findTypeDeserializer(context.config, contentType) != null
126+
112127
/**
113128
* Returns the properties Jackson knows if it fails on the field, or null if it maps or ignores it.
114129
*
@@ -140,23 +155,21 @@ internal class UnmappedInputFieldChecker(
140155
private fun inputObjectOf(type: GraphQLType): GraphQLInputObjectType? =
141156
(GraphQLTypeUtil.unwrapAllAs<GraphQLType>(type) as? GraphQLNamedType)?.let { inputObjects[it.name] }
142157

143-
private fun message(
144-
mapper: ObjectMapper,
145-
inputObject: GraphQLInputObjectType,
146-
field: GraphQLInputObjectField,
147-
clazz: Class<*>,
148-
usage: String,
149-
knownProperties: Collection<Any>
150-
): String {
151-
val consequence = when {
158+
private fun consequence(mapper: ObjectMapper, inputObject: GraphQLInputObjectType, field: GraphQLInputObjectField): String =
159+
when {
152160
!mapper.deserializationConfig.isEnabled(FAIL_ON_UNKNOWN_PROPERTIES) -> "drops the field's value, since the ObjectMapper ignores unknown properties"
153161
field.hasSetDefaultValue() -> "fails for every '${inputObject.name}' value sent, since the field has a default value"
154162
else -> "fails whenever the field is sent"
155163
}
164+
165+
private fun message(unmapped: Unmapped, usage: String, knownProperties: Collection<Any>): String {
156166
val properties = knownProperties.map { it.toString() }.sorted().ifEmpty { listOf("none") }
157167

158-
return "Input field '${inputObject.name}.${field.name}' has no matching property in ${clazz.name}. Converting $usage $consequence. " +
168+
return "Input field '${unmapped.inputObject}.${unmapped.field}' has no matching property in ${unmapped.clazz.name}. " +
169+
"Converting $usage ${unmapped.consequence}. " +
159170
"Known properties: ${properties.joinToString(", ")}. " +
160-
"Rename the property or map it with @JsonProperty(\"${field.name}\"). If it's left unmapped on purpose, list it in @JsonIgnoreProperties on the class."
171+
"Rename the property or map it with @JsonProperty(\"${unmapped.field}\"). If it's left unmapped on purpose, list it in @JsonIgnoreProperties on the class."
161172
}
173+
174+
private data class Unmapped(val inputObject: String, val field: String, val clazz: Class<*>, val consequence: String)
162175
}

0 commit comments

Comments
 (0)