mirror of
https://github.com/yu-i-i/overleaf-cep.git
synced 2026-05-28 19:41:33 +02:00
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
This commit is contained in:
@@ -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
services/web/app/src/infrastructure/AsyncLocalStorage.js
Normal file
21
services/web/app/src/infrastructure/AsyncLocalStorage.js
Normal file
@@ -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 }
|
||||
@@ -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
|
||||
|
||||
@@ -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 () {
|
||||
|
||||
Reference in New Issue
Block a user