diff --git a/pom.xml b/pom.xml index 28978594..e96e43c4 100644 --- a/pom.xml +++ b/pom.xml @@ -16,7 +16,7 @@ 17 2.3.10 1.11.0 - 2.22.4 + 3.2.4 26.1 1.0.4 @@ -48,7 +48,7 @@ ${graphql-java.version} - com.fasterxml.jackson + tools.jackson jackson-bom ${jackson.version} import @@ -161,16 +161,16 @@ classmate - com.fasterxml.jackson.core + tools.jackson.core jackson-core - com.fasterxml.jackson.module - jackson-module-kotlin + tools.jackson.core + jackson-databind - com.fasterxml.jackson.datatype - jackson-datatype-jdk8 + tools.jackson.module + jackson-module-kotlin org.apache.commons diff --git a/src/main/kotlin/graphql/kickstart/tools/DefaultObjectMapperProvider.kt b/src/main/kotlin/graphql/kickstart/tools/DefaultObjectMapperProvider.kt new file mode 100644 index 00000000..0284ce81 --- /dev/null +++ b/src/main/kotlin/graphql/kickstart/tools/DefaultObjectMapperProvider.kt @@ -0,0 +1,23 @@ +package graphql.kickstart.tools + +import graphql.language.FieldDefinition +import tools.jackson.databind.DeserializationFeature +import tools.jackson.databind.ObjectMapper +import tools.jackson.databind.cfg.EnumFeature +import tools.jackson.databind.json.JsonMapper +import tools.jackson.module.kotlin.jacksonMapperBuilder + +internal class DefaultObjectMapperProvider(private val configurer: ObjectMapperConfigurer? = null) : PerFieldObjectMapperProvider { + + private val shared by lazy { builder().build() } + + override fun provide(fieldDefinition: FieldDefinition): ObjectMapper = + if (configurer == null) shared + else builder().also { configurer.configure(it, ObjectMapperConfigurerContext(fieldDefinition)) }.build() + + private fun builder(): JsonMapper.Builder = jacksonMapperBuilder() + // an input field the class has no property for fails the request rather than losing the client's value + .enable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES) + // GraphQL enum values are the Java constants' names, so an enum converts to and from a String by name() + .disable(EnumFeature.READ_ENUMS_USING_TO_STRING, EnumFeature.WRITE_ENUMS_USING_TO_STRING) +} diff --git a/src/main/kotlin/graphql/kickstart/tools/ObjectMapperConfigurer.java b/src/main/kotlin/graphql/kickstart/tools/ObjectMapperConfigurer.java index a9367c27..d8536c59 100644 --- a/src/main/kotlin/graphql/kickstart/tools/ObjectMapperConfigurer.java +++ b/src/main/kotlin/graphql/kickstart/tools/ObjectMapperConfigurer.java @@ -1,11 +1,15 @@ package graphql.kickstart.tools; -import com.fasterxml.jackson.databind.ObjectMapper; +import tools.jackson.databind.json.JsonMapper; /** + * Customizes the library's ObjectMapper for one resolver field. Jackson 3 mappers are immutable, so this gets the builder that + * the mapper is built from. It already has the Kotlin module and the library's defaults, and a module added here replaces one + * of the same type that was added before, e.g. a KotlinModule with other features. + * * @author Andrew Potter */ public interface ObjectMapperConfigurer { - void configure(ObjectMapper mapper, ObjectMapperConfigurerContext context); + void configure(JsonMapper.Builder builder, ObjectMapperConfigurerContext context); } diff --git a/src/main/kotlin/graphql/kickstart/tools/PerFieldConfiguringObjectMapperProvider.kt b/src/main/kotlin/graphql/kickstart/tools/PerFieldConfiguringObjectMapperProvider.kt deleted file mode 100644 index 71e7b9f6..00000000 --- a/src/main/kotlin/graphql/kickstart/tools/PerFieldConfiguringObjectMapperProvider.kt +++ /dev/null @@ -1,17 +0,0 @@ -package graphql.kickstart.tools - -import com.fasterxml.jackson.databind.ObjectMapper -import com.fasterxml.jackson.datatype.jdk8.Jdk8Module -import com.fasterxml.jackson.module.kotlin.registerKotlinModule -import graphql.language.FieldDefinition - -class PerFieldConfiguringObjectMapperProvider( - private val objectMapperConfigurer: ObjectMapperConfigurer = ObjectMapperConfigurer { _, _ -> } -) : PerFieldObjectMapperProvider { - - override fun provide(fieldDefinition: FieldDefinition): ObjectMapper { - return ObjectMapper().apply { - objectMapperConfigurer.configure(this, ObjectMapperConfigurerContext(fieldDefinition)) - }.registerModule(Jdk8Module()).registerKotlinModule() - } -} diff --git a/src/main/kotlin/graphql/kickstart/tools/PerFieldObjectMapperProvider.kt b/src/main/kotlin/graphql/kickstart/tools/PerFieldObjectMapperProvider.kt index 80acfa57..ce926712 100644 --- a/src/main/kotlin/graphql/kickstart/tools/PerFieldObjectMapperProvider.kt +++ b/src/main/kotlin/graphql/kickstart/tools/PerFieldObjectMapperProvider.kt @@ -1,9 +1,14 @@ package graphql.kickstart.tools -import com.fasterxml.jackson.databind.ObjectMapper import graphql.language.FieldDefinition +import tools.jackson.databind.ObjectMapper -interface PerFieldObjectMapperProvider { +/** + * Supplies the ObjectMapper that converts the arguments of a resolver method, and that checks its input classes when the schema + * is built. Called once per method-backed field, at schema build. The mapper is used as it is: its own features decide how + * arguments convert, except FAIL_ON_EMPTY_BEANS, which the argument conversion always turns on for itself. + */ +fun interface PerFieldObjectMapperProvider { fun provide(fieldDefinition: FieldDefinition): ObjectMapper } diff --git a/src/main/kotlin/graphql/kickstart/tools/ScanDictionary.kt b/src/main/kotlin/graphql/kickstart/tools/ScanDictionary.kt index 187667ec..a4d5c764 100644 --- a/src/main/kotlin/graphql/kickstart/tools/ScanDictionary.kt +++ b/src/main/kotlin/graphql/kickstart/tools/ScanDictionary.kt @@ -1,12 +1,12 @@ package graphql.kickstart.tools -import com.fasterxml.jackson.databind.JsonNode import graphql.kickstart.tools.util.JavaType import graphql.kickstart.tools.util.unwrap import graphql.kickstart.tools.util.withExtensions import graphql.language.* import graphql.schema.idl.ScalarInfo import org.slf4j.Logger +import tools.jackson.databind.JsonNode import java.lang.reflect.Member import java.lang.reflect.Method @@ -131,7 +131,7 @@ internal class ScanDictionary( Map::class.java.isAssignableFrom(clazz) || JsonNode::class.java.isAssignableFrom(clazz) || clazz == Any::class.java private fun isLibraryClass(clazz: Class<*>) = - listOf("java.", "kotlin.", "com.fasterxml.jackson.").any { clazz.name.startsWith(it) } + listOf("java.", "kotlin.", JACKSON_PACKAGE_PREFIX).any { clazz.name.startsWith(it) } private fun enumWithoutClassMessage(enumType: EnumTypeDefinition, graphQLType: InputObjectTypeDefinition, inputValueDefinition: InputValueDefinition, clazz: Class<*>?): String { val field = inputValueDefinition.name @@ -267,3 +267,6 @@ internal class ScanDictionary( override fun getDescription() = "type of field $field" } } + +// not a literal, which would keep compiling when Jackson's packages move but stop matching. Covers jackson-core's TreeNode too +private val JACKSON_PACKAGE_PREFIX = JsonNode::class.java.packageName.substringBeforeLast('.') + "." diff --git a/src/main/kotlin/graphql/kickstart/tools/SchemaParserOptions.kt b/src/main/kotlin/graphql/kickstart/tools/SchemaParserOptions.kt index 42dd965b..e5edd089 100644 --- a/src/main/kotlin/graphql/kickstart/tools/SchemaParserOptions.kt +++ b/src/main/kotlin/graphql/kickstart/tools/SchemaParserOptions.kt @@ -1,6 +1,5 @@ package graphql.kickstart.tools -import com.fasterxml.jackson.databind.ObjectMapper import graphql.kickstart.tools.proxy.* import graphql.kickstart.tools.relay.RelayConnectionFactory import graphql.kickstart.tools.resolver.MissingResolverDataFetcherProvider @@ -53,7 +52,7 @@ data class SchemaParserOptions internal constructor( private var useDefaultGenericWrappers = true private var allowUnimplementedResolvers = false private var missingResolverDataFetcherProvider: MissingResolverDataFetcherProvider? = null - private var objectMapperProvider: PerFieldObjectMapperProvider = PerFieldConfiguringObjectMapperProvider() + private var objectMapperProvider: PerFieldObjectMapperProvider = DefaultObjectMapperProvider() private val proxyHandlers: MutableList = mutableListOf(Spring4AopProxyHandler(), GuiceAopProxyHandler(), JavassistProxyHandler(), WeldProxyHandler()) private var inputArgumentOptionalDetectOmission = false private var preferGraphQLResolver = false @@ -101,18 +100,23 @@ data class SchemaParserOptions internal constructor( this.preferGraphQLResolver = preferGraphQLResolver } + /** + * Customizes the ObjectMapper of each resolver field, starting from the library's defaults and the Kotlin module. + * Replaces a provider set before, and the other way round. + */ fun objectMapperConfigurer(objectMapperConfigurer: ObjectMapperConfigurer) = this.apply { - this.objectMapperProvider = PerFieldConfiguringObjectMapperProvider(objectMapperConfigurer) + this.objectMapperProvider = DefaultObjectMapperProvider(objectMapperConfigurer) } + /** + * Uses these ObjectMappers as they are, e.g. `objectMapperProvider { springMapper }` to share an application's + * JsonMapper. They don't get the library's defaults or the Kotlin module. For a per-field variant of your own mapper, + * return `mapper.rebuild()...build()` from the provider. + */ fun objectMapperProvider(objectMapperProvider: PerFieldObjectMapperProvider) = this.apply { this.objectMapperProvider = objectMapperProvider } - fun objectMapperConfigurer(objectMapperConfigurer: (ObjectMapper, ObjectMapperConfigurerContext) -> Unit) = this.apply { - this.objectMapperConfigurer(ObjectMapperConfigurer(objectMapperConfigurer)) - } - fun addProxyHandler(proxyHandler: ProxyHandler) = this.apply { this.proxyHandlers.add(proxyHandler) } @@ -150,9 +154,9 @@ data class SchemaParserOptions internal constructor( } /** - * 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]. + * What to do about input object fields that the class of a resolver argument has no property for. With the library's + * ObjectMapper, converting the argument fails when such a field is sent. An ObjectMapper that ignores unknown + * properties, the Jackson 3 default and Spring's, drops the field's value instead. 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 @@ -160,8 +164,8 @@ data class SchemaParserOptions internal constructor( * 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. + * delegating creator, a builder or type info, and those with injected values. Neither are arguments bound to a Map, a + * JsonNode or a type variable. */ fun unmappedInputFields(unmappedInputFields: UnmappedInputFields) = this.apply { this.unmappedInputFields = unmappedInputFields diff --git a/src/main/kotlin/graphql/kickstart/tools/UnmappedInputFieldChecker.kt b/src/main/kotlin/graphql/kickstart/tools/UnmappedInputFieldChecker.kt index de99bdcf..dcee5d8b 100644 --- a/src/main/kotlin/graphql/kickstart/tools/UnmappedInputFieldChecker.kt +++ b/src/main/kotlin/graphql/kickstart/tools/UnmappedInputFieldChecker.kt @@ -1,16 +1,5 @@ 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 -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.schema.GraphQLInputObjectField @@ -19,6 +8,19 @@ import graphql.schema.GraphQLNamedType import graphql.schema.GraphQLType import graphql.schema.GraphQLTypeUtil import org.slf4j.LoggerFactory +import tools.jackson.core.JsonParser +import tools.jackson.databind.BeanProperty +import tools.jackson.databind.DeserializationContext +import tools.jackson.databind.DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES +import tools.jackson.databind.JavaType +import tools.jackson.databind.ObjectMapper +import tools.jackson.databind.PropertyName +import tools.jackson.databind.ValueDeserializer +import tools.jackson.databind.deser.bean.BeanDeserializerBase +import tools.jackson.databind.deser.std.ContainerDeserializerBase +import tools.jackson.databind.deser.std.ReferenceTypeDeserializer +import tools.jackson.databind.exc.UnrecognizedPropertyException +import tools.jackson.databind.node.NullNode internal class UnmappedInputFieldChecker( private val inputObjects: Map, @@ -26,7 +28,7 @@ internal class UnmappedInputFieldChecker( ) { private val log = LoggerFactory.getLogger(javaClass) - private val checked = mutableSetOf, String>>() + private val checked = mutableSetOf, String>>() private val messages = linkedMapOf() @@ -37,11 +39,9 @@ internal class UnmappedInputFieldChecker( try { val parameterType = resolver.getMethodParameterType(index) ?: return@forEachIndexed - val mapper = resolver.objectMapper - val context = (mapper.deserializationContext as? DefaultDeserializationContext) - ?.createDummyInstance(mapper.deserializationConfig) ?: return@forEachIndexed - val deserializer = context.findRootValueDeserializer(mapper.typeFactory.constructType(parameterType)) - checkFields(mapper, context, deserializer, null, inputObject, usage) + val reads = StrictReads(resolver.objectMapper) + val deserializer = reads.context.findRootValueDeserializer(reads.mapper.typeFactory.constructType(parameterType)) + checkFields(reads, deserializer, null, inputObject, usage) } catch (e: VirtualMachineError) { throw e } catch (e: Throwable) { @@ -62,33 +62,32 @@ internal class UnmappedInputFieldChecker( } private fun checkFields( - mapper: ObjectMapper, - context: DefaultDeserializationContext, - deserializer: JsonDeserializer<*>?, + reads: StrictReads, + deserializer: ValueDeserializer<*>?, property: BeanProperty?, inputObject: GraphQLInputObjectType, usage: String ) { - val beanDeserializer = beanDeserializer(context, deserializer, property) ?: return + val beanDeserializer = beanDeserializer(reads.context, deserializer, property) ?: return if (!checked.add(beanDeserializer to inputObject.name)) { return } inputObject.fields.forEach { field -> - val property = beanDeserializer.findProperty(field.name) + val property = beanDeserializer.findProperty(PropertyName.construct(field.name)) if (property == null) { - knownPropertiesIfUnmapped(mapper, context, beanDeserializer, field.name)?.let { knownProperties -> - val unmapped = Unmapped(inputObject.name, field.name, beanDeserializer.handledType(), consequence(mapper, inputObject, field)) + reads.knownPropertiesIfUnmapped(beanDeserializer, field.name)?.let { knownProperties -> + val unmapped = Unmapped(inputObject.name, field.name, beanDeserializer.handledType(), consequence(reads.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 - inputObjectOf(field.type)?.let { checkFields(mapper, context, property.valueDeserializer, property, it, usage) } + inputObjectOf(field.type)?.let { checkFields(reads, property.valueDeserializer, property, it, usage) } } } } - private tailrec fun beanDeserializer(context: DeserializationContext, deserializer: JsonDeserializer<*>?, property: BeanProperty?): BeanDeserializerBase? = + private tailrec fun beanDeserializer(context: DeserializationContext, deserializer: ValueDeserializer<*>?, property: BeanProperty?): BeanDeserializerBase? = when (deserializer) { is BeanDeserializerBase -> deserializer.takeUnless { it.valueInstantiator.canCreateUsingDelegate() } is ContainerDeserializerBase<*> -> @@ -109,39 +108,14 @@ internal class UnmappedInputFieldChecker( * or default typing. */ private fun isPolymorphic(context: DeserializationContext, contentType: JavaType): Boolean = - contentType.getTypeHandler() != null || context.factory.findTypeDeserializer(context.config, contentType) != null - - /** - * `{"": 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. - */ - 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 - } - } + contentType.typeHandler != null || context.findTypeDeserializer(contentType) != null private fun inputObjectOf(type: GraphQLType): GraphQLInputObjectType? = inputObjects[GraphQLTypeUtil.unwrapAllAs(type).name] 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" + !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" } @@ -156,4 +130,34 @@ internal class UnmappedInputFieldChecker( } private data class Unmapped(val inputObject: String, val field: String, val clazz: Class<*>, val consequence: String) + + /** + * A parser made by an ObjectReader carries the reader's context, which keeps the mapper's injectable values and problem + * handlers and shares its deserializer cache. + */ + private class StrictReads(val mapper: ObjectMapper) { + + private val reader = mapper.reader().with(FAIL_ON_UNKNOWN_PROPERTIES) + + val context: DeserializationContext = contextOf(reader.treeAsTokens(NullNode.instance)) + + /** + * `{"": 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. + */ + fun knownPropertiesIfUnmapped(deserializer: BeanDeserializerBase, field: String): Collection? = try { + reader.treeAsTokens(mapper.createObjectNode().putNull(field)).use { parser -> + parser.nextToken() + deserializer.deserialize(parser, contextOf(parser), Any()) + } + null + } catch (e: UnrecognizedPropertyException) { + if (e.propertyName == field) e.knownPropertyIds.orEmpty() else null + } catch (e: Exception) { + null + } + + private fun contextOf(parser: JsonParser) = parser.objectReadContext() as DeserializationContext + } } diff --git a/src/main/kotlin/graphql/kickstart/tools/resolver/ArgumentConverter.kt b/src/main/kotlin/graphql/kickstart/tools/resolver/ArgumentConverter.kt index c3708cf1..2b3530b1 100644 --- a/src/main/kotlin/graphql/kickstart/tools/resolver/ArgumentConverter.kt +++ b/src/main/kotlin/graphql/kickstart/tools/resolver/ArgumentConverter.kt @@ -1,36 +1,37 @@ package graphql.kickstart.tools.resolver -import com.fasterxml.jackson.core.JsonGenerator -import com.fasterxml.jackson.core.JsonParser -import com.fasterxml.jackson.core.JsonToken -import com.fasterxml.jackson.databind.DeserializationContext -import com.fasterxml.jackson.databind.DeserializationFeature -import com.fasterxml.jackson.databind.JavaType -import com.fasterxml.jackson.databind.JsonSerializable -import com.fasterxml.jackson.databind.ObjectMapper -import com.fasterxml.jackson.databind.SerializationFeature -import com.fasterxml.jackson.databind.SerializerProvider -import com.fasterxml.jackson.databind.deser.DeserializationProblemHandler -import com.fasterxml.jackson.databind.deser.ValueInstantiator -import com.fasterxml.jackson.databind.jsontype.TypeSerializer -import com.fasterxml.jackson.databind.util.TokenBuffer -import java.io.IOException +import tools.jackson.core.JacksonException +import tools.jackson.core.JsonGenerator +import tools.jackson.core.JsonParser +import tools.jackson.core.JsonToken +import tools.jackson.databind.DeserializationContext +import tools.jackson.databind.DeserializationFeature +import tools.jackson.databind.JacksonSerializable +import tools.jackson.databind.JavaType +import tools.jackson.databind.ObjectMapper +import tools.jackson.databind.SerializationContext +import tools.jackson.databind.SerializationFeature +import tools.jackson.databind.deser.DeserializationProblemHandler +import tools.jackson.databind.deser.ValueInstantiator +import tools.jackson.databind.jsontype.TypeSerializer +import tools.jackson.databind.util.TokenBuffer import java.lang.reflect.Type /** - * ObjectMapper.convertValue serializes a value and reads it back, which the coerced values of many custom scalars don't - * survive, e.g. an upload, or a LocalDate without the jsr310 module. Those values are written as embedded objects - * instead, which Jackson's deserializers return as they are if they have the type being read, and convert with - * toString() if a String is read. + * Converts a coerced argument value to a resolver parameter's type the way ObjectMapper.convertValue does, by writing it into + * a TokenBuffer and reading it back. The coerced values of many custom scalars don't survive that, e.g. an upload, or a class + * without a creator. Those values are written as embedded objects instead, which Jackson's deserializers return as they are if + * they have the type being read, and convert with toString() if a String is read. */ internal class ArgumentConverter(private val mapper: ObjectMapper, type: Type) { private val javaType = mapper.typeFactory.constructType(type) private val isUntyped = javaType.isUntyped() + private val useBigDecimalForFloats = mapper.isEnabled(DeserializationFeature.USE_BIG_DECIMAL_FOR_FLOATS) fun convert(value: Any?): Any? { if (value == null) { - return mapper.convertValue(value, javaType) + return nullToJavaDefaultReader.readValue(buffer(null)) } if (isUntyped) { @@ -38,8 +39,8 @@ internal class ArgumentConverter(private val mapper: ObjectMapper, type: Type) { } return try { - mapper.convertValue(value, javaType) - } catch (e: IllegalArgumentException) { + read(value) + } catch (e: JacksonException) { val nonConvertingClasses = HashSet>() val embedded = embed(value) { scalar -> when { @@ -53,7 +54,7 @@ internal class ArgumentConverter(private val mapper: ObjectMapper, type: Type) { } ?: throw e try { read(embedded) - } catch (retryError: IllegalArgumentException) { + } catch (retryError: JacksonException) { throw retryError.apply { addSuppressed(e) } } } @@ -72,22 +73,16 @@ internal class ArgumentConverter(private val mapper: ObjectMapper, type: Type) { } private fun convertsToOwnClass(value: Any): Boolean = try { - mapper.convertValue(value, value.javaClass) + mapper.readValue(buffer(value), value.javaClass) true - } catch (e: IllegalArgumentException) { + } catch (e: JacksonException) { false } - // ObjectMapper.convertValue, but read with the handler for embedded values - private fun read(value: Any): Any? { - val buffer = TokenBuffer(mapper, false).forceUseOfBigDecimal(mapper.isEnabled(DeserializationFeature.USE_BIG_DECIMAL_FOR_FLOATS)) - return try { - writer.writeValue(buffer, value) - reader.readValue(buffer.asParser()) - } catch (e: IOException) { - throw IllegalArgumentException(e.message, e) - } - } + private fun read(value: Any?): Any? = reader.readValue(buffer(value)) + + private fun buffer(value: Any?): TokenBuffer = + writer.writeValueIntoBuffer(value).forceUseOfBigDecimal(useBigDecimalForFloats) private val embeddedValueHandler = object : DeserializationProblemHandler() { // called for an interface or abstract class, e.g. Part, since no deserializer of it returns embedded values @@ -104,23 +99,29 @@ internal class ArgumentConverter(private val mapper: ObjectMapper, type: Type) { // called for an embedded value read as a bean of another class, which the value is converted to on its own override fun handleWeirdNativeValue(context: DeserializationContext, targetType: JavaType, valueToConvert: Any, p: JsonParser): Any? = - mapper.convertValue(valueToConvert, targetType) + mapper.readValue(buffer(valueToConvert), targetType) } - private val writer = mapper.writer().without(SerializationFeature.WRAP_ROOT_VALUE) + // FAIL_ON_EMPTY_BEANS whatever the mapper says: a scalar class with no visible properties would otherwise be read back as + // a new default instance, losing its value without the error that makes it embedded + private val writer = mapper.writer().without(SerializationFeature.WRAP_ROOT_VALUE).with(SerializationFeature.FAIL_ON_EMPTY_BEANS) - // built on first use, since readerFor looks up the deserializer right away, at schema build, and most arguments - // never need the reader private val reader by lazy { mapper.readerFor(javaType) .without(DeserializationFeature.UNWRAP_ROOT_VALUE) .withHandler(embeddedValueHandler) } - private class EmbeddedValue(private val value: Any) : JsonSerializable { - override fun serialize(gen: JsonGenerator, serializers: SerializerProvider) = gen.writeEmbeddedObject(value) + private val nullToJavaDefaultReader by lazy { reader.without(DeserializationFeature.FAIL_ON_NULL_FOR_PRIMITIVES) } - override fun serializeWithType(gen: JsonGenerator, serializers: SerializerProvider, typeSer: TypeSerializer) = gen.writeEmbeddedObject(value) + private class EmbeddedValue(private val value: Any) : JacksonSerializable { + override fun serialize(gen: JsonGenerator, serializers: SerializationContext) { + gen.writeEmbeddedObject(value) + } + + override fun serializeWithType(gen: JsonGenerator, serializers: SerializationContext, typeSer: TypeSerializer) { + gen.writeEmbeddedObject(value) + } } } diff --git a/src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt b/src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt index a3ce80be..7457dfc4 100644 --- a/src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt +++ b/src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt @@ -1,6 +1,5 @@ package graphql.kickstart.tools.resolver -import com.fasterxml.jackson.databind.ObjectMapper import graphql.GraphQLContext import graphql.kickstart.tools.* import graphql.kickstart.tools.util.JavaType @@ -20,6 +19,7 @@ import kotlinx.coroutines.ensureActive import kotlinx.coroutines.future.future import org.apache.commons.lang3.reflect.TypeUtils import org.reactivestreams.Publisher +import tools.jackson.databind.ObjectMapper import java.lang.reflect.InvocationTargetException import java.lang.reflect.Method import java.util.* diff --git a/src/main/kotlin/module-info.java b/src/main/kotlin/module-info.java index 55f7ef21..e0590cc3 100644 --- a/src/main/kotlin/module-info.java +++ b/src/main/kotlin/module-info.java @@ -8,10 +8,9 @@ // graphql-java's nullability annotations, only needed at compile time requires static org.jspecify; - requires com.fasterxml.jackson.core; - requires com.fasterxml.jackson.kotlin; - requires com.fasterxml.jackson.databind; - requires com.fasterxml.jackson.datatype.jdk8; + requires tools.jackson.core; + requires transitive tools.jackson.databind; + requires tools.jackson.module.kotlin; requires com.fasterxml.classmate; requires org.apache.commons.lang3; @@ -21,5 +20,5 @@ exports graphql.kickstart.tools; - opens graphql.kickstart.tools to com.fasterxml.jackson.databind; + opens graphql.kickstart.tools to tools.jackson.databind; } diff --git a/src/test/kotlin/graphql/kickstart/tools/CustomScalarInputFixtures.java b/src/test/kotlin/graphql/kickstart/tools/CustomScalarInputFixtures.java new file mode 100644 index 00000000..ffc94d60 --- /dev/null +++ b/src/test/kotlin/graphql/kickstart/tools/CustomScalarInputFixtures.java @@ -0,0 +1,22 @@ +package graphql.kickstart.tools; + +// Java, since Jackson writes a Kotlin class without visible properties as {} even with FAIL_ON_EMPTY_BEANS on +public class CustomScalarInputFixtures { + + public static class Secret { + private final String value; + + public Secret() { + this("default"); + } + + public Secret(String value) { + this.value = value; + } + + @Override + public String toString() { + return value; + } + } +} diff --git a/src/test/kotlin/graphql/kickstart/tools/CustomScalarInputTest.kt b/src/test/kotlin/graphql/kickstart/tools/CustomScalarInputTest.kt index f4311f80..39221484 100644 --- a/src/test/kotlin/graphql/kickstart/tools/CustomScalarInputTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/CustomScalarInputTest.kt @@ -1,29 +1,32 @@ package graphql.kickstart.tools import com.fasterxml.jackson.annotation.JsonInclude -import com.fasterxml.jackson.core.JsonGenerator -import com.fasterxml.jackson.core.JsonParser -import com.fasterxml.jackson.databind.DeserializationContext -import com.fasterxml.jackson.databind.JsonNode -import com.fasterxml.jackson.databind.SerializerProvider -import com.fasterxml.jackson.databind.annotation.JsonDeserialize -import com.fasterxml.jackson.databind.deser.std.StdDeserializer -import com.fasterxml.jackson.databind.exc.InvalidDefinitionException -import com.fasterxml.jackson.databind.exc.MismatchedInputException -import com.fasterxml.jackson.databind.module.SimpleModule -import com.fasterxml.jackson.databind.node.ObjectNode -import com.fasterxml.jackson.databind.node.POJONode -import com.fasterxml.jackson.databind.ser.std.StdSerializer -import com.fasterxml.jackson.databind.ser.std.ToStringSerializer import graphql.ExceptionWhileDataFetching import graphql.GraphQL import graphql.GraphQLContext import graphql.execution.CoercedVariables +import graphql.kickstart.tools.CustomScalarInputFixtures.Secret import graphql.language.StringValue import graphql.language.Value import graphql.schema.Coercing import graphql.schema.GraphQLScalarType import org.junit.Test +import tools.jackson.core.JsonGenerator +import tools.jackson.core.JsonParser +import tools.jackson.databind.DeserializationContext +import tools.jackson.databind.JsonNode +import tools.jackson.databind.SerializationContext +import tools.jackson.databind.SerializationFeature +import tools.jackson.databind.annotation.JsonDeserialize +import tools.jackson.databind.deser.std.StdDeserializer +import tools.jackson.databind.exc.InvalidDefinitionException +import tools.jackson.databind.exc.MismatchedInputException +import tools.jackson.databind.module.SimpleModule +import tools.jackson.databind.node.ObjectNode +import tools.jackson.databind.node.POJONode +import tools.jackson.databind.ser.std.StdSerializer +import tools.jackson.databind.ser.std.ToStringSerializer +import tools.jackson.module.kotlin.jacksonMapperBuilder import java.time.LocalDate import java.util.* import javax.servlet.http.Part @@ -49,7 +52,7 @@ class CustomScalarInputTest { events(inputs: [{ date: "2020-01-02" }, { opaque: "second" }]) optionalEvent(input: { date: "2020-01-02", opaque: "optional" }) message(input: { name: "greeting", file: ${'$'}file, files: ${'$'}files }) - dateNode(input: { date: "2020-01-02" }) + secretNode(input: { secret: "s" }) dateLabel: label(input: { date: "2020-01-02" }) opaqueLabel: label(input: { opaque: "secret" }) } @@ -64,7 +67,7 @@ class CustomScalarInputTest { "events" to listOf("2020-01-02", "second"), "optionalEvent" to "2020-01-02 optional", "message" to "greeting: Hello big world", - "dateNode" to "LocalDate 2020-01-02", + "secretNode" to "Secret s", "dateLabel" to "2020-01-02 null" )) assert((data["opaqueLabel"] as String).startsWith("null ${Opaque::class.java.name}@")) { data["opaqueLabel"].toString() } @@ -142,9 +145,9 @@ class CustomScalarInputTest { """ query { payment(input: { price: "5", reference: "ref", box: "text" }) - withDate: payment(input: { price: "5", reference: "ref", box: "text", date: "2020-01-02" }) - boxOfDate: payment(input: { box: "date:2021-03-04" }) - afterBoxOfDate: payment(input: { price: "5", reference: "ref", box: "text", date: "2020-01-02" }) + withSecret: payment(input: { price: "5", reference: "ref", box: "text", secret: "s" }) + boxOfSecret: payment(input: { box: "secret:s" }) + afterBoxOfSecret: payment(input: { price: "5", reference: "ref", box: "text", secret: "s" }) priceNode(input: { price: "5" }) price(price: "5") } @@ -160,33 +163,49 @@ class CustomScalarInputTest { assertEquals(data, mapOf( "payment" to "MoneyDto(amount=5, currency=CAD) OpaqueDto(value=ref) text", - "withDate" to "MoneyDto(amount=5, currency=CAD) OpaqueDto(value=ref) 2020-01-02 text", - "boxOfDate" to "", - "afterBoxOfDate" to "MoneyDto(amount=5, currency=CAD) OpaqueDto(value=ref) 2020-01-02 text", + "withSecret" to "MoneyDto(amount=5, currency=CAD) OpaqueDto(value=ref) s text", + "boxOfSecret" to "", + "afterBoxOfSecret" to "MoneyDto(amount=5, currency=CAD) OpaqueDto(value=ref) s text", "priceNode" to """{"price":{"amount":5,"currency":"CAD"}}""", "price" to "MoneyDto(amount=5, currency=CAD)" )) assertEquals(configured, mapOf("event" to "2020-01-03 copy of a.txt: Hello")) } + @Test + fun `custom scalars without visible properties are passed even if the ObjectMapper writes them as empty objects`() { + val mapper = jacksonMapperBuilder().disable(SerializationFeature.FAIL_ON_EMPTY_BEANS).build() + val options = SchemaParserOptions.newOptions().objectMapperProvider { mapper }.build() + + val data = assertNoGraphQlErrors(graphQL(inputSchema, options)) { + """ + query { + payment(input: { secret: "kept" }) + } + """ + } + + assertEquals(data, mapOf("payment" to "kept")) + } + @Test fun `errors converting inputs with custom scalars name the actual problem`() { val result = graphQL(inputSchema).execute( """ query { - message(input: { name: null, date: "2020-01-02" }) + message(input: { name: null, secret: "s" }) } """) val exception = (result.errors.single() as ExceptionWhileDataFetching).exception - assertEquals((exception.cause as MismatchedInputException).path.map { it.fieldName }, listOf("name")) - assertEquals((exception.suppressed.single().cause as InvalidDefinitionException).type.rawClass, LocalDate::class.java) + assertEquals((exception as MismatchedInputException).path.map { it.propertyName }, listOf("name")) + assertEquals((exception.suppressed.single() as InvalidDefinitionException).type.rawClass, Secret::class.java) } private fun graphQL(schema: String, options: SchemaParserOptions = SchemaParserOptions.defaultOptions()): GraphQL { val graphQLSchema = SchemaParser.newParser() .schemaString(schema) - .scalars(localDateScalar, opaqueScalar, boxScalar, moneyScalar, customScalarUUID, uploadScalar) + .scalars(localDateScalar, opaqueScalar, secretScalar, boxScalar, moneyScalar, customScalarUUID, uploadScalar) .resolvers(resolver) .options(options) .build() @@ -198,6 +217,7 @@ class CustomScalarInputTest { private val inputSchema = """ scalar LocalDate scalar Opaque + scalar Secret scalar Box scalar Money scalar Upload @@ -208,7 +228,7 @@ class CustomScalarInputTest { optionalEvent(input: EventInput): String! message(input: MessageInput!): String! label(input: LabelInput!): String! - dateNode(input: NodeInput!): String! + secretNode(input: NodeInput!): String! priceNode(input: NodeInput!): String! payment(input: PaymentInput!): String! price(price: Money!): String! @@ -222,7 +242,7 @@ class CustomScalarInputTest { input MessageInput { name: String - date: LocalDate + secret: Secret file: Upload files: [Upload!] } @@ -233,26 +253,26 @@ class CustomScalarInputTest { } input NodeInput { - date: LocalDate + secret: Secret price: Money } input PaymentInput { price: Money reference: Opaque - date: LocalDate + secret: Secret box: Box } """ private val configuredMapperOptions = SchemaParserOptions.newOptions() - .objectMapperConfigurer { mapper, _ -> - mapper.registerModule(SimpleModule() + .objectMapperConfigurer { builder, _ -> + builder.addModule(SimpleModule() .addSerializer(LocalDate::class.java, ToStringSerializer.instance) .addDeserializer(LocalDate::class.java, NextDayDeserializer()) .addSerializer(Part::class.java, PartSerializer()) .addDeserializer(Part::class.java, PartDeserializer())) - mapper.setDefaultPropertyInclusion(JsonInclude.Include.NON_NULL) + builder.changeDefaultPropertyInclusion { JsonInclude.Value.construct(JsonInclude.Include.NON_NULL, JsonInclude.Include.NON_NULL) } } .build() @@ -266,9 +286,9 @@ class CustomScalarInputTest { fun optionalEvent(input: Optional): String = input.map { event(it) }.orElse("empty") fun message(input: MessageInput): String = "${input.name}: ${(listOfNotNull(input.file) + input.files).joinToString(" ") { it.content() }}" fun label(input: LabelInput): String = "${input.date} ${input.opaque}" - fun dateNode(input: ObjectNode): String = (input["date"] as POJONode?)?.pojo?.let { "${it.javaClass.simpleName} $it" }.orEmpty() + fun secretNode(input: ObjectNode): String = (input["secret"] as POJONode?)?.pojo?.let { "${it.javaClass.simpleName} $it" }.orEmpty() fun priceNode(input: ObjectNode): String = input.toString() - fun payment(input: PaymentInput): String = listOfNotNull(input.price, input.reference, input.date, input.box).joinToString(" ") + fun payment(input: PaymentInput): String = listOfNotNull(input.price, input.reference, input.secret, input.box).joinToString(" ") fun price(price: MoneyDto): String = price.toString() fun map(input: Map) = receive("map", input) @@ -291,14 +311,14 @@ class CustomScalarInputTest { var file: Part? = null } - data class MessageInput(val name: String, val date: LocalDate? = null, val file: Part? = null, val files: List = emptyList()) + data class MessageInput(val name: String, val secret: Secret? = null, val file: Part? = null, val files: List = emptyList()) data class LabelInput(val date: String? = null, val opaque: String? = null) class PaymentInput { var price: MoneyDto? = null var reference: OpaqueDto? = null - var date: LocalDate? = null + var secret: Secret? = null @field:JsonDeserialize(using = BoxContentDeserializer::class) var box: String? = null @@ -318,32 +338,33 @@ class CustomScalarInputTest { class BoxContentDeserializer : StdDeserializer(String::class.java) { override fun deserialize(p: JsonParser, context: DeserializationContext): String = - p.codec.readTree(p).path("content").textValue() ?: "" + p.readValueAsTree().path("content").stringValue("") } class NextDayDeserializer : StdDeserializer(LocalDate::class.java) { - override fun deserialize(p: JsonParser, context: DeserializationContext): LocalDate = LocalDate.parse(p.text).plusDays(1) + override fun deserialize(p: JsonParser, context: DeserializationContext): LocalDate = LocalDate.parse(p.string).plusDays(1) } class PartSerializer : StdSerializer(Part::class.java) { - override fun serialize(value: Part, gen: JsonGenerator, provider: SerializerProvider) { + override fun serialize(value: Part, gen: JsonGenerator, provider: SerializationContext) { gen.writeStartObject() - gen.writeStringField("name", value.name) - gen.writeBinaryField("content", value.inputStream.readBytes()) + gen.writeStringProperty("name", value.name) + gen.writeBinaryProperty("content", value.inputStream.readBytes()) gen.writeEndObject() } } class PartDeserializer : StdDeserializer(Part::class.java) { override fun deserialize(p: JsonParser, context: DeserializationContext): Part { - val node = p.codec.readTree(p) - return MockPart("copy of ${node["name"].textValue()}", String(node["content"].binaryValue())) + val node = context.readTree(p) + return MockPart("copy of ${node["name"].stringValue()}", String(node["content"].binaryValue())) } } private val localDateScalar = scalar("LocalDate") { LocalDate.parse(it) } private val opaqueScalar = scalar("Opaque") { Opaque(it) } - private val boxScalar = scalar("Box") { Box(if (it.startsWith("date:")) LocalDate.parse(it.removePrefix("date:")) else it) } + private val secretScalar = scalar("Secret") { Secret(it) } + private val boxScalar = scalar("Box") { Box(if (it.startsWith("secret:")) Secret(it.removePrefix("secret:")) else it) } private val moneyScalar = scalar("Money") { Money(it.toInt(), "CAD") } private fun scalar(name: String, parse: (String) -> T): GraphQLScalarType = GraphQLScalarType.newScalar() diff --git a/src/test/kotlin/graphql/kickstart/tools/NestedInputTypesTest.kt b/src/test/kotlin/graphql/kickstart/tools/NestedInputTypesTest.kt index 4c1ea779..7fae924b 100644 --- a/src/test/kotlin/graphql/kickstart/tools/NestedInputTypesTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/NestedInputTypesTest.kt @@ -1,10 +1,10 @@ package graphql.kickstart.tools import com.fasterxml.jackson.annotation.JsonProperty -import com.fasterxml.jackson.databind.node.ObjectNode import graphql.GraphQL import graphql.execution.AsyncExecutionStrategy import org.junit.Test +import tools.jackson.databind.node.ObjectNode class NestedInputTypesTest { @@ -177,8 +177,8 @@ class NestedInputTypesTest { id: ID! # named like methods of ObjectNode values: [Nested] - elements: [Nested] - fields: [Nested] + properties: [Nested] + propertyNames: [Nested] } input Nested { @@ -200,13 +200,13 @@ class NestedInputTypesTest { """ mutation { create(input: { name: "n", nested: { x: 1 } }) - update(input: { id: "2", values: [{ x: 3 }], elements: [{ x: 4 }], fields: [{ x: 5 }] }) + update(input: { id: "2", values: [{ x: 3 }], properties: [{ x: 4 }], propertyNames: [{ x: 5 }] }) } """ } assertEquals(data["create"], """{"name":"n","nested":{"x":1}}""") - assertEquals(data["update"], """{"id":"2","values":[{"x":3}],"elements":[{"x":4}],"fields":[{"x":5}]}""") + assertEquals(data["update"], """{"id":"2","values":[{"x":3}],"properties":[{"x":4}],"propertyNames":[{"x":5}]}""") } @Test diff --git a/src/test/kotlin/graphql/kickstart/tools/ObjectMapperOptionsTest.kt b/src/test/kotlin/graphql/kickstart/tools/ObjectMapperOptionsTest.kt new file mode 100644 index 00000000..cba638af --- /dev/null +++ b/src/test/kotlin/graphql/kickstart/tools/ObjectMapperOptionsTest.kt @@ -0,0 +1,112 @@ +package graphql.kickstart.tools + +import graphql.GraphQL +import graphql.language.FieldDefinition +import graphql.language.TypeName +import org.junit.Assert.assertNotSame +import org.junit.Assert.assertSame +import org.junit.Test +import tools.jackson.module.kotlin.KotlinFeature +import tools.jackson.module.kotlin.KotlinModule + +class ObjectMapperOptionsTest { + + @Test + fun `enums are converted to and from Strings by name`() { + val schema = SchemaParser.newParser() + .schemaString( + """ + type Query { + names(input: ColorInput!): String + colors(input: ColorNameInput!): String + } + + enum Color { + RED + } + + input ColorInput { + color: Color! + } + + input ColorNameInput { + color: String! + } + """) + .resolvers(object : GraphQLQueryResolver { + fun names(input: Map): String = input.getValue("color") + fun colors(input: ColorNameInput): String = input.color.name + }) + .dictionary(Color::class) + .build() + .makeExecutableSchema() + .assertEveryFieldResolves(except = setOf("Query.colors")) + + val data = assertNoGraphQlErrors(GraphQL.newGraphQL(schema).build()) { + """ + query { + names(input: { color: RED }) + colors(input: { color: "RED" }) + } + """ + } + + assertEquals(data, mapOf("names" to "RED", "colors" to "RED")) + } + + @Test + fun `fields share the library's ObjectMapper unless a configurer is set`() { + val first = FieldDefinition.newFieldDefinition().name("first").type(TypeName("String")).build() + val second = FieldDefinition.newFieldDefinition().name("second").type(TypeName("String")).build() + val library = SchemaParserOptions.defaultOptions().objectMapperProvider + val configured = SchemaParserOptions.newOptions().objectMapperConfigurer { _, _ -> }.build().objectMapperProvider + + assertSame(library.provide(first), library.provide(second)) + assertNotSame(configured.provide(first), configured.provide(second)) + } + + @Test + fun `a KotlinModule from the configurer replaces the library's`() { + val query = """query { names(input: { names: ["a", null] }) }""" + + val strict = namesGraphQL(SchemaParserOptions.defaultOptions()).execute(query) + val lenient = namesGraphQL(SchemaParserOptions.newOptions() + .objectMapperConfigurer { builder, _ -> builder.addModule(KotlinModule.Builder().disable(KotlinFeature.StrictNullChecks).build()) } + .build()).execute(query) + + assertEquals(strict.errors.single().path, listOf("names")) + assertEquals(lenient.getData>(), mapOf("names" to "[a, null]")) + } + + private fun namesGraphQL(options: SchemaParserOptions): GraphQL { + val schema = SchemaParser.newParser() + .schemaString( + """ + type Query { + names(input: NamesInput!): String + } + + input NamesInput { + names: [String]! + } + """) + .resolvers(object : GraphQLQueryResolver { + fun names(input: NamesInput): String = input.names.toString() + }) + .options(options) + .build() + .makeExecutableSchema() + .assertEveryFieldResolves() + return GraphQL.newGraphQL(schema).build() + } + + enum class Color { + RED; + + override fun toString() = "red-ish" + } + + data class ColorNameInput(val color: Color) + + data class NamesInput(val names: List) +} diff --git a/src/test/kotlin/graphql/kickstart/tools/ResolverMethodsTest.java b/src/test/kotlin/graphql/kickstart/tools/ResolverMethodsTest.java index 17416eca..245029ee 100644 --- a/src/test/kotlin/graphql/kickstart/tools/ResolverMethodsTest.java +++ b/src/test/kotlin/graphql/kickstart/tools/ResolverMethodsTest.java @@ -40,6 +40,21 @@ public void testOmittedBooleanArgument() { assertEquals("false,null", ((Map) result.getData()).get("testOmittedBoolean")); } + @Test + public void testNullPrimitiveInputField() { + GraphQLSchema schema = SchemaParser.newParser() + .schemaString("type Query { limit(page: Page!): Int } input Page { limit: Int }") + .resolvers(new PageLimitResolver()) + .build() + .makeExecutableSchema(); + + ExecutionResult result = GraphQL.newGraphQL(schema).build() + .execute("query { limit(page: { limit: null }) }"); + + assertEquals(1, result.getErrors().size()); + assertEquals(List.of("limit"), result.getErrors().get(0).getPath()); + } + // Kotlin reflection used to fail on anonymous Java classes (KT-41373), so these resolvers must stay in Java. @Test public void testAnonymousClassResolvers() { @@ -140,6 +155,18 @@ public int size(Page page) { } } + static class PageInput { + public int limit; + } + + static class PageLimitResolver implements GraphQLQueryResolver { + + @SuppressWarnings("unused") + public int limit(PageInput page) { + return page.limit; + } + } + static class Resolver implements GraphQLQueryResolver { @SuppressWarnings("unused") diff --git a/src/test/kotlin/graphql/kickstart/tools/SchemaParserDictionaryTest.kt b/src/test/kotlin/graphql/kickstart/tools/SchemaParserDictionaryTest.kt index 8660bca4..17a4adf5 100644 --- a/src/test/kotlin/graphql/kickstart/tools/SchemaParserDictionaryTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/SchemaParserDictionaryTest.kt @@ -1,9 +1,9 @@ package graphql.kickstart.tools -import com.fasterxml.jackson.databind.node.ObjectNode import graphql.GraphQL import org.junit.Assert.assertThrows import org.junit.Test +import tools.jackson.databind.node.ObjectNode class SchemaParserDictionaryTest { diff --git a/src/test/kotlin/graphql/kickstart/tools/TestUtils.kt b/src/test/kotlin/graphql/kickstart/tools/TestUtils.kt index 98a78d47..2874c8f7 100644 --- a/src/test/kotlin/graphql/kickstart/tools/TestUtils.kt +++ b/src/test/kotlin/graphql/kickstart/tools/TestUtils.kt @@ -4,13 +4,10 @@ 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.databind.ObjectMapper import graphql.ExecutionInput import graphql.GraphQL import org.slf4j.LoggerFactory -private val mapper = ObjectMapper() - fun assertNoGraphQlErrors(gql: GraphQL, args: Map = mapOf(), context: Map = mapOf(), closure: () -> String): Map { val result = gql.execute( ExecutionInput.newExecutionInput() diff --git a/src/test/kotlin/graphql/kickstart/tools/UnmappedInputFieldsTest.kt b/src/test/kotlin/graphql/kickstart/tools/UnmappedInputFieldsTest.kt index 0a6aad28..802ea71e 100644 --- a/src/test/kotlin/graphql/kickstart/tools/UnmappedInputFieldsTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/UnmappedInputFieldsTest.kt @@ -7,22 +7,6 @@ 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.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 graphql.GraphQL import graphql.kickstart.tools.UnmappedInputFieldsFixtures.* import graphql.schema.DataFetchingEnvironment @@ -31,6 +15,23 @@ import graphql.schema.idl.SchemaDirectiveWiring import graphql.schema.idl.SchemaDirectiveWiringEnvironment import org.junit.Assert.assertThrows import org.junit.Test +import tools.jackson.core.JsonParser +import tools.jackson.databind.BeanDescription +import tools.jackson.databind.DeserializationConfig +import tools.jackson.databind.DeserializationContext +import tools.jackson.databind.DeserializationFeature +import tools.jackson.databind.InjectableValues +import tools.jackson.databind.JavaType +import tools.jackson.databind.JsonNode +import tools.jackson.databind.ObjectMapper +import tools.jackson.databind.PropertyNamingStrategies +import tools.jackson.databind.ValueDeserializer +import tools.jackson.databind.annotation.JsonDeserialize +import tools.jackson.databind.annotation.JsonNaming +import tools.jackson.databind.annotation.JsonPOJOBuilder +import tools.jackson.databind.deser.std.StdDeserializer +import tools.jackson.databind.module.SimpleDeserializers +import tools.jackson.databind.module.SimpleModule import java.util.* class UnmappedInputFieldsTest { @@ -47,7 +48,7 @@ class UnmappedInputFieldsTest { createProject(input: { name: "tools", technologies: [{ name: "Kotlin" }] }) } """) - assertContains(result.errors.single().message, "Unrecognized field \"technologies\"") + assertContains(result.errors.single().message, "Unrecognized property \"technologies\"") } @Test @@ -93,7 +94,7 @@ class UnmappedInputFieldsTest { )) val result = GraphQL.newGraphQL(schema).build().execute("query { active(input: { active: true }) }") - assertContains(result.errors.single().message, "Unrecognized field \"active\"") + assertContains(result.errors.single().message, "Unrecognized property \"active\"") } @Test @@ -183,6 +184,16 @@ class UnmappedInputFieldsTest { @Test fun `misspelled property is reported if the ObjectMapper ignores unknown properties`() { + assertReportedAsDropped(SchemaParserOptions.newOptions() + .objectMapperConfigurer { builder, _ -> builder.disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES) }) + } + + @Test + fun `misspelled property is reported as dropped with a Jackson ObjectMapper from a provider`() { + assertReportedAsDropped(SchemaParserOptions.newOptions().objectMapperProvider { ObjectMapper() }) + } + + private fun assertReportedAsDropped(options: SchemaParserOptions.Builder) { val (schema, warnings) = withWarnings { SchemaParser.newParser() .schemaString( @@ -198,9 +209,7 @@ class UnmappedInputFieldsTest { .resolvers(object : GraphQLQueryResolver { fun greet(input: GreetingInput): String? = input.nmae }) - .options(SchemaParserOptions.newOptions() - .objectMapperConfigurer { mapper, _ -> mapper.disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES) } - .build()) + .options(options.build()) .build() .makeExecutableSchema() } @@ -239,9 +248,9 @@ class UnmappedInputFieldsTest { fun camel(input: NameInput): String? = input.firstName }) .options(options - .objectMapperConfigurer { mapper, context -> + .objectMapperConfigurer { builder, context -> if (context.field.name == "snake") { - mapper.setPropertyNamingStrategy(PropertyNamingStrategies.SNAKE_CASE) + builder.propertyNamingStrategy(PropertyNamingStrategies.SNAKE_CASE) } } .build()) @@ -258,7 +267,7 @@ class UnmappedInputFieldsTest { .assertEveryFieldResolves(except = setOf("Query.snake")) val gql = GraphQL.newGraphQL(schema).build() val result = gql.execute("query { snake(input: { firstName: \"Ada\" }) camel(input: { firstName: \"Ada\" }) }") - assertContains(result.errors.single().message, "Unrecognized field \"firstName\"") + assertContains(result.errors.single().message, "Unrecognized property \"firstName\"") assertEquals(result.getData>()!!["camel"], "Ada") } @@ -284,9 +293,9 @@ class UnmappedInputFieldsTest { fun alsoStrict(input: GreetingInput): String? = input.nmae }) .options(failOptions() - .objectMapperConfigurer { mapper, context -> + .objectMapperConfigurer { builder, context -> if (context.field.name == "lenient") { - mapper.disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES) + builder.disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES) } } .build()) @@ -360,7 +369,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\" }) }") - assertContains(result.errors.single().message, "Unrecognized field \"limit\"") + assertContains(result.errors.single().message, "Unrecognized property \"limit\"") } @Test @@ -687,7 +696,7 @@ class UnmappedInputFieldsTest { fun money(input: Money): String = input.amount.toString() }) .options(failOptions() - .objectMapperConfigurer { mapper, _ -> mapper.registerModule(SimpleModule().addDeserializer(Money::class.java, MoneyDeserializer())) } + .objectMapperConfigurer { builder, _ -> builder.addModule(SimpleModule().addDeserializer(Money::class.java, MoneyDeserializer())) } .build()) .build() .makeExecutableSchema() @@ -731,7 +740,6 @@ class UnmappedInputFieldsTest { type Query { injected(input: InjectedInput!): String builder(input: BuilderTypoInput!): String - unwrapped(input: KotlinUnwrappedInput!): String } input InjectedInput { @@ -741,19 +749,13 @@ class UnmappedInputFieldsTest { input BuilderTypoInput { name: String } - - input KotlinUnwrappedInput { - name: String - street: 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}" }) .options(failOptions() - .objectMapperConfigurer { mapper, _ -> mapper.setInjectableValues(InjectableValues.Std().addValue("tenant", "acme")) } + .objectMapperConfigurer { builder, _ -> builder.injectableValues(InjectableValues.Std().addValue("tenant", "acme")) } .build()) .build() .makeExecutableSchema() @@ -765,14 +767,43 @@ class UnmappedInputFieldsTest { query { injected(input: { name: "a" }) builder(input: { name: "b" }) - unwrapped(input: { name: "c", street: "d" }) } """) assertEquals(result.errors.map { it.path }, listOf(listOf("injected"), listOf("builder"))) - result.errors.forEach { assertContains(it.message, "Unrecognized field \"name\"") } - // Jackson drops what none of the unwrapped properties know - assertEquals(result.getData>()!!["unwrapped"], "c null") + result.errors.forEach { assertContains(it.message, "Unrecognized property \"name\"") } + } + + @Test + fun `input field that none of the unwrapped properties know is reported`() { + val parser = { options: SchemaParserOptions.Builder -> + SchemaParser.newParser() + .schemaString( + """ + type Query { + unwrapped(input: KotlinUnwrappedInput!): String + } + + input KotlinUnwrappedInput { + name: String + street: String + } + """) + .resolvers(object : GraphQLQueryResolver { + fun unwrapped(input: KotlinUnwrappedInput): String = "${input.name} ${input.address?.stret}" + }) + .options(options.build()) + .build() + } + + val error = assertThrows(SchemaError::class.java) { + parser(failOptions()).makeExecutableSchema() + } + assertEquals(error.message, unmapped("KotlinUnwrappedInput.street", KotlinUnwrappedInput::class.java, "argument 'input' of field 'Query.unwrapped'", "name")) + + val result = GraphQL.newGraphQL(parser(SchemaParserOptions.newOptions()).makeExecutableSchema()).build() + .execute("query { unwrapped(input: { name: \"c\", street: \"d\" }) }") + assertContains(result.errors.single().message, "Unrecognized property \"street\"") } @Test @@ -798,9 +829,9 @@ class UnmappedInputFieldsTest { fun missingClass(input: GreetingInput): String? = input.nmae }) .options(failOptions() - .objectMapperConfigurer { mapper, context -> + .objectMapperConfigurer { builder, context -> if (context.field.name == "missingClass") { - mapper.registerModule(SimpleModule().apply { setDeserializers(MissingClassDeserializers()) }) + builder.addModule(SimpleModule().apply { setDeserializers(MissingClassDeserializers()) }) } } .build()) @@ -1034,7 +1065,7 @@ class UnmappedInputFieldsTest { } class MissingClassDeserializers : SimpleDeserializers() { - override fun findBeanDeserializer(type: JavaType, config: DeserializationConfig, beanDesc: BeanDescription): JsonDeserializer<*> = + override fun findBeanDeserializer(type: JavaType, config: DeserializationConfig, beanDescRef: BeanDescription.Supplier): ValueDeserializer<*> = throw NoClassDefFoundError("com/example/Missing") }