Skip to content

Unwrap generic wrappers and futures nested in lists and other wrappers - #903

Merged
oryan-block merged 5 commits into
masterfrom
feature/254-nested-wrappers
Oct 23, 2026
Merged

oryan-block merged 5 commits into
masterfrom
feature/254-nested-wrappers

Conversation

@oryan-block

@oryan-block oryan-block commented Oct 22, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes #254

Checklist

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

Description

The schema build looks through generic wrappers at every level of a method's return type, but the data fetcher only unwrapped the outermost value. So with GenericWrapper.withTransformer(Try::class, 0, { it.get() }):

fun payload(): Try<Payload>          // works
fun payloads(): List<Try<Payload>>   // builds, then every query fails

Expected source object to be an instance of 'Payload' but instead got 'Try$Success'

Same for List<CompletableFuture<Item>> (graphql-java only waits on a future the data fetcher itself returns) and CompletableFuture<CompletableFuture<Item>>. On a [String] field, List<Try<String>> and List<CompletableFuture<String>> returned the wrapper's toString() instead of failing.

Now the data fetcher unwraps the value at the positions where the build unwrapped its type. The new ReturnValueUnwrapper gets them from the same GenericTypeResolver.resolve() the matcher calls (through a new onUnwrap callback), plus a copy of how TypeClassMatcher walks the GraphQL type (DataFetcherResult, Optional, non-null, list elements). Lists are mapped and the futures below the outermost value are combined with allOf, so graphql-java gets one future. Nested wrappers are picked by the value's class with the same code as the outermost one, which moved from MethodFieldResolver into the new file as is.

One failed element fails the whole field. A failed future or a transformer that throws anywhere in the list gives one error at the field's path and a null field, not a null element.

What stays the same:

  • fields with no wrapper inside their return type's type arguments (so almost all of them) get no nested steps. They return the resolver's own value and do the same work as before
  • subscriptions, property fields and custom scalars. A [JSON] field returning List<Try<Map>> still hands the Try to the scalar, which also means [DateTime] from List<CompletableFuture<OffsetDateTime>> is still broken
  • a nested future graphql-java can't wait on (e.g. a FutureTask) is passed through instead of rejected, so an app whose ValueUnboxer calls get() on it keeps working
  • Try<CompletableFuture<Item>> still returns the resolver's own future

Apps that already unwrap these with their own ValueUnboxer get the same data, their unboxer just sees unwrapped values now. Three cases that worked can change, all where the registered transformer disagrees with what handled the value before:

  1. an unboxer that turns a failed Try or a failed future into null while the transformer throws or the future fails. The element was null, now the whole field fails
  2. Optional registered as a generic wrapper with a transformer that throws when empty, used in a list. graphql-java used to turn the empty one into null
  3. an Iterable class registered with listCollectionWithTransformer, nested in a list. graphql-java used to iterate it, now the transformer's list is used. Only different if the two disagree

DataLoaders: the unwrapper never calls load() itself, it only combines the futures it gets. But with graphql-java's default dispatch, a load() made after the field's level was dispatched is never dispatched, so the field hangs. That happens when the load waits on an async step that's still running at dispatch time, and it's already the case today for e.g. supplyAsync { slowCall() }.thenCompose { loader.load(it) }. It's timing dependent, a fast async step finishes before the dispatch and works. Two shapes that used to fail with "Expected source object to be an instance of ..." can now hang the same way instead:

  • a future of a list of futures where the loads happen inside the outer future (supplyAsync { ids.map { loader.load(it) } })
  • a generic wrapper whose transformer calls load(), behind a future that completes later (CompletableFuture<List<Ref<Item>>>). The transformer now runs when that future completes, which can be after the dispatch

Both resolve with DataLoaderDispatchingContextKeys.setEnableDataLoaderChaining (or exhausted dispatching), checked on graphql-java 26.1 and 25.1. A transformer that doesn't load, or loads from a value that isn't behind a future, is fine either way.

Suspend functions: Kotlin reflection types suspend fun items(): List<CompletableFuture<Item>> as List<? extends CompletableFuture<Item>> (the bytecode has no generic return type, and CompletableFuture is open). GenericTypeResolver.resolve() returned a wildcard's upper bound as is, so [Item] failed the build with "Two different classes used for type Item" ([Item!] only worked because non-null resolves again). Now a parameterized bound is resolved too. A type variable bound stays as is, so suspend fun <T> strings(): List<T> for [String] keeps building.

Release note: a generic wrapper or future nested in a list or in another wrapper (List<Try<Payload>>, List<CompletableFuture<Item>>, CompletableFuture<CompletableFuture<Item>>) is now unwrapped instead of failing every query. If one element fails, the whole field fails. A transformer or nested future that calls a DataLoader after an async step needs graphql-java's DataLoader chaining (25+) enabled, otherwise the field never completes.

NestedGenericWrapperTest has 26 cases, 22 of them fail on master, plus 2 for the suspend case above. The other 4 pin what stays the same (custom scalar, plain futures in a list, the resolver's own value, Optional<CompletableFuture<Item>>).

Since the walk is a copy, one more test keeps it in step with TypeClassMatcher. It builds every return type up to 4 deep (with and without ? extends list elements) out of List, CompletableFuture, a generic wrapper, a list collection wrapper, DataFetcherResult and Optional, against 5 GraphQL types. Every combination the matcher accepts (2224) has to resolve through graphql-java with the real unwrapper. With the nested unwrapping turned off, 2022 of them fail.

mvn clean verify passes on JDK 25 against graphql-java 26.1 and 25.1, and on JDK 11 (385 tests).

Also checked it through the installed jar in a separate consumer project, with graphql-java 26.1 and 25.1 at runtime: the issue's example, lists of futures (also from a DataLoader), nested futures, and a failed element in a list of Trys and of futures. On master every one fails or returns toString(), on this branch they resolve and a failed element gives one field error. With an app ValueUnboxer that unwraps Try and joins futures, every result is identical to master except the failed elements, which threw out of execute() on master and are a field error now. The suspend functions above fail the build on master and resolve on this branch, on both graphql-java versions. javap only shows internal classes changing (GenericTypeResolver.resolve and the data fetchers' constructors).

JMH on graphql-java 25.1 (gjt-bench, 3 forks x 5 iterations, master and branch alternated):

  • schema build: no measurable difference. edealerSchema 62.7 / 60.5 ms on master vs 62.4 / 60.9 on the branch, largeSchema 33.8 / 33.2 vs 34.4 / 32.6. None of those schemas has a nested wrapper, so this is only the check that skips the walk
  • a 500 item list with 2 fields per item: List<CompletableFuture<Item>> takes 257-268 µs and List<Box<Item>> 238-243 µs, vs 211 µs for CompletableFuture<List<Item>> and 216-223 µs for a plain list. So roughly 100 ns per future and 40 ns per wrapper
  • the wrapper search now returns before it allocates anything when the value isn't a wrapper, which is most fetches. The plain list went from 278-282 µs on master to 216-223 µs (1.25 -> 1.12 MB per query)

The schema build matches a return type with its generic wrappers unwrapped at
every level, so List<Try<Payload>> builds against [Payload!]!, but the data
fetcher only unwrapped the outermost value. Every query then failed, or a
[String] field returned the wrapper's toString().

The data fetcher now unwraps the value at the positions where the build
unwrapped its type. Lists are mapped, and futures below the outermost value
are combined into one, so one failed element fails the whole field. Fields
with no nested wrapper, subscriptions and custom scalars keep today's
behaviour.

Fixes #254
…tches

Behind ReturnValueUnwrapper the JIT stopped optimizing away the search's
sequence, which cost about 40 bytes per fetch. Also shares the outermost
unwrap between the suspend and non-suspend paths and drops comments the
code already says.
@sonarqubecloud

Copy link
Copy Markdown

@oryan-block

Copy link
Copy Markdown
Collaborator Author

Verifier verdict for 37b0bd7: PASS+NOTES
Ran every scenario from probe.sh list (25) through the installed jar on graphql-java 26.1 and 25.1, at head 37b0bd7 and at parent a4736c9 (target cleaned before each install, jar checked for stale classes). Exit codes are equal at parent and head everywhere except nested-wrappers-suspend, which is 2 at parent ("Two different classes used for type Item") and 0 at head on both versions. bookstore-missing-resolver and context-and-environment-invalid exit 2 on both sides, as designed.
The 22 scenarios that don't touch nested wrappers (44 runs) are identical between parent and head in JSON results, schema, warnings and errors.
nested-wrappers, both versions: payloads, payload-strings, items, item-names, future-of-future, future-of-tries, tries-of-futures and loaded-items go from per-element errors or toString() output to correct data (item-names returns "one","two" instead of CompletableFuture@...[Completed normally]). The two failed-element cases go from per-element errors to one error at the field path and a null field. payload and the controls are unchanged. All of it matches the description.
nested-wrappers-unboxer, both versions: only the two failed-element operations differ. Parent threw out of execute() ("payload failed", "item failed"), head returns one field error each. Matches.
nested-wrappers-suspend: head resolves [Item], [Item!]! and suspend fun <T> strings(): List<T> for [String] on both versions. Matches.
I also ran untracked extra scenarios at parent and head on both versions. At head these all resolve: DataFetcherResult<List<Try>>, CompletableFuture<DataFetcherResult<List<CompletableFuture>>>, Optional<List<Try>>, null elements, Set<Try>, List<List<CompletableFuture>>, enum futures, a wrapper whose transformer returns a future, List<CompletableFuture<Try>>, Try<List<CompletableFuture>>, Try<Try>, a type resolver field with an argument, a root field with an argument, and suspend List<Try>. A failed element in a suspend list gives one field error, no hang. At parent every one of them errors.
Note 1. A ValueUnboxer that swallows failures changes for failed futures, not only for the Try case in "three cases". With a Try.Failure and a failed future in [Payload] and [Item] lists, parent returned [{"id":"ok"}, null] with no error, head returns failedFutureInList: null plus one error. The Try variant is case 1 in the description, the future variant is not listed. Evidence: nested-extra-nulling, parent 20261023-094533, head 20261023-094802.
Note 2. I could not reproduce the DataLoader hang. supplyAsync { ids.map { loader.load(it) } } returned as CompletableFuture<List<CompletableFuture<Item>>>, and a generic wrapper whose transformer calls load() behind a future, both resolve at head on 26.1 and 25.1 with and without chaining. The baseline supplyAsync{}.thenCompose { load } also resolves at parent without chaining, so "already hangs today" did not hold for a single root field. The release note's "never completes" may depend on query shape.
Note 3. [DateTime] from List<CompletableFuture<OffsetDateTime>> still throws a ClassCastException out of execute() at head and parent. Listed as known in the description.
Not checked: the JMH numbers and mvn verify.

@oryan-block
oryan-block merged commit 84dcb8e into master Oct 23, 2026
7 checks passed
@oryan-block
oryan-block deleted the feature/254-nested-wrappers branch October 23, 2026 13:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Generic wrapper don't work with Lists

1 participant