Skip to content

Fix value cache batch loader environment#278

Draft
andimarek wants to merge 4 commits into
masterfrom
fix-pr-277-value-cache-environment
Draft

Fix value cache batch loader environment#278
andimarek wants to merge 4 commits into
masterfrom
fix-pr-277-value-cache-environment

Conversation

@andimarek
Copy link
Copy Markdown
Member

@andimarek andimarek commented May 20, 2026

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

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.

3 participants