Skip to content

Commit 8938334

Browse files
EmilianoSanchezEmilianoSanchez
committed
Extract validation logic into SplitsCacheInLocal::validateCache method
1 parent d392cc8 commit 8938334

File tree

3 files changed

+57
-48
lines changed

3 files changed

+57
-48
lines changed

‎src/storages/inLocalStorage/SplitsCacheInLocal.ts‎

Lines changed: 34 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -20,16 +20,47 @@ export class SplitsCacheInLocal extends AbstractSplitsCacheSync {
2020
private hasSync?: boolean;
2121
private updateNewFilter?: boolean;
2222

23-
constructor(settings: ISettings, keys: KeyBuilderCS, expirationTimestamp?: number) {
23+
constructor(settings: ISettings, keys: KeyBuilderCS) {
2424
super();
2525
this.keys = keys;
2626
this.log = settings.log;
2727
this.storageHash = getStorageHash(settings);
2828
this.flagSetsFilter = settings.sync.__splitFiltersValidation.groupedFilters.bySet;
29+
}
2930

30-
this._checkExpiration(expirationTimestamp);
31+
/**
32+
* Clean Splits cache if its `lastUpdated` timestamp is older than the given `expirationTimestamp`,
33+
*
34+
* @param expirationTimestamp - if the value is not a number, data will not be cleaned
35+
*/
36+
public validateCache(expirationTimestamp?: number) {
37+
// _checkExpiration
38+
let value: string | number | null = localStorage.getItem(this.keys.buildLastUpdatedKey());
39+
if (value !== null) {
40+
value = parseInt(value, 10);
41+
if (!isNaNNumber(value) && expirationTimestamp && value < expirationTimestamp) this.clear();
42+
}
3143

32-
this._checkFilterQuery();
44+
// @TODO eventually remove `_checkFilterQuery`. Cache should be cleared at the storage level, reusing same logic than PluggableStorage
45+
// _checkFilterQuery
46+
const storageHashKey = this.keys.buildHashKey();
47+
const storageHash = localStorage.getItem(storageHashKey);
48+
49+
if (storageHash !== this.storageHash) {
50+
try {
51+
// mark cache to update the new query filter on first successful splits fetch
52+
this.updateNewFilter = true;
53+
54+
// if there is cache, clear it
55+
if (this.getChangeNumber() > -1) this.clear();
56+
57+
} catch (e) {
58+
this.log.error(LOG_PREFIX + e);
59+
}
60+
}
61+
// if the filter didn't change, nothing is done
62+
63+
return this.getChangeNumber() > -1;
3364
}
3465

3566
private _decrementCount(key: string) {
@@ -212,39 +243,6 @@ export class SplitsCacheInLocal extends AbstractSplitsCacheSync {
212243
}
213244
}
214245

215-
/**
216-
* Clean Splits cache if its `lastUpdated` timestamp is older than the given `expirationTimestamp`,
217-
*
218-
* @param expirationTimestamp - if the value is not a number, data will not be cleaned
219-
*/
220-
private _checkExpiration(expirationTimestamp?: number) {
221-
let value: string | number | null = localStorage.getItem(this.keys.buildLastUpdatedKey());
222-
if (value !== null) {
223-
value = parseInt(value, 10);
224-
if (!isNaNNumber(value) && expirationTimestamp && value < expirationTimestamp) this.clear();
225-
}
226-
}
227-
228-
// @TODO eventually remove `_checkFilterQuery`. Cache should be cleared at the storage level, reusing same logic than PluggableStorage
229-
private _checkFilterQuery() {
230-
const storageHashKey = this.keys.buildHashKey();
231-
const storageHash = localStorage.getItem(storageHashKey);
232-
233-
if (storageHash !== this.storageHash) {
234-
try {
235-
// mark cache to update the new query filter on first successful splits fetch
236-
this.updateNewFilter = true;
237-
238-
// if there is cache, clear it
239-
if (this.getChangeNumber() > -1) this.clear();
240-
241-
} catch (e) {
242-
this.log.error(LOG_PREFIX + e);
243-
}
244-
}
245-
// if the filter didn't change, nothing is done
246-
}
247-
248246
getNamesByFlagSets(flagSets: string[]): Set<string>[] {
249247
return flagSets.map(flagSet => {
250248
const flagSetKey = this.keys.buildFlagSetKey(flagSet);

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

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { fullSettings } from '../../../utils/settingsValidation/__tests__/settin
77

88
test('SPLIT CACHE / LocalStorage', () => {
99
const cache = new SplitsCacheInLocal(fullSettings, new KeyBuilderCS('SPLITIO', 'user'));
10+
cache.validateCache();
1011

1112
cache.clear();
1213

@@ -40,6 +41,7 @@ test('SPLIT CACHE / LocalStorage', () => {
4041

4142
test('SPLIT CACHE / LocalStorage / Get Keys', () => {
4243
const cache = new SplitsCacheInLocal(fullSettings, new KeyBuilderCS('SPLITIO', 'user'));
44+
cache.validateCache();
4345

4446
cache.addSplit('lol1', something);
4547
cache.addSplit('lol2', somethingElse);
@@ -52,6 +54,7 @@ test('SPLIT CACHE / LocalStorage / Get Keys', () => {
5254

5355
test('SPLIT CACHE / LocalStorage / Add Splits', () => {
5456
const cache = new SplitsCacheInLocal(fullSettings, new KeyBuilderCS('SPLITIO', 'user'));
57+
cache.validateCache();
5558

5659
cache.addSplits([
5760
['lol1', something],
@@ -66,6 +69,7 @@ test('SPLIT CACHE / LocalStorage / Add Splits', () => {
6669

6770
test('SPLIT CACHE / LocalStorage / trafficTypeExists and ttcache tests', () => {
6871
const cache = new SplitsCacheInLocal(fullSettings, new KeyBuilderCS('SPLITIO', 'user'));
72+
cache.validateCache();
6973

7074
cache.addSplits([ // loop of addSplit
7175
['split1', splitWithUserTT],
@@ -104,6 +108,8 @@ test('SPLIT CACHE / LocalStorage / trafficTypeExists and ttcache tests', () => {
104108

105109
test('SPLIT CACHE / LocalStorage / killLocally', () => {
106110
const cache = new SplitsCacheInLocal(fullSettings, new KeyBuilderCS('SPLITIO', 'user'));
111+
cache.validateCache();
112+
107113
cache.addSplit('lol1', something);
108114
cache.addSplit('lol2', somethingElse);
109115
const initialChangeNumber = cache.getChangeNumber();
@@ -136,6 +142,7 @@ test('SPLIT CACHE / LocalStorage / killLocally', () => {
136142

137143
test('SPLIT CACHE / LocalStorage / usesSegments', () => {
138144
const cache = new SplitsCacheInLocal(fullSettings, new KeyBuilderCS('SPLITIO', 'user'));
145+
cache.validateCache();
139146

140147
expect(cache.usesSegments()).toBe(true); // true initially, until data is synchronized
141148
cache.setChangeNumber(1); // to indicate that data has been synced.
@@ -167,6 +174,8 @@ test('SPLIT CACHE / LocalStorage / flag set cache tests', () => {
167174
}
168175
}
169176
}, new KeyBuilderCS('SPLITIO', 'user'));
177+
cache.validateCache();
178+
170179
const emptySet = new Set([]);
171180

172181
cache.addSplits([
@@ -206,25 +215,27 @@ test('SPLIT CACHE / LocalStorage / flag set cache tests', () => {
206215

207216
// if FlagSets are not defined, it should store all FlagSets in memory.
208217
test('SPLIT CACHE / LocalStorage / flag set cache tests without filters', () => {
209-
const cacheWithoutFilters = new SplitsCacheInLocal(fullSettings, new KeyBuilderCS('SPLITIO', 'user'));
218+
const cache = new SplitsCacheInLocal(fullSettings, new KeyBuilderCS('SPLITIO', 'user'));
219+
cache.validateCache();
220+
210221
const emptySet = new Set([]);
211222

212-
cacheWithoutFilters.addSplits([
223+
cache.addSplits([
213224
[featureFlagOne.name, featureFlagOne],
214225
[featureFlagTwo.name, featureFlagTwo],
215226
[featureFlagThree.name, featureFlagThree],
216227
]);
217-
cacheWithoutFilters.addSplit(featureFlagWithEmptyFS.name, featureFlagWithEmptyFS);
228+
cache.addSplit(featureFlagWithEmptyFS.name, featureFlagWithEmptyFS);
218229

219-
expect(cacheWithoutFilters.getNamesByFlagSets(['o'])).toEqual([new Set(['ff_one', 'ff_two'])]);
220-
expect(cacheWithoutFilters.getNamesByFlagSets(['n'])).toEqual([new Set(['ff_one'])]);
221-
expect(cacheWithoutFilters.getNamesByFlagSets(['e'])).toEqual([new Set(['ff_one', 'ff_three'])]);
222-
expect(cacheWithoutFilters.getNamesByFlagSets(['t'])).toEqual([new Set(['ff_two', 'ff_three'])]);
223-
expect(cacheWithoutFilters.getNamesByFlagSets(['y'])).toEqual([emptySet]);
224-
expect(cacheWithoutFilters.getNamesByFlagSets(['o', 'n', 'e'])).toEqual([new Set(['ff_one', 'ff_two']), new Set(['ff_one']), new Set(['ff_one', 'ff_three'])]);
230+
expect(cache.getNamesByFlagSets(['o'])).toEqual([new Set(['ff_one', 'ff_two'])]);
231+
expect(cache.getNamesByFlagSets(['n'])).toEqual([new Set(['ff_one'])]);
232+
expect(cache.getNamesByFlagSets(['e'])).toEqual([new Set(['ff_one', 'ff_three'])]);
233+
expect(cache.getNamesByFlagSets(['t'])).toEqual([new Set(['ff_two', 'ff_three'])]);
234+
expect(cache.getNamesByFlagSets(['y'])).toEqual([emptySet]);
235+
expect(cache.getNamesByFlagSets(['o', 'n', 'e'])).toEqual([new Set(['ff_one', 'ff_two']), new Set(['ff_one']), new Set(['ff_one', 'ff_three'])]);
225236

226237
// Validate that the feature flag cache is cleared when calling `clear` method
227-
cacheWithoutFilters.clear();
238+
cache.clear();
228239
expect(localStorage.length).toBe(1); // only 'SPLITIO.hash' should remain in localStorage
229240
expect(localStorage.key(0)).toBe('SPLITIO.hash');
230241
});

‎src/storages/inLocalStorage/index.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ export function InLocalStorage(options: InLocalStorageOptions = {}): IStorageSyn
3939
const keys = new KeyBuilderCS(prefix, matchingKey);
4040
const expirationTimestamp = Date.now() - DEFAULT_CACHE_EXPIRATION_IN_MILLIS;
4141

42-
const splits = new SplitsCacheInLocal(settings, keys, expirationTimestamp);
42+
const splits = new SplitsCacheInLocal(settings, keys);
4343
const segments = new MySegmentsCacheInLocal(log, keys);
4444
const largeSegments = new MySegmentsCacheInLocal(log, myLargeSegmentsKeyBuilder(prefix, matchingKey));
4545

@@ -55,7 +55,7 @@ export function InLocalStorage(options: InLocalStorageOptions = {}): IStorageSyn
5555

5656
// @TODO implement
5757
validateCache() {
58-
return splits.getChangeNumber() > -1;
58+
return splits.validateCache(expirationTimestamp);
5959
},
6060

6161
destroy() { },

0 commit comments

Comments
 (0)