github/overleaf
1
0
Fork 0
mirror of https://github.com/overleaf/overleaf.git synced 2024-11-21 20:47:08 -05:00
Code Issues Projects Releases Packages Wiki Activity

Merge pull request #18058 from overleaf/ab-split-test-convert-race-cond-fix

Browse source 
[web] Prevent failure due to race condition where converting anon assignments

GitOrigin-RevId: 81eb16689724b9ddc2ec7e23df2c3ea55837b83c
This commit is contained in:
Alexandre Bourdin 2024-04-22 18:33:34 +02:00 committed by Copybot
parent 4fcade16cb
commit 1336b2daeb
2 changed files with 87 additions and 9 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

24
services/web/app/src/Features/SplitTests/SplitTestSessionHandler.js
View file

@ -60,12 +60,18 @@ async function getAssignments(session) {
if (!assignments[splitTestName]) {
assignments[splitTestName] = []
}
assignments[splitTestName].push({
versionNumber,
variantName,
phase: 'release', // anonymous users can only be exposed to tests in release phase
assignedAt,
})
if (
!_.find(assignments[splitTestName], {
versionNumber,
variantName,
})
)
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 },
@ -193,13 +199,15 @@ async function _convertAnonymousAssignmentsIfNeeded(session) {
session.sta = ''
}
for (const [splitTestName, assignments] of Object.entries(
session.splitTests
session.splitTests || {}
)) {
const splitTest = splitTests.get(splitTestName)
for (const assignment of assignments) {
const assignmentString = _buildAssignmentString(splitTest, assignment)
const separator = session.sta.length > 0 ? TOKEN_SEP : ''
session.sta += `${separator}${assignmentString}`
if (!session.sta.includes(assignmentString)) {
session.sta += `${separator}${assignmentString}`
}
}
}
delete session.splitTests

72
services/web/test/unit/src/SplitTests/SplitTestSessionHandlerTests.js
View file

@ -120,7 +120,7 @@ describe('SplitTestSessionHandler', function () {
})
})
it('should not read from the sta field', async function () {
it('should read from the sta field', async function () {
this.SplitTestCache.get = sinon.stub().resolves(
new Map(
Object.entries({
@ -190,6 +190,76 @@ describe('SplitTestSessionHandler', function () {
})
})
it('should deduplicate entries 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_1=d:sbtqg0;Zh+SpGaXZLsD9z43_1=d:sbrvs0;Zh+SqdaOpxHWvy30_2=1:sbsi00;Zh+SqdaOpxHWvy30_1=d:sbtqg0`,
}
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 () {
const session = {
splitTests: {