From 501a8397683887267d3676e6b3dbeda7f8d8a392 Mon Sep 17 00:00:00 2001 From: Sameeran Kunche Date: Mon, 3 Mar 2025 18:17:54 -0800 Subject: [PATCH 1/6] Only check for salt and store initialization if the store is non-empty --- src/client/eppo-precomputed-client.spec.ts | 48 ++++++++++++++++++++++ src/client/eppo-precomputed-client.ts | 27 ++++++------ 2 files changed, 63 insertions(+), 12 deletions(-) diff --git a/src/client/eppo-precomputed-client.spec.ts b/src/client/eppo-precomputed-client.spec.ts index 606ff17..37d362c 100644 --- a/src/client/eppo-precomputed-client.spec.ts +++ b/src/client/eppo-precomputed-client.spec.ts @@ -30,6 +30,7 @@ import EppoPrecomputedClient, { PrecomputedFlagsRequestParameters, Subject, } from './eppo-precomputed-client'; +import { applicationLogger } from '..'; describe('EppoPrecomputedClient E2E test', () => { const precomputedConfigurationWire = readMockConfigurationWireResponse( @@ -52,6 +53,7 @@ describe('EppoPrecomputedClient E2E test', () => { subjectKey: 'test-subject', subjectAttributes: { attr1: 'value1' }, }; + beforeEach(async () => { storage = new MemoryOnlyConfigurationStore(); storage.setFormat(FormatEnum.PRECOMPUTED); @@ -110,6 +112,52 @@ describe('EppoPrecomputedClient E2E test', () => { }); }); + describe('store initialization logged errors', () => { + let mockError: jest.SpyInstance; + + beforeEach(() => { + mockError = jest.spyOn(applicationLogger, 'error'); + }); + + afterEach(() => { + mockError.mockRestore(); + }); + + it('logs error when initialized with store without salt', () => { + const emptyStore = new MemoryOnlyConfigurationStore(); + new EppoPrecomputedClient({ + precomputedFlagStore: emptyStore, + subject: { + subjectKey: '', + subjectAttributes: {}, + }, + }); + expect(mockError).not.toHaveBeenCalledWith( + 'EppoPrecomputedClient requires a precomputedFlagStore with a salt if requestParameters are not provided', + ); + }); + + it('logs error when initialized with store without salt', () => { + const nonemptyStore = new MemoryOnlyConfigurationStore(); + // Incorrectly initialized: no salt, not set to initialized + jest.spyOn(nonemptyStore, 'getKeys').mockReturnValue(['some-key']); + + new EppoPrecomputedClient({ + precomputedFlagStore: nonemptyStore, + subject: { + subjectKey: '', + subjectAttributes: {}, + }, + }); + expect(mockError).toHaveBeenCalledWith( + '[Eppo SDK] EppoPrecomputedClient requires an initialized precomputedFlagStore if requestParameters are not provided', + ); + expect(mockError).toHaveBeenCalledWith( + '[Eppo SDK] EppoPrecomputedClient requires a precomputedFlagStore with a salt if requestParameters are not provided', + ); + }); + }); + describe('setLogger', () => { let flagStorage: IConfigurationStore; let subject: Subject; diff --git a/src/client/eppo-precomputed-client.ts b/src/client/eppo-precomputed-client.ts index 72a843e..3e94333 100644 --- a/src/client/eppo-precomputed-client.ts +++ b/src/client/eppo-precomputed-client.ts @@ -100,13 +100,21 @@ export default class EppoPrecomputedClient { // Online-mode this.requestParameters = options.requestParameters; } else { - // Offline-mode - - // Offline mode depends on pre-populated IConfigurationStores (flags and bandits) to source configuration. - if (!this.precomputedFlagStore.isInitialized()) { - logger.error( - `${loggerPrefix} EppoPrecomputedClient requires an initialized precomputedFlagStore if requestParameters are not provided`, - ); + // Offline-mode -- depends on pre-populated IConfigurationStores (flags and bandits) to source configuration. + + // Allow an empty precomputedFlagStore to be passed in, but if it has items, ensure it was initialized properly. + if (this.precomputedFlagStore.getKeys().length > 0) { + if (!this.precomputedFlagStore.isInitialized()) { + logger.error( + `${loggerPrefix} EppoPrecomputedClient requires an initialized precomputedFlagStore if requestParameters are not provided`, + ); + } + + if (!this.precomputedFlagStore.salt) { + logger.error( + `${loggerPrefix} EppoPrecomputedClient requires a precomputedFlagStore with a salt if requestParameters are not provided`, + ); + } } if (this.precomputedBanditStore && !this.precomputedBanditStore.isInitialized()) { @@ -115,11 +123,6 @@ export default class EppoPrecomputedClient { ); } - if (!this.precomputedFlagStore.salt) { - logger.error( - `${loggerPrefix} EppoPrecomputedClient requires a precomputedFlagStore with a salt if requestParameters are not provided`, - ); - } if (this.precomputedBanditStore && !this.precomputedBanditStore.salt) { logger.warn( From 09e9d045bd932b8e3028d93f86306a0a9d240520 Mon Sep 17 00:00:00 2001 From: Sameeran Kunche Date: Mon, 3 Mar 2025 18:22:44 -0800 Subject: [PATCH 2/6] One more test --- src/client/eppo-precomputed-client.spec.ts | 23 +++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/src/client/eppo-precomputed-client.spec.ts b/src/client/eppo-precomputed-client.spec.ts index 37d362c..63e614e 100644 --- a/src/client/eppo-precomputed-client.spec.ts +++ b/src/client/eppo-precomputed-client.spec.ts @@ -137,7 +137,7 @@ describe('EppoPrecomputedClient E2E test', () => { ); }); - it('logs error when initialized with store without salt', () => { + it('logs errors when constructor receives an uninitialized store without a salt', () => { const nonemptyStore = new MemoryOnlyConfigurationStore(); // Incorrectly initialized: no salt, not set to initialized jest.spyOn(nonemptyStore, 'getKeys').mockReturnValue(['some-key']); @@ -156,6 +156,27 @@ describe('EppoPrecomputedClient E2E test', () => { '[Eppo SDK] EppoPrecomputedClient requires a precomputedFlagStore with a salt if requestParameters are not provided', ); }); + + it('only logs initialization error when constructor receives an uninitialized store with salt', () => { + const nonemptyStore = new MemoryOnlyConfigurationStore(); + nonemptyStore.salt = 'nacl'; + // Incorrectly initialized: no salt, not set to initialized + jest.spyOn(nonemptyStore, 'getKeys').mockReturnValue(['some-key']); + + new EppoPrecomputedClient({ + precomputedFlagStore: nonemptyStore, + subject: { + subjectKey: '', + subjectAttributes: {}, + }, + }); + expect(mockError).toHaveBeenCalledWith( + '[Eppo SDK] EppoPrecomputedClient requires an initialized precomputedFlagStore if requestParameters are not provided', + ); + expect(mockError).not.toHaveBeenCalledWith( + '[Eppo SDK] EppoPrecomputedClient requires a precomputedFlagStore with a salt if requestParameters are not provided', + ); + }); }); describe('setLogger', () => { From ab6256b939d2d1f8a9c5c8d2ff8fda8433177ef2 Mon Sep 17 00:00:00 2001 From: Sameeran Kunche Date: Mon, 3 Mar 2025 18:47:05 -0800 Subject: [PATCH 3/6] Fix failing tests --- src/client/eppo-precomputed-client.spec.ts | 152 ++++++++++----------- 1 file changed, 72 insertions(+), 80 deletions(-) diff --git a/src/client/eppo-precomputed-client.spec.ts b/src/client/eppo-precomputed-client.spec.ts index 63e614e..6148ad9 100644 --- a/src/client/eppo-precomputed-client.spec.ts +++ b/src/client/eppo-precomputed-client.spec.ts @@ -112,73 +112,6 @@ describe('EppoPrecomputedClient E2E test', () => { }); }); - describe('store initialization logged errors', () => { - let mockError: jest.SpyInstance; - - beforeEach(() => { - mockError = jest.spyOn(applicationLogger, 'error'); - }); - - afterEach(() => { - mockError.mockRestore(); - }); - - it('logs error when initialized with store without salt', () => { - const emptyStore = new MemoryOnlyConfigurationStore(); - new EppoPrecomputedClient({ - precomputedFlagStore: emptyStore, - subject: { - subjectKey: '', - subjectAttributes: {}, - }, - }); - expect(mockError).not.toHaveBeenCalledWith( - 'EppoPrecomputedClient requires a precomputedFlagStore with a salt if requestParameters are not provided', - ); - }); - - it('logs errors when constructor receives an uninitialized store without a salt', () => { - const nonemptyStore = new MemoryOnlyConfigurationStore(); - // Incorrectly initialized: no salt, not set to initialized - jest.spyOn(nonemptyStore, 'getKeys').mockReturnValue(['some-key']); - - new EppoPrecomputedClient({ - precomputedFlagStore: nonemptyStore, - subject: { - subjectKey: '', - subjectAttributes: {}, - }, - }); - expect(mockError).toHaveBeenCalledWith( - '[Eppo SDK] EppoPrecomputedClient requires an initialized precomputedFlagStore if requestParameters are not provided', - ); - expect(mockError).toHaveBeenCalledWith( - '[Eppo SDK] EppoPrecomputedClient requires a precomputedFlagStore with a salt if requestParameters are not provided', - ); - }); - - it('only logs initialization error when constructor receives an uninitialized store with salt', () => { - const nonemptyStore = new MemoryOnlyConfigurationStore(); - nonemptyStore.salt = 'nacl'; - // Incorrectly initialized: no salt, not set to initialized - jest.spyOn(nonemptyStore, 'getKeys').mockReturnValue(['some-key']); - - new EppoPrecomputedClient({ - precomputedFlagStore: nonemptyStore, - subject: { - subjectKey: '', - subjectAttributes: {}, - }, - }); - expect(mockError).toHaveBeenCalledWith( - '[Eppo SDK] EppoPrecomputedClient requires an initialized precomputedFlagStore if requestParameters are not provided', - ); - expect(mockError).not.toHaveBeenCalledWith( - '[Eppo SDK] EppoPrecomputedClient requires a precomputedFlagStore with a salt if requestParameters are not provided', - ); - }); - }); - describe('setLogger', () => { let flagStorage: IConfigurationStore; let subject: Subject; @@ -835,6 +768,77 @@ describe('EppoPrecomputedClient E2E test', () => { expect(loggedEvent.format).toEqual(FormatEnum.PRECOMPUTED); }); + + describe('Constructor logs errors according to the store state', () => { + let mockError: jest.SpyInstance; + + beforeEach(() => { + mockError = jest.spyOn(applicationLogger, 'error'); + }); + + afterEach(() => { + mockError.mockRestore(); + }); + + it('does not log errors when constructor receives an empty, uninitialized store', () => { + const emptyStore = new MemoryOnlyConfigurationStore(); + new EppoPrecomputedClient({ + precomputedFlagStore: emptyStore, + subject: { + subjectKey: '', + subjectAttributes: {}, + }, + }); + expect(mockError).not.toHaveBeenCalledWith( + '[Eppo SDK] EppoPrecomputedClient requires an initialized precomputedFlagStore if requestParameters are not provided', + ); + expect(mockError).not.toHaveBeenCalledWith( + '[Eppo SDK] EppoPrecomputedClient requires a precomputedFlagStore with a salt if requestParameters are not provided', + ); + }); + + it('logs errors when constructor receives an uninitialized store without a salt', () => { + const nonemptyStore = new MemoryOnlyConfigurationStore(); + // Incorrectly initialized: no salt, not set to initialized + jest.spyOn(nonemptyStore, 'getKeys').mockReturnValue(['some-key']); + + new EppoPrecomputedClient({ + precomputedFlagStore: nonemptyStore, + subject: { + subjectKey: '', + subjectAttributes: {}, + }, + }); + expect(mockError).toHaveBeenCalledWith( + '[Eppo SDK] EppoPrecomputedClient requires an initialized precomputedFlagStore if requestParameters are not provided', + ); + expect(mockError).toHaveBeenCalledWith( + '[Eppo SDK] EppoPrecomputedClient requires a precomputedFlagStore with a salt if requestParameters are not provided', + ); + }); + + it('only logs initialization error when constructor receives an uninitialized store with salt', () => { + const nonemptyStore = new MemoryOnlyConfigurationStore(); + nonemptyStore.salt = 'nacl'; + // Incorrectly initialized: no salt, not set to initialized + jest.spyOn(nonemptyStore, 'getKeys').mockReturnValue(['some-key']); + + new EppoPrecomputedClient({ + precomputedFlagStore: nonemptyStore, + subject: { + subjectKey: '', + subjectAttributes: {}, + }, + }); + expect(mockError).toHaveBeenCalledWith( + '[Eppo SDK] EppoPrecomputedClient requires an initialized precomputedFlagStore if requestParameters are not provided', + ); + expect(mockError).not.toHaveBeenCalledWith( + '[Eppo SDK] EppoPrecomputedClient requires a precomputedFlagStore with a salt if requestParameters are not provided', + ); + }); + }); + describe('EppoPrecomputedClient subject data and store initialization', () => { let client: EppoPrecomputedClient; let store: IConfigurationStore; @@ -853,13 +857,7 @@ describe('EppoPrecomputedClient E2E test', () => { subject, }); }).not.toThrow(); - expect(loggerErrorSpy).toHaveBeenCalledTimes(2); - expect(loggerErrorSpy).toHaveBeenCalledWith( - '[Eppo SDK] EppoPrecomputedClient requires an initialized precomputedFlagStore if requestParameters are not provided', - ); - expect(loggerErrorSpy).toHaveBeenCalledWith( - '[Eppo SDK] EppoPrecomputedClient requires a precomputedFlagStore with a salt if requestParameters are not provided', - ); + expect(loggerErrorSpy).toHaveBeenCalledTimes(0); loggerErrorSpy.mockRestore(); expect(client.getStringAssignment('string-flag', 'default')).toBe('default'); }); @@ -953,12 +951,6 @@ describe('Precomputed Bandit Store', () => { subject, }); - expect(loggerErrorSpy).toHaveBeenCalledWith( - '[Eppo SDK] EppoPrecomputedClient requires an initialized precomputedFlagStore if requestParameters are not provided', - ); - expect(loggerErrorSpy).toHaveBeenCalledWith( - '[Eppo SDK] EppoPrecomputedClient requires a precomputedFlagStore with a salt if requestParameters are not provided', - ); expect(loggerErrorSpy).toHaveBeenCalledWith( '[Eppo SDK] Passing banditOptions without requestParameters requires an initialized precomputedBanditStore', ); From d4b869d5fa93253f5be0c025c27bcabfa6408bce Mon Sep 17 00:00:00 2001 From: Sameeran Kunche Date: Mon, 3 Mar 2025 18:49:51 -0800 Subject: [PATCH 4/6] Some eslint fixes --- src/client/eppo-precomputed-client.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/client/eppo-precomputed-client.spec.ts b/src/client/eppo-precomputed-client.spec.ts index 6148ad9..89f2064 100644 --- a/src/client/eppo-precomputed-client.spec.ts +++ b/src/client/eppo-precomputed-client.spec.ts @@ -30,7 +30,7 @@ import EppoPrecomputedClient, { PrecomputedFlagsRequestParameters, Subject, } from './eppo-precomputed-client'; -import { applicationLogger } from '..'; + describe('EppoPrecomputedClient E2E test', () => { const precomputedConfigurationWire = readMockConfigurationWireResponse( @@ -773,7 +773,7 @@ describe('EppoPrecomputedClient E2E test', () => { let mockError: jest.SpyInstance; beforeEach(() => { - mockError = jest.spyOn(applicationLogger, 'error'); + mockError = jest.spyOn(logger, 'error'); }); afterEach(() => { From ee984f367bc0f014e342edbc2f340e4688f7ca1f Mon Sep 17 00:00:00 2001 From: Sameeran Kunche Date: Mon, 3 Mar 2025 22:18:12 -0800 Subject: [PATCH 5/6] v4.12.1-alpha.0 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 0779c6d..382a074 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@eppo/js-client-sdk-common", - "version": "4.12.0", + "version": "4.12.1-alpha.0", "description": "Common library for Eppo JavaScript SDKs (web, react native, and node)", "main": "dist/index.js", "files": [ From 8371bd88614501326a455d98b5b3a122d700d604 Mon Sep 17 00:00:00 2001 From: Sameeran Kunche Date: Mon, 3 Mar 2025 22:23:50 -0800 Subject: [PATCH 6/6] More lint warnings --- src/client/eppo-precomputed-client.spec.ts | 2 -- src/client/eppo-precomputed-client.ts | 1 - 2 files changed, 3 deletions(-) diff --git a/src/client/eppo-precomputed-client.spec.ts b/src/client/eppo-precomputed-client.spec.ts index 89f2064..d2b14ac 100644 --- a/src/client/eppo-precomputed-client.spec.ts +++ b/src/client/eppo-precomputed-client.spec.ts @@ -31,7 +31,6 @@ import EppoPrecomputedClient, { Subject, } from './eppo-precomputed-client'; - describe('EppoPrecomputedClient E2E test', () => { const precomputedConfigurationWire = readMockConfigurationWireResponse( MOCK_PRECOMPUTED_WIRE_FILE, @@ -768,7 +767,6 @@ describe('EppoPrecomputedClient E2E test', () => { expect(loggedEvent.format).toEqual(FormatEnum.PRECOMPUTED); }); - describe('Constructor logs errors according to the store state', () => { let mockError: jest.SpyInstance; diff --git a/src/client/eppo-precomputed-client.ts b/src/client/eppo-precomputed-client.ts index 3e94333..7db1436 100644 --- a/src/client/eppo-precomputed-client.ts +++ b/src/client/eppo-precomputed-client.ts @@ -123,7 +123,6 @@ export default class EppoPrecomputedClient { ); } - if (this.precomputedBanditStore && !this.precomputedBanditStore.salt) { logger.warn( `${loggerPrefix} EppoPrecomputedClient missing or empty salt for precomputedBanditStore`,