Skip to content

Commit 8a80a72

Browse files
oryan-blockclaude
andcommitted
Register input field types that have no Java property
When the scanner couldn't find a Java property for an input field, e.g. because the input is bound to a Map or ObjectNode, the getter name doesn't match the schema field (getFoo_Back for foo_back) or the field is private, it only logged "Cannot find definition for field ..." and never registered the field's type. Nested input objects and custom scalars only reachable that way were left out of the scan, and the parser then failed with "Expected type 'X' to be a GraphQLInputType, but it wasn't!", which pointed users at the wrong problem. The scanner now still registers what such a field needs. A nested input object is registered without a class and its fields are scanned the same way; a class found for it later is set and scanned as usual, so it doesn't clash. A custom scalar is registered like any other. An enum can't be built without its Java class, so if none is found for it anywhere, the scan fails with a message naming the enum, the input type, the field and the class, and how to fix it. The garbled warning is replaced by one that's only logged when an input class (not a Map or JsonNode) lacks a matching property. Input fields of a Map or JsonNode are no longer matched against the methods and fields it inherits from the JDK or Jackson, so fields named like values, elements or fields work with ObjectNode. Getters declared by a Map subclass are still used. A class taken from the dictionary for an input field is matched against the field's unwrapped type, so a list field whose element type is in the dictionary no longer fails with "Java class is not a List". A misspelled property on an input class whose nested input type is only reachable through it used to fail the build with the misleading error above. It now builds with the warning and, like any other misspelled property, fails when a client sends the field, or the value is dropped if the ObjectMapper ignores unknown properties. There's still no build-time check that input class properties match the schema (#422). The parser's "Expected type ... to be a GraphQLInputType" error is replaced by one that tells apart a type that isn't defined, a type of the wrong kind (an output type used as an input or vice versa) and a type that was defined but never reached while scanning, with a hint for reaching it that depends on the kind of type. It names the field or argument using the type and its source location. Schemas passed through schemaString() are still parsed as one document without a source name, so their line numbers count from the start of all strings (#392). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 2dbc802 commit 8a80a72

6 files changed

Lines changed: 824 additions & 47 deletions

File tree

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

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

3+
import com.fasterxml.jackson.databind.JsonNode
34
import graphql.kickstart.tools.resolver.FieldResolver
45
import graphql.kickstart.tools.resolver.FieldResolverScanner
56
import graphql.kickstart.tools.util.*
@@ -8,6 +9,7 @@ import graphql.schema.GraphQLScalarType
89
import graphql.schema.idl.ScalarInfo
910
import org.apache.commons.lang3.ClassUtils
1011
import org.slf4j.LoggerFactory
12+
import java.lang.reflect.Member
1113
import java.lang.reflect.Method
1214

1315
/**
@@ -45,6 +47,7 @@ internal class SchemaClassScanner(
4547
private val typeClassMatcher = TypeClassMatcher(definitionsByName)
4648
private val dictionary = mutableMapOf<TypeDefinition<*>, DictionaryEntry>()
4749
private val unvalidatedTypes = mutableSetOf<TypeDefinition<*>>()
50+
private val enumsWithoutClass = mutableMapOf<EnumTypeDefinition, String>()
4851
private val queue = linkedSetOf<QueueItem>()
4952

5053
private val fieldResolversByType = mutableMapOf<ObjectTypeDefinition, MutableMap<FieldDefinition, FieldResolver>>()
@@ -173,6 +176,11 @@ internal class SchemaClassScanner(
173176
}
174177

175178
private fun validateAndCreateResult(rootTypeHolder: RootTypesHolder): ScannedSchemaObjects {
179+
enumsWithoutClass
180+
.filter { dictionary[it.key]?.javaType == null }
181+
.values.firstOrNull()
182+
?.let { throw SchemaClassScannerError(it) }
183+
176184
initialDictionary
177185
.filter { !it.value.accessed }
178186
.forEach {
@@ -384,31 +392,83 @@ internal class SchemaClassScanner(
384392
scanInterfacesOfType(graphQLType)
385393
}
386394

387-
is InputObjectTypeDefinition -> {
388-
val inputObjectTypes = listOf(graphQLType) + inputExtensionDefinitions.filter { it.name == graphQLType.name }
389-
inputObjectTypes
390-
.flatMap { it.inputValueDefinitions }
391-
.forEach { inputValueDefinition ->
392-
val inputGraphQLType = inputValueDefinition.type.unwrap()
393-
if (inputGraphQLType is TypeName && !ScalarInfo.GRAPHQL_SPECIFICATION_SCALARS_DEFINITIONS.containsKey(inputGraphQLType.name)) {
394-
val inputValueJavaType = findInputValueType(inputValueDefinition.name, inputGraphQLType, javaType.unwrap())
395-
if (inputValueJavaType != null) {
396-
handleFoundType(typeClassMatcher.match(TypeClassMatcher.PotentialMatch.parameterType(
397-
inputValueDefinition.type,
398-
inputValueJavaType,
399-
GenericType(javaType, options).relativeToType(javaType),
400-
InputObjectReference(inputValueDefinition)
401-
)))
402-
} else {
403-
var mappingAdvice = "Try adding it manually to the dictionary"
404-
if (javaType.unwrap().name.contains("Map")) {
405-
mappingAdvice = " or add a class to represent your input type instead of a Map."
406-
}
407-
log.warn("Cannot find definition for field '${inputValueDefinition.name}: ${inputGraphQLType.name}' on input type '${graphQLType.name}' -> ${javaType.unwrap().name}. $mappingAdvice")
408-
}
409-
}
395+
is InputObjectTypeDefinition -> handleInputObjectFields(graphQLType, javaType)
396+
}
397+
}
398+
399+
/**
400+
* Finds the types of the input object's fields through the properties of its class. A null [javaType] means the input object
401+
* has no known class, e.g. because it's only reachable through a Map.
402+
*/
403+
private fun handleInputObjectFields(graphQLType: InputObjectTypeDefinition, javaType: JavaType?) {
404+
val inputObjectTypes = listOf(graphQLType) + inputExtensionDefinitions.filter { it.name == graphQLType.name }
405+
inputObjectTypes
406+
.flatMap { it.inputValueDefinitions }
407+
.forEach { inputValueDefinition ->
408+
val inputGraphQLType = inputValueDefinition.type.unwrap()
409+
if (inputGraphQLType is TypeName && !ScalarInfo.GRAPHQL_SPECIFICATION_SCALARS_DEFINITIONS.containsKey(inputGraphQLType.name)) {
410+
val propertyType = javaType?.unwrap()?.let { findInputValueTypeInType(inputValueDefinition.name, it) }
411+
val inputValueJavaType = propertyType ?: initialDictionary[inputGraphQLType.name]?.get()
412+
if (inputValueJavaType != null) {
413+
val genericContext = javaType ?: inputValueJavaType
414+
handleFoundType(typeClassMatcher.match(TypeClassMatcher.PotentialMatch.parameterType(
415+
// a class from the dictionary is for the unwrapped type, not for its list or non-null wrapper
416+
if (propertyType != null) inputValueDefinition.type else inputGraphQLType,
417+
inputValueJavaType,
418+
GenericType(genericContext, options).relativeToType(genericContext),
419+
InputObjectReference(inputValueDefinition)
420+
)))
421+
} else {
422+
handleInputValueWithoutJavaType(graphQLType, inputValueDefinition, inputGraphQLType, javaType?.unwrap())
410423
}
424+
}
411425
}
426+
}
427+
428+
/**
429+
* Without a Java property for an input field its type can still be built, as long as it doesn't need a class of its own.
430+
* At runtime the value is whatever the input object's class (or the Map) deserializes it into.
431+
*/
432+
private fun handleInputValueWithoutJavaType(graphQLType: InputObjectTypeDefinition, inputValueDefinition: InputValueDefinition, inputGraphQLType: TypeName, clazz: Class<*>?) {
433+
if (clazz != null && !isPropertyMap(clazz)) {
434+
log.warn("Cannot find a public getter or field for field '${inputValueDefinition.name}: ${inputGraphQLType.name}' of input type '${graphQLType.name}' on ${clazz.name}. " +
435+
"Unless Jackson maps it some other way, e.g. through @JsonProperty, check that the property name matches the schema, " +
436+
"otherwise the field's value is rejected or dropped when the input is deserialized.")
437+
}
438+
439+
when (val definition = definitionsByName[inputGraphQLType.name]) {
440+
is ScalarTypeDefinition -> handleFoundScalarType(definition)
441+
is InputObjectTypeDefinition -> {
442+
val isNewType = !dictionary.containsKey(definition)
443+
// registered without a class, so a class found for it later still gets set and scanned
444+
handleFoundType(definition, null, InputObjectReference(inputValueDefinition))
445+
if (isNewType) {
446+
handleInputObjectFields(definition, null)
447+
}
448+
}
449+
// the enum class may still be found through another field, so this is only reported once scanning is done
450+
is EnumTypeDefinition -> enumsWithoutClass.putIfAbsent(definition, enumWithoutClassMessage(definition, graphQLType, inputValueDefinition, clazz))
451+
// undefined types and output types are reported by the SchemaParser
452+
else -> {}
453+
}
454+
}
455+
456+
private fun isPropertyMap(clazz: Class<*>) =
457+
java.util.Map::class.java.isAssignableFrom(clazz) || JsonNode::class.java.isAssignableFrom(clazz) || clazz == Object::class.java
458+
459+
private fun isLibraryClass(clazz: Class<*>) =
460+
listOf("java.", "javax.", "kotlin.", "com.fasterxml.jackson.").any { clazz.name.startsWith(it) }
461+
462+
private fun enumWithoutClassMessage(enum: EnumTypeDefinition, graphQLType: InputObjectTypeDefinition, inputValueDefinition: InputValueDefinition, clazz: Class<*>?): String {
463+
val field = inputValueDefinition.name
464+
val problem = "Enum type '${enum.name}' is used by field '$field' of input type '${graphQLType.name}', but its Java enum can't be determined"
465+
return when {
466+
clazz == null -> "$problem because no class was found for input type '${graphQLType.name}'. " +
467+
"Pass the enum class for type '${enum.name}' in the parser's dictionary, or a class for '${graphQLType.name}' with a public getter or field '$field'."
468+
isPropertyMap(clazz) && isLibraryClass(clazz) -> "$problem because '${graphQLType.name}' is bound to ${clazz.name}, which has no type information for its fields. " +
469+
"Pass the enum class for type '${enum.name}' in the parser's dictionary, or use a class for '${graphQLType.name}' with a public getter or field '$field'."
470+
else -> "$problem because ${clazz.name} has no public getter or field named '$field'. " +
471+
"Add a public getter or field '$field' to the class used for '${graphQLType.name}', or pass the enum class for type '${enum.name}' in the parser's dictionary."
412472
}
413473
}
414474

