Fix value cache batch loader environment by andimarek · Pull Request #278 · graphql-java/java-dataloader

bbakerman

This is a demonstration patch stacked on top of #277. It addresses the BatchLoaderEnvironment semantic regression described in the requested-changes review.

Problem

#277 builds a BatchLoaderEnvironment before value-cache filtering. If a dispatch has a partial ValueCache hit, the actual batch loader is invoked only with the missed keys, but the environment can still describe the original full key/context list.

That breaks the existing invariant that environment.getKeyContextsList() is aligned with the keys argument received by a BatchLoaderWithContext.

Fix

When ValueCache filtering produces missedKeys, build a fresh BatchLoaderEnvironment from missedKeys and missedKeyContexts before invoking the concrete batch loader.

The actual fix is intentionally small:

src/main/java/org/dataloader/DataLoaderHelper.java:483 builds missedEnvironment from missedKeys and missedKeyContexts.
src/main/java/org/dataloader/DataLoaderHelper.java:484 passes missedEnvironment into the concrete invokeLoader(...) call for the cache misses.

GitHub line link:
https://github.com/graphql-java/java-dataloader/blob/fix-pr-277-value-cache-environment/src/main/java/org/dataloader/DataLoaderHelper.java#L483-L484

Test

Adds a regression test where:

A is returned from ValueCache
B misses and is sent to the batch loader
the batch loader asserts it receives only B
environment.getKeyContextsList().get(0) matches B's context, not A's

The key test lines are:

src/test/java/org/dataloader/DataLoaderBatchLoaderEnvironmentTest.java:221-249

GitHub line link:
https://github.com/graphql-java/java-dataloader/blob/fix-pr-277-value-cache-environment/src/test/java/org/dataloader/DataLoaderBatchLoaderEnvironmentTest.java#L221-L249

Verification

./gradlew test --tests org.dataloader.DataLoaderBatchLoaderEnvironmentTest
./gradlew test
git diff --check origin/pr/277