Просмотр исходного кода

[web] avoid UserAnalyticsIdCache for split-test assignments (#34803)

* [web] split-tests: add helpers to using an already fetched mongo user

* [web] split-tests: add ignoreOverrides option to getAssignment

* [web] split-tests: reuse already fetched mongo users where possible

* [web] notifications: fetch users in bulk for split-test assignment

* [web] split-tests: use session for assignment where possible

* [web] split-tests: pass req/res down into domain-capture hook

* [web] split-tests: explicitly throw early

* [web] split-tests: refine jsdoc types

* [web] mark analyticsId and labsProgram as required in user model

* [web] split-tests: refine jsdoc types

* Revert "[web] split-tests: pass req/res down into domain-capture hook"

This reverts commit 897a158384cd2b3e416fc34495b20d1e2a0e68f0.

GitOrigin-RevId: ea4afd15c2506f369827b5193fd16d220d3c7d0e
Jakob Ackermann 1 месяц назад
Родитель
Сommit
1cc18dd0f0

+ 5 - 1
services/web/app/src/Features/Project/ProjectController.mjs

@@ -648,7 +648,11 @@ const _ProjectController = {
         req,
         projectId
       )
-      const imageNames = await ProjectHelper.getAllowedImagesForUser(user)
+      const imageNames = await ProjectHelper.getAllowedImagesForUser(
+        req,
+        res,
+        user
+      )
 
       const privilegeLevel =
         await AuthorizationManager.promises.getPrivilegeLevelForProject(

+ 4 - 2
services/web/app/src/Features/Project/ProjectCreationHandler.mjs

@@ -17,6 +17,7 @@ import _ from 'lodash'
 import AnalyticsManager from '../Analytics/AnalyticsManager.mjs'
 import TpdsUpdateSender from '../ThirdPartyDataStore/TpdsUpdateSender.mjs'
 import SplitTestHandler from '../SplitTests/SplitTestHandler.mjs'
+import SplitTestUserGetter from '../SplitTests/SplitTestUserGetter.mjs'
 import ClsiCacheManager from '../Compile/ClsiCacheManager.mjs'
 import crypto from 'node:crypto'
 
@@ -271,11 +272,12 @@ async function _createBlankProject(
   const user = await User.findById(ownerId, {
     'ace.spellCheckLanguage': 1,
     _id: 1,
+    ...SplitTestUserGetter.getProjection('history-ranges-support'),
   })
   project.spellCheckLanguage = user.ace.spellCheckLanguage
   const historyRangesSupportAssignment =
-    await SplitTestHandler.promises.getAssignmentForUser(
-      user._id,
+    await SplitTestHandler.promises.getAssignmentForMongoUser(
+      user,
       'history-ranges-support'
     )
   if (historyRangesSupportAssignment.variant === 'enabled') {

+ 6 - 8
services/web/app/src/Features/Project/ProjectHelper.mjs

@@ -149,14 +149,12 @@ function _addNumericSuffixToProjectName(name, allProjectNames, maxLength) {
   return null
 }
 
-async function _monthlyExperimentalImageAllowed(user) {
-  const userId = user?._id?.toString()
-  if (!userId) return false
-  const { variant } = await SplitTestHandler.promises.getAssignmentForUser(
-    userId,
+async function _monthlyExperimentalImageAllowed(req, res) {
+  return await SplitTestHandler.promises.featureFlagEnabled(
+    req,
+    res,
     'monthly-texlive'
   )
-  return variant === 'enabled'
 }
 
 function _imageAllowed(
@@ -173,12 +171,12 @@ function _imageAllowed(
   return true
 }
 
-async function getAllowedImagesForUser(user) {
+async function getAllowedImagesForUser(req, res, user) {
   let images = Settings.allowedImageNames || []
 
   const alphaImagesAllowed = Boolean(user?.alphaProgram)
   const monthlyExperimentalImagesAllowed =
-    await _monthlyExperimentalImageAllowed(user)
+    await _monthlyExperimentalImageAllowed(req, res)
 
   images = images.map(image => {
     return {

+ 9 - 7
services/web/app/src/Features/Project/ProjectListController.mjs

@@ -539,7 +539,7 @@ async function projectListPage(req, res, next) {
   const aiBlocked =
     Features.hasFeature('saas') && !(await _canUseAIAssist(user))
   const hasAiAssist =
-    Features.hasFeature('saas') && (await _userHasAIAssist(user))
+    Features.hasFeature('saas') && (await _userHasAIAssist(req, res, user))
 
   const splitTests = [
     // Split tests that will be made available to the frontend
@@ -949,16 +949,18 @@ function _hasActiveFilter(filters) {
 }
 
 /**
+ * @param {any} req
+ * @param {any} res
  * @param {any} user
  */
 // todo: quota clean-up: rename function and vars
-async function _userHasAIAssist(user) {
+async function _userHasAIAssist(req, res, user) {
   let hasPremiumAiFeatures
-  const inQuotaSplitTest =
-    await SplitTestHandler.promises.featureFlagEnabledForUser(
-      user._id,
-      'plans-2026-phase-1'
-    )
+  const inQuotaSplitTest = await SplitTestHandler.promises.featureFlagEnabled(
+    req,
+    res,
+    'plans-2026-phase-1'
+  )
   if (inQuotaSplitTest) {
     hasPremiumAiFeatures =
       user.features?.aiUsageQuota === Settings.aiFeatures.unlimitedQuota

+ 122 - 27
services/web/app/src/Features/SplitTests/SplitTestHandler.mjs

@@ -18,6 +18,7 @@ import SplitTestUserGetter from './SplitTestUserGetter.mjs'
 
 /**
  * @import { Assignment } from "./types"
+ * @import { SplitTestUser } from "./SplitTestUserGetter"
  */
 
 const DEFAULT_VARIANT = 'default'
@@ -47,16 +48,17 @@ const DEFAULT_ASSIGNMENT = {
  * @param req the request
  * @param res the Express response object
  * @param splitTestName the unique name of the split test
- * @param {Object} options
- * @param {boolean} options.sync - for test purposes only, to force the synchronous update of the user's profile
- * @param {boolean} options.includeReferer For ajax requests and downloads include the split test overrides of the page
+ * @param {Object} [options]
+ * @param {boolean} [options.sync] - for test purposes only, to force the synchronous update of the user's profile
+ * @param {boolean} [options.includeReferer] For ajax requests and downloads include the split test overrides of the page
+ * @param {boolean} [options.ignoreOverrides] Ignore query-string variant overrides (e.g. for backend gating where the user must not be able to force a variant)
  * @returns {Promise<Assignment>}
  */
 async function getAssignment(
   req,
   res,
   splitTestName,
-  { sync = false, includeReferer = false } = {}
+  { sync = false, includeReferer = false, ignoreOverrides = false } = {}
 ) {
   let assignment
 
@@ -66,28 +68,30 @@ async function getAssignment(
     } else {
       await _loadSplitTestInfoInLocals(res.locals, splitTestName, req.session)
 
-      let query = req.query || {}
-      if (includeReferer && req.headers.referer) {
-        // Pick up the query of the top-level page, i.e. what's in the browsers address bar, from ajax requests.
-        // E.g. /project/:id?split-test=foo -> ajax /project/:id/compile should see split-test=foo.
-        // E.g. /project/:id?split-test=foo -> redirect /project/:id/download/zip should see split-test=foo.
-        try {
-          const u = new URL(req.headers.referer, Settings.siteUrl)
-          query = {
-            ...Object.fromEntries(u.searchParams.entries()),
-            ...query,
-          }
-        } catch {}
-      }
+      if (!ignoreOverrides) {
+        let query = req.query || {}
+        if (includeReferer && req.headers.referer) {
+          // Pick up the query of the top-level page, i.e. what's in the browsers address bar, from ajax requests.
+          // E.g. /project/:id?split-test=foo -> ajax /project/:id/compile should see split-test=foo.
+          // E.g. /project/:id?split-test=foo -> redirect /project/:id/download/zip should see split-test=foo.
+          try {
+            const u = new URL(req.headers.referer, Settings.siteUrl)
+            query = {
+              ...Object.fromEntries(u.searchParams.entries()),
+              ...query,
+            }
+          } catch {}
+        }
 
-      // Check the query string for an override, ignoring an invalid value
-      const queryVariant = query[splitTestName]
-      if (queryVariant) {
-        const variants = await _getVariantNames(splitTestName)
-        if (variants.includes(queryVariant)) {
-          assignment = {
-            variant: queryVariant,
-            metadata: {},
+        // Check the query string for an override, ignoring an invalid value
+        const queryVariant = query[splitTestName]
+        if (queryVariant) {
+          const variants = await _getVariantNames(splitTestName)
+          if (variants.includes(queryVariant)) {
+            assignment = {
+              variant: queryVariant,
+              metadata: {},
+            }
           }
         }
       }
@@ -151,6 +155,33 @@ async function getAssignmentForUser(
   }
 }
 
+/**
+ * Get the assignment of a user to a split test from an already-fetched mongo user.
+ *
+ * The user must include all the relevant fields. Unless you fetch the full user record, add `SplitTestUserGetter.getProjection(splitTestName)` to the projection.
+ *
+ * @param {SplitTestUser} user an already-fetched mongo user
+ * @param splitTestName the unique name of the split test
+ * @param options {Object<sync: boolean>} - for test purposes only, to force the synchronous update of the user's profile
+ * @returns {Promise<Assignment>}
+ */
+async function getAssignmentForMongoUser(
+  user,
+  splitTestName,
+  { sync = false } = {}
+) {
+  const { userId, analyticsId } = _getIdsFromMongoUser(user) // throw outside the try/catch.
+  try {
+    if (!Features.hasFeature('saas')) {
+      return _getNonSaasAssignment(splitTestName)
+    }
+    return _getAssignment(splitTestName, { analyticsId, userId, user, sync })
+  } catch (error) {
+    logger.error({ err: error }, 'Failed to get split test assignment for user')
+    return DEFAULT_ASSIGNMENT
+  }
+}
+
 /**
  * Returns true if user has already been explicitly assigned to a variant.
  * This will be false if the user **would** be assigned when calling getAssignment but hasn't yet.
@@ -259,13 +290,33 @@ async function getActiveAssignmentsForUser(
     return {}
   }
 
+  return getActiveAssignmentsForMongoUser(user, removeArchived, ignoreVersion)
+}
+
+/**
+ * Get a mapping of the active split test assignments from an already-fetched mongo user, avoiding a re-fetch. This should be the full user record.
+ * @param {SplitTestUser} user
+ * @param {boolean} removeArchived
+ * @param {boolean} ignoreVersion
+ */
+async function getActiveAssignmentsForMongoUser(
+  user,
+  removeArchived = false,
+  ignoreVersion = false
+) {
+  if (!Features.hasFeature('saas')) {
+    return {}
+  }
+
+  const { analyticsId } = _getIdsFromMongoUser(user) // throw early.
+
   const splitTests = (await SplitTestCache.get('')).values()
   const assignments = {}
   for (const splitTest of splitTests) {
     if (!splitTest.versions[splitTest.versions.length - 1].active) continue
     if (removeArchived && splitTest.archived) continue
     const { activeForUser, selectedVariantName, phase, versionNumber } =
-      await _getAssignmentMetadata(user.analyticsId, user, splitTest)
+      await _getAssignmentMetadata(analyticsId, user, splitTest)
     if (activeForUser) {
       const assignment = {
         variantName: selectedVariantName,
@@ -371,6 +422,20 @@ async function featureFlagEnabledForUser(userId, splitTestName) {
   return variant === 'enabled'
 }
 
+/**
+ * Checks if a feature flag is enabled from an already-fetched mongo user
+ *
+ * See getAssignmentForMongoUser for details on the user.
+ *
+ * @param {SplitTestUser} user an already-fetched mongo user
+ * @param {string} splitTestName - The unique name of the feature flag
+ * @returns {Promise<boolean>} True if the user's assigned variant is 'enabled', false otherwise
+ */
+async function featureFlagEnabledForMongoUser(user, splitTestName) {
+  const { variant } = await getAssignmentForMongoUser(user, splitTestName)
+  return variant === 'enabled'
+}
+
 /**
  * Returns an array of valid variant names for the given split test, including default
  *
@@ -388,6 +453,28 @@ async function _getVariantNames(splitTestName) {
   }
 }
 
+/**
+ * Extract the ids needed for a split test assignment from an already-fetched
+ * mongo user, throwing if a required field is missing from the projection.
+ *
+ * Only the ids are validated: the program/`splitTests` fields are read with
+ * optional chaining and a missing value is a legitimate "not enrolled" state.
+ *
+ * @param {SplitTestUser} user
+ * @return {{userId: string, analyticsId: string}}
+ */
+function _getIdsFromMongoUser(user) {
+  const userId = user?._id?.toString()
+  if (!userId) {
+    throw new Error('bug: include db.users._id in projection')
+  }
+  const analyticsId = user?.analyticsId
+  if (!analyticsId) {
+    throw new Error('bug: include db.users.analyticsId in projection')
+  }
+  return { userId, analyticsId }
+}
+
 async function _getAssignment(
   splitTestName,
   { analyticsId, user, userId, session, sync }
@@ -749,7 +836,7 @@ async function _recordAssignment({
  * @param {string} splitTestName - The name of the split test
  * @param {string} variantName - The name of the variant
  * @param {string} phase - The phase of the split test
- * @param {Object} user - The user object
+ * @param {SplitTestUser} user - The user object
  * @returns {Promise<boolean>} Whether the counter should be incremented
  */
 async function _shouldIncrementVariantCounter(
@@ -1042,20 +1129,28 @@ export default {
   getPercentile,
   getAssignment: callbackify(getAssignment),
   getAssignmentForUser: callbackify(getAssignmentForUser),
+  getAssignmentForMongoUser: callbackify(getAssignmentForMongoUser),
   featureFlagEnabled: callbackify(featureFlagEnabled),
   featureFlagEnabledForUser: callbackify(featureFlagEnabledForUser),
+  featureFlagEnabledForMongoUser: callbackify(featureFlagEnabledForMongoUser),
   getOneTimeAssignment: callbackify(getOneTimeAssignment),
   getActiveAssignmentsForUser: callbackify(getActiveAssignmentsForUser),
+  getActiveAssignmentsForMongoUser: callbackify(
+    getActiveAssignmentsForMongoUser
+  ),
   hasUserBeenAssignedToVariant: callbackify(hasUserBeenAssignedToVariant),
   setOverrideInSession,
   clearOverridesInSession,
   promises: {
     getAssignment,
     getAssignmentForUser,
+    getAssignmentForMongoUser,
     featureFlagEnabled,
     featureFlagEnabledForUser,
+    featureFlagEnabledForMongoUser,
     getOneTimeAssignment,
     getActiveAssignmentsForUser,
+    getActiveAssignmentsForMongoUser,
     hasUserBeenAssignedToVariant,
     decrementLabsVariantCounter,
     incrementLabsVariantCounterIfBelowLimit,

+ 39 - 2
services/web/app/src/Features/SplitTests/SplitTestUserGetter.mjs

@@ -2,8 +2,32 @@ import { callbackify } from 'node:util'
 import Metrics from '@overleaf/metrics'
 import UserGetter from '../User/UserGetter.mjs'
 
-async function getUser(id, splitTestName, path) {
-  Metrics.inc('split_test_get_user', 1, { path })
+/**
+ * A mongo user fetched with the projection from `getProjection`, carrying the
+ * fields the split test assignment logic reads. This is the shape the
+ * `*ForMongoUser` SplitTestHandler methods expect.
+ *
+ * @typedef {object} SplitTestUser
+ * @property {import('mongodb').ObjectId} _id
+ * @property {string} analyticsId
+ * @property {boolean} [alphaProgram]
+ * @property {boolean} [betaProgram]
+ * @property {boolean} labsProgram
+ * @property {string[]} [labsExperiments]
+ * @property {Record<string, unknown>} [splitTests]
+ */
+
+/**
+ * Build the mongo projection needed to compute split test assignments for a user.
+ *
+ * Call-sites that already fetch a user and want to pass it to one of the
+ * `*ForMongoUser` SplitTestHandler methods should spread this into their own
+ * projection, so the user carries exactly the fields the assignment logic reads.
+ *
+ * @param {string} [splitTestName] restrict the `splitTests` sub-document to a
+ *   single test (for feature-flag style lookups); omit to fetch all assignments.
+ */
+function getProjection(splitTestName) {
   const projection = {
     analyticsId: 1,
     alphaProgram: 1,
@@ -16,6 +40,18 @@ async function getUser(id, splitTestName, path) {
   } else {
     projection.splitTests = 1
   }
+  return projection
+}
+
+/**
+ * @param id
+ * @param {string} splitTestName
+ * @param {string} path
+ * @return {Promise<SplitTestUser>}
+ */
+async function getUser(id, splitTestName, path) {
+  Metrics.inc('split_test_get_user', 1, { path })
+  const projection = getProjection(splitTestName)
   const user = await UserGetter.promises.getUser(id, projection)
   Metrics.histogram(
     'split_test_get_user_from_mongo_size',
@@ -26,6 +62,7 @@ async function getUser(id, splitTestName, path) {
 }
 
 export default {
+  getProjection,
   getUser: callbackify(getUser),
   promises: {
     getUser,

+ 5 - 4
services/web/app/src/Features/Subscription/FeaturesUpdater.mjs

@@ -21,6 +21,7 @@ import { GroupPolicy } from '../../models/GroupPolicy.mjs'
 import { AI_ADD_ON_CODE } from './AiHelper.mjs'
 import { fetchNothing } from '@overleaf/fetch-utils'
 import SplitTestHandler from '../SplitTests/SplitTestHandler.mjs'
+import SplitTestUserGetter from '../SplitTests/SplitTestUserGetter.mjs'
 
 /**
  * Enqueue a job for refreshing features for the given user
@@ -45,10 +46,10 @@ function featuresEpochIsCurrent(user) {
 async function refreshFeatures(userId, reason) {
   const user = await UserGetter.promises.getUser(userId, {
     _id: 1,
-    analyticsId: 1,
-    labsProgram: 1,
     features: 1,
     email: 1,
+    // analyticsId + labsProgram (analytics) and the split test fields below
+    ...SplitTestUserGetter.getProjection('plans-2026-phase-1'),
   })
   const oldFeatures = _.clone(user.features)
   const features = await computeFeatures(userId)
@@ -96,8 +97,8 @@ async function refreshFeatures(userId, reason) {
       // todo: quota clean-up: simplify once split test isnt needed
       let hasPremiumAiFeatures
       const inQuotaSplitTest =
-        await SplitTestHandler.promises.featureFlagEnabledForUser(
-          userId,
+        await SplitTestHandler.promises.featureFlagEnabledForMongoUser(
+          user,
           'plans-2026-phase-1'
         )
       if (inQuotaSplitTest) {

+ 4 - 2
services/web/app/src/infrastructure/rate-limiters/AiFeatureUsageRateLimiter.mjs

@@ -4,6 +4,7 @@ import UserGetter from '../../Features/User/UserGetter.mjs'
 import FeatureUsageRateLimiter from './FeatureUsageRateLimiter.mjs'
 import Settings from '@overleaf/settings'
 import SplitTestHandler from '../../Features/SplitTests/SplitTestHandler.mjs'
+import SplitTestUserGetter from '../../Features/SplitTests/SplitTestUserGetter.mjs'
 import FeaturesHelper from '../../Features/Subscription/FeaturesHelper.mjs'
 
 class AiFeatureUsageRateLimiter extends FeatureUsageRateLimiter {
@@ -19,11 +20,12 @@ class AiFeatureUsageRateLimiter extends FeatureUsageRateLimiter {
     const user = await UserGetter.promises.getUser(userId, {
       features: 1,
       writefull: 1,
+      ...SplitTestUserGetter.getProjection('plans-2026-phase-1'),
     })
     // todo: quota clean-up: remove aiErrorAssistant checking, and split test
     const inQuotaSplitTest =
-      await SplitTestHandler.promises.featureFlagEnabledForUser(
-        userId,
+      await SplitTestHandler.promises.featureFlagEnabledForMongoUser(
+        user,
         'plans-2026-phase-1'
       )
 

+ 4 - 3
services/web/app/src/infrastructure/rate-limiters/WorkbenchRateLimiter.mjs

@@ -1,5 +1,6 @@
 // @ts-check
 import SplitTestHandler from '../../Features/SplitTests/SplitTestHandler.mjs'
+import SplitTestUserGetter from '../../Features/SplitTests/SplitTestUserGetter.mjs'
 import UserGetter from '../../Features/User/UserGetter.mjs'
 import TokenUsageRateLimiter from './TokenUsageRateLimiter.mjs'
 /** @typedef {{usage?: number | null, periodStart?: Date | null}} FeatureUsage */
@@ -29,7 +30,7 @@ class WorkbenchRateLimiter extends TokenUsageRateLimiter {
     const user = await UserGetter.promises.getUser(userId, {
       features: 1,
       writefull: 1,
-      alphaProgram: 1,
+      ...SplitTestUserGetter.getProjection('plans-2026-phase-1'),
     })
 
     if (user?.alphaProgram) {
@@ -39,8 +40,8 @@ class WorkbenchRateLimiter extends TokenUsageRateLimiter {
     // todo: quota clean-up: remove split test
     let hasAddOn
     const inQuotaSplitTest =
-      await SplitTestHandler.promises.featureFlagEnabledForUser(
-        userId,
+      await SplitTestHandler.promises.featureFlagEnabledForMongoUser(
+        user,
         'plans-2026-phase-1'
       )
     if (inQuotaSplitTest) {

+ 2 - 2
services/web/app/src/models/User.mjs

@@ -221,7 +221,7 @@ export const UserSchema = new Schema(
     },
     alphaProgram: { type: Boolean, default: false }, // experimental features
     betaProgram: { type: Boolean, default: false },
-    labsProgram: { type: Boolean, default: false },
+    labsProgram: { type: Boolean, default: false, required: true },
     labsExperiments: { type: Array, default: [] },
     overleaf: {
       id: { type: Number },
@@ -239,7 +239,7 @@ export const UserSchema = new Schema(
     },
     onboardingEmailSentAt: { type: Date },
     splitTests: Schema.Types.Mixed,
-    analyticsId: { type: String },
+    analyticsId: { type: String, required: true },
     completedTutorials: Schema.Types.Mixed,
     suspended: { type: Boolean },
     dsMobileApp: {

+ 9 - 4
services/web/test/acceptance/src/ModelTests.mjs

@@ -6,15 +6,16 @@ import Features from '../../../app/src/infrastructure/Features.mjs'
 describe('mongoose', function () {
   describe('User', function () {
     const email = 'wombat@potato.net'
+    const defaultArgs = { analyticsId: '8055c676-bcc7-4e64-a66f-8069f9a0bd92' }
 
     it('allows the creation of a user', async function () {
-      await expect(User.create({ email })).to.be.fulfilled
+      await expect(User.create({ email, ...defaultArgs })).to.be.fulfilled
       await expect(User.findOne({ email }, { _id: 1 })).to.eventually.exist
     })
 
     it('does not allow the creation of multiple users with the same email', async function () {
-      await expect(User.create({ email })).to.be.fulfilled
-      await expect(User.create({ email })).to.be.rejected
+      await expect(User.create({ email, ...defaultArgs })).to.be.fulfilled
+      await expect(User.create({ email, ...defaultArgs })).to.be.rejected
       await expect(User.countDocuments({ email })).to.eventually.equal(1)
     })
 
@@ -38,6 +39,7 @@ describe('mongoose', function () {
               },
             ],
           },
+          ...defaultArgs,
         })
       ).to.be.fulfilled
 
@@ -55,7 +57,10 @@ describe('mongoose', function () {
         this.skip()
       }
 
-      user = await User.create({ email: 'wombat@potato.net' })
+      user = await User.create({
+        email: 'wombat@potato.net',
+        analyticsId: '8055c676-bcc7-4e64-a66f-8069f9a0bd92',
+      })
     })
 
     it('allows the creation of a subscription', async function () {

+ 26 - 8
services/web/test/unit/src/Project/ProjectHelper.test.mjs

@@ -51,9 +51,11 @@ describe('ProjectHelper', function () {
 
     ctx.SplitTestHandler = {
       promises: {
-        getAssignmentForUser: vi.fn().mockResolvedValue({ variant: 'default' }),
+        featureFlagEnabled: vi.fn().mockResolvedValue(false),
       },
     }
+    ctx.req = {}
+    ctx.res = {}
 
     vi.doMock('mongodb-legacy', () => ({
       default: { ObjectId },
@@ -159,7 +161,11 @@ describe('ProjectHelper', function () {
 
   describe('getAllowedImagesForUser', function () {
     it('marks alpha only images as not allowed when the user is anonymous', async function (ctx) {
-      const images = await ctx.ProjectHelper.getAllowedImagesForUser(null)
+      const images = await ctx.ProjectHelper.getAllowedImagesForUser(
+        ctx.req,
+        ctx.res,
+        null
+      )
       const imageNames = _mapToAllowed(images)
       expect(imageNames).to.deep.equal([
         { imageName: 'texlive-full:2018.1', allowed: true },
@@ -170,7 +176,11 @@ describe('ProjectHelper', function () {
     })
 
     it('marks monthly labs images as not allowed when the user is anonymous', async function (ctx) {
-      const images = await ctx.ProjectHelper.getAllowedImagesForUser(null)
+      const images = await ctx.ProjectHelper.getAllowedImagesForUser(
+        ctx.req,
+        ctx.res,
+        null
+      )
       const imageNames = _mapToAllowed(images)
       expect(imageNames).to.deep.equal([
         { imageName: 'texlive-full:2018.1', allowed: true },
@@ -181,10 +191,12 @@ describe('ProjectHelper', function () {
     })
 
     it('marks monthly labs images as allowed when the user is enrolled', async function (ctx) {
-      ctx.SplitTestHandler.promises.getAssignmentForUser.mockResolvedValue({
-        variant: 'enabled',
-      })
-      const images = await ctx.ProjectHelper.getAllowedImagesForUser(ctx.user)
+      ctx.SplitTestHandler.promises.featureFlagEnabled.mockResolvedValue(true)
+      const images = await ctx.ProjectHelper.getAllowedImagesForUser(
+        ctx.req,
+        ctx.res,
+        ctx.user
+      )
       const imageNames = _mapToAllowed(images)
       expect(imageNames).to.deep.equal([
         { imageName: 'texlive-full:2018.1', allowed: true },
@@ -195,7 +207,11 @@ describe('ProjectHelper', function () {
     })
 
     it('marks alpha only images as not allowed when when the user is not admin', async function (ctx) {
-      const images = await ctx.ProjectHelper.getAllowedImagesForUser(ctx.user)
+      const images = await ctx.ProjectHelper.getAllowedImagesForUser(
+        ctx.req,
+        ctx.res,
+        ctx.user
+      )
       const imageNames = _mapToAllowed(images)
       expect(imageNames).to.deep.equal([
         { imageName: 'texlive-full:2018.1', allowed: true },
@@ -207,6 +223,8 @@ describe('ProjectHelper', function () {
 
     it('returns all images when the user is admin', async function (ctx) {
       const images = await ctx.ProjectHelper.getAllowedImagesForUser(
+        ctx.req,
+        ctx.res,
         ctx.adminUser
       )
       const imageNames = _mapToAllowed(images)

+ 1 - 1
services/web/test/unit/src/Project/ProjectListController.test.mjs

@@ -137,7 +137,7 @@ describe('ProjectListController', function () {
     ctx.SplitTestHandler = {
       promises: {
         getAssignment: sinon.stub().resolves({ variant: 'default' }),
-        featureFlagEnabledForUser: sinon.stub().resolves(false),
+        featureFlagEnabled: sinon.stub().resolves(false),
         hasUserBeenAssignedToVariant: sinon.stub().resolves(false),
       },
     }

+ 123 - 1
services/web/test/unit/src/SplitTests/SplitTestHandler.test.mjs

@@ -139,6 +139,7 @@ describe('SplitTestHandler', function () {
     beforeEach(async function (ctx) {
       ctx.user = {
         _id: new ObjectId(),
+        analyticsId: 'analytics-id',
         splitTests: {
           'active-test': [
             {
@@ -256,9 +257,101 @@ describe('SplitTestHandler', function () {
     })
   })
 
+  describe('mongo user variants', function () {
+    beforeEach(function (ctx) {
+      ctx.mongoUser = {
+        _id: new ObjectId(),
+        analyticsId: 'analytics-id',
+        splitTests: {},
+      }
+      // a feature flag whose enabled variant covers 100% of users
+      ctx.cachedSplitTests.set('my-flag', {
+        name: 'my-flag',
+        versions: [
+          {
+            active: true,
+            analyticsEnabled: true,
+            phase: 'release',
+            versionNumber: 1,
+            variants: [
+              {
+                name: 'enabled',
+                rolloutPercent: 100,
+                rolloutStripes: [{ start: 0, end: 100 }],
+              },
+            ],
+          },
+        ],
+      })
+    })
+
+    describe('getActiveAssignmentsForMongoUser', function () {
+      it('returns the assignments without re-fetching the user', async function (ctx) {
+        const assignments =
+          await ctx.SplitTestHandler.promises.getActiveAssignmentsForMongoUser(
+            ctx.mongoUser
+          )
+        expect(assignments['active-test']).to.deep.equal({
+          variantName: 'variant-1',
+          phase: 'release',
+          versionNumber: 2,
+        })
+        expect(ctx.SplitTestUserGetter.promises.getUser).to.not.have.been.called
+      })
+
+      it('throws when _id is missing from the projection', async function (ctx) {
+        await expect(
+          ctx.SplitTestHandler.promises.getActiveAssignmentsForMongoUser({
+            analyticsId: 'analytics-id',
+          })
+        ).to.be.rejectedWith('bug: include db.users._id in projection')
+      })
+
+      it('throws when analyticsId is missing from the projection', async function (ctx) {
+        await expect(
+          ctx.SplitTestHandler.promises.getActiveAssignmentsForMongoUser({
+            _id: new ObjectId(),
+          })
+        ).to.be.rejectedWith('bug: include db.users.analyticsId in projection')
+      })
+    })
+
+    describe('getAssignmentForMongoUser', function () {
+      it('returns the assignment without re-fetching the user', async function (ctx) {
+        const assignment =
+          await ctx.SplitTestHandler.promises.getAssignmentForMongoUser(
+            ctx.mongoUser,
+            'my-flag'
+          )
+        expect(assignment.variant).to.equal('enabled')
+        expect(ctx.SplitTestUserGetter.promises.getUser).to.not.have.been.called
+      })
+    })
+
+    describe('featureFlagEnabledForMongoUser', function () {
+      it('returns true when the assigned variant is enabled', async function (ctx) {
+        const enabled =
+          await ctx.SplitTestHandler.promises.featureFlagEnabledForMongoUser(
+            ctx.mongoUser,
+            'my-flag'
+          )
+        expect(enabled).to.be.true
+      })
+
+      it('returns false when the assigned variant is not enabled', async function (ctx) {
+        const enabled =
+          await ctx.SplitTestHandler.promises.featureFlagEnabledForMongoUser(
+            ctx.mongoUser,
+            'active-test'
+          )
+        expect(enabled).to.be.false
+      })
+    })
+  })
+
   describe('with a user without assignments', function () {
     beforeEach(async function (ctx) {
-      ctx.user = { _id: new ObjectId() }
+      ctx.user = { _id: new ObjectId(), analyticsId: 'analytics-id' }
       ctx.SplitTestUserGetter.promises.getUser.resolves(ctx.user)
       ctx.assignments =
         await ctx.SplitTestHandler.promises.getActiveAssignmentsForUser(
@@ -421,6 +514,35 @@ describe('SplitTestHandler', function () {
     })
   })
 
+  describe('getAssignment query overrides', function () {
+    beforeEach(function (ctx) {
+      ctx.AnalyticsManager.getIdsFromSession.returns({
+        userId: 'abc123abc123',
+      })
+      // 'active-test' would compute to 'variant-1'; override it to 'default'
+      ctx.req.query = { 'active-test': 'default' }
+    })
+
+    it('applies a query-string override by default', async function (ctx) {
+      const { variant } = await ctx.SplitTestHandler.promises.getAssignment(
+        ctx.req,
+        ctx.res,
+        'active-test'
+      )
+      expect(variant).to.equal('default')
+    })
+
+    it('ignores query-string overrides when ignoreOverrides is set', async function (ctx) {
+      const { variant } = await ctx.SplitTestHandler.promises.getAssignment(
+        ctx.req,
+        ctx.res,
+        'active-test',
+        { ignoreOverrides: true }
+      )
+      expect(variant).to.equal('variant-1')
+    })
+  })
+
   describe('variant user limits', function () {
     beforeEach(function (ctx) {
       ctx.AnalyticsManager.getIdsFromSession.returns({

+ 1 - 1
services/web/test/unit/src/Subscription/FeaturesUpdater.test.mjs

@@ -163,7 +163,7 @@ describe('FeaturesUpdater', function () {
 
     ctx.SplitTestHandler = {
       promises: {
-        featureFlagEnabledForUser: sinon.stub().resolves(false),
+        featureFlagEnabledForMongoUser: sinon.stub().resolves(false),
       },
     }
 

+ 1 - 1
services/web/test/unit/src/infrastructure/AiFeatureUsageRateLimiter.test.mjs

@@ -78,7 +78,7 @@ describe('AiFeatureUsageRateLimiter', function () {
 
     ctx.SplitTestHandler = {
       promises: {
-        featureFlagEnabledForUser: sinon.stub().resolves(true),
+        featureFlagEnabledForMongoUser: sinon.stub().resolves(true),
       },
     }
 

+ 1 - 1
services/web/test/unit/src/infrastructure/WorkbenchRateLimiter.sequential.test.mjs

@@ -62,7 +62,7 @@ describe('WorkbenchRateLimiter', function () {
     ctx.SplitTestHandler = {
       promises: {
         getAssignmentForUser: sinon.stub(),
-        featureFlagEnabledForUser: sinon.stub().resolves(true),
+        featureFlagEnabledForMongoUser: sinon.stub().resolves(true),
       },
     }
     ctx.SplitTestHandler.promises.getAssignmentForUser