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

Split/Hybrid Caching between InMemory and SQL review and thoughts #3378

Open
jaikumar-b opened this issue May 9, 2024 · 2 comments
Open
Labels
feature New addition or enhancement to existing solutions

Comments

@jaikumar-b
Copy link

jaikumar-b commented May 9, 2024

Use case

Howdy folks!

The topic of hybrid caching has been bought up multiple times and seems like you guys don't have it on your roadmap anytime soon. Gonna take a stab at this and wanted get some feedback on an idea that can help folks interested in this to get a workable solution until you guys come up with a first class support for hybrid caching or using cache control headers

#3275
#1467

Background

Would like to create an API at the query level that developers can use to define the storage policy of the query when initializing an instance like so

 // or .allowed like https://developer.apple.com/documentation/foundation/urlcache/storagepolicy
let query = HeroQuery(fetchPolicy: .returnCacheAndFetch).storagePolicy(.inMemoryAndDisk)

This is different than the Kotlin version of Apollo which uses cache control headers but I somehow prefer this over the Kotlin api if the storage policy is exclusively defined on the client side.

One hurdle in implementation of a custom normalized cache based on the policy defined by query is that there is no way to associate normalized cache methods with the query's storage policy so that the custom normalized cache implementation can make decisions according to the policies.

Describe the solution you'd like

Proposal

Changes for reference https://github.com/apollographql/apollo-ios/pull/3377/files

Noticed that we have a contextIdentifier which can be instantiated for each fetch operation, propagating that identifier all the way to the caching layer allows for extending the functionality of existing InMemory and SQLite implementations Apollo has by creating wrapper implementations. These wrapper implementations can have a StoragePolicyResolver which maintains the policy vs identifier in memory till the request is complete from network chain.

I was able to create a NormalizedCacheChain object that accepts implementations of NormalizedCache.

guard let sqliteCache = SQLiteNormalizedCacheFactory().makeNormalizedCache(name: "apollo_db.sqlite") else { return InMemoryNormalizedCache() }
 return NormalizedCacheChain(normalizedCaches: [
      InMemoryNormalizedChainCache(),
     SQLiteChainedNormalizedCache(sqlite: sqliteCache)
])

Probably can extend more cache implementations as needed by the application.

Questions

Any feedback on the solution?
Could this be considered something that we can add to the SDK, considering its non-breaking
NOTE - Have this built out for older versions due to familiarity, seems like can be done for 1.x versions as well

@jaikumar-b jaikumar-b added the feature New addition or enhancement to existing solutions label May 9, 2024
@AnthonyMDev
Copy link
Contributor

Hi there @jaikumar-b. Thanks so much for starting this conversation. You're right that we don't have it on our roadmap to build chained caching right now. But we aren't against you implementing it yourself, and if there are minor, non-breaking changes we can make to the library to help facilitate that, we're open to the idea!

I'm going to look at the closed PR you made and make some inline comments and suggestions there. I think that will help us point this in the right direction and if you want to make another PR after the conversation, we'll happily review it!

I'm assuming you closed that PR simply because you were working against an older legacy version of the client. Would you be wanting to migrate to 1.0 and make the additions to the 1.0 APIs? If you're going to just modify the older client, I suggest you do that on a fork of your own, since we aren't actively adding features to those versions anymore.

@jaikumar-b
Copy link
Author

ons to the 1.0 APIs? If you're going to just modify the older client, I suggest you do that on a fork of your own, since we aren't actively adding features to those v

Hey @AnthonyMDev! Thanks for all the initial comments and great to hear that you're open to making non-breaking changes to enable this and also appreciate the quick review. We won't be migrating to 1.0 anytime soon due to the amount of effort needed although it's in our plans in the coming quarters. But willing to add these changes to 1.0 and maintain a version internally for the legacy version until we migrate over. Have started taking a look at 1.0 will try and take your feedback from the close PR and open a new on the dev repo this week

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New addition or enhancement to existing solutions
Projects
None yet
Development

No branches or pull requests

2 participants