Merge pull request #5881 from overleaf/ab-split-test-middleware

New global split test middleware for locals

GitOrigin-RevId: b530572f709572663fc3d051f544064bd8804f76

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

@@ -0,0 +1,10 @@
+function setSplitTestVariant(locals, splitTestName, variant) {
+  if (!locals.splitTestVariants) {
+    locals.splitTestVariants = {}
+  }
+  locals.splitTestVariants[splitTestName] = variant
+}
+
+module.exports = {
+  setSplitTestVariant,
+}

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

@@ -0,0 +1,54 @@
+const SplitTestV2Handler = require('./SplitTestV2Handler')
+const SplitTestCache = require('./SplitTestCache')
+const LocalsHelper = require('./LocalsHelper')
+const logger = require('@overleaf/logger')
+
+function loadAssignmentsInLocals(splitTestNames) {
+  return async function (req, res, next) {
+    try {
+      if (!req.session.cachedSplitTestAssignments) {
+        req.session.cachedSplitTestAssignments = {}
+      }
+      for (const splitTestName of splitTestNames) {
+        const splitTest = await SplitTestCache.get(splitTestName)
+        if (splitTest) {
+          await _loadAssignmentInLocals(splitTest, req.session, res.locals)
+        }
+      }
+    } catch (error) {
+      logger.error(
+        { err: error, splitTestNames },
+        'Failed to load split test assignments in express locals in middleware'
+      )
+    }
+    next()
+  }
+}
+
+async function _loadAssignmentInLocals(splitTest, session, locals) {
+  const currentVersion = splitTest.getCurrentVersion()
+  const cacheKey = `${splitTest.name}-${currentVersion.versionNumber}`
+  if (currentVersion.active) {
+    const cachedVariant = session.cachedSplitTestAssignments[cacheKey]
+    if (cachedVariant) {
+      LocalsHelper.setSplitTestVariant(locals, splitTest.name, cachedVariant)
+    } else {
+      const assignment = await SplitTestV2Handler.promises.getAssignmentForSession(
+        session,
+        splitTest.name
+      )
+      session.cachedSplitTestAssignments[cacheKey] = assignment.variant
+      LocalsHelper.setSplitTestVariant(
+        locals,
+        splitTest.name,
+        assignment.variant
+      )
+    }
+  } else {
+    delete session.cachedSplitTestAssignments[cacheKey]
+  }
+}
+
+module.exports = {
+  loadAssignmentsInLocals,
+}

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

@@ -2,6 +2,7 @@ const UserGetter = require('../User/UserGetter')
 const UserUpdater = require('../User/UserUpdater')
 const AnalyticsManager = require('../Analytics/AnalyticsManager')
 const UserAnalyticsIdCache = require('../Analytics/UserAnalyticsIdCache')
+const LocalsHelper = require('./LocalsHelper')
 const crypto = require('crypto')
 const _ = require('lodash')
 const { callbackify } = require('util')
