Merge pull request #10266 from overleaf/ab-split-test-no-serialization

[web] Skip mongoose object transformations for the split test cache

GitOrigin-RevId: 8fb5420f6f938c0ab7cfe1ca82c107c7ce3522ca

services/web/app/src/Features/SplitTests/SplitTestCache.js

@@ -1,5 +1,4 @@
 const SplitTestManager = require('./SplitTestManager')
-const { SplitTest } = require('../../models/SplitTest')
 const { CacheLoader } = require('cache-flow')
 
 class SplitTestCache extends CacheLoader {
@@ -10,18 +9,19 @@ class SplitTestCache extends CacheLoader {
   }
 
   async load(name) {
-    return await SplitTestManager.getSplitTest({
+    const splitTest = await SplitTestManager.getSplitTest({
       name,
       archived: { $ne: true },
     })
+    return splitTest?.toObject()
   }
 
   serialize(value) {
-    return value ? value.toObject() : undefined
+    return value
   }
 
   deserialize(value) {
-    return new SplitTest(value)
+    return value
   }
 }
 

services/web/app/src/Features/SplitTests/SplitTestHandler.js

@@ -10,6 +10,7 @@ const { SplitTest } = require('../../models/SplitTest')
 const UserAnalyticsIdCache = require('../Analytics/UserAnalyticsIdCache')
 const { getAnalyticsIdFromMongoUser } = require('../Analytics/AnalyticsHelper')
 const Features = require('../../infrastructure/Features')
+const SplitTestUtils = require('./SplitTestUtils')
 
 const DEFAULT_VARIANT = 'default'
 const ALPHA_PHASE = 'alpha'
@@ -191,7 +192,7 @@ async function getActiveAssignmentsForUser(userId) {
  */
 async function _getVariantNames(splitTestName) {
   const splitTest = await SplitTestCache.get(splitTestName)
-  const currentVersion = splitTest?.getCurrentVersion()
+  const currentVersion = SplitTestUtils.getCurrentVersion(splitTest)
   if (currentVersion?.active) {
     return currentVersion.variants.map(v => v.name).concat([DEFAULT_VARIANT])
   } else {
@@ -208,7 +209,7 @@ async function _getAssignment(
   }
 
   const splitTest = await SplitTestCache.get(splitTestName)
-  const currentVersion = splitTest?.getCurrentVersion()
+  const currentVersion = SplitTestUtils.getCurrentVersion(splitTest)
   if (!currentVersion?.active) {
     return DEFAULT_ASSIGNMENT
   }
@@ -251,7 +252,7 @@ async function _getAssignment(
 }
 
 async function _getAssignmentMetadata(analyticsId, user, splitTest) {
-  const currentVersion = splitTest.getCurrentVersion()
+  const currentVersion = SplitTestUtils.getCurrentVersion(splitTest)
   const phase = currentVersion.phase
   if (
     (phase === ALPHA_PHASE && !user?.alphaProgram) ||
@@ -398,10 +399,10 @@ async function _getUser(id) {
 async function _loadSplitTestInfoInLocals(locals, splitTestName) {
   const splitTest = await SplitTestCache.get(splitTestName)
   if (splitTest) {
-    const phase = splitTest.getCurrentVersion().phase
+    const phase = SplitTestUtils.getCurrentVersion(splitTest).phase
     LocalsHelper.setSplitTestInfo(locals, splitTestName, {
       phase,
-      badgeInfo: splitTest.toObject().badgeInfo?.[phase],
+      badgeInfo: splitTest.badgeInfo?.[phase],
     })
   }
 }

services/web/app/src/Features/SplitTests/SplitTestManager.js

@@ -1,4 +1,5 @@
 const { SplitTest } = require('../../models/SplitTest')
+const SplitTestUtils = require('./SplitTestUtils')
 const OError = require('@overleaf/o-error')
 const _ = require('lodash')
 
@@ -89,7 +90,7 @@ async function updateSplitTestConfig(name, configuration) {
   if (splitTest.archived) {
     throw new OError('Cannot update an archived split test', { name })
   }
-  const lastVersion = splitTest.getCurrentVersion().toObject()
+  const lastVersion = SplitTestUtils.getCurrentVersion(splitTest).toObject()
   if (configuration.phase !== lastVersion.phase) {
     throw new OError(
       `Cannot update with different phase - use /switch-to-next-phase endpoint instead`
@@ -145,7 +146,7 @@ async function switchToNextPhase(name) {
       name,
     })
   }
-  const lastVersionCopy = splitTest.getCurrentVersion().toObject()
+  const lastVersionCopy = SplitTestUtils.getCurrentVersion(splitTest).toObject()
   lastVersionCopy.versionNumber++
   if (lastVersionCopy.phase === ALPHA_PHASE) {
     lastVersionCopy.phase = BETA_PHASE
@@ -189,13 +190,13 @@ async function revertToPreviousVersion(name, versionNumber) {
       `Cannot revert split test with ID '${name}' to previous version: split test must have at least 2 versions`
     )
   }
-  const previousVersion = splitTest.getVersion(versionNumber)
+  const previousVersion = SplitTestUtils.getVersion(splitTest, versionNumber)
   if (!previousVersion) {
     throw new OError(
       `Cannot revert split test with ID '${name}' to version number ${versionNumber}: version not found`
     )
   }
-  const lastVersion = splitTest.getCurrentVersion()
+  const lastVersion = SplitTestUtils.getCurrentVersion(splitTest)
   if (
     lastVersion.phase === RELEASE_PHASE &&
     previousVersion.phase !== RELEASE_PHASE
@@ -217,7 +218,8 @@ async function archive(name) {
     throw new OError(`Split test with ID '${name}' is already archived`)
   }
   splitTest.archived = true
-  const previousVersionCopy = splitTest.getCurrentVersion().toObject()
+  const previousVersionCopy =
+    SplitTestUtils.getCurrentVersion(splitTest).toObject()
   previousVersionCopy.versionNumber += 1
   previousVersionCopy.active = false
   splitTest.versions.push(previousVersionCopy)

services/web/app/src/Features/SplitTests/SplitTestUtils.js

@@ -0,0 +1,20 @@
+const _ = require('lodash')
+
+function getCurrentVersion(splitTest) {
+  if (splitTest?.versions?.length > 0) {
+    return _.maxBy(splitTest.versions, 'versionNumber')
+  } else {
+    return undefined
+  }
+}
+
+function getVersion(splitTest, versionNumber) {
+  return _.find(splitTest.versions || [], {
+    versionNumber,
+  })
+}
+
+module.exports = {
+  getCurrentVersion,
+  getVersion,
+}

services/web/app/src/models/SplitTest.js

@@ -1,6 +1,5 @@
 const mongoose = require('../infrastructure/Mongoose')
 const { Schema } = mongoose
-const _ = require('lodash')
 
 const MIN_NAME_LENGTH = 3
 const MAX_NAME_LENGTH = 200
@@ -147,20 +146,6 @@ const SplitTestSchema = new Schema({
   },
 })
 
-SplitTestSchema.methods.getCurrentVersion = function () {
-  if (this.versions && this.versions.length > 0) {
-    return _.maxBy(this.versions, 'versionNumber')
-  } else {
-    return undefined
-  }
-}
-
-SplitTestSchema.methods.getVersion = function (versionNumber) {
-  return _.find(this.versions || [], {
-    versionNumber,
-  })
-}
-
 module.exports = {
   SplitTest: mongoose.model('SplitTest', SplitTestSchema),
   SplitTestSchema,

services/web/test/unit/src/SplitTests/SplitTestHandlerTests.js

@@ -176,28 +176,31 @@ describe('SplitTestHandler', function () {
   })
 })
 
-function makeSplitTest(name, opts = {}) {
-  const {
+function makeSplitTest(
+  name,
+  {
     active = true,
     analyticsEnabled = active,
     phase = 'release',
     versionNumber = 1,
-  } = opts
-
+  } = {}
+) {
   return {
     name,
-    getCurrentVersion: sinon.stub().returns({
-      active,
-      analyticsEnabled,
-      phase,
-      versionNumber,
-      variants: [
-        {
-          name: 'variant-1',
-          rolloutPercent: 100,
-          rolloutStripes: [{ start: 0, end: 100 }],
-        },
-      ],
-    }),
+    versions: [
+      {
+        active,
+        analyticsEnabled,
+        phase,
+        versionNumber,
+        variants: [
+          {
+            name: 'variant-1',
+            rolloutPercent: 100,
+            rolloutStripes: [{ start: 0, end: 100 }],
+          },
+        ],
+      },
+    ],
   }
 }