Make anon id mandatory. Simplify API, remove refresh #245
Make anon id mandatory. Simplify API, remove refresh #245wzieba merged 11 commits intoexperiments_public_contractfrom
refresh #245Conversation
This commit adds a new class: `CacheDto` that saves previously saved data with addition to `anonymousId`. This additional field will be used for cache validation.
The `initialize` method now takes a mandatory `anonymousId` parameter and is responsible for setting the anonymous identifier and triggering the initial assignment refresh and cache validation if `anonymousId` is different
I came to a conclusion, that `refresh` is not needed for the client: SDK should on its own manage the state of the cache and not delegate additional responsibility to SDK consumer. The cache is refreshed whenever needed during initialization
As SDK doesn't offer refreshing anymore, we don't need to use internal-only `RefreshStrategy`. Refactor seems successful as unit tests are still passing without any changes
If ExPlat was not initialized, log or throw an exception for quicker debugging
It doesn't have to be exposed to SDK consumers
| private fun getAssignments(): Assignments? = repository.getCached() | ||
|
|
||
| private fun getAssignments(refreshStrategy: RefreshStrategy): Assignments? { | ||
| private fun invalidateCache(): Assignments? { |
There was a problem hiding this comment.
Not really clear to me why invalidateCache() may return cached assignments?
I mean, even if they're not stale, a function to invalidate them should force (now or on the next fetch of the cache) the cache creation, no? 🤔
There was a problem hiding this comment.
Of course you're right, thank you for spotting this 🙌 Addressed in 3dd77fa
|
|
||
| override fun configure(anonymousId: String?) { | ||
| clear() | ||
| override fun initialize(anonymousId: String) { |
There was a problem hiding this comment.
💭
Not for now, and just thinking out loud (and maybe missing more context): perhaps another approach for the API could be to have a builder class (or even a static factory method on the interface?) that will take the anonymousId and initialize the classes, so that they wouldn't be able for a client to be create these classes without properly initializing the SDK.
There was a problem hiding this comment.
Oh I see that's kinda what VariationsRepository.create() does? Can't it also take the anonymousId? 🤔
There was a problem hiding this comment.
Oh I see that's kinda what VariationsRepository.create() does? Can't it also take the anonymousId? 🤔
Not really, because user can change during the session (logout/sign in). VariationsRepository is meant to be a singleton, so VariationsRepository#create will be called once for the app lifecycle.
2 participants
Description
After learning that "anon id" is a necessary piece when requesting experiments assignments (internal source: p1725007206945009-slack-C7HH3V5AS), I've decided to update the public contract to reflect this change and inform SDK consumers early, that anon id is necessary (
initialization) rather than optional (configure)Furthermore, this PR removes
refreshmethod. To my understanding, this was leaking implementation detail. We shouldn't bother SDK consumers when and how cache should be updated - the SDK should handle its own cache.How to review
I recommend checking this PR commit by commit and check commit descriptions - I've tried to add more context for each change there and tell a story of changes.
How to test
The changes are covered with automated unit tests. You can run an example app and follow instructions from #241 but it's not necessary - especially as this PR targets a feature branch.