Making `getAnalyticsIdFromMongoUser` private to `UserAnalyticsIdCache` (#16125)

* Making getAnalyticsIdFromMongoUser private to UserAnalyticsIdCache

* renaming userAnalyticsIdCache to UserAnalyticsIdCache

* deleting function _getAnalyticsIdFromMongoUser

* renaming userAnalyticsIdCache to UserAnalyticsIdCache part 2

* format:fix

* adding upperCamelCase

* capital case first letter

GitOrigin-RevId: 2e9c18c544b8cffb53838aed56e1ef16979606a5

services/web/app/src/Features/Analytics/AnalyticsHelper.js

@@ -1,8 +0,0 @@
-function getAnalyticsIdFromMongoUser(user) {
-  // ensure `analyticsId` is set to the user's `analyticsId`, and fallback to their `userId` for pre-analyticsId users
-  return user.analyticsId || user._id
-}
-
-module.exports = {
-  getAnalyticsIdFromMongoUser,
-}

services/web/app/src/Features/Analytics/AnalyticsManager.js

@@ -7,7 +7,6 @@ const crypto = require('crypto')
 const _ = require('lodash')
 const { expressify } = require('@overleaf/promise-utils')
 const logger = require('@overleaf/logger')
-const { getAnalyticsIdFromMongoUser } = require('./AnalyticsHelper')
 
 const analyticsEventsQueue = Queues.getQueue('analytics-events')
 const analyticsEditingSessionsQueue = Queues.getQueue(
@@ -308,12 +307,11 @@ async function analyticsIdMiddleware(req, res, next) {
   const sessionUser = SessionManager.getSessionUser(session)
 
   if (sessionUser) {
-    session.analyticsId = getAnalyticsIdFromMongoUser(sessionUser)
+    session.analyticsId = await UserAnalyticsIdCache.get(sessionUser._id)
   } else if (!session.analyticsId) {
     // generate an `analyticsId` if needed
     session.analyticsId = crypto.randomUUID()
   }
-
   next()
 }
 

services/web/app/src/Features/Analytics/UserAnalyticsIdCache.js

@@ -1,6 +1,6 @@
 const UserGetter = require('../User/UserGetter')
 const { CacheLoader } = require('cache-flow')
-const { getAnalyticsIdFromMongoUser } = require('./AnalyticsHelper')
+const { callbackify } = require('util')
 
 class UserAnalyticsIdCache extends CacheLoader {
   constructor() {
@@ -13,7 +13,7 @@ class UserAnalyticsIdCache extends CacheLoader {
   async load(userId) {
     const user = await UserGetter.promises.getUser(userId, { analyticsId: 1 })
     if (user) {
-      return getAnalyticsIdFromMongoUser(user)
+      return user.analyticsId || user._id.toString()
     }
   }
 
@@ -24,4 +24,8 @@ class UserAnalyticsIdCache extends CacheLoader {
   }
 }
 
-module.exports = new UserAnalyticsIdCache()
+const userAnalyticsIdCache = new UserAnalyticsIdCache()
+userAnalyticsIdCache.callbacks = {
+  get: callbackify(userAnalyticsIdCache.get).bind(userAnalyticsIdCache),
+}
+module.exports = userAnalyticsIdCache

services/web/app/src/Features/Compile/CompileManager.js

@@ -9,7 +9,7 @@ const ClsiManager = require('./ClsiManager')
 const Metrics = require('@overleaf/metrics')
 const { RateLimiter } = require('../../infrastructure/RateLimiter')
 const SplitTestHandler = require('../SplitTests/SplitTestHandler')
-const { getAnalyticsIdFromMongoUser } = require('../Analytics/AnalyticsHelper')
+const UserAnalyticsIdCache = require('../Analytics/UserAnalyticsIdCache')
 
 const NEW_COMPILE_TIMEOUT_ENFORCED_CUTOFF = new Date('2023-09-18T11:00:00.000Z')
 const NEW_COMPILE_TIMEOUT_ENFORCED_CUTOFF_DEFAULT_BASELINE = new Date(
@@ -178,70 +178,84 @@ module.exports = CompileManager = {
             if (owner && owner.alphaProgram) {
               ownerFeatures.compileGroup = 'alpha'
             }
-            const limits = {
-              timeout:
-                ownerFeatures.compileTimeout ||
-                Settings.defaultFeatures.compileTimeout,
-              compileGroup:
-                ownerFeatures.compileGroup ||
-                Settings.defaultFeatures.compileGroup,
-              ownerAnalyticsId: getAnalyticsIdFromMongoUser(owner),
-            }
-            CompileManager._getCompileBackendClassDetails(
-              owner,
-              limits.compileGroup,
-              (err, { compileBackendClass, showFasterCompilesFeedbackUI }) => {
-                if (err) return callback(err)
-                limits.compileBackendClass = compileBackendClass
-                limits.showFasterCompilesFeedbackUI =
-                  showFasterCompilesFeedbackUI
-                if (compileBackendClass === 'n2d' && limits.timeout <= 60) {
-                  // project owners with faster compiles but with <= 60 compile timeout (default)
-                  // will have a 20s compile timeout
-                  // The compile-timeout-20s split test exists to enable a gradual rollout
-                  SplitTestHandler.getAssignmentForMongoUser(
-                    owner,
-                    'compile-timeout-20s',
-                    (err, assignment) => {
-                      if (err) return callback(err)
-                      // users who were on the 'default' servers at time of original rollout
-                      // will have a later cutoff date for the 20s timeout in the next phase
-                      // we check the backend class at version 8 (baseline)
-                      const backendClassHistory =
-                        owner.splitTests?.['compile-backend-class-n2d'] || []
-                      const backendClassBaselineVariant =
-                        backendClassHistory.find(version => {
-                          return version.versionNumber === 8
-                        })?.variantName
-                      const timeoutEnforcedCutoff =
-                        backendClassBaselineVariant === 'default'
-                          ? NEW_COMPILE_TIMEOUT_ENFORCED_CUTOFF_DEFAULT_BASELINE
-                          : NEW_COMPILE_TIMEOUT_ENFORCED_CUTOFF
-                      if (assignment?.variant === '20s') {
-                        if (owner.signUpDate > timeoutEnforcedCutoff) {
-                          limits.timeout = 20
-                          callback(null, limits)
-                        } else {
-                          SplitTestHandler.getAssignmentForMongoUser(
-                            owner,
-                            'compile-timeout-20s-existing-users',
-                            (err, assignmentExistingUsers) => {
-                              if (err) return callback(err)
-                              if (assignmentExistingUsers?.variant === '20s') {
-                                limits.timeout = 20
-                              }
+            UserAnalyticsIdCache.callbacks.get(
+              owner._id,
+              function (err, analyticsId) {
+                if (err) {
+                  return callback(err)
+                }
+                const limits = {
+                  timeout:
+                    ownerFeatures.compileTimeout ||
+                    Settings.defaultFeatures.compileTimeout,
+                  compileGroup:
+                    ownerFeatures.compileGroup ||
+                    Settings.defaultFeatures.compileGroup,
+                  ownerAnalyticsId: analyticsId,
+                }
+                CompileManager._getCompileBackendClassDetails(
+                  owner,
+                  limits.compileGroup,
+                  (
+                    err,
+                    { compileBackendClass, showFasterCompilesFeedbackUI }
+                  ) => {
+                    if (err) return callback(err)
+                    limits.compileBackendClass = compileBackendClass
+                    limits.showFasterCompilesFeedbackUI =
+                      showFasterCompilesFeedbackUI
+                    if (compileBackendClass === 'n2d' && limits.timeout <= 60) {
+                      // project owners with faster compiles but with <= 60 compile timeout (default)
+                      // will have a 20s compile timeout
+                      // The compile-timeout-20s split test exists to enable a gradual rollout
+                      SplitTestHandler.getAssignmentForMongoUser(
+                        owner,
+                        'compile-timeout-20s',
+                        (err, assignment) => {
+                          if (err) return callback(err)
+                          // users who were on the 'default' servers at time of original rollout
+                          // will have a later cutoff date for the 20s timeout in the next phase
+                          // we check the backend class at version 8 (baseline)
+                          const backendClassHistory =
+                            owner.splitTests?.['compile-backend-class-n2d'] ||
+                            []
+                          const backendClassBaselineVariant =
+                            backendClassHistory.find(version => {
+                              return version.versionNumber === 8
+                            })?.variantName
+                          const timeoutEnforcedCutoff =
+                            backendClassBaselineVariant === 'default'
+                              ? NEW_COMPILE_TIMEOUT_ENFORCED_CUTOFF_DEFAULT_BASELINE
+                              : NEW_COMPILE_TIMEOUT_ENFORCED_CUTOFF
+                          if (assignment?.variant === '20s') {
+                            if (owner.signUpDate > timeoutEnforcedCutoff) {
+                              limits.timeout = 20
                               callback(null, limits)
+                            } else {
+                              SplitTestHandler.getAssignmentForMongoUser(
+                                owner,
+                                'compile-timeout-20s-existing-users',
+                                (err, assignmentExistingUsers) => {
+                                  if (err) return callback(err)
+                                  if (
+                                    assignmentExistingUsers?.variant === '20s'
+                                  ) {
+                                    limits.timeout = 20
+                                  }
+                                  callback(null, limits)
+                                }
+                              )
                             }
-                          )
+                          } else {
+                            callback(null, limits)
+                          }
                         }
-                      } else {
-                        callback(null, limits)
-                      }
+                      )
+                    } else {
+                      callback(null, limits)
                     }
-                  )
-                } else {
-                  callback(null, limits)
-                }
+                  }
+                )
               }
             )
           }

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

@@ -9,7 +9,6 @@ const { callbackify } = require('util')
 const SplitTestCache = require('./SplitTestCache')
 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 Settings = require('@overleaf/settings')
@@ -142,7 +141,7 @@ async function getAssignmentForMongoUser(
   }
 
   return _getAssignment(splitTestName, {
-    analyticsId: getAnalyticsIdFromMongoUser(user),
+    analyticsId: await UserAnalyticsIdCache.get(user._id),
     sync,
     user,
     userId: user._id.toString(),

services/web/test/unit/src/Analytics/AnalyticsManagerTests.js

@@ -304,7 +304,10 @@ describe('AnalyticsManager', function () {
               }
             },
           },
-          './UserAnalyticsIdCache': {},
+
+          './UserAnalyticsIdCache': (this.UserAnalyticsIdCache = {
+            get: sinon.stub().resolves(this.analyticsId),
+          }),
           crypto: {
             randomUUID: () => this.analyticsId,
           },
@@ -343,12 +346,14 @@ describe('AnalyticsManager', function () {
       await this.AnalyticsManager.analyticsIdMiddleware(
         this.req,
         this.res,
-        this.next
+        () => {
+          assert.equal(this.analyticsId, this.req.session.analyticsId)
+        }
       )
-      assert.equal(this.analyticsId, this.req.session.analyticsId)
     })
 
     it('sets session.analyticsId with a legacy user session without an analyticsId', async function () {
+      this.UserAnalyticsIdCache.get.resolves(this.userId)
       this.req.session.user = {
         _id: this.userId,
         analyticsId: undefined,
@@ -356,26 +361,26 @@ describe('AnalyticsManager', function () {
       await this.AnalyticsManager.analyticsIdMiddleware(
         this.req,
         this.res,
-        this.next
+        () => {
+          assert.equal(this.userId, this.req.session.analyticsId)
+        }
       )
-      assert.equal(this.userId, this.req.session.analyticsId)
     })
 
     it('updates session.analyticsId with a legacy user session without an analyticsId if different', async function () {
+      this.UserAnalyticsIdCache.get.resolves(this.userId)
       this.req.session.user = {
         _id: this.userId,
         analyticsId: undefined,
       }
       this.req.analyticsId = 'foo'
-      await this.AnalyticsManager.analyticsIdMiddleware(
-        this.req,
-        this.res,
-        this.next
-      )
-      assert.equal(this.userId, this.req.session.analyticsId)
+      this.AnalyticsManager.analyticsIdMiddleware(this.req, this.res, () => {
+        assert.equal(this.userId, this.req.session.analyticsId)
+      })
     })
 
     it('does not update session.analyticsId with a legacy user session without an analyticsId if same', async function () {
+      this.UserAnalyticsIdCache.get.resolves(this.userId)
       this.req.session.user = {
         _id: this.userId,
         analyticsId: undefined,
@@ -384,16 +389,10 @@ describe('AnalyticsManager', function () {
       await this.AnalyticsManager.analyticsIdMiddleware(
         this.req,
         this.res,
-        this.next
-      )
-      assert.equal(this.userId, this.req.session.analyticsId)
-
-      await this.AnalyticsManager.analyticsIdMiddleware(
-        this.req,
-        this.res,
-        this.next
+        () => {
+          assert.equal(this.userId, this.req.session.analyticsId)
+        }
       )
-      assert.equal(this.userId, this.req.session.analyticsId)
     })
   })
 })

services/web/test/unit/src/Compile/CompileManagerTests.js

@@ -37,6 +37,11 @@ describe('CompileManager', function () {
         './ClsiManager': (this.ClsiManager = {}),
         '../../infrastructure/RateLimiter': this.RateLimiter,
         '@overleaf/metrics': this.Metrics,
+        '../Analytics/UserAnalyticsIdCache': (this.UserAnalyticsIdCache = {
+          callbacks: {
+            get: sinon.stub().yields(null, 'abc'),
+          },
+        }),
         '../SplitTests/SplitTestHandler': {
           getAssignmentForMongoUser: (this.getAssignmentForMongoUser = sinon
             .stub()
@@ -177,7 +182,7 @@ describe('CompileManager', function () {
   })
 
   describe('getProjectCompileLimits', function () {
-    beforeEach(function () {
+    beforeEach(function (done) {
       this.features = {
         compileTimeout: (this.timeout = 42),
         compileGroup: (this.group = 'priority'),
@@ -198,7 +203,10 @@ describe('CompileManager', function () {
         )
       this.CompileManager.getProjectCompileLimits(
         this.project_id,
-        this.callback
+        (err, res) => {
+          this.callback(err, res)
+          done()
+        }
       )
     })