Allow scheduling of CF completion in parallel by fxbonnet · Pull Request #277 · graphql-java/java-dataloader

Requesting changes for a semantic regression around ValueCache hits and context-aware batch loaders.

I opened #278 as a small draft PR showing one possible fix: #278

The PR now constructs a BatchLoaderEnvironment before value-cache filtering:

BatchLoaderEnvironment environment = mkBatchLoaderEnv(keys, keyContexts);
CompletableFuture<List<V>> batchLoad = invokeLoader(environment, keys, keyContexts, queuedFutures, loaderOptions.cachingEnabled());

But invokeLoader(..., cachingEnabled=true) can then remove value-cache hits and invoke the actual batch loader with only missedKeys / missedKeyContexts while still passing the original environment:

CompletableFuture<List<V>> batchLoad = invokeLoader(environment, missedKeys, missedKeyContexts, missedQueuedFutures);

That breaks the existing semantics of BatchLoaderEnvironment: environment.getKeyContextsList() is expected to be aligned with the keys argument passed to the loader. A concrete failure case:

1. The queued dispatch contains keys=[A, B] and keyContexts=[ctxA, ctxB].
2. ValueCache returns a hit for A and a miss for B.
3. The actual batch loader is invoked with keys=[B].
4. The reused environment was built from [A, B], so environment.getKeyContextsList().get(0) is ctxA, not ctxB.

That means a BatchLoaderWithContext implementation that indexes environment.getKeyContextsList() alongside its keys argument can load B using A's context. This is observable behavior and can be a security/data-isolation issue if key contexts carry tenant, auth, locale, or request-scoped metadata.

Please build the BatchLoaderEnvironment from the actual key/context list passed to the concrete loader call after value-cache filtering, or otherwise preserve the invariant that environment.getKeyContextsList() is ordered and sized to match the loader's keys argument. This should also get a regression test with a partial ValueCache hit and a context-aware batch loader asserting that the missed key receives its own context.

PR #278 does exactly that by rebuilding the environment for missedKeys / missedKeyContexts before the concrete loader invocation and adding the regression test.