Merge pull request #14803 from overleaf/jpa-split-test-cache-alpha-beta

[web] invalidate split test cache when alpha/beta program status changes

GitOrigin-RevId: 3023d2adf8466b48490c51497f5c80e7b0a1fe3d
This commit is contained in:
Jakob Ackermann 2023-09-13 13:07:00 +02:00 committed by Copybot
parent 7d3c8fb78d
commit 1e4dcc84d9
8 changed files with 99 additions and 3 deletions

View file

@ -66,6 +66,8 @@ const AuthenticationController = {
must_reconfirm: user.must_reconfirm,
v1_id: user.overleaf != null ? user.overleaf.id : undefined,
analyticsId: user.analyticsId || user._id,
alphaProgram: user.alphaProgram || undefined, // only store if set
betaProgram: user.betaProgram || undefined, // only store if set
}
callback(null, lightUser)
},

View file

@ -4,6 +4,7 @@ const UserGetter = require('../User/UserGetter')
const Settings = require('@overleaf/settings')
const logger = require('@overleaf/logger')
const SessionManager = require('../Authentication/SessionManager')
const SplitTestHandler = require('../SplitTests/SplitTestHandler')
const BetaProgramController = {
optIn(req, res, next) {
@ -16,7 +17,9 @@ const BetaProgramController = {
if (err) {
return next(err)
}
res.redirect('/beta/participate')
SplitTestHandler.sessionMaintenance(req, null, () =>
res.redirect('/beta/participate')
)
})
},
@ -30,7 +33,9 @@ const BetaProgramController = {
if (err) {
return next(err)
}
res.redirect('/beta/participate')
SplitTestHandler.sessionMaintenance(req, null, () =>
res.redirect('/beta/participate')
)
})
},

View file

@ -427,6 +427,7 @@ const ProjectController = {
},
user(cb) {
if (userId == null) {
SplitTestHandler.sessionMaintenance(req, null, () => {})
cb(null, defaultSettingsForAnonymousUser(userId))
} else {
User.updateOne(
@ -448,6 +449,7 @@ const ProjectController = {
return cb(err)
}
logger.debug({ projectId, userId }, 'got user')
SplitTestHandler.sessionMaintenance(req, user, () => {})
if (FeaturesUpdater.featuresEpochIsCurrent(user)) {
return cb(null, user)
}

View file

@ -101,7 +101,7 @@ async function projectListPage(req, res, next) {
})
const user = await User.findById(
userId,
`email emails features lastPrimaryEmailCheck signUpDate${
`email emails features alphaProgram betaProgram lastPrimaryEmailCheck signUpDate${
isSaas ? ' enrollment' : ''
}`
)
@ -113,6 +113,8 @@ async function projectListPage(req, res, next) {
}
if (isSaas) {
await SplitTestHandler.promises.sessionMaintenance(req, user)
try {
usersBestSubscription =
await SubscriptionViewModelBuilder.promises.getBestSubscription({

View file

@ -13,6 +13,7 @@ const { getAnalyticsIdFromMongoUser } = require('../Analytics/AnalyticsHelper')
const Features = require('../../infrastructure/Features')
const SplitTestUtils = require('./SplitTestUtils')
const Settings = require('@overleaf/settings')
const SessionManager = require('../Authentication/SessionManager')
const DEFAULT_VARIANT = 'default'
const ALPHA_PHASE = 'alpha'
@ -188,6 +189,34 @@ 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
*
@ -507,10 +536,12 @@ module.exports = {
getAssignmentForMongoUser: callbackify(getAssignmentForMongoUser),
getAssignmentForUser: callbackify(getAssignmentForUser),
getActiveAssignmentsForUser: callbackify(getActiveAssignmentsForUser),
sessionMaintenance: callbackify(sessionMaintenance),
promises: {
getAssignment,
getAssignmentForMongoUser,
getAssignmentForUser,
getActiveAssignmentsForUser,
sessionMaintenance,
},
}

View file

@ -21,8 +21,12 @@ describe('BetaProgramController', function () {
user: this.user,
},
}
this.SplitTestHandler = {
sessionMaintenance: sinon.stub().yields(),
}
this.BetaProgramController = SandboxedModule.require(modulePath, {
requires: {
'../SplitTests/SplitTestHandler': this.SplitTestHandler,
'./BetaProgramHandler': (this.BetaProgramHandler = {
optIn: sinon.stub(),
optOut: sinon.stub(),
@ -68,6 +72,13 @@ describe('BetaProgramController', function () {
this.BetaProgramHandler.optIn.callCount.should.equal(1)
})
it('should invoke the session maintenance', function () {
this.BetaProgramController.optIn(this.req, this.res, this.next)
this.SplitTestHandler.sessionMaintenance.should.have.been.calledWith(
this.req
)
})
describe('when BetaProgramHandler.opIn produces an error', function () {
beforeEach(function () {
this.BetaProgramHandler.optIn.callsArgWith(1, new Error('woops'))
@ -107,6 +118,13 @@ describe('BetaProgramController', function () {
this.BetaProgramHandler.optOut.callCount.should.equal(1)
})
it('should invoke the session maintenance', function () {
this.BetaProgramController.optOut(this.req, this.res, this.next)
this.SplitTestHandler.sessionMaintenance.should.have.been.calledWith(
this.req
)
})
describe('when BetaProgramHandler.optOut produces an error', function () {
beforeEach(function () {
this.BetaProgramHandler.optOut.callsArgWith(1, new Error('woops'))

View file

@ -128,6 +128,7 @@ describe('ProjectController', function () {
getAssignment: sinon.stub().resolves({ variant: 'default' }),
},
getAssignment: sinon.stub().yields(null, { variant: 'default' }),
sessionMaintenance: sinon.stub().yields(),
}
this.InstitutionsFeatures = {
hasLicence: sinon.stub().callsArgWith(1, null, false),
@ -515,6 +516,28 @@ describe('ProjectController', function () {
return this.ProjectController.loadEditor(this.req, this.res)
})
it('should invoke the session maintenance for logged in user', function (done) {
this.res.render = () => {
this.SplitTestHandler.sessionMaintenance.should.have.been.calledWith(
this.req,
this.user
)
done()
}
this.ProjectController.loadEditor(this.req, this.res)
})
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.req
)
done()
}
this.ProjectController.loadEditor(this.req, this.res)
})
it('should render the closed page if the editor is closed', function (done) {
this.settings.editorIsOpen = false
this.res.render = (pageName, opts) => {

View file

@ -102,6 +102,7 @@ describe('ProjectListController', function () {
}
this.SplitTestHandler = {
promises: {
sessionMaintenance: sinon.stub().resolves(),
getAssignment: sinon.stub().resolves({ variant: 'default' }),
},
}
@ -204,6 +205,18 @@ describe('ProjectListController', function () {
this.ProjectListController.projectListPage(this.req, this.res)
})
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.req,
this.user
)
done()
}
this.ProjectListController.projectListPage(this.req, this.res)
})
it('should send the tags', function (done) {
this.res.render = (pageName, opts) => {
opts.tags.length.should.equal(this.tags.length)