From d11f76e54d83ee2e907648f61932ea7d46f4260c Mon Sep 17 00:00:00 2001 From: Peter Zhu Date: Thu, 1 Aug 2024 13:10:32 -0700 Subject: [PATCH] Log error on eval, dont init fail on if cohort fail, add tests --- packages/node/src/local/client.ts | 41 +- packages/node/src/local/cohort/cohort-api.ts | 4 +- packages/node/src/local/cohort/poller.ts | 10 +- packages/node/src/local/poller.ts | 4 +- packages/node/src/local/streamer.ts | 4 +- packages/node/src/local/updater.ts | 62 +- packages/node/src/util/cohort.ts | 23 +- packages/node/test/local/client.test.ts | 837 ++++++++++++------ .../test/local/cohort/cohortPoller.test.ts | 17 +- .../node/test/local/flagConfigPoller.test.ts | 64 +- .../test/local/flagConfigStreamer.test.ts | 85 +- .../node/test/local/flagConfigUpdater.test.ts | 32 +- .../node/test/local/util/cohortUtils.test.ts | 182 +--- packages/node/test/local/util/flags.ts | 164 ---- packages/node/test/local/util/mockData.ts | 273 ++++++ 15 files changed, 1010 insertions(+), 792 deletions(-) delete mode 100644 packages/node/test/local/util/flags.ts create mode 100644 packages/node/test/local/util/mockData.ts diff --git a/packages/node/src/local/client.ts b/packages/node/src/local/client.ts index 7cd4db6..10c6cf5 100644 --- a/packages/node/src/local/client.ts +++ b/packages/node/src/local/client.ts @@ -88,7 +88,7 @@ export class LocalEvaluationClient { this.logger = new ConsoleLogger(this.config.debug); this.cohortStorage = new InMemoryCohortStorage(); - let cohortFetcher = undefined; + let cohortFetcher: CohortFetcher = undefined; if (this.config.cohortConfig) { cohortFetcher = new CohortFetcher( this.config.cohortConfig.apiKey, @@ -102,6 +102,7 @@ export class LocalEvaluationClient { this.cohortUpdater = new CohortPoller( cohortFetcher, this.cohortStorage, + this.cache, 60000, this.config.debug, ); @@ -188,11 +189,47 @@ export class LocalEvaluationClient { return evaluationVariantsToVariants(results); } + protected checkFlagsCohortsAvailable( + cohortIdsByFlag: Record>, + ): boolean { + const availableCohortIds = this.cohortStorage.getAllCohortIds(); + for (const key in cohortIdsByFlag) { + const flagCohortIds = cohortIdsByFlag[key]; + const unavailableCohortIds = CohortUtils.setSubtract( + flagCohortIds, + availableCohortIds, + ); + if (unavailableCohortIds.size > 0) { + this.logger.error( + `[Experiment] Flag ${key} has cohort ids ${[ + ...unavailableCohortIds, + ]} unavailable, evaluation may be incorrect`, + ); + return false; + } + } + return true; + } + protected enrichUserWithCohorts( user: ExperimentUser, flags: Record, ): void { - const cohortIdsByGroup = CohortUtils.extractCohortIdsByGroup(flags); + const cohortIdsByFlag: Record> = {}; + const cohortIdsByGroup = {}; + for (const key in flags) { + const cohortIdsByGroupOfFlag = + CohortUtils.extractCohortIdsByGroupFromFlag(flags[key]); + + CohortUtils.mergeValuesOfBIntoValuesOfA( + cohortIdsByGroup, + cohortIdsByGroupOfFlag, + ); + + cohortIdsByFlag[key] = CohortUtils.mergeAllValues(cohortIdsByGroupOfFlag); + } + + this.checkFlagsCohortsAvailable(cohortIdsByFlag); // Enrich cohorts with user group type. const userCohortIds = cohortIdsByGroup[USER_GROUP_TYPE]; diff --git a/packages/node/src/local/cohort/cohort-api.ts b/packages/node/src/local/cohort/cohort-api.ts index a033d6c..5d10994 100644 --- a/packages/node/src/local/cohort/cohort-api.ts +++ b/packages/node/src/local/cohort/cohort-api.ts @@ -77,11 +77,11 @@ export class SdkCohortApi implements CohortApi { return undefined; } else if (response.status == 413) { throw new CohortMaxSizeExceededError( - `Cohort error response: size > ${options.maxCohortSize}`, + `Cohort size > ${options.maxCohortSize}`, ); } else { throw new CohortDownloadError( - `Cohort error response: status ${response.status}, body ${response.body}`, + `Cohort error response status ${response.status}, body ${response.body}`, ); } } diff --git a/packages/node/src/local/cohort/poller.ts b/packages/node/src/local/cohort/poller.ts index e7ab9bd..8326b6c 100644 --- a/packages/node/src/local/cohort/poller.ts +++ b/packages/node/src/local/cohort/poller.ts @@ -1,4 +1,6 @@ import { CohortStorage } from '../../types/cohort'; +import { FlagConfigCache } from '../../types/flag'; +import { CohortUtils } from '../../util/cohort'; import { ConsoleLogger } from '../../util/logger'; import { Logger } from '../../util/logger'; @@ -10,6 +12,7 @@ export class CohortPoller implements CohortUpdater { public readonly fetcher: CohortFetcher; public readonly storage: CohortStorage; + public readonly flagCache: FlagConfigCache; private poller: NodeJS.Timeout; private pollingIntervalMillis: number; @@ -17,11 +20,13 @@ export class CohortPoller implements CohortUpdater { constructor( fetcher: CohortFetcher, storage: CohortStorage, + flagCache: FlagConfigCache, pollingIntervalMillis = 60000, debug = false, ) { this.fetcher = fetcher; this.storage = storage; + this.flagCache = flagCache; this.pollingIntervalMillis = pollingIntervalMillis; this.logger = new ConsoleLogger(debug); } @@ -64,8 +69,11 @@ export class CohortPoller implements CohortUpdater { ): Promise { let changed = false; const promises = []; + const cohortIds = CohortUtils.extractCohortIds( + await this.flagCache.getAll(), + ); - for (const cohortId of this.storage.getAllCohortIds()) { + for (const cohortId of cohortIds) { this.logger.debug(`[Experiment] updating cohort ${cohortId}`); // Get existing cohort and lastModified. diff --git a/packages/node/src/local/poller.ts b/packages/node/src/local/poller.ts index b523d3e..e6ad1eb 100644 --- a/packages/node/src/local/poller.ts +++ b/packages/node/src/local/poller.ts @@ -65,7 +65,7 @@ export class FlagConfigPoller async () => await this.fetcher.fetch(), BACKOFF_POLICY, ); - await super._update(flagConfigs, true, onChange); + await super._update(flagConfigs, onChange); } catch (e) { this.logger.error( '[Experiment] flag config initial poll failed, stopping', @@ -95,6 +95,6 @@ export class FlagConfigPoller ): Promise { this.logger.debug('[Experiment] updating flag configs'); const flagConfigs = await this.fetcher.fetch(); - await super._update(flagConfigs, false, onChange); + await super._update(flagConfigs, onChange); } } diff --git a/packages/node/src/local/streamer.ts b/packages/node/src/local/streamer.ts index 7f89ade..84653d2 100644 --- a/packages/node/src/local/streamer.ts +++ b/packages/node/src/local/streamer.ts @@ -74,12 +74,12 @@ export class FlagConfigStreamer this.stream.onInitUpdate = async (flagConfigs) => { this.logger.debug('[Experiment] streamer - receives updates'); - await super._update(flagConfigs, true, onChange); + await super._update(flagConfigs, onChange); this.logger.debug('[Experiment] streamer - start flags stream success'); }; this.stream.onUpdate = async (flagConfigs) => { this.logger.debug('[Experiment] streamer - receives updates'); - await super._update(flagConfigs, false, onChange); + await super._update(flagConfigs, onChange); }; try { diff --git a/packages/node/src/local/updater.ts b/packages/node/src/local/updater.ts index 1068c58..5e88bf1 100644 --- a/packages/node/src/local/updater.ts +++ b/packages/node/src/local/updater.ts @@ -52,7 +52,6 @@ export class FlagConfigUpdaterBase { protected async _update( flagConfigs: Record, - isInit: boolean, onChange?: (cache: FlagConfigCache) => Promise, ): Promise { let changed = false; @@ -66,30 +65,20 @@ export class FlagConfigUpdaterBase { // Get all cohort needs update. const cohortIds = CohortUtils.extractCohortIds(flagConfigs); if (cohortIds && cohortIds.size > 0 && !this.cohortFetcher) { - throw Error( - 'cohort found in flag configs but no cohort download configured', + this.logger.error( + 'Cohorts found in flag configs but no cohort download configured', ); + } else { + // Download new cohorts into cohortStorage. + await this.downloadNewCohorts(cohortIds); } - // Download new cohorts into cohortStorage. - const failedCohortIds = await this.downloadNewCohorts(cohortIds); - if (isInit && failedCohortIds.size > 0) { - throw Error('Cohort download failed'); - } - - // Update the flags that has all cohorts successfully updated into flags cache. - const newFlagConfigs = await this.filterFlagConfigsWithFullCohorts( - flagConfigs, - ); - // Update the flags with new flags. await this.cache.clear(); - await this.cache.putAll(newFlagConfigs); + await this.cache.putAll(flagConfigs); // Remove cohorts not used by new flags. - await this.removeUnusedCohorts( - CohortUtils.extractCohortIds(newFlagConfigs), - ); + await this.removeUnusedCohorts(cohortIds); if (changed) { await onChange(this.cache); @@ -111,8 +100,8 @@ export class FlagConfigUpdaterBase { } }) .catch((err) => { - this.logger.warn( - `[Experiment] Cohort download failed ${cohortId}, using existing cohort if exist`, + this.logger.error( + `[Experiment] Cohort download failed ${cohortId}`, err, ); failedCohortIds.add(cohortId); @@ -122,39 +111,6 @@ export class FlagConfigUpdaterBase { return failedCohortIds; } - protected async filterFlagConfigsWithFullCohorts( - flagConfigs: Record, - ): Promise> { - const newFlagConfigs = {}; - const availableCohortIds = this.cohortStorage.getAllCohortIds(); - for (const flagKey in flagConfigs) { - // Get cohorts for this flag. - const cohortIds = CohortUtils.extractCohortIdsFromFlag( - flagConfigs[flagKey], - ); - - // Check if all cohorts for this flag has downloaded. - // If any cohort failed, don't use the new flag. - const updateFlag = - cohortIds.size === 0 || - CohortUtils.setSubtract(cohortIds, availableCohortIds).size === 0; - - if (updateFlag) { - newFlagConfigs[flagKey] = flagConfigs[flagKey]; - } else { - this.logger.warn( - `[Experiment] Flag ${flagKey} failed to update due to cohort update failure`, - ); - const existingFlag = await this.cache.get(flagKey); - if (existingFlag) { - newFlagConfigs[flagKey] = existingFlag; - } - } - } - - return newFlagConfigs; - } - protected async removeUnusedCohorts( validCohortIds: Set, ): Promise { diff --git a/packages/node/src/util/cohort.ts b/packages/node/src/util/cohort.ts index 516541e..f06b015 100644 --- a/packages/node/src/util/cohort.ts +++ b/packages/node/src/util/cohort.ts @@ -20,28 +20,13 @@ export class CohortUtils { public static extractCohortIds( flagConfigs: Record, ): Set { - return CohortUtils.mergeAllValues( - CohortUtils.extractCohortIdsByGroup(flagConfigs), - ); - } - - public static extractCohortIdsByGroup( - flagConfigs: Record, - ): Record> { - const cohortIdsByGroup = {}; + const cohortIdsByFlag = {}; for (const key in flagConfigs) { - CohortUtils.mergeBIntoA( - cohortIdsByGroup, + cohortIdsByFlag[key] = CohortUtils.mergeAllValues( CohortUtils.extractCohortIdsByGroupFromFlag(flagConfigs[key]), ); } - return cohortIdsByGroup; - } - - public static extractCohortIdsFromFlag(flag: FlagConfig): Set { - return CohortUtils.mergeAllValues( - CohortUtils.extractCohortIdsByGroupFromFlag(flag), - ); + return CohortUtils.mergeAllValues(cohortIdsByFlag); } public static extractCohortIdsByGroupFromFlag( @@ -85,7 +70,7 @@ export class CohortUtils { return cohortIdsByGroup; } - public static mergeBIntoA( + public static mergeValuesOfBIntoValuesOfA( a: Record>, b: Record>, ): void { diff --git a/packages/node/test/local/client.test.ts b/packages/node/test/local/client.test.ts index d348baf..b75d93d 100644 --- a/packages/node/test/local/client.test.ts +++ b/packages/node/test/local/client.test.ts @@ -3,10 +3,23 @@ import path from 'path'; import { EvaluationFlag } from '@amplitude/experiment-core'; import * as dotenv from 'dotenv'; import { Experiment } from 'src/factory'; -import { InMemoryFlagConfigCache, LocalEvaluationClient } from 'src/index'; +import { + FlagConfigFetcher, + InMemoryFlagConfigCache, + LocalEvaluationClient, +} from 'src/index'; import { USER_GROUP_TYPE } from 'src/types/cohort'; import { LocalEvaluationDefaults } from 'src/types/config'; import { ExperimentUser } from 'src/types/user'; +import { MockHttpClient } from './util/mockHttpClient'; +import { COHORTS, FLAGS, NEW_FLAGS, getFlagWithCohort } from './util/mockData'; +import { CohortFetcher } from 'src/local/cohort/fetcher'; +import { + CohortDownloadError, + CohortMaxSizeExceededError, + SdkCohortApi, +} from 'src/local/cohort/cohort-api'; +import { sleep } from 'src/util/time'; dotenv.config({ path: path.join(__dirname, '../../', '.env') }); @@ -20,321 +33,597 @@ if (!process.env['API_KEY'] && !process.env['SECRET_KEY']) { ); } -const cohortConfig = { - apiKey: process.env['API_KEY'], - secretKey: process.env['SECRET_KEY'], -}; -const client = Experiment.initializeLocal(apiKey, { - cohortConfig: cohortConfig, -}); - -beforeAll(async () => { - await client.start(); -}); +const setupEvaluateTestNormalCases = (client: LocalEvaluationClient) => { + test('ExperimentClient.evaluate all flags, success', async () => { + const variants = await client.evaluate(testUser); + const variant = variants['sdk-local-evaluation-ci-test']; + expect(variant.key).toEqual('on'); + expect(variant.value).toEqual('on'); + expect(variant.payload).toEqual('payload'); + }); -afterAll(async () => { - client.stop(); -}); + test('ExperimentClient.evaluate one flag, success', async () => { + const variants = await client.evaluate(testUser, [ + 'sdk-local-evaluation-ci-test', + ]); + const variant = variants['sdk-local-evaluation-ci-test']; + expect(variant.key).toEqual('on'); + expect(variant.value).toEqual('on'); + expect(variant.payload).toEqual('payload'); + }); -test('ExperimentClient.evaluate all flags, success', async () => { - const variants = await client.evaluate(testUser); - const variant = variants['sdk-local-evaluation-ci-test']; - expect(variant.key).toEqual('on'); - expect(variant.value).toEqual('on'); - expect(variant.payload).toEqual('payload'); -}); + test('ExperimentClient.evaluate with dependencies, no flag keys, success', async () => { + const variants = await client.evaluate({ + user_id: 'user_id', + device_id: 'device_id', + }); + const variant = variants['sdk-ci-local-dependencies-test']; + expect(variant.key).toEqual('control'); + expect(variant.value).toEqual('control'); + }); -test('ExperimentClient.evaluate one flag, success', async () => { - const variants = await client.evaluate(testUser, [ - 'sdk-local-evaluation-ci-test', - ]); - const variant = variants['sdk-local-evaluation-ci-test']; - expect(variant.key).toEqual('on'); - expect(variant.value).toEqual('on'); - expect(variant.payload).toEqual('payload'); -}); + test('ExperimentClient.evaluate with dependencies, with flag keys, success', async () => { + const variants = await client.evaluate( + { + user_id: 'user_id', + device_id: 'device_id', + }, + ['sdk-ci-local-dependencies-test'], + ); + const variant = variants['sdk-ci-local-dependencies-test']; + expect(variant.key).toEqual('control'); + expect(variant.value).toEqual('control'); + }); -test('ExperimentClient.evaluate with dependencies, no flag keys, success', async () => { - const variants = await client.evaluate({ - user_id: 'user_id', - device_id: 'device_id', + test('ExperimentClient.evaluate with dependencies, with unknown flag keys, no variant', async () => { + const variants = await client.evaluate( + { + user_id: 'user_id', + device_id: 'device_id', + }, + ['does-not-exist'], + ); + const variant = variants['sdk-ci-local-dependencies-test']; + expect(variant).toBeUndefined(); }); - const variant = variants['sdk-ci-local-dependencies-test']; - expect(variant.key).toEqual('control'); - expect(variant.value).toEqual('control'); -}); -test('ExperimentClient.evaluate with dependencies, with flag keys, success', async () => { - const variants = await client.evaluate( - { + test('ExperimentClient.evaluate with dependencies, variant held out', async () => { + const variants = await client.evaluate({ user_id: 'user_id', device_id: 'device_id', - }, - ['sdk-ci-local-dependencies-test'], - ); - const variant = variants['sdk-ci-local-dependencies-test']; - expect(variant.key).toEqual('control'); - expect(variant.value).toEqual('control'); -}); + }); + const variant = variants['sdk-ci-local-dependencies-test-holdout']; + expect(variant).toBeUndefined(); + expect( + await client.cache.get('sdk-ci-local-dependencies-test-holdout'), + ).toBeDefined(); + }); + + // Evaluate V2. + test('ExperimentClient.evaluateV2 all flags, success', async () => { + const variants = await client.evaluateV2(testUser); + const variant = variants['sdk-local-evaluation-ci-test']; + expect(variant.key).toEqual('on'); + expect(variant.value).toEqual('on'); + expect(variant.payload).toEqual('payload'); + }); -test('ExperimentClient.evaluate with dependencies, with unknown flag keys, no variant', async () => { - const variants = await client.evaluate( - { + test('ExperimentClient.evaluateV2 one flag, success', async () => { + const variants = await client.evaluateV2(testUser, [ + 'sdk-local-evaluation-ci-test', + ]); + const variant = variants['sdk-local-evaluation-ci-test']; + expect(variant.key).toEqual('on'); + expect(variant.value).toEqual('on'); + expect(variant.payload).toEqual('payload'); + }); + + test('ExperimentClient.evaluateV2 with dependencies, no flag keys, success', async () => { + const variants = await client.evaluateV2({ user_id: 'user_id', device_id: 'device_id', - }, - ['does-not-exist'], - ); - const variant = variants['sdk-ci-local-dependencies-test']; - expect(variant).toBeUndefined(); -}); + }); + const variant = variants['sdk-ci-local-dependencies-test']; + expect(variant.key).toEqual('control'); + expect(variant.value).toEqual('control'); + }); -test('ExperimentClient.evaluate with dependencies, variant held out', async () => { - const variants = await client.evaluate({ - user_id: 'user_id', - device_id: 'device_id', + test('ExperimentClient.evaluateV2 with dependencies, with flag keys, success', async () => { + const variants = await client.evaluateV2( + { + user_id: 'user_id', + device_id: 'device_id', + }, + ['sdk-ci-local-dependencies-test'], + ); + const variant = variants['sdk-ci-local-dependencies-test']; + expect(variant.key).toEqual('control'); + expect(variant.value).toEqual('control'); }); - const variant = variants['sdk-ci-local-dependencies-test-holdout']; - expect(variant).toBeUndefined(); - expect( - await client.cache.get('sdk-ci-local-dependencies-test-holdout'), - ).toBeDefined(); -}); -// Evaluate V2 + test('ExperimentClient.evaluateV2 with dependencies, with unknown flag keys, no variant', async () => { + const variants = await client.evaluateV2( + { + user_id: 'user_id', + device_id: 'device_id', + }, + ['does-not-exist'], + ); + const variant = variants['sdk-ci-local-dependencies-test']; + expect(variant).toBeUndefined(); + }); -test('ExperimentClient.evaluateV2 all flags, success', async () => { - const variants = await client.evaluateV2(testUser); - const variant = variants['sdk-local-evaluation-ci-test']; - expect(variant.key).toEqual('on'); - expect(variant.value).toEqual('on'); - expect(variant.payload).toEqual('payload'); -}); + test('ExperimentClient.evaluateV2 with dependencies, variant held out', async () => { + const variants = await client.evaluateV2({ + user_id: 'user_id', + device_id: 'device_id', + }); + const variant = variants['sdk-ci-local-dependencies-test-holdout']; + expect(variant.key).toEqual('off'); + expect(variant.value).toBeUndefined(); + expect( + await client.cache.get('sdk-ci-local-dependencies-test-holdout'), + ).toBeDefined(); + }); +}; -test('ExperimentClient.evaluateV2 one flag, success', async () => { - const variants = await client.evaluateV2(testUser, [ - 'sdk-local-evaluation-ci-test', - ]); - const variant = variants['sdk-local-evaluation-ci-test']; - expect(variant.key).toEqual('on'); - expect(variant.value).toEqual('on'); - expect(variant.payload).toEqual('payload'); -}); +const setupEvaluateCohortTestNormalCases = (client: LocalEvaluationClient) => { + test('ExperimentClient.evaluateV2 with user or group cohort not targeted', async () => { + const variants = await client.evaluateV2({ + user_id: '2333', + device_id: 'device_id', + groups: { + 'org name': ['Amplitude Inc sth sth sth'], + }, + }); + const userVariant = variants['sdk-local-evaluation-user-cohort-ci-test']; + expect(userVariant.key).toEqual('off'); + expect(userVariant.value).toBeUndefined(); + expect( + await client.cache.get('sdk-local-evaluation-user-cohort-ci-test'), + ).toBeDefined(); + const groupVariant = variants['sdk-local-evaluation-group-cohort-ci-test']; + expect(groupVariant.key).toEqual('off'); + expect(groupVariant.value).toBeUndefined(); + expect( + await client.cache.get('sdk-local-evaluation-group-cohort-ci-test'), + ).toBeDefined(); + }); -test('ExperimentClient.evaluateV2 with dependencies, no flag keys, success', async () => { - const variants = await client.evaluateV2({ - user_id: 'user_id', - device_id: 'device_id', + test('ExperimentClient.evaluateV2 with user cohort segment targeted', async () => { + const variants = await client.evaluateV2({ + user_id: '12345', + device_id: 'device_id', + }); + const variant = variants['sdk-local-evaluation-user-cohort-ci-test']; + expect(variant.key).toEqual('on'); + expect(variant.value).toEqual('on'); + expect( + await client.cache.get('sdk-local-evaluation-user-cohort-ci-test'), + ).toBeDefined(); }); - const variant = variants['sdk-ci-local-dependencies-test']; - expect(variant.key).toEqual('control'); - expect(variant.value).toEqual('control'); -}); -test('ExperimentClient.evaluateV2 with dependencies, with flag keys, success', async () => { - const variants = await client.evaluateV2( - { - user_id: 'user_id', + test('ExperimentClient.evaluateV2 with user cohort tester targeted', async () => { + const variants = await client.evaluateV2({ + user_id: '1', device_id: 'device_id', - }, - ['sdk-ci-local-dependencies-test'], - ); - const variant = variants['sdk-ci-local-dependencies-test']; - expect(variant.key).toEqual('control'); - expect(variant.value).toEqual('control'); -}); + }); + const variant = variants['sdk-local-evaluation-user-cohort-ci-test']; + expect(variant.key).toEqual('on'); + expect(variant.value).toEqual('on'); + expect( + await client.cache.get('sdk-local-evaluation-user-cohort-ci-test'), + ).toBeDefined(); + }); -test('ExperimentClient.evaluateV2 with dependencies, with unknown flag keys, no variant', async () => { - const variants = await client.evaluateV2( - { - user_id: 'user_id', + test('ExperimentClient.evaluateV2 with group cohort segment targeted', async () => { + const variants = await client.evaluateV2({ + user_id: '12345', device_id: 'device_id', - }, - ['does-not-exist'], - ); - const variant = variants['sdk-ci-local-dependencies-test']; - expect(variant).toBeUndefined(); -}); + groups: { + 'org id': ['1'], + }, + }); + const variant = variants['sdk-local-evaluation-group-cohort-ci-test']; + expect(variant.key).toEqual('on'); + expect(variant.value).toEqual('on'); + expect( + await client.cache.get('sdk-local-evaluation-group-cohort-ci-test'), + ).toBeDefined(); + }); -test('ExperimentClient.evaluateV2 with dependencies, variant held out', async () => { - const variants = await client.evaluateV2({ - user_id: 'user_id', - device_id: 'device_id', + test('ExperimentClient.evaluateV2 with group cohort tester targeted', async () => { + const variants = await client.evaluateV2({ + user_id: '2333', + device_id: 'device_id', + groups: { + 'org name': ['Amplitude Website (Portfolio)'], + }, + }); + const variant = variants['sdk-local-evaluation-group-cohort-ci-test']; + expect(variant.key).toEqual('on'); + expect(variant.value).toEqual('on'); + expect( + await client.cache.get('sdk-local-evaluation-group-cohort-ci-test'), + ).toBeDefined(); }); - const variant = variants['sdk-ci-local-dependencies-test-holdout']; - expect(variant.key).toEqual('off'); - expect(variant.value).toBeUndefined(); - expect( - await client.cache.get('sdk-ci-local-dependencies-test-holdout'), - ).toBeDefined(); -}); +}; -test('ExperimentClient.evaluateV2 with user or group cohort not targeted', async () => { - const variants = await client.evaluateV2({ - user_id: '2333', - device_id: 'device_id', - groups: { - 'org name': ['Amplitude Inc sth sth sth'], - }, +const setupEvaluateCohortTestErrorClientCases = ( + client: LocalEvaluationClient, +) => { + test('ExperimentClient.evaluateV2 with user or group cohort, no error thrown, but unknown behavior', async () => { + const variants = await client.evaluateV2({ + user_id: '2333', + device_id: 'device_id', + groups: { + 'org name': ['Amplitude Inc sth sth sth'], + }, + }); + const userVariant = variants['sdk-local-evaluation-user-cohort-ci-test']; + expect(userVariant).toBeDefined(); + expect( + await client.cache.get('sdk-local-evaluation-user-cohort-ci-test'), + ).toBeDefined(); + const groupVariant = variants['sdk-local-evaluation-group-cohort-ci-test']; + expect(groupVariant).toBeDefined(); + expect( + await client.cache.get('sdk-local-evaluation-group-cohort-ci-test'), + ).toBeDefined(); }); - const userVariant = variants['sdk-local-evaluation-user-cohort-ci-test']; - expect(userVariant.key).toEqual('off'); - expect(userVariant.value).toBeUndefined(); - expect( - await client.cache.get('sdk-local-evaluation-user-cohort-ci-test'), - ).toBeDefined(); - const groupVariant = variants['sdk-local-evaluation-group-cohort-ci-test']; - expect(groupVariant.key).toEqual('off'); - expect(groupVariant.value).toBeUndefined(); - expect( - await client.cache.get('sdk-local-evaluation-group-cohort-ci-test'), - ).toBeDefined(); -}); +}; + +describe('ExperimentClient end-to-end tests, normal cases', () => { + describe('Normal cases', () => { + const client = Experiment.initializeLocal(apiKey, { + cohortConfig: { + apiKey: process.env['API_KEY'], + secretKey: process.env['SECRET_KEY'], + }, + }); + + beforeAll(async () => { + await client.start(); + }); -test('ExperimentClient.evaluateV2 with user cohort segment targeted', async () => { - const variants = await client.evaluateV2({ - user_id: '12345', - device_id: 'device_id', + afterAll(async () => { + client.stop(); + }); + + setupEvaluateTestNormalCases(client); + setupEvaluateCohortTestNormalCases(client); }); - const variant = variants['sdk-local-evaluation-user-cohort-ci-test']; - expect(variant.key).toEqual('on'); - expect(variant.value).toEqual('on'); - expect( - await client.cache.get('sdk-local-evaluation-user-cohort-ci-test'), - ).toBeDefined(); -}); -test('ExperimentClient.evaluateV2 with user cohort tester targeted', async () => { - const variants = await client.evaluateV2({ - user_id: '1', - device_id: 'device_id', + describe('No cohort config', () => { + const client = Experiment.initializeLocal(apiKey); + + beforeAll(async () => { + await client.start(); + }); + + afterAll(async () => { + client.stop(); + }); + + setupEvaluateTestNormalCases(client); + setupEvaluateCohortTestErrorClientCases(client); }); - const variant = variants['sdk-local-evaluation-user-cohort-ci-test']; - expect(variant.key).toEqual('on'); - expect(variant.value).toEqual('on'); - expect( - await client.cache.get('sdk-local-evaluation-user-cohort-ci-test'), - ).toBeDefined(); -}); -test('ExperimentClient.evaluateV2 with group cohort segment targeted', async () => { - const variants = await client.evaluateV2({ - user_id: '12345', - device_id: 'device_id', - groups: { - 'org id': ['1'], - }, + describe('Bad cohort config', () => { + const client = Experiment.initializeLocal(apiKey, { + cohortConfig: { + apiKey: 'bad_api_key', + secretKey: 'bad_secret_key', + }, + }); + + beforeAll(async () => { + await client.start(); + }); + + afterAll(async () => { + client.stop(); + }); + + setupEvaluateTestNormalCases(client); + setupEvaluateCohortTestErrorClientCases(client); }); - const variant = variants['sdk-local-evaluation-group-cohort-ci-test']; - expect(variant.key).toEqual('on'); - expect(variant.value).toEqual('on'); - expect( - await client.cache.get('sdk-local-evaluation-group-cohort-ci-test'), - ).toBeDefined(); }); -test('ExperimentClient.evaluateV2 with group cohort tester targeted', async () => { - const variants = await client.evaluateV2({ - user_id: '2333', - device_id: 'device_id', - groups: { - 'org name': ['Amplitude Website (Portfolio)'], - }, +describe('ExperimentClient integration tests', () => { + let flagFetchRequestCount; + let cohortFetchRequestCount; + let mockHttpClient; + + beforeEach(() => { + jest.clearAllMocks(); + + flagFetchRequestCount = 0; + cohortFetchRequestCount = 0; + mockHttpClient = new MockHttpClient(async (params) => { + const url = new URL(params.requestUrl); + let body; + if (url.pathname.startsWith('/sdk/v2/flags')) { + // Flags. + flagFetchRequestCount++; + if (flagFetchRequestCount == 1) { + body = JSON.stringify(Object.values(FLAGS)); + } else { + body = JSON.stringify(Object.values(NEW_FLAGS)); + } + } else if (url.pathname.startsWith('/sdk/v1/cohort')) { + // Cohorts. + const cohortId = url.pathname.substring(15); + if (!(cohortId in COHORTS)) { + return { status: 404, body: 'Not found' }; + } + if (url.searchParams.get('maxCohortSize') < COHORTS[cohortId].size) { + return { status: 413, body: 'Max size exceeded' }; + } + if ( + url.searchParams.get('lastModified') == COHORTS[cohortId].lastModified + ) { + return { status: 204, body: '' }; + } + const cohort = { ...COHORTS[cohortId] }; + cohort.memberIds = [...cohort.memberIds]; + body = JSON.stringify(cohort); + } + return { status: 200, body: body }; + }); }); - const variant = variants['sdk-local-evaluation-group-cohort-ci-test']; - expect(variant.key).toEqual('on'); - expect(variant.value).toEqual('on'); - expect( - await client.cache.get('sdk-local-evaluation-group-cohort-ci-test'), - ).toBeDefined(); -}); -// Unit tests -class TestLocalEvaluationClient extends LocalEvaluationClient { - public enrichUserWithCohorts( - user: ExperimentUser, - flags: Record, - ) { - super.enrichUserWithCohorts(user, flags); - } -} + test('ExperimentClient cohort targeting success', async () => { + const client = new LocalEvaluationClient( + 'apikey', + { + cohortConfig: { + apiKey: 'apiKey', + secretKey: 'secretKey', + maxCohortSize: 10, + }, + }, + null, + mockHttpClient, + ); + await client.start(); -test('ExperimentClient.enrichUserWithCohorts', async () => { - const client = new TestLocalEvaluationClient( - apiKey, - LocalEvaluationDefaults, - new InMemoryFlagConfigCache(), - ); - client.cohortStorage.put({ - cohortId: 'cohort1', - groupType: USER_GROUP_TYPE, - groupTypeId: 0, - lastComputed: 0, - lastModified: 0, - size: 1, - memberIds: new Set(['userId']), + let result; + result = client.evaluateV2({ user_id: 'membera1', device_id: '1' }); + expect(result['flag1'].key).toBe('on'); + expect(result['flag2'].key).toBe('on'); + expect(result['flag3'].key).toBe('var1'); + expect(result['flag4'].key).toBe('var1'); + + result = client.evaluateV2({ user_id: 'membera2', device_id: '1' }); + expect(result['flag1'].key).toBe('off'); + expect(result['flag2'].key).toBe('on'); + expect(result['flag3'].key).toBe('var2'); + expect(result['flag4'].key).toBe('var1'); + + result = client.evaluateV2({ user_id: 'membera3', device_id: '1' }); + expect(result['flag1'].key).toBe('off'); + expect(result['flag2'].key).toBe('on'); + expect(result['flag3'].key).toBe('var1'); + expect(result['flag4'].key).toBe('var1'); + + result = client.evaluateV2({ + user_id: '1', + device_id: '1', + groups: { 'org name': ['org name 1'] }, + }); + expect(result['flag1'].key).toBe('off'); + expect(result['flag2'].key).toBe('off'); + expect(result['flag3'].key).toBe('off'); + expect(result['flag4'].key).toBe('var2'); + + result = client.evaluateV2({ + user_id: '1', + device_id: '1', + }); + expect(result['flag1'].key).toBe('off'); + expect(result['flag2'].key).toBe('off'); + expect(result['flag3'].key).toBe('off'); + expect(result['flag4'].key).toBe('off'); + + client.stop(); }); - client.cohortStorage.put({ - cohortId: 'groupcohort1', - groupType: 'groupname', - groupTypeId: 1, - lastComputed: 0, - lastModified: 0, - size: 1, - memberIds: new Set(['amplitude', 'experiment']), + + test('ExperimentClient cohort maxCohortSize download fail', async () => { + const client = new LocalEvaluationClient( + 'apikey', + { + cohortConfig: { + apiKey: 'apiKey', + secretKey: 'secretKey', + maxCohortSize: 0, + }, + }, + null, + mockHttpClient, + ); + await client.start(); + + const result = client.evaluateV2({ user_id: 'membera1', device_id: '1' }); + // Currently cohort failed to download simply means there's no members in cohort as it's not going to be added to evaluation context. + // This behavior will change. + expect(result['flag1'].key).toBe('off'); + expect(result['flag2'].key).toBe('off'); + expect(result['flag3'].key).toBe('off'); + expect(result['flag4'].key).toBe('off'); + + client.stop(); }); - const user = { - user_id: 'userId', - groups: { - groupname: ['amplitude'], - }, - }; - client.enrichUserWithCohorts(user, { - flag1: { - key: 'flag1', - variants: {}, - segments: [ - { - conditions: [ - [ - { - op: 'set contains any', - selector: ['context', 'user', 'cohort_ids'], - values: ['cohort1'], - }, - ], - ], + + test('ExperimentClient cohort download initial failures, but poller would success', async () => { + jest.setTimeout(70000); + const client = new LocalEvaluationClient( + 'apikey', + { + flagConfigPollingIntervalMillis: 40000, + cohortConfig: { + apiKey: 'apiKey', + secretKey: 'secretKey', + maxCohortSize: 10, }, - ], - }, - flag2: { - key: 'flag2', - variants: {}, - segments: [ - { - conditions: [ - [ - { - op: 'set contains any', - selector: ['context', 'groups', 'groupname', 'cohort_ids'], - values: ['groupcohort1', 'groupcohortnotinstorage'], - }, + }, + null, + new MockHttpClient(async (params) => { + const url = new URL(params.requestUrl); + let body; + if (url.pathname.startsWith('/sdk/v2/flags')) { + // Flags. + flagFetchRequestCount++; + if (flagFetchRequestCount == 1) { + body = JSON.stringify(Object.values(FLAGS)); + } else { + body = JSON.stringify(Object.values(NEW_FLAGS)); + } + } else if (url.pathname.startsWith('/sdk/v1/cohort')) { + // Cohorts. + cohortFetchRequestCount++; + if (cohortFetchRequestCount <= 3 * 11) { + // 3 retries per cohort, 11 requests before poller poll. + // 5 initial requests, 6 requests after flag update. + throw Error('Timeout'); + } + const cohortId = url.pathname.substring(15); + if (!(cohortId in COHORTS)) { + return { status: 404, body: 'Not found' }; + } + if (url.searchParams.get('maxCohortSize') < COHORTS[cohortId].size) { + return { status: 413, body: 'Max size exceeded' }; + } + if ( + url.searchParams.get('lastModified') == + COHORTS[cohortId].lastModified + ) { + return { status: 204, body: '' }; + } + const cohort = { ...COHORTS[cohortId] }; + cohort.memberIds = [...cohort.memberIds]; + body = JSON.stringify(cohort); + } + return { status: 200, body: body }; + }), + ); + await client.start(); + + let result = client.evaluateV2({ user_id: 'membera1', device_id: '1' }); + // Currently cohort failed to download simply means there's no members in cohort as it's not going to be added to evaluation context. + // This behavior will change. + expect(result['flag1'].key).toBe('off'); + expect(result['flag2'].key).toBe('off'); + expect(result['flag3'].key).toBe('off'); + expect(result['flag4'].key).toBe('off'); + expect(result['flag5']).toBeUndefined(); + + await sleep(62000); // Poller polls after 60s. + // Within this time, + // Flag poller (flagConfigPollingIntervalMillis = 40000) will poll a new version, NEW_FLAGS which contains flag5. + // Cohort poller (pollingIntervalMillis = 60000) will poll all cohorts in the flags, which will all success. + + result = client.evaluateV2({ user_id: 'membera1', device_id: '1' }); + // Currently cohort failed to download simply means there's no members in cohort as it's not going to be added to evaluation context. + // This behavior will change. + expect(result['flag1'].key).toBe('on'); + expect(result['flag2'].key).toBe('on'); + expect(result['flag3'].key).toBe('var1'); + expect(result['flag4'].key).toBe('var1'); + expect(result['flag5'].key).toBe('off'); + + client.stop(); + }); +}); + +describe('ExperimentClient unit tests', () => { + // Unit tests + class TestLocalEvaluationClient extends LocalEvaluationClient { + public enrichUserWithCohorts( + user: ExperimentUser, + flags: Record, + ) { + super.enrichUserWithCohorts(user, flags); + } + } + + test('ExperimentClient.enrichUserWithCohorts', async () => { + const client = new TestLocalEvaluationClient( + apiKey, + LocalEvaluationDefaults, + new InMemoryFlagConfigCache(), + ); + client.cohortStorage.put({ + cohortId: 'cohort1', + groupType: USER_GROUP_TYPE, + groupTypeId: 0, + lastComputed: 0, + lastModified: 0, + size: 1, + memberIds: new Set(['userId']), + }); + client.cohortStorage.put({ + cohortId: 'groupcohort1', + groupType: 'groupname', + groupTypeId: 1, + lastComputed: 0, + lastModified: 0, + size: 1, + memberIds: new Set(['amplitude', 'experiment']), + }); + const user = { + user_id: 'userId', + groups: { + groupname: ['amplitude'], + }, + }; + client.enrichUserWithCohorts(user, { + flag1: { + key: 'flag1', + variants: {}, + segments: [ + { + conditions: [ + [ + { + op: 'set contains any', + selector: ['context', 'user', 'cohort_ids'], + values: ['cohort1'], + }, + ], + ], + }, + ], + }, + flag2: { + key: 'flag2', + variants: {}, + segments: [ + { + conditions: [ + [ + { + op: 'set contains any', + selector: ['context', 'groups', 'groupname', 'cohort_ids'], + values: ['groupcohort1', 'groupcohortnotinstorage'], + }, + ], ], - ], + }, + ], + }, + }); + expect(user).toStrictEqual({ + user_id: 'userId', + cohort_ids: ['cohort1'], + groups: { + groupname: ['amplitude'], + }, + group_cohort_ids: { + groupname: { + amplitude: ['groupcohort1'], }, - ], - }, - }); - expect(user).toStrictEqual({ - user_id: 'userId', - cohort_ids: ['cohort1'], - groups: { - groupname: ['amplitude'], - }, - group_cohort_ids: { - groupname: { - amplitude: ['groupcohort1'], }, - }, + }); }); }); diff --git a/packages/node/test/local/cohort/cohortPoller.test.ts b/packages/node/test/local/cohort/cohortPoller.test.ts index ddb50cb..729cd5a 100644 --- a/packages/node/test/local/cohort/cohortPoller.test.ts +++ b/packages/node/test/local/cohort/cohortPoller.test.ts @@ -1,3 +1,4 @@ +import { FlagConfigCache, InMemoryFlagConfigCache } from 'src/index'; import { SdkCohortApi } from 'src/local/cohort/cohort-api'; import { CohortFetcher } from 'src/local/cohort/fetcher'; import { CohortPoller } from 'src/local/cohort/poller'; @@ -5,6 +6,8 @@ import { InMemoryCohortStorage } from 'src/local/cohort/storage'; import { CohortStorage } from 'src/types/cohort'; import { sleep } from 'src/util/time'; +import { getFlagWithCohort } from '../util/mockData'; + const OLD_COHORTS = { c1: { cohortId: 'c1', @@ -66,22 +69,24 @@ const NEW_COHORTS = { }; const POLL_MILLIS = 500; +let flagsCache: FlagConfigCache; let storage: CohortStorage; let fetcher: CohortFetcher; let poller: CohortPoller; -let storageGetAllCohortIdsSpy: jest.SpyInstance; let storageGetCohortSpy: jest.SpyInstance; let storagePutSpy: jest.SpyInstance; beforeEach(() => { + flagsCache = new InMemoryFlagConfigCache(); storage = new InMemoryCohortStorage(); fetcher = new CohortFetcher('', '', null); - poller = new CohortPoller(fetcher, storage, POLL_MILLIS); + poller = new CohortPoller(fetcher, storage, flagsCache, POLL_MILLIS); - storageGetAllCohortIdsSpy = jest.spyOn(storage, 'getAllCohortIds'); - storageGetAllCohortIdsSpy.mockImplementation( - () => new Set(['c1', 'c2']), - ); + const flagsCacheGetAllSpy = jest.spyOn(flagsCache, 'getAll'); + flagsCacheGetAllSpy.mockImplementation(async () => ({ + flag_c1: getFlagWithCohort('c1'), + flag_c2: getFlagWithCohort('c2'), + })); storageGetCohortSpy = jest.spyOn(storage, 'getCohort'); storageGetCohortSpy.mockImplementation( (cohortId: string) => OLD_COHORTS[cohortId], diff --git a/packages/node/test/local/flagConfigPoller.test.ts b/packages/node/test/local/flagConfigPoller.test.ts index 053a647..157991f 100644 --- a/packages/node/test/local/flagConfigPoller.test.ts +++ b/packages/node/test/local/flagConfigPoller.test.ts @@ -8,7 +8,7 @@ import { CohortFetcher } from 'src/local/cohort/fetcher'; import { InMemoryCohortStorage } from 'src/local/cohort/storage'; import { sleep } from 'src/util/time'; -import { FLAGS, NEW_FLAGS } from './util/flags'; +import { FLAGS, NEW_FLAGS } from './util/mockData'; import { MockHttpClient } from './util/mockHttpClient'; afterEach(() => { @@ -64,17 +64,19 @@ test('flagConfig poller success', async () => { ...FLAGS, flagPolled: { key: flagPolled }, }); - expect(cohortStorage.getCohort('hahahaha1').cohortId).toBe('hahahaha1'); - expect(cohortStorage.getCohort('hahahaha2').cohortId).toBe('hahahaha2'); - expect(cohortStorage.getCohort('hahahaha3').cohortId).toBe('hahahaha3'); - expect(cohortStorage.getCohort('hahahaha4').cohortId).toBe('hahahaha4'); - expect(cohortStorage.getCohort('hahaorgname1').cohortId).toBe('hahaorgname1'); + expect(cohortStorage.getCohort('usercohort1').cohortId).toBe('usercohort1'); + expect(cohortStorage.getCohort('usercohort2').cohortId).toBe('usercohort2'); + expect(cohortStorage.getCohort('usercohort3').cohortId).toBe('usercohort3'); + expect(cohortStorage.getCohort('usercohort4').cohortId).toBe('usercohort4'); + expect(cohortStorage.getCohort('orgnamecohort1').cohortId).toBe( + 'orgnamecohort1', + ); expect(cohortStorage.getCohort('newcohortid')).toBeUndefined(); - expect(cohortStorage.getCohort('hahahaha1').lastModified).toBe(1); - expect(cohortStorage.getCohort('hahahaha2').lastModified).toBe(1); - expect(cohortStorage.getCohort('hahahaha3').lastModified).toBe(1); - expect(cohortStorage.getCohort('hahahaha4').lastModified).toBe(1); - expect(cohortStorage.getCohort('hahaorgname1').lastModified).toBe(1); + expect(cohortStorage.getCohort('usercohort1').lastModified).toBe(1); + expect(cohortStorage.getCohort('usercohort2').lastModified).toBe(1); + expect(cohortStorage.getCohort('usercohort3').lastModified).toBe(1); + expect(cohortStorage.getCohort('usercohort4').lastModified).toBe(1); + expect(cohortStorage.getCohort('orgnamecohort1').lastModified).toBe(1); // On update, flag, existing cohort doesn't update. await sleep(2000); @@ -83,22 +85,24 @@ test('flagConfig poller success', async () => { ...NEW_FLAGS, flagPolled: { key: flagPolled }, }); - expect(cohortStorage.getCohort('hahahaha1').cohortId).toBe('hahahaha1'); - expect(cohortStorage.getCohort('hahahaha2').cohortId).toBe('hahahaha2'); - expect(cohortStorage.getCohort('hahahaha3').cohortId).toBe('hahahaha3'); - expect(cohortStorage.getCohort('hahahaha4').cohortId).toBe('hahahaha4'); - expect(cohortStorage.getCohort('hahaorgname1').cohortId).toBe('hahaorgname1'); + expect(cohortStorage.getCohort('usercohort1').cohortId).toBe('usercohort1'); + expect(cohortStorage.getCohort('usercohort2').cohortId).toBe('usercohort2'); + expect(cohortStorage.getCohort('usercohort3').cohortId).toBe('usercohort3'); + expect(cohortStorage.getCohort('usercohort4').cohortId).toBe('usercohort4'); + expect(cohortStorage.getCohort('orgnamecohort1').cohortId).toBe( + 'orgnamecohort1', + ); expect(cohortStorage.getCohort('anewcohortid').cohortId).toBe('anewcohortid'); - expect(cohortStorage.getCohort('hahahaha1').lastModified).toBe(1); - expect(cohortStorage.getCohort('hahahaha2').lastModified).toBe(1); - expect(cohortStorage.getCohort('hahahaha3').lastModified).toBe(1); - expect(cohortStorage.getCohort('hahahaha4').lastModified).toBe(1); - expect(cohortStorage.getCohort('hahaorgname1').lastModified).toBe(1); + expect(cohortStorage.getCohort('usercohort1').lastModified).toBe(1); + expect(cohortStorage.getCohort('usercohort2').lastModified).toBe(1); + expect(cohortStorage.getCohort('usercohort3').lastModified).toBe(1); + expect(cohortStorage.getCohort('usercohort4').lastModified).toBe(1); + expect(cohortStorage.getCohort('orgnamecohort1').lastModified).toBe(1); expect(cohortStorage.getCohort('anewcohortid').lastModified).toBe(2); poller.stop(); }); -test('flagConfig poller initial error', async () => { +test('flagConfig poller initial cohort error, still init', async () => { const poller = new FlagConfigPoller( new FlagConfigFetcher( 'key', @@ -128,13 +132,18 @@ test('flagConfig poller initial error', async () => { try { // Should throw when init failed. await poller.start(); + } catch { fail(); - // eslint-disable-next-line no-empty - } catch {} - expect(await poller.cache.getAll()).toStrictEqual({}); + } + expect(await poller.cache.getAll()).toStrictEqual(FLAGS); + expect(poller.cohortStorage.getAllCohortIds()).toStrictEqual( + new Set(), + ); + + poller.stop(); }); -test('flagConfig poller initial success, polling error and use old flags', async () => { +test('flagConfig poller initial success, polling flag success, cohort failed, and still updates flags', async () => { const poller = new FlagConfigPoller( new FlagConfigFetcher( 'key', @@ -185,7 +194,8 @@ test('flagConfig poller initial success, polling error and use old flags', async // The different flag should not be updated. await sleep(2000); expect(flagPolled).toBeGreaterThanOrEqual(2); - expect(await poller.cache.getAll()).toStrictEqual(FLAGS); + await sleep(250); // Wait for cohort download retry to finish. + expect(await poller.cache.getAll()).toStrictEqual(NEW_FLAGS); poller.stop(); }); diff --git a/packages/node/test/local/flagConfigStreamer.test.ts b/packages/node/test/local/flagConfigStreamer.test.ts index 28c93eb..8f222af 100644 --- a/packages/node/test/local/flagConfigStreamer.test.ts +++ b/packages/node/test/local/flagConfigStreamer.test.ts @@ -7,16 +7,10 @@ import { InMemoryCohortStorage } from 'src/local/cohort/storage'; import { FlagConfigFetcher } from 'src/local/fetcher'; import { FlagConfigStreamer } from 'src/local/streamer'; +import { getFlagStrWithCohort } from './util/mockData'; import { MockHttpClient } from './util/mockHttpClient'; import { getNewClient } from './util/mockStreamEventSource'; -const getFlagWithCohort = ( - cohortId, -) => `[{"key":"flag_${cohortId}","segments":[{ - "conditions":[[{"op":"set contains any","selector":["context","user","cohort_ids"],"values":["${cohortId}"]}]], - "metadata":{"segmentName": "Segment 1"},"variant": "off" - }],"variants": {}}]`; - let updater; afterEach(() => { updater?.stop(); @@ -609,16 +603,28 @@ test('FlagConfigUpdater.connect, flag success, cohort success', async () => { updater.start(); await mockClient.client.doOpen({ type: 'open' }); await mockClient.client.doMsg({ - data: getFlagWithCohort('cohort1'), + data: `[${getFlagStrWithCohort('cohort1')}]`, }); await new Promise((r) => setTimeout(r, 1000)); // Wait for poller to poll. expect(fetchObj.fetchCalls).toBe(0); expect(mockClient.numCreated).toBe(1); expect(await cache.get('flag_cohort1')).toBeDefined(); + + // Return cohort with their own cohortId. + // Now update the flags with a new cohort that will fail to download. + await mockClient.client.doMsg({ + data: `[${getFlagStrWithCohort('cohort2')}]`, + }); + await new Promise((r) => setTimeout(r, 1000)); // Wait for poller to poll. + + expect(fetchObj.fetchCalls).toBe(0); // No poller poll. + expect(mockClient.numCreated).toBe(1); + expect(await cache.get('flag_cohort1')).toBeUndefined(); // Old flag removed. + expect(await cache.get('flag_cohort2')).toBeDefined(); // New flag added. updater.stop(); }); -test('FlagConfigUpdater.connect, flag success, success, flag update success, cohort fail, wont fallback to poller as flag stream is ok', async () => { +test('FlagConfigUpdater.connect, flag cohort and init success, flag update success, cohort fail, wont fallback to poller as flag stream is ok', async () => { jest.setTimeout(20000); jest .spyOn(SdkCohortApi.prototype, 'getCohort') @@ -644,7 +650,7 @@ test('FlagConfigUpdater.connect, flag success, success, flag update success, coh updater.start(); await mockClient.client.doOpen({ type: 'open' }); await mockClient.client.doMsg({ - data: getFlagWithCohort('cohort1'), + data: `[${getFlagStrWithCohort('cohort1')}]`, }); await new Promise((r) => setTimeout(r, 1000)); // Wait for poller to poll. expect(fetchObj.fetchCalls).toBe(0); // No poller poll. @@ -654,18 +660,18 @@ test('FlagConfigUpdater.connect, flag success, success, flag update success, coh // Return cohort with their own cohortId. // Now update the flags with a new cohort that will fail to download. await mockClient.client.doMsg({ - data: getFlagWithCohort('cohort2'), + data: `[${getFlagStrWithCohort('cohort2')}]`, }); await new Promise((r) => setTimeout(r, 1000)); // Wait for poller to poll. - expect(fetchObj.fetchCalls).toBeGreaterThanOrEqual(0); // No poller poll. + expect(fetchObj.fetchCalls).toBe(0); // No poller poll. expect(mockClient.numCreated).toBe(1); expect(await cache.get('flag_cohort1')).toBeUndefined(); // Old flag removed. - expect(await cache.get('flag_cohort2')).toBeUndefined(); // Won't add flag to cache if new cohort fails. + expect(await cache.get('flag_cohort2')).toBeDefined(); // Still add flag to cache if new cohort fails. updater.stop(); }); -test('FlagConfigUpdater.connect, flag success, cohort fail, retry fail, initialization fails, fallback to poller', async () => { +test('FlagConfigUpdater.connect, flag success, cohort fail, retry fail, initialization still success, no fallback to poller', async () => { jest.setTimeout(20000); jest .spyOn(SdkCohortApi.prototype, 'getCohort') @@ -683,55 +689,12 @@ test('FlagConfigUpdater.connect, flag success, cohort fail, retry fail, initiali updater.start(); await mockClient.client.doOpen({ type: 'open' }); await mockClient.client.doMsg({ - data: getFlagWithCohort('cohort1'), - }); - await new Promise((resolve) => setTimeout(resolve, 250)); // Wait for cohort download done retries and fails. - await new Promise((resolve) => setTimeout(resolve, 1050)); // Wait for retry stream. - // Second try - await mockClient.client.doOpen({ type: 'open' }); - await mockClient.client.doMsg({ - data: getFlagWithCohort('cohort1'), + data: `[${getFlagStrWithCohort('cohort1')}]`, }); await new Promise((resolve) => setTimeout(resolve, 250)); // Wait for cohort download done retries and fails. + await new Promise((resolve) => setTimeout(resolve, 1050)); // Wait for poller start. - expect(fetchObj.fetchCalls).toBeGreaterThanOrEqual(1); - expect(mockClient.numCreated).toBe(2); - updater.stop(); -}); - -test('FlagConfigUpdater.connect, flag success, cohort fail, initialization fails, fallback to poller, poller fails, streamer start error', async () => { - jest.setTimeout(10000); - jest - .spyOn(SdkCohortApi.prototype, 'getCohort') - .mockImplementation(async () => { - throw Error(); - }); - const { fetchObj, mockClient, updater } = getTestObjs({ - pollingIntervalMillis: 30000, - streamFlagTryAttempts: 1, - streamFlagTryDelayMillis: 1000, - streamFlagRetryDelayMillis: 100000, - fetcherData: [ - getFlagWithCohort('cohort1'), - getFlagWithCohort('cohort1'), - getFlagWithCohort('cohort1'), - getFlagWithCohort('cohort1'), - getFlagWithCohort('cohort1'), - ], - }); - // Return cohort with their own cohortId. - const startPromise = updater.start(); - await mockClient.client.doOpen({ type: 'open' }); - await mockClient.client.doMsg({ - data: getFlagWithCohort('cohort1'), - }); - // Stream failed, poller should fail as well given the flags and cohort mock. - expect(fetchObj.fetchCalls).toBeGreaterThanOrEqual(1); + expect(fetchObj.fetchCalls).toBe(0); expect(mockClient.numCreated).toBe(1); - // Test should exit cleanly as updater.start() failure should stop the streamer. - try { - await startPromise; - fail(); - // eslint-disable-next-line no-empty - } catch {} + updater.stop(); }); diff --git a/packages/node/test/local/flagConfigUpdater.test.ts b/packages/node/test/local/flagConfigUpdater.test.ts index 0cfde15..31b0757 100644 --- a/packages/node/test/local/flagConfigUpdater.test.ts +++ b/packages/node/test/local/flagConfigUpdater.test.ts @@ -6,19 +6,16 @@ import { InMemoryCohortStorage } from 'src/local/cohort/storage'; import { FlagConfigUpdaterBase } from 'src/local/updater'; import { CohortUtils } from 'src/util/cohort'; -import { FLAGS, NEW_FLAGS } from './util/flags'; +import { FLAGS, NEW_FLAGS } from './util/mockData'; import { MockHttpClient } from './util/mockHttpClient'; class TestFlagConfigUpdaterBase extends FlagConfigUpdaterBase { - public async update(flagConfigs, isInit, onChange) { - await super._update(flagConfigs, isInit, onChange); + public async update(flagConfigs, onChange) { + await super._update(flagConfigs, onChange); } public async downloadNewCohorts(cohortIds) { return await super.downloadNewCohorts(cohortIds); } - public async filterFlagConfigsWithFullCohorts(flagConfigs) { - return await super.filterFlagConfigsWithFullCohorts(flagConfigs); - } public async removeUnusedCohorts(validCohortIds) { return await super.removeUnusedCohorts(validCohortIds); } @@ -70,17 +67,13 @@ afterEach(() => { }); test('FlagConfigUpdaterBase, update success', async () => { - await updater.update(FLAGS, true, () => {}); + await updater.update(FLAGS, () => {}); expect(await updater.cache.getAll()).toStrictEqual(FLAGS); }); -test('FlagConfigUpdaterBase, update no error if not init', async () => { - await updater.update(NEW_FLAGS, false, () => {}); - expect(await updater.cache.getAll()).toStrictEqual(FLAGS); -}); - -test('FlagConfigUpdaterBase, update raise error if not init', async () => { - await expect(updater.update(NEW_FLAGS, true, () => {})).rejects.toThrow(); +test('FlagConfigUpdaterBase, update no error even if cohort download error', async () => { + await updater.update(NEW_FLAGS, () => {}); + expect(await updater.cache.getAll()).toStrictEqual(NEW_FLAGS); }); test('FlagConfigUpdaterBase.downloadNewCohorts', async () => { @@ -93,17 +86,6 @@ test('FlagConfigUpdaterBase.downloadNewCohorts', async () => { expect(failedCohortIds).toStrictEqual(new Set(['anewcohortid'])); }); -test('FlagConfigUpdaterBase.filterFlagConfigsWithFullCohorts', async () => { - CohortUtils.extractCohortIds(FLAGS).forEach((cohortId) => { - updater.cohortStorage.put(createCohort(cohortId)); - }); - - const filteredFlags = await updater.filterFlagConfigsWithFullCohorts( - NEW_FLAGS, - ); - expect(filteredFlags).toStrictEqual(FLAGS); -}); - test('FlagConfigUpdaterBase.removeUnusedCohorts', async () => { CohortUtils.extractCohortIds(NEW_FLAGS).forEach((cohortId) => { updater.cohortStorage.put(createCohort(cohortId)); diff --git a/packages/node/test/local/util/cohortUtils.test.ts b/packages/node/test/local/util/cohortUtils.test.ts index 1c931b2..2a32325 100644 --- a/packages/node/test/local/util/cohortUtils.test.ts +++ b/packages/node/test/local/util/cohortUtils.test.ts @@ -1,162 +1,36 @@ import { CohortUtils } from 'src/util/cohort'; -test('test extract cohortIds from flags', async () => { - // Flag definition is not complete, only those useful for thest is included. - const flags = [ - { - key: 'flag1', - segments: [ - { - conditions: [ - [ - { - op: 'set contains any', - selector: ['context', 'user', 'cohort_ids'], - values: ['hahahaha1'], - }, - ], - ], - }, - { - metadata: { - segmentName: 'All Other Users', - }, - variant: 'off', - }, - ], - variants: {}, - }, - { - key: 'flag2', - segments: [ - { - conditions: [ - [ - { - op: 'set contains any', - selector: ['context', 'user', 'cohort_ids'], - values: ['hahahaha2'], - }, - ], - ], - metadata: { - segmentName: 'Segment 1', - }, - variant: 'off', - }, - { - metadata: { - segmentName: 'All Other Users', - }, - variant: 'off', - }, - ], - variants: {}, - }, - { - key: 'flag3', - metadata: { - deployed: true, - evaluationMode: 'local', - experimentKey: 'exp-1', - flagType: 'experiment', - flagVersion: 6, - }, - segments: [ - { - conditions: [ - [ - { - op: 'set contains any', - selector: ['context', 'user', 'cohort_ids'], - values: ['hahahaha3'], - }, - ], - ], - variant: 'off', - }, - { - conditions: [ - [ - { - op: 'set contains any', - selector: ['context', 'user', 'cocoids'], - values: ['nohaha'], - }, - ], - ], - variant: 'off', - }, - { - metadata: { - segmentName: 'All Other Users', - }, - variant: 'off', - }, - ], - variants: {}, - }, - { - key: 'flag5', - segments: [ - { - conditions: [ - [ - { - op: 'set contains any', - selector: ['context', 'user', 'cohort_ids'], - values: ['hahahaha3', 'hahahaha4'], - }, - ], - ], - }, - { - conditions: [ - [ - { - op: 'set contains any', - selector: ['context', 'groups', 'org name', 'cohort_ids'], - values: ['hahaorgname1'], - }, - ], - ], - metadata: { - segmentName: 'Segment 1', - }, - }, - { - conditions: [ - [ - { - op: 'set contains any', - selector: ['context', 'gg', 'org name', 'cohort_ids'], - values: ['nohahaorgname'], - }, - ], - ], - metadata: { - segmentName: 'Segment 1', - }, - }, - ], - variants: {}, - }, - ].reduce((acc, flag) => { - acc[flag.key] = flag; - return acc; - }, {}); +import { FLAGS } from './mockData'; - expect(CohortUtils.extractCohortIdsByGroup(flags)).toStrictEqual({ - User: new Set(['hahahaha1', 'hahahaha2', 'hahahaha3', 'hahahaha4']), - 'org name': new Set(['hahaorgname1']), +test('test extract cohortIds from flags', async () => { + expect( + CohortUtils.extractCohortIdsByGroupFromFlag(FLAGS['flag1']), + ).toStrictEqual({ + User: new Set(['usercohort1']), + }); + expect( + CohortUtils.extractCohortIdsByGroupFromFlag(FLAGS['flag2']), + ).toStrictEqual({ + User: new Set(['usercohort2']), + }); + expect( + CohortUtils.extractCohortIdsByGroupFromFlag(FLAGS['flag3']), + ).toStrictEqual({ + User: new Set(['usercohort3', 'usercohort4']), + }); + expect( + CohortUtils.extractCohortIdsByGroupFromFlag(FLAGS['flag4']), + ).toStrictEqual({ + User: new Set(['usercohort3', 'usercohort4']), + 'org name': new Set(['orgnamecohort1']), }); - expect(CohortUtils.extractCohortIds(flags)).toStrictEqual( + expect(CohortUtils.extractCohortIds(FLAGS)).toStrictEqual( new Set([ - 'hahahaha1', - 'hahahaha2', - 'hahahaha3', - 'hahahaha4', - 'hahaorgname1', + 'usercohort1', + 'usercohort2', + 'usercohort3', + 'usercohort4', + 'orgnamecohort1', ]), ); }); diff --git a/packages/node/test/local/util/flags.ts b/packages/node/test/local/util/flags.ts deleted file mode 100644 index 683400a..0000000 --- a/packages/node/test/local/util/flags.ts +++ /dev/null @@ -1,164 +0,0 @@ -export const FLAGS = [ - { - key: 'flag1', - segments: [ - { - conditions: [ - [ - { - op: 'set contains any', - selector: ['context', 'user', 'cohort_ids'], - values: ['hahahaha1'], - }, - ], - ], - }, - { - metadata: { - segmentName: 'All Other Users', - }, - variant: 'off', - }, - ], - variants: {}, - }, - { - key: 'flag2', - segments: [ - { - conditions: [ - [ - { - op: 'set contains any', - selector: ['context', 'user', 'cohort_ids'], - values: ['hahahaha2'], - }, - ], - ], - metadata: { - segmentName: 'Segment 1', - }, - variant: 'off', - }, - { - metadata: { - segmentName: 'All Other Users', - }, - variant: 'off', - }, - ], - variants: {}, - }, - { - key: 'flag3', - metadata: { - deployed: true, - evaluationMode: 'local', - experimentKey: 'exp-1', - flagType: 'experiment', - flagVersion: 6, - }, - segments: [ - { - conditions: [ - [ - { - op: 'set contains any', - selector: ['context', 'user', 'cohort_ids'], - values: ['hahahaha3'], - }, - ], - ], - variant: 'off', - }, - { - conditions: [ - [ - { - op: 'set contains any', - selector: ['context', 'user', 'cocoids'], - values: ['nohaha'], - }, - ], - ], - variant: 'off', - }, - { - metadata: { - segmentName: 'All Other Users', - }, - variant: 'off', - }, - ], - variants: {}, - }, - { - key: 'flag5', - segments: [ - { - conditions: [ - [ - { - op: 'set contains any', - selector: ['context', 'user', 'cohort_ids'], - values: ['hahahaha3', 'hahahaha4'], - }, - ], - ], - }, - { - conditions: [ - [ - { - op: 'set contains any', - selector: ['context', 'groups', 'org name', 'cohort_ids'], - values: ['hahaorgname1'], - }, - ], - ], - metadata: { - segmentName: 'Segment 1', - }, - }, - { - conditions: [ - [ - { - op: 'set contains any', - selector: ['context', 'gg', 'org name', 'cohort_ids'], - values: ['nohahaorgname'], - }, - ], - ], - metadata: { - segmentName: 'Segment 1', - }, - }, - ], - variants: {}, - }, -].reduce((acc, flag) => { - acc[flag.key] = flag; - return acc; -}, {}); - -export const NEW_FLAGS = { - ...FLAGS, - flag6: { - key: 'flag6', - segments: [ - { - conditions: [ - [ - { - op: 'set contains any', - selector: ['context', 'user', 'cohort_ids'], - values: ['anewcohortid'], - }, - ], - ], - }, - ], - variants: {}, - }, -}; diff --git a/packages/node/test/local/util/mockData.ts b/packages/node/test/local/util/mockData.ts new file mode 100644 index 0000000..0f76b6f --- /dev/null +++ b/packages/node/test/local/util/mockData.ts @@ -0,0 +1,273 @@ +// Some test flags. +// FLAGS are normal flags with cohortIds. +// NEW_FLAGS adds a flag with cohortId `anewcohortid` on top of FLAGS. + +export const getFlagStrWithCohort = ( + cohortId: string, +) => `{"key":"flag_${cohortId}","segments":[{ + "conditions":[[{"op":"set contains any","selector":["context","user","cohort_ids"],"values":["${cohortId}"]}]], + "metadata":{"segmentName": "Segment 1"},"variant": "off" + }],"variants": {}}`; + +export const getFlagWithCohort = (cohortId: string) => + JSON.parse(getFlagStrWithCohort(cohortId)); + +export const FLAGS = [ + { + key: 'flag1', + segments: [ + { + conditions: [ + [ + { + op: 'set contains any', + selector: ['context', 'user', 'cohort_ids'], + values: ['usercohort1'], + }, + ], + ], + metadata: { + segmentName: 'Segment 1', + }, + variant: 'on', + }, + { + metadata: { + segmentName: 'All Other Users', + }, + variant: 'off', + }, + ], + variants: { + on: { key: 'on', value: 'on' }, + off: { key: 'off', metadata: { default: true } }, + }, + }, + { + key: 'flag2', + segments: [ + { + conditions: [ + [ + { + op: 'set contains any', + selector: ['context', 'user', 'cohort_ids'], + values: ['usercohort2'], + }, + ], + ], + metadata: { + segmentName: 'Segment 1', + }, + variant: 'on', + }, + { + metadata: { + segmentName: 'All Other Users', + }, + variant: 'off', + }, + ], + variants: { + on: { key: 'on', value: 'on' }, + off: { key: 'off', metadata: { default: true } }, + }, + }, + { + key: 'flag3', + metadata: { + deployed: true, + evaluationMode: 'local', + experimentKey: 'exp-1', + flagType: 'experiment', + flagVersion: 6, + }, + segments: [ + { + conditions: [ + [ + { + op: 'set contains any', + selector: ['context', 'user', 'cohort_ids'], + values: ['usercohort3'], + }, + ], + ], + variant: 'var1', + }, + { + conditions: [ + [ + { + op: 'set contains any', + selector: ['context', 'user', 'cohort_ids'], + values: ['usercohort4'], + }, + ], + ], + variant: 'var2', + }, + { + conditions: [ + [ + { + op: 'set contains any', + selector: ['context', 'user', 'cocoids'], + values: ['nohaha'], + }, + ], + ], + variant: 'var2', + }, + { + metadata: { + segmentName: 'All Other Users', + }, + variant: 'off', + }, + ], + variants: { + var1: { key: 'var1', value: 'var1value' }, + var2: { key: 'var2', value: 'var2value' }, + off: { key: 'off', metadata: { default: true } }, + }, + }, + { + key: 'flag4', + segments: [ + { + conditions: [ + [ + { + op: 'set contains any', + selector: ['context', 'user', 'cohort_ids'], + values: ['usercohort3', 'usercohort4'], + }, + ], + ], + variant: 'var1', + }, + { + conditions: [ + [ + { + op: 'set contains any', + selector: ['context', 'groups', 'org name', 'cohort_ids'], + values: ['orgnamecohort1'], + }, + ], + ], + metadata: { + segmentName: 'Segment 2', + }, + variant: 'var2', + }, + { + conditions: [ + [ + { + op: 'set contains any', + selector: ['context', 'gg', 'org name', 'cohort_ids'], + values: ['nohahaorgname'], + }, + ], + ], + metadata: { + segmentName: 'Segment 3', + }, + variant: 'var3', + }, + { + metadata: { + segmentName: 'All Other Users', + }, + variant: 'off', + }, + ], + variants: { + var1: { key: 'var1', value: 'var1value' }, + var2: { key: 'var2', value: 'var2value' }, + var3: { key: 'var3', value: 'var3value' }, + off: { key: 'off', metadata: { default: true } }, + }, + }, +].reduce((acc, flag) => { + acc[flag.key] = flag; + return acc; +}, {}); + +export const NEW_FLAGS = { + ...FLAGS, + flag5: { + key: 'flag5', + segments: [ + { + conditions: [ + [ + { + op: 'set contains any', + selector: ['context', 'user', 'cohort_ids'], + values: ['anewcohortid'], + }, + ], + ], + variant: 'on', + }, + { + variant: 'off', + }, + ], + variants: { + on: { key: 'on', value: 'on' }, + off: { key: 'off', metadata: { default: true } }, + }, + }, +}; + +export const COHORTS = { + usercohort1: { + cohortId: 'usercohort1', + groupType: 'User', + groupTypeId: 0, + lastComputed: 0, + lastModified: 1, + size: 1, + memberIds: new Set(['membera1']), + }, + usercohort2: { + cohortId: 'usercohort2', + groupType: 'User', + groupTypeId: 0, + lastComputed: 0, + lastModified: 2, + size: 3, + memberIds: new Set(['membera1', 'membera2', 'membera3']), + }, + usercohort3: { + cohortId: 'usercohort3', + groupType: 'User', + groupTypeId: 0, + lastComputed: 0, + lastModified: 10, + size: 3, + memberIds: new Set(['membera1', 'membera3']), + }, + usercohort4: { + cohortId: 'usercohort4', + groupType: 'User', + groupTypeId: 0, + lastComputed: 0, + lastModified: 10, + size: 2, + memberIds: new Set(['membera1', 'membera2']), + }, + orgnamecohort1: { + cohortId: 'orgnamecohort1', + groupType: 'org name', + groupTypeId: 100, + lastComputed: 6, + lastModified: 10, + size: 2, + memberIds: new Set(['org name 1', 'org name 2']), + }, +};