Pass coerced custom scalar values through to input objects and Map arguments

[oryan-block]

Fixes #436
Fixes #424

Checklist

• Pull requests follows the contribution guide
• New or modified functionality is covered by tests

Description

A custom scalar inside an input object fails to reach the resolver if Jackson can't round-trip its value. In #436 that's a LocalDate on an input class with no jsr310 module registered:

Java 8 date/time type java.time.LocalDate not supported by default ... (through reference chain: java.util.LinkedHashMap["date"])

Uploads in input objects (#466, and #198 before it) fail the same way, because Jackson tries to serialize the Part:

No serializer found for class java.io.ByteArrayInputStream ... (through reference chain: java.util.LinkedHashMap["file"]->...["inputStream"])

Map arguments are worse, because nothing fails. In #424 a scalar value inside a Map<String, Any?> argument arrives as its serialized form, so a bean-like Money scalar comes through as {amount=5, currency=CAD} instead of a Money.

graphql-java has already coerced the argument by the time the resolver runs, so the input object is a Map holding the scalar instances. MethodFieldResolver then ran that through ObjectMapper#convertValue to get the parameter type. convertValue serializes the whole value and reads it back, so every scalar instance has to survive a round trip through Jackson's serializers and deserializers.

The new internal ArgumentConverter is built once per argument, and MethodFieldResolver calls ArgumentConverter#convert where it called convertValue. Jackson stays in the path, since that's why #471 was closed.

For typed parameters (input classes, Kotlin data classes, lists of inputs, Optional, ObjectNode, top-level scalars that need converting), convertValue still runs first. So anything that converts today gives the same result, including custom deserializers, objectMapperConfigurer, naming strategies and the serializer workaround from #466. Only if it throws, the value is rewritten into a TokenBuffer. Maps, lists, strings, numbers, booleans and enums become tokens as before. A leaf whose value can't be converted to its own class is written with writeEmbeddedObject. The buffer is then read with ObjectMapper#readerFor(type) and a DeserializationProblemHandler. Its handleMissingInstantiator returns the embedded instance for an interface or abstract property such as Part, and handleWeirdNativeValue converts the value on its own when the property has a different class. Classes that fail to convert are cached per argument. Other values are checked every time, because whether a value converts can depend on what's in it. If the retry fails too, its error is thrown with the original one attached as suppressed, since the retry's error names the real problem. If no value qualifies for embedding, the original error is rethrown unchanged.

Untyped parameters (Object, or a Map, Collection, array or Optional of it, so Map<String, Any?>, a raw LinkedHashMap, List<Map<String, Any?>>, Optional<Map<String, Any?>>) skip convertValue. They're read from the buffer with UntypedObjectDeserializer, and every leaf other than a String, Number, Boolean or enum is embedded. So the resolver gets exactly what graphql-java coerced, whatever serializers or modules the mapper has. That's what #424 asks for.

I didn't add a second pass for a scalar whose class Jackson can round-trip but that's assigned to an interface or abstract property. That's Jackson's ordinary abstract type mapping error, master reports it clearly, and @JsonDeserialize(as = ...) fixes it. Embedding those values would hand wrong values to custom deserializers and String properties in inputs that master rejects with a clear error. I also didn't add a SchemaParserOptions flag to keep the old Map behaviour. Declaring a typed parameter already does that, and a flag would be new API for a behaviour #424 explicitly asks for.

Left out:

• Untyped properties nested inside a typed input (e.g. var extra: Map<String, Any?> on an input class) still get serialized scalar values when conversion succeeds, as on master. Only top-level untyped parameters take the embedded path.
• A scalar that's only used in a Map or ObjectNode input still fails the schema build with "Expected type 'LocalDate' to be a GraphQLInputType". That's Custom scalars are not properly scanned when only used in input types #355, fixed by Register input field types that have no Java property #842. The Map tests reference the scalars from another input for now, and a Map-only variant can be added once Register input field types that have no Java property #842 merges.

The new CustomScalarInputTest has 8 tests, all running queries through GraphQL#execute and checking the data or errors. They cover a LocalDate (no jsr310) and an opaque class that can't be rebuilt from its serialized properties in an input object, sent as a literal and as a variable, in a list, in an Optional and in an ObjectNode, plus the opaque value on its own. Uploads go through the existing Upload scalar into a Kotlin data class (Part?, List<Part>) and a Java-style POJO, and the test checks the file contents. The Map test covers Map<String, Any?>, a raw LinkedHashMap, List<Map> and Optional<Map> with a UUID, a LocalDate, a bean-like Money and a nested input. Guards check that values the ObjectMapper can convert still go through it: Money into a MoneyDto, the opaque class into an OpaqueDto, a @JsonDeserialize custom deserializer with and without a LocalDate forcing the retry, an ObjectNode with a bean scalar, and a top-level DTO argument. Another uses a mapper configured with custom LocalDate and Part (de)serializers and NON_NULL. The rest cover the error naming the missing name with the jsr310 error suppressed, a class whose values only sometimes convert, and String targets getting toString(). All 8 fail on master (603d40e). I also checked that dropping handleWeirdNativeValue or handleMissingInstantiator, or embedding by serializer class in the typed retry, makes the tests fail. mvn clean verify passes with 266 tests.

Behaviour change: a top-level untyped parameter now gets the values graphql-java coerced. Strings, numbers, booleans and enums come through as before (enums as names). Every other scalar value arrives as its instance. So a bean-like scalar is no longer a nested LinkedHashMap, a UUID is a UUID instead of a String, a java.util.Date is a Date instead of a Long, URL, Locale and Character are instances instead of strings, and java.time values are instances with or without jsr310 (previously an error, or arrays and epoch numbers with jsr310). Resolvers that cast these to String, Map or Long will throw ClassCastException. To keep the old form, declare a typed parameter such as an input class or ObjectNode. Inclusion settings like NON_NULL still apply to the Map. The rest only affects inputs that fail on master: an ObjectNode parameter gets a POJONode for a value that can't be converted, and a String property receiving one gets its toString(), which is Class@hash for a class that doesn't override it. When both paths fail, the error message differs from master's, which is attached as suppressed. Typed inputs that would fail on master pay one failed convertValue per call before the retry. I think this wants a minor release with the Map rule in the release notes.

🤖 Generated with Claude Code

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud