Merge pull request #379 from splitio/cache_expiration_refactor_validateCache_method

[Cache expiration] Add `validateCache` function

Author: EmilianoSanchez

src/storages/inLocalStorage/SplitsCacheInLocal.ts

@@ -5,7 +5,6 @@ import { KeyBuilderCS } from '../KeyBuilderCS';
 import { ILogger } from '../../logger/types';
 import { LOG_PREFIX } from './constants';
 import { ISettings } from '../../types';
-import { getStorageHash } from '../KeyBuilder';
 import { setToArray } from '../../utils/lang/sets';
 
 /**
@@ -15,21 +14,14 @@ export class SplitsCacheInLocal extends AbstractSplitsCacheSync {
 
   private readonly keys: KeyBuilderCS;
   private readonly log: ILogger;
-  private readonly storageHash: string;
   private readonly flagSetsFilter: string[];
   private hasSync?: boolean;
-  private updateNewFilter?: boolean;
 
-  constructor(settings: ISettings, keys: KeyBuilderCS, expirationTimestamp?: number) {
+  constructor(settings: ISettings, keys: KeyBuilderCS) {
     super();
     this.keys = keys;
     this.log = settings.log;
-    this.storageHash = getStorageHash(settings);
     this.flagSetsFilter = settings.sync.__splitFiltersValidation.groupedFilters.bySet;
-
-    this._checkExpiration(expirationTimestamp);
-
-    this._checkFilterQuery();
   }
 
   private _decrementCount(key: string) {
@@ -138,19 +130,6 @@ export class SplitsCacheInLocal extends AbstractSplitsCacheSync {
   }
 
   setChangeNumber(changeNumber: number): boolean {
-
-    // when using a new split query, we must update it at the store
-    if (this.updateNewFilter) {
-      this.log.info(LOG_PREFIX + 'SDK key, flags filter criteria or flags spec version was modified. Updating cache');
-      const storageHashKey = this.keys.buildHashKey();
-      try {
-        localStorage.setItem(storageHashKey, this.storageHash);
-      } catch (e) {
-        this.log.error(LOG_PREFIX + e);
-      }
-      this.updateNewFilter = false;
-    }
-
     try {
       localStorage.setItem(this.keys.buildSplitsTillKey(), changeNumber + '');
       // update "last updated" timestamp with current time
@@ -212,39 +191,6 @@ export class SplitsCacheInLocal extends AbstractSplitsCacheSync {
     }
   }
 
-  /**
-   * Clean Splits cache if its `lastUpdated` timestamp is older than the given `expirationTimestamp`,
-   *
-   * @param expirationTimestamp - if the value is not a number, data will not be cleaned
-   */
-  private _checkExpiration(expirationTimestamp?: number) {
-    let value: string | number | null = localStorage.getItem(this.keys.buildLastUpdatedKey());
-    if (value !== null) {
-      value = parseInt(value, 10);
-      if (!isNaNNumber(value) && expirationTimestamp && value < expirationTimestamp) this.clear();
-    }
-  }
-
-  // @TODO eventually remove `_checkFilterQuery`. Cache should be cleared at the storage level, reusing same logic than PluggableStorage
-  private _checkFilterQuery() {
-    const storageHashKey = this.keys.buildHashKey();
-    const storageHash = localStorage.getItem(storageHashKey);
-
-    if (storageHash !== this.storageHash) {
-      try {
-        // mark cache to update the new query filter on first successful splits fetch
-        this.updateNewFilter = true;
-
-        // if there is cache, clear it
-        if (this.getChangeNumber() > -1) this.clear();
-
-      } catch (e) {
-        this.log.error(LOG_PREFIX + e);
-      }
-    }
-    // if the filter didn't change, nothing is done
-  }
-
   getNamesByFlagSets(flagSets: string[]): Set<string>[] {
     return flagSets.map(flagSet => {
       const flagSetKey = this.keys.buildFlagSetKey(flagSet);

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

@@ -104,6 +104,7 @@ test('SPLIT CACHE / LocalStorage / trafficTypeExists and ttcache tests', () => {
 
 test('SPLIT CACHE / LocalStorage / killLocally', () => {
   const cache = new SplitsCacheInLocal(fullSettings, new KeyBuilderCS('SPLITIO', 'user'));
+
   cache.addSplit('lol1', something);
   cache.addSplit('lol2', somethingElse);
   const initialChangeNumber = cache.getChangeNumber();
@@ -167,6 +168,7 @@ test('SPLIT CACHE / LocalStorage / flag set cache tests', () => {
       }
     }
   }, new KeyBuilderCS('SPLITIO', 'user'));
+
   const emptySet = new Set([]);
 
   cache.addSplits([
@@ -206,25 +208,25 @@ test('SPLIT CACHE / LocalStorage / flag set cache tests', () => {
 
 // if FlagSets are not defined, it should store all FlagSets in memory.
 test('SPLIT CACHE / LocalStorage / flag set cache tests without filters', () => {
-  const cacheWithoutFilters = new SplitsCacheInLocal(fullSettings, new KeyBuilderCS('SPLITIO', 'user'));
+  const cache = new SplitsCacheInLocal(fullSettings, new KeyBuilderCS('SPLITIO', 'user'));
+
   const emptySet = new Set([]);
 
-  cacheWithoutFilters.addSplits([
+  cache.addSplits([
     [featureFlagOne.name, featureFlagOne],
     [featureFlagTwo.name, featureFlagTwo],
     [featureFlagThree.name, featureFlagThree],
   ]);
-  cacheWithoutFilters.addSplit(featureFlagWithEmptyFS.name, featureFlagWithEmptyFS);
+  cache.addSplit(featureFlagWithEmptyFS.name, featureFlagWithEmptyFS);
 
-  expect(cacheWithoutFilters.getNamesByFlagSets(['o'])).toEqual([new Set(['ff_one', 'ff_two'])]);
-  expect(cacheWithoutFilters.getNamesByFlagSets(['n'])).toEqual([new Set(['ff_one'])]);
-  expect(cacheWithoutFilters.getNamesByFlagSets(['e'])).toEqual([new Set(['ff_one', 'ff_three'])]);
-  expect(cacheWithoutFilters.getNamesByFlagSets(['t'])).toEqual([new Set(['ff_two', 'ff_three'])]);
-  expect(cacheWithoutFilters.getNamesByFlagSets(['y'])).toEqual([emptySet]);
-  expect(cacheWithoutFilters.getNamesByFlagSets(['o', 'n', 'e'])).toEqual([new Set(['ff_one', 'ff_two']), new Set(['ff_one']), new Set(['ff_one', 'ff_three'])]);
+  expect(cache.getNamesByFlagSets(['o'])).toEqual([new Set(['ff_one', 'ff_two'])]);
+  expect(cache.getNamesByFlagSets(['n'])).toEqual([new Set(['ff_one'])]);
+  expect(cache.getNamesByFlagSets(['e'])).toEqual([new Set(['ff_one', 'ff_three'])]);
+  expect(cache.getNamesByFlagSets(['t'])).toEqual([new Set(['ff_two', 'ff_three'])]);
+  expect(cache.getNamesByFlagSets(['y'])).toEqual([emptySet]);
+  expect(cache.getNamesByFlagSets(['o', 'n', 'e'])).toEqual([new Set(['ff_one', 'ff_two']), new Set(['ff_one']), new Set(['ff_one', 'ff_three'])]);
 
   // Validate that the feature flag cache is cleared when calling `clear` method
-  cacheWithoutFilters.clear();
-  expect(localStorage.length).toBe(1); // only 'SPLITIO.hash' should remain in localStorage
-  expect(localStorage.key(0)).toBe('SPLITIO.hash');
+  cache.clear();
+  expect(localStorage.length).toBe(0);
 });

src/storages/inLocalStorage/index.ts

@@ -7,13 +7,13 @@ import { KeyBuilderCS, myLargeSegmentsKeyBuilder } from '../KeyBuilderCS';
 import { isLocalStorageAvailable } from '../../utils/env/isLocalStorageAvailable';
 import { SplitsCacheInLocal } from './SplitsCacheInLocal';
 import { MySegmentsCacheInLocal } from './MySegmentsCacheInLocal';
-import { DEFAULT_CACHE_EXPIRATION_IN_MILLIS } from '../../utils/constants/browser';
 import { InMemoryStorageCSFactory } from '../inMemory/InMemoryStorageCS';
 import { LOG_PREFIX } from './constants';
 import { DEBUG, NONE, STORAGE_LOCALSTORAGE } from '../../utils/constants';
 import { shouldRecordTelemetry, TelemetryCacheInMemory } from '../inMemory/TelemetryCacheInMemory';
 import { UniqueKeysCacheInMemoryCS } from '../inMemory/UniqueKeysCacheInMemoryCS';
 import { getMatching } from '../../utils/key';
+import { validateCache } from './validateCache';
 
 export interface InLocalStorageOptions {
   prefix?: string
@@ -37,9 +37,8 @@ export function InLocalStorage(options: InLocalStorageOptions = {}): IStorageSyn
     const { settings, settings: { log, scheduler: { impressionsQueueSize, eventsQueueSize, }, sync: { impressionsMode } } } = params;
     const matchingKey = getMatching(settings.core.key);
     const keys = new KeyBuilderCS(prefix, matchingKey);
-    const expirationTimestamp = Date.now() - DEFAULT_CACHE_EXPIRATION_IN_MILLIS;
 
-    const splits = new SplitsCacheInLocal(settings, keys, expirationTimestamp);
+    const splits = new SplitsCacheInLocal(settings, keys);
     const segments = new MySegmentsCacheInLocal(log, keys);
     const largeSegments = new MySegmentsCacheInLocal(log, myLargeSegmentsKeyBuilder(prefix, matchingKey));
 
@@ -53,9 +52,8 @@ export function InLocalStorage(options: InLocalStorageOptions = {}): IStorageSyn
       telemetry: shouldRecordTelemetry(params) ? new TelemetryCacheInMemory(splits, segments) : undefined,
       uniqueKeys: impressionsMode === NONE ? new UniqueKeysCacheInMemoryCS() : undefined,
 
-      // @TODO implement
       validateCache() {
-        return splits.getChangeNumber() > -1;
+        return validateCache(settings, keys, splits);
       },
 
       destroy() { },

src/storages/inLocalStorage/validateCache.ts

@@ -0,0 +1,49 @@
+import { ISettings } from '../../types';
+import { DEFAULT_CACHE_EXPIRATION_IN_MILLIS } from '../../utils/constants/browser';
+import { isNaNNumber } from '../../utils/lang';
+import { getStorageHash } from '../KeyBuilder';
+import { LOG_PREFIX } from './constants';
+import type { SplitsCacheInLocal } from './SplitsCacheInLocal';
+import { KeyBuilderCS } from '../KeyBuilderCS';
+
+function validateExpiration(settings: ISettings, keys: KeyBuilderCS) {
+  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;
+  }
+
+  // Check hash
+  const storageHashKey = keys.buildHashKey();
+  const storageHash = localStorage.getItem(storageHashKey);
+  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;
+  }
+}
+
+/**
+ * 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
+ */
+export function validateCache(settings: ISettings, keys: KeyBuilderCS, splits: SplitsCacheInLocal): boolean {
+
+  if (validateExpiration(settings, keys)) {
+    splits.clear();
+  }
+
+  // Check if the cache is ready
+  return splits.getChangeNumber() > -1;
+}

src/storages/pluggable/index.ts

@@ -92,7 +92,8 @@ export function PluggableStorage(options: PluggableStorageOptions): IStorageAsyn
     // Connects to wrapper and emits SDK_READY event on main client
     const connectPromise = wrapper.connect().then(() => {
       if (isSyncronizer) {
-        // In standalone or producer mode, clear storage if SDK key or feature flag filter has changed
+        // @TODO reuse InLocalStorage::validateCache logic
+        // In standalone or producer mode, clear storage if SDK key, flags filter criteria or flags spec version was modified
         return wrapper.get(keys.buildHashKey()).then((hash) => {
           const currentHash = getStorageHash(settings);
           if (hash !== currentHash) {