@@ -425,25 +485,19 @@ internal class SchemaClassScanner(
425485
queue.add(QueueItem(graphQLType, javaType))
426486
}
427487

428-
private fun findInputValueType(name: String, inputGraphQLType: TypeName, clazz: Class<out Any>): JavaType? {
429-
val inputValueType = findInputValueTypeInType(name, clazz)
430-
if (inputValueType != null) {
431-
return inputValueType
432-
}
433-
434-
return initialDictionary[inputGraphQLType.name]?.get()
435-
}
436-
437488
private fun findInputValueTypeInType(name: String, clazz: Class<*>): JavaType? {
438-
val methods = clazz.methods
489+
// a Map or JsonNode has no properties for the input's fields, only methods that may share their names, e.g. ObjectNode.fields(),
490+
// but a subclass of one may still declare typed getters or fields of its own
491+
val isProperty = { member: Member -> !isPropertyMap(clazz) || !isLibraryClass(member.declaringClass) }
492+
val methods = clazz.methods.filter(isProperty)
439493

440494
val (getters, methodsWithParameters) = methods.filter {
441495
it.name == name || it.name == "get${name.replaceFirstChar(Char::titlecase)}"
442496
}.sortedBy { it.name.length }.partition { it.parameterCount == 0 }
443497

444498
return getters.find { !it.isSynthetic }?.genericReturnType
445499
?: getters.firstOrNull()?.genericReturnType
446-
?: clazz.fields.find { it.name == name }?.genericType
500+
?: clazz.fields.find { it.name == name && isProperty(it) }?.genericType
447501
?: methodsWithParameters.find { !it.isSynthetic }?.genericReturnType
448502
?: methodsWithParameters.firstOrNull()?.genericReturnType
449503
}

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

