Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrating CACHE_ONLY queries to v3 #3689

Closed
mateuszkwiecinski opened this issue Dec 11, 2021 · 9 comments
Closed

Migrating CACHE_ONLY queries to v3 #3689

mateuszkwiecinski opened this issue Dec 11, 2021 · 9 comments

Comments

@mateuszkwiecinski
Copy link
Contributor

mateuszkwiecinski commented Dec 11, 2021

Context.
The whole codebase of my project relies on apollo cache having a valid state. I treat cache as source of truth. I either watch the cache or mutate it, never together. Translating that to Apollo-specific language I've been using either CACHE_ONLY or NETWORK_ONLY response fetcher (+ CACHE_FIRST refetch response fetcher, just in case, but the assumption is it should never reach the network).
The only customization, in addition to classes provided by Apollo was a workaround for #2798 where I ended up with custom fetcher that only listens cache until dependentKeys are known for given query. In other words, it uses CACHE_ONLY refetch fetcher and after first successful refresh call with NETWORK_ONLY it switches back to default CACHE_FIRST refetch fetcher.

Repro steps
I created playground project: https://github.com/lwasyl/apollo-playground/blob/apollo3_cache_only/src/test/kotlin/com/example/Test.kt#L19 where I'm trying to understand v3.0.0 changes and how to migrate from response fetchers to fetch policies.
I noticed it's not possible to provide custom fetch policy anymore as FetchPolicy is an enum (which brings back issue described in #2798), but I noticed that I'm basically unable to watch the cache anymore. Queried called with FetchPolicy.CacheOnly on an empty cache kills the flow with:

com.apollographql.apollo3.exception.CacheMissException: Object 'QUERY_ROOT' has no field named 'viewer' on the linked repro project.

so calling the same query with FetchPolicy.NetworkOnly will never notify collectors of CacheOnly Flow, as it is already disposed 🤔

Question
What's the proper way of observing query without making network calls as a side effect? Especially in a scenario where the cache is empty, before dependentKeys (not sure if they are still called that way) are known?
Is there a way to provide custom FetchPolicy to work around the issue described in: #2798?

@martinbonnin
Copy link
Contributor

martinbonnin commented Dec 11, 2021

Thanks for sending this! I'll dig into this a bit more later but a few initial thoughts:

Queried called with FetchPolicy.CacheOnly on an empty cache kills the flow with CacheMissException

Indeed, that's a difference with 2.x. ApolloCall.execute is guaranteed to return either a ApolloResponse or throw (it was returning null on cache miss before).

To watch the cache, you should be able to do something like below:

apolloClient.query(query).fetchPolicy(FetchPolicy.CacheOnly).watch()

There is a special case to not throw on errors if the query is watched so I think it should work?

I don't think I have access to https://github.com/lwasyl/apollo-playground/blob/apollo3_cache_only ? But will definitely look if you can give access to user martinbonnin.

@mateuszkwiecinski
Copy link
Contributor Author

mateuszkwiecinski commented Dec 11, 2021

Thanks for reply! Indeed I used toFlow() instead of watch(). I should pay more attention when going through the documentation 🤦

Using watch indeed fixes the repro project, but I'm observing different behavior in my main project when collecting changedKeys here - they never get properly re-fetched
Let me keep the issue open for a while, and I'll either share more details and update repro project (and I make sure it's public!) or I close the issue with a summary of my findings.

@mateuszkwiecinski
Copy link
Contributor Author

mateuszkwiecinski commented Dec 12, 2021

So, I was initially wrong by saying FetchinPolicy.CacheOnly doesn't work. When using watch() all seems to be working the same way as in 2.x, at least the default behavior.
The thing that wasn't working for me above was a mistake I made when migrating CacheKeyResolver. My project manually overrides cache key of viewer entry so I couldn't use the suggested implementation of CacheKeyResolver - that was the reason I was getting cache misses.

Anyway, I was able to get my project compile again on Apollo 3.0.0-rc02, but I'm still having a few tests failing. I was able to debug 2 of them. One is related to CacheOnly fetch policy so I'll use this issue to continue discussion and open separate one with the other bug I found (edit: here).

