Skip to content

Commit e8e07ef

Browse files
authored
fix: Increment version for changing flagValues (#317)
When using FileData you have the option of just providing values. The pre-typescript SDK was liberal with what it considered an update and it would emit updates as things changed. Some improvements to prevent spurious update events were added in typscript and these interfered with the basic way the FileData implementation works. This PR makes a couple updates: 1.) If the value of a "flagValue" changes, then we will increment the version associated with it. 2.) If the value of a "flagValue" does not change, then we will not increment the version. This should mean when values change the version will change. Standard flag/segment JSON must update their versions manually. This will add some performance overhead. This addresses #310
1 parent 4371870 commit e8e07ef

File tree

2 files changed

+106
-7
lines changed

2 files changed

+106
-7
lines changed

packages/shared/sdk-server/__tests__/data_sources/FileDataSource.test.ts

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -443,4 +443,92 @@ describe('given a mock filesystem and memory feature store', () => {
443443
expect(filesystem.readFile).toHaveBeenCalledTimes(1);
444444
expect(yamlParser).toHaveBeenCalledWith('the data');
445445
});
446+
447+
it('it updates the version numbers for value based flags when their values change', async () => {
448+
await createFileDataSource(
449+
true,
450+
[
451+
{
452+
path: 'file1.json',
453+
data: `{
454+
"flagValues":
455+
{
456+
"${flag2Key}": "${flag2Value}"
457+
}
458+
}`,
459+
},
460+
],
461+
false,
462+
true,
463+
);
464+
465+
expect(await asyncFeatureStore.initialized()).toBeTruthy();
466+
467+
const flags = await asyncFeatureStore.all(VersionedDataKinds.Features);
468+
expect(Object.keys(flags).length).toEqual(1);
469+
470+
const segments = await asyncFeatureStore.all(VersionedDataKinds.Segments);
471+
expect(Object.keys(segments).length).toEqual(0);
472+
473+
// Need to update the timestamp, or it will think the file has not changed.
474+
filesystem.fileData['file1.json'] = {
475+
timestamp: 100,
476+
data: `{
477+
"flagValues":
478+
{
479+
"${flag2Key}": "differentValue"
480+
}
481+
}`,
482+
};
483+
filesystem.watches['file1.json'][0].cb('change', 'file1.json');
484+
485+
await jest.runAllTimersAsync();
486+
487+
const readFlag = await asyncFeatureStore.get(VersionedDataKinds.Features, flag2Key);
488+
expect(readFlag?.version).toEqual(2);
489+
});
490+
491+
it('it does not update the version when the value does not change', async () => {
492+
await createFileDataSource(
493+
true,
494+
[
495+
{
496+
path: 'file1.json',
497+
data: `{
498+
"flagValues":
499+
{
500+
"${flag2Key}": "${flag2Value}"
501+
}
502+
}`,
503+
},
504+
],
505+
false,
506+
true,
507+
);
508+
509+
expect(await asyncFeatureStore.initialized()).toBeTruthy();
510+
511+
const flags = await asyncFeatureStore.all(VersionedDataKinds.Features);
512+
expect(Object.keys(flags).length).toEqual(1);
513+
514+
const segments = await asyncFeatureStore.all(VersionedDataKinds.Segments);
515+
expect(Object.keys(segments).length).toEqual(0);
516+
517+
// Need to update the timestamp, or it will think the file has not changed.
518+
filesystem.fileData['file1.json'] = {
519+
timestamp: 100,
520+
data: `{
521+
"flagValues":
522+
{
523+
"${flag2Key}": "${flag2Value}"
524+
}
525+
}`,
526+
};
527+
filesystem.watches['file1.json'][0].cb('change', 'file1.json');
528+
529+
await jest.runAllTimersAsync();
530+
531+
const readFlag = await asyncFeatureStore.get(VersionedDataKinds.Features, flag2Key);
532+
expect(readFlag?.version).toEqual(1);
533+
});
446534
});

packages/shared/sdk-server/src/data_sources/FileDataSource.ts

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,22 +6,23 @@ import {
66
VoidFunction,
77
} from '@launchdarkly/js-sdk-common';
88

9-
import { DataKind, LDFeatureStore, LDFeatureStoreDataStorage } from '../api';
9+
import { DataKind, LDFeatureStoreDataStorage } from '../api';
1010
import { FileDataSourceOptions } from '../api/integrations';
11+
import { LDDataSourceUpdates } from '../api/subsystems';
1112
import { Flag } from '../evaluation/data/Flag';
1213
import { processFlag, processSegment } from '../store/serialization';
1314
import VersionedDataKinds from '../store/VersionedDataKinds';
1415
import FileLoader from './FileLoader';
1516

1617
export type FileDataSourceErrorHandler = (err: LDFileDataSourceError) => void;
1718

18-
function makeFlagWithValue(key: string, value: any): Flag {
19+
function makeFlagWithValue(key: string, value: any, version: number): Flag {
1920
return {
2021
key,
2122
on: true,
2223
fallthrough: { variation: 0 },
2324
variations: [value],
24-
version: 1,
25+
version,
2526
};
2627
}
2728

@@ -42,7 +43,7 @@ export default class FileDataSource implements subsystem.LDStreamProcessor {
4243
constructor(
4344
options: FileDataSourceOptions,
4445
filesystem: Filesystem,
45-
private readonly featureStore: LDFeatureStore,
46+
private readonly featureStore: LDDataSourceUpdates,
4647
private initSuccessHandler: VoidFunction = () => {},
4748
private readonly errorHandler?: FileDataSourceErrorHandler,
4849
) {
@@ -102,6 +103,7 @@ export default class FileDataSource implements subsystem.LDStreamProcessor {
102103

103104
private processFileData(fileData: { path: string; data: string }[]) {
104105
// Clear any existing data before re-populating it.
106+
const oldData = this.allData;
105107
this.allData = {};
106108

107109
// We let the parsers throw, and the caller can handle the rejection.
@@ -117,7 +119,7 @@ export default class FileDataSource implements subsystem.LDStreamProcessor {
117119
parsed = JSON.parse(fd.data);
118120
}
119121

120-
this.processParsedData(parsed);
122+
this.processParsedData(parsed, oldData);
121123
});
122124

123125
this.featureStore.init(this.allData, () => {
@@ -128,13 +130,22 @@ export default class FileDataSource implements subsystem.LDStreamProcessor {
128130
});
129131
}
130132

131-
private processParsedData(parsed: any) {
133+
private processParsedData(parsed: any, oldData: LDFeatureStoreDataStorage) {
132134
Object.keys(parsed.flags || {}).forEach((key) => {
133135
processFlag(parsed.flags[key]);
134136
this.addItem(VersionedDataKinds.Features, parsed.flags[key]);
135137
});
136138
Object.keys(parsed.flagValues || {}).forEach((key) => {
137-
const flag = makeFlagWithValue(key, parsed.flagValues[key]);
139+
const previousInstance = oldData[VersionedDataKinds.Features.namespace]?.[key];
140+
let { version } = previousInstance ?? { version: 1 };
141+
// If the data is different, then we want to increment the version.
142+
if (
143+
previousInstance &&
144+
JSON.stringify(parsed.flagValues[key]) !== JSON.stringify(previousInstance?.variations?.[0])
145+
) {
146+
version += 1;
147+
}
148+
const flag = makeFlagWithValue(key, parsed.flagValues[key], version);
138149
processFlag(flag);
139150
this.addItem(VersionedDataKinds.Features, flag);
140151
});

0 commit comments

Comments
 (0)