Convert IConfigurationStore to IAsyncWriteSyncReadStore (FF-1980) (#59)

* Convert implementation of ConfigurationStore to support async writing (FF-1980)

* add pino pretty and prefix logs with `[Eppo SDK]

* remove one level of nesting

* return T | null

* Revert "add pino pretty and prefix logs with `[Eppo SDK]"

This reverts commit b9db61a.

* log manually for now

* v3.0.3

Author: leoromanovsky

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@eppo/js-client-sdk-common",
-  "version": "3.0.2",
+  "version": "3.0.3",
   "description": "Eppo SDK for client-side JavaScript applications (base for both web and react native)",
   "main": "dist/index.js",
   "files": [

src/application-logger.ts

@@ -1,5 +1,7 @@
 import pino from 'pino';
 
+export const loggerPrefix = '[Eppo SDK]';
+
 // Create a Pino logger instance
 export const logger = pino({
   level: process.env.NODE_ENV === 'production' ? 'warn' : 'info',

src/client/eppo-client.spec.ts

@@ -11,40 +11,16 @@ import {
   validateTestAssignments,
 } from '../../test/testHelpers';
 import { IAssignmentLogger } from '../assignment-logger';
-import { IConfigurationStore } from '../configuration-store';
+import { IConfigurationStore } from '../configuration-store/configuration-store';
+import { MemoryOnlyConfigurationStore } from '../configuration-store/memory.store';
 import { MAX_EVENT_QUEUE_SIZE, POLL_INTERVAL_MS, POLL_JITTER_PCT } from '../constants';
 import FlagConfigurationRequestor from '../flag-configuration-requestor';
 import FetchHttpClient from '../http-client';
-import { Flag, VariationType } from '../interfaces';
+import { Flag, ObfuscatedFlag, VariationType } from '../interfaces';
 
 import EppoClient, { FlagConfigurationRequestParameters, checkTypeMatch } from './eppo-client';
 
-class TestConfigurationStore implements IConfigurationStore {
-  private store: Record<string, string> = {};
-  private _isInitialized = false;
-
-  public get<T>(key: string): T {
-    const rval = this.store[key];
-    return rval ? JSON.parse(rval) : null;
-  }
-
-  public setEntries<T>(entries: Record<string, T>) {
-    Object.entries(entries).forEach(([key, val]) => {
-      this.store[key] = JSON.stringify(val);
-    });
-    this._isInitialized = true;
-  }
-
-  public getKeys(): string[] {
-    return Object.keys(this.store);
-  }
-
-  public isInitialized(): boolean {
-    return this._isInitialized;
-  }
-}
-
-export async function init(configurationStore: IConfigurationStore) {
+export async function init(configurationStore: IConfigurationStore<Flag | ObfuscatedFlag>) {
   const httpClient = new FetchHttpClient(
     'http://127.0.0.1:4000',
     {
@@ -68,7 +44,7 @@ describe('EppoClient E2E test', () => {
       json: () => Promise.resolve(ufc),
     });
   }) as jest.Mock;
-  const storage = new TestConfigurationStore();
+  const storage = new MemoryOnlyConfigurationStore<Flag | ObfuscatedFlag>();
 
   beforeAll(async () => {
     await init(storage);
@@ -312,7 +288,7 @@ describe('EppoClient E2E test', () => {
   });
 
   it('returns null if getStringAssignment was called for the subject before any UFC was loaded', () => {
-    const localClient = new EppoClient(new TestConfigurationStore());
+    const localClient = new EppoClient(new MemoryOnlyConfigurationStore());
     expect(localClient.getStringAssignment(flagKey, 'subject-1', {}, 'hello world')).toEqual(
       'hello world',
     );
@@ -580,7 +556,7 @@ describe('EppoClient E2E test', () => {
 
   describe('Eppo Client constructed with configuration request parameters', () => {
     let client: EppoClient;
-    let thisStorage: IConfigurationStore;
+    let thisStorage: IConfigurationStore<Flag | ObfuscatedFlag>;
     let requestConfiguration: FlagConfigurationRequestParameters;
 
     const flagKey = 'numeric_flag';
@@ -606,7 +582,7 @@ describe('EppoClient E2E test', () => {
         sdkVersion: '1.0.0',
       };
 
-      thisStorage = new TestConfigurationStore();
+      thisStorage = new MemoryOnlyConfigurationStore();
 
       // We only want to fake setTimeout() and clearTimeout()
       jest.useFakeTimers({

src/client/eppo-client.ts

@@ -5,7 +5,7 @@ import {
   NonExpiringInMemoryAssignmentCache,
 } from '../assignment-cache';
 import { IAssignmentEvent, IAssignmentLogger } from '../assignment-logger';
-import { IConfigurationStore } from '../configuration-store';
+import { IConfigurationStore } from '../configuration-store/configuration-store';
 import {
   BASE_URL as DEFAULT_BASE_URL,
   DEFAULT_INITIAL_CONFIG_REQUEST_RETRIES,
@@ -116,13 +116,13 @@ export default class EppoClient implements IEppoClient {
   private isGracefulFailureMode = true;
   private isObfuscated = false;
   private assignmentCache: AssignmentCache<Cacheable> | undefined;
-  private configurationStore: IConfigurationStore;
+  private configurationStore: IConfigurationStore<Flag | ObfuscatedFlag>;
   private configurationRequestParameters: FlagConfigurationRequestParameters | undefined;
   private requestPoller: IPoller | undefined;
   private evaluator: Evaluator;
 
   constructor(
-    configurationStore: IConfigurationStore,
+    configurationStore: IConfigurationStore<Flag | ObfuscatedFlag>,
     configurationRequestParameters?: FlagConfigurationRequestParameters,
     obfuscated = false,
   ) {
@@ -387,7 +387,9 @@ export default class EppoClient implements IEppoClient {
   }
 
   private getObfuscatedFlag(flagKey: string): Flag | null {
-    const flag: ObfuscatedFlag | null = this.configurationStore.get(getMD5Hash(flagKey));
+    const flag: ObfuscatedFlag | null = this.configurationStore.get(
+      getMD5Hash(flagKey),
+    ) as ObfuscatedFlag;
     return flag ? decodeFlag(flag) : null;
   }
 

src/configuration-store.ts

@@ -0,0 +1,45 @@
+/**
+ * ConfigurationStore interface
+ *
+ * The interface guides implementation
+ * of a policy for handling a mixture of async and sync storage.
+ *
+ * The goal is to support remote API responses to be written to the sync and async store,
+ * while also supporting reading from the sync store to maintain public SDK APIs.
+ *
+ * Implementation is handled in upstream libraries to best support their use case, some ideas:
+ *
+ * - Javascript frontend:
+ *   - SyncStore: backed by localStorage
+ *   - AsyncStore: backed by IndexedDB or chrome.storage.local
+ *
+ * - NodeJS backend:
+ *   - SyncStore: backed by LRU cache
+ *   - AsyncStore: none
+ *
+ * The policy choices surrounding the use of one or more underlying storages are
+ * implementation specific and handled upstream.
+ */
+export interface IConfigurationStore<T> {
+  servingStore: ISyncStore<T>;
+  persistentStore: IAsyncStore<T> | null;
+
+  init(): Promise<void>;
+  get(key: string): T | null;
+  getKeys(): string[];
+  isInitialized(): boolean;
+  setEntries(entries: Record<string, T>): Promise<void>;
+}
+
+export interface ISyncStore<T> {
+  get(key: string): T | null;
+  getKeys(): string[];
+  isInitialized(): boolean;
+  setEntries(entries: Record<string, T>): void;
+}
+
+export interface IAsyncStore<T> {
+  isInitialized(): boolean;
+  getEntries(): Promise<Record<string, T>>;
+  setEntries(entries: Record<string, T>): Promise<void>;
+}

src/configuration-store/configuration-store.ts

@@ -0,0 +1,71 @@
+import { IAsyncStore, ISyncStore } from './configuration-store';
+import { HybridConfigurationStore } from './hybrid.store';
+
+describe('HybridConfigurationStore', () => {
+  let syncStoreMock: ISyncStore<string>;
+  let asyncStoreMock: IAsyncStore<string>;
+  let store: HybridConfigurationStore<string>;
+
+  beforeEach(() => {
+    syncStoreMock = {
+      get: jest.fn(),
+      getKeys: jest.fn(),
+      isInitialized: jest.fn(),
+      setEntries: jest.fn(),
+    };
+
+    asyncStoreMock = {
+      getEntries: jest.fn(),
+      isInitialized: jest.fn(),
+      setEntries: jest.fn(),
+    };
+
+    store = new HybridConfigurationStore(syncStoreMock, asyncStoreMock);
+  });
+
+  describe('init', () => {
+    it('should initialize the serving store with entries from the persistent store if the persistent store is initialized', async () => {
+      const entries = { key1: 'value1', key2: 'value2' };
+      (asyncStoreMock.isInitialized as jest.Mock).mockReturnValue(true);
+      (asyncStoreMock.getEntries as jest.Mock).mockResolvedValue(entries);
+
+      await store.init();
+
+      expect(syncStoreMock.setEntries).toHaveBeenCalledWith(entries);
+    });
+  });
+
+  describe('isInitialized', () => {
+    it('should return true if both stores are initialized', () => {
+      (syncStoreMock.isInitialized as jest.Mock).mockReturnValue(true);
+      (asyncStoreMock.isInitialized as jest.Mock).mockReturnValue(true);
+
+      expect(store.isInitialized()).toBe(true);
+    });
+
+    it('should return false if either store is not initialized', () => {
+      (syncStoreMock.isInitialized as jest.Mock).mockReturnValue(false);
+      (asyncStoreMock.isInitialized as jest.Mock).mockReturnValue(true);
+
+      expect(store.isInitialized()).toBe(false);
+    });
+  });
+
+  describe('setEntries', () => {
+    it('should set entries in both stores if the persistent store is present', async () => {
+      const entries = { key1: 'value1', key2: 'value2' };
+      await store.setEntries(entries);
+
+      expect(asyncStoreMock.setEntries).toHaveBeenCalledWith(entries);
+      expect(syncStoreMock.setEntries).toHaveBeenCalledWith(entries);
+    });
+
+    it('should only set entries in the serving store if the persistent store is null', async () => {
+      const mixedStoreWithNull = new HybridConfigurationStore(syncStoreMock, null);
+      const entries = { key1: 'value1', key2: 'value2' };
+      await mixedStoreWithNull.setEntries(entries);
+
+      expect(syncStoreMock.setEntries).toHaveBeenCalledWith(entries);
+    });
+  });
+});

src/configuration-store/hybrid.store.spec.ts

@@ -0,0 +1,61 @@
+import { logger, loggerPrefix } from '../application-logger';
+
+import { IAsyncStore, IConfigurationStore, ISyncStore } from './configuration-store';
+
+export class HybridConfigurationStore<T> implements IConfigurationStore<T> {
+  servingStore: ISyncStore<T>;
+  persistentStore: IAsyncStore<T> | null;
+
+  constructor(servingStore: ISyncStore<T>, persistentStore: IAsyncStore<T> | null) {
+    this.servingStore = servingStore;
+    this.persistentStore = persistentStore;
+  }
+
+  /**
+   * Initialize the configuration store by loading the entries from the persistent store into the serving store.
+   */
+  async init(): Promise<void> {
+    if (!this.persistentStore) {
+      return;
+    }
+
+    if (!this.persistentStore.isInitialized()) {
+      /**
+       * The initial remote request to the remote API failed
+       * or never happened because we are in the cool down period.
+       *
+       * Shows a log message that the assignments served from the serving store
+       * may be stale.
+       */
+      logger.warn(
+        `${loggerPrefix} Persistent store is not initialized from remote configuration. Serving assignments that may be stale.`,
+      );
+    }
+
+    const entries = await this.persistentStore.getEntries();
+    this.servingStore.setEntries(entries);
+  }
+
+  public isInitialized(): boolean {
+    return this.servingStore.isInitialized() && (this.persistentStore?.isInitialized() ?? true);
+  }
+
+  public get(key: string): T | null {
+    if (!this.servingStore.isInitialized()) {
+      logger.warn(`${loggerPrefix} getting a value from a ServingStore that is not initialized.`);
+    }
+    return this.servingStore.get(key);
+  }
+
+  public getKeys(): string[] {
+    return this.servingStore.getKeys();
+  }
+
+  public async setEntries(entries: Record<string, T>): Promise<void> {
+    if (this.persistentStore) {
+      // Persistence store is now initialized and should mark itself accordingly.
+      await this.persistentStore.setEntries(entries);
+    }
+    this.servingStore.setEntries(entries);
+  }
+}

src/configuration-store/hybrid.store.ts

@@ -0,0 +1,40 @@
+import { MemoryOnlyConfigurationStore } from './memory.store';
+
+describe('MemoryOnlyConfigurationStore', () => {
+  let memoryStore: MemoryOnlyConfigurationStore<string>;
+
+  beforeEach(() => {
+    memoryStore = new MemoryOnlyConfigurationStore();
+  });
+
+  it('should initialize without any entries', () => {
+    expect(memoryStore.isInitialized()).toBe(false);
+    expect(memoryStore.getKeys()).toEqual([]);
+  });
+
+  it('should return null for non-existent keys', () => {
+    expect(memoryStore.get('nonexistent')).toBeNull();
+  });
+
+  it('should allow setting and retrieving entries', async () => {
+    await memoryStore.setEntries({ key1: 'value1', key2: 'value2' });
+    expect(memoryStore.get('key1')).toBe('value1');
+    expect(memoryStore.get('key2')).toBe('value2');
+  });
+
+  it('should report initialized after setting entries', async () => {
+    await memoryStore.setEntries({ key1: 'value1' });
+    expect(memoryStore.isInitialized()).toBe(true);
+  });
+
+  it('should return all keys', async () => {
+    await memoryStore.setEntries({ key1: 'value1', key2: 'value2', key3: 'value3' });
+    expect(memoryStore.getKeys()).toEqual(['key1', 'key2', 'key3']);
+  });
+
+  it('should overwrite existing entries', async () => {
+    await memoryStore.setEntries({ key1: 'value1' });
+    await memoryStore.setEntries({ key1: 'newValue1' });
+    expect(memoryStore.get('key1')).toBe('newValue1');
+  });
+});