Merge pull request #380 from splitio/cache_expiration_validateExpiration_method

EmilianoSanchez · web-flow · commit 4069c2401930 · 2024-12-26T17:07:20.000-03:00
[Cache expiration] Update `validateCache` function with `expirationDays` and `clearOnInit` options
diff --git a/CHANGES.txt b/CHANGES.txt
@@ -1,4 +1,7 @@
-2.0.3 (December 29, 2024)
+2.1.0 (January XX, 2025)
+ - Added two new configuration options for the SDK storage in browsers when using storage type `LOCALSTORAGE`:
+    - `storage.expirationDays` to specify the validity period of the rollout cache.
+    - `storage.clearOnInit` to clear the rollout cache on SDK initialization.
  - Bugfixing - Properly handle rejected promises when using targeting rules with segment matchers in consumer modes (e.g., Redis and Pluggable storages).
 
 2.0.2 (December 3, 2024)
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@splitsoftware/splitio-commons",
-  "version": "2.0.2",
+  "version": "2.1.0-rc.0",
   "description": "Split JavaScript SDK common components",
   "main": "cjs/index.js",
   "module": "esm/index.js",
diff --git a/src/storages/KeyBuilderCS.ts b/src/storages/KeyBuilderCS.ts
@@ -43,6 +43,10 @@ export class KeyBuilderCS extends KeyBuilder implements MySegmentsKeyBuilder {
   buildTillKey() {
     return `${this.prefix}.${this.matchingKey}.segments.till`;
   }
+
+  buildLastClear() {
+    return `${this.prefix}.lastClear`;
+  }
 }
 
 export function myLargeSegmentsKeyBuilder(prefix: string, matchingKey: string): MySegmentsKeyBuilder {
diff --git a/src/storages/dataLoader.ts b/src/storages/dataLoader.ts
@@ -1,7 +1,9 @@
 import { PreloadedData } from '../types';
-import { DEFAULT_CACHE_EXPIRATION_IN_MILLIS } from '../utils/constants/browser';
 import { DataLoader, ISegmentsCacheSync, ISplitsCacheSync } from './types';
 
+// This value might be eventually set via a config parameter
+const DEFAULT_CACHE_EXPIRATION_IN_MILLIS = 864000000; // 10 days
+
 /**
  * Factory of client-side storage loader
  *
diff --git a/src/storages/inLocalStorage/SplitsCacheInLocal.ts b/src/storages/inLocalStorage/SplitsCacheInLocal.ts
@@ -71,8 +71,6 @@ export class SplitsCacheInLocal extends AbstractSplitsCacheSync {
    * We cannot simply call `localStorage.clear()` since that implies removing user items from the storage.
    */
   clear() {
-    this.log.info(LOG_PREFIX + 'Flushing Splits data from localStorage');
-
     // collect item keys
     const len = localStorage.length;
     const accum = [];
diff --git a/src/storages/inLocalStorage/__tests__/validateCache.spec.ts b/src/storages/inLocalStorage/__tests__/validateCache.spec.ts
@@ -0,0 +1,125 @@
+import { validateCache } from '../validateCache';
+
+import { KeyBuilderCS } from '../../KeyBuilderCS';
+import { fullSettings } from '../../../utils/settingsValidation/__tests__/settings.mocks';
+import { SplitsCacheInLocal } from '../SplitsCacheInLocal';
+import { nearlyEqual } from '../../../__tests__/testUtils';
+import { MySegmentsCacheInLocal } from '../MySegmentsCacheInLocal';
+
+const FULL_SETTINGS_HASH = '404832b3';
+
+describe('validateCache', () => {
+  const keys = new KeyBuilderCS('SPLITIO', 'user');
+  const logSpy = jest.spyOn(fullSettings.log, 'info');
+  const segments = new MySegmentsCacheInLocal(fullSettings.log, keys);
+  const largeSegments = new MySegmentsCacheInLocal(fullSettings.log, keys);
+  const splits = new SplitsCacheInLocal(fullSettings, keys);
+
+  jest.spyOn(splits, 'clear');
+  jest.spyOn(splits, 'getChangeNumber');
+  jest.spyOn(segments, 'clear');
+  jest.spyOn(largeSegments, 'clear');
+
+  beforeEach(() => {
+    jest.clearAllMocks();
+    localStorage.clear();
+  });
+
+  test('if there is no cache, it should return false', () => {
+    expect(validateCache({}, fullSettings, keys, splits, segments, largeSegments)).toBe(false);
+
+    expect(logSpy).not.toHaveBeenCalled();
+
+    expect(splits.clear).not.toHaveBeenCalled();
+    expect(segments.clear).not.toHaveBeenCalled();
+    expect(largeSegments.clear).not.toHaveBeenCalled();
+    expect(splits.getChangeNumber).toHaveBeenCalledTimes(1);
+
+    expect(localStorage.getItem(keys.buildHashKey())).toBe(FULL_SETTINGS_HASH);
+    expect(localStorage.getItem(keys.buildLastClear())).toBeNull();
+  });
+
+  test('if there is cache and it must not be cleared, it should return true', () => {
+    localStorage.setItem(keys.buildSplitsTillKey(), '1');
+    localStorage.setItem(keys.buildHashKey(), FULL_SETTINGS_HASH);
+
+    expect(validateCache({}, fullSettings, keys, splits, segments, largeSegments)).toBe(true);
+
+    expect(logSpy).not.toHaveBeenCalled();
+
+    expect(splits.clear).not.toHaveBeenCalled();
+    expect(segments.clear).not.toHaveBeenCalled();
+    expect(largeSegments.clear).not.toHaveBeenCalled();
+    expect(splits.getChangeNumber).toHaveBeenCalledTimes(1);
+
+    expect(localStorage.getItem(keys.buildHashKey())).toBe(FULL_SETTINGS_HASH);
+    expect(localStorage.getItem(keys.buildLastClear())).toBeNull();
+  });
+
+  test('if there is cache and it has expired, it should clear cache and return false', () => {
+    localStorage.setItem(keys.buildSplitsTillKey(), '1');
+    localStorage.setItem(keys.buildHashKey(), FULL_SETTINGS_HASH);
+    localStorage.setItem(keys.buildLastUpdatedKey(), Date.now() - 1000 * 60 * 60 * 24 * 2 + ''); // 2 days ago
+
+    expect(validateCache({ expirationDays: 1 }, fullSettings, keys, splits, segments, largeSegments)).toBe(false);
+
+    expect(logSpy).toHaveBeenCalledWith('storage:localstorage: Cache expired more than 1 days ago. Cleaning up cache');
+
+    expect(splits.clear).toHaveBeenCalledTimes(1);
+    expect(segments.clear).toHaveBeenCalledTimes(1);
+    expect(largeSegments.clear).toHaveBeenCalledTimes(1);
+
+    expect(localStorage.getItem(keys.buildHashKey())).toBe(FULL_SETTINGS_HASH);
+    expect(nearlyEqual(parseInt(localStorage.getItem(keys.buildLastClear()) as string), Date.now())).toBe(true);
+  });
+
+  test('if there is cache and its hash has changed, it should clear cache and return false', () => {
+    localStorage.setItem(keys.buildSplitsTillKey(), '1');
+    localStorage.setItem(keys.buildHashKey(), FULL_SETTINGS_HASH);
+
+    expect(validateCache({}, { ...fullSettings, core: { ...fullSettings.core, authorizationKey: 'another' } }, keys, splits, segments, largeSegments)).toBe(false);
+
+    expect(logSpy).toHaveBeenCalledWith('storage:localstorage: SDK key, flags filter criteria, or flags spec version has changed. Cleaning up cache');
+
+    expect(splits.clear).toHaveBeenCalledTimes(1);
+    expect(segments.clear).toHaveBeenCalledTimes(1);
+    expect(largeSegments.clear).toHaveBeenCalledTimes(1);
+
+    expect(localStorage.getItem(keys.buildHashKey())).toBe('aa4877c2');
+    expect(nearlyEqual(parseInt(localStorage.getItem(keys.buildLastClear()) as string), Date.now())).toBe(true);
+  });
+
+  test('if there is cache and clearOnInit is true, it should clear cache and return false', () => {
+    // Older cache version (without last clear)
+    localStorage.setItem(keys.buildSplitsTillKey(), '1');
+    localStorage.setItem(keys.buildHashKey(), FULL_SETTINGS_HASH);
+
+    expect(validateCache({ clearOnInit: true }, fullSettings, keys, splits, segments, largeSegments)).toBe(false);
+
+    expect(logSpy).toHaveBeenCalledWith('storage:localstorage: clearOnInit was set and cache was not cleared in the last 24 hours. Cleaning up cache');
+
+    expect(splits.clear).toHaveBeenCalledTimes(1);
+    expect(segments.clear).toHaveBeenCalledTimes(1);
+    expect(largeSegments.clear).toHaveBeenCalledTimes(1);
+
+    expect(localStorage.getItem(keys.buildHashKey())).toBe(FULL_SETTINGS_HASH);
+    const lastClear = localStorage.getItem(keys.buildLastClear());
+    expect(nearlyEqual(parseInt(lastClear as string), Date.now())).toBe(true);
+
+    // If cache is cleared, it should not clear again until a day has passed
+    logSpy.mockClear();
+    localStorage.setItem(keys.buildSplitsTillKey(), '1');
+    expect(validateCache({ clearOnInit: true }, fullSettings, keys, splits, segments, largeSegments)).toBe(true);
+    expect(logSpy).not.toHaveBeenCalled();
+    expect(localStorage.getItem(keys.buildLastClear())).toBe(lastClear); // Last clear should not have changed
+
+    // If a day has passed, it should clear again
+    localStorage.setItem(keys.buildLastClear(), (Date.now() - 1000 * 60 * 60 * 24 - 1) + '');
+    expect(validateCache({ clearOnInit: true }, fullSettings, keys, splits, segments, largeSegments)).toBe(false);
+    expect(logSpy).toHaveBeenCalledWith('storage:localstorage: clearOnInit was set and cache was not cleared in the last 24 hours. Cleaning up cache');
+    expect(splits.clear).toHaveBeenCalledTimes(2);
+    expect(segments.clear).toHaveBeenCalledTimes(2);
+    expect(largeSegments.clear).toHaveBeenCalledTimes(2);
+    expect(nearlyEqual(parseInt(localStorage.getItem(keys.buildLastClear()) as string), Date.now())).toBe(true);
+  });
+});
diff --git a/src/storages/inLocalStorage/index.ts b/src/storages/inLocalStorage/index.ts
@@ -14,15 +14,12 @@ import { shouldRecordTelemetry, TelemetryCacheInMemory } from '../inMemory/Telem
 import { UniqueKeysCacheInMemoryCS } from '../inMemory/UniqueKeysCacheInMemoryCS';
 import { getMatching } from '../../utils/key';
 import { validateCache } from './validateCache';
-
-export interface InLocalStorageOptions {
-  prefix?: string
-}
+import SplitIO from '../../../types/splitio';
 
 /**
  * InLocal storage factory for standalone client-side SplitFactory
  */
-export function InLocalStorage(options: InLocalStorageOptions = {}): IStorageSyncFactory {
+export function InLocalStorage(options: SplitIO.InLocalStorageOptions = {}): IStorageSyncFactory {
 
   const prefix = validatePrefix(options.prefix);
 
@@ -53,7 +50,7 @@ export function InLocalStorage(options: InLocalStorageOptions = {}): IStorageSyn
       uniqueKeys: impressionsMode === NONE ? new UniqueKeysCacheInMemoryCS() : undefined,
 
       validateCache() {
-        return validateCache(settings, keys, splits);
+        return validateCache(options, settings, keys, splits, segments, largeSegments);
       },
 
       destroy() { },
diff --git a/src/storages/inLocalStorage/validateCache.ts b/src/storages/inLocalStorage/validateCache.ts
@@ -1,20 +1,32 @@
 import { ISettings } from '../../types';
-import { DEFAULT_CACHE_EXPIRATION_IN_MILLIS } from '../../utils/constants/browser';
-import { isNaNNumber } from '../../utils/lang';
+import { isFiniteNumber, isNaNNumber } from '../../utils/lang';
 import { getStorageHash } from '../KeyBuilder';
 import { LOG_PREFIX } from './constants';
 import type { SplitsCacheInLocal } from './SplitsCacheInLocal';
+import type { MySegmentsCacheInLocal } from './MySegmentsCacheInLocal';
 import { KeyBuilderCS } from '../KeyBuilderCS';
+import SplitIO from '../../../types/splitio';
 
-function validateExpiration(settings: ISettings, keys: KeyBuilderCS) {
+const DEFAULT_CACHE_EXPIRATION_IN_DAYS = 10;
+const MILLIS_IN_A_DAY = 86400000;
+
+/**
+ * Validates if cache should be cleared and sets the cache `hash` if needed.
+ *
+ * @returns `true` if cache should be cleared, `false` otherwise
+ */
+function validateExpiration(options: SplitIO.InLocalStorageOptions, settings: ISettings, keys: KeyBuilderCS, currentTimestamp: number, isThereCache: boolean) {
   const { log } = settings;
 
   // Check expiration
-  const expirationTimestamp = Date.now() - DEFAULT_CACHE_EXPIRATION_IN_MILLIS;
-  let value: string | number | null = localStorage.getItem(keys.buildLastUpdatedKey());
-  if (value !== null) {
-    value = parseInt(value, 10);
-    if (!isNaNNumber(value) && value < expirationTimestamp) return true;
+  const lastUpdatedTimestamp = parseInt(localStorage.getItem(keys.buildLastUpdatedKey()) as string, 10);
+  if (!isNaNNumber(lastUpdatedTimestamp)) {
+    const cacheExpirationInDays = isFiniteNumber(options.expirationDays) && options.expirationDays >= 1 ? options.expirationDays : DEFAULT_CACHE_EXPIRATION_IN_DAYS;
+    const expirationTimestamp = currentTimestamp - MILLIS_IN_A_DAY * cacheExpirationInDays;
+    if (lastUpdatedTimestamp < expirationTimestamp) {
+      log.info(LOG_PREFIX + 'Cache expired more than ' + cacheExpirationInDays + ' days ago. Cleaning up cache');
+      return true;
+    }
   }
 
   // Check hash
@@ -23,27 +35,57 @@ function validateExpiration(settings: ISettings, keys: KeyBuilderCS) {
   const currentStorageHash = getStorageHash(settings);
 
   if (storageHash !== currentStorageHash) {
-    log.info(LOG_PREFIX + 'SDK key, flags filter criteria or flags spec version was modified. Updating cache');
     try {
       localStorage.setItem(storageHashKey, currentStorageHash);
     } catch (e) {
       log.error(LOG_PREFIX + e);
     }
-    return true;
+    if (isThereCache) {
+      log.info(LOG_PREFIX + 'SDK key, flags filter criteria, or flags spec version has changed. Cleaning up cache');
+      return true;
+    }
+    return false; // No cache to clear
+  }
+
+  // Clear on init
+  if (options.clearOnInit) {
+    const lastClearTimestamp = parseInt(localStorage.getItem(keys.buildLastClear()) as string, 10);
+
+    if (isNaNNumber(lastClearTimestamp) || lastClearTimestamp < currentTimestamp - MILLIS_IN_A_DAY) {
+      log.info(LOG_PREFIX + 'clearOnInit was set and cache was not cleared in the last 24 hours. Cleaning up cache');
+      return true;
+    }
   }
 }
 
 /**
  * Clean cache if:
  * - it has expired, i.e., its `lastUpdated` timestamp is older than the given `expirationTimestamp`
- * - hash has changed, i.e., the SDK key, flags filter criteria or flags spec version was modified
+ * - its hash has changed, i.e., the SDK key, flags filter criteria or flags spec version was modified
+ * - `clearOnInit` was set and cache was not cleared in the last 24 hours
+ *
+ * @returns `true` if cache is ready to be used, `false` otherwise (cache was cleared or there is no cache)
  */
-export function validateCache(settings: ISettings, keys: KeyBuilderCS, splits: SplitsCacheInLocal): boolean {
+export function validateCache(options: SplitIO.InLocalStorageOptions, settings: ISettings, keys: KeyBuilderCS, splits: SplitsCacheInLocal, segments: MySegmentsCacheInLocal, largeSegments: MySegmentsCacheInLocal): boolean {
+
+  const currentTimestamp = Date.now();
+  const isThereCache = splits.getChangeNumber() > -1;
 
-  if (validateExpiration(settings, keys)) {
+  if (validateExpiration(options, settings, keys, currentTimestamp, isThereCache)) {
     splits.clear();
+    segments.clear();
+    largeSegments.clear();
+
+    // Update last clear timestamp
+    try {
+      localStorage.setItem(keys.buildLastClear(), currentTimestamp + '');
+    } catch (e) {
+      settings.log.error(LOG_PREFIX + e);
+    }
+
+    return false;
   }
 
-  // Check if the cache is ready
-  return splits.getChangeNumber() > -1;
+  // Check if ready from cache
+  return isThereCache;
 }
diff --git a/src/utils/constants/browser.ts b/src/utils/constants/browser.ts
diff --git a/src/utils/lang/index.ts b/src/utils/lang/index.ts
@@ -120,7 +120,7 @@ export function isBoolean(val: any): boolean {
  * Unlike `Number.isFinite`, it also tests Number object instances.
  * Unlike global `isFinite`, it returns false if the value is not a number or Number object instance.
  */
-export function isFiniteNumber(val: any): boolean {
+export function isFiniteNumber(val: any): val is number {
   if (val instanceof Number) val = val.valueOf();
   return typeof val === 'number' ?
     Number.isFinite ? Number.isFinite(val) : isFinite(val) :
diff --git a/types/splitio.d.ts b/types/splitio.d.ts
@@ -906,6 +906,18 @@ declare namespace SplitIO {
      * @defaultValue `'SPLITIO'`
      */
     prefix?: string;
+    /**
+     * Number of days before cached data expires if it was not updated. If cache expires, it is cleared on initialization.
+     *
+     * @defaultValue `10`
+     */
+    expirationDays?: number;
+    /**
+     * Optional settings to clear the cache. If set to `true`, the SDK clears the cached data on initialization, unless the cache was cleared within the last 24 hours.
+     *
+     * @defaultValue `false`
+     */
+    clearOnInit?: boolean;
   }
   /**
    * Storage for asynchronous (consumer) SDK.
@@ -1229,11 +1241,23 @@ declare namespace SplitIO {
        */
       type?: BrowserStorage;
       /**
-       * Optional prefix to prevent any kind of data collision between SDK versions.
+       * Optional prefix to prevent any kind of data collision between SDK versions when using 'LOCALSTORAGE'.
        *
        * @defaultValue `'SPLITIO'`
        */
       prefix?: string;
+      /**
+       * Optional settings for the 'LOCALSTORAGE' storage type. It specifies the number of days before cached data expires if it was not updated. If cache expires, it is cleared on initialization.
+       *
+       * @defaultValue `10`
+       */
+      expirationDays?: number;
+      /**
+       * Optional settings for the 'LOCALSTORAGE' storage type. If set to `true`, the SDK clears the cached data on initialization, unless the cache was cleared within the last 24 hours.
+       *
+       * @defaultValue `false`
+       */
+      clearOnInit?: boolean;
     };
   }
   /**

Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,6 @@`
`1`	`1`	`{`
`2`	`2`	`"name": "@splitsoftware/splitio-commons",`
`3`		`- "version": "2.0.2",`
	`3`	`+ "version": "2.1.0-rc.0",`
`4`	`4`	`"description": "Split JavaScript SDK common components",`
`5`	`5`	`"main": "cjs/index.js",`
`6`	`6`	`"module": "esm/index.js",`
Original file line number	Diff line number	Diff line change
`@@ -43,6 +43,10 @@ export class KeyBuilderCS extends KeyBuilder implements MySegmentsKeyBuilder {`
`43`	`43`	`buildTillKey() {`
`44`	`44`	return `${this.prefix}.${this.matchingKey}.segments.till`;
`45`	`45`	`}`
	`46`	`+`
	`47`	`+ buildLastClear() {`
	`48`	+ return `${this.prefix}.lastClear`;
	`49`	`+ }`
`46`	`50`	`}`
`47`	`51`
`48`	`52`	`export function myLargeSegmentsKeyBuilder(prefix: string, matchingKey: string): MySegmentsKeyBuilder {`