Skip to content

Commit 43c1e50

Browse files
oryan-blockclaude
andcommitted
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 <noreply@anthropic.com>
1 parent 77bfb83 commit 43c1e50

8 files changed

Lines changed: 1422 additions & 10 deletions

File tree

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -650,8 +650,10 @@ internal class SchemaClassScanner(
650650
*/
651651
private fun handleInputValueWithoutJavaType(graphQLType: InputObjectTypeDefinition, inputValueDefinition: InputValueDefinition, inputGraphQLType: TypeName, clazz: Class<*>?) {
652652
if (clazz != null && !isPropertyMap(clazz)) {
653-
log.warn("Cannot find a public getter or field for field '${inputValueDefinition.name}: ${inputGraphQLType.name}' of input type '${graphQLType.name}' on ${clazz.name}. " +
654-
"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.")
653+
// not a warning, since Jackson may map the field some other way, e.g. through a setter. The SchemaParser checks that with
654+
// Jackson itself, see UnmappedInputFields
655+
log.debug("Cannot find a public getter or field for field '${inputValueDefinition.name}: ${inputGraphQLType.name}' of input type '${graphQLType.name}' on ${clazz.name}, " +
656+
"so its type is taken from the schema.")
655657
}
656658

657659
when (val definition = definitionsByName[inputGraphQLType.name]) {

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

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import graphql.Scalars
55
import graphql.introspection.Introspection
66
import graphql.introspection.Introspection.DirectiveLocation.INPUT_FIELD_DEFINITION
77
import graphql.kickstart.tools.directive.DirectiveWiringHelper
8+
import graphql.kickstart.tools.resolver.MethodFieldResolver
89
import graphql.kickstart.tools.util.extensionsOf
910
import graphql.kickstart.tools.util.getDocumentation
1011
import graphql.kickstart.tools.util.unwrap
@@ -85,7 +86,10 @@ class SchemaParser internal constructor(
8586
}
8687
}
8788
val interfaces = interfaceDefinitions.map { createInterfaceObject(it, inputObjects) }
88-
val objects = objectDefinitions.map { createObject(it, interfaces, inputObjects) }
89+
val unmappedInputFieldChecker = if (options.unmappedInputFields == UnmappedInputFields.IGNORE) null
90+
else UnmappedInputFieldChecker(inputObjects.associateBy { it.name }, options.unmappedInputFields)
91+
val objects = objectDefinitions.map { createObject(it, interfaces, inputObjects, unmappedInputFieldChecker) }
92+
unmappedInputFieldChecker?.report()
8993
val unions = unionDefinitions.map { createUnionObject(it, objects) }
9094
val enums = enumDefinitions.map { createEnumObject(it) }
9195

@@ -125,7 +129,12 @@ class SchemaParser internal constructor(
125129
@Suppress("unused")
126130
fun getUnusedDefinitions(): Set<TypeDefinition<*>> = unusedDefinitions
127131

128-
private fun createObject(objectDefinition: ObjectTypeDefinition, interfaces: List<GraphQLInterfaceType>, inputObjects: List<GraphQLInputObjectType>): GraphQLObjectType {
132+
private fun createObject(
133+
objectDefinition: ObjectTypeDefinition,
134+
interfaces: List<GraphQLInterfaceType>,
135+
inputObjects: List<GraphQLInputObjectType>,
136+
unmappedInputFieldChecker: UnmappedInputFieldChecker?
137+
): GraphQLObjectType {
129138
val name = objectDefinition.name
130139
val extensions = typeExtensions.extensionsOf<ObjectTypeExtensionDefinition>(name)
131140
val directives = directivesOf(objectDefinition, extensions)
@@ -147,11 +156,15 @@ class SchemaParser internal constructor(
147156
objectDefinition.withExtensions(typeExtensions).flatMap { it.fieldDefinitions }.forEach { fieldDefinition ->
148157
field { field ->
149158
createField(field, fieldDefinition, inputObjects)
159+
val fieldResolver = fieldResolversByType[objectDefinition]?.get(fieldDefinition)
160+
?: 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")
150161
codeRegistryBuilder.dataFetcher(
151162
FieldCoordinates.coordinates(objectDefinition.name, fieldDefinition.name),
152-
fieldResolversByType[objectDefinition]?.get(fieldDefinition)?.createDataFetcher()
153-
?: 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")
163+
fieldResolver.createDataFetcher()
154164
)
165+
if (fieldResolver is MethodFieldResolver) {
166+
unmappedInputFieldChecker?.check(objectDefinition.name, fieldResolver)
167+
}
155168

156169
val wiredField = field.build()
157170
GraphQLFieldDefinition.Builder(wiredField)

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

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,8 @@ data class SchemaParserOptions internal constructor(
3737
val fieldVisibility: GraphqlFieldVisibility?,
3838
val includeUnusedTypes: Boolean,
3939
val useCommentsForDescriptions: Boolean,
40-
val allowUndeclaredDirectives: Boolean
40+
val allowUndeclaredDirectives: Boolean,
41+
val unmappedInputFields: UnmappedInputFields
4142
) {
4243
companion object {
4344
@JvmStatic
@@ -68,6 +69,7 @@ data class SchemaParserOptions internal constructor(
6869
private var includeUnusedTypes = false
6970
private var useCommentsForDescriptions = true
7071
private var allowUndeclaredDirectives = false
72+
private var unmappedInputFields = UnmappedInputFields.WARN
7173

7274
fun contextClass(contextClass: Class<*>) = this.apply {
7375
this.contextClass = contextClass
@@ -162,6 +164,24 @@ data class SchemaParserOptions internal constructor(
162164
this.allowUndeclaredDirectives = allowUndeclaredDirectives
163165
}
164166

167+
/**
168+
* What to do about input object fields that the class of a resolver argument has no property for. Jackson fails to convert
169+
* the argument when such a field is sent, or drops the field's value if the ObjectMapper ignores unknown properties.
170+
* Defaults to [UnmappedInputFields.WARN].
171+
*
172+
* The classes are checked when the schema is built, with the deserializer of each resolver field's own ObjectMapper, so
173+
* anything that maps a property for Jackson counts, e.g. setters, creators, naming strategies, aliases, ignored properties
174+
* and any setters. A field is still reported if the ObjectMapper ignores unknown properties, since that's usually set for
175+
* every class at once, but not if its class has `@JsonIgnoreProperties(ignoreUnknown = true)`.
176+
*
177+
* Classes that Jackson doesn't deserialize property by property aren't checked: those with a custom deserializer, a
178+
* delegating creator, a builder or type info, and those with injected values. Neither are classes with `@JsonUnwrapped`
179+
* properties, whose unknown fields Jackson drops, nor arguments bound to a Map, a JsonNode or a type variable.
180+
*/
181+
fun unmappedInputFields(unmappedInputFields: UnmappedInputFields) = this.apply {
182+
this.unmappedInputFields = unmappedInputFields
183+
}
184+
165185
fun build(): SchemaParserOptions {
166186
val coroutineContextProvider = coroutineContextProvider
167187
?: DefaultCoroutineContextProvider(Dispatchers.Default)
@@ -203,7 +223,8 @@ data class SchemaParserOptions internal constructor(
203223
fieldVisibility,
204224
includeUnusedTypes,
205225
useCommentsForDescriptions,
206-
allowUndeclaredDirectives
226+
allowUndeclaredDirectives,
227+
unmappedInputFields
207228
)
208229
}
209230
}
Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
1+
package graphql.kickstart.tools
2+
3+
import com.fasterxml.jackson.databind.DeserializationContext
4+
import com.fasterxml.jackson.databind.DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES
5+
import com.fasterxml.jackson.databind.JsonDeserializer
6+
import com.fasterxml.jackson.databind.ObjectMapper
7+
import com.fasterxml.jackson.databind.deser.BeanDeserializerBase
8+
import com.fasterxml.jackson.databind.deser.DefaultDeserializationContext
9+
import com.fasterxml.jackson.databind.deser.std.ContainerDeserializerBase
10+
import com.fasterxml.jackson.databind.deser.std.ReferenceTypeDeserializer
11+
import com.fasterxml.jackson.databind.exc.UnrecognizedPropertyException
12+
import graphql.kickstart.tools.resolver.MethodFieldResolver
13+
import graphql.kickstart.tools.util.unwrap
14+
import graphql.language.TypeName
15+
import graphql.schema.GraphQLInputObjectField
16+
import graphql.schema.GraphQLInputObjectType
17+
import graphql.schema.GraphQLNamedType
18+
import graphql.schema.GraphQLType
19+
import graphql.schema.GraphQLTypeUtil
20+
import org.slf4j.LoggerFactory
21+
22+
/**
23+
* Checks that the classes of resolver arguments have a property for each field of their input objects, see [UnmappedInputFields].
24+
*
25+
* The fields are looked up in the deserializer that the resolver's ObjectMapper converts the argument with, not in the class
26+
* itself, so that a field counts as mapped whenever Jackson maps it. An input type can be used by fields whose mappers are
27+
* configured differently, so it's checked once for each mapper.
28+
*/
29+
internal class UnmappedInputFieldChecker(
30+
private val inputObjects: Map<String, GraphQLInputObjectType>,
31+
private val unmappedInputFields: UnmappedInputFields
32+
) {
33+
private val log = LoggerFactory.getLogger(javaClass)
34+
35+
// a deserializer is cached by its mapper, so this is per mapper, class and input type, and ends recursive input types
36+
private val checked = mutableSetOf<Pair<JsonDeserializer<*>, String>>()
37+
38+
// reported once, for the first argument found using it
39+
private val messages = linkedMapOf<Triple<String, String, Class<*>>, String>()
40+
41+
fun check(objectType: String, resolver: MethodFieldResolver) {
42+
val mapper = resolver.objectMapper
43+
val context = (mapper.deserializationContext as? DefaultDeserializationContext)?.createDummyInstance(mapper.deserializationConfig) ?: return
44+
45+
resolver.field.inputValueDefinitions.forEachIndexed { index, argument ->
46+
val inputObject = (argument.type.unwrap() as? TypeName)?.let { inputObjects[it.name] } ?: return@forEachIndexed
47+
val parameterType = resolver.getMethodParameterType(index) ?: return@forEachIndexed
48+
val usage = "argument '${argument.name}' of field '$objectType.${resolver.field.name}'"
49+
50+
try {
51+
// the same lookup as convertValue's when it converts the argument
52+
val deserializer = context.findRootValueDeserializer(mapper.typeFactory.constructType(parameterType))
53+
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)
57+
}
58+
}
59+
}
60+
61+
fun report() {
62+
if (messages.isEmpty()) {
63+
return
64+
}
65+
66+
if (unmappedInputFields == UnmappedInputFields.FAIL) {
67+
throw SchemaError(messages.values.joinToString("\n"))
68+
}
69+
messages.values.forEach { log.warn(it) }
70+
}
71+
72+
private fun checkFields(
73+
mapper: ObjectMapper,
74+
context: DefaultDeserializationContext,
75+
deserializer: JsonDeserializer<*>?,
76+
inputObject: GraphQLInputObjectType,
77+
usage: String
78+
) {
79+
val beanDeserializer = beanDeserializer(context, deserializer) ?: return
80+
if (!checked.add(beanDeserializer to inputObject.name)) {
81+
return
82+
}
83+
84+
inputObject.fields.forEach { field ->
85+
val property = beanDeserializer.findProperty(field.name)
86+
if (property == null) {
87+
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+
}
91+
}
92+
} else if (property.valueTypeDeserializer == null) {
93+
// a polymorphic property's class is only known once its type id is read
94+
inputObjectOf(field.type)?.let { checkFields(mapper, context, property.valueDeserializer, it, usage) }
95+
}
96+
}
97+
}
98+
99+
// the deserializer of the class an input object is converted to, past the lists, arrays and Optionals around it
100+
private tailrec fun beanDeserializer(context: DeserializationContext, deserializer: JsonDeserializer<*>?): BeanDeserializerBase? =
101+
when (deserializer) {
102+
// a delegating creator gets the whole input object, e.g. as a Map
103+
is BeanDeserializerBase -> deserializer.takeUnless { it.valueInstantiator.canCreateUsingDelegate() }
104+
// a Map gets the fields as its keys
105+
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+
else -> null
110+
}
111+
112+
/**
113+
* Returns the properties Jackson knows if it fails on the field, or null if it maps or ignores it.
114+
*
115+
* `{"<field>": null}` is read into a placeholder object rather than an instance of the class, so neither its constructors nor
116+
* its setters are called, but the field still goes through Jackson's handling of a property the class doesn't have: ignored
117+
* names, any setters, ignoreUnknown and problem handlers. Unknown properties are made to fail even if the mapper ignores them.
118+
* Any other exception, e.g. from a deserializer that can't read into an existing object, means the field can't be checked.
119+
*/
120+
private fun knownPropertiesIfUnmapped(
121+
mapper: ObjectMapper,
122+
context: DefaultDeserializationContext,
123+
deserializer: BeanDeserializerBase,
124+
field: String
125+
): Collection<Any>? {
126+
val config = mapper.deserializationConfig.with(FAIL_ON_UNKNOWN_PROPERTIES)
127+
return try {
128+
mapper.treeAsTokens(mapper.createObjectNode().putNull(field)).use { parser ->
129+
parser.nextToken()
130+
deserializer.deserialize(parser, context.createInstance(config, parser, mapper.injectableValues), Any())
131+
}
132+
null
133+
} catch (e: UnrecognizedPropertyException) {
134+
if (e.propertyName == field) e.knownPropertyIds.orEmpty() else null
135+
} catch (e: Exception) {
136+
null
137+
}
138+
}
139+
140+
private fun inputObjectOf(type: GraphQLType): GraphQLInputObjectType? =
141+
(GraphQLTypeUtil.unwrapAllAs<GraphQLType>(type) as? GraphQLNamedType)?.let { inputObjects[it.name] }
142+
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 {
152+
!mapper.deserializationConfig.isEnabled(FAIL_ON_UNKNOWN_PROPERTIES) -> "drops the field's value, since the ObjectMapper ignores unknown properties"
153+
field.hasSetDefaultValue() -> "fails for every '${inputObject.name}' value sent, since the field has a default value"
154+
else -> "fails whenever the field is sent"
155+
}
156+
val properties = knownProperties.map { it.toString() }.sorted().ifEmpty { listOf("none") }
157+
158+
return "Input field '${inputObject.name}.${field.name}' has no matching property in ${clazz.name}. Converting $usage $consequence. " +
159+
"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."
161+
}
162+
}
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
package graphql.kickstart.tools
2+
3+
/**
4+
* What the [SchemaParser] does about input object fields that the class of a resolver argument has no property for,
5+
* see [SchemaParserOptions.Builder.unmappedInputFields].
6+
*/
7+
enum class UnmappedInputFields {
8+
/** Doesn't check the classes. */
9+
IGNORE,
10+
11+
/** Logs a warning for each field. */
12+
WARN,
13+
14+
/** Fails with a [SchemaError] that lists every field. */
15+
FAIL
16+
}

‎src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt‎

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

3+
import com.fasterxml.jackson.databind.ObjectMapper
34
import graphql.GraphQLContext
45
import graphql.kickstart.tools.*
56
import graphql.kickstart.tools.SchemaParserOptions.GenericWrapper
@@ -48,9 +49,12 @@ internal class MethodFieldResolver(
4849
private val numberOfParameters = method.parameterCountWithoutContinuation()
4950
private val hasAdditionalParameter = numberOfParameters == (field.inputValueDefinitions.size + getIndexOffset() + 1)
5051

52+
// the arguments are converted with the same ObjectMapper that the UnmappedInputFieldChecker checks their classes with
53+
internal val objectMapper: ObjectMapper by lazy { options.objectMapperProvider.provide(field) }
54+
5155
override fun createDataFetcher(): DataFetcher<*> {
5256
val args = mutableListOf<ArgumentPlaceholder>()
53-
val mapper = options.objectMapperProvider.provide(field)
57+
val mapper = objectMapper
5458

5559
// Add source argument if this is a resolver (but not a root resolver)
5660
if (this.search.requiredFirstParameterType != null) {
@@ -198,7 +202,8 @@ internal class MethodFieldResolver(
198202

199203
private fun getJavaMethodParameterIndex(index: Int) = index + getIndexOffset()
200204

201-
private fun getMethodParameterType(index: Int): JavaType? {
205+
// the type that the value of the argument at the index is converted to
206+
internal fun getMethodParameterType(index: Int): JavaType? {
202207
val methodIndex = getJavaMethodParameterIndex(index)
203208
val parameters = method.parameterTypes
204209

0 commit comments

Comments
 (0)