Lines changed: 61 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -506,8 +506,7 @@ class SchemaParser internal constructor(
506506
scalarType
507507
} else {
508508
if (!allowedTypeReferences.contains(typeDefinition.name)) {
509-
throw SchemaError("Expected type '${typeDefinition.name}' to be a ${expectedType.simpleName}, but it wasn't! " +
510-
"Was a type only permitted for object types incorrectly used as an input type, or vice-versa?")
509+
throw unexpectedTypeError(typeDefinition, expectedType)
511510
}
512511
inputObjects.find { it.name == typeDefinition.name } ?: GraphQLTypeReference(typeDefinition.name)
513512
}
@@ -538,8 +537,7 @@ class SchemaParser internal constructor(
538537
scalarType
539538
} else {
540539
if (!allowedTypeReferences.contains(typeDefinition.name)) {
541-
throw SchemaError("Expected type '${typeDefinition.name}' to be a ${expectedType.simpleName}, but it wasn't! " +
542-
"Was a type only permitted for object types incorrectly used as an input type, or vice-versa?")
540+
throw unexpectedTypeError(typeDefinition, expectedType)
543541
}
544542
val found = inputObjects.filter { it.name == typeDefinition.name }
545543
if (found.size == 1) {
@@ -565,6 +563,65 @@ class SchemaParser internal constructor(
565563
else -> throw SchemaError("Unknown type: $typeDefinition")
566564
}
567565

566+
private fun unexpectedTypeError(typeName: TypeName, expectedType: KClass<*>): SchemaError {
567+
val name = typeName.name
568+
val subject = "Type '$name'" +
569+
(findUsage(typeName)?.let { " used by $it" } ?: "") +
570+
(typeName.sourceLocation?.let { " at line ${it.line}, column ${it.column}" + (it.sourceName?.let { source -> " of $source" } ?: "") } ?: "")
571+
// unused ones first, since the extensions of an unused type are still among the definitions
572+
val definition = unusedDefinitions.find { it.name == name }
573+
?: definitions.filterIsInstance<TypeDefinition<*>>().find { it.name == name }
574+
val isInput = expectedType == GraphQLInputType::class
575+
val kind = when (definition) {
576+
is InputObjectTypeDefinition -> "an input object type"
577+
is InterfaceTypeDefinition -> "an interface"
578+
is UnionTypeDefinition -> "a union"
579+
else -> "an object type"
580+
}
581+
582+
return SchemaError(when {
583+
definition == null -> "$subject is not defined in the schema."
584+
isInput && definition !is InputObjectTypeDefinition && definition !is EnumTypeDefinition && definition !is ScalarTypeDefinition ->
585+
"$subject is $kind, which can't be used as an input type. Input types are input objects, enums and scalars."
586+
!isInput && definition is InputObjectTypeDefinition ->
587+
"$subject is $kind, which can't be used as an output type. Output types are object types, interfaces, unions, enums and scalars."
588+
definition in unusedDefinitions ->
589+
"$subject is defined in the schema, but was not reached while scanning the resolvers and the classes they use, so it wasn't built. " +
590+
unreachedTypeHint(definition)
591+
else -> "$subject can't be used as ${if (isInput) "an input" else "an output"} type."
592+
})
593+
}
594+
595+
// what makes the scanner reach the type depends on its kind, see SchemaClassScanner.canIncludeUnusedType
596+
private fun unreachedTypeHint(definition: TypeDefinition<*>): String = when (definition) {
597+
is InterfaceTypeDefinition, is UnionTypeDefinition ->
598+
"Return it from a resolver method or one of those classes, or set SchemaParserOptions.includeUnusedTypes."
599+
is EnumTypeDefinition ->
600+
"Use it in a resolver method or one of those classes, or set SchemaParserOptions.includeUnusedTypes and pass its enum class in the parser's dictionary."
601+
is InputObjectTypeDefinition ->
602+
"Use it in an argument of a resolver method, directly or through another input type. SchemaParserOptions.includeUnusedTypes doesn't include input types."
603+
is ScalarTypeDefinition ->
604+
"Use it in a resolver method or one of those classes. SchemaParserOptions.includeUnusedTypes doesn't include scalars."
605+
else ->
606+
"Return it from a resolver method or one of those classes, or set SchemaParserOptions.includeUnusedTypes and pass a class for it in the parser's dictionary."
607+
}
608+
609+
// determineType only gets the type reference, so the definition using it is looked up for the error message
610+
private fun findUsage(typeName: TypeName): String? = definitions.asSequence().flatMap { definition ->
611+
when (definition) {
612+
is ObjectTypeDefinition -> fieldUsages(definition.name, definition.fieldDefinitions)
613+
is InterfaceTypeDefinition -> fieldUsages(definition.name, definition.fieldDefinitions)
614+
is InputObjectTypeDefinition -> definition.inputValueDefinitions.map { "input field '${definition.name}.${it.name}'" to it.type }
615+
is DirectiveDefinition -> definition.inputValueDefinitions.map { "argument '${it.name}' of directive '@${definition.name}'" to it.type }
616+
else -> emptyList()
617+
}
618+
}.find { (_, type) -> type.unwrap() === typeName }?.first
619+
620+
private fun fieldUsages(typeName: String, fields: List<FieldDefinition>): List<Pair<String, Type<*>>> = fields.flatMap { field ->
621+
listOf("field '$typeName.${field.name}'" to field.type) +
622+
field.inputValueDefinitions.map { "argument '${it.name}' of field '$typeName.${field.name}'" to it.type }
623+
}
624+
568625
/**
569626
* Returns an optional [String] describing a deprecated field/enum.
570627
* If a deprecation directive was defined using the @deprecated directive,

0 commit comments

Comments
 (0)