@@ -89,10 +90,11 @@ async function getAssignmentForSession(session, splitTestName, options) {
  */
 async function assignInLocalsContext(res, userId, splitTestName, options) {
   const assignment = await getAssignment(userId, splitTestName, options)
-  if (!res.locals.splitTestVariants) {
-    res.locals.splitTestVariants = {}
-  }
-  res.locals.splitTestVariants[splitTestName] = assignment.variant
+  LocalsHelper.setSplitTestVariant(
+    res.locals,
+    splitTestName,
+    assignment.variant
+  )
 }
 
 /**
@@ -115,10 +117,11 @@ async function assignInLocalsContextForSession(
     splitTestName,
     options
   )
-  if (!res.locals.splitTestVariants) {
-    res.locals.splitTestVariants = {}
-  }
-  res.locals.splitTestVariants[splitTestName] = assignment.variant
+  LocalsHelper.setSplitTestVariant(
+    res.locals,
+    splitTestName,
+    assignment.variant
+  )
 }
 
 async function _getAssignment(

services/web/app/src/router.js

@@ -51,6 +51,7 @@ const UserMembershipRouter = require('./Features/UserMembership/UserMembershipRo
 const SystemMessageController = require('./Features/SystemMessages/SystemMessageController')
 const AnalyticsRegistrationSourceMiddleware = require('./Features/Analytics/AnalyticsRegistrationSourceMiddleware')
 const AnalyticsUTMTrackingMiddleware = require('./Features/Analytics/AnalyticsUTMTrackingMiddleware')
+const SplitTestMiddleware = require('./Features/SplitTests/SplitTestMiddleware')
 const { Joi, validate } = require('./infrastructure/Validation')
 const {
   renderUnsupportedBrowserPage,
@@ -59,6 +60,7 @@ const {
 
 const logger = require('@overleaf/logger')
 const _ = require('underscore')
+const { expressify } = require('./util/promises')
 
 module.exports = { initialize }
 
@@ -71,6 +73,10 @@ function initialize(webRouter, privateApiRouter, publicApiRouter) {
 
   webRouter.get('*', AnalyticsRegistrationSourceMiddleware.setInbound())
   webRouter.get('*', AnalyticsUTMTrackingMiddleware.recordUTMTags())
+  webRouter.get(
+    '*',
+    expressify(SplitTestMiddleware.loadAssignmentsInLocals([]))
+  )
 
   webRouter.get('/login', UserPagesController.loginPage)
   AuthenticationController.addEndpointToLoginWhitelist('/login')

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

@@ -0,0 +1,178 @@
+const SandboxedModule = require('sandboxed-module')
+const path = require('path')
+const modulePath = path.join(
+  __dirname,
+  '../../../../app/src/Features/SplitTests/SplitTestMiddleware'
+)
+const sinon = require('sinon')
+const { assert } = require('chai')
+const MockResponse = require('../helpers/MockResponse')
+const MockRequest = require('../helpers/MockRequest')
+
+describe('SplitTestMiddleware', function () {
+  beforeEach(function () {
+    this.SplitTestMiddleware = SandboxedModule.require(modulePath, {
+      requires: {
+        './SplitTestV2Handler': (this.SplitTestV2Handler = {
+          promises: {
+            getAssignmentForSession: sinon.stub().resolves(),
+          },
+        }),
+        './SplitTestCache': (this.SplitTestCache = {
+          get: sinon.stub().resolves(),
+        }),
+      },
+    })
+
+    this.req = new MockRequest()
+    this.req.session = {}
+    this.res = new MockResponse()
+    this.next = sinon.stub()
+  })
+
+  it('assign split test variant in locals', async function () {
+    this.SplitTestCache.get.withArgs('ui-overhaul').resolves({
+      name: 'ui-overhaul',
+      getCurrentVersion: () => ({
+        versionNumber: 1,
+        active: true,
+      }),
+    })
+    this.SplitTestV2Handler.promises.getAssignmentForSession
+      .withArgs(this.req.session, 'ui-overhaul')
+      .resolves({
+        variant: 'new',
+      })
+
+    const middleware = this.SplitTestMiddleware.loadAssignmentsInLocals([
+      'ui-overhaul',
+    ])
+    await middleware(this.req, this.res, this.next)
+
+    assert.equal(this.res.locals.splitTestVariants['ui-overhaul'], 'new')
+    assert.deepEqual(this.req.session.cachedSplitTestAssignments, {
+      'ui-overhaul-1': 'new',
+    })
+    sinon.assert.calledOnce(this.next)
+  })
+
+  it('assign multiple split test variant in locals', async function () {
+    this.SplitTestCache.get
+      .withArgs('ui-overhaul')
+      .resolves({
+        name: 'ui-overhaul',
+        getCurrentVersion: () => ({
+          versionNumber: 1,
+          active: true,
+        }),
+      })
+      .withArgs('other-test')
+      .resolves({
+        name: 'other-test',
+        getCurrentVersion: () => ({
+          versionNumber: 1,
+          active: true,
+        }),
+      })
+
+    this.SplitTestV2Handler.promises.getAssignmentForSession
+      .withArgs(this.req.session, 'ui-overhaul')
+      .resolves({
+        variant: 'default',
+      })
+    this.SplitTestV2Handler.promises.getAssignmentForSession
+      .withArgs(this.req.session, 'other-test')
+      .resolves({
+        variant: 'foobar',
+      })
+
+    const middleware = this.SplitTestMiddleware.loadAssignmentsInLocals([
+      'ui-overhaul',
+      'other-test',
+    ])
+    await middleware(this.req, this.res, this.next)
+
+    assert.equal(this.res.locals.splitTestVariants['ui-overhaul'], 'default')
+    assert.equal(this.res.locals.splitTestVariants['other-test'], 'foobar')
+    assert.deepEqual(this.req.session.cachedSplitTestAssignments, {
+      'ui-overhaul-1': 'default',
+      'other-test-1': 'foobar',
+    })
+    sinon.assert.calledOnce(this.next)
+  })
+
+  it('cached assignment in session is used', async function () {
+    this.req.session.cachedSplitTestAssignments = {
+      'ui-overhaul-1': 'cached-variant',
+    }
+    this.SplitTestCache.get.withArgs('ui-overhaul').resolves({
+      name: 'ui-overhaul',
+      getCurrentVersion: () => ({
+        versionNumber: 1,
+        active: true,
+      }),
+    })
+
+    const middleware = this.SplitTestMiddleware.loadAssignmentsInLocals([
+      'ui-overhaul',
+    ])
+    await middleware(this.req, this.res, this.next)
+
+    sinon.assert.notCalled(
+      this.SplitTestV2Handler.promises.getAssignmentForSession
+    )
+    assert.equal(
+      this.res.locals.splitTestVariants['ui-overhaul'],
+      'cached-variant'
+    )
+    assert.deepEqual(this.req.session.cachedSplitTestAssignments, {
+      'ui-overhaul-1': 'cached-variant',
+    })
+    sinon.assert.calledOnce(this.next)
+  })
+
+  it('inactive split test is not assigned in locals', async function () {
+    this.SplitTestCache.get.withArgs('ui-overhaul').resolves({
+      name: 'ui-overhaul',
+      getCurrentVersion: () => ({
+        versionNumber: 1,
+        active: false,
+      }),
+    })
+
+    const middleware = this.SplitTestMiddleware.loadAssignmentsInLocals([
+      'ui-overhaul',
+    ])
+    await middleware(this.req, this.res, this.next)
+
+    assert.equal(this.res.locals.splitTestVariants, undefined)
+    assert.deepEqual(this.req.session.cachedSplitTestAssignments, {})
+    sinon.assert.calledOnce(this.next)
+  })
+
+  it('not existing split test is not assigned in locals', async function () {
+    this.SplitTestCache.get.withArgs('not-found').resolves(undefined)
+
+    const middleware = this.SplitTestMiddleware.loadAssignmentsInLocals([
+      'not-found',
+    ])
+    await middleware(this.req, this.res, this.next)
+
+    assert.equal(this.res.locals.splitTestVariants, undefined)
+    assert.deepEqual(this.req.session.cachedSplitTestAssignments, {})
+    sinon.assert.calledOnce(this.next)
+  })
+
+  it('next middleware is called even if there is an error', async function () {
+    this.SplitTestCache.get.throws('some error')
+
+    const middleware = this.SplitTestMiddleware.loadAssignmentsInLocals([
+      'some-test',
+    ])
+    await middleware(this.req, this.res, this.next)
+
+    assert.equal(this.res.locals.splitTestVariants, undefined)
+    assert.deepEqual(this.req.session.cachedSplitTestAssignments, {})
+    sinon.assert.calledOnce(this.next)
+  })
+})

services/web/test/unit/src/helpers/MockResponse.js

@@ -27,6 +27,7 @@ class MockResponse {
     this.redirected = false
     this.returned = false
     this.headers = {}
+    this.locals = {}
   }
 
   render(template, variables) {