[oryan-block]

Fixes #905

Checklist

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

Description

#903 unwraps generic wrappers and futures nested in lists and other wrappers, but ReturnValueUnwrapper works out where to unwrap with its own copy of the order TypeClassMatcher unwraps in. Only NestedGenericWrapperTest kept the two in step. It also kept a few cases where the schema build and the runtime disagree, so 14.x apps wouldn't break.

Now the matcher records what it unwraps and the data fetcher replays that:

• PotentialMatch.unwrapping is the matcher's walk, kept as a list of Wrapper(GenericWrapper), Data (DataFetcherResult), Present (Optional) and Elements (list elements), outermost first. match() only looks up the named type at its end
• MethodFieldResolver matches its return value from the declared type, so the record starts at the outermost wrapper. It builds the data fetcher's steps from the same PotentialMatch the scanner matched, so nothing new goes through the scanner or the parser
• every wrapper in the record gets a step, and each step sees the value at its own position. A future's step waits on it. Any other wrapper's step hands what its transformer returns to the next step as is when that's a future's step, so in Try<CompletableFuture<X>> the future's step gets the future. Otherwise it waits on a CompletionStage its transformer returns (e.g. from a DataLoader)

What changes, one per item of the issue:

1. A wrapper that only a custom schemaWrapper introduces is unwrapped too. E.g. a Batch<T> whose schemaWrapper is List<CompletableFuture<T>> used to hand graphql-java the futures, since the old check only looked for wrappers in the declared type's arguments.

2. Custom scalars aren't exempt anymore. [DateTime] from List<CompletableFuture<OffsetDateTime>> gives the Coercing the OffsetDateTimes, and [JSON] from List<Try<Map>> the maps.

