Check input object classes against the schema with Jackson

[oryan-block]

Fixes #422

Checklist

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

Description

Stacked on #842, merge that first.

A property name on an input class that doesn't match the schema isn't caught when the schema is built. In the #77 comment that #422 links to, ProjectInput has technologies: [TechnologyInput!] in the schema but private List<TechnologyInput> technolgies in the class. Before #842 that failed the build with the misleading "Expected type 'TechnologyInput' to be a GraphQLInputType" error. With #842 it builds, and then fails when a client sends the field, with Jackson's Unrecognized field "technologies". A typo on a scalar field never failed the build at all. Either way you find out at runtime, or not at all if the ObjectMapper ignores unknown properties and the value is silently dropped.

The only check today is the scanner's findInputValueTypeInType, which looks for a name/getName method or a public field. That's fine for finding types, but as a strictness check it's wrong in both directions. It flags setter-only classes, isX getters, @JsonCreator and @ConstructorProperties classes, naming strategies, @JsonAlias and @JsonAnySetter, all of which Jackson maps fine. That's why #842 left #422 out. The thing that decides whether a field is mapped is the field's own ObjectMapper, and that's per field (PerFieldObjectMapperProvider) while an input type can be shared by several fields.

So I ask Jackson. The new internal UnmappedInputFieldChecker runs per resolver field, after SchemaParser has built the input objects and wired their directives:

• For each input-object argument it gets the deserializer convertValue would use, from that field's mapper. MethodFieldResolver now creates its ObjectMapper once, lazily, as internal val objectMapper, so the argument conversion and the check use the same one, along with getMethodParameterType and the same argument index offsets.
• It walks the fields of the built input types, so extend input fields are included, and follows lists, arrays, Optionals and nested inputs.
• A field with no property is read into a placeholder object with FAIL_ON_UNKNOWN_PROPERTIES forced on. It only counts as unmapped if that throws an UnrecognizedPropertyException for that exact name. So anything Jackson maps or deliberately ignores (setters, creators, naming strategies, aliases, @JsonIgnoreProperties, any setters, ignoreUnknown) isn't reported.
• It stops at things it can't check property by property: Map-like types, delegating creators, builders, custom deserializers, and polymorphic values (type info on the property, the class, or from default typing, including elements of lists, arrays and Optionals).
• Visited checks are keyed by deserializer and input type, which handles recursion and shared mappers. Reports are de-duplicated by input type, field, class and consequence, and name the first argument that uses the type.
• Any exception or error during the lookup, apart from VirtualMachineError, is logged at DEBUG and the argument is skipped, so the check can't break a build that worked before.

The message names the field, the class, the usage, what happens at runtime, the properties Jackson knows about, and the fix:

Input field 'ProjectInput.technologies' has no matching property in ProjectInput. Converting argument 'input' of field 'Mutation.createProject' fails whenever the field is sent. Known properties: description, id, name, profiles, technolgies. Rename the property or map it with @JsonProperty("technologies"). If it's left unmapped on purpose, list it in @JsonIgnoreProperties on the class.

The consequence depends on the mapper and the field. It "fails whenever the field is sent", "fails for every 'X' value sent" when the field has a default (graphql-java fills it in, so it's always sent), or "drops the field's value" when the mapper ignores unknown properties. If one type is used by fields whose mappers disagree, each consequence is reported once.

The new SchemaParserOptions.unmappedInputFields takes a new public enum UnmappedInputFields: IGNORE, WARN or FAIL. I made WARN the default. The issue asks for an error, but an error by default would break every schema that has a deliberately unmapped field, and there's no way to tell those apart from typos. WARN logs one line per mismatch. FAIL throws a single SchemaError listing all of them. IGNORE doesn't create the checker at all. Since the check now covers this properly, #842's "Cannot find a public getter or field" WARN in SchemaClassScanner is now DEBUG, because it's the one that gave false positives for setter-only classes and the like.

What isn't checked: Map, JsonNode, Object and type-variable parameters, custom deserializers, delegating creators, builders (only their nested inputs), polymorphic values (skipped, not checked per subtype), classes with @JacksonInject, unknown fields of classes with @JsonUnwrapped properties, and directive arguments. Type-variable parameters already fail at runtime with a ClassCastException in shouldValueBeConverted. That's an existing bug I didn't touch, and since the check uses the same getMethodParameterType, fixing it later fixes both.

Expect small textual conflicts with #843 in SchemaParser.kt and with #849 in SchemaParserOptions.kt.

The new UnmappedInputFieldsTest (with Java fixtures in UnmappedInputFieldsFixtures.java) captures log output and runs real queries next to the schema build, so each report is backed by what actually happens at runtime. Reported cases: the #77 technolgies typo in WARN and FAIL mode (exact text, and the mutation fails), a scalar typo, a Kotlin isActive for a schema active, typos two levels deep through List, array and Optional under an Optional argument, a type shared by two fields plus an extend input field, a mapper that ignores unknown properties, a SNAKE_CASE mapper on one field only, a mismatched field with a default value, mappers with different consequences for one type, and argument index offsets (GraphQLResolver source, DataFetchingEnvironment, suspend continuation). Not reported, in FAIL mode with every value round-tripping in a query: the Java patterns listed above plus @JsonIgnore and @JsonUnwrapped, Kotlin data classes, @JsonNaming, case-insensitive properties, property-level @JsonIgnoreProperties, recursive inputs, Map, Map<String, bean> and ObjectNode parameters, delegating, builder, polymorphic and custom-deserializer classes, polymorphic list and Optional elements, and a module throwing NoClassDefFoundError during the lookup. The known gaps above have tests that pin the current behaviour. I wrote the first four tests before the fix, against existing API only, and all four fail on bugfix/479: the #77 and scalar and isActive cases log nothing useful, and the Java patterns test gets the scanner's false-positive WARN. The rest use the new option, so they don't compile on the base. mvn clean verify passes.

Behaviour change: with the default WARN, schemas build as before but each input field the field's mapper can't map logs one WARN from graphql.kickstart.tools.UnmappedInputFieldChecker, including scalar fields that used to get no message. Deliberately unmapped fields can go in @JsonIgnoreProperties on the class, or the check can be turned off with unmappedInputFields(UnmappedInputFields.IGNORE). Three existing fixtures in this repo now log it, and all three really do fail at runtime. #842's scanner WARN is now DEBUG with shorter text. Jackson deserializers for resolver argument types are now built when the schema is built instead of on the first query, so startup is a bit slower and the first query a bit faster. In a synthetic benchmark that was about 0.1 ms and 33 KB per field with a nested Kotlin input and the default per-field provider, and nothing measurable with one shared mapper. I haven't measured it on a large real schema yet. The per-field ObjectMapper is now created once per MethodFieldResolver, so if one SchemaParser builds the schema twice, both share it, where provide() used to be called again. SchemaParserOptions gains a property, so the data class's copy() gets another parameter and there's a new component17(). That's binary-incompatible for compiled callers of copy(), like when allowUndeclaredDirectives was added, so this should go out in a minor version. Worth a note in the release notes, along with the IGNORE opt-out.

🤖 Generated with Claude Code

[sonarqubecloud]

Quality Gate passed

Issues
2 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