GeezerVison

Hey folks, we've been seeing a pretty high cost with oneOf validation on our graphql gateway. I've launched an LLM at the problem, and while it does improve things, I'm not sure if you're OK with the added complexity here. Wanted to discuss it here first.

The TL;DR is that we can avoid a lot of work in the common case by validating only one level and not recursing down on variables that were already validated, with the edge case of default values.

Let me know what you think.

AI Summary

Problem

While coercing variables, ValuesResolverConversion.externalValueToInternalValueImpl calls
ValuesResolverOneOfValidation.validateOneOfInputTypes(inputObjectType, coercedMap, ...) after converting
every input object. That validator is a recursive walk: it descends into lists and into every field
defined on each input object type (inputObjectType.getFields() + objectMap.get(...)), including fields
that were not provided.

Conversion itself is already recursive (externalValueToInternalValueForObject calls
externalValueToInternalValueImpl per field, externalValueToInternalValueForList calls it per element), and
each of those nested calls runs the walk again over its own subtree. So:

• the innermost object is walked by itself, then again by its parent, then again by its grandparent, ...
• an input object at depth d is validated d + 1 times, and the total work is O(depth x size) instead of
  O(size);
• types with no @oneOf anywhere below them pay the full cost, because the walk has to look at every field to
  find out.

We noticed this in a production CPU profile of a GraphQL federation gateway, where it accounted for ~5% of total
CPU: validateOneOfInputTypes <- validateOneOfInputTypes self-recursion, dominated by HashMap.getNode self time
from the per-field map lookups.

Fix

In the variable conversion path, each input object is now checked at its own level only, right after it has
been converted:

• if the type is @oneOf, the existing single-object check (validateOneOfInputTypesInternal) runs on the just
  coerced map;
• provided fields don't need re-walking: their values were produced by externalValueToInternalValueImpl, which
  already checked them at their own level (and so on, recursively);
• fields that were filled from a literal or internal default value never go through
  externalValueToInternalValueImpl, so those (and only those) are still walked recursively with the existing
  validateOneOfInputTypes. This is rare and only walks the default value's subtree.

The literal/argument path (ValuesResolver.getArgumentValuesImpl) is unchanged and keeps its single recursive walk
per argument: literal coercion does not validate per level, so that walk is not redundant there.

The change is ~20 lines in ValuesResolverConversion plus making validateOneOfInputTypesInternal
package-private. No public API, signature or schema changes.

Why this is equivalent

Every input object map in a coerced variable value is produced by exactly one of:

1. externalValueToInternalValueImpl on an input object type (variables, list elements, provided object fields,
   external defaults, ValuesResolver.externalValueToInternalValue, valueToInternalValue for external values,
   DefaultValuesAreValid schema validation). Each such map is now validated at that point, which is exactly
   when the old code first validated it (its own Impl call ran the walk on its own subtree before returning).
   The parent re-walks could only re-visit maps that had already passed.
2. defaultValueToInternalValue for a literal or internal default. These were previously only covered by the
   parent's walk, and are still walked recursively. This gap is real and not covered by schema validation:
   internal defaults (GraphQLInputObjectField.Builder.defaultValue(Object)) are not validated by
   DefaultValuesAreValid at all, and a literal default such as child: OneOf = {a: null} passes
   ValidationUtil.isValidLiteralValue (it only checks the key count) but must fail with
   OneOfNullValueException at coercion time. Both are covered by new tests, which fail if the default handling
   is removed.

Other points checked:

• Lists coerced from a single non-list value (8c143ef): externalValueToInternalValueForList converts each
  element through externalValueToInternalValueImpl, including the singleton case, so each element is validated
  at its own level. Tested for top-level and nested lists.
• Null handling: unchanged. The walk skipped nulls; explicit nulls are still put in the coerced map as before
  and a oneOf {a: null} is still rejected by the single-level check.
