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

Merge pull request #17907 from overleaf/ab-split-test-assignments-optim-pt1

[web] Read anonymous split test assignments in session from both old&new fields

GitOrigin-RevId: 5235bb3e7d72d5ff9e89c6543b70fb80e9f1213c
Alexandre Bourdin 2 лет назад
Родитель
Сommit
bee4c95c28

+ 3 - 3
services/web/app/src/Features/BetaProgram/BetaProgramController.js

@@ -3,14 +3,14 @@ const OError = require('@overleaf/o-error')
 const UserGetter = require('../User/UserGetter')
 const logger = require('@overleaf/logger')
 const SessionManager = require('../Authentication/SessionManager')
-const SplitTestHandler = require('../SplitTests/SplitTestHandler')
+const SplitTestSessionHandler = require('../SplitTests/SplitTestSessionHandler')
 const { expressify } = require('@overleaf/promise-utils')
 
 async function optIn(req, res) {
   const userId = SessionManager.getLoggedInUserId(req.session)
   await BetaProgramHandler.promises.optIn(userId)
   try {
-    await SplitTestHandler.promises.sessionMaintenance(req, null)
+    await SplitTestSessionHandler.promises.sessionMaintenance(req, null)
   } catch (error) {
     logger.error(
       { err: error },
@@ -24,7 +24,7 @@ async function optOut(req, res) {
   const userId = SessionManager.getLoggedInUserId(req.session)
   await BetaProgramHandler.promises.optOut(userId)
   try {
-    await SplitTestHandler.promises.sessionMaintenance(req, null)
+    await SplitTestSessionHandler.promises.sessionMaintenance(req, null)
   } catch (error) {
     logger.error(
       { err: error },

+ 3 - 2
services/web/app/src/Features/Project/ProjectController.js

@@ -31,6 +31,7 @@ const BrandVariationsHandler = require('../BrandVariations/BrandVariationsHandle
 const UserController = require('../User/UserController')
 const AnalyticsManager = require('../Analytics/AnalyticsManager')
 const SplitTestHandler = require('../SplitTests/SplitTestHandler')
+const SplitTestSessionHandler = require('../SplitTests/SplitTestSessionHandler')
 const FeaturesUpdater = require('../Subscription/FeaturesUpdater')
 const SpellingHandler = require('../Spelling/SpellingHandler')
 const { hasAdminAccess } = require('../Helpers/AdminAuthorizationHelper')
@@ -431,7 +432,7 @@ const ProjectController = {
         },
         user(cb) {
           if (userId == null) {
-            SplitTestHandler.sessionMaintenance(req, null, () => {})
+            SplitTestSessionHandler.sessionMaintenance(req, null, () => {})
             cb(null, defaultSettingsForAnonymousUser(userId))
           } else {
             User.updateOne(
@@ -453,7 +454,7 @@ const ProjectController = {
                   return cb(err)
                 }
                 logger.debug({ projectId, userId }, 'got user')
-                SplitTestHandler.sessionMaintenance(req, user, () => {})
+                SplitTestSessionHandler.sessionMaintenance(req, user, () => {})
                 if (FeaturesUpdater.featuresEpochIsCurrent(user)) {
                   return cb(null, user)
                 }

+ 2 - 1
services/web/app/src/Features/Project/ProjectListController.js

@@ -24,6 +24,7 @@ const LimitationsManager = require('../Subscription/LimitationsManager')
 const NotificationsBuilder = require('../Notifications/NotificationsBuilder')
 const GeoIpLookup = require('../../infrastructure/GeoIpLookup')
 const SplitTestHandler = require('../SplitTests/SplitTestHandler')
+const SplitTestSessionHandler = require('../SplitTests/SplitTestSessionHandler')
 const SubscriptionLocator = require('../Subscription/SubscriptionLocator')
 
 /** @typedef {import("./types").GetProjectsRequest} GetProjectsRequest */
@@ -115,7 +116,7 @@ async function projectListPage(req, res, next) {
   }
 
   if (isSaas) {
-    await SplitTestHandler.promises.sessionMaintenance(req, user)
+    await SplitTestSessionHandler.promises.sessionMaintenance(req, user)
 
     try {
       usersBestSubscription =

+ 63 - 178
services/web/app/src/Features/SplitTests/SplitTestHandler.js

@@ -1,5 +1,4 @@
 const Metrics = require('@overleaf/metrics')
-const UserGetter = require('../User/UserGetter')
 const UserUpdater = require('../User/UserUpdater')
 const AnalyticsManager = require('../Analytics/AnalyticsManager')
 const LocalsHelper = require('./LocalsHelper')
@@ -14,11 +13,12 @@ const SplitTestUtils = require('./SplitTestUtils')
 const Settings = require('@overleaf/settings')
 const SessionManager = require('../Authentication/SessionManager')
 const logger = require('@overleaf/logger')
+const SplitTestSessionHandler = require('./SplitTestSessionHandler')
+const SplitTestUserGetter = require('./SplitTestUserGetter')
 
 const DEFAULT_VARIANT = 'default'
 const ALPHA_PHASE = 'alpha'
 const BETA_PHASE = 'beta'
-const CACHE_TOMBSTONE_SPLIT_TEST_NOT_ACTIVE_FOR_USER = null
 const DEFAULT_ASSIGNMENT = {
   variant: DEFAULT_VARIANT,
   analytics: {
@@ -85,7 +85,7 @@ async function getAssignment(req, res, splitTestName, { sync = false } = {}) {
           session: req.session,
           sync,
         })
-        _collectSessionStats(req.session)
+        SplitTestSessionHandler.collectSessionStats(req.session)
       }
     }
   } catch (error) {
@@ -175,7 +175,7 @@ async function getActiveAssignmentsForUser(userId, removeArchived = false) {
     return {}
   }
 
-  const user = await _getUser(userId)
+  const user = await SplitTestUserGetter.promises.getUser(userId)
   if (user == null) {
     return {}
   }
@@ -209,34 +209,6 @@ async function getActiveAssignmentsForUser(userId, removeArchived = false) {
   return assignments
 }
 
-/**
- * @param {import('express').Request} req
- * @param {Object|null} user optional, prefetched user with alphaProgram and betaProgram field
- * @return {Promise<void>}
- */
-async function sessionMaintenance(req, user) {
-  const session = req.session
-  const sessionUser = SessionManager.getSessionUser(session)
-
-  Metrics.inc('split_test_session_maintenance', 1, { status: 'start' })
-  if (sessionUser) {
-    user = user || (await _getUser(sessionUser._id))
-    if (
-      Boolean(sessionUser.alphaProgram) !== Boolean(user.alphaProgram) ||
-      Boolean(sessionUser.betaProgram) !== Boolean(user.betaProgram)
-    ) {
-      Metrics.inc('split_test_session_maintenance', 1, {
-        status: 'program-change',
-      })
-      sessionUser.alphaProgram = user.alphaProgram || undefined // only store if set
-      sessionUser.betaProgram = user.betaProgram || undefined // only store if set
-      session.cachedSplitTestAssignments = {}
-    }
-  }
-
-  // TODO: After changing the split test config fetching: remove split test assignments for archived split tests
-}
-
 /**
  * Returns an array of valid variant names for the given split test, including default
  *
@@ -285,18 +257,22 @@ async function _getAssignment(
   }
 
   if (canUseSessionCache) {
-    const cachedVariant = _getCachedVariantFromSession(
+    const cachedVariant = SplitTestSessionHandler.getCachedVariant(
       session,
       splitTest.name,
       currentVersion
     )
-    if (cachedVariant === CACHE_TOMBSTONE_SPLIT_TEST_NOT_ACTIVE_FOR_USER) {
-      Metrics.inc('split_test_get_assignment_source', 1, { status: 'cache' })
-      return DEFAULT_ASSIGNMENT
-    }
+
     if (cachedVariant) {
       Metrics.inc('split_test_get_assignment_source', 1, { status: 'cache' })
-      return _makeAssignment(splitTest, cachedVariant, currentVersion)
+      if (
+        cachedVariant ===
+        SplitTestSessionHandler.CACHE_TOMBSTONE_SPLIT_TEST_NOT_ACTIVE_FOR_USER
+      ) {
+        return DEFAULT_ASSIGNMENT
+      } else {
+        return _makeAssignment(splitTest, cachedVariant, currentVersion)
+      }
     }
   }
 
@@ -308,11 +284,14 @@ async function _getAssignment(
     Metrics.inc('split_test_get_assignment_source', 1, { status: 'none' })
   }
 
-  user = user || (userId && (await _getUser(userId, splitTestName)))
+  user =
+    user ||
+    (userId &&
+      (await SplitTestUserGetter.promises.getUser(userId, splitTestName)))
   const { activeForUser, selectedVariantName, phase, versionNumber } =
     await _getAssignmentMetadata(analyticsId, user, splitTest)
   if (canUseSessionCache) {
-    _setVariantInSession({
+    SplitTestSessionHandler.setVariantInCache({
       session,
       splitTestName,
       currentVersion,
@@ -321,22 +300,40 @@ async function _getAssignment(
     })
   }
   if (activeForUser) {
-    const assignmentConfig = {
-      user,
-      userId,
-      analyticsId,
-      session,
-      splitTestName,
-      variantName: selectedVariantName,
-      phase,
-      versionNumber,
-    }
     if (currentVersion.analyticsEnabled) {
-      if (sync === true) {
-        await _updateVariantAssignment(assignmentConfig)
-      } else {
-        _updateVariantAssignment(assignmentConfig)
+      // if the user is logged in, persist the assignment
+      if (userId) {
+        const assignmentData = {
+          user,
+          userId,
+          splitTestName,
+          phase,
+          versionNumber,
+          variantName: selectedVariantName,
+        }
+        if (sync === true) {
+          await _recordAssignment(assignmentData)
+        } else {
+          _recordAssignment(assignmentData)
+        }
       }
+      // otherwise this is an anonymous user, we store assignments in session to persist them on registration
+      else {
+        await SplitTestSessionHandler.promises.appendAssignment(session, {
+          splitTestId: splitTest._id,
+          splitTestName,
+          phase,
+          versionNumber,
+          variantName: selectedVariantName,
+          assignedAt: new Date(),
+        })
+      }
+
+      AnalyticsManager.setUserPropertyForAnalyticsId(
+        user?.analyticsId || analyticsId || userId,
+        `split-test-${splitTestName}-${versionNumber}`,
+        selectedVariantName
+      )
     }
     return _makeAssignment(splitTest, selectedVariantName, currentVersion)
   }
@@ -404,11 +401,9 @@ function _getVariantFromPercentile(variants, percentile) {
   }
 }
 
-async function _updateVariantAssignment({
+async function _recordAssignment({
   user,
   userId,
-  analyticsId,
-  session,
   splitTestName,
   phase,
   versionNumber,
@@ -420,45 +415,18 @@ async function _updateVariantAssignment({
     phase,
     assignedAt: new Date(),
   }
-  // if the user is logged in
-  if (userId) {
-    user = user || (await _getUser(userId, splitTestName))
-    if (user) {
-      const assignedSplitTests = user.splitTests || []
-      const assignmentLog = assignedSplitTests[splitTestName] || []
-      const existingAssignment = _.find(assignmentLog, { versionNumber })
-      if (!existingAssignment) {
-        await UserUpdater.promises.updateUser(userId, {
-          $addToSet: {
-            [`splitTests.${splitTestName}`]: persistedAssignment,
-          },
-        })
-        AnalyticsManager.setUserPropertyForAnalyticsId(
-          user.analyticsId || analyticsId || userId,
-          `split-test-${splitTestName}-${versionNumber}`,
-          variantName
-        )
-      }
-    }
-  }
-  // otherwise this is an anonymous user, we store assignments in session to persist them on registration
-  else if (session) {
-    if (!session.splitTests) {
-      session.splitTests = {}
-    }
-    if (!session.splitTests[splitTestName]) {
-      session.splitTests[splitTestName] = []
-    }
-    const existingAssignment = _.find(session.splitTests[splitTestName], {
-      versionNumber,
-    })
+  user =
+    user || (await SplitTestUserGetter.promises.getUser(userId, splitTestName))
+  if (user) {
+    const assignedSplitTests = user.splitTests || []
+    const assignmentLog = assignedSplitTests[splitTestName] || []
+    const existingAssignment = _.find(assignmentLog, { versionNumber })
     if (!existingAssignment) {
-      session.splitTests[splitTestName].push(persistedAssignment)
-      AnalyticsManager.setUserPropertyForAnalyticsId(
-        analyticsId,
-        `split-test-${splitTestName}-${versionNumber}`,
-        variantName
-      )
+      await UserUpdater.promises.updateUser(userId, {
+        $addToSet: {
+          [`splitTests.${splitTestName}`]: persistedAssignment,
+        },
+      })
     }
   }
 }
@@ -479,63 +447,6 @@ function _makeAssignment(splitTest, variant, currentVersion) {
   }
 }
 
-function _getCachedVariantFromSession(session, splitTestName, currentVersion) {
-  if (!session.cachedSplitTestAssignments) {
-    session.cachedSplitTestAssignments = {}
-  }
-  const cacheKey = `${splitTestName}-${currentVersion.versionNumber}`
-  return session.cachedSplitTestAssignments[cacheKey]
-}
-
-function _setVariantInSession({
-  session,
-  splitTestName,
-  currentVersion,
-  selectedVariantName,
-  activeForUser,
-}) {
-  if (!session.cachedSplitTestAssignments) {
-    session.cachedSplitTestAssignments = {}
-  }
-
-  // clean up previous entries from this split test
-  for (const cacheKey of Object.keys(session.cachedSplitTestAssignments)) {
-    // drop '-versionNumber'
-    const name = cacheKey.split('-').slice(0, -1).join('-')
-    if (name === splitTestName) {
-      delete session.cachedSplitTestAssignments[cacheKey]
-    }
-  }
-
-  const cacheKey = `${splitTestName}-${currentVersion.versionNumber}`
-  if (activeForUser) {
-    session.cachedSplitTestAssignments[cacheKey] = selectedVariantName
-  } else {
-    session.cachedSplitTestAssignments[cacheKey] =
-      CACHE_TOMBSTONE_SPLIT_TEST_NOT_ACTIVE_FOR_USER
-  }
-}
-
-async function _getUser(id, splitTestName) {
-  const projection = {
-    analyticsId: 1,
-    alphaProgram: 1,
-    betaProgram: 1,
-  }
-  if (splitTestName) {
-    projection[`splitTests.${splitTestName}`] = 1
-  } else {
-    projection.splitTests = 1
-  }
-  const user = await UserGetter.promises.getUser(id, projection)
-  Metrics.histogram(
-    'split_test_get_user_from_mongo_size',
-    JSON.stringify(user).length,
-    [0, 100, 500, 1000, 2000, 5000, 10000, 15000, 20000, 50000, 100000]
-  )
-  return user
-}
-
 async function _loadSplitTestInfoInLocals(locals, splitTestName, session) {
   const splitTest = await _getSplitTest(splitTestName)
   if (splitTest) {
@@ -579,41 +490,16 @@ function _getNonSaasAssignment(splitTestName) {
   return DEFAULT_ASSIGNMENT
 }
 
-function _collectSessionStats(session) {
-  if (session.cachedSplitTestAssignments) {
-    Metrics.summary(
-      'split_test_session_cache_count',
-      Object.keys(session.cachedSplitTestAssignments).length
-    )
-    Metrics.summary(
-      'split_test_session_cache_size',
-      JSON.stringify(session.cachedSplitTestAssignments).length
-    )
-  }
-  if (session.splitTests) {
-    Metrics.summary(
-      'split_test_session_storage_count',
-      Object.keys(session.splitTests).length
-    )
-    Metrics.summary(
-      'split_test_session_storage_size',
-      JSON.stringify(session.splitTests).length
-    )
-  }
-}
-
 async function _getSplitTest(name) {
   const splitTests = await SplitTestCache.get('')
   return splitTests?.get(name)
 }
 
 module.exports = {
-  getPercentile,
   getAssignment: callbackify(getAssignment),
   getAssignmentForMongoUser: callbackify(getAssignmentForMongoUser),
   getAssignmentForUser: callbackify(getAssignmentForUser),
   getActiveAssignmentsForUser: callbackify(getActiveAssignmentsForUser),
-  sessionMaintenance: callbackify(sessionMaintenance),
   setOverrideInSession,
   clearOverridesInSession,
   promises: {
@@ -621,6 +507,5 @@ module.exports = {
     getAssignmentForMongoUser,
     getAssignmentForUser,
     getActiveAssignmentsForUser,
-    sessionMaintenance,
   },
 }

+ 250 - 0
services/web/app/src/Features/SplitTests/SplitTestSessionHandler.js

@@ -0,0 +1,250 @@
+const { callbackify } = require('util')
+const _ = require('lodash')
+const { ObjectId } = require('mongodb')
+const logger = require('@overleaf/logger')
+const Metrics = require('@overleaf/metrics')
+const SessionManager = require('../Authentication/SessionManager')
+const SplitTestCache = require('./SplitTestCache')
+const SplitTestUtils = require('./SplitTestUtils')
+const SplitTestUserGetter = require('./SplitTestUserGetter')
+
+const CACHE_TOMBSTONE_SPLIT_TEST_NOT_ACTIVE_FOR_USER = null
+
+async function getAssignments(session) {
+  if (!session.splitTests && !session.sta) {
+    return undefined
+  }
+
+  // await _convertAnonymousAssignmentsIfNeeded(session)
+  const assignments = _.clone(session.splitTests || {})
+  if (session.sta) {
+    const tokens = session.sta.split(';')
+    const splitTests = Array.from((await SplitTestCache.get('')).values())
+    for (const token of tokens) {
+      try {
+        if (!token.length) {
+          continue
+        }
+        const [splitTestNameVersion, info] = token.split('=')
+        const [splitTestId64, versionStr] = splitTestNameVersion.split('_')
+
+        const splitTest = splitTests.find(
+          test =>
+            test._id.toString() ===
+            new ObjectId(Buffer.from(splitTestId64, 'base64')).toString()
+        )
+        if (!splitTest) {
+          continue
+        }
+
+        const splitTestName = splitTest.name
+        const versionNumber = parseInt(versionStr)
+        const [variantChar, timestampStr36] = info.split(':')
+        const assignedAt = new Date(parseInt(timestampStr36, 36) * 1000)
+        let variantName
+        if (variantChar === 'd') {
+          variantName = 'default'
+        } else {
+          const variantIndex = parseInt(variantChar)
+          variantName =
+            SplitTestUtils.getCurrentVersion(splitTest).variants[variantIndex]
+              .name
+        }
+
+        if (!assignments[splitTestName]) {
+          assignments[splitTestName] = []
+        }
+        assignments[splitTestName].push({
+          versionNumber,
+          variantName,
+          phase: 'release', // anonymous users can only be exposed to tests in release phase
+          assignedAt,
+        })
+      } catch (error) {
+        logger.error(
+          { err: error, token },
+          'Failed to resolve anonymous split test assignment from session'
+        )
+      }
+    }
+  }
+
+  return assignments
+}
+
+async function appendAssignment(session, assignment) {
+  // await _convertAnonymousAssignmentsIfNeeded(session)
+
+  if (!session.splitTests) {
+    session.splitTests = {}
+  }
+  if (!session.splitTests[assignment.splitTestName]) {
+    session.splitTests[assignment.splitTestName] = []
+  }
+
+  const assignments = await getAssignments(session)
+  if (
+    _.find(assignments[assignment.splitTestName], {
+      splitTestName: assignment.splitTestName,
+      versionNumber: assignment.versionNumber,
+    })
+  ) {
+    // if (!session.sta) {
+    //   session.sta = ''
+    // }
+    // const splitTests = await SplitTestCache.get('')
+    // const splitTest = splitTests.get(assignment.splitTestName)
+    // const assignmentString = _buildAssignmentString(splitTest, assignment)
+    // const separator = session.sta.length > 0 ? ';' : ''
+    // session.sta += `${separator}${assignmentString}`
+    session.splitTests[assignment.splitTestName].push(assignment)
+  }
+}
+
+function getCachedVariant(session, splitTestName, currentVersion) {
+  if (!session.cachedSplitTestAssignments) {
+    session.cachedSplitTestAssignments = {}
+  }
+  const cacheKey = `${splitTestName}-${currentVersion.versionNumber}`
+  return session.cachedSplitTestAssignments[cacheKey]
+}
+
+function setVariantInCache({
+  session,
+  splitTestName,
+  currentVersion,
+  selectedVariantName,
+  activeForUser,
+}) {
+  if (!session.cachedSplitTestAssignments) {
+    session.cachedSplitTestAssignments = {}
+  }
+
+  // clean up previous entries from this split test
+  for (const cacheKey of Object.keys(session.cachedSplitTestAssignments)) {
+    // drop '-versionNumber'
+    const name = cacheKey.split('-').slice(0, -1).join('-')
+    if (name === splitTestName) {
+      delete session.cachedSplitTestAssignments[cacheKey]
+    }
+  }
+
+  const cacheKey = `${splitTestName}-${currentVersion.versionNumber}`
+  if (activeForUser) {
+    session.cachedSplitTestAssignments[cacheKey] = selectedVariantName
+  } else {
+    session.cachedSplitTestAssignments[cacheKey] =
+      CACHE_TOMBSTONE_SPLIT_TEST_NOT_ACTIVE_FOR_USER
+  }
+}
+
+/**
+ * @param {import('express').Request} req
+ * @param {Object|null} user optional, prefetched user with alphaProgram and betaProgram field
+ * @return {Promise<void>}
+ */
+async function sessionMaintenance(req, user) {
+  const session = req.session
+  const sessionUser = SessionManager.getSessionUser(session)
+
+  Metrics.inc('split_test_session_maintenance', 1, { status: 'start' })
+  if (sessionUser) {
+    user = user || (await SplitTestUserGetter.promises.getUser(sessionUser._id))
+    if (
+      Boolean(sessionUser.alphaProgram) !== Boolean(user.alphaProgram) ||
+      Boolean(sessionUser.betaProgram) !== Boolean(user.betaProgram)
+    ) {
+      Metrics.inc('split_test_session_maintenance', 1, {
+        status: 'program-change',
+      })
+      sessionUser.alphaProgram = user.alphaProgram || undefined // only store if set
+      sessionUser.betaProgram = user.betaProgram || undefined // only store if set
+      session.cachedSplitTestAssignments = {}
+    }
+  }
+
+  // TODO: After changing the split test config fetching: remove split test assignments for archived split tests
+}
+
+function collectSessionStats(session) {
+  if (session.cachedSplitTestAssignments) {
+    Metrics.summary(
+      'split_test_session_cache_count',
+      Object.keys(session.cachedSplitTestAssignments).length
+    )
+    Metrics.summary(
+      'split_test_session_cache_size',
+      JSON.stringify(session.cachedSplitTestAssignments).length
+    )
+  }
+  if (session.splitTests) {
+    Metrics.summary(
+      'split_test_session_storage_count',
+      (session.sta || '').split(';').length +
+        Object.keys(session.splitTests).length
+    )
+    Metrics.summary(
+      'split_test_session_storage_size',
+      (session.sta || '').length + JSON.stringify(session.splitTests).length
+    )
+  }
+}
+
+// async function _convertAnonymousAssignmentsIfNeeded(session) {
+//   if (typeof session.splitTests === 'object') {
+//     const sessionAssignments = session.splitTests
+//     const splitTests = await SplitTestCache.get('')
+//     session.splitTests = ''
+//     for (const [splitTestName, assignments] of Object.entries(
+//       sessionAssignments
+//     )) {
+//       const splitTest = splitTests.get(splitTestName)
+//       for (const assignment of assignments) {
+//         const assignmentString = _buildAssignmentString(splitTest, assignment)
+//         const separator = session.splitTests.length > 0 ? ';' : ''
+//         session.splitTests += `${separator}${assignmentString}`
+//       }
+//     }
+//   }
+// }
+
+// function _hasExistingAssignment(session, splitTest, versionNumber) {
+//   if (!session.sta) {
+//     return false
+//   }
+//   const index = session.sta.indexOf(
+//     `${_convertIdToBase64(splitTest._id)}_${versionNumber}=`
+//   )
+//   return index >= 0
+// }
+
+// function _buildAssignmentString(splitTest, assignment) {
+//   const { versionNumber, variantName, assignedAt } = assignment
+//   const variants = SplitTestUtils.getCurrentVersion(splitTest).variants
+//   const splitTestId = _convertIdToBase64(splitTest._id)
+//   const variantChar =
+//     variantName === 'default'
+//       ? 'd'
+//       : _.findIndex(variants, { name: variantName })
+//   const timestamp = Math.floor(assignedAt.getTime() / 1000).toString(36)
+//   return `${splitTestId}_${versionNumber}=${variantChar}:${timestamp}`
+// }
+
+// function _convertIdToBase64(id) {
+//   return new ObjectId(id).toString('base64')
+// }
+
+module.exports = {
+  getAssignments: callbackify(getAssignments),
+  appendAssignment: callbackify(appendAssignment),
+  getCachedVariant,
+  setVariantInCache,
+  sessionMaintenance: callbackify(sessionMaintenance),
+  collectSessionStats,
+  CACHE_TOMBSTONE_SPLIT_TEST_NOT_ACTIVE_FOR_USER,
+  promises: {
+    getAssignments,
+    appendAssignment,
+    sessionMaintenance,
+  },
+}

+ 30 - 0
services/web/app/src/Features/SplitTests/SplitTestUserGetter.js

@@ -0,0 +1,30 @@
+const { callbackify } = require('util')
+const Metrics = require('@overleaf/metrics')
+const UserGetter = require('../User/UserGetter')
+
+async function getUser(id, splitTestName) {
+  const projection = {
+    analyticsId: 1,
+    alphaProgram: 1,
+    betaProgram: 1,
+  }
+  if (splitTestName) {
+    projection[`splitTests.${splitTestName}`] = 1
+  } else {
+    projection.splitTests = 1
+  }
+  const user = await UserGetter.promises.getUser(id, projection)
+  Metrics.histogram(
+    'split_test_get_user_from_mongo_size',
+    JSON.stringify(user).length,
+    [0, 100, 500, 1000, 2000, 5000, 10000, 15000, 20000, 50000, 100000]
+  )
+  return user
+}
+
+module.exports = {
+  getUser: callbackify(getUser),
+  promises: {
+    getUser,
+  },
+}

+ 4 - 4
services/web/test/unit/src/BetaProgram/BetaProgramControllerTests.js

@@ -23,14 +23,14 @@ describe('BetaProgramController', function () {
         user: this.user,
       },
     }
-    this.SplitTestHandler = {
+    this.SplitTestSessionHandler = {
       promises: {
         sessionMaintenance: sinon.stub(),
       },
     }
     this.BetaProgramController = SandboxedModule.require(modulePath, {
       requires: {
-        '../SplitTests/SplitTestHandler': this.SplitTestHandler,
+        '../SplitTests/SplitTestSessionHandler': this.SplitTestSessionHandler,
         './BetaProgramHandler': (this.BetaProgramHandler = {
           promises: {
             optIn: sinon.stub().resolves(),
@@ -76,7 +76,7 @@ describe('BetaProgramController', function () {
 
     it('should invoke the session maintenance', function (done) {
       this.res.callback = () => {
-        this.SplitTestHandler.promises.sessionMaintenance.should.have.been.calledWith(
+        this.SplitTestSessionHandler.promises.sessionMaintenance.should.have.been.calledWith(
           this.req
         )
         done()
@@ -130,7 +130,7 @@ describe('BetaProgramController', function () {
 
     it('should invoke the session maintenance', function (done) {
       this.res.callback = () => {
-        this.SplitTestHandler.promises.sessionMaintenance.should.have.been.calledWith(
+        this.SplitTestSessionHandler.promises.sessionMaintenance.should.have.been.calledWith(
           this.req,
           null
         )

+ 5 - 2
services/web/test/unit/src/Project/ProjectControllerTests.js

@@ -136,6 +136,8 @@ describe('ProjectController', function () {
         getAssignment: sinon.stub().resolves({ variant: 'default' }),
       },
       getAssignment: sinon.stub().yields(null, { variant: 'default' }),
+    }
+    this.SplitTestSessionHandler = {
       sessionMaintenance: sinon.stub().yields(),
     }
     this.InstitutionsFeatures = {
@@ -160,6 +162,7 @@ describe('ProjectController', function () {
         '@overleaf/settings': this.settings,
         '@overleaf/metrics': this.Metrics,
         '../SplitTests/SplitTestHandler': this.SplitTestHandler,
+        '../SplitTests/SplitTestSessionHandler': this.SplitTestSessionHandler,
         './ProjectDeleter': this.ProjectDeleter,
         './ProjectDuplicator': this.ProjectDuplicator,
         './ProjectCreationHandler': this.ProjectCreationHandler,
@@ -536,7 +539,7 @@ describe('ProjectController', function () {
 
     it('should invoke the session maintenance for logged in user', function (done) {
       this.res.render = () => {
-        this.SplitTestHandler.sessionMaintenance.should.have.been.calledWith(
+        this.SplitTestSessionHandler.sessionMaintenance.should.have.been.calledWith(
           this.req,
           this.user
         )
@@ -548,7 +551,7 @@ describe('ProjectController', function () {
     it('should invoke the session maintenance for anonymous user', function (done) {
       this.SessionManager.getLoggedInUserId.returns(null)
       this.res.render = () => {
-        this.SplitTestHandler.sessionMaintenance.should.have.been.calledWith(
+        this.SplitTestSessionHandler.sessionMaintenance.should.have.been.calledWith(
           this.req
         )
         done()

+ 7 - 2
services/web/test/unit/src/Project/ProjectListControllerTests.js

@@ -102,10 +102,14 @@ describe('ProjectListController', function () {
     }
     this.SplitTestHandler = {
       promises: {
-        sessionMaintenance: sinon.stub().resolves(),
         getAssignment: sinon.stub().resolves({ variant: 'default' }),
       },
     }
+    this.SplitTestSessionHandler = {
+      promises: {
+        sessionMaintenance: sinon.stub().resolves(),
+      },
+    }
     this.SubscriptionViewModelBuilder = {
       promises: {
         getBestSubscription: sinon.stub().resolves({ type: 'free' }),
@@ -141,6 +145,7 @@ describe('ProjectListController', function () {
         '@overleaf/settings': this.settings,
         '@overleaf/metrics': this.Metrics,
         '../SplitTests/SplitTestHandler': this.SplitTestHandler,
+        '../SplitTests/SplitTestSessionHandler': this.SplitTestSessionHandler,
         '../User/UserController': this.UserController,
         './ProjectHelper': this.ProjectHelper,
         '../Subscription/LimitationsManager': this.LimitationsManager,
@@ -223,7 +228,7 @@ describe('ProjectListController', function () {
     it('should invoke the session maintenance', function (done) {
       this.Features.hasFeature.withArgs('saas').returns(true)
       this.res.render = () => {
-        this.SplitTestHandler.promises.sessionMaintenance.should.have.been.calledWith(
+        this.SplitTestSessionHandler.promises.sessionMaintenance.should.have.been.calledWith(
           this.req,
           this.user
         )

+ 13 - 12
services/web/test/unit/src/SplitTests/SplitTestHandlerTests.js

@@ -27,12 +27,6 @@ describe('SplitTestHandler', function () {
       this.cachedSplitTests.set(splitTest.name, splitTest)
     }
 
-    this.UserGetter = {
-      promises: {
-        getUser: sinon.stub().resolves(null),
-      },
-    }
-
     this.SplitTest = {
       find: sinon.stub().returns({
         exec: sinon.stub().resolves(this.splitTests),
@@ -54,11 +48,20 @@ describe('SplitTestHandler', function () {
     }
     this.AnalyticsManager = {
       getIdsFromSession: sinon.stub(),
+      setUserPropertyForAnalyticsId: sinon.stub(),
     }
     this.LocalsHelper = {
       setSplitTestVariant: sinon.stub(),
       setSplitTestInfo: sinon.stub(),
     }
+    this.SplitTestSessionHandler = {
+      collectSessionStats: sinon.stub(),
+    }
+    this.SplitTestUserGetter = {
+      promises: {
+        getUser: sinon.stub().resolves(null),
+      },
+    }
 
     this.SplitTestHandler = SandboxedModule.require(MODULE_PATH, {
       requires: {
@@ -68,6 +71,8 @@ describe('SplitTestHandler', function () {
         '../User/UserUpdater': {},
         '../Analytics/AnalyticsManager': this.AnalyticsManager,
         './LocalsHelper': this.LocalsHelper,
+        './SplitTestSessionHandler': this.SplitTestSessionHandler,
+        './SplitTestUserGetter': this.SplitTestUserGetter,
         '@overleaf/settings': this.Settings,
       },
     })
@@ -100,9 +105,7 @@ describe('SplitTestHandler', function () {
           ],
         },
       }
-      this.UserGetter.promises.getUser
-        .withArgs(this.user._id)
-        .resolves(this.user)
+      this.SplitTestUserGetter.promises.getUser.resolves(this.user)
       this.assignments =
         await this.SplitTestHandler.promises.getActiveAssignmentsForUser(
           this.user._id
@@ -164,9 +167,7 @@ describe('SplitTestHandler', function () {
   describe('with a user without assignments', function () {
     beforeEach(async function () {
       this.user = { _id: new ObjectId() }
-      this.UserGetter.promises.getUser
-        .withArgs(this.user._id)
-        .resolves(this.user)
+      this.SplitTestUserGetter.promises.getUser.resolves(this.user)
       this.assignments =
         await this.SplitTestHandler.promises.getActiveAssignmentsForUser(
           this.user._id

+ 241 - 0
services/web/test/unit/src/SplitTests/SplitTestSessionHandlerTests.js

@@ -0,0 +1,241 @@
+const Path = require('path')
+const SandboxedModule = require('sandboxed-module')
+const sinon = require('sinon')
+const { expect } = require('chai')
+
+const MODULE_PATH = Path.join(
+  __dirname,
+  '../../../../app/src/Features/SplitTests/SplitTestSessionHandler'
+)
+
+describe('SplitTestSessionHandler', function () {
+  beforeEach(function () {
+    this.SplitTestCache = {
+      get: sinon.stub().resolves(),
+    }
+    this.SplitTestUserGetter = {}
+    this.Metrics = {}
+    this.SplitTestSessionHandler = SandboxedModule.require(MODULE_PATH, {
+      requires: {
+        './SplitTestCache': this.SplitTestCache,
+        './SplitTestUserGetter': this.SplitTestUserGetter,
+        '@overleaf/metrics': this.Metrics,
+      },
+    })
+  })
+
+  it('should read from the splitTests field', async function () {
+    const session = {
+      splitTests: {
+        'anon-test-1': [
+          {
+            variantName: 'default',
+            versionNumber: 1,
+            phase: 'release',
+            assignedAt: new Date(1712872800000), // 2024-04-11 22:00:00
+          },
+        ],
+        'anon-test-2': [
+          {
+            variantName: 'default',
+            versionNumber: 1,
+            phase: 'release',
+            assignedAt: new Date(1712307600000), // 2024-04-05 09:00:00
+          },
+          {
+            variantName: 'v-2',
+            versionNumber: 2,
+            phase: 'release',
+            assignedAt: new Date(1712581200000), // 2024-04-08 13:00:00
+          },
+        ],
+      },
+      sta: ``,
+    }
+
+    const assignments =
+      await this.SplitTestSessionHandler.promises.getAssignments(session)
+    expect(assignments).to.deep.equal({
+      'anon-test-1': [
+        {
+          variantName: 'default',
+          versionNumber: 1,
+          phase: 'release',
+          assignedAt: new Date(1712872800000),
+        },
+      ],
+      'anon-test-2': [
+        {
+          variantName: 'default',
+          versionNumber: 1,
+          phase: 'release',
+          assignedAt: new Date(1712307600000),
+        },
+        {
+          variantName: 'v-2',
+          versionNumber: 2,
+          phase: 'release',
+          assignedAt: new Date(1712581200000),
+        },
+      ],
+    })
+  })
+
+  it('should not read from the sta field', async function () {
+    this.SplitTestCache.get = sinon.stub().resolves(
+      new Map(
+        Object.entries({
+          'anon-test-1': {
+            _id: '661f92a4669764bb03f73e37',
+            name: 'anon-test-1',
+            versions: [
+              {
+                versionNumber: 1,
+                variants: [
+                  {
+                    name: 'enabled',
+                  },
+                ],
+              },
+            ],
+          },
+          'anon-test-2': {
+            _id: '661f92a9d68ea711d6bf2df4',
+            name: 'anon-test-2',
+            versions: [
+              {
+                versionNumber: 1,
+                variants: [
+                  {
+                    name: 'v-1',
+                  },
+                  {
+                    name: 'v-2',
+                  },
+                ],
+              },
+            ],
+          },
+        })
+      )
+    )
+    const session = {
+      sta: `Zh+SpGaXZLsD9z43_1=d:sbrvs0;Zh+SqdaOpxHWvy30_1=d:sbtqg0;Zh+SqdaOpxHWvy30_2=1:sbsi00`,
+    }
+
+    const assignments =
+      await this.SplitTestSessionHandler.promises.getAssignments(session)
+    expect(assignments).to.deep.equal({
+      'anon-test-1': [
+        {
+          variantName: 'default',
+          versionNumber: 1,
+          phase: 'release',
+          assignedAt: new Date(1712829600000),
+        },
+      ],
+      'anon-test-2': [
+        {
+          variantName: 'default',
+          versionNumber: 1,
+          phase: 'release',
+          assignedAt: new Date(1712916000000),
+        },
+        {
+          variantName: 'v-2',
+          versionNumber: 2,
+          phase: 'release',
+          assignedAt: new Date(1712858400000),
+        },
+      ],
+    })
+  })
+
+  it('should merge assignments from both splitTests and sta fields', async function () {
+    this.SplitTestCache.get = sinon.stub().resolves(
+      new Map(
+        Object.entries({
+          'anon-test-1': {
+            _id: '661f92a4669764bb03f73e37',
+            name: 'anon-test-1',
+            versions: [
+              {
+                versionNumber: 1,
+                variants: [
+                  {
+                    name: 'enabled',
+                  },
+                ],
+              },
+            ],
+          },
+          'anon-test-2': {
+            _id: '661f92a9d68ea711d6bf2df4',
+            name: 'anon-test-2',
+            versions: [
+              {
+                versionNumber: 1,
+                variants: [
+                  {
+                    name: 'v-1',
+                  },
+                  {
+                    name: 'v-2',
+                  },
+                ],
+              },
+            ],
+          },
+        })
+      )
+    )
+    const session = {
+      splitTests: {
+        'anon-test-1': [
+          {
+            variantName: 'default',
+            versionNumber: 1,
+            phase: 'release',
+            assignedAt: new Date(1712872800000),
+          },
+        ],
+        'anon-test-2': [
+          {
+            variantName: 'default',
+            versionNumber: 1,
+            phase: 'release',
+            assignedAt: new Date(1712307600000),
+          },
+        ],
+      },
+      sta: `Zh+SqdaOpxHWvy30_2=1:sbsi00`,
+    }
+
+    const assignments =
+      await this.SplitTestSessionHandler.promises.getAssignments(session)
+    expect(assignments).to.deep.equal({
+      'anon-test-1': [
+        {
+          variantName: 'default',
+          versionNumber: 1,
+          phase: 'release',
+          assignedAt: new Date(1712872800000),
+        },
+      ],
+      'anon-test-2': [
+        {
+          variantName: 'default',
+          versionNumber: 1,
+          phase: 'release',
+          assignedAt: new Date(1712307600000),
+        },
+        {
+          variantName: 'v-2',
+          versionNumber: 2,
+          phase: 'release',
+          assignedAt: new Date(1712858400000),
+        },
+      ],
+    })
+  })
+})