Here's a repro project (I made sure it's public this time 😅 )
https://github.com/mateuszkwiecinski/apollo-playground/blob/979d9f9d7070aaf584a6899b360445b550b25885/src/test/kotlin/com/example/Test.kt#L39
disjoint queries test makes unexpected network calls, despite using CacheOnly fetch policy. The quickfix is to use CacheOnly as a refetchPolicy, but then I lose the fail safe mechanism which helps to maintain proper app state when someone makes a mistake in query definition. I also can't add logs to track when refetcher tries to reach the network.

It's basically the same as #2798, but the challenge in 3.0.0 is I can't provide my own fetch policy.
I don't want a CacheOnly query to make network calls when the cache is empty and some unrelated query is made, but at the same time, I still want it to log and re-fetch required data in case someone makes a mistake and creates a partially disjoint query which would trigger a refetch. I want to use the refetching mechanism, but not when watchedKeys aren't computed yet.
As mentioned above, in 2.x I was able to easily tweak Apollo behavior by providing custom ResponseFetcher as a refetch response fetcher which would behave initially as CacheOnly and switches to CacheFirst right after receiving first successful response. In 3.x I can't provide custom FetchPolicy and as a workaround I can only think of moving the workaround to a upper level, similary to what iOS devs have to do to deal with apollographql/apollo-ios#99 🤷
Is there anything Apollo 3.x exposes that would help me override refetching policy?

@martinbonnin
Copy link
Contributor

Hi 👋 Indeed, that's something that's not possible to do in Apollo Android 3. We're looking at adding it back

@martinbonnin
Copy link
Contributor

martinbonnin commented Dec 17, 2021

Hi @mateuszkwiecinski

#3743 introduces apolloCall.refetchPolicyInterceptor(refetchPolicyInterceptor). This way, it's possible to customize the refetchBehaviour using the ApolloInterceptor API.

I think the interceptors from the test does what you're looking for?

  private val refetchPolicyInterceptor = object : ApolloInterceptor {
    var hasSeenValidResponse: Boolean = false
    override fun <D : Operation.Data> intercept(request: ApolloRequest<D>, chain: ApolloInterceptorChain): Flow<ApolloResponse<D>> {
      return if (!hasSeenValidResponse) {
        CacheOnlyInterceptor.intercept(request, chain).onEach {
          if (it.data != null) {
            // We have valid data, we can now use the network
            hasSeenValidResponse = true
          }
        }
      } else {
        // If for some reason we have a cache miss, get fresh data from the network
        CacheFirstInterceptor.intercept(request, chain)
      }
    }
  }

It's a pretty significant change to the watcher and cache pipeline so didn't want to include that last minute in the 3.0 release but let us know if that'd work for you and we'll include it in the next release.

@mateuszkwiecinski
Copy link
Contributor Author

Wow! That looks great 🚀 The test you linked describes the scenario I had in mind when reporting this issue. Most probably I'll want to add additional logging when CacheFirst interceptor actually reaches network, but yeah, it seems to be quite straightforward knwing I can provide my own decorated interceptor 👌

It's a pretty significant change to the watcher and cache pipeline so didn't want to include that last minute in the 3.0 release

Sure, that wasn't a problem at all. I really appreciate you're willing to include such feature only because I asked for it 😄

@martinbonnin
Copy link
Contributor

This should now be in the snapshots

@martinbonnin martinbonnin added the ✔️ Fixed in SNAPSHOTs The fix has been merged and is available in SNAPSHOTs, and will be available in the next release label Dec 20, 2021
@mateuszkwiecinski
Copy link
Contributor Author

I confirm this works an intented in one of snapshot builds I tried 👍 Thank you 🙏

@martinbonnin
Copy link
Contributor

Fixed in 3.1.0

@martinbonnin martinbonnin removed the ✔️ Fixed in SNAPSHOTs The fix has been merged and is available in SNAPSHOTs, and will be available in the next release label Feb 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants