Merge branch 'inlocalstorage_storageAdapter' into inlocalstorage_sessionStorage

Author: EmilianoSanchez

src/storages/inLocalStorage/MySegmentsCacheInLocal.ts

@@ -51,7 +51,7 @@ export class MySegmentsCacheInLocal extends AbstractMySegmentsCacheSync {
 
   getRegisteredSegments(): string[] {
     const registeredSegments: string[] = [];
-    for (let i = 0; i < this.storage.length; i++) {
+    for (let i = 0, len = this.storage.length; i < len; i++) {
       const segmentName = this.keys.extractSegmentName(this.storage.key(i)!);
       if (segmentName) registeredSegments.push(segmentName);
     }

src/storages/inLocalStorage/__tests__/storageAdapter.spec.ts

@@ -0,0 +1,65 @@
+import { storageAdapter } from '../storageAdapter';
+import { loggerMock } from '../../../logger/__tests__/sdkLogger.mock';
+
+
+const syncWrapper = {
+  getItem: jest.fn(() => JSON.stringify({ key1: 'value1' })),
+  setItem: jest.fn(),
+  removeItem: jest.fn(),
+};
+
+const asyncWrapper = {
+  getItem: jest.fn(() => Promise.resolve(JSON.stringify({ key1: 'value1' }))),
+  setItem: jest.fn(() => Promise.resolve()),
+  removeItem: jest.fn(() => Promise.resolve()),
+};
+
+test.each([
+  [syncWrapper],
+  [asyncWrapper],
+])('storageAdapter', async (wrapper) => {
+
+  const storage = storageAdapter(loggerMock, 'prefix', wrapper);
+
+  expect(storage.length).toBe(0);
+
+  // Load cache from storage wrapper
+  await storage.load();
+
+  expect(wrapper.getItem).toHaveBeenCalledWith('prefix');
+  expect(storage.length).toBe(1);
+  expect(storage.key(0)).toBe('key1');
+  expect(storage.getItem('key1')).toBe('value1');
+
+  // Set item
+  storage.setItem('key2', 'value2');
+  expect(storage.getItem('key2')).toBe('value2');
+  expect(storage.length).toBe(2);
+
+  // Remove item
+  storage.removeItem('key1');
+  expect(storage.getItem('key1')).toBe(null);
+  expect(storage.length).toBe(1);
+
+  // Until a till key is set/removed, changes should not be saved/persisted
+  await storage.whenSaved();
+  expect(wrapper.setItem).not.toHaveBeenCalled();
+
+  // When setting a till key, changes should be saved/persisted immediately
+  storage.setItem('.till', '1');
+  expect(storage.length).toBe(2);
+  expect(storage.key(0)).toBe('key2');
+  expect(storage.key(1)).toBe('.till');
+
+  await storage.whenSaved();
+  expect(wrapper.setItem).toHaveBeenCalledWith('prefix', JSON.stringify({ key2: 'value2', '.till': '1' }));
+
+  // When removing a till key, changes should be saved/persisted immediately
+  storage.removeItem('.till');
+
+  await storage.whenSaved();
+  expect(wrapper.setItem).toHaveBeenCalledWith('prefix', JSON.stringify({ key2: 'value2' }));
+
+  await storage.whenSaved();
+  expect(wrapper.setItem).toHaveBeenCalledTimes(2);
+});

src/storages/inLocalStorage/__tests__/wrapper.mock.ts

@@ -4,7 +4,7 @@ import { loggerMock } from '../../../logger/__tests__/sdkLogger.mock';
 
 export const PREFIX = 'SPLITIO';
 
-export function createMemoryStorage(): SplitIO.StorageWrapper {
+export function createMemoryStorage(): SplitIO.AsyncStorageWrapper {
   let cache: Record<string, string> = {};
   return {
     getItem(key: string) {

src/storages/inLocalStorage/index.ts

@@ -19,7 +19,7 @@ import { ILogger } from '../../logger/types';
 import SplitIO from '../../../types/splitio';
 import { storageAdapter } from './storageAdapter';
 
-function validateStorage(log: ILogger, prefix: string, wrapper?: SplitIO.StorageWrapper): StorageAdapter | undefined {
+function validateStorage(log: ILogger, prefix: string, wrapper?: SplitIO.SyncStorageWrapper | SplitIO.AsyncStorageWrapper): StorageAdapter | undefined {
   if (wrapper) {
     if (isValidStorageWrapper(wrapper)) {
       return isWebStorage(wrapper) ?
@@ -72,7 +72,7 @@ export function InLocalStorage(options: SplitIO.InLocalStorageOptions = {}): ISt
       },
 
       destroy() {
-        return storage.save && storage.save();
+        return storage.whenSaved && storage.whenSaved();
       },
 
       // When using shared instantiation with MEMORY we reuse everything but segments (they are customer per key).

src/storages/inLocalStorage/storageAdapter.ts

@@ -7,44 +7,58 @@ function isTillKey(key: string) {
   return key.endsWith('.till');
 }
 
-export function storageAdapter(log: ILogger, prefix: string, wrapper: SplitIO.StorageWrapper): StorageAdapter {
+export function storageAdapter(log: ILogger, prefix: string, wrapper: SplitIO.SyncStorageWrapper | SplitIO.AsyncStorageWrapper): Required<StorageAdapter> {
+  let keys: string[] = [];
   let cache: Record<string, string> = {};
 
-  let connectPromise: Promise<void> | undefined;
-  let disconnectPromise = Promise.resolve();
+  let loadPromise: Promise<void> | undefined;
+  let savePromise = Promise.resolve();
+
+  function _save() {
+    return savePromise = savePromise.then(() => {
+      return Promise.resolve(wrapper.setItem(prefix, JSON.stringify(cache)));
+    }).catch((e) => {
+      log.error(LOG_PREFIX + 'Rejected promise calling wrapper `setItem` method, with error: ' + e);
+    });
+  }
 
   return {
     load() {
-      return connectPromise || (connectPromise = Promise.resolve(wrapper.getItem(prefix)).then((storedCache) => {
+      return loadPromise || (loadPromise = Promise.resolve().then(() => {
+        return wrapper.getItem(prefix);
+      }).then((storedCache) => {
         cache = JSON.parse(storedCache || '{}');
+        keys = Object.keys(cache);
       }).catch((e) => {
-        log.error(LOG_PREFIX + 'Rejected promise calling storage getItem, with error: ' + e);
+        log.error(LOG_PREFIX + 'Rejected promise calling wrapper `getItem` method, with error: ' + e);
       }));
     },
-    save() {
-      return disconnectPromise = disconnectPromise.then(() => {
-        return Promise.resolve(wrapper.setItem(prefix, JSON.stringify(cache))).catch((e) => {
-          log.error(LOG_PREFIX + 'Rejected promise calling storage setItem, with error: ' + e);
-        });
-      });
+    whenSaved() {
+      return savePromise;
     },
 
     get length() {
-      return Object.keys(cache).length;
+      return keys.length;
     },
     getItem(key: string) {
       return cache[key] || null;
     },
     key(index: number) {
-      return Object.keys(cache)[index] || null;
+      return keys[index] || null;
     },
     removeItem(key: string) {
+      const index = keys.indexOf(key);
+      if (index === -1) return;
+      keys.splice(index, 1);
       delete cache[key];
-      if (isTillKey(key)) this.save!();
+
+      if (isTillKey(key)) _save();
     },
     setItem(key: string, value: string) {
+      if (keys.indexOf(key) === -1) keys.push(key);
       cache[key] = value;
-      if (isTillKey(key)) this.save!();
+
+      if (isTillKey(key)) _save();
     }
   };
 }

src/storages/types.ts

@@ -11,11 +11,11 @@ import { ISettings } from '../types';
 export interface StorageAdapter {
   // Methods to support async storages
   load?: () => Promise<void>;
-  save?: () => Promise<void>;
+  whenSaved?: () => Promise<void>;
   // Methods based on https://developer.mozilla.org/en-US/docs/Web/API/Storage
   readonly length: number;
-  getItem(key: string): string | null;
   key(index: number): string | null;
+  getItem(key: string): string | null;
   removeItem(key: string): void;
   setItem(key: string, value: string): void;
 }

types/splitio.d.ts

@@ -458,19 +458,34 @@ interface IClientSideSyncSharedSettings extends IClientSideSharedSettings, ISync
  */
 declare namespace SplitIO {
 
-  interface StorageWrapper {
+  interface SyncStorageWrapper {
     /**
-     * Returns a promise that resolves to the current value associated with the given key, or null if the given key does not exist.
+     * Returns the value associated with the given key, or null if the key does not exist.
      */
-    getItem(key: string): Promise<string | null> | string | null;
+    getItem(key: string): string | null;
     /**
-     * Returns a promise that resolves when the value of the pair identified by key is set to value, creating a new key/value pair if none existed for key previously.
+     * Sets the value for the given key, creating a new key/value pair if key does not exist.
      */
-    setItem(key: string, value: string): Promise<void> | void;
+    setItem(key: string, value: string): void;
     /**
-     * Returns a promise that resolves when the key/value pair with the given key is removed, if a key/value pair with the given key exists.
+     * Removes the key/value pair for the given key, if the key exists.
      */
-    removeItem(key: string): Promise<void> | void;
+    removeItem(key: string): void;
+  }
+
+  interface AsyncStorageWrapper {
+    /**
+     * Returns a promise that resolves to the value associated with the given key, or null if the key does not exist.
+     */
+    getItem(key: string): Promise<string | null>;
+    /**
+     * Returns a promise that resolves when the value of the pair identified by key is set to value, creating a new key/value pair if key does not exist.
+     */
+    setItem(key: string, value: string): Promise<void>;
+    /**
+     * Returns a promise that resolves when the key/value pair for the given key is removed, if the key exists.
+     */
+    removeItem(key: string): Promise<void>;
   }
 
   /**
@@ -992,7 +1007,7 @@ declare namespace SplitIO {
      *
      * @defaultValue `window.localStorage`
      */
-    wrapper?: StorageWrapper;
+    wrapper?: SyncStorageWrapper | AsyncStorageWrapper;
   }
   /**
    * Storage for asynchronous (consumer) SDK.
@@ -1338,7 +1353,7 @@ declare namespace SplitIO {
        *
        * @defaultValue `window.localStorage`
        */
-      wrapper?: StorageWrapper;
+      wrapper?: SyncStorageWrapper | AsyncStorageWrapper;
     };
   }
   /**