apollo-kotlin icon indicating copy to clipboard operation
apollo-kotlin copied to clipboard

Complete flow only after cache writes have completed

Open rohandhruva opened this issue 9 months ago • 3 comments

This is a proposal PR that changes the ApolloCacheInterceptor flows to wait for the cache writes to complete before completing the flow. This is typically only useful when using writeToCacheAsynchronously(true), so that you can get a signal of when the cache write has completed.

Thinking about this a bit more, it gets a bit tricky when you use execute(): singleSuccessOrException uses flow.toList() which will always wait for flow completion by default. With this change, you effectively lose all benefits of asynchronous caching if you are also using execute().

Let me know your thoughts: maybe the execute() limitation means that we don't go ahead with this unless we find some workaround for toList().

Relates to #5852

rohandhruva avatar May 06 '24 04:05 rohandhruva

Deploy request for apollo-android-docs pending review.

Visit the deploys page to approve it

Name Link
Latest commit 2087ca14272e2cba067cd6f7fc7e8880c422b2c5

netlify[bot] avatar May 06 '24 04:05 netlify[bot]

Thanks for this @rohandhruva !

Indeed as you noticed, the execute() case is problematic because the change would defeat the purpose of writeToCacheAsynchronously. I thought we could maybe get around that by using isLast, but unfortunately we can’t fully rely on it as there can be false negatives.

Long term, maybe Martin’s proposition of exposing events would help as we could emit an event to signal the end of async cache writes.

Back to your original question, your best bet would be to not use writeToCacheAsynchronously in specific cases - but I guess that’s probably what you currently do already?

BoD avatar May 06 '24 09:05 BoD

Back to your original question, your best bet would be to not use writeToCacheAsynchronously in specific cases - but I guess that’s probably what you currently do already?

Yes, that's what we do for now, especially for places that are only able to accept a single response.

However, for UIs that are able to use a flow of responses, we would like the ability to get the response but still wait for the cache write, and then do something else in the flow.onComplete { } event with the guarantee that the cache was written to. That was the primary motivation behind this PR :)

I thought we could maybe get around that by using isLast

That would be nice indeed :( Another option is that we can use take(1) for execute(), but we would need to hold the singleSuccessOrException logic somewhere in the stream of the flow, which does sound tricky.

Let me think about this a bit more, and please let me know if you have any other suggestions I can try!

rohandhruva avatar May 07 '24 21:05 rohandhruva