• InputInterceptor: the check runs on the coerced map, after interception, exactly as before.
• Field visibility: unchanged. The old walk iterated getFields(), but hidden fields are never in the coerced
  map, so they were no-ops.
• Error types, messages and precedence: unchanged. Because each object was already validated by its own
  Impl call before its parent continued with the next sibling, the first error raised is the same in both
  versions: a nested oneOf violation still wins over a later sibling's coercion error, and an earlier sibling's
  coercion error still wins over a later oneOf violation (tested). For an object with literal/internal defaults,
  the object's own oneOf check still runs before its defaults are walked, matching the old walk's order.
• All new tests also pass against master, i.e. they pin the existing behaviour rather than a new one.

Prior attempt: #4516

#4516 ("Skip OneOf input validation walk when no @OneOf type is reachable") added a schema-owned
OneOf reachability map computed at schema construction (GraphQLSchema constructors, FastBuilder,
transformWithoutTypes), a new @Internal GraphQLSchema.inputTypeContainsOneOf, and threaded an optional
GraphQLSchema through ValuesResolver, ValuesResolverConversion, DirectivesResolver and others. The author
closed it because local testing did not show a benefit worth the complexity.

That approach only short-circuits types from which no @oneOf is reachable. When a oneOf is reachable (which is
the case for any schema where oneOf matters), every level still ran the full recursive walk, so the
O(depth x size) re-walk remained. It also introduced new schema state that every schema construction path has
to keep correct.

This change instead removes the redundant re-walk itself:

• it is a local change to the conversion code, with no new API, no schema state and no signature changes;
• it helps whether or not a @oneOf type is reachable (see the oneOfLeaf=true rows below, which Skip OneOf input validation walk when no @oneOf type is reachable #4516 could
  not have improved);
• it is backed by the benchmark below.

Benchmark

New benchmark.OneOfVariableCoercionBenchmark calls ValuesResolver.coerceVariableValues on a variable made of
nested input objects depth levels deep. Each level has 8 optional String fields of which 3 are provided, plus a
next field; the innermost level holds a Leaf { a: Int, b: String } input object, either plain
(oneOfLeaf=false, no oneOf reachable anywhere) or @oneOf (oneOfLeaf=true). singleObject coerces one such
value, listOfObjects a [Level0!] list of 20 of them.

Settings: JMH 1.37, AverageTime, 2 forks, 3 warmup iterations x 2 s, 5 measurement iterations x 2 s
(./gradlew jmh -PjmhInclude=benchmark.OneOfVariableCoercionBenchmark), JDK 25.0.2 (Zulu), Apple M2 Max.
"Before" is the same benchmark run against master's src/main.

With the fix, the cost is the same whether or not a oneOf is reachable, and it scales with the size of the value
instead of size x depth.

Tests

New Spock tests in ValuesResolverTest:

• nested oneOf violations in variables are reported (too many keys, empty object, null value) at depth >= 3
  through non-oneOf parents, for oneOf nested in oneOf, inside lists of oneOf, inside lists of non-oneOf parents,
  in a top-level list variable, and for single values coerced into (nested) lists;
• a valid deeply nested value is accepted unchanged;
• error precedence between a nested oneOf violation and a sibling coercion error, in both orders;
• oneOf violations in nested internal and literal defaults are still reported during variable coercion;
• external defaults with a oneOf violation are still rejected while the schema is validated.

./gradlew test (JDK 25 toolchain, the project default): 6088 tests, 0 failures, 0 errors, 7 skipped. This
includes the existing ValuesResolver*, oneOf, graphql.execution and ArchUnit (JMH fork limit) tests.

Side observation (unchanged by this PR)

DefaultValuesAreValid.isValidExternalValue only catches CoercingParseValueException and
NonNullableValueCoercedAsNullException, so an external default with a oneOf violation makes schema building throw
the raw OneOfTooManyKeysException / OneOfNullValueException rather than an InvalidSchemaException. This is the
same on master; the new test pins the current behaviour.