Per-API Key Caches #74
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
aarsilv
commented
May 29, 2024
src/index.spec.ts
Outdated
| @@ -550,7 +550,6 @@ describe('initialization options', () => { | |||
| let client = await init({ | |||
| apiKey, | |||
| baseUrl, | |||
| assignmentLogger: mockLogger, | |||
| @@ -2,8 +2,14 @@ import { AssignmentCache } from '@eppo/js-client-sdk-common'; | |||
|
|
|||
| import { hasWindowLocalStorage } from './configuration-factory'; | |||
|
|
|||
| // noinspection JSUnusedGlobalSymbols (methods are used by common repository) | |||
There was a problem hiding this comment.
stop Webstorm for yelling at me for unused methods
src/local-storage.spec.ts
Outdated
| beforeEach(() => { | ||
| window.localStorage.clear(); | ||
| }); | ||
|
|
||
| describe('get and set', () => { |
There was a problem hiding this comment.
opted to remove the nested describe()
leoromanovsky
approved these changes
May 29, 2024
| @@ -179,9 +179,15 @@ export async function init(config: IClientConfig): Promise<IEppoClient> { | |||
| EppoJSClient.instance.stopPolling(); | |||
| // Set up assignment logger and cache | |||
| EppoJSClient.instance.setLogger(config.assignmentLogger); | |||
|
|
|||
| // Note that we use the first 8 characters of the API key to create per-API key persistent storages | |||
| const storageKeySuffix = config.apiKey.replace(/\W/g, '').substring(0, 8); | |||
Comment on lines
+188
to
+190
| EppoJSClient.instance.useCustomAssignmentCache( | ||
| new LocalStorageAssignmentCache(storageKeySuffix), | ||
| ); |
There was a problem hiding this comment.
FYI - a factory is needed here as well https://linear.app/eppo/issue/FF-2113/create-chromestorage-assignment-cache
| @@ -49,4 +49,69 @@ describe('LocalStorageAssignmentCache', () => { | |||
| }), | |||
| ).toEqual(false); // this key has not been logged | |||
| }); | |||
|
|
|||
| it('can have independent caches', () => { | |||
| LOCAL_STORAGE_KEY = 'EPPO_LOCAL_STORAGE_ASSIGNMENT_CACHE'; | ||
| private readonly localStorageKey: string; | ||
|
|
||
| public constructor(storageKeySuffix?: string) { |
There was a problem hiding this comment.
did you consider making this a required parameter?
There was a problem hiding this comment.
Thought about it. I suppose I should since we never really use it without one.
There was a problem hiding this comment.
Making this required caught a place I forgot to pass it through! Excellent suggestion!
greghuels
approved these changes
May 29, 2024
aarsilv
force-pushed
the
aaron/ff-2218/cache-outcomes
branch
from
17d61b8 to
011f8c6
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Eppo Internal:
🎟️ Ticket: FF-2230 - JS SDK - local and chrome storage caches should be isolated per-API key
During development, different environments (and thus API keys) may be used on the same device. By isolating our provided persistent storages (Local Storage, Chrome Storage, and Assignment Cache) at the per-API key level, we will prevent an SDK initialized for one environment from loading a cached configuration from a different environment.