From ad2127ccf2bb0a2bf3b4a5a2bdc5f6ec8abd1330 Mon Sep 17 00:00:00 2001 From: Kim Gustyr Date: Fri, 19 Apr 2024 14:20:31 +0100 Subject: [PATCH] feat: Identity overrides in local evaluation mode (#143) * feat: Identity overrides in local evaluation mode - Support environment-supplied identity overrides - Remove redundant integration config models - Parse feature state and multivariate feature state UUIDs correctly --- .../environments/integrations/models.ts | 4 - flagsmith-engine/environments/models.ts | 7 +- flagsmith-engine/environments/util.ts | 6 ++ flagsmith-engine/features/util.ts | 27 +++---- flagsmith-engine/index.ts | 1 - index.ts | 1 - package-lock.json | 2 +- sdk/index.ts | 32 +++++--- tests/sdk/data/environment.json | 28 ++++++- tests/sdk/flagsmith.test.ts | 73 ++++++++----------- 10 files changed, 103 insertions(+), 78 deletions(-) delete mode 100644 flagsmith-engine/environments/integrations/models.ts diff --git a/flagsmith-engine/environments/integrations/models.ts b/flagsmith-engine/environments/integrations/models.ts deleted file mode 100644 index ecce776..0000000 --- a/flagsmith-engine/environments/integrations/models.ts +++ /dev/null @@ -1,4 +0,0 @@ -export class IntegrationModel { - api_key?: string = undefined; - base_url?: string = undefined; -} diff --git a/flagsmith-engine/environments/models.ts b/flagsmith-engine/environments/models.ts index 9b32716..211b19b 100644 --- a/flagsmith-engine/environments/models.ts +++ b/flagsmith-engine/environments/models.ts @@ -1,6 +1,6 @@ import { FeatureStateModel } from '../features/models'; +import { IdentityModel } from '../identities/models'; import { ProjectModel } from '../projects/models'; -import { IntegrationModel } from './integrations/models'; export class EnvironmentAPIKeyModel { id: number; @@ -37,10 +37,7 @@ export class EnvironmentModel { apiKey: string; project: ProjectModel; featureStates: FeatureStateModel[] = []; - amplitude_config?: IntegrationModel; - segment_config?: IntegrationModel; - mixpanel_config?: IntegrationModel; - heap_config?: IntegrationModel; + identityOverrides: IdentityModel[] = []; constructor(id: number, apiKey: string, project: ProjectModel) { this.id = id; diff --git a/flagsmith-engine/environments/util.ts b/flagsmith-engine/environments/util.ts index 4a800c9..1086a18 100644 --- a/flagsmith-engine/environments/util.ts +++ b/flagsmith-engine/environments/util.ts @@ -1,4 +1,5 @@ import { buildFeatureStateModel } from '../features/util'; +import { buildIdentityModel } from '../identities/util'; import { buildProjectModel } from '../projects/util'; import { EnvironmentAPIKeyModel, EnvironmentModel } from './models'; @@ -13,6 +14,11 @@ export function buildEnvironmentModel(environmentJSON: any) { project ); environmentModel.featureStates = featureStates; + if (!!environmentJSON.identity_overrides) { + environmentModel.identityOverrides = environmentJSON.identity_overrides.map((identityData: any) => + buildIdentityModel(identityData) + ); + } return environmentModel; } diff --git a/flagsmith-engine/features/util.ts b/flagsmith-engine/features/util.ts index 0c09b62..036e115 100644 --- a/flagsmith-engine/features/util.ts +++ b/flagsmith-engine/features/util.ts @@ -16,25 +16,26 @@ export function buildFeatureStateModel(featuresStateModelJSON: any): FeatureStat featuresStateModelJSON.enabled, featuresStateModelJSON.django_id, featuresStateModelJSON.feature_state_value, - featuresStateModelJSON.uuid + featuresStateModelJSON.featurestate_uuid ); - featureStateModel.featureSegment = featuresStateModelJSON.feature_segment ? - buildFeatureSegment(featuresStateModelJSON.feature_segment) : + featureStateModel.featureSegment = featuresStateModelJSON.feature_segment ? + buildFeatureSegment(featuresStateModelJSON.feature_segment) : undefined; const multivariateFeatureStateValues = featuresStateModelJSON.multivariate_feature_state_values ? featuresStateModelJSON.multivariate_feature_state_values.map((fsv: any) => { - const featureOption = new MultivariateFeatureOptionModel( - fsv.multivariate_feature_option.value, - fsv.multivariate_feature_option.id - ); - return new MultivariateFeatureStateValueModel( - featureOption, - fsv.percentage_allocation, - fsv.id - ); - }) + const featureOption = new MultivariateFeatureOptionModel( + fsv.multivariate_feature_option.value, + fsv.multivariate_feature_option.id + ); + return new MultivariateFeatureStateValueModel( + featureOption, + fsv.percentage_allocation, + fsv.id, + fsv.mv_fs_value_uuid + ); + }) : []; featureStateModel.multivariateFeatureStateValues = multivariateFeatureStateValues; diff --git a/flagsmith-engine/index.ts b/flagsmith-engine/index.ts index 0510b6d..e577fad 100644 --- a/flagsmith-engine/index.ts +++ b/flagsmith-engine/index.ts @@ -7,7 +7,6 @@ import { SegmentModel } from './segments/models'; import { FeatureStateNotFound } from './utils/errors'; export { EnvironmentModel } from './environments/models'; -export { IntegrationModel } from './environments/integrations/models'; export { FeatureStateModel } from './features/models'; export { IdentityModel } from './identities/models'; export { TraitModel } from './identities/traits/models'; diff --git a/index.ts b/index.ts index 46f799a..15050d1 100644 --- a/index.ts +++ b/index.ts @@ -17,7 +17,6 @@ export { export { EnvironmentModel, - IntegrationModel, FeatureStateModel, IdentityModel, TraitModel, diff --git a/package-lock.json b/package-lock.json index 074e6fe..f9f03a5 100644 --- a/package-lock.json +++ b/package-lock.json @@ -6,7 +6,7 @@ "packages": { "": { "name": "flagsmith-nodejs", - "version": "3.1.1", + "version": "3.3.0", "license": "MIT", "dependencies": { "big-integer": "^1.6.51", diff --git a/sdk/index.ts b/sdk/index.ts index 6846684..bd45f4d 100644 --- a/sdk/index.ts +++ b/sdk/index.ts @@ -49,6 +49,8 @@ export class Flagsmith { offlineMode: boolean = false; offlineHandler?: BaseOfflineHandler = undefined; + identitiesWithOverridesByIdentifier?: Map; + private cache?: FlagsmithCache; private onEnvironmentChange?: (error: Error | null, result: EnvironmentModel) => void; private analyticsProcessor?: AnalyticsProcessor; @@ -143,13 +145,13 @@ export class Flagsmith { if (!this.environmentKey) { throw new Error('ValueError: environmentKey is required.'); } - + const apiUrl = data.apiUrl || DEFAULT_API_URL; this.apiUrl = apiUrl.endsWith('/') ? apiUrl : `${apiUrl}/`; this.environmentFlagsUrl = `${this.apiUrl}flags/`; this.identitiesUrl = `${this.apiUrl}identities/`; this.environmentUrl = `${this.apiUrl}environment-document/`; - + if (this.enableLocalEvaluation) { if (!this.environmentKey.startsWith('ser.')) { console.error( @@ -166,11 +168,11 @@ export class Flagsmith { this.analyticsProcessor = data.enableAnalytics ? new AnalyticsProcessor({ - environmentKey: this.environmentKey, - baseApiUrl: this.apiUrl, - requestTimeoutMs: this.requestTimeoutMs, - logger: this.logger - }) + environmentKey: this.environmentKey, + baseApiUrl: this.apiUrl, + requestTimeoutMs: this.requestTimeoutMs, + logger: this.logger + }) : undefined; } } @@ -256,7 +258,7 @@ export class Flagsmith { if (this.enableLocalEvaluation) { return new Promise((resolve, reject) => { return this.environmentPromise!.then(() => { - const identityModel = this.buildIdentityModel( + const identityModel = this.getIdentityModel( identifier, Object.keys(traits || {}).map(key => ({ key, @@ -289,6 +291,11 @@ export class Flagsmith { } else { this.environment = await request; } + if (this.environment.identityOverrides?.length) { + this.identitiesWithOverridesByIdentifier = new Map( + this.environment.identityOverrides.map(identity => [identity.identifier, identity] + )); + } if (this.onEnvironmentChange) { this.onEnvironmentChange(null, this.environment); } @@ -370,7 +377,7 @@ export class Flagsmith { identifier: string, traits: { [key: string]: any } ): Promise { - const identityModel = this.buildIdentityModel( + const identityModel = this.getIdentityModel( identifier, Object.keys(traits).map(key => ({ key, @@ -458,8 +465,13 @@ export class Flagsmith { } } - private buildIdentityModel(identifier: string, traits: { key: string; value: any }[]) { + private getIdentityModel(identifier: string, traits: { key: string; value: any }[]) { const traitModels = traits.map(trait => new TraitModel(trait.key, trait.value)); + let identityWithOverrides = this.identitiesWithOverridesByIdentifier?.get(identifier); + if (identityWithOverrides) { + identityWithOverrides.updateTraits(traitModels); + return identityWithOverrides; + } return new IdentityModel('0', traitModels, [], this.environment.apiKey, identifier); } } diff --git a/tests/sdk/data/environment.json b/tests/sdk/data/environment.json index b1d486a..2dd3687 100644 --- a/tests/sdk/data/environment.json +++ b/tests/sdk/data/environment.json @@ -17,6 +17,7 @@ "feature_states": [ { "feature_state_value": "segment_override", + "featurestate_uuid": "dd77a1ab-08cf-4743-8a3b-19e730444a14", "multivariate_feature_state_values": [], "django_id": 81027, "feature": { @@ -88,5 +89,30 @@ "featurestate_uuid": "96fc3503-09d7-48f1-a83b-2dc903d5c08a", "enabled": false } + ], + "identity_overrides": [ + { + "identifier": "overridden-id", + "identity_uuid": "0f21cde8-63c5-4e50-baca-87897fa6cd01", + "created_date": "2019-08-27T14:53:45.698555Z", + "updated_at": "2023-07-14 16:12:00.000000", + "environment_api_key": "B62qaMZNwfiqT76p38ggrQ", + "identity_features": [ + { + "id": 1, + "feature": { + "id": 1, + "name": "some_feature", + "type": "STANDARD" + }, + "featurestate_uuid": "1bddb9a5-7e59-42c6-9be9-625fa369749f", + "feature_state_value": "some-overridden-value", + "enabled": false, + "environment": 1, + "identity": null, + "feature_segment": null + } + ] + } ] -} +} \ No newline at end of file diff --git a/tests/sdk/flagsmith.test.ts b/tests/sdk/flagsmith.test.ts index a87391e..292c680 100644 --- a/tests/sdk/flagsmith.test.ts +++ b/tests/sdk/flagsmith.test.ts @@ -1,10 +1,9 @@ import Flagsmith from '../../sdk'; import { EnvironmentDataPollingManager } from '../../sdk/polling_manager'; -import fetch, {RequestInit} from 'node-fetch'; +import fetch, { RequestInit } from 'node-fetch'; import { environmentJSON, environmentModel, flagsJSON, flagsmith, identitiesJSON } from './utils'; import { DefaultFlag, Flags } from '../../sdk/models'; -import {delay, retryFetch} from '../../sdk/utils'; -import * as utils from '../../sdk/utils'; +import { delay } from '../../sdk/utils'; import { EnvironmentModel } from '../../flagsmith-engine/environments/models'; import https from 'https' import { BaseOfflineHandler } from '../../sdk/offline_handlers'; @@ -48,9 +47,6 @@ test('test_update_environment_sets_environment', async () => { const model = environmentModel(JSON.parse(environmentJSON())); - wipeFeatureStateUUIDs(flg.environment) - wipeFeatureStateUUIDs(model) - expect(flg.environment).toStrictEqual(model); }); @@ -58,8 +54,8 @@ test('test_set_agent_options', async () => { const agent = new https.Agent({}) // @ts-ignore - fetch.mockImplementation((url:string, options:RequestInit)=>{ - if(options.agent!==agent) { + fetch.mockImplementation((url: string, options: RequestInit) => { + if (options.agent !== agent) { throw new Error("Agent has not been set on retry fetch") } return Promise.resolve(new Response(environmentJSON())) @@ -276,7 +272,7 @@ test('getIdentitySegments throws error if identifier is empty string', () => { }) -test('offline_mode', async() => { +test('offline_mode', async () => { // Given const environment: EnvironmentModel = environmentModel(JSON.parse(environmentJSON('offline-environment.json'))); @@ -311,19 +307,19 @@ test('test_flagsmith_uses_offline_handler_if_set_and_no_api_response', async () const environment: EnvironmentModel = environmentModel(JSON.parse(environmentJSON('offline-environment.json'))); const api_url = 'http://some.flagsmith.com/api/v1/'; const mock_offline_handler = new BaseOfflineHandler() as jest.Mocked; - + jest.spyOn(mock_offline_handler, 'getEnvironment').mockReturnValue(environment); const flagsmith = new Flagsmith({ - environmentKey: 'some-key', - apiUrl: api_url, - offlineHandler: mock_offline_handler, + environmentKey: 'some-key', + apiUrl: api_url, + offlineHandler: mock_offline_handler, }); jest.spyOn(flagsmith, 'getEnvironmentFlags'); jest.spyOn(flagsmith, 'getIdentityFlags'); - + flagsmith.environmentFlagsUrl = 'http://some.flagsmith.com/api/v1/environment-flags'; flagsmith.identitiesUrl = 'http://some.flagsmith.com/api/v1/identities'; @@ -337,17 +333,17 @@ test('test_flagsmith_uses_offline_handler_if_set_and_no_api_response', async () fetch.mockReturnValue(Promise.resolve(errorResponse)); // When - const environmentFlags:Flags = await flagsmith.getEnvironmentFlags(); - const identityFlags:Flags = await flagsmith.getIdentityFlags('identity', {}); + const environmentFlags: Flags = await flagsmith.getEnvironmentFlags(); + const identityFlags: Flags = await flagsmith.getIdentityFlags('identity', {}); // Then expect(mock_offline_handler.getEnvironment).toHaveBeenCalledTimes(1); expect(flagsmith.getEnvironmentFlags).toHaveBeenCalled(); expect(flagsmith.getIdentityFlags).toHaveBeenCalled(); - + expect(environmentFlags.isFeatureEnabled('some_feature')).toBe(true); expect(environmentFlags.getFeatureValue('some_feature')).toBe('offline-value'); - + expect(identityFlags.isFeatureEnabled('some_feature')).toBe(true); expect(identityFlags.getFeatureValue('some_feature')).toBe('offline-value'); }); @@ -355,18 +351,18 @@ test('test_flagsmith_uses_offline_handler_if_set_and_no_api_response', async () test('cannot use offline mode without offline handler', () => { // When and Then expect(() => new Flagsmith({ offlineMode: true, offlineHandler: undefined })).toThrowError( - 'ValueError: offlineHandler must be provided to use offline mode.' + 'ValueError: offlineHandler must be provided to use offline mode.' ); }); - + test('cannot use both default handler and offline handler', () => { // When and Then expect(() => new Flagsmith({ - offlineHandler: new BaseOfflineHandler(), - defaultFlagHandler: (flagName) => new DefaultFlag('foo', true) + offlineHandler: new BaseOfflineHandler(), + defaultFlagHandler: (flagName) => new DefaultFlag('foo', true) })).toThrowError('ValueError: Cannot use both defaultFlagHandler and offlineHandler.'); }); - + test('cannot create Flagsmith client in remote evaluation without API key', () => { // When and Then // @ts-ignore @@ -374,27 +370,20 @@ test('cannot create Flagsmith client in remote evaluation without API key', () = }); -async function wipeFeatureStateUUIDs (environmentModel: EnvironmentModel) { - // TODO: this has been pulled out of tests above as a helper function. - // I'm not entirely sure why it's necessary, however, we should look to remove. - environmentModel.featureStates.forEach(fs => { - // @ts-ignore - fs.featurestateUUID = undefined; - fs.multivariateFeatureStateValues.forEach(mvfsv => { - // @ts-ignore - mvfsv.mvFsValueUuid = undefined; - }) - }); - environmentModel.project.segments.forEach(s => { - s.featureStates.forEach(fs => { - // @ts-ignore - fs.featurestateUUID = undefined; - }) - }) -} - function sleep(ms: number) { return new Promise((resolve) => { setTimeout(resolve, ms); }); } +test('test_localEvaluation_true__identity_overrides_evaluated', async () => { + // @ts-ignore + fetch.mockReturnValue(Promise.resolve(new Response(environmentJSON()))); + + const flg = new Flagsmith({ + environmentKey: 'ser.key', + enableLocalEvaluation: true, + }); + + const flags = await flg.getIdentityFlags("overridden-id"); + expect(flags.getFeatureValue("some_feature")).toEqual("some-overridden-value"); +});