ios-client-sdk icon indicating copy to clipboard operation
ios-client-sdk copied to clipboard

feat: Add support for external cache storage

Open nalexn opened this issue 1 year ago • 3 comments

Requirements

  • [x] I have added test coverage for new or changed functionality
  • [x] I have followed the repository's pull request submission guidelines
  • [x] I have validated my changes against all supported platform versions

Related issues

Provide links to any issues in this repository or elsewhere relating to this pull request: #316 Also, provides an alternative to #408 when using LDInMemoryCache

Describe the solution you've provided

This PR adds a new property to the LDConfig file allowing to replace the default cache storage (NSUserDefaults) with either LDInMemoryCache, LDFileCache, or a custom cache user implements by conforming to KeyedValueCaching protocol.

var config = LDConfig(mobileKey: ..., autoEnvAttributes: .disabled)
config.cacheFactory = LDFileCache.factory(encryptionKey: ...)

Describe alternatives you've considered

Provide a clear and concise description of any alternative solutions or features you've considered.

Additional context

  • 100% backward compatible, all inner workings of the library don't change if the LDConfig isn't touched
  • Both LDInMemoryCache and LDFileCache are thread-safe and fully covered with tests
  • LDFileCache supports optional encryption of the file with the AES algorithm
  • LDFileCache supports migration from the NSUserDefaults so there is no flag flicker when updating
  • Writing to file happens from a background queue and is throttled

nalexn avatar Oct 25 '24 18:10 nalexn

Hi @nalexn, thank you for your contribution. We on the SDK team will take a look early next week and provide our thoughts and feedback.

I did take a brief look through just now. My initial reaction is your code is thoughtful, so thank you for that. Some tasks popped into my head that I want to document to come back to during review:

  • During review, examine guarantees around data making it to persistence.
  • Discuss migration pros/cons.

tanderson-ld avatar Oct 25 '24 21:10 tanderson-ld

All the suggested improvements and refactorings totally make sense, but my team is limited on the time we can dedicate to solving our peculiar problem, so we'll likely just use our branch for the time being. The PR can be closed or updated at your discretion.

nalexn avatar Nov 05 '24 13:11 nalexn

All the suggested improvements and refactorings totally make sense, but my team is limited on the time we can dedicate to solving our peculiar problem, so we'll likely just use our branch for the time being. The PR can be closed or updated at your discretion.

Thank you for letting me know. I'll take a look at folding in those changes for you.

keelerm84 avatar Nov 05 '24 18:11 keelerm84