Convert fetchBehavior to cachePolicy and set URLRequest cachePolicy on GET requests

[samjdurante]

Summary

We noticed that after migrating to Apollo V2, graphQL requests using the .NetworkOnly fetchBehavior seemed to still be returning a cached version.

Turns out that Apollo was not adding the cachePolicy to the URLRequest, so the URLRequests were going out with the default .useProtocolCachePolicy set as the cache policy.

Steps to reproduce

• Make a graphQL request using the .networkOnly fetchBehavior
• Check the URLRequest's cachePolicy
  request.cachePolicy.rawValue logged as 0 (.useProtocolCachePolicy) when it should be 1 (.reloadIgnoringLocalCacheData)

Previous Fix

We encountered this a while ago in Apollo V1, which ended up getting fixed, but the bug seems to have been reintroduced when Apollo switched to fetchBehavior in V2

• Issue: JSONRequest and HTTPRequest ignoring Apollo cachePolicy apollo-ios#3432
• Commit: 511b76a

[netlify]

👷 Deploy request for apollo-ios-docc pending review.

Visit the deploys page to approve it

Name	Link
🔨 Latest commit	4d96403

[calvincestari]

I don't think this matches the intent of CacheAndNetwork but there doesn't seem to be an equivalent though.

[samjdurante]

Maybe .reloadRevalidatingCacheData to match the apollo v1 behavior?

[calvincestari]

That is the value we used in v1 yes although it's still not the correct behaviour; CacheAndNetwork is meant to get something back to the caller asap, and then fetch the latest data from the source. reloadRevalidatingCacheData will refresh any stale data at least but without any cached response if there is available.

@AnthonyMDev - any comment?

[AnthonyMDev]

It's either .reloadIgnoringLocalCacheData or .reloadRevalidatingCacheData. When using the CacheAndNetwork policy, Apollo will still hit the GraphQL NormalizedCache and get that back to the caller ASAP. It will also begin a network fetch every time. This network fetch is where the URLRequest.CachePolicy is considered, when interacting with the URL cache. But either way we've already done a fetch from the normalized cache.

The intent here is that the developer is telling us that even if they have cache data, they do want to do a refetch from the server. I first thought that as a default behavior, .reloadRevalidatingCacheData makes sense here. If the server can verify that the URL cache data is still valid, go ahead and use it.

But the issue I see there is that if the normalized cache data has been mutated since that specific network request was last made (by a local cache mutation or another fetch request that shared fields) , then you could actually get back URL cache data that is stale compared to what you already got from the NormalizedCache

So I think .reloadIgnoringLocalCacheData is correct here.

[calvincestari]

I'd ignored the fact that Apollo cache and URL cache are separate. Mutations are a perfect example of why to use .reloadIgnoringLocalCacheData. Thanks for the input @AnthonyMDev.

@samjdurante, if you get that value updated and look into moving this to GraphQLRequest.createDefaultRequest() we can move this PR forward - thanks.

[AnthonyMDev]

This should all probably live in GraphQLRequest.createDefaultRequest() instead of the JSONRequest.

[AnthonyMDev]

It's either .reloadIgnoringLocalCacheData or .reloadRevalidatingCacheData. When using the CacheAndNetwork policy, Apollo will still hit the GraphQL NormalizedCache and get that back to the caller ASAP. It will also begin a network fetch every time. This network fetch is where the URLRequest.CachePolicy is considered, when interacting with the URL cache. But either way we've already done a fetch from the normalized cache.

The intent here is that the developer is telling us that even if they have cache data, they do want to do a refetch from the server. I first thought that as a default behavior, .reloadRevalidatingCacheData makes sense here. If the server can verify that the URL cache data is still valid, go ahead and use it.

But the issue I see there is that if the normalized cache data has been mutated since that specific network request was last made (by a local cache mutation or another fetch request that shared fields) , then you could actually get back URL cache data that is stale compared to what you already got from the NormalizedCache

So I think .reloadIgnoringLocalCacheData is correct here.

[AnthonyMDev]

I don't think the default protocol cache policy is ever correct?

I'm not sure there should even be a default case here. Instead I think we should use `switch (fetchBehavior.cacheRead, fetchBehavior.networkFetch) and decide how to handle every permutation of these appropriately.