diff --git a/packages/shared/sdk-client/src/LDClientImpl.storage.test.ts b/packages/shared/sdk-client/src/LDClientImpl.storage.test.ts index a2042baec..13b295d3b 100644 --- a/packages/shared/sdk-client/src/LDClientImpl.storage.test.ts +++ b/packages/shared/sdk-client/src/LDClientImpl.storage.test.ts @@ -8,15 +8,13 @@ import { DeleteFlag, Flag, Flags, PatchFlag } from './types'; jest.mock('@launchdarkly/js-sdk-common', () => { const actual = jest.requireActual('@launchdarkly/js-sdk-common'); - const { MockStreamingProcessor: mockStreamer } = jest.requireActual( - '@launchdarkly/private-js-mocks', - ); + const { MockStreamingProcessor } = jest.requireActual('@launchdarkly/private-js-mocks'); return { ...actual, ...{ internal: { ...actual.internal, - StreamingProcessor: mockStreamer, + StreamingProcessor: MockStreamingProcessor, }, }, }; @@ -330,7 +328,7 @@ describe('sdk-client storage', () => { test('delete should emit change event', async () => { const deleteResponse = { key: 'dev-test-flag', - version: defaultPutResponse['dev-test-flag'].version, + version: defaultPutResponse['dev-test-flag'].version + 1, }; const allFlags = await identifyGetAllFlags( @@ -344,13 +342,32 @@ describe('sdk-client storage', () => { expect(allFlags).not.toHaveProperty('dev-test-flag'); expect(basicPlatform.storage.set).toHaveBeenCalledWith( 'org:Testy Pizza', - expect.not.stringContaining('dev-test-flag'), + expect.stringContaining('dev-test-flag'), ); - expect(flagsInStorage['dev-test-flag']).toBeUndefined(); + expect(flagsInStorage['dev-test-flag']).toMatchObject({ ...deleteResponse, deleted: true }); expect(emitter.emit).toHaveBeenCalledTimes(3); expect(emitter.emit).toHaveBeenNthCalledWith(3, 'change', context, ['dev-test-flag']); }); + test('delete should not delete equal version', async () => { + const deleteResponse = { + key: 'dev-test-flag', + version: defaultPutResponse['dev-test-flag'].version, + }; + + const allFlags = await identifyGetAllFlags( + false, + defaultPutResponse, + undefined, + deleteResponse, + false, + ); + + expect(allFlags).toHaveProperty('dev-test-flag'); + expect(basicPlatform.storage.set).toHaveBeenCalledTimes(1); + expect(emitter.emit).not.toHaveBeenCalledWith('change'); + }); + test('delete should not delete newer version', async () => { const deleteResponse = { key: 'dev-test-flag', @@ -370,15 +387,17 @@ describe('sdk-client storage', () => { expect(emitter.emit).not.toHaveBeenCalledWith('change'); }); - test('delete should ignore non-existing flag', async () => { + test('delete should add and tombstone non-existing flag', async () => { const deleteResponse = { key: 'does-not-exist', version: 1, }; await identifyGetAllFlags(false, defaultPutResponse, undefined, deleteResponse, false); + const flagsInStorage = JSON.parse(basicPlatform.storage.set.mock.lastCall[1]) as Flags; - expect(basicPlatform.storage.set).toHaveBeenCalledTimes(1); - expect(emitter.emit).not.toHaveBeenCalledWith('change'); + expect(basicPlatform.storage.set).toHaveBeenCalledTimes(2); + expect(flagsInStorage['does-not-exist']).toMatchObject({ ...deleteResponse, deleted: true }); + expect(emitter.emit).toHaveBeenCalledWith('change', context, ['does-not-exist']); }); }); diff --git a/packages/shared/sdk-client/src/LDClientImpl.test.ts b/packages/shared/sdk-client/src/LDClientImpl.test.ts index 52395b85c..bbc396a0b 100644 --- a/packages/shared/sdk-client/src/LDClientImpl.test.ts +++ b/packages/shared/sdk-client/src/LDClientImpl.test.ts @@ -1,16 +1,13 @@ -import { LDContext } from '@launchdarkly/js-sdk-common'; -import { - basicPlatform, - logger, - MockStreamingProcessor, - setupMockStreamingProcessor, -} from '@launchdarkly/private-js-mocks'; +import { clone, LDContext } from '@launchdarkly/js-sdk-common'; +import { basicPlatform, logger, setupMockStreamingProcessor } from '@launchdarkly/private-js-mocks'; import * as mockResponseJson from './evaluation/mockResponse.json'; import LDClientImpl from './LDClientImpl'; +import { Flags } from './types'; jest.mock('@launchdarkly/js-sdk-common', () => { const actual = jest.requireActual('@launchdarkly/js-sdk-common'); + const { MockStreamingProcessor } = jest.requireActual('@launchdarkly/private-js-mocks'); return { ...actual, ...{ @@ -21,13 +18,16 @@ jest.mock('@launchdarkly/js-sdk-common', () => { }, }; }); -describe('sdk-client object', () => { - const testSdkKey = 'test-sdk-key'; - const context: LDContext = { kind: 'org', key: 'Testy Pizza' }; - let ldc: LDClientImpl; +const testSdkKey = 'test-sdk-key'; +const context: LDContext = { kind: 'org', key: 'Testy Pizza' }; +let ldc: LDClientImpl; +let defaultPutResponse: Flags; + +describe('sdk-client object', () => { beforeEach(() => { - setupMockStreamingProcessor(false, mockResponseJson); + defaultPutResponse = clone(mockResponseJson); + setupMockStreamingProcessor(false, defaultPutResponse); ldc = new LDClientImpl(testSdkKey, basicPlatform, { logger, sendEvents: false }); jest @@ -95,8 +95,32 @@ describe('sdk-client object', () => { expect(devTestFlag).toBe(true); }); + test('variationDetail flag not found', async () => { + await ldc.identify(context); + const flag = ldc.variationDetail('does-not-exist', 'not-found'); + + expect(flag).toEqual({ + reason: { errorKind: 'FLAG_NOT_FOUND', kind: 'ERROR' }, + value: 'not-found', + variationIndex: null, + }); + }); + + test('variationDetail deleted flag not found', async () => { + await ldc.identify(context); + // @ts-ignore + ldc.flags['dev-test-flag'].deleted = true; + const flag = ldc.variationDetail('dev-test-flag', 'deleted'); + + expect(flag).toEqual({ + reason: { errorKind: 'FLAG_NOT_FOUND', kind: 'ERROR' }, + value: 'deleted', + variationIndex: null, + }); + }); + test('identify success', async () => { - mockResponseJson['dev-test-flag'].value = false; + defaultPutResponse['dev-test-flag'].value = false; const carContext: LDContext = { kind: 'car', key: 'mazda-cx7' }; await ldc.identify(carContext); diff --git a/packages/shared/sdk-client/src/LDClientImpl.ts b/packages/shared/sdk-client/src/LDClientImpl.ts index e3e061126..cfa3a7179 100644 --- a/packages/shared/sdk-client/src/LDClientImpl.ts +++ b/packages/shared/sdk-client/src/LDClientImpl.ts @@ -79,7 +79,9 @@ export default class LDClientImpl implements LDClient { allFlags(): LDFlagSet { const result: LDFlagSet = {}; Object.entries(this.flags).forEach(([k, r]) => { - result[k] = r.value; + if (!r.deleted) { + result[k] = r.value; + } }); return result; } @@ -154,8 +156,18 @@ export default class LDClientImpl implements LDClient { this.logger.debug(`Streamer DELETE ${JSON.stringify(dataJson, null, 2)}`); const existing = this.flags[dataJson.key]; - if (existing && existing.version < dataJson.version) { - delete this.flags[dataJson.key]; + // the deleted flag is saved as tombstoned + if (!existing || existing.version < dataJson.version) { + this.flags[dataJson.key] = { + ...dataJson, + deleted: true, + // props below are set to sensible defaults. they are irrelevant + // because this flag has been deleted. + flagVersion: 0, + value: undefined, + variation: 0, + trackEvents: false, + }; await this.platform.storage?.set(canonicalKey, JSON.stringify(this.flags)); const changedKeys = [dataJson.key]; this.logger.debug(`Emitting changes from DELETE: ${changedKeys}`); @@ -299,7 +311,7 @@ export default class LDClientImpl implements LDClient { const evalContext = Context.fromLDContext(this.context); const found = this.flags[flagKey]; - if (!found) { + if (!found || found.deleted) { const defVal = defaultValue ?? null; const error = new LDClientError( `Unknown feature flag "${flagKey}"; returning default value ${defVal}`, diff --git a/packages/shared/sdk-client/src/types/index.ts b/packages/shared/sdk-client/src/types/index.ts index 1ab9cb481..18b24736d 100644 --- a/packages/shared/sdk-client/src/types/index.ts +++ b/packages/shared/sdk-client/src/types/index.ts @@ -9,6 +9,7 @@ export interface Flag { trackReason?: boolean; reason?: LDEvaluationReason; debugEventsUntilDate?: number; + deleted?: boolean; } export interface PatchFlag extends Flag {