Просмотр исходного кода

Use AsyncLocalStorage to cache userFullEmails on the request, avoiding duplicated calls to the affiliations endpoint (#27542)

* Use AsyncLocalStorage to cache userFullEmails

* Rename temporary fakeUser override to avoid conflicts

GitOrigin-RevId: 3a74816f677c1357293b0d46f245b4cfc499f2fa
Thomas 1 год назад
Родитель
Сommit
7b7b560431

+ 14 - 1
services/web/app/src/Features/User/UserGetter.js

@@ -13,6 +13,7 @@ const { User } = require('../../models/User')
 const { normalizeQuery, normalizeMultiQuery } = require('../Helpers/Mongo')
 const Modules = require('../../infrastructure/Modules')
 const FeaturesHelper = require('../Subscription/FeaturesHelper')
+const AsyncLocalStorage = require('../../infrastructure/AsyncLocalStorage')
 
 function _lastDayToReconfirm(emailData, institutionData) {
   const globalReconfirmPeriod = settings.reconfirmNotificationDays
@@ -72,6 +73,10 @@ function _emailInReconfirmNotificationPeriod(
 }
 
 async function getUserFullEmails(userId) {
+  const store = AsyncLocalStorage.storage.getStore()
+  if (store?.userFullEmails?.[userId]) {
+    return store.userFullEmails[userId]
+  }
   const user = await UserGetter.promises.getUser(userId, {
     email: 1,
     emails: 1,
@@ -89,12 +94,20 @@ async function getUserFullEmails(userId) {
   const affiliationsData =
     await InstitutionsAPIPromises.getUserAffiliations(userId)
 
-  return decorateFullEmails(
+  const fullEmails = decorateFullEmails(
     user.email,
     user.emails || [],
     affiliationsData,
     user.samlIdentifiers || []
   )
+
+  if (store) {
+    if (!store.userFullEmails) {
+      store.userFullEmails = {}
+    }
+    store.userFullEmails[userId] = fullEmails
+  }
+  return fullEmails
 }
 
 async function getUserFeatures(userId) {

+ 21 - 0
services/web/app/src/infrastructure/AsyncLocalStorage.js

@@ -0,0 +1,21 @@
+// @ts-check
+const { AsyncLocalStorage } = require('node:async_hooks')
+
+/**
+ * @typedef {Object} RequestContext
+ * @property {Object.<string, array>} [userFullEmails] - Dictionary mapping userId to an array of full emails
+ */
+
+/** @type {AsyncLocalStorage<RequestContext>} */
+const asyncLocalStorage = new AsyncLocalStorage()
+
+/**
+ * @param {import("express").Request} req
+ * @param {import("express").Response} res
+ * @param {import("express").NextFunction} next
+ */
+function middleware(req, res, next) {
+  asyncLocalStorage.run({}, next)
+}
+
+module.exports = { middleware, storage: asyncLocalStorage }

+ 4 - 0
services/web/app/src/router.mjs

@@ -67,6 +67,7 @@ import { plainTextResponse } from './infrastructure/Response.js'
 import PublicAccessLevels from './Features/Authorization/PublicAccessLevels.js'
 import SocketDiagnostics from './Features/SocketDiagnostics/SocketDiagnostics.mjs'
 import ClsiCacheController from './Features/Compile/ClsiCacheController.js'
+import AsyncLocalStorage from './infrastructure/AsyncLocalStorage.js'
 
 const ClsiCookieManager = ClsiCookieManagerFactory(
   Settings.apis.clsi != null ? Settings.apis.clsi.backendGroupName : undefined
@@ -326,6 +327,7 @@ async function initialize(webRouter, privateApiRouter, publicApiRouter) {
   webRouter.get(
     '/user/emails',
     AuthenticationController.requireLogin(),
+    AsyncLocalStorage.middleware,
     PermissionsController.useCapabilities(),
     UserController.ensureAffiliationMiddleware,
     UserEmailsController.list
@@ -515,6 +517,7 @@ async function initialize(webRouter, privateApiRouter, publicApiRouter) {
     '/project',
     AuthenticationController.requireLogin(),
     RateLimiterMiddleware.rateLimit(rateLimiters.openDashboard),
+    AsyncLocalStorage.middleware,
     PermissionsController.useCapabilities(),
     ProjectListController.projectListPage
   )
@@ -542,6 +545,7 @@ async function initialize(webRouter, privateApiRouter, publicApiRouter) {
       RateLimiterMiddleware.rateLimit(openProjectRateLimiter, {
         params: ['Project_id'],
       }),
+      AsyncLocalStorage.middleware,
       PermissionsController.useCapabilities(),
       AuthorizationMiddleware.ensureUserCanReadProject,
       ProjectController.loadEditor

+ 108 - 4
services/web/test/unit/src/User/UserGetterTests.js

@@ -48,6 +48,11 @@ describe('UserGetter', function () {
     this.Modules = {
       promises: { hooks: { fire: sinon.stub().resolves() } },
     }
+    this.AsyncLocalStorage = {
+      storage: {
+        getStore: sinon.stub().returns(undefined),
+      },
+    }
 
     this.UserGetter = SandboxedModule.require(modulePath, {
       requires: {
@@ -68,6 +73,7 @@ describe('UserGetter', function () {
           User: (this.User = {}),
         },
         '../../infrastructure/Modules': this.Modules,
+        '../../infrastructure/AsyncLocalStorage': this.AsyncLocalStorage,
       },
     })
   })
@@ -287,19 +293,21 @@ describe('UserGetter', function () {
     })
 
     it('should get user when it has no emails field', function (done) {
-      this.fakeUser = {
+      this.fakeUserNoEmails = {
         _id: '12390i',
         email: 'email2@foo.bar',
       }
-      this.UserGetter.promises.getUser = sinon.stub().resolves(this.fakeUser)
+      this.UserGetter.promises.getUser = sinon
+        .stub()
+        .resolves(this.fakeUserNoEmails)
       const projection = { email: 1, emails: 1, samlIdentifiers: 1 }
       this.UserGetter.getUserFullEmails(
-        this.fakeUser._id,
+        this.fakeUserNoEmails._id,
         (error, fullEmails) => {
           expect(error).to.not.exist
           this.UserGetter.promises.getUser.called.should.equal(true)
           this.UserGetter.promises.getUser
-            .calledWith(this.fakeUser._id, projection)
+            .calledWith(this.fakeUserNoEmails._id, projection)
             .should.equal(true)
           assert.deepEqual(fullEmails, [])
           done()
@@ -1066,6 +1074,102 @@ describe('UserGetter', function () {
         })
       })
     })
+
+    describe('caching full emails data if run inside AsyncLocalStorage context', function () {
+      it('should store the data in the AsyncLocalStorage store', async function () {
+        this.store = {}
+        this.AsyncLocalStorage.storage.getStore.returns(this.store)
+        this.UserGetter.promises.getUser = sinon.stub().resolves(this.fakeUser)
+        this.getUserAffiliations.resolves([
+          {
+            email: 'email1@foo.bar',
+            licence: 'professional',
+            institution: {},
+          },
+        ])
+        const fullEmails = await this.UserGetter.promises.getUserFullEmails(
+          this.fakeUser._id
+        )
+        expect(this.UserGetter.promises.getUser).to.have.been.calledOnce
+        expect(this.getUserAffiliations).to.have.been.calledOnce
+        expect(fullEmails).to.be.an('array')
+        expect(fullEmails.length).to.equal(2)
+        expect(this.store.userFullEmails[this.fakeUser._id]).to.deep.equal(
+          fullEmails
+        )
+      })
+
+      it('should fetch data from the store if available', async function () {
+        this.store = {
+          userFullEmails: {
+            [this.fakeUser._id]: [{ email: '1' }, { email: '2' }],
+          },
+        }
+        this.AsyncLocalStorage.storage.getStore.returns(this.store)
+        this.UserGetter.promises.getUser = sinon.stub().resolves(this.fakeUser)
+        const fullEmails = await this.UserGetter.promises.getUserFullEmails(
+          this.fakeUser._id,
+          this.req
+        )
+        expect(this.UserGetter.promises.getUser).to.not.have.been.called
+        expect(this.getUserAffiliations).to.not.have.been.called
+        expect(fullEmails).to.be.an('array')
+        expect(fullEmails.length).to.equal(2)
+        expect(this.store.userFullEmails[this.fakeUser._id]).to.deep.equal(
+          fullEmails
+        )
+      })
+
+      it('should not return cached data for different user ids', async function () {
+        this.store = {}
+        this.AsyncLocalStorage.storage.getStore.returns(this.store)
+        this.UserGetter.promises.getUser = sinon.stub().resolves(this.fakeUser)
+        const fullEmails = await this.UserGetter.promises.getUserFullEmails(
+          this.fakeUser._id,
+          this.req
+        )
+        expect(this.UserGetter.promises.getUser).to.have.been.calledOnce
+        expect(this.getUserAffiliations).to.have.been.calledOnce
+        expect(fullEmails).to.be.an('array')
+        expect(fullEmails.length).to.equal(2)
+        this.otherUser = {
+          _id: new ObjectId(),
+          email: 'other@foo.bar',
+          emails: [
+            {
+              email: 'other@foo.bar',
+              reversedHostname: 'rab.oof',
+              confirmedAt: new Date(),
+              lastConfirmedAt: new Date(),
+            },
+          ],
+        }
+        this.UserGetter.promises.getUser.resolves(this.otherUser)
+        this.getUserAffiliations.resolves([
+          {
+            email: 'other@foo.bar',
+            licence: 'professional',
+            institution: {},
+          },
+        ])
+        const fullEmailsOther =
+          await this.UserGetter.promises.getUserFullEmails(
+            this.otherUser._id,
+            this.req
+          )
+        expect(this.UserGetter.promises.getUser).to.have.been.calledTwice
+        expect(this.getUserAffiliations).to.have.been.calledTwice
+        expect(fullEmailsOther).to.not.deep.equal(fullEmails)
+        expect(fullEmailsOther).to.be.an('array')
+        expect(fullEmailsOther.length).to.equal(1)
+        expect(this.store.userFullEmails[this.fakeUser._id]).to.deep.equal(
+          fullEmails
+        )
+        expect(this.store.userFullEmails[this.otherUser._id]).to.deep.equal(
+          fullEmailsOther
+        )
+      })
+    })
   })
 
   describe('getUserConfirmedEmails', function () {