Skip to content

Commit

Permalink
fix: Increment version for changing flagValues (#317)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
kinyoklion authored Dec 4, 2023
1 parent 4371870 commit e8e07ef
Show file tree
Hide file tree
Showing 2 changed files with 106 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -443,4 +443,92 @@ describe('given a mock filesystem and memory feature store', () => {
expect(filesystem.readFile).toHaveBeenCalledTimes(1);
expect(yamlParser).toHaveBeenCalledWith('the data');
});

it('it updates the version numbers for value based flags when their values change', async () => {
await createFileDataSource(
true,
[
{
path: 'file1.json',
data: `{
"flagValues":
{
"${flag2Key}": "${flag2Value}"
}
}`,
},
],
false,
true,
);

expect(await asyncFeatureStore.initialized()).toBeTruthy();

const flags = await asyncFeatureStore.all(VersionedDataKinds.Features);
expect(Object.keys(flags).length).toEqual(1);

const segments = await asyncFeatureStore.all(VersionedDataKinds.Segments);
expect(Object.keys(segments).length).toEqual(0);

// Need to update the timestamp, or it will think the file has not changed.
filesystem.fileData['file1.json'] = {
timestamp: 100,
data: `{
"flagValues":
{
"${flag2Key}": "differentValue"
}
}`,
};
filesystem.watches['file1.json'][0].cb('change', 'file1.json');

await jest.runAllTimersAsync();

const readFlag = await asyncFeatureStore.get(VersionedDataKinds.Features, flag2Key);
expect(readFlag?.version).toEqual(2);
});

it('it does not update the version when the value does not change', async () => {
await createFileDataSource(
true,
[
{
path: 'file1.json',
data: `{
"flagValues":
{
"${flag2Key}": "${flag2Value}"
}
}`,
},
],
false,
true,
);

expect(await asyncFeatureStore.initialized()).toBeTruthy();

const flags = await asyncFeatureStore.all(VersionedDataKinds.Features);
expect(Object.keys(flags).length).toEqual(1);

const segments = await asyncFeatureStore.all(VersionedDataKinds.Segments);
expect(Object.keys(segments).length).toEqual(0);

// Need to update the timestamp, or it will think the file has not changed.
filesystem.fileData['file1.json'] = {
timestamp: 100,
data: `{
"flagValues":
{
"${flag2Key}": "${flag2Value}"
}
}`,
};
filesystem.watches['file1.json'][0].cb('change', 'file1.json');

await jest.runAllTimersAsync();

const readFlag = await asyncFeatureStore.get(VersionedDataKinds.Features, flag2Key);
expect(readFlag?.version).toEqual(1);
});
});
25 changes: 18 additions & 7 deletions packages/shared/sdk-server/src/data_sources/FileDataSource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,22 +6,23 @@ import {
VoidFunction,
} from '@launchdarkly/js-sdk-common';

import { DataKind, LDFeatureStore, LDFeatureStoreDataStorage } from '../api';
import { DataKind, LDFeatureStoreDataStorage } from '../api';
import { FileDataSourceOptions } from '../api/integrations';
import { LDDataSourceUpdates } from '../api/subsystems';
import { Flag } from '../evaluation/data/Flag';
import { processFlag, processSegment } from '../store/serialization';
import VersionedDataKinds from '../store/VersionedDataKinds';
import FileLoader from './FileLoader';

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

function makeFlagWithValue(key: string, value: any): Flag {
function makeFlagWithValue(key: string, value: any, version: number): Flag {
return {
key,
on: true,
fallthrough: { variation: 0 },
variations: [value],
version: 1,
version,
};
}

Expand All @@ -42,7 +43,7 @@ export default class FileDataSource implements subsystem.LDStreamProcessor {
constructor(
options: FileDataSourceOptions,
filesystem: Filesystem,
private readonly featureStore: LDFeatureStore,
private readonly featureStore: LDDataSourceUpdates,
private initSuccessHandler: VoidFunction = () => {},
private readonly errorHandler?: FileDataSourceErrorHandler,
) {
Expand Down Expand Up @@ -102,6 +103,7 @@ export default class FileDataSource implements subsystem.LDStreamProcessor {

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

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

this.processParsedData(parsed);
this.processParsedData(parsed, oldData);
});

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

private processParsedData(parsed: any) {
private processParsedData(parsed: any, oldData: LDFeatureStoreDataStorage) {
Object.keys(parsed.flags || {}).forEach((key) => {
processFlag(parsed.flags[key]);
this.addItem(VersionedDataKinds.Features, parsed.flags[key]);
});
Object.keys(parsed.flagValues || {}).forEach((key) => {
const flag = makeFlagWithValue(key, parsed.flagValues[key]);
const previousInstance = oldData[VersionedDataKinds.Features.namespace]?.[key];
let { version } = previousInstance ?? { version: 1 };
// If the data is different, then we want to increment the version.
if (
previousInstance &&
JSON.stringify(parsed.flagValues[key]) !== JSON.stringify(previousInstance?.variations?.[0])
) {
version += 1;
}
const flag = makeFlagWithValue(key, parsed.flagValues[key], version);
processFlag(flag);
this.addItem(VersionedDataKinds.Features, flag);
});
Expand Down

0 comments on commit e8e07ef

Please sign in to comment.