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

JSONRequest and HTTPRequest ignoring Apollo cachePolicy #3432

Closed
PatrickDanino opened this issue Aug 28, 2024 · 9 comments · Fixed by apollographql/apollo-ios-dev#476
Closed
Assignees
Labels
bug Generally incorrect behavior enhancement Issues outlining new things we want to do or things that will make our lives as devs easier networking-stack

Comments

@PatrickDanino
Copy link

Summary

We were surprised to find out that despite setting a cache policy of fetchIgnoringCacheCompletely, we were still being returned cached data when making a query.

We suspect the issue is with JSONRequest and HTTPRequest. Neither appears to be using the cachePolicy passed in as a parameter when generating the URLRequest when toURLRequest() is called.

By default, iOS can cache data using the URLCache and allows this to be configured using the request's cachePolicy property (which is of type URLRequest.CachePolicy, not to be confused with Apollo.CachePolicy). Apollo should be setting the policy on the URLRequest it is creating to an appropriate value based on its own request cache policy.

Version

1.15

Steps to reproduce the behavior

  • Create a query
  • Set the cachePolicy to fetchIgnoringCacheCompletely
  • Repeat the same query

Expected: The server should be hit every time
Current: The data may be cached by URLCache

Logs

We confirmed that calling `URLCache.shared.removeAllCachedResponses()` allowed the next query to be performed.

We then also confirmed that adjusting the `cachePolicy` on the `JSONRequest` also worked.

Anything else?

No response

@PatrickDanino PatrickDanino added bug Generally incorrect behavior needs investigation labels Aug 28, 2024
@PatrickDanino
Copy link
Author

Here's the changes we made to map Apollo's cache policy to URLRequest's

    private var requestCachePolicy: URLRequest.CachePolicy {
        
        switch self.cachePolicy {
        case .returnCacheDataElseFetch:
            .returnCacheDataElseLoad
            
        case .fetchIgnoringCacheData:
            .reloadIgnoringLocalCacheData
            
        case .fetchIgnoringCacheCompletely:
            // URLRequest has no way to avoid storing the response in the cache
            .reloadIgnoringLocalCacheData
            
        case .returnCacheDataDontFetch:
            .returnCacheDataDontLoad
            
        case .returnCacheDataAndFetch:
            // No corresponding behavior exists in URLRequest
            .returnCacheDataElseLoad
        }
    }

@calvincestari
Copy link
Member

Hi @PatrickDanino 👋🏻

We were surprised to find out that despite setting a cache policy of fetchIgnoringCacheCompletely, we were still being returned cached data when making a query.

Are you using the standard POST method for GraphQL requests? POST requests are typically not cached but if you're using GET requests it could help explain this behaviour.

@PatrickDanino
Copy link
Author

Hi @PatrickDanino 👋🏻

We were surprised to find out that despite setting a cache policy of fetchIgnoringCacheCompletely, we were still being returned cached data when making a query.

Are you using the standard POST method for GraphQL requests? POST requests are typically not cached but if you're using GET requests it could help explain this behaviour.

Hi,

We do, but wouldn't you potentially have the same problem if you're using persisted queries? I thought that defaulted to a GET irrespectively.

@calvincestari
Copy link
Member

We have guidance on how to send APQ retries using GET but I don't think GET is the default method for persisted queries. The only difference between a persisted query and 'normal' query is to not send the query body.

@PatrickDanino
Copy link
Author

I'll admit I'm not entirely clear on what is being suggested. Are you saying GET operations are not recommended or supported by Apollo? Even if the default is to use POST, I don't see why or how that should impact GET at all.

Separately, it's not clear to us why the default is to use POST to begin with. We generally use GET in order to ensure that when we do need caching, the right thing happens. In some specific cases, we need to "force" load from the server, so we switch that particular request's caching policy. Support for CDN caching when using POST is inconsistent, at best.

@calvincestari
Copy link
Member

I'll admit I'm not entirely clear on what is being suggested.

Nothing has been suggested yet, I was trying to understand why we're seeing the standard iOS URL caching behaviour which we now know is because of GET requests. I don't know where the assumption that persisted queries defaults to GET comes from.

Are you saying GET operations are not recommended or supported by Apollo?

It is supported. We have documentation stating how to do that.

Separately, it's not clear to us why the default is to use POST to begin with.

GraphQL uses POST because it has a request body, GET requests do not. Therefore when using GET the entire query, including all variables, etc. must be sent as a URL query parameter. Queries can be large which means you're more likely to exceed the maximum length of the URL. If you're using persisted queries this is less of a concern.

We generally use GET in order to ensure that when we do need caching, the right thing happens.

Then you should probably be trusting what the Control-Cache directive is sending back in the response header. This will give you the URL cache behaviour as you're seeing now.

In some specific cases, we need to "force" load from the server, so we switch that particular request's caching policy.

This is the only unresolved issue since there is no way to opt-out of the default iOS URL caching behaviour.

@calvincestari
Copy link
Member

In some specific cases, we need to "force" load from the server, so we switch that particular request's caching policy.

This is the only unresolved issue since there is no way to opt-out of the default iOS URL caching behaviour.

I'll take this to the rest of the team and see what we can do about it.

Copy link
Contributor

Do you have any feedback for the maintainers? Please tell us by taking a one-minute survey. Your responses will help us understand Apollo iOS usage and allow us to serve you better.

@calvincestari
Copy link
Member

The change for this has been merged and will go out in the next release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Generally incorrect behavior enhancement Issues outlining new things we want to do or things that will make our lives as devs easier networking-stack
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants