diff --git a/packages/experiment-browser/src/experimentClient.ts b/packages/experiment-browser/src/experimentClient.ts index dbd19c3d..eab54be9 100644 --- a/packages/experiment-browser/src/experimentClient.ts +++ b/packages/experiment-browser/src/experimentClient.ts @@ -27,6 +27,7 @@ import { transformVariantFromStorage, } from './storage/cache'; import { LocalStorage } from './storage/local-storage'; +import { SessionStorage } from './storage/session-storage'; import { FetchHttpClient, WrapperClient } from './transport/http'; import { exposureEvent } from './types/analytics'; import { Client, FetchOptions } from './types/client'; @@ -34,6 +35,7 @@ import { Exposure, ExposureTrackingProvider } from './types/exposure'; import { ExperimentPlugin, IntegrationPlugin } from './types/plugin'; import { ExperimentUserProvider } from './types/provider'; import { isFallback, Source, VariantSource } from './types/source'; +import { Storage } from './types/storage'; import { ExperimentUser } from './types/user'; import { Variant, Variants } from './types/variant'; import { @@ -83,6 +85,8 @@ export class ExperimentClient implements Client { private poller: Poller; private isRunning = false; private readonly integrationManager: IntegrationManager; + // Web experiment adds a user to the flags request + private readonly isWebExperiment: boolean; // Deprecated private analyticsProvider: SessionAnalyticsProvider | undefined; @@ -120,6 +124,8 @@ export class ExperimentClient implements Client { : config.flagConfigPollingIntervalMillis ?? Defaults.flagConfigPollingIntervalMillis, }; + const internalInstanceName = this.config?.['internalInstanceNameSuffix']; + this.isWebExperiment = internalInstanceName === 'web'; this.poller = new Poller( () => this.doFlags(), this.config.flagConfigPollingIntervalMillis, @@ -161,13 +167,21 @@ export class ExperimentClient implements Client { httpClient, ); // Storage & Caching - const storage = new LocalStorage(); + let storage: Storage; + const storageInstanceName = internalInstanceName + ? `${this.config.instanceName}-${internalInstanceName}` + : this.config.instanceName; + if (this.isWebExperiment) { + storage = new SessionStorage(); + } else { + storage = new LocalStorage(); + } this.variants = getVariantStorage( this.apiKey, - this.config.instanceName, + storageInstanceName, storage, ); - this.flags = getFlagStorage(this.apiKey, this.config.instanceName, storage); + this.flags = getFlagStorage(this.apiKey, storageInstanceName, storage); try { this.flags.load(); this.variants.load(); @@ -710,18 +724,15 @@ export class ExperimentClient implements Client { private async doFlags(): Promise { try { - // Web experiment adds a user to the flags request - const isWebExperiment = - this.config?.['internalInstanceNameSuffix'] === 'web'; let user: ExperimentUser; - if (isWebExperiment) { + if (this.isWebExperiment) { user = await this.addContextOrWait(this.getUser()); } const flags = await this.flagApi.getFlags({ libraryName: 'experiment-js-client', libraryVersion: PACKAGE_VERSION, timeoutMillis: this.config.fetchTimeoutMillis, - deliveryMethod: isWebExperiment ? 'web' : undefined, + deliveryMethod: this.isWebExperiment ? 'web' : undefined, user: user?.user_id || user?.device_id ? { user_id: user?.user_id, device_id: user?.device_id } diff --git a/packages/experiment-tag/src/experiment.ts b/packages/experiment-tag/src/experiment.ts index 74595ce9..b9e32f3f 100644 --- a/packages/experiment-tag/src/experiment.ts +++ b/packages/experiment-tag/src/experiment.ts @@ -124,12 +124,6 @@ export class DefaultWebExperimentClient implements WebExperimentClient { if (metadata.evaluationMode !== 'local') { this.remoteFlagKeys.push(key); - - // allow local evaluation for remote flags - metadata.evaluationMode = 'local'; - } else { - // Add locally evaluable flags to the local flag set - this.localFlagKeys.push(key); } flag.metadata = metadata; @@ -147,8 +141,14 @@ export class DefaultWebExperimentClient implements WebExperimentClient { fetchTimeoutMillis: 1000, pollOnStart: false, fetchOnStart: false, + automaticExposureTracking: false, ...this.config, }); + // Get all the locally available flag keys from the SDK. + const variants = this.experimentClient.all(); + this.localFlagKeys = Object.keys(variants).filter( + (key) => variants[key]?.metadata?.evaluationMode === 'local', + ); } /** @@ -323,6 +323,7 @@ export class DefaultWebExperimentClient implements WebExperimentClient { (variant.metadata?.['trackExposure'] as boolean) ?? true; // if payload is falsy or empty array, consider it as control variant const payloadIsArray = Array.isArray(variant.payload); + // TODO(bgiori) this will need to change when we introduce control variant mutations const isControlPayload = !variant.payload || (payloadIsArray && variant.payload.length === 0); if (shouldTrackExposure && isControlPayload) { @@ -388,15 +389,11 @@ export class DefaultWebExperimentClient implements WebExperimentClient { * Get all variants for the current web experiment context. */ public getVariants(): Variants { - const allVariants = this.experimentClient.all(); - - const isRelevantKey = (key: string) => - this.localFlagKeys.includes(key) || this.remoteFlagKeys.includes(key); - - return Object.keys(allVariants).reduce>((acc, key) => { - if (isRelevantKey(key)) acc[key] = allVariants[key]; - return acc; - }, {}); + const variants: Variants = {}; + for (const key of [...this.localFlagKeys, ...this.remoteFlagKeys]) { + variants[key] = this.experimentClient.variant(key); + } + return variants; } /** diff --git a/packages/experiment-tag/test/experiment.test.ts b/packages/experiment-tag/test/experiment.test.ts index 0395a10f..984ceeac 100644 --- a/packages/experiment-tag/test/experiment.test.ts +++ b/packages/experiment-tag/test/experiment.test.ts @@ -10,7 +10,11 @@ import { import * as util from 'src/util'; import { stringify } from 'ts-jest'; -import { createMutateFlag, createRedirectFlag } from './util/create-flag'; +import { + createFlag, + createMutateFlag, + createRedirectFlag, +} from './util/create-flag'; import { MockHttpClient } from './util/mock-http-client'; let apiKey = 0; @@ -41,6 +45,10 @@ describe('initializeExperiment', () => { getItem: jest.fn().mockReturnValue(undefined), setItem: jest.fn(), }, + sessionStorage: { + getItem: jest.fn().mockReturnValue(undefined), + setItem: jest.fn(), + }, location: { href: 'http://test.com', replace: jest.fn(), @@ -100,7 +108,7 @@ describe('initializeExperiment', () => { ); const initialFlags = [ // remote flag - createMutateFlag('test-2', 'treatment', [], [], [], 'remote'), + createMutateFlag('test-2', 'treatment', [], [], 'remote'), // local flag createMutateFlag('test-1', 'treatment'), ]; @@ -425,7 +433,7 @@ describe('initializeExperiment', () => { const initialFlags = [ // remote flag - createMutateFlag('test-2', 'treatment', [], [], [], 'remote'), + createMutateFlag('test-2', 'treatment', [], [], 'remote'), ]; const mockHttpClient = new MockHttpClient(JSON.stringify([])); @@ -452,7 +460,7 @@ describe('initializeExperiment', () => { test('remote evaluation - fetch successful, antiflicker applied', () => { const initialFlags = [ // remote flag - createMutateFlag('test-2', 'treatment', [], [], [], 'remote'), + createMutateFlag('test-2', 'treatment', [], [], 'remote'), // local flag createMutateFlag('test-1', 'treatment'), ]; @@ -480,7 +488,7 @@ describe('initializeExperiment', () => { test('remote evaluation - fetch fail, locally evaluate remote and local flags success', () => { const initialFlags = [ // remote flag - createMutateFlag('test-2', 'treatment', [], [], [], 'remote'), + createMutateFlag('test-2', 'treatment', [], [], 'remote'), // local flag createMutateFlag('test-1', 'treatment'), ]; @@ -509,7 +517,7 @@ describe('initializeExperiment', () => { test('remote evaluation - fetch fail, test initialFlags variant actions called', () => { const initialFlags = [ // remote flag - createMutateFlag('test', 'treatment', [], [], [], 'remote'), + createMutateFlag('test', 'treatment', [], [], 'remote'), ]; const mockHttpClient = new MockHttpClient('', 404); @@ -552,7 +560,7 @@ describe('initializeExperiment', () => { mockGetGlobalScope.mockReturnValue(mockGlobal); const initialFlags = [ // remote flag - createMutateFlag('test', 'treatment', [], [], [], 'remote'), + createMutateFlag('test', 'treatment', [], [], 'remote'), ]; const remoteFlags = [createMutateFlag('test', 'treatment')]; const mockHttpClient = new MockHttpClient(JSON.stringify(remoteFlags), 200); @@ -606,6 +614,159 @@ describe('initializeExperiment', () => { 'http://test.com/2', ); }); + + describe('remote evaluation - flag already stored in session storage', () => { + const sessionStorageMock = () => { + let store = {}; + return { + getItem: jest.fn((key) => store[key] || null), + setItem: jest.fn((key, value) => { + store[key] = value; + }), + removeItem: jest.fn((key) => { + delete store[key]; + }), + clear: jest.fn(() => { + store = {}; + }), + }; + }; + beforeEach(() => { + Object.defineProperty(safeGlobal, 'sessionStorage', { + value: sessionStorageMock(), + }); + }); + afterEach(() => { + safeGlobal.sessionStorage.clear(); + }); + test('evaluated, applied, and impression tracked, start updates flag in storage, applied, impression deduped', async () => { + const apiKey = 'api1'; + const storageKey = `amp-exp-$default_instance-web-${apiKey}-flags`; + // Create mock session storage with initial value + const storedFlag = createFlag('test', 'treatment', 'local', false, { + flagVersion: 2, + }); + safeGlobal.sessionStorage.setItem( + storageKey, + JSON.stringify({ test: storedFlag }), + ); + const initialFlags = [ + createMutateFlag('test', 'treatment', [], [], 'remote', false, { + flagVersion: 3, + }), + ]; + const remoteFlags = [ + createMutateFlag('test', 'treatment', [], [], 'local', false, { + flagVersion: 4, + }), + ]; + const client = DefaultWebExperimentClient.getInstance( + apiKey, + JSON.stringify(initialFlags), + { + httpClient: new MockHttpClient(JSON.stringify(remoteFlags), 200), + }, + ); + const integrationManagerTrack = jest.spyOn( + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + client.getExperimentClient().integrationManager, + 'track', + ); + let version = client.getExperimentClient().variant('test') + .metadata?.flagVersion; + expect(version).toEqual(2); + await client.start(); + version = client.getExperimentClient().variant('test') + .metadata?.flagVersion; + expect(version).toEqual(4); + // check exposure tracked once + expect(mockExposure).toHaveBeenCalledTimes(1); + // Check remote flag store in storage + const flags = JSON.parse( + safeGlobal.sessionStorage.getItem(storageKey) as string, + ); + expect(flags['test'].metadata.flagVersion).toEqual(4); + expect(flags['test'].metadata.evaluationMode).toEqual('local'); + expect(integrationManagerTrack).toBeCalledTimes(1); + const call = integrationManagerTrack.mock.calls[0][0] as unknown as { + flag_key: string; + metadata: Record; + }; + expect(call.flag_key).toEqual('test'); + expect(call.metadata.flagVersion).toEqual(2); + }); + test('evaluated, applied, and impression tracked, start updates flag in storage, applied, impression re-tracked', async () => { + const apiKey = 'api2'; + const storageKey = `amp-exp-$default_instance-web-${apiKey}-flags`; + // Create mock session storage with initial value + const storedFlag = createFlag('test', 'treatment', 'local', false, { + flagVersion: 2, + }); + safeGlobal.sessionStorage.setItem( + storageKey, + JSON.stringify({ test: storedFlag }), + ); + Object.defineProperty(safeGlobal, 'sessionStorage', { + value: sessionStorageMock, + }); + const initialFlags = [ + createMutateFlag('test', 'treatment', [], [], 'remote', false, { + flagVersion: 3, + }), + ]; + const remoteFlags = [ + createMutateFlag('test', 'control', [], [], 'local', false, { + flagVersion: 4, + }), + ]; + const client = DefaultWebExperimentClient.getInstance( + apiKey, + JSON.stringify(initialFlags), + { + httpClient: new MockHttpClient(JSON.stringify(remoteFlags), 200), + }, + ); + const integrationManagerTrack = jest.spyOn( + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + client.getExperimentClient().integrationManager, + 'track', + ); + let version = client.getExperimentClient().variant('test') + .metadata?.flagVersion; + expect(version).toEqual(2); + await client.start(); + version = client.getExperimentClient().variant('test') + .metadata?.flagVersion; + expect(version).toEqual(4); + // check exposure tracked once + expect(mockExposure).toHaveBeenCalledTimes(2); + // Check remote flag store in storage + const flags = JSON.parse( + safeGlobal.sessionStorage.getItem(storageKey) as string, + ); + expect(flags['test'].metadata.flagVersion).toEqual(4); + expect(flags['test'].metadata.evaluationMode).toEqual('local'); + expect(integrationManagerTrack).toBeCalledTimes(2); + const call1 = integrationManagerTrack.mock.calls[0][0] as unknown as { + flag_key: string; + variant: string; + metadata: Record; + }; + const call2 = integrationManagerTrack.mock.calls[1][0] as unknown as { + flag_key: string; + variant: string; + metadata: Record; + }; + expect(call1.flag_key).toEqual('test'); + expect(call1.variant).toEqual('treatment'); + expect(call1.metadata.flagVersion).toEqual(2); + expect(call2.flag_key).toEqual('test'); + expect(call2.variant).toEqual('control'); + expect(call2.metadata.flagVersion).toEqual(4); + }); + }); }); test('feature experiment on global Experiment object', () => { diff --git a/packages/experiment-tag/test/util/create-flag.ts b/packages/experiment-tag/test/util/create-flag.ts index 3d0750fe..fde03ec7 100644 --- a/packages/experiment-tag/test/util/create-flag.ts +++ b/packages/experiment-tag/test/util/create-flag.ts @@ -1,3 +1,5 @@ +import { EvaluationFlag } from '@amplitude/experiment-core'; + export const createRedirectFlag = ( flagKey = 'test', variant: 'treatment' | 'control' | 'off', @@ -5,7 +7,7 @@ export const createRedirectFlag = ( controlUrl: string | undefined = undefined, segments: any[] = [], evaluationMode: 'local' | 'remote' = 'local', -) => { +): EvaluationFlag => { const controlPayload = controlUrl ? [ { @@ -61,15 +63,33 @@ export const createRedirectFlag = ( }; }; +export const createFlag = ( + flagKey = 'test', + variant: 'treatment' | 'control' | 'off', + evaluationMode: 'local' | 'remote' = 'local', + blockingEvaluation = true, + metadata: Record = {}, +): EvaluationFlag => { + return createMutateFlag( + flagKey, + variant, + [], + [], + evaluationMode, + blockingEvaluation, + metadata, + ); +}; + export const createMutateFlag = ( flagKey = 'test', variant: 'treatment' | 'control' | 'off', treatmentMutations: any[] = [], - controlMutations: any[] = [], segments: any[] = [], evaluationMode: 'local' | 'remote' = 'local', blockingEvaluation = true, -) => { + metadata: Record = {}, +): EvaluationFlag => { return { key: flagKey, metadata: { @@ -78,6 +98,7 @@ export const createMutateFlag = ( flagType: 'experiment', deliveryMethod: 'web', blockingEvaluation: evaluationMode === 'remote' && blockingEvaluation, + ...metadata, }, segments: [ ...segments, @@ -91,14 +112,7 @@ export const createMutateFlag = ( variants: { control: { key: 'control', - payload: [ - { - action: 'mutate', - data: { - mutations: controlMutations, - }, - }, - ], + payload: [], value: 'control', }, off: {