Fetch the initial responses of watch() from the watcher interceptor - #379
Open
AlexanderGH wants to merge 6 commits into
Open
Fetch the initial responses of watch() from the watcher interceptor #379AlexanderGH wants to merge 6 commits into
AlexanderGH wants to merge 6 commits into
Conversation
AlexanderGH
force-pushed
the
perf/overlap-watch-subscription
branch
from
August 13, 2026 02:40
0269212 to
8cfd67c
Compare
watch() executed the operation twice: once through toFlow() for the initial responses, then again through copy().watchInternal() to subscribe to the cache. The last initial response was withheld until that second execution had subscribed, so callers waited on a flow teardown, a fresh channelFlow/withContext dispatch, and a re-run of every interceptor ahead of the cache before receiving data that was already in hand. Move the initial fetch into WatcherInterceptor. It proceeds once with the fetch policy, subscribes, releases the withheld response, and then proceeds per cache change with the refetch policy - all from a single execution of the interceptor chain. What the withheld response now waits on is a SharedFlow subscription and nothing else. The synchronisation point of apollographql/apollo-kotlin#3853 is preserved: the response is still released only after the subscription is established, so modifying the store on receiving it is still observed. Subscribing *before* the initial fetch was tried first and is not viable: the watcher then sees its own initial write publish and refetches, which breaks 13 tests with duplicated emissions and shifted sequences. The ordering here is load-bearing, and the cost it used to carry came from the second chain execution rather than from the ordering itself. watch(data) keeps its existing behaviour through fetchInitialResponses=false: no initial request, fetch policy used throughout, sentinel still emitted for the public overload to filter. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three properties the previous commit depends on had no test of their own. Each was verified to fail against a deliberate break of what it covers. - initialFetchDoesNotTriggerTheWatcher: the initial fetch writes to the cache and publishes, and the watcher subscribes only afterwards, so it must not react to its own write. Fails when the subscription is moved ahead of the fetch. - refetchUsesTheRefetchPolicyRatherThanTheFetchPolicy: the initial responses use the fetch policy and the refetches use the refetch policy, including when they disagree. Pins the refetch request built by the interceptor, which has to clear the noCache set by a NetworkOnly fetch policy. Fails when refetches reuse the original request. - watchExecutesTheInterceptorChainOnce: counts executions of an interceptor installed ahead of the cache. Fails with expected:<1> but was:<2> against the previous two-execution implementation, which is the regression it exists to catch. The synchronisation point of apollographql/apollo-kotlin#3853 and the absence of an initial request in watch(data) were already covered, by storeWriteTriggersWatcher and cacheOnlyFetchPolicy respectively. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
AlexanderGH
force-pushed
the
perf/overlap-watch-subscription
branch
from
August 13, 2026 18:17
8cfd67c to
d2de9ac
Compare
AlexanderGH
marked this pull request as ready for review
August 14, 2026 02:28
Cache headers travel in the ExecutionContext, and both the client and the call can set them, so CacheHeadersContext merged the two in `fold`. ExecutionContext.plus only reaches an element's `fold` when that element sits at the far left of the combined context, though — everywhere else it hands the element straight to the fold operation, and the accumulated element for that key is dropped. Whether the client's headers survived a call that set headers of its own therefore came down to the order the options happened to be set in. Give every CacheHeadersContext its own key so plus never drops one, and merge them where they are read instead. This surfaced on watch(): its refetch used to be a second execution of the chain whose context happened to be shaped so the merge ran, so folding the two executions into one lost the client's headers on the refetch and `CacheOptionsTest.memoryCacheOnlyIsPropagated` failed. The initial fetch never got them either way, which is what the new test covers directly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
BoD
reviewed
Aug 14, 2026
| * same [com.apollographql.apollo.api.ExecutionContext] and have to be merged. | ||
| */ | ||
| @Test | ||
| fun clientCacheHeadersSurviveCallCacheHeaders() = runTest(before = { setUp() }, after = { tearDown() }) { |
Collaborator
There was a problem hiding this comment.
Thanks for noticing this bug + the test! I just simplified a little bit the fix in 45efbfb
BoD
reviewed
Aug 14, 2026
| } | ||
|
|
||
| assertEquals(channel.awaitElement()?.hero?.name, "R2-D2") | ||
| assertEquals("R2-D2", channel.awaitElement()?.hero?.name) |
Collaborator
There was a problem hiding this comment.
Sorry for the noise, unrelated to that PR, but I fixed the warnings in that file in 9d7a5d5
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
watch() executed the operation twice: once through toFlow() for the initial
responses, then again through copy().watchInternal() to subscribe to the
cache. The last initial response was withheld until that second execution had
subscribed, so callers waited on a flow teardown, a fresh
channelFlow/withContext dispatch, and a re-run of every interceptor ahead of
the cache before receiving data that was already in hand.
Move the initial fetch into WatcherInterceptor. It proceeds once with the
fetch policy, subscribes, releases the withheld response, and then proceeds
per cache change with the refetch policy - all from a single execution of the
interceptor chain. What the withheld response now waits on is a SharedFlow
subscription and nothing else.
The synchronisation point of apollographql/apollo-kotlin#3853 is preserved:
the response is still released only after the subscription is established, so
modifying the store on receiving it is still observed.
Subscribing before the initial fetch was tried first and is not viable: the
watcher then sees its own initial write publish and refetches, which breaks 13
tests with duplicated emissions and shifted sequences. The ordering here is
load-bearing, and the cost it used to carry came from the second chain
execution rather than from the ordering itself.
watch(data) keeps its existing behaviour through fetchInitialResponses=false:
no initial request, fetch policy used throughout, sentinel still emitted for
the public overload to filter.