Additional params to experiment hooks methods. Enable hooks in getAssignment (#8)

* additional params to experiment hooks methods. enable hooks in getAssignment

* only check assignment override after enabled check

* ensure logging callback is invoked when overriding via hook

* create internal method to avoid multiple calls to log assignment/post assignment callback

* add undefined check

Author: petzel

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@eppo/js-client-sdk-common",
-  "version": "1.2.2",
+  "version": "1.3.0",
   "description": "Eppo SDK for client-side JavaScript applications (base for both web and react native)",
   "main": "dist/index.js",
   "files": [

src/assignment-hooks.ts

@@ -6,18 +6,21 @@ export interface IAssignmentHooks {
   /**
    * Invoked before a subject is assigned to an experiment variation.
    *
+   * @param experimentKey key of the experiment being assigned
    * @param subject id of subject being assigned
    * @returns variation to override for the given subject. If null is returned,
    * then the subject will be assigned with the default assignment logic.
    * @public
    */
-  onPreAssignment(subject: string): Promise<string | null>;
+  onPreAssignment(experimentKey: string, subject: string): string | null;
 
   /**
    * Invoked after a subject is assigned. Useful for any post assignment logic needed which is specific
    * to an experiment/flag. Do not use this for logging assignments - use IAssignmentLogger instead.
+   * @param experimentKey key of the experiment being assigned
+   * @param subject id of subject being assigned
    * @param variation the assigned variation
    * @public
    */
-  onPostAssignment(variation: string): Promise<void>;
+  onPostAssignment(experimentKey: string, subject: string, variation: string): void;
 }

src/client/eppo-client.spec.ts

@@ -300,7 +300,7 @@ describe('EppoClient E2E test', () => {
     });
   }
 
-  describe('getAssignmentWithHooks', () => {
+  describe('getAssignment with hooks', () => {
     let client: EppoClient;
 
     beforeAll(() => {
@@ -309,41 +309,52 @@ describe('EppoClient E2E test', () => {
     });
 
     describe('onPreAssignment', () => {
-      it('called with subject ID', () => {
+      it('called with experiment key and subject id', () => {
         const mockHooks = td.object<IAssignmentHooks>();
-        client.getAssignmentWithHooks('subject-identifer', experimentName, mockHooks);
+        client.getAssignment('subject-identifer', experimentName, {}, mockHooks);
         expect(td.explain(mockHooks.onPreAssignment).callCount).toEqual(1);
-        expect(td.explain(mockHooks.onPreAssignment).calls[0].args[0]).toEqual('subject-identifer');
+        expect(td.explain(mockHooks.onPreAssignment).calls[0].args[0]).toEqual(experimentName);
+        expect(td.explain(mockHooks.onPreAssignment).calls[0].args[1]).toEqual('subject-identifer');
       });
 
       it('overrides returned assignment', async () => {
-        const variation = await client.getAssignmentWithHooks('subject-identifer', experimentName, {
-          // eslint-disable-next-line @typescript-eslint/no-unused-vars
-          onPreAssignment(subject: string): Promise<string> {
-            return Promise.resolve('my-overridden-variation');
-          },
+        const variation = await client.getAssignment(
+          'subject-identifer',
+          experimentName,
+          {},
+          {
+            // eslint-disable-next-line @typescript-eslint/no-unused-vars
+            onPreAssignment(experimentKey: string, subject: string): string {
+              return 'my-overridden-variation';
+            },
 
-          // eslint-disable-next-line @typescript-eslint/no-unused-vars
-          onPostAssignment(variation: string): Promise<void> {
-            return Promise.resolve();
+            // eslint-disable-next-line @typescript-eslint/no-unused-vars
+            onPostAssignment(experimentKey: string, subject: string, variation: string): void {
+              // no-op
+            },
           },
-        });
+        );
 
         expect(variation).toEqual('my-overridden-variation');
       });
 
       it('uses regular assignment logic if onPreAssignment returns null', async () => {
-        const variation = await client.getAssignmentWithHooks('subject-identifer', experimentName, {
-          // eslint-disable-next-line @typescript-eslint/no-unused-vars
-          onPreAssignment(subject: string): Promise<string | null> {
-            return Promise.resolve(null);
-          },
+        const variation = await client.getAssignment(
+          'subject-identifer',
+          experimentName,
+          {},
+          {
+            // eslint-disable-next-line @typescript-eslint/no-unused-vars
+            onPreAssignment(experimentKey: string, subject: string): string | null {
+              return null;
+            },
 
-          // eslint-disable-next-line @typescript-eslint/no-unused-vars
-          onPostAssignment(variation: string): Promise<void> {
-            return Promise.resolve();
+            // eslint-disable-next-line @typescript-eslint/no-unused-vars
+            onPostAssignment(experimentKey: string, subject: string, variation: string): void {
+              // no-op
+            },
           },
-        });
+        );
 
         expect(variation).not.toEqual(null);
       });
@@ -352,13 +363,13 @@ describe('EppoClient E2E test', () => {
     describe('onPostAssignment', () => {
       it('called with assigned variation after assignment', async () => {
         const mockHooks = td.object<IAssignmentHooks>();
-        const variation = await client.getAssignmentWithHooks(
-          'subject-identifer',
-          experimentName,
-          mockHooks,
-        );
+        const subject = 'subject-identifier';
+        const variation = client.getAssignment(subject, experimentName, {}, mockHooks);
+        expect(td.explain(mockHooks.onPostAssignment).callCount).toEqual(1);
         expect(td.explain(mockHooks.onPostAssignment).callCount).toEqual(1);
-        expect(td.explain(mockHooks.onPostAssignment).calls[0].args[0]).toEqual(variation);
+        expect(td.explain(mockHooks.onPostAssignment).calls[0].args[0]).toEqual(experimentName);
+        expect(td.explain(mockHooks.onPostAssignment).calls[0].args[1]).toEqual(subject);
+        expect(td.explain(mockHooks.onPostAssignment).calls[0].args[2]).toEqual(variation);
       });
     });
   });

src/client/eppo-client.ts

@@ -22,6 +22,7 @@ export interface IEppoClient {
    * @param experimentKey experiment identifier
    * @param subjectAttributes optional attributes associated with the subject, for example name and email.
    * The subject attributes are used for evaluating any targeting rules tied to the experiment.
+   * @param assignmentHooks optional interface for pre and post assignment hooks
    * @returns a variation value if the subject is part of the experiment sample, otherwise null
    * @public
    */
@@ -30,26 +31,8 @@ export interface IEppoClient {
     experimentKey: string,
     // eslint-disable-next-line @typescript-eslint/no-explicit-any
     subjectAttributes?: Record<string, any>,
+    assignmentHooks?: IAssignmentHooks,
   ): string;
-
-  /**
-   * Asynchronously maps a subject to a variation for a given experiment, with pre and post assignment hooks
-   *
-   * @param subjectKey an identifier of the experiment subject, for example a user ID.
-   * @param experimentKey experiment identifier
-   * @param assignmentHooks interface for pre and post assignment hooks
-   * @param subjectAttributes optional attributes associated with the subject, for example name and email.
-   * The subject attributes are used for evaluating any targeting rules tied to the experiment.
-   * @returns a variation value if the subject is part of the experiment sample, otherwise null
-   * @public
-   */
-  getAssignmentWithHooks(
-    subjectKey: string,
-    experimentKey: string,
-    assignmentHooks: IAssignmentHooks,
-    // eslint-disable-next-line @typescript-eslint/no-explicit-any
-    subjectAttributes?: Record<string, any>,
-  ): Promise<string>;
 }
 
 export default class EppoClient implements IEppoClient {
@@ -58,10 +41,35 @@ export default class EppoClient implements IEppoClient {
 
   constructor(private configurationStore: IConfigurationStore) {}
 
-  getAssignment(subjectKey: string, experimentKey: string, subjectAttributes = {}): string {
+  getAssignment(
+    subjectKey: string,
+    experimentKey: string,
+    subjectAttributes = {},
+    assignmentHooks: IAssignmentHooks = null,
+  ): string {
     validateNotBlank(subjectKey, 'Invalid argument: subjectKey cannot be blank');
     validateNotBlank(experimentKey, 'Invalid argument: experimentKey cannot be blank');
 
+    const assignment = this.getAssignmentInternal(
+      subjectKey,
+      experimentKey,
+      subjectAttributes,
+      assignmentHooks,
+    );
+    assignmentHooks?.onPostAssignment(experimentKey, subjectKey, assignment);
+
+    if (assignment !== null)
+      this.logAssignment(experimentKey, assignment, subjectKey, subjectAttributes);
+
+    return assignment;
+  }
+
+  private getAssignmentInternal(
+    subjectKey: string,
+    experimentKey: string,
+    subjectAttributes = {},
+    assignmentHooks: IAssignmentHooks = null,
+  ): string {
     const experimentConfig = this.configurationStore.get<IExperimentConfiguration>(experimentKey);
     const allowListOverride = this.getSubjectVariationOverride(subjectKey, experimentConfig);
 
@@ -70,6 +78,12 @@ export default class EppoClient implements IEppoClient {
     // Check for disabled flag.
     if (!experimentConfig?.enabled) return null;
 
+    // check for overridden assignment via hook
+    const overriddenAssignment = assignmentHooks?.onPreAssignment(experimentKey, subjectKey);
+    if (overriddenAssignment !== null && overriddenAssignment !== undefined) {
+      return overriddenAssignment;
+    }
+
     // Attempt to match a rule from the list.
     const matchedRule = findMatchingRule(subjectAttributes || {}, experimentConfig.rules);
     if (!matchedRule) return null;
@@ -88,27 +102,9 @@ export default class EppoClient implements IEppoClient {
       isShardInRange(shard, variation.shardRange),
     ).value;
 
-    // Finally, log assignment and return assignment.
-    this.logAssignment(experimentKey, assignedVariation, subjectKey, subjectAttributes);
     return assignedVariation;
   }
 
-  async getAssignmentWithHooks(
-    subjectKey: string,
-    experimentKey: string,
-    assignmentHooks: IAssignmentHooks,
-    subjectAttributes = {},
-  ): Promise<string> {
-    let assignment = await assignmentHooks?.onPreAssignment(subjectKey);
-    if (assignment == null) {
-      assignment = this.getAssignment(subjectKey, experimentKey, subjectAttributes);
-    }
-
-    assignmentHooks?.onPostAssignment(assignment);
-
-    return assignment;
-  }
-
   public setLogger(logger: IAssignmentLogger) {
     this.assignmentLogger = logger;
     this.flushQueuedEvents(); // log any events that may have been queued while initializing