fix: store remote flags in session storage for reuse within session

packages/experiment-browser/src/experimentClient.ts

@@ -27,13 +27,15 @@ 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';
 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<void> {
     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 }

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<Record<string, any>>((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;
   }
 
   /**

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<string, unknown>;
+      };
+      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<string, unknown>;
+      };
+      const call2 = integrationManagerTrack.mock.calls[1][0] as unknown as {
+        flag_key: string;
+        variant: string;
+        metadata: Record<string, unknown>;
+      };
+      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', () => {