Skip to content

Commit 418b804

Browse files
oryan-blockoryan-block
authored
Merge pull request #842 from graphql-java-kickstart/bugfix/479
Register input field types that have no Java property
2 parents 47cf27c + 0d3e8e3 commit 418b804

7 files changed

Lines changed: 780 additions & 50 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>>()
@@ -217,6 +220,10 @@ internal class SchemaClassScanner(
217220
}
218221

219222
private fun validateAndCreateResult(rootTypeHolder: RootTypesHolder): ScannedSchemaObjects {
223+
enumsWithoutClass.entries
224+
.find { dictionary[it.key]?.javaType == null }
225+
?.let { throw SchemaClassScannerError(it.value) }
226+
220227
initialDictionary
221228
.filter { !it.value.accessed }
222229
.forEach {
@@ -454,32 +461,85 @@ internal class SchemaClassScanner(
454461
scanInterfacesOfType(graphQLType)
455462
}
456463

464+
is InputObjectTypeDefinition -> handleInputObjectFields(graphQLType, javaType)
465+
}
466+
}
467+
468+
/**
469+
* Finds the types of the input object's fields through the properties of its class. A null [javaType] means the input object
470+
* has no known class, e.g. because it's only reachable through a Map.
471+
*/
472+
private fun handleInputObjectFields(graphQLType: InputObjectTypeDefinition, javaType: JavaType?) {
473+
val clazz = javaType?.unwrap()
474+
val inputObjectTypes = listOf(graphQLType) + inputExtensionDefinitions.filter { it.name == graphQLType.name }
475+
inputObjectTypes
476+
.flatMap { it.inputValueDefinitions }
477+
.forEach { inputValueDefinition ->
478+
val inputGraphQLType = inputValueDefinition.type.unwrap()
479+
if (inputGraphQLType !is TypeName || ScalarInfo.GRAPHQL_SPECIFICATION_SCALARS_DEFINITIONS.containsKey(inputGraphQLType.name)) {
480+
return@forEach
481+
}
482+
483+
val propertyType = clazz?.let { findInputValueTypeInType(inputValueDefinition.name, it) }
484+
val inputValueJavaType = propertyType ?: initialDictionary[inputGraphQLType.name]?.get()
485+
if (inputValueJavaType == null) {
486+
handleInputValueWithoutJavaType(graphQLType, inputValueDefinition, inputGraphQLType, clazz)
487+
return@forEach
488+
}
489+
490+
handleFoundType(typeClassMatcher.match(TypeClassMatcher.PotentialMatch.parameterType(
491+
// a class from the dictionary is for the unwrapped type, not for its list or non-null wrapper
492+
if (propertyType != null) inputValueDefinition.type else inputGraphQLType,
493+
inputValueJavaType,
494+
GenericTypeResolver(javaType ?: inputValueJavaType, options),
495+
InputObjectReference(inputValueDefinition)
496+
)))
497+
}
498+
}
499+
500+
/**
501+
* 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.
502+
* At runtime the value is whatever the input object's class (or the Map) deserializes it into.
503+
*/
504+
private fun handleInputValueWithoutJavaType(graphQLType: InputObjectTypeDefinition, inputValueDefinition: InputValueDefinition, inputGraphQLType: TypeName, clazz: Class<*>?) {
505+
if (clazz != null && !isPropertyMap(clazz)) {
506+
log.warn("Cannot find a public getter or field for field '${inputValueDefinition.name}: ${inputGraphQLType.name}' of input type '${graphQLType.name}' on ${clazz.name}. " +
507+
"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.")
508+
}
509+
510+
when (val definition = definitionsByName[inputGraphQLType.name]) {
511+
is ScalarTypeDefinition -> handleFoundScalarType(definition)
457512
is InputObjectTypeDefinition -> {
458-
val inputObjectTypes = listOf(graphQLType) + inputExtensionDefinitions.filter { it.name == graphQLType.name }
459-
inputObjectTypes
460-
.flatMap { it.inputValueDefinitions }
461-
.forEach { inputValueDefinition ->
462-
val inputGraphQLType = inputValueDefinition.type.unwrap()
463-
if (inputGraphQLType is TypeName && !ScalarInfo.GRAPHQL_SPECIFICATION_SCALARS_DEFINITIONS.containsKey(inputGraphQLType.name)) {
464-
val inputValueJavaType = findInputValueType(inputValueDefinition.name, inputGraphQLType, javaType.unwrap())
465-
if (inputValueJavaType != null) {
466-
handleFoundType(typeClassMatcher.match(TypeClassMatcher.PotentialMatch.parameterType(
467-
inputValueDefinition.type,
468-
inputValueJavaType,
469-
GenericTypeResolver(javaType, options),
470-
InputObjectReference(inputValueDefinition)
471-
)))
472-
} else {
473-
var mappingAdvice = "Try adding it manually to the dictionary"
474-
if (javaType.unwrap().name.contains("Map")) {
475-
mappingAdvice = " or add a class to represent your input type instead of a Map."
476-
}
477-
log.warn("Cannot find definition for field '${inputValueDefinition.name}: ${inputGraphQLType.name}' on input type '${graphQLType.name}' -> ${javaType.unwrap().name}. $mappingAdvice")
478-
}
479-
}
480-
}
513+
val isNewType = !dictionary.containsKey(definition)
514+
// registered without a class, so a class found for it later still gets set and scanned
515+
handleFoundType(definition, null, InputObjectReference(inputValueDefinition))
516+
if (isNewType) {
517+
handleInputObjectFields(definition, null)
518+
}
481519
}
520+
// the enum class may still be found through another field, so this is only reported once scanning is done
521+
is EnumTypeDefinition -> enumsWithoutClass.putIfAbsent(definition, enumWithoutClassMessage(definition, graphQLType, inputValueDefinition, clazz))
522+
// undefined types and output types are reported by the SchemaParser
523+
else -> {}
524+
}
525+
}
526+
527+
// an input bound to Any gets a Map at runtime too
528+
private fun isPropertyMap(clazz: Class<*>) =
529+
Map::class.java.isAssignableFrom(clazz) || JsonNode::class.java.isAssignableFrom(clazz) || clazz == Any::class.java
530+
531+
private fun isLibraryClass(clazz: Class<*>) =
532+
listOf("java.", "kotlin.", "com.fasterxml.jackson.").any { clazz.name.startsWith(it) }
533+
534+
private fun enumWithoutClassMessage(enumType: EnumTypeDefinition, graphQLType: InputObjectTypeDefinition, inputValueDefinition: InputValueDefinition, clazz: Class<*>?): String {
535+
val field = inputValueDefinition.name
536+
val reason = when {
537+
clazz == null -> "no class was found for input type '${graphQLType.name}'"
538+
isPropertyMap(clazz) && isLibraryClass(clazz) -> "'${graphQLType.name}' is bound to ${clazz.name}, which has no type information for its fields"
539+
else -> "${clazz.name} has no public getter or field named '$field'"
482540
}
541+
return "Enum type '${enumType.name}' is used by field '$field' of input type '${graphQLType.name}', but its Java enum can't be determined because $reason. " +
542+
"Pass the enum class for type '${enumType.name}' in the parser's dictionary, or use a class for '${graphQLType.name}' with a public getter or field '$field'."
483543
}
484544

485545
private fun scanInterfacesOfType(graphQLType: ObjectTypeDefinition) {
@@ -495,25 +555,19 @@ internal class SchemaClassScanner(
495555
queue.add(QueueItem(graphQLType, javaType))
496556
}
497557

498-
private fun findInputValueType(name: String, inputGraphQLType: TypeName, clazz: Class<out Any>): JavaType? {
499-
val inputValueType = findInputValueTypeInType(name, clazz)
500-
if (inputValueType != null) {
501-
return inputValueType
502-
}
503-
504-
return initialDictionary[inputGraphQLType.name]?.get()
505-
}
506-
507558
private fun findInputValueTypeInType(name: String, clazz: Class<*>): JavaType? {
508-
val methods = clazz.methods
559+
// a Map or JsonNode has no properties for the input's fields, only methods that may share their names, e.g. ObjectNode.fields(),
560+
// but a subclass of one may still declare typed getters or fields of its own
561+
val isProperty = { member: Member -> !isPropertyMap(clazz) || !isLibraryClass(member.declaringClass) }
562+
val methods = clazz.methods.filter(isProperty)
509563

510564
val (getters, methodsWithParameters) = methods.filter {
511565
it.name == name || it.name == "get${name.replaceFirstChar(Char::titlecase)}"
512566
}.sortedBy { it.name.length }.partition { it.parameterCount == 0 }
513567

514568
return getters.find { !it.isSynthetic }?.genericReturnType
515569
?: getters.firstOrNull()?.genericReturnType
516-
?: clazz.fields.find { it.name == name }?.genericType
570+
?: clazz.fields.find { it.name == name && isProperty(it) }?.genericType
517571
?: methodsWithParameters.find { !it.isSynthetic }?.genericReturnType
518572
?: methodsWithParameters.firstOrNull()?.genericReturnType
519573
}

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

Lines changed: 62 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -545,8 +545,7 @@ class SchemaParser internal constructor(
545545
scalarType
546546
} else {
547547
if (!allowedTypeReferences.contains(typeDefinition.name)) {
548-
throw SchemaError("Expected type '${typeDefinition.name}' to be a ${expectedType.simpleName}, but it wasn't! " +
549-
"Was a type only permitted for object types incorrectly used as an input type, or vice-versa?")
548+
throw unexpectedTypeError(typeDefinition, expectedType)
550549
}
551550
inputObjects.find { it.name == typeDefinition.name } ?: GraphQLTypeReference(typeDefinition.name)
552551
}
@@ -577,8 +576,7 @@ class SchemaParser internal constructor(
577576
scalarType
578577
} else {
579578
if (!allowedTypeReferences.contains(typeDefinition.name)) {
580-
throw SchemaError("Expected type '${typeDefinition.name}' to be a ${expectedType.simpleName}, but it wasn't! " +
581-
"Was a type only permitted for object types incorrectly used as an input type, or vice-versa?")
579+
throw unexpectedTypeError(typeDefinition, expectedType)
582580
}
583581
val found = inputObjects.filter { it.name == typeDefinition.name }
584582
if (found.size == 1) {
@@ -604,6 +602,66 @@ class SchemaParser internal constructor(
604602
else -> throw SchemaError("Unknown type: $typeDefinition")
605603
}
606604

605+
private fun unexpectedTypeError(typeName: TypeName, expectedType: KClass<*>): SchemaError {
606+
val isInput = expectedType == GraphQLInputType::class
607+
// unused ones first, since the extensions of an unused type are still among the definitions
608+
val definition = unusedDefinitions.find { it.name == typeName.name }
609+
?: definitions.filterIsInstance<TypeDefinition<*>>().find { it.name == typeName.name }
610+
val outputOnlyKind = when (definition) {
611+
is ObjectTypeDefinition -> "an object type"
612+
is InterfaceTypeDefinition -> "an interface"
613+
is UnionTypeDefinition -> "a union"
614+
else -> null
615+
}
616+
617+
val problem = when {
618+
definition == null -> "is not defined in the schema."
619+
isInput && outputOnlyKind != null ->
620+
"is $outputOnlyKind, which can't be used as an input type. Input types are input objects, enums and scalars."
621+
!isInput && definition is InputObjectTypeDefinition ->
622+
"is an input object type, which can't be used as an output type. Output types are object types, interfaces, unions, enums and scalars."
623+
definition in unusedDefinitions ->
624+
"is defined in the schema, but was not reached while scanning the resolvers and the classes they use, so it wasn't built. " +
625+
unreachedTypeHint(definition)
626+
else -> "can't be used as ${if (isInput) "an input" else "an output"} type."
627+
}
628+
val usage = findUsage(typeName)?.let { " used by $it" }.orEmpty()
629+
val location = typeName.sourceLocation
630+
?.let { " at line ${it.line}, column ${it.column}" + it.sourceName?.let { name -> " in $name" }.orEmpty() }
631+
.orEmpty()
632+
return SchemaError("Type '${typeName.name}'$usage$location $problem")
633+
}
634+
635+
// what makes the scanner reach the type depends on its kind, see SchemaClassScanner.canIncludeUnusedType
636+
private fun unreachedTypeHint(definition: TypeDefinition<*>): String = when (definition) {
637+
is InterfaceTypeDefinition, is UnionTypeDefinition ->
638+
"Return it from a resolver method or one of those classes, or set SchemaParserOptions.includeUnusedTypes."
639+
is EnumTypeDefinition ->
640+
"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."
641+
is InputObjectTypeDefinition ->
642+
"Use it in an argument of a resolver method, directly or through another input type. SchemaParserOptions.includeUnusedTypes doesn't include input types."
643+
is ScalarTypeDefinition ->
644+
"Use it in a resolver method or one of those classes. SchemaParserOptions.includeUnusedTypes doesn't include scalars."
645+
else ->
646+
"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."
647+
}
648+
649+
// determineType only gets the type reference, so the definition using it is looked up for the error message
650+
private fun findUsage(typeName: TypeName): String? = definitions.asSequence().flatMap { definition ->
651+
when (definition) {
652+
is ObjectTypeDefinition -> fieldUsages(definition.name, definition.fieldDefinitions)
653+
is InterfaceTypeDefinition -> fieldUsages(definition.name, definition.fieldDefinitions)
654+
is InputObjectTypeDefinition -> definition.inputValueDefinitions.map { "input field '${definition.name}.${it.name}'" to it.type }
655+
is DirectiveDefinition -> definition.inputValueDefinitions.map { "argument '${it.name}' of directive '@${definition.name}'" to it.type }
656+
else -> emptyList()
657+
}
658+
}.find { (_, type) -> type.unwrap() === typeName }?.first
659+
660+
private fun fieldUsages(typeName: String, fields: List<FieldDefinition>): List<Pair<String, Type<*>>> = fields.flatMap { field ->
661+
listOf("field '$typeName.${field.name}'" to field.type) +
662+
field.inputValueDefinitions.map { "argument '${it.name}' of field '$typeName.${field.name}'" to it.type }
663+
}
664+
607665
/**
608666
* Returns an optional [String] describing a deprecated field/enum.
609667
* If a deprecation directive was defined using the @deprecated directive,

0 commit comments

Comments
 (0)