From f42670c2349911838f79e6215ac1edce0287df7f Mon Sep 17 00:00:00 2001 From: Oryan Date: Mon, 12 Oct 2026 15:47:32 -0400 Subject: [PATCH 1/5] Keep coerced custom scalar values in input objects and Map arguments Arguments that need converting go through ObjectMapper.convertValue, which serializes the values graphql-java already coerced and deserializes them again. Custom scalar values Jackson can't do that for, such as an upload or a LocalDate without the jsr310 module, failed inside input objects (#436, #466), and a Map argument got bean-like scalar values as a nested Map (#424). Arguments are still converted by the ObjectMapper first, so anything that converts today gets the same result. If that fails, the values that don't convert to their own class are written as embedded objects, which Jackson's deserializers return as they are, with a problem handler for interfaces such as Part. If that fails too, its error is thrown, with the original one suppressed. Nothing that reads an untyped value, such as those of a Map, depends on the serialized form, so there the scalar values are kept unless the mapper has a serializer for them, e.g. UUID, or java.time with the jsr310 module. Co-Authored-By: Claude Opus 5.5 --- .../tools/resolver/ArgumentConverter.kt | 140 ++++++ .../tools/resolver/MethodFieldResolver.kt | 6 +- .../kickstart/tools/CustomScalarInputTest.kt | 466 ++++++++++++++++++ 3 files changed, 608 insertions(+), 4 deletions(-) create mode 100644 src/main/kotlin/graphql/kickstart/tools/resolver/ArgumentConverter.kt create mode 100644 src/test/kotlin/graphql/kickstart/tools/CustomScalarInputTest.kt diff --git a/src/main/kotlin/graphql/kickstart/tools/resolver/ArgumentConverter.kt b/src/main/kotlin/graphql/kickstart/tools/resolver/ArgumentConverter.kt new file mode 100644 index 00000000..318d2d48 --- /dev/null +++ b/src/main/kotlin/graphql/kickstart/tools/resolver/ArgumentConverter.kt @@ -0,0 +1,140 @@ +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.JsonMappingException +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.ser.impl.UnknownSerializer +import com.fasterxml.jackson.databind.ser.impl.UnsupportedTypeSerializer +import com.fasterxml.jackson.databind.ser.std.BeanSerializerBase +import com.fasterxml.jackson.databind.util.TokenBuffer +import java.io.IOException +import java.lang.reflect.Type +import java.util.concurrent.ConcurrentHashMap + +/** + * Converts an argument value that graphql-java already coerced to the type of the resolver method's parameter. + * + * Jackson converts a value by serializing it and deserializing it again, which the coerced values of custom scalars + * often don't survive, e.g. an upload, or a LocalDate without the jsr310 module. Those are handed to Jackson as embedded + * objects instead, which its deserializers return as they are if they have the type being read. Where a String is read, + * they're converted with toString(). + */ +internal class ArgumentConverter(private val mapper: ObjectMapper, type: Type) { + + private val javaType = mapper.typeFactory.constructType(type) + private val isUntyped = javaType.isUntyped() + private val convertsToOwnClassCache = ConcurrentHashMap, Boolean>() + + fun convert(value: Any?): Any? { + if (value == null) { + return mapper.convertValue(value, javaType) + } + + if (isUntyped) { + // nothing that reads an untyped value depends on its serialized form, so the scalar values are kept, + // unless the mapper has a serializer for them that defines that form + val embedded = embedValues(value) { !hasSerializerOfItsOwn(it) } + return if (embedded != null) readEmbedded(embedded) else mapper.convertValue(value, javaType) + } + + // a deserializer may depend on the serialized form, so values are only embedded if converting fails + return try { + mapper.convertValue(value, javaType) + } catch (e: IllegalArgumentException) { + val embedded = embedValues(value) { !convertsToOwnClass(it) } ?: throw e + try { + readEmbedded(embedded) + } catch (embeddedError: IllegalArgumentException) { + // the values Jackson couldn't convert likely hid the actual problem + throw embeddedError.apply { addSuppressed(e) } + } + } + } + + /** + * Returns a copy of the value with the scalar values [isEmbedded] matches wrapped in [EmbeddedValue], or null if there are none. + */ + private fun embedValues(value: Any, isEmbedded: (Any) -> Boolean): Any? { + var embedsAny = false + fun embed(value: Any?): Any? = when (value) { + null, is String, is Number, is Boolean, is Enum<*> -> value + is Map<*, *> -> value.mapValuesTo(LinkedHashMap()) { embed(it.value) } + is Collection<*> -> value.map { embed(it) } + else -> if (isEmbedded(value)) EmbeddedValue(value).also { embedsAny = true } else value + } + + return embed(value).takeIf { embedsAny } + } + + // a bean serializer writes the value's properties, which an untyped value would get as a Map, the other two fail + private fun hasSerializerOfItsOwn(value: Any): Boolean { + val serializer = try { + mapper.serializerProviderInstance.findValueSerializer(value.javaClass) + } catch (e: JsonMappingException) { + return false + } + return serializer !is BeanSerializerBase && serializer !is UnknownSerializer && serializer !is UnsupportedTypeSerializer + } + + // the values of a scalar usually all convert or all don't, and converting one can be expensive, e.g. an upload + private fun convertsToOwnClass(value: Any): Boolean = convertsToOwnClassCache.getOrPut(value.javaClass) { + try { + mapper.convertValue(value, value.javaClass) + true + } catch (e: IllegalArgumentException) { + false + } + } + + // like ObjectMapper.convertValue, but reads embedded values + private fun readEmbedded(value: Any): Any? { + val buffer = TokenBuffer(mapper, false).forceUseOfBigDecimal(mapper.isEnabled(DeserializationFeature.USE_BIG_DECIMAL_FOR_FLOATS)) + return try { + mapper.writer().without(SerializationFeature.WRAP_ROOT_VALUE).writeValue(buffer, value) + mapper.readerFor(javaType) + .without(DeserializationFeature.UNWRAP_ROOT_VALUE) + .withHandler(embeddedValueHandler) + .readValue(buffer.asParser()) + } catch (e: IOException) { + throw IllegalArgumentException(e.message, e) + } + } + + private val embeddedValueHandler = object : DeserializationProblemHandler() { + // an interface or abstract class, e.g. Part, has no deserializer that returns embedded values + override fun handleMissingInstantiator( + context: DeserializationContext, + instClass: Class<*>, + valueInstantiator: ValueInstantiator?, + p: JsonParser, + msg: String? + ): Any? { + val value = if (p.hasToken(JsonToken.VALUE_EMBEDDED_OBJECT)) p.embeddedObject else null + return if (instClass.isInstance(value)) value else NOT_HANDLED + } + + // 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) + } + + private class EmbeddedValue(private val value: Any) : JsonSerializable { + override fun serialize(gen: JsonGenerator, serializers: SerializerProvider) = gen.writeEmbeddedObject(value) + + override fun serializeWithType(gen: JsonGenerator, serializers: SerializerProvider, typeSer: TypeSerializer) = gen.writeEmbeddedObject(value) + } +} + +// Object, or a Map, Collection, array or Optional of it, is read by UntypedObjectDeserializer, which returns embedded values as they are +private fun JavaType.isUntyped(): Boolean = isJavaLangObject || (isContainerType || isReferenceType) && contentType.isUntyped() diff --git a/src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt b/src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt index 0a9e65d6..5d7df72f 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.core.type.TypeReference import graphql.GraphQLContext import graphql.kickstart.tools.* import graphql.kickstart.tools.SchemaParserOptions.GenericWrapper @@ -77,6 +76,7 @@ internal class MethodFieldResolver( val isNonNull = definition.type is NonNullType val isOptional = this.typeResolver.getRawClass(parameterType) == Optional::class.java + val argumentConverter = ArgumentConverter(mapper, parameterType) args.add { environment -> val argumentPresent = environment.arguments.containsKey(definition.name) @@ -94,9 +94,7 @@ internal class MethodFieldResolver( } if (value == null || shouldValueBeConverted(value, definition, parameterType, environment)) { - return@add mapper.convertValue(value, object : TypeReference() { - override fun getType() = parameterType - }) + return@add argumentConverter.convert(value) } return@add value diff --git a/src/test/kotlin/graphql/kickstart/tools/CustomScalarInputTest.kt b/src/test/kotlin/graphql/kickstart/tools/CustomScalarInputTest.kt new file mode 100644 index 00000000..7e16176c --- /dev/null +++ b/src/test/kotlin/graphql/kickstart/tools/CustomScalarInputTest.kt @@ -0,0 +1,466 @@ +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.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.ExecutionInput +import graphql.GraphQL +import graphql.GraphQLContext +import graphql.execution.CoercedVariables +import graphql.language.StringValue +import graphql.language.Value +import graphql.schema.Coercing +import graphql.schema.GraphQLScalarType +import kotlinx.coroutines.ExperimentalCoroutinesApi +import org.junit.Test +import java.time.LocalDate +import java.util.* +import javax.servlet.http.Part + +@OptIn(ExperimentalCoroutinesApi::class) +class CustomScalarInputTest { + + @Test + fun `custom scalars in input objects are passed to the resolver`() { + val schema = SchemaParser.newParser() + .schemaString( + """ + scalar LocalDate + scalar Opaque + + type Query { + event(input: EventInput!): String! + events(inputs: [EventInput!]!): [String!]! + optionalEvent(input: EventInput): String! + eventNode(input: EventNodeInput!): String! + } + + input EventInput { + date: LocalDate + opaque: Opaque + } + + input EventNodeInput { + date: LocalDate + } + """) + .scalars(localDateScalar, opaqueScalar) + .resolvers(EventQueryResolver()) + .build() + .makeExecutableSchema() + val gql = GraphQL.newGraphQL(schema).build() + + val data = assertNoGraphQlErrors(gql, mapOf("input" to mapOf("date" to "2021-03-04", "opaque" to "variable"))) { + """ + query(${'$'}input: EventInput!) { + literal: event(input: { date: "2020-01-02", opaque: "literal" }) + variable: event(input: ${'$'}input) + events(inputs: [{ date: "2020-01-02" }, { opaque: "second" }]) + optionalEvent(input: { date: "2020-01-02", opaque: "optional" }) + eventNode(input: { date: "2020-01-02" }) + } + """ + } + + assertEquals(data, mapOf( + "literal" to "2020-01-02 literal", + "variable" to "2021-03-04 variable", + "events" to listOf("2020-01-02 null", "null second"), + "optionalEvent" to "2020-01-02 optional", + "eventNode" to "LocalDate 2020-01-02" + )) + } + + @Test + fun `uploads in input objects are passed to the resolver`() { + val schema = SchemaParser.newParser() + .schemaString( + """ + scalar Upload + + type Query { + upload(input: UploadInput!): String! + javaUpload(input: JavaUploadInput!): String! + } + + input UploadInput { + name: String! + file: Upload + files: [Upload!] + } + + input JavaUploadInput { + file: Upload + } + """) + .scalars(uploadScalar) + .resolvers(UploadQueryResolver()) + .build() + .makeExecutableSchema() + val gql = GraphQL.newGraphQL(schema).build() + + val files = mapOf("file" to MockPart("a.txt", "Hello"), "files" to listOf(MockPart("b.txt", "big"), MockPart("c.txt", "world"))) + val data = assertNoGraphQlErrors(gql, files) { + """ + query(${'$'}file: Upload, ${'$'}files: [Upload!]) { + upload(input: { name: "greeting", file: ${'$'}file, files: ${'$'}files }) + javaUpload(input: { file: ${'$'}file }) + } + """ + } + + assertEquals(data, mapOf( + "upload" to "greeting: Hello big world", + "javaUpload" to "a.txt: Hello" + )) + } + + @Test + fun `custom scalars in Map arguments are passed as coerced values`() { + val resolver = MapQueryResolver() + val schema = SchemaParser.newParser() + .schemaString( + """ + scalar LocalDate + scalar Money + scalar UUID + + type Query { + event(input: EventInput!): String! + eventMap(input: EventMapInput!): Boolean! + } + + input EventInput { + date: LocalDate + price: Money + id: UUID + } + + input EventMapInput { + name: String + date: LocalDate + price: Money + id: UUID + next: EventMapInput + } + """) + .scalars(localDateScalar, moneyScalar, uuidScalar) + .resolvers(resolver) + .build() + .makeExecutableSchema() + val gql = GraphQL.newGraphQL(schema).build() + + assertNoGraphQlErrors(gql) { + """ + query { + eventMap(input: { price: "5" }) + } + """ + } + + assertEquals(resolver.input, mapOf("price" to Money(5, "CAD"))) + + assertNoGraphQlErrors(gql) { + """ + query { + eventMap(input: { + name: null + date: "2020-01-02" + price: "5" + id: "4dbd9a3d-4a26-4d10-9bb2-5b2dc4c8a0f0" + next: { date: "2021-03-04" } + }) + } + """ + } + + // the ObjectMapper has a serializer for UUID, so that still arrives in its serialized form + assertEquals(resolver.input, mapOf( + "name" to null, + "date" to LocalDate.of(2020, 1, 2), + "price" to Money(5, "CAD"), + "id" to "4dbd9a3d-4a26-4d10-9bb2-5b2dc4c8a0f0", + "next" to mapOf("date" to LocalDate.of(2021, 3, 4)) + )) + } + + @Test + fun `custom scalars the ObjectMapper can convert are still converted by it`() { + val schema = SchemaParser.newParser() + .schemaString( + """ + scalar LocalDate + scalar Money + scalar Opaque + scalar GeoPoint + + type Query { + payment(input: PaymentInput!): String! + paymentNode(input: PaymentNodeInput!): String! + price(price: Money!): String! + } + + input PaymentInput { + price: Money + reference: Opaque + location: GeoPoint + date: LocalDate + } + + input PaymentNodeInput { + price: Money + } + """) + .scalars(localDateScalar, moneyScalar, opaqueScalar, geoPointScalar) + .resolvers(PaymentQueryResolver()) + .build() + .makeExecutableSchema() + val gql = GraphQL.newGraphQL(schema).build() + + val data = assertNoGraphQlErrors(gql) { + """ + query { + payment(input: { price: "5", reference: "ref", location: "1.5,2.5" }) + withDate: payment(input: { price: "5", reference: "ref", location: "1.5,2.5", date: "2020-01-02" }) + paymentNode(input: { price: "5" }) + price(price: "5") + } + """ + } + + assertEquals(data, mapOf( + "payment" to "MoneyDto(amount=5, currency=CAD) OpaqueDto(value=ref) GeoPoint(lat=1.5, lng=2.5) null", + "withDate" to "MoneyDto(amount=5, currency=CAD) OpaqueDto(value=ref) GeoPoint(lat=1.5, lng=2.5) 2020-01-02", + "paymentNode" to """{"price":{"amount":5,"currency":"CAD"}}""", + "price" to "MoneyDto(amount=5, currency=CAD)" + )) + } + + @Test + fun `custom scalars in inputs are converted by a configured ObjectMapper`() { + val resolver = MapQueryResolver() + val schema = SchemaParser.newParser() + .schemaString( + """ + scalar LocalDate + scalar Upload + + type Query { + event(input: EventInput!): String! + eventMap(input: EventMapInput!): Boolean! + javaUpload(input: JavaUploadInput!): String! + } + + input EventInput { + date: LocalDate + } + + input EventMapInput { + name: String + date: LocalDate + } + + input JavaUploadInput { + file: Upload + } + """) + .scalars(localDateScalar, uploadScalar) + .resolvers(resolver, UploadQueryResolver()) + .options(SchemaParserOptions.newOptions() + .objectMapperConfigurer { mapper, _ -> + mapper.registerModule(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) + } + .build()) + .build() + .makeExecutableSchema() + val gql = GraphQL.newGraphQL(schema).build() + + val data = assertNoGraphQlErrors(gql, mapOf("file" to MockPart("a.txt", "Hello"))) { + """ + query(${'$'}file: Upload) { + event(input: { date: "2020-01-02" }) + eventMap(input: { name: null, date: "2020-01-02" }) + javaUpload(input: { file: ${'$'}file }) + } + """ + } + + assertEquals(data, mapOf("event" to "2020-01-03 null", "eventMap" to true, "javaUpload" to "copy of a.txt: Hello")) + assertEquals(resolver.input, mapOf("date" to "2020-01-02")) + } + + @Test + fun `errors converting inputs with custom scalars name the actual problem`() { + val schema = SchemaParser.newParser() + .schemaString( + """ + scalar LocalDate + + type Query { + defaulted(input: DefaultedInput!): String! + } + + input DefaultedInput { + name: String + date: LocalDate + } + """) + .scalars(localDateScalar) + .resolvers(DefaultedQueryResolver()) + .build() + .makeExecutableSchema() + val gql = GraphQL.newGraphQL(schema).build() + + val result = gql.execute(ExecutionInput.newExecutionInput().query( + """ + query { + defaulted(input: { name: null, date: "2020-01-02" }) + } + """)) + + assertEquals(result.errors.size, 1) + val exception = (result.errors[0] as ExceptionWhileDataFetching).exception + assert(exception.message.orEmpty().contains("JSON property name due to missing")) { exception.message.orEmpty() } + val conversionError = exception.suppressed.single() + assert(conversionError.message.orEmpty().startsWith("Java 8 date/time type `java.time.LocalDate` not supported")) { conversionError.message.orEmpty() } + } + + class EventQueryResolver : GraphQLQueryResolver { + fun event(input: EventInput): String = "${input.date} ${input.opaque?.value}" + fun events(inputs: List): List = inputs.map { event(it) } + fun optionalEvent(input: Optional): String = input.map { event(it) }.orElse("empty") + fun eventNode(input: ObjectNode): String = (input["date"] as POJONode).pojo.let { "${it.javaClass.simpleName} $it" } + } + + class EventInput { + var date: LocalDate? = null + var opaque: Opaque? = null + var price: Money? = null + var id: UUID? = null + } + + class UploadQueryResolver : GraphQLQueryResolver { + fun upload(input: UploadInput): String = "${input.name}: ${(listOfNotNull(input.file) + input.files).joinToString(" ") { it.content() }}" + fun javaUpload(input: JavaUploadInput): String = input.file?.let { "${it.name}: ${it.content()}" }.orEmpty() + + private fun Part.content() = String(inputStream.readBytes()) + } + + data class UploadInput(val name: String, val file: Part?, val files: List = emptyList()) + + class JavaUploadInput { + var file: Part? = null + } + + class MapQueryResolver : GraphQLQueryResolver { + var input: Map? = null + + // the scalars are only found through the fields of input classes, not of Map inputs + fun event(input: EventInput): String = "${input.date} ${input.price}" + + fun eventMap(input: Map): Boolean { + this.input = input + return true + } + } + + class PaymentQueryResolver : GraphQLQueryResolver { + fun payment(input: PaymentInput): String = "${input.price} ${input.reference} ${input.location} ${input.date}" + fun paymentNode(input: ObjectNode): String = input.toString() + fun price(price: MoneyDto): String = price.toString() + } + + class PaymentInput { + var price: MoneyDto? = null + var reference: OpaqueDto? = null + + @field:JsonDeserialize(using = GeoPointDeserializer::class) + var location: GeoPoint? = null + var date: LocalDate? = null + } + + class DefaultedQueryResolver : GraphQLQueryResolver { + fun defaulted(input: DefaultedInput): String = "${input.name} ${input.date}" + } + + data class DefaultedInput(val name: String = "default", val date: LocalDate? = null) + + /** + * Has no creator Jackson could use, like the coerced values of many custom scalars. + */ + class Opaque private constructor(private val internalValue: String) { + val value get() = internalValue + + companion object { + fun of(value: String) = Opaque(value) + } + } + + data class OpaqueDto(val value: String) + + data class Money(val amount: Int, val currency: String) + + data class MoneyDto(val amount: Int, val currency: String) + + data class GeoPoint(val lat: Double, val lng: Double) + + // reads the serialized form, and an embedded GeoPoint wouldn't fail it + class GeoPointDeserializer : StdDeserializer(GeoPoint::class.java) { + override fun deserialize(p: JsonParser, context: DeserializationContext): GeoPoint { + val node = p.codec.readTree(p) + return GeoPoint(node.path("lat").asDouble(), node.path("lng").asDouble()) + } + } + + class NextDayDeserializer : StdDeserializer(LocalDate::class.java) { + override fun deserialize(p: JsonParser, context: DeserializationContext): LocalDate = LocalDate.parse(p.text).plusDays(1) + } + + // the workaround from https://github.com/graphql-java-kickstart/graphql-java-tools/issues/466 + class PartSerializer : StdSerializer(Part::class.java) { + override fun serialize(value: Part, gen: JsonGenerator, provider: SerializerProvider) { + gen.writeStartObject() + gen.writeStringField("name", value.name) + gen.writeBinaryField("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())) + } + } + + private val localDateScalar = scalar("LocalDate") { LocalDate.parse(it) } + private val opaqueScalar = scalar("Opaque") { Opaque.of(it) } + private val moneyScalar = scalar("Money") { Money(it.toInt(), "CAD") } + private val uuidScalar = scalar("UUID") { UUID.fromString(it) } + private val geoPointScalar = scalar("GeoPoint") { value -> value.split(",").map { it.toDouble() }.let { GeoPoint(it[0], it[1]) } } + + private fun scalar(name: String, parse: (String) -> T): GraphQLScalarType = GraphQLScalarType.newScalar() + .name(name) + .coercing(object : Coercing { + override fun serialize(dataFetcherResult: Any, context: GraphQLContext, locale: Locale) = dataFetcherResult.toString() + override fun parseValue(input: Any, context: GraphQLContext, locale: Locale): T = parse(input as String) + override fun parseLiteral(input: Value<*>, variables: CoercedVariables, context: GraphQLContext, locale: Locale): T = parse((input as StringValue).value!!) + }) + .build() +} From 7c3750454dc16d2511c8ad4eb61caaa3860c66d9 Mon Sep 17 00:00:00 2001 From: Oryan Date: Mon, 12 Oct 2026 16:05:33 -0400 Subject: [PATCH 2/5] Pass every coerced scalar value to Map arguments A Map, Object or other untyped argument kept a scalar value only if the ObjectMapper had no serializer of its own for it, so a UUID still arrived as a String, a Date as a Long, and a LocalDate as an array once the jsr310 module was registered. Nothing that reads an untyped value depends on its serialized form, so these now get every coerced value other than strings, numbers, booleans and enums, whatever modules the mapper has (#424). Only the classes whose values failed to convert to their own class are remembered now. Whether a value converts can depend on what it holds, so a value that converted no longer stops a later one of the same class from being embedded. Co-Authored-By: Claude Opus 5.5 --- .../tools/resolver/ArgumentConverter.kt | 31 ++-- .../kickstart/tools/CustomScalarInputTest.kt | 160 +++++++++++++++++- 2 files changed, 165 insertions(+), 26 deletions(-) diff --git a/src/main/kotlin/graphql/kickstart/tools/resolver/ArgumentConverter.kt b/src/main/kotlin/graphql/kickstart/tools/resolver/ArgumentConverter.kt index 318d2d48..c946c61a 100644 --- a/src/main/kotlin/graphql/kickstart/tools/resolver/ArgumentConverter.kt +++ b/src/main/kotlin/graphql/kickstart/tools/resolver/ArgumentConverter.kt @@ -6,7 +6,6 @@ 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.JsonMappingException import com.fasterxml.jackson.databind.JsonSerializable import com.fasterxml.jackson.databind.ObjectMapper import com.fasterxml.jackson.databind.SerializationFeature @@ -14,9 +13,6 @@ 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.ser.impl.UnknownSerializer -import com.fasterxml.jackson.databind.ser.impl.UnsupportedTypeSerializer -import com.fasterxml.jackson.databind.ser.std.BeanSerializerBase import com.fasterxml.jackson.databind.util.TokenBuffer import java.io.IOException import java.lang.reflect.Type @@ -29,12 +25,15 @@ import java.util.concurrent.ConcurrentHashMap * often don't survive, e.g. an upload, or a LocalDate without the jsr310 module. Those are handed to Jackson as embedded * objects instead, which its deserializers return as they are if they have the type being read. Where a String is read, * they're converted with toString(). + * + * An untyped parameter, e.g. a Map, gets the coerced values as they are, apart from strings, numbers, + * booleans and enums, which are converted as before. */ internal class ArgumentConverter(private val mapper: ObjectMapper, type: Type) { private val javaType = mapper.typeFactory.constructType(type) private val isUntyped = javaType.isUntyped() - private val convertsToOwnClassCache = ConcurrentHashMap, Boolean>() + private val nonConvertingClasses: MutableSet> = ConcurrentHashMap.newKeySet() fun convert(value: Any?): Any? { if (value == null) { @@ -42,9 +41,8 @@ internal class ArgumentConverter(private val mapper: ObjectMapper, type: Type) { } if (isUntyped) { - // nothing that reads an untyped value depends on its serialized form, so the scalar values are kept, - // unless the mapper has a serializer for them that defines that form - val embedded = embedValues(value) { !hasSerializerOfItsOwn(it) } + // nothing that reads an untyped value depends on its serialized form, so the coerced values are kept + val embedded = embedValues(value) { true } return if (embedded != null) readEmbedded(embedded) else mapper.convertValue(value, javaType) } @@ -77,22 +75,17 @@ internal class ArgumentConverter(private val mapper: ObjectMapper, type: Type) { return embed(value).takeIf { embedsAny } } - // a bean serializer writes the value's properties, which an untyped value would get as a Map, the other two fail - private fun hasSerializerOfItsOwn(value: Any): Boolean { - val serializer = try { - mapper.serializerProviderInstance.findValueSerializer(value.javaClass) - } catch (e: JsonMappingException) { + // failing to convert can be expensive, e.g. an upload, so those classes are remembered. Whether a value converts + // can depend on what it holds, so values of the other classes are checked every time + private fun convertsToOwnClass(value: Any): Boolean { + if (value.javaClass in nonConvertingClasses) { return false } - return serializer !is BeanSerializerBase && serializer !is UnknownSerializer && serializer !is UnsupportedTypeSerializer - } - - // the values of a scalar usually all convert or all don't, and converting one can be expensive, e.g. an upload - private fun convertsToOwnClass(value: Any): Boolean = convertsToOwnClassCache.getOrPut(value.javaClass) { - try { + return try { mapper.convertValue(value, value.javaClass) true } catch (e: IllegalArgumentException) { + nonConvertingClasses.add(value.javaClass) false } } diff --git a/src/test/kotlin/graphql/kickstart/tools/CustomScalarInputTest.kt b/src/test/kotlin/graphql/kickstart/tools/CustomScalarInputTest.kt index 7e16176c..5a90a5c4 100644 --- a/src/test/kotlin/graphql/kickstart/tools/CustomScalarInputTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/CustomScalarInputTest.kt @@ -8,6 +8,8 @@ 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 @@ -65,6 +67,7 @@ class CustomScalarInputTest { """ query(${'$'}input: EventInput!) { literal: event(input: { date: "2020-01-02", opaque: "literal" }) + opaqueOnly: event(input: { opaque: "alone" }) variable: event(input: ${'$'}input) events(inputs: [{ date: "2020-01-02" }, { opaque: "second" }]) optionalEvent(input: { date: "2020-01-02", opaque: "optional" }) @@ -75,6 +78,7 @@ class CustomScalarInputTest { assertEquals(data, mapOf( "literal" to "2020-01-02 literal", + "opaqueOnly" to "null alone", "variable" to "2021-03-04 variable", "events" to listOf("2020-01-02 null", "null second"), "optionalEvent" to "2020-01-02 optional", @@ -139,6 +143,9 @@ class CustomScalarInputTest { type Query { event(input: EventInput!): String! eventMap(input: EventMapInput!): Boolean! + rawMap(input: RawMapInput!): Boolean! + listMap(inputs: [ListMapInput!]!): Boolean! + optionalMap(input: OptionalMapInput): Boolean! } input EventInput { @@ -154,6 +161,18 @@ class CustomScalarInputTest { id: UUID next: EventMapInput } + + input RawMapInput { + price: Money + } + + input ListMapInput { + price: Money + } + + input OptionalMapInput { + price: Money + } """) .scalars(localDateScalar, moneyScalar, uuidScalar) .resolvers(resolver) @@ -181,18 +200,24 @@ class CustomScalarInputTest { id: "4dbd9a3d-4a26-4d10-9bb2-5b2dc4c8a0f0" next: { date: "2021-03-04" } }) + rawMap(input: { price: "6" }) + listMap(inputs: [{ price: "7" }, { price: "8" }]) + optionalMap(input: { price: "9" }) } """ } - // the ObjectMapper has a serializer for UUID, so that still arrives in its serialized form + // including those the ObjectMapper has a serializer for, e.g. UUID assertEquals(resolver.input, mapOf( "name" to null, "date" to LocalDate.of(2020, 1, 2), "price" to Money(5, "CAD"), - "id" to "4dbd9a3d-4a26-4d10-9bb2-5b2dc4c8a0f0", + "id" to UUID.fromString("4dbd9a3d-4a26-4d10-9bb2-5b2dc4c8a0f0"), "next" to mapOf("date" to LocalDate.of(2021, 3, 4)) )) + assertEquals(resolver.rawInput, mapOf("price" to Money(6, "CAD"))) + assertEquals(resolver.listInput, listOf(mapOf("price" to Money(7, "CAD")), mapOf("price" to Money(8, "CAD")))) + assertEquals(resolver.optionalInput, Optional.of(mapOf("price" to Money(9, "CAD")))) } @Test @@ -302,7 +327,8 @@ class CustomScalarInputTest { } assertEquals(data, mapOf("event" to "2020-01-03 null", "eventMap" to true, "javaUpload" to "copy of a.txt: Hello")) - assertEquals(resolver.input, mapOf("date" to "2020-01-02")) + // a Map gets the coerced value, not the serialized one, but the inclusion setting still applies + assertEquals(resolver.input, mapOf("date" to LocalDate.of(2020, 1, 2))) } @Test @@ -336,9 +362,89 @@ class CustomScalarInputTest { assertEquals(result.errors.size, 1) val exception = (result.errors[0] as ExceptionWhileDataFetching).exception - assert(exception.message.orEmpty().contains("JSON property name due to missing")) { exception.message.orEmpty() } - val conversionError = exception.suppressed.single() - assert(conversionError.message.orEmpty().startsWith("Java 8 date/time type `java.time.LocalDate` not supported")) { conversionError.message.orEmpty() } + assertEquals((exception.cause as MismatchedInputException).path.map { it.fieldName }, listOf("name")) + val conversionError = exception.suppressed.single().cause as InvalidDefinitionException + assertEquals(conversionError.type.rawClass, LocalDate::class.java) + } + + @Test + fun `custom scalars whose values only sometimes convert are passed to the resolver`() { + val schema = SchemaParser.newParser() + .schemaString( + """ + scalar LocalDate + scalar Box + + type Query { + box(input: BoxInput!): String! + } + + input BoxInput { + box: Box + date: LocalDate + } + """) + .scalars(localDateScalar, boxScalar) + .resolvers(BoxQueryResolver()) + .build() + .makeExecutableSchema() + val gql = GraphQL.newGraphQL(schema).build() + + // a box of text converts, so only the date is embedded, but a box of a date has to be embedded too + val text = assertNoGraphQlErrors(gql) { + """ + query { + box(input: { box: "text", date: "2020-01-02" }) + } + """ + } + val date = assertNoGraphQlErrors(gql) { + """ + query { + box(input: { box: "date:2021-03-04" }) + } + """ + } + + assertEquals(text, mapOf("box" to "text 2020-01-02")) + assertEquals(date, mapOf("box" to "2021-03-04 null")) + } + + @Test + fun `custom scalars read as a String are converted with toString`() { + val schema = SchemaParser.newParser() + .schemaString( + """ + scalar LocalDate + scalar Opaque + + type Query { + label(input: LabelInput!): String! + } + + input LabelInput { + date: LocalDate + opaque: Opaque + } + """) + .scalars(localDateScalar, opaqueScalar) + .resolvers(LabelQueryResolver()) + .build() + .makeExecutableSchema() + val gql = GraphQL.newGraphQL(schema).build() + + val data = assertNoGraphQlErrors(gql) { + """ + query { + date: label(input: { date: "2020-01-02" }) + opaque: label(input: { opaque: "secret" }) + } + """ + } + + // a LocalDate gives its ISO date, but a class that doesn't override toString() gives a meaningless one + assertEquals(data["date"], "2020-01-02 null") + assert((data["opaque"] as String).startsWith("null ${Opaque::class.java.name}@")) { data["opaque"].toString() } } class EventQueryResolver : GraphQLQueryResolver { @@ -370,6 +476,9 @@ class CustomScalarInputTest { class MapQueryResolver : GraphQLQueryResolver { var input: Map? = null + var rawInput: LinkedHashMap<*, *>? = null + var listInput: List>? = null + var optionalInput: Optional>? = null // the scalars are only found through the fields of input classes, not of Map inputs fun event(input: EventInput): String = "${input.date} ${input.price}" @@ -378,6 +487,22 @@ class CustomScalarInputTest { this.input = input return true } + + // the signature from https://github.com/graphql-java-kickstart/graphql-java-tools/issues/424 + fun rawMap(input: LinkedHashMap<*, *>): Boolean { + rawInput = input + return true + } + + fun listMap(inputs: List>): Boolean { + listInput = inputs + return true + } + + fun optionalMap(input: Optional>): Boolean { + optionalInput = input + return true + } } class PaymentQueryResolver : GraphQLQueryResolver { @@ -401,8 +526,28 @@ class CustomScalarInputTest { data class DefaultedInput(val name: String = "default", val date: LocalDate? = null) + class BoxQueryResolver : GraphQLQueryResolver { + fun box(input: BoxInput): String = "${input.box?.content} ${input.date}" + } + + class BoxInput { + var box: Box? = null + var date: LocalDate? = null + } + + data class Box(val content: Any) + + class LabelQueryResolver : GraphQLQueryResolver { + fun label(input: LabelInput): String = "${input.date} ${input.opaque}" + } + + class LabelInput { + var date: String? = null + var opaque: String? = null + } + /** - * Has no creator Jackson could use, like the coerced values of many custom scalars. + * Can't be rebuilt from its serialized properties, like the coerced values of many custom scalars. */ class Opaque private constructor(private val internalValue: String) { val value get() = internalValue @@ -454,6 +599,7 @@ class CustomScalarInputTest { private val moneyScalar = scalar("Money") { Money(it.toInt(), "CAD") } private val uuidScalar = scalar("UUID") { UUID.fromString(it) } private val geoPointScalar = scalar("GeoPoint") { value -> value.split(",").map { it.toDouble() }.let { GeoPoint(it[0], it[1]) } } + private val boxScalar = scalar("Box") { Box(if (it.startsWith("date:")) LocalDate.parse(it.removePrefix("date:")) else it) } private fun scalar(name: String, parse: (String) -> T): GraphQLScalarType = GraphQLScalarType.newScalar() .name(name) From f34a2953ad16f4a5e9a79637aa72c0d1592b1c35 Mon Sep 17 00:00:00 2001 From: Oryan Date: Sat, 17 Oct 2026 12:07:33 -0400 Subject: [PATCH 3/5] Simplify the argument converter's untyped path and names An untyped value is now always read through the buffer, which gives the same result as convertValue when there's nothing to embed. Co-Authored-By: Claude Opus 5.5 --- .../tools/resolver/ArgumentConverter.kt | 53 ++++++++++--------- 1 file changed, 27 insertions(+), 26 deletions(-) diff --git a/src/main/kotlin/graphql/kickstart/tools/resolver/ArgumentConverter.kt b/src/main/kotlin/graphql/kickstart/tools/resolver/ArgumentConverter.kt index c946c61a..f195f347 100644 --- a/src/main/kotlin/graphql/kickstart/tools/resolver/ArgumentConverter.kt +++ b/src/main/kotlin/graphql/kickstart/tools/resolver/ArgumentConverter.kt @@ -21,12 +21,12 @@ import java.util.concurrent.ConcurrentHashMap /** * Converts an argument value that graphql-java already coerced to the type of the resolver method's parameter. * - * Jackson converts a value by serializing it and deserializing it again, which the coerced values of custom scalars - * often don't survive, e.g. an upload, or a LocalDate without the jsr310 module. Those are handed to Jackson as embedded - * objects instead, which its deserializers return as they are if they have the type being read. Where a String is read, - * they're converted with toString(). + * 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. * - * An untyped parameter, e.g. a Map, gets the coerced values as they are, apart from strings, numbers, + * An untyped parameter, e.g. a Map, gets every coerced value as it is, apart from strings, numbers, * booleans and enums, which are converted as before. */ internal class ArgumentConverter(private val mapper: ObjectMapper, type: Type) { @@ -41,38 +41,38 @@ internal class ArgumentConverter(private val mapper: ObjectMapper, type: Type) { } if (isUntyped) { - // nothing that reads an untyped value depends on its serialized form, so the coerced values are kept - val embedded = embedValues(value) { true } - return if (embedded != null) readEmbedded(embedded) else mapper.convertValue(value, javaType) + // nothing reads an untyped value through its serialized form, so every coerced value is kept + return read(embed(value) { true } ?: value) } - // a deserializer may depend on the serialized form, so values are only embedded if converting fails + // a deserializer may read the serialized form, so values are only embedded if converting fails return try { mapper.convertValue(value, javaType) } catch (e: IllegalArgumentException) { - val embedded = embedValues(value) { !convertsToOwnClass(it) } ?: throw e + val embedded = embed(value) { !convertsToOwnClass(it) } ?: throw e try { - readEmbedded(embedded) - } catch (embeddedError: IllegalArgumentException) { - // the values Jackson couldn't convert likely hid the actual problem - throw embeddedError.apply { addSuppressed(e) } + read(embedded) + } catch (retryError: IllegalArgumentException) { + // the values that failed to convert likely hid the actual problem + throw retryError.apply { addSuppressed(e) } } } } /** - * Returns a copy of the value with the scalar values [isEmbedded] matches wrapped in [EmbeddedValue], or null if there are none. + * Returns a copy of the value in which the scalar values that [shouldEmbed] accepts are wrapped in [EmbeddedValue], + * or null if it accepts none. */ - private fun embedValues(value: Any, isEmbedded: (Any) -> Boolean): Any? { + private fun embed(value: Any, shouldEmbed: (Any) -> Boolean): Any? { var embedsAny = false - fun embed(value: Any?): Any? = when (value) { + fun copy(value: Any?): Any? = when (value) { null, is String, is Number, is Boolean, is Enum<*> -> value - is Map<*, *> -> value.mapValuesTo(LinkedHashMap()) { embed(it.value) } - is Collection<*> -> value.map { embed(it) } - else -> if (isEmbedded(value)) EmbeddedValue(value).also { embedsAny = true } else value + is Map<*, *> -> value.mapValuesTo(LinkedHashMap()) { copy(it.value) } + is Collection<*> -> value.map { copy(it) } + else -> if (shouldEmbed(value)) EmbeddedValue(value).also { embedsAny = true } else value } - return embed(value).takeIf { embedsAny } + return copy(value).takeIf { embedsAny } } // failing to convert can be expensive, e.g. an upload, so those classes are remembered. Whether a value converts @@ -85,13 +85,13 @@ internal class ArgumentConverter(private val mapper: ObjectMapper, type: Type) { mapper.convertValue(value, value.javaClass) true } catch (e: IllegalArgumentException) { - nonConvertingClasses.add(value.javaClass) + nonConvertingClasses += value.javaClass false } } - // like ObjectMapper.convertValue, but reads embedded values - private fun readEmbedded(value: Any): Any? { + // 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 { mapper.writer().without(SerializationFeature.WRAP_ROOT_VALUE).writeValue(buffer, value) @@ -105,7 +105,7 @@ internal class ArgumentConverter(private val mapper: ObjectMapper, type: Type) { } private val embeddedValueHandler = object : DeserializationProblemHandler() { - // an interface or abstract class, e.g. Part, has no deserializer that returns embedded values + // called for an interface or abstract class, e.g. Part, since no deserializer of it returns embedded values override fun handleMissingInstantiator( context: DeserializationContext, instClass: Class<*>, @@ -117,11 +117,12 @@ internal class ArgumentConverter(private val mapper: ObjectMapper, type: Type) { return if (instClass.isInstance(value)) value else NOT_HANDLED } - // a bean of another class, which the value is converted to on its own + // 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) } + // written to the buffer as an embedded object, which it keeps as it is private class EmbeddedValue(private val value: Any) : JsonSerializable { override fun serialize(gen: JsonGenerator, serializers: SerializerProvider) = gen.writeEmbeddedObject(value) From 19a2f21f719c1fe274a9a52b5fba4c53b5f58280 Mon Sep 17 00:00:00 2001 From: Oryan Date: Sat, 17 Oct 2026 12:07:36 -0400 Subject: [PATCH 4/5] Fold the custom scalar input tests and use the scalars only in Map inputs Since #842, a scalar used only in a Map input is registered, so the Map test no longer needs another input to reach its scalars. The typed cases share one schema and resolver, and the UUID scalar comes from the end-to-end fixtures. Co-Authored-By: Claude Opus 5.5 --- .../kickstart/tools/CustomScalarInputTest.kt | 623 ++++++------------ 1 file changed, 192 insertions(+), 431 deletions(-) diff --git a/src/test/kotlin/graphql/kickstart/tools/CustomScalarInputTest.kt b/src/test/kotlin/graphql/kickstart/tools/CustomScalarInputTest.kt index 5a90a5c4..1b8851b3 100644 --- a/src/test/kotlin/graphql/kickstart/tools/CustomScalarInputTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/CustomScalarInputTest.kt @@ -16,7 +16,6 @@ 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.ExecutionInput import graphql.GraphQL import graphql.GraphQLContext import graphql.execution.CoercedVariables @@ -24,493 +23,285 @@ import graphql.language.StringValue import graphql.language.Value import graphql.schema.Coercing import graphql.schema.GraphQLScalarType -import kotlinx.coroutines.ExperimentalCoroutinesApi import org.junit.Test import java.time.LocalDate import java.util.* import javax.servlet.http.Part -@OptIn(ExperimentalCoroutinesApi::class) class CustomScalarInputTest { + private val resolver = QueryResolver() + @Test fun `custom scalars in input objects are passed to the resolver`() { - val schema = SchemaParser.newParser() - .schemaString( - """ - scalar LocalDate - scalar Opaque - - type Query { - event(input: EventInput!): String! - events(inputs: [EventInput!]!): [String!]! - optionalEvent(input: EventInput): String! - eventNode(input: EventNodeInput!): String! - } - - input EventInput { - date: LocalDate - opaque: Opaque - } - - input EventNodeInput { - date: LocalDate - } - """) - .scalars(localDateScalar, opaqueScalar) - .resolvers(EventQueryResolver()) - .build() - .makeExecutableSchema() - val gql = GraphQL.newGraphQL(schema).build() - - val data = assertNoGraphQlErrors(gql, mapOf("input" to mapOf("date" to "2021-03-04", "opaque" to "variable"))) { + val variables = mapOf( + "event" to mapOf("date" to "2021-03-04", "opaque" to "variable"), + "file" to MockPart("a.txt", "Hello"), + "files" to listOf(MockPart("b.txt", "big"), MockPart("c.txt", "world")) + ) + val data = assertNoGraphQlErrors(graphQL(inputSchema), variables) { """ - query(${'$'}input: EventInput!) { + query(${'$'}event: EventInput!, ${'$'}file: Upload, ${'$'}files: [Upload!]) { literal: event(input: { date: "2020-01-02", opaque: "literal" }) opaqueOnly: event(input: { opaque: "alone" }) - variable: event(input: ${'$'}input) + variable: event(input: ${'$'}event) + file: event(input: { file: ${'$'}file }) + boxOfText: event(input: { box: "text", date: "2020-01-02" }) + boxOfDate: event(input: { box: "date:2021-03-04" }) events(inputs: [{ date: "2020-01-02" }, { opaque: "second" }]) optionalEvent(input: { date: "2020-01-02", opaque: "optional" }) - eventNode(input: { date: "2020-01-02" }) + message(input: { name: "greeting", file: ${'$'}file, files: ${'$'}files }) + dateNode(input: { date: "2020-01-02" }) + dateLabel: label(input: { date: "2020-01-02" }) + opaqueLabel: label(input: { opaque: "secret" }) } """ } - assertEquals(data, mapOf( + // a box of text converts, so only the date is embedded, but the box of a date after it has to be embedded too + assertEquals(data - "opaqueLabel", mapOf( "literal" to "2020-01-02 literal", - "opaqueOnly" to "null alone", + "opaqueOnly" to "alone", "variable" to "2021-03-04 variable", - "events" to listOf("2020-01-02 null", "null second"), + "file" to "a.txt: Hello", + "boxOfText" to "2020-01-02 text", + "boxOfDate" to "2021-03-04", + "events" to listOf("2020-01-02", "second"), "optionalEvent" to "2020-01-02 optional", - "eventNode" to "LocalDate 2020-01-02" + "message" to "greeting: Hello big world", + "dateNode" to "LocalDate 2020-01-02", + "dateLabel" to "2020-01-02 null" )) + // a String gets toString(), which is meaningless for a class that doesn't override it + assert((data["opaqueLabel"] as String).startsWith("null ${Opaque::class.java.name}@")) { data["opaqueLabel"].toString() } } @Test - fun `uploads in input objects are passed to the resolver`() { - val schema = SchemaParser.newParser() - .schemaString( - """ - scalar Upload - - type Query { - upload(input: UploadInput!): String! - javaUpload(input: JavaUploadInput!): String! - } - - input UploadInput { - name: String! - file: Upload - files: [Upload!] - } - - input JavaUploadInput { - file: Upload - } - """) - .scalars(uploadScalar) - .resolvers(UploadQueryResolver()) - .build() - .makeExecutableSchema() - val gql = GraphQL.newGraphQL(schema).build() + fun `custom scalars in Map arguments are passed as coerced values`() { + val schema = """ + scalar LocalDate + scalar Money + scalar UUID + + type Query { + map(input: MapInput!): Boolean! + mapList(inputs: [MapInput!]!): Boolean! + optionalMap(input: OptionalMapInput): Boolean! + rawMap(input: RawMapInput!): Boolean! + } - val files = mapOf("file" to MockPart("a.txt", "Hello"), "files" to listOf(MockPart("b.txt", "big"), MockPart("c.txt", "world"))) - val data = assertNoGraphQlErrors(gql, files) { - """ - query(${'$'}file: Upload, ${'$'}files: [Upload!]) { - upload(input: { name: "greeting", file: ${'$'}file, files: ${'$'}files }) - javaUpload(input: { file: ${'$'}file }) + input MapInput { + name: String + date: LocalDate + price: Money + id: UUID + next: MapInput } - """ - } - assertEquals(data, mapOf( - "upload" to "greeting: Hello big world", - "javaUpload" to "a.txt: Hello" - )) - } + input OptionalMapInput { + price: Money + } - @Test - fun `custom scalars in Map arguments are passed as coerced values`() { - val resolver = MapQueryResolver() - val schema = SchemaParser.newParser() - .schemaString( - """ - scalar LocalDate - scalar Money - scalar UUID - - type Query { - event(input: EventInput!): String! - eventMap(input: EventMapInput!): Boolean! - rawMap(input: RawMapInput!): Boolean! - listMap(inputs: [ListMapInput!]!): Boolean! - optionalMap(input: OptionalMapInput): Boolean! - } - - input EventInput { - date: LocalDate - price: Money - id: UUID - } - - input EventMapInput { - name: String - date: LocalDate - price: Money - id: UUID - next: EventMapInput - } - - input RawMapInput { - price: Money - } - - input ListMapInput { - price: Money - } - - input OptionalMapInput { - price: Money - } - """) - .scalars(localDateScalar, moneyScalar, uuidScalar) - .resolvers(resolver) - .build() - .makeExecutableSchema() - val gql = GraphQL.newGraphQL(schema).build() + input RawMapInput { + price: Money + } + """ - assertNoGraphQlErrors(gql) { + assertNoGraphQlErrors(graphQL(schema)) { """ query { - eventMap(input: { price: "5" }) + map(input: { name: null, date: "2020-01-02", price: "5", id: "4dbd9a3d-4a26-4d10-9bb2-5b2dc4c8a0f0", next: { date: "2021-03-04" } }) + mapList(inputs: [{ price: "6" }, { price: "7" }]) + optionalMap(input: { price: "8" }) + rawMap(input: { price: "9" }) } """ } - assertEquals(resolver.input, mapOf("price" to Money(5, "CAD"))) + // including those the ObjectMapper has a serializer for, e.g. UUID + assertEquals(resolver.received, mapOf( + "map" to mapOf( + "name" to null, + "date" to LocalDate.of(2020, 1, 2), + "price" to Money(5, "CAD"), + "id" to UUID.fromString("4dbd9a3d-4a26-4d10-9bb2-5b2dc4c8a0f0"), + "next" to mapOf("date" to LocalDate.of(2021, 3, 4)) + ), + "mapList" to listOf(mapOf("price" to Money(6, "CAD")), mapOf("price" to Money(7, "CAD"))), + "optionalMap" to Optional.of(mapOf("price" to Money(8, "CAD"))), + "rawMap" to mapOf("price" to Money(9, "CAD")) + )) - assertNoGraphQlErrors(gql) { + assertNoGraphQlErrors(graphQL(schema, configuredMapperOptions)) { """ query { - eventMap(input: { - name: null - date: "2020-01-02" - price: "5" - id: "4dbd9a3d-4a26-4d10-9bb2-5b2dc4c8a0f0" - next: { date: "2021-03-04" } - }) - rawMap(input: { price: "6" }) - listMap(inputs: [{ price: "7" }, { price: "8" }]) - optionalMap(input: { price: "9" }) + map(input: { name: null, date: "2020-01-02" }) } """ } - // including those the ObjectMapper has a serializer for, e.g. UUID - assertEquals(resolver.input, mapOf( - "name" to null, - "date" to LocalDate.of(2020, 1, 2), - "price" to Money(5, "CAD"), - "id" to UUID.fromString("4dbd9a3d-4a26-4d10-9bb2-5b2dc4c8a0f0"), - "next" to mapOf("date" to LocalDate.of(2021, 3, 4)) - )) - assertEquals(resolver.rawInput, mapOf("price" to Money(6, "CAD"))) - assertEquals(resolver.listInput, listOf(mapOf("price" to Money(7, "CAD")), mapOf("price" to Money(8, "CAD")))) - assertEquals(resolver.optionalInput, Optional.of(mapOf("price" to Money(9, "CAD")))) + // the configured (de)serializers aren't used, but the inclusion setting is + assertEquals(resolver.received["map"], mapOf("date" to LocalDate.of(2020, 1, 2))) } @Test fun `custom scalars the ObjectMapper can convert are still converted by it`() { - val schema = SchemaParser.newParser() - .schemaString( - """ - scalar LocalDate - scalar Money - scalar Opaque - scalar GeoPoint - - type Query { - payment(input: PaymentInput!): String! - paymentNode(input: PaymentNodeInput!): String! - price(price: Money!): String! - } - - input PaymentInput { - price: Money - reference: Opaque - location: GeoPoint - date: LocalDate - } - - input PaymentNodeInput { - price: Money - } - """) - .scalars(localDateScalar, moneyScalar, opaqueScalar, geoPointScalar) - .resolvers(PaymentQueryResolver()) - .build() - .makeExecutableSchema() - val gql = GraphQL.newGraphQL(schema).build() - - val data = assertNoGraphQlErrors(gql) { + val data = assertNoGraphQlErrors(graphQL(inputSchema)) { """ query { payment(input: { price: "5", reference: "ref", location: "1.5,2.5" }) withDate: payment(input: { price: "5", reference: "ref", location: "1.5,2.5", date: "2020-01-02" }) - paymentNode(input: { price: "5" }) + priceNode(input: { price: "5" }) price(price: "5") } """ } - - assertEquals(data, mapOf( - "payment" to "MoneyDto(amount=5, currency=CAD) OpaqueDto(value=ref) GeoPoint(lat=1.5, lng=2.5) null", - "withDate" to "MoneyDto(amount=5, currency=CAD) OpaqueDto(value=ref) GeoPoint(lat=1.5, lng=2.5) 2020-01-02", - "paymentNode" to """{"price":{"amount":5,"currency":"CAD"}}""", - "price" to "MoneyDto(amount=5, currency=CAD)" - )) - } - - @Test - fun `custom scalars in inputs are converted by a configured ObjectMapper`() { - val resolver = MapQueryResolver() - val schema = SchemaParser.newParser() - .schemaString( - """ - scalar LocalDate - scalar Upload - - type Query { - event(input: EventInput!): String! - eventMap(input: EventMapInput!): Boolean! - javaUpload(input: JavaUploadInput!): String! - } - - input EventInput { - date: LocalDate - } - - input EventMapInput { - name: String - date: LocalDate - } - - input JavaUploadInput { - file: Upload - } - """) - .scalars(localDateScalar, uploadScalar) - .resolvers(resolver, UploadQueryResolver()) - .options(SchemaParserOptions.newOptions() - .objectMapperConfigurer { mapper, _ -> - mapper.registerModule(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) - } - .build()) - .build() - .makeExecutableSchema() - val gql = GraphQL.newGraphQL(schema).build() - - val data = assertNoGraphQlErrors(gql, mapOf("file" to MockPart("a.txt", "Hello"))) { + val configured = assertNoGraphQlErrors(graphQL(inputSchema, configuredMapperOptions), mapOf("file" to MockPart("a.txt", "Hello"))) { """ query(${'$'}file: Upload) { - event(input: { date: "2020-01-02" }) - eventMap(input: { name: null, date: "2020-01-02" }) - javaUpload(input: { file: ${'$'}file }) + event(input: { date: "2020-01-02", file: ${'$'}file }) } """ } - assertEquals(data, mapOf("event" to "2020-01-03 null", "eventMap" to true, "javaUpload" to "copy of a.txt: Hello")) - // a Map gets the coerced value, not the serialized one, but the inclusion setting still applies - assertEquals(resolver.input, mapOf("date" to LocalDate.of(2020, 1, 2))) + assertEquals(data, mapOf( + "payment" to "MoneyDto(amount=5, currency=CAD) OpaqueDto(value=ref) GeoPoint(lat=1.5, lng=2.5) null", + "withDate" to "MoneyDto(amount=5, currency=CAD) OpaqueDto(value=ref) GeoPoint(lat=1.5, lng=2.5) 2020-01-02", + "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 `errors converting inputs with custom scalars name the actual problem`() { - val schema = SchemaParser.newParser() - .schemaString( - """ - scalar LocalDate - - type Query { - defaulted(input: DefaultedInput!): String! - } - - input DefaultedInput { - name: String - date: LocalDate - } - """) - .scalars(localDateScalar) - .resolvers(DefaultedQueryResolver()) - .build() - .makeExecutableSchema() - val gql = GraphQL.newGraphQL(schema).build() - - val result = gql.execute(ExecutionInput.newExecutionInput().query( + val result = graphQL(inputSchema).execute( """ query { - defaulted(input: { name: null, date: "2020-01-02" }) + message(input: { name: null, date: "2020-01-02" }) } - """)) + """) - assertEquals(result.errors.size, 1) - val exception = (result.errors[0] as ExceptionWhileDataFetching).exception + val exception = (result.errors.single() as ExceptionWhileDataFetching).exception assertEquals((exception.cause as MismatchedInputException).path.map { it.fieldName }, listOf("name")) - val conversionError = exception.suppressed.single().cause as InvalidDefinitionException - assertEquals(conversionError.type.rawClass, LocalDate::class.java) + assertEquals((exception.suppressed.single().cause as InvalidDefinitionException).type.rawClass, LocalDate::class.java) } - @Test - fun `custom scalars whose values only sometimes convert are passed to the resolver`() { - val schema = SchemaParser.newParser() - .schemaString( - """ - scalar LocalDate - scalar Box - - type Query { - box(input: BoxInput!): String! - } - - input BoxInput { - box: Box - date: LocalDate - } - """) - .scalars(localDateScalar, boxScalar) - .resolvers(BoxQueryResolver()) + private fun graphQL(schema: String, options: SchemaParserOptions = SchemaParserOptions.defaultOptions()): GraphQL { + val graphQLSchema = SchemaParser.newParser() + .schemaString(schema) + .scalars(localDateScalar, opaqueScalar, boxScalar, moneyScalar, geoPointScalar, customScalarUUID, uploadScalar) + .resolvers(resolver) + .options(options) .build() .makeExecutableSchema() - val gql = GraphQL.newGraphQL(schema).build() - - // a box of text converts, so only the date is embedded, but a box of a date has to be embedded too - val text = assertNoGraphQlErrors(gql) { - """ - query { - box(input: { box: "text", date: "2020-01-02" }) - } - """ - } - val date = assertNoGraphQlErrors(gql) { - """ - query { - box(input: { box: "date:2021-03-04" }) - } - """ - } - - assertEquals(text, mapOf("box" to "text 2020-01-02")) - assertEquals(date, mapOf("box" to "2021-03-04 null")) + return GraphQL.newGraphQL(graphQLSchema).build() } - @Test - fun `custom scalars read as a String are converted with toString`() { - val schema = SchemaParser.newParser() - .schemaString( - """ - scalar LocalDate - scalar Opaque - - type Query { - label(input: LabelInput!): String! - } - - input LabelInput { - date: LocalDate - opaque: Opaque - } - """) - .scalars(localDateScalar, opaqueScalar) - .resolvers(LabelQueryResolver()) - .build() - .makeExecutableSchema() - val gql = GraphQL.newGraphQL(schema).build() - - val data = assertNoGraphQlErrors(gql) { - """ - query { - date: label(input: { date: "2020-01-02" }) - opaque: label(input: { opaque: "secret" }) - } - """ + private val inputSchema = """ + scalar LocalDate + scalar Opaque + scalar Box + scalar Money + scalar GeoPoint + scalar Upload + + type Query { + event(input: EventInput!): String! + events(inputs: [EventInput!]!): [String!]! + optionalEvent(input: EventInput): String! + message(input: MessageInput!): String! + label(input: LabelInput!): String! + dateNode(input: NodeInput!): String! + priceNode(input: NodeInput!): String! + payment(input: PaymentInput!): String! + price(price: Money!): String! } - // a LocalDate gives its ISO date, but a class that doesn't override toString() gives a meaningless one - assertEquals(data["date"], "2020-01-02 null") - assert((data["opaque"] as String).startsWith("null ${Opaque::class.java.name}@")) { data["opaque"].toString() } - } - - class EventQueryResolver : GraphQLQueryResolver { - fun event(input: EventInput): String = "${input.date} ${input.opaque?.value}" - fun events(inputs: List): List = inputs.map { event(it) } - fun optionalEvent(input: Optional): String = input.map { event(it) }.orElse("empty") - fun eventNode(input: ObjectNode): String = (input["date"] as POJONode).pojo.let { "${it.javaClass.simpleName} $it" } - } + input EventInput { + date: LocalDate + opaque: Opaque + box: Box + file: Upload + } - class EventInput { - var date: LocalDate? = null - var opaque: Opaque? = null - var price: Money? = null - var id: UUID? = null - } + input MessageInput { + name: String + date: LocalDate + file: Upload + files: [Upload!] + } - class UploadQueryResolver : GraphQLQueryResolver { - fun upload(input: UploadInput): String = "${input.name}: ${(listOfNotNull(input.file) + input.files).joinToString(" ") { it.content() }}" - fun javaUpload(input: JavaUploadInput): String = input.file?.let { "${it.name}: ${it.content()}" }.orEmpty() + input LabelInput { + date: LocalDate + opaque: Opaque + } - private fun Part.content() = String(inputStream.readBytes()) - } + input NodeInput { + date: LocalDate + price: Money + } - data class UploadInput(val name: String, val file: Part?, val files: List = emptyList()) + input PaymentInput { + price: Money + reference: Opaque + location: GeoPoint + date: LocalDate + } + """ + + private val configuredMapperOptions = SchemaParserOptions.newOptions() + .objectMapperConfigurer { mapper, _ -> + mapper.registerModule(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) + } + .build() - class JavaUploadInput { - var file: Part? = null - } + class QueryResolver : GraphQLQueryResolver { + val received = mutableMapOf() - class MapQueryResolver : GraphQLQueryResolver { - var input: Map? = null - var rawInput: LinkedHashMap<*, *>? = null - var listInput: List>? = null - var optionalInput: Optional>? = null + fun event(input: EventInput): String = + listOfNotNull(input.date, input.opaque?.value, input.box?.content, input.file?.let { "${it.name}: ${it.content()}" }).joinToString(" ") - // the scalars are only found through the fields of input classes, not of Map inputs - fun event(input: EventInput): String = "${input.date} ${input.price}" + fun events(inputs: List): List = inputs.map { event(it) } + 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" } + fun priceNode(input: ObjectNode): String = input.toString() + fun payment(input: PaymentInput): String = "${input.price} ${input.reference} ${input.location} ${input.date}" + fun price(price: MoneyDto): String = price.toString() - fun eventMap(input: Map): Boolean { - this.input = input - return true - } + fun map(input: Map) = receive("map", input) + fun mapList(inputs: List>) = receive("mapList", inputs) + fun optionalMap(input: Optional>) = receive("optionalMap", input) // the signature from https://github.com/graphql-java-kickstart/graphql-java-tools/issues/424 - fun rawMap(input: LinkedHashMap<*, *>): Boolean { - rawInput = input - return true - } + fun rawMap(input: LinkedHashMap<*, *>) = receive("rawMap", input) - fun listMap(inputs: List>): Boolean { - listInput = inputs + private fun receive(field: String, input: Any): Boolean { + received[field] = input return true } - fun optionalMap(input: Optional>): Boolean { - optionalInput = input - return true - } + private fun Part.content() = String(inputStream.readBytes()) } - class PaymentQueryResolver : GraphQLQueryResolver { - fun payment(input: PaymentInput): String = "${input.price} ${input.reference} ${input.location} ${input.date}" - fun paymentNode(input: ObjectNode): String = input.toString() - fun price(price: MoneyDto): String = price.toString() + class EventInput { + var date: LocalDate? = null + var opaque: Opaque? = null + var box: Box? = null + var file: Part? = null } + data class MessageInput(val name: String, val date: LocalDate? = 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 @@ -520,41 +311,9 @@ class CustomScalarInputTest { var date: LocalDate? = null } - class DefaultedQueryResolver : GraphQLQueryResolver { - fun defaulted(input: DefaultedInput): String = "${input.name} ${input.date}" - } - - data class DefaultedInput(val name: String = "default", val date: LocalDate? = null) - - class BoxQueryResolver : GraphQLQueryResolver { - fun box(input: BoxInput): String = "${input.box?.content} ${input.date}" - } - - class BoxInput { - var box: Box? = null - var date: LocalDate? = null - } - - data class Box(val content: Any) - - class LabelQueryResolver : GraphQLQueryResolver { - fun label(input: LabelInput): String = "${input.date} ${input.opaque}" - } - - class LabelInput { - var date: String? = null - var opaque: String? = null - } - - /** - * Can't be rebuilt from its serialized properties, like the coerced values of many custom scalars. - */ - class Opaque private constructor(private val internalValue: String) { - val value get() = internalValue - - companion object { - fun of(value: String) = Opaque(value) - } + // serialized as {"value": ...}, which Jackson can't create it from, like the coerced values of many custom scalars + class Opaque(text: String) { + val value = text } data class OpaqueDto(val value: String) @@ -563,6 +322,8 @@ class CustomScalarInputTest { data class MoneyDto(val amount: Int, val currency: String) + data class Box(val content: Any) + data class GeoPoint(val lat: Double, val lng: Double) // reads the serialized form, and an embedded GeoPoint wouldn't fail it @@ -595,18 +356,18 @@ class CustomScalarInputTest { } private val localDateScalar = scalar("LocalDate") { LocalDate.parse(it) } - private val opaqueScalar = scalar("Opaque") { Opaque.of(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 moneyScalar = scalar("Money") { Money(it.toInt(), "CAD") } - private val uuidScalar = scalar("UUID") { UUID.fromString(it) } private val geoPointScalar = scalar("GeoPoint") { value -> value.split(",").map { it.toDouble() }.let { GeoPoint(it[0], it[1]) } } - private val boxScalar = scalar("Box") { Box(if (it.startsWith("date:")) LocalDate.parse(it.removePrefix("date:")) else it) } private fun scalar(name: String, parse: (String) -> T): GraphQLScalarType = GraphQLScalarType.newScalar() .name(name) .coercing(object : Coercing { override fun serialize(dataFetcherResult: Any, context: GraphQLContext, locale: Locale) = dataFetcherResult.toString() override fun parseValue(input: Any, context: GraphQLContext, locale: Locale): T = parse(input as String) - override fun parseLiteral(input: Value<*>, variables: CoercedVariables, context: GraphQLContext, locale: Locale): T = parse((input as StringValue).value!!) + override fun parseLiteral(input: Value<*>, variables: CoercedVariables, context: GraphQLContext, locale: Locale): T = + parse(checkNotNull((input as StringValue).value)) }) .build() } From 2095a2f1a8399c898c1781d1875a4ba80ffdedb5 Mon Sep 17 00:00:00 2001 From: Oryan Date: Sat, 17 Oct 2026 12:29:13 -0400 Subject: [PATCH 5/5] Only remember non-converting classes within one conversion The classes were remembered per argument, so after one request with a value that couldn't be converted, later values of the same class were embedded even when they converted. A deserializer that reads the serialized form then got nothing, and the same request gave a different result. The memo now lasts one convert call. The guard test checks the same request before and after such a value, with a tree-reading deserializer in place of the GeoPoint one. The writer is built once and the reader on first use, instead of on every read. Co-Authored-By: Claude Opus 5.5 --- .../tools/resolver/ArgumentConverter.kt | 51 +++++++++++-------- .../kickstart/tools/CustomScalarInputTest.kt | 50 ++++++++---------- 2 files changed, 52 insertions(+), 49 deletions(-) diff --git a/src/main/kotlin/graphql/kickstart/tools/resolver/ArgumentConverter.kt b/src/main/kotlin/graphql/kickstart/tools/resolver/ArgumentConverter.kt index f195f347..6c55b85f 100644 --- a/src/main/kotlin/graphql/kickstart/tools/resolver/ArgumentConverter.kt +++ b/src/main/kotlin/graphql/kickstart/tools/resolver/ArgumentConverter.kt @@ -16,7 +16,6 @@ import com.fasterxml.jackson.databind.jsontype.TypeSerializer import com.fasterxml.jackson.databind.util.TokenBuffer import java.io.IOException import java.lang.reflect.Type -import java.util.concurrent.ConcurrentHashMap /** * Converts an argument value that graphql-java already coerced to the type of the resolver method's parameter. @@ -33,7 +32,6 @@ internal class ArgumentConverter(private val mapper: ObjectMapper, type: Type) { private val javaType = mapper.typeFactory.constructType(type) private val isUntyped = javaType.isUntyped() - private val nonConvertingClasses: MutableSet> = ConcurrentHashMap.newKeySet() fun convert(value: Any?): Any? { if (value == null) { @@ -49,7 +47,19 @@ internal class ArgumentConverter(private val mapper: ObjectMapper, type: Type) { return try { mapper.convertValue(value, javaType) } catch (e: IllegalArgumentException) { - val embedded = embed(value) { !convertsToOwnClass(it) } ?: throw e + // a class whose value failed to convert isn't tried again in this value, since that can be expensive, e.g. for + // a list of uploads. It isn't remembered across calls, so the same input always gives the same result + val nonConvertingClasses = HashSet>() + val embedded = embed(value) { scalar -> + when { + scalar.javaClass in nonConvertingClasses -> true + convertsToOwnClass(scalar) -> false + else -> { + nonConvertingClasses += scalar.javaClass + true + } + } + } ?: throw e try { read(embedded) } catch (retryError: IllegalArgumentException) { @@ -75,30 +85,19 @@ internal class ArgumentConverter(private val mapper: ObjectMapper, type: Type) { return copy(value).takeIf { embedsAny } } - // failing to convert can be expensive, e.g. an upload, so those classes are remembered. Whether a value converts - // can depend on what it holds, so values of the other classes are checked every time - private fun convertsToOwnClass(value: Any): Boolean { - if (value.javaClass in nonConvertingClasses) { - return false - } - return try { - mapper.convertValue(value, value.javaClass) - true - } catch (e: IllegalArgumentException) { - nonConvertingClasses += value.javaClass - false - } + private fun convertsToOwnClass(value: Any): Boolean = try { + mapper.convertValue(value, value.javaClass) + true + } catch (e: IllegalArgumentException) { + 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 { - mapper.writer().without(SerializationFeature.WRAP_ROOT_VALUE).writeValue(buffer, value) - mapper.readerFor(javaType) - .without(DeserializationFeature.UNWRAP_ROOT_VALUE) - .withHandler(embeddedValueHandler) - .readValue(buffer.asParser()) + writer.writeValue(buffer, value) + reader.readValue(buffer.asParser()) } catch (e: IOException) { throw IllegalArgumentException(e.message, e) } @@ -122,6 +121,16 @@ internal class ArgumentConverter(private val mapper: ObjectMapper, type: Type) { mapper.convertValue(valueToConvert, targetType) } + private val writer = mapper.writer().without(SerializationFeature.WRAP_ROOT_VALUE) + + // 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) + } + // written to the buffer as an embedded object, which it keeps as it is private class EmbeddedValue(private val value: Any) : JsonSerializable { override fun serialize(gen: JsonGenerator, serializers: SerializerProvider) = gen.writeEmbeddedObject(value) diff --git a/src/test/kotlin/graphql/kickstart/tools/CustomScalarInputTest.kt b/src/test/kotlin/graphql/kickstart/tools/CustomScalarInputTest.kt index 1b8851b3..ffb8f9cc 100644 --- a/src/test/kotlin/graphql/kickstart/tools/CustomScalarInputTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/CustomScalarInputTest.kt @@ -46,8 +46,6 @@ class CustomScalarInputTest { opaqueOnly: event(input: { opaque: "alone" }) variable: event(input: ${'$'}event) file: event(input: { file: ${'$'}file }) - boxOfText: event(input: { box: "text", date: "2020-01-02" }) - boxOfDate: event(input: { box: "date:2021-03-04" }) events(inputs: [{ date: "2020-01-02" }, { opaque: "second" }]) optionalEvent(input: { date: "2020-01-02", opaque: "optional" }) message(input: { name: "greeting", file: ${'$'}file, files: ${'$'}files }) @@ -58,14 +56,11 @@ class CustomScalarInputTest { """ } - // a box of text converts, so only the date is embedded, but the box of a date after it has to be embedded too assertEquals(data - "opaqueLabel", mapOf( "literal" to "2020-01-02 literal", "opaqueOnly" to "alone", "variable" to "2021-03-04 variable", "file" to "a.txt: Hello", - "boxOfText" to "2020-01-02 text", - "boxOfDate" to "2021-03-04", "events" to listOf("2020-01-02", "second"), "optionalEvent" to "2020-01-02 optional", "message" to "greeting: Hello big world", @@ -149,8 +144,10 @@ class CustomScalarInputTest { val data = assertNoGraphQlErrors(graphQL(inputSchema)) { """ query { - payment(input: { price: "5", reference: "ref", location: "1.5,2.5" }) - withDate: payment(input: { price: "5", reference: "ref", location: "1.5,2.5", date: "2020-01-02" }) + 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" }) priceNode(input: { price: "5" }) price(price: "5") } @@ -164,9 +161,13 @@ class CustomScalarInputTest { """ } + // a box of a date can't be converted, so the deserializer gets it embedded and has no serialized form to read. + // That doesn't change how a later box of text is read assertEquals(data, mapOf( - "payment" to "MoneyDto(amount=5, currency=CAD) OpaqueDto(value=ref) GeoPoint(lat=1.5, lng=2.5) null", - "withDate" to "MoneyDto(amount=5, currency=CAD) OpaqueDto(value=ref) GeoPoint(lat=1.5, lng=2.5) 2020-01-02", + "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", "priceNode" to """{"price":{"amount":5,"currency":"CAD"}}""", "price" to "MoneyDto(amount=5, currency=CAD)" )) @@ -190,7 +191,7 @@ class CustomScalarInputTest { private fun graphQL(schema: String, options: SchemaParserOptions = SchemaParserOptions.defaultOptions()): GraphQL { val graphQLSchema = SchemaParser.newParser() .schemaString(schema) - .scalars(localDateScalar, opaqueScalar, boxScalar, moneyScalar, geoPointScalar, customScalarUUID, uploadScalar) + .scalars(localDateScalar, opaqueScalar, boxScalar, moneyScalar, customScalarUUID, uploadScalar) .resolvers(resolver) .options(options) .build() @@ -203,7 +204,6 @@ class CustomScalarInputTest { scalar Opaque scalar Box scalar Money - scalar GeoPoint scalar Upload type Query { @@ -221,7 +221,6 @@ class CustomScalarInputTest { input EventInput { date: LocalDate opaque: Opaque - box: Box file: Upload } @@ -245,8 +244,8 @@ class CustomScalarInputTest { input PaymentInput { price: Money reference: Opaque - location: GeoPoint date: LocalDate + box: Box } """ @@ -265,7 +264,7 @@ class CustomScalarInputTest { val received = mutableMapOf() fun event(input: EventInput): String = - listOfNotNull(input.date, input.opaque?.value, input.box?.content, input.file?.let { "${it.name}: ${it.content()}" }).joinToString(" ") + listOfNotNull(input.date, input.opaque?.value, input.file?.let { "${it.name}: ${it.content()}" }).joinToString(" ") fun events(inputs: List): List = inputs.map { event(it) } fun optionalEvent(input: Optional): String = input.map { event(it) }.orElse("empty") @@ -273,7 +272,7 @@ class CustomScalarInputTest { fun label(input: LabelInput): String = "${input.date} ${input.opaque}" fun dateNode(input: ObjectNode): String = (input["date"] as POJONode).pojo.let { "${it.javaClass.simpleName} $it" } fun priceNode(input: ObjectNode): String = input.toString() - fun payment(input: PaymentInput): String = "${input.price} ${input.reference} ${input.location} ${input.date}" + fun payment(input: PaymentInput): String = listOfNotNull(input.price, input.reference, input.date, input.box).joinToString(" ") fun price(price: MoneyDto): String = price.toString() fun map(input: Map) = receive("map", input) @@ -294,7 +293,6 @@ class CustomScalarInputTest { class EventInput { var date: LocalDate? = null var opaque: Opaque? = null - var box: Box? = null var file: Part? = null } @@ -305,10 +303,10 @@ class CustomScalarInputTest { class PaymentInput { var price: MoneyDto? = null var reference: OpaqueDto? = null - - @field:JsonDeserialize(using = GeoPointDeserializer::class) - var location: GeoPoint? = null var date: LocalDate? = null + + @field:JsonDeserialize(using = BoxContentDeserializer::class) + var box: String? = null } // serialized as {"value": ...}, which Jackson can't create it from, like the coerced values of many custom scalars @@ -322,16 +320,13 @@ class CustomScalarInputTest { data class MoneyDto(val amount: Int, val currency: String) + // only converts if its content does data class Box(val content: Any) - data class GeoPoint(val lat: Double, val lng: Double) - - // reads the serialized form, and an embedded GeoPoint wouldn't fail it - class GeoPointDeserializer : StdDeserializer(GeoPoint::class.java) { - override fun deserialize(p: JsonParser, context: DeserializationContext): GeoPoint { - val node = p.codec.readTree(p) - return GeoPoint(node.path("lat").asDouble(), node.path("lng").asDouble()) - } + // reads the serialized form, and an embedded Box wouldn't fail it + class BoxContentDeserializer : StdDeserializer(String::class.java) { + override fun deserialize(p: JsonParser, context: DeserializationContext): String = + p.codec.readTree(p).path("content").textValue() ?: "" } class NextDayDeserializer : StdDeserializer(LocalDate::class.java) { @@ -359,7 +354,6 @@ class CustomScalarInputTest { 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 moneyScalar = scalar("Money") { Money(it.toInt(), "CAD") } - private val geoPointScalar = scalar("GeoPoint") { value -> value.split(",").map { it.toDouble() }.let { GeoPoint(it[0], it[1]) } } private fun scalar(name: String, parse: (String) -> T): GraphQLScalarType = GraphQLScalarType.newScalar() .name(name)