3. A nested Future that isn't a CompletionStage (List<Future<Item>> holding a FutureTask, or a wrapper in a list whose transformer returns one) fails with the error the outermost value already gives:

   Resolver method '...' resolved to a java.util.concurrent.FutureTask, which graphql-java can't wait on. Return a CompletionStage, such as a CompletableFuture, from the method or its generic wrapper transformer instead.

   instead of Expected source object to be an instance of 'Item' but instead got 'java.util.concurrent.FutureTask'. A suspend function gets the message its outermost value gets (Suspend function '...' resolved to a java.util.concurrent.FutureTask, which graphql-java can't wait on. Return its value instead.). A value of the class the return type ends in isn't rejected even if it implements Future. When that class is Object (e.g. for a union) every value is an instance of it, so only a declared future like List<Future<Any>> is checked. A wrapper's transformer returning a FutureTask there still fails like on master, when graphql-java resolves the union's type (Expected object type with name 'FutureTask' to exist for union 'Item', but it doesn't!).

4. A nested wrapper is unwrapped with the wrapper the schema build matched for its declared type, not the most specific one its value is an instance of. The record already has that wrapper, so this came for free, and nested values skip the wrapper search. It's only different when a subclass is registered as a wrapper of its own, e.g. a ListenableFuture wrapper with a List<Future<Item>> return type. The outermost value is still picked by its class, since a resolver declared as Object (e.g. for a union) can return a wrapper.

5. isFutureReturnedByWrapperBefore is gone. It skipped the step of a future directly inside another wrapper, because that wrapper's step had already waited on it. Now the future gets its own step. With the default future wrappers nothing changes, Try<CompletableFuture<X>> still hands graphql-java the resolver's own future. But if you registered a wrapper for the future's type, e.g. CompletableFuture with a transformer, that transformer now runs there and graphql-java gets the future it returns. Before, it was skipped and graphql-java got the resolver's own future.

Fields with no nested wrapper do the same work per fetch as before, plus two checks. Steps only exist when the record has a wrapper below the outermost value (Data, Present and Elements with nothing below them are dropped). So the data fetcher still does the wrapper search, requireOutermost and returns the value. For a plain list the wrapper search returns before it allocates. A CompletableFuture<List<Item>> matches the default future wrapper, so like on master it also sorts the matching wrappers and runs the transformer. The two checks are isOutermostFuture and, when the outermost value isn't a future, whether the next step is a future's, which is false with no steps.

Tradeoffs:

• I first sketched keeping the old waiting (wait on any CompletionStage between steps) and only dropping the rule. It's less code, but then in Try<CompletableFuture<X>> the future's step gets the X instead of the future. That's harmless while every step picks its wrapper by the value's class, but a check or a declared wrapper at that position acts on the wrong value. E.g. a transformer you registered for CompletableFuture gets the X, and graphql-java gets a new future instead of the resolver's own one. returns the resolver's own value when nothing is nested catches that.
• The record could also travel on the Match through ScannedSchemaObjects and SchemaParser into createDataFetcher(...), or be filled in by a callback during the scan. Both add plumbing or mutable state for something the field resolver already holds, so it lives on the PotentialMatch (lazily, so the walk runs once).
• The matcher's error now names a method's declared return type (java.util.concurrent.CompletableFuture<Foo>) instead of the one with its wrappers removed (Foo). No test checked that text.

resolves every return type the matcher accepts still builds every return type up to 4 deep, now with Future as a container and a custom scalar leaf next to Payload. The scalar's value class implements Future, so it also checks that a value which only happens to be one isn't rejected. That's 56,020 combinations, 10,136 of them accepted by the matcher, and the 5,688 that contain a Future expect the "can't wait on" error. The same test written against master's API fails 5,382 of them on master (3,702 of those with the scalar leaf). Making the outermost step wait on a future before handing it over (the first tradeoff) fails returns the resolver's own value when nothing is nested, and doing the same in the other wrappers' steps also fails one of the tests for a wrapper handing a future to its own wrapper. The matrix still passes either way. There are new tests for the schemaWrapper case, the subclass wrapper, a wrapper whose transformer returns a FutureTask, a suspend function's plain futures, plain futures for a union, a union member that implements Future in a list of Trys and a Try handing a plain future to its own registered wrapper, and the custom scalar and plain future tests now expect the new behaviour.

mvn clean verify passes on JDK 25 against graphql-java 26.1 and 25.1 (417 tests each).

Also checked through the installed jar with verify-tools, on graphql-java 26.1 and 25.1:

• nested-wrappers, nested-wrappers-unboxer and nested-wrappers-suspend give identical results on master and this branch, so features/generic-wrappers.md still holds
• a new nested-wrappers-exact scenario. On master [DateTime] hands the scalar the futures and the Trys (Can't serialize value (/dates[0]) : Expected an OffsetDateTime but got a java.util.concurrent.CompletableFuture), the FutureTask fails on its children, the schemaWrapper's futures fail the same way and the subclass wrapper's transformer runs. On this branch the dates serialize, the FutureTask gives the error above and the other two resolve. It also has a wrapper whose transformer returns a FutureTask and a suspend function returning List<Future<Item>>. Both fail on their children on master and give the errors from 3 on this branch

I didn't run JMH. That nested values are faster without the wrapper search is a guess.

Release note:

• a generic wrapper that only a custom schemaWrapper introduces is now unwrapped below the outermost value like any other
• custom scalars now get the unwrapped values of nested wrappers and futures, e.g. [DateTime] from List<CompletableFuture<OffsetDateTime>> gets the OffsetDateTimes. A Coercing that unwrapped them itself now only sees the plain values
• a Future that isn't a CompletionStage, nested in a list or another wrapper or returned by a nested wrapper's transformer (unless the return type ends in Object), now fails the field with "graphql-java can't wait on" (the suspend function message for a suspend function). If your ValueUnboxer called get() on it, return a CompletableFuture instead
• a nested generic wrapper is now unwrapped with the wrapper registered for its declared type, not the most specific one its value is an instance of. If you registered a subclass as a wrapper of its own, declare that subclass in the return type
• a future directly inside another wrapper (Try<CompletableFuture<X>>) now goes through the transformer of a wrapper you registered for its type, which used to be skipped there. With the default future wrappers it still reaches graphql-java as the future the resolver returned. If you need the resolver's own future there, declare it as a type you didn't register a wrapper for (e.g. Try<CompletionStage<X>>) or make the transformer return the future it gets

[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