Bug 1961629 - Use ExperimentFeature.getEnrollmentMetadata() in browser/components/asrouter r=omc-reviewers,aminomancer

Differential Revision: https://phabricator.services.mozilla.com/D246149

browser/components/asrouter/modules/ASRouter.sys.mjs

@@ -356,16 +356,11 @@ export const MessageLoaderUtils = {
     let experiments = [];
     for (const featureId of featureIds) {
       const featureAPI = lazy.NimbusFeatures[featureId];
-      const experimentData = lazy.ExperimentAPI.getExperimentMetaData({
+      const enrollmentData = featureAPI.getEnrollmentMetadata();
-        featureId,
-      });
 
       // We are not enrolled in any experiment or rollout for this feature, so
       // we can skip the feature.
-      if (
+      if (!enrollmentData) {
-        !experimentData &&
-        !lazy.ExperimentAPI.getRolloutMetaData({ featureId })
-      ) {
         continue;
       }
 
@@ -393,7 +388,7 @@ export const MessageLoaderUtils = {
       if (
         NO_REACH_EVENT_GROUPS.includes(featureId) ||
         !MESSAGING_EXPERIMENTS_DEFAULT_FEATURES.includes(featureId) ||
-        !experimentData
+        enrollmentData.isRollout
       ) {
         continue;
       }
@@ -402,10 +397,10 @@ export const MessageLoaderUtils = {
       // if found any. The `forReachEvent` label is used to identify those
       // branches so that they would only be used to record the Reach event.
       const branches =
-        (await lazy.ExperimentAPI.getAllBranches(experimentData.slug)) || [];
+        (await lazy.ExperimentAPI.getAllBranches(enrollmentData.slug)) || [];
       for (const branch of branches) {
         let branchValue = branch[featureId].value;
-        if (!branchValue || branch.slug === experimentData.branch.slug) {
+        if (!branchValue || branch.slug === enrollmentData.branch) {
           continue;
         }
         const branchMessages =
@@ -419,7 +414,7 @@ export const MessageLoaderUtils = {
           }
           experiments.push({
             forReachEvent: { sent: false, group: featureId },
-            experimentSlug: experimentData.slug,
+            experimentSlug: enrollmentData.slug,
             branchSlug: branch.slug,
             ...message,
           });

browser/components/asrouter/modules/ASRouterTelemetry.sys.mjs

@@ -24,7 +24,8 @@ ChromeUtils.defineESModuleGetters(lazy, {
   ClientID: "resource://gre/modules/ClientID.sys.mjs",
   AboutWelcomeTelemetry:
     "resource:///modules/aboutwelcome/AboutWelcomeTelemetry.sys.mjs",
-  ExperimentAPI: "resource://nimbus/ExperimentAPI.sys.mjs",
+  EnrollmentType: "resource://nimbus/ExperimentAPI.sys.mjs",
+  NimbusFeatures: "resource://nimbus/ExperimentAPI.sys.mjs",
   TelemetrySession: "resource://gre/modules/TelemetrySession.sys.mjs",
   UpdateUtils: "resource://gre/modules/UpdateUtils.sys.mjs",
 });
@@ -76,14 +77,9 @@ export class ASRouterTelemetry {
    *  @return {bool}
    */
   get isInCFRCohort() {
-    const experimentData = lazy.ExperimentAPI.getExperimentMetaData({
+    return !!lazy.NimbusFeatures.cfr.getEnrollmentMetadata(
-      featureId: "cfr",
+      lazy.EnrollmentType.EXPERIMENT
-    });
+    );
-    if (experimentData && experimentData.slug) {
-      return true;
-    }
-
-    return false;
   }
 
   /**

browser/components/asrouter/modules/ToastNotification.sys.mjs

@@ -7,7 +7,8 @@ import { XPCOMUtils } from "resource://gre/modules/XPCOMUtils.sys.mjs";
 const lazy = {};
 
 ChromeUtils.defineESModuleGetters(lazy, {
-  ExperimentAPI: "resource://nimbus/ExperimentAPI.sys.mjs",
+  NimbusFeatures: "resource://nimbus/ExperimentAPI.sys.mjs",
+  EnrollmentType: "resource://nimbus/ExperimentAPI.sys.mjs",
   RemoteL10n: "resource:///modules/asrouter/RemoteL10n.sys.mjs",
 });
 
@@ -51,17 +52,13 @@ export const ToastNotification = {
     let { tag } = content;
 
     let experimentMetadata =
-      lazy.ExperimentAPI.getExperimentMetaData({
+      lazy.NimbusFeatures.backgroundTaskMessage.getEnrollmentMetadata(
-        featureId: "backgroundTaskMessage",
+        lazy.EnrollmentType.EXPERIMENT
-      }) || {};
+      ) ?? {};
 
-    if (
+    if (experimentMetadata) {
-      experimentMetadata?.active &&
-      experimentMetadata?.slug &&
-      experimentMetadata?.branch?.slug
-    ) {
       // Like `my-experiment:my-branch`.
-      tag = `${experimentMetadata?.slug}:${experimentMetadata?.branch?.slug}`;
+      tag = `${experimentMetadata.slug}:${experimentMetadata.branch}`;
     }
 
     // There are two events named `IMPRESSION` the first one refers to telemetry

browser/components/asrouter/tests/browser/browser_asrouter_experimentsAPILoader.js

@@ -236,7 +236,7 @@ add_task(async function test_loading_experimentsAPI_rollout() {
   await setup(rollout);
   await RemoteSettingsExperimentLoader.updateRecipes();
   await BrowserTestUtils.waitForCondition(() =>
-    ExperimentAPI.getRolloutMetaData({ featureId: "cfr" })
+    NimbusFeatures.cfr.getEnrollmentMetadata("rollout")
   );
 
   await assertMessageInState("xman_test_message");

browser/components/asrouter/tests/unit/ASRouter.test.js

@@ -231,7 +231,12 @@ describe("ASRouter", () => {
       "pbNewtab",
     ].reduce((features, featureId) => {
       features[featureId] = {
-        getAllVariables: sandbox.stub().returns(null),
+        getEnrollmentMetadata: sandbox.stub().returns({
+          slug: "experiment-slug",
+          branch: "experiment-branch-slug",
+          isRollout: false,
+        }),
+        getAllVariables: sandbox.stub().returns(undefined),
         recordExposureEvent: sandbox.stub(),
       };
       return features;
@@ -271,11 +276,6 @@ describe("ASRouter", () => {
       },
       NimbusFeatures: fakeNimbusFeatures,
       ExperimentAPI: {
-        getExperimentMetaData: sandbox.stub().returns({
-          slug: "experiment-slug",
-          active: true,
-          branch: { slug: "experiment-branch-slug" },
-        }),
         getAllBranches: sandbox.stub().resolves([]),
         ready: sandbox.stub().resolves(),
       },
@@ -1578,17 +1578,15 @@ describe("ASRouter", () => {
     let experimentAPIStub;
     let featureIds = ["cfr", "moments-page", "infobar", "spotlight"];
     beforeEach(() => {
-      let getExperimentMetaDataStub = sandbox.stub();
       let getAllBranchesStub = sandbox.stub();
       featureIds.forEach(feature => {
         global.NimbusFeatures[feature].getAllVariables.returns({
           id: `message-${feature}`,
         });
-        getExperimentMetaDataStub.withArgs({ featureId: feature }).returns({
+        global.NimbusFeatures[feature].getEnrollmentMetadata.returns({
           slug: `slug-${feature}`,
-          branch: {
+          branch: `branch-${feature}`,
-            slug: `branch-${feature}`,
+          isRollout: false,
-          },
         });
         getAllBranchesStub.withArgs(`slug-${feature}`).resolves([
           {
@@ -1598,7 +1596,6 @@ describe("ASRouter", () => {
         ]);
       });
       experimentAPIStub = {
-        getExperimentMetaData: getExperimentMetaDataStub,
         getAllBranches: getAllBranchesStub,
       };
       globals.set("ExperimentAPI", experimentAPIStub);
@@ -2461,11 +2458,8 @@ describe("ASRouter", () => {
 
       await MessageLoaderUtils.loadMessagesForProvider(args);
 
+      assert.calledOnce(global.NimbusFeatures.spotlight.getEnrollmentMetadata);
       assert.calledOnce(global.NimbusFeatures.spotlight.getAllVariables);
-      assert.calledOnce(global.ExperimentAPI.getExperimentMetaData);
-      assert.calledWithExactly(global.ExperimentAPI.getExperimentMetaData, {
-        featureId: "spotlight",
-      });
     });
     it("should handle the case of no experiments in the ExperimentAPI", async () => {
       const args = {
@@ -2483,6 +2477,7 @@ describe("ASRouter", () => {
         featureIds: ["infobar"],
       };
       const enrollment = {
+        slug: "enrollment01",
         branch: {
           slug: "branch01",
           infobar: {
@@ -2495,8 +2490,10 @@ describe("ASRouter", () => {
       global.NimbusFeatures.infobar.getAllVariables.returns(
         enrollment.branch.infobar.value
       );
-      global.ExperimentAPI.getExperimentMetaData.returns({
+      global.NimbusFeatures.infobar.getEnrollmentMetadata.returns({
-        branch: { slug: enrollment.branch.slug },
+        slug: enrollment.slug,
+        branch: enrollment.branch.slug,
+        isRollout: false,
       });
       global.ExperimentAPI.getAllBranches.returns([
         enrollment.branch,
@@ -2544,10 +2541,10 @@ describe("ASRouter", () => {
       global.NimbusFeatures.cfr.getAllVariables.returns(
         enrollment.branch.cfr.value
       );
-      global.ExperimentAPI.getExperimentMetaData.returns({
+      global.NimbusFeatures.cfr.getEnrollmentMetadata.returns({
         slug: enrollment.slug,
-        active: true,
+        branch: enrollment.branch.slug,
-        branch: { slug: enrollment.branch.slug },
+        isRollout: false,
       });
       global.ExperimentAPI.getAllBranches.resolves([
         enrollment.branch,
@@ -2596,15 +2593,15 @@ describe("ASRouter", () => {
         },
       };
 
-      // Nedds to match the `featureIds` value to return an enrollment
+      // Needs to match the `featureIds` value to return an enrollment
       // for that feature
       global.NimbusFeatures.cfr.getAllVariables.returns(
         enrollment.branch.cfr.value
       );
-      global.ExperimentAPI.getExperimentMetaData.returns({
+      global.NimbusFeatures.cfr.getEnrollmentMetadata.returns({
         slug: enrollment.slug,
-        active: true,
+        branch: enrollment.branch.slug,
-        branch: { slug: enrollment.branch.slug },
+        isRollout: false,
       });
       global.ExperimentAPI.getAllBranches.resolves([
         enrollment.branch,

browser/components/asrouter/tests/unit/unit-entry.js

@@ -4,6 +4,7 @@ import {
   GlobalOverrider,
   FakeConsoleAPI,
   FakeLogger,
+  FakeNimbusFeatures,
 } from "tests/unit/utils";
 import Adapter from "enzyme-adapter-react-16";
 import chaiJsonSchema from "chai-json-schema";
@@ -13,6 +14,7 @@ import {
   MESSAGE_TYPE_LIST,
   MESSAGE_TYPE_HASH,
 } from "modules/ActorConstants.mjs";
+import { MESSAGING_EXPERIMENTS_DEFAULT_FEATURES } from "modules/MessagingExperimentConstants.sys.mjs";
 
 enzyme.configure({ adapter: new Adapter() });
 
@@ -515,30 +517,14 @@ const TEST_GLOBAL = {
     },
   },
   FX_MONITOR_OAUTH_CLIENT_ID: "fake_client_id",
-  ExperimentAPI: {
+  ExperimentAPI: {},
-    getExperimentMetaData() {},
+  NimbusFeatures: FakeNimbusFeatures([
-    getRolloutMetaData() {},
+    ...MESSAGING_EXPERIMENTS_DEFAULT_FEATURES,
-  },
+    "glean",
-  NimbusFeatures: {
+    "newtab",
-    glean: {
+    "pocketNewtab",
-      getVariable() {},
+    "cookieBannerHandling",
-    },
+  ]),
-    newtab: {
-      getVariable() {},
-      getAllVariables() {},
-      onUpdate() {},
-      offUpdate() {},
-    },
-    pocketNewtab: {
-      getVariable() {},
-      getAllVariables() {},
-      onUpdate() {},
-      offUpdate() {},
-    },
-    cookieBannerHandling: {
-      getVariable() {},
-    },
-  },
   TelemetryEnvironment: {
     setExperimentActive() {},
     currentEnvironment: {

browser/components/asrouter/tests/unit/utils.js

@@ -268,6 +268,22 @@ export class FakeConsoleAPI {
   }
 }
 
+export function FakeNimbusFeature() {
+  return {
+    getEnrollmentMetadata() {},
+    getVariable() {},
+    getAllVariables() {},
+    onUpdate() {},
+    offUpdate() {},
+  };
+}
+
+export function FakeNimbusFeatures(featureIds) {
+  return Object.fromEntries(
+    featureIds.map(featureId => [featureId, FakeNimbusFeature()])
+  );
+}
+
 export class FakeLogger extends FakeConsoleAPI {
   constructor() {
     super({

browser/components/asrouter/tests/xpcshell/test_ASRouterTelemetry.js

@@ -14,7 +14,7 @@ const { ASRouterTelemetry } = ChromeUtils.importESModule(
 ChromeUtils.defineESModuleGetters(this, {
   AboutWelcomeTelemetry:
     "resource:///modules/aboutwelcome/AboutWelcomeTelemetry.sys.mjs",
-  ExperimentAPI: "resource://nimbus/ExperimentAPI.sys.mjs",
+  NimbusFeatures: "resource://nimbus/ExperimentAPI.sys.mjs",
   JsonSchemaValidator:
     "resource://gre/modules/components-utils/JsonSchemaValidator.sys.mjs",
   sinon: "resource://testing-common/Sinon.sys.mjs",
@@ -126,8 +126,10 @@ add_task(async function test_applyCFRPolicy_experiment_release() {
   );
   let sandbox = sinon.createSandbox();
   sandbox.stub(UpdateUtils, "getUpdateChannel").returns("release");
-  sandbox.stub(ExperimentAPI, "getExperimentMetaData").returns({
+  sandbox.stub(NimbusFeatures.cfr, "getEnrollmentMetadata").returns({
     slug: "SOME-CFR-EXP",
+    branch: "branch-slug",
+    isRollout: false,
   });
 
   let instance = new ASRouterTelemetry();
@@ -191,8 +193,10 @@ add_task(
     );
     let sandbox = sinon.createSandbox();
     sandbox.stub(UpdateUtils, "getUpdateChannel").returns("release");
-    sandbox.stub(ExperimentAPI, "getExperimentMetaData").returns({
+    sandbox.stub(NimbusFeatures.cfr, "getEnrollmentMetadata").returns({
       slug: "SOME-CFR-EXP",
+      branch: "branch-slug",
+      isRollout: false,
     });
 
     let instance = new ASRouterTelemetry();
@@ -345,8 +349,10 @@ add_task(async function test_applyMomentsPolicy_experiment_release() {
   );
   let sandbox = sinon.createSandbox();
   sandbox.stub(UpdateUtils, "getUpdateChannel").returns("release");
-  sandbox.stub(ExperimentAPI, "getExperimentMetaData").returns({
+  sandbox.stub(NimbusFeatures.cfr, "getEnrollmentMetadata").returns({
     slug: "SOME-CFR-EXP",
+    branch: "branch-slug",
+    isRollout: false,
   });
 
   let instance = new ASRouterTelemetry();
@@ -663,15 +669,13 @@ add_task(
     let sandbox = sinon.createSandbox();
     let instance = new ASRouterTelemetry();
 
-    sandbox.stub(ExperimentAPI, "getExperimentMetaData").returns({
+    sandbox.stub(NimbusFeatures.cfr, "getEnrollmentMetadata").returns({
       slug: "SOME-CFR-EXP",
+      branch: "branch-slug",
+      isRollout: false,
     });
 
     Assert.ok(instance.isInCFRCohort, "Should be in a CFR cohort");
-    Assert.equal(
-      ExperimentAPI.getExperimentMetaData.firstCall.args[0].featureId,
-      "cfr"
-    );
 
     sandbox.restore();
   }