[web] confirmDomain: skip fetching affiliations per user

Nothing is looking at either
 - `user.emails[i].affiliation` or
 - `user.emails[i].emailHasInstitutionLicence`
So we might as well skip fetching the data.

This eliminates N v1 calls and N mongo calls from the endpoint.

GitOrigin-RevId: bb1d077df19910b9dfb7ef06562cf35ce5302290
This commit is contained in:
Jakob Ackermann 2022-01-05 10:13:28 +00:00 committed by Copybot
parent 5a7698ddef
commit c97e95aeba
3 changed files with 37 additions and 24 deletions

View file

@ -19,7 +19,7 @@ module.exports = {
function affiliateUsers(hostname, callback) { function affiliateUsers(hostname, callback) {
const reversedHostname = hostname.trim().split('').reverse().join('') const reversedHostname = hostname.trim().split('').reverse().join('')
UserGetter.getUsersByHostname(hostname, { _id: 1 }, function (error, users) { UserGetter.getInstitutionUsersByHostname(hostname, (error, users) => {
if (error) { if (error) {
OError.tag(error, 'problem fetching users by hostname') OError.tag(error, 'problem fetching users by hostname')
return callback(error) return callback(error)
@ -29,11 +29,7 @@ function affiliateUsers(hostname, callback) {
users, users,
ASYNC_AFFILIATIONS_LIMIT, ASYNC_AFFILIATIONS_LIMIT,
(user, innerCallback) => { (user, innerCallback) => {
UserGetter.getUserFullEmails(user._id, (error, emails) => {
if (error) return innerCallback(error)
user.emails = emails
affiliateUserByReversedHostname(user, reversedHostname, innerCallback) affiliateUserByReversedHostname(user, reversedHostname, innerCallback)
})
}, },
callback callback
) )

View file

@ -196,6 +196,28 @@ const UserGetter = {
db.users.find(query, { projection }).toArray(callback) db.users.find(query, { projection }).toArray(callback)
}, },
getInstitutionUsersByHostname(hostname, callback) {
const projection = {
_id: 1,
email: 1,
emails: 1,
samlIdentifiers: 1,
}
UserGetter.getUsersByHostname(hostname, projection, (err, users) => {
if (err) return callback(err)
users.forEach(user => {
user.emails = decorateFullEmails(
user.email,
user.emails,
[],
user.samlIdentifiers || []
)
})
callback(null, users)
})
},
getUsers(query, projection, callback) { getUsers(query, projection, callback) {
try { try {
query = normalizeMultiQuery(query) query = normalizeMultiQuery(query)

View file

@ -59,27 +59,22 @@ describe('InstitutionsController', function () {
}, },
] ]
this.getUsersByHostname = sinon.stub().callsArgWith( this.getInstitutionUsersByHostname = sinon.stub().yields(null, [
2, {
null, _id: this.stubbedUser1._id,
[this.stubbedUser1, this.stubbedUser2].map(user => { emails: this.stubbedUser1DecoratedEmails,
return { _id: user._id } },
}) {
) _id: this.stubbedUser2._id,
emails: this.stubbedUser2DecoratedEmails,
},
])
this.addAffiliation = sinon.stub().callsArgWith(3, null) this.addAffiliation = sinon.stub().callsArgWith(3, null)
this.refreshFeatures = sinon.stub().yields(null) this.refreshFeatures = sinon.stub().yields(null)
this.getUserFullEmails = sinon.stub()
this.getUserFullEmails
.withArgs(this.stubbedUser1._id)
.yields(null, this.stubbedUser1DecoratedEmails)
this.getUserFullEmails
.withArgs(this.stubbedUser2._id)
.yields(null, this.stubbedUser2DecoratedEmails)
this.InstitutionsController = SandboxedModule.require(modulePath, { this.InstitutionsController = SandboxedModule.require(modulePath, {
requires: { requires: {
'../User/UserGetter': { '../User/UserGetter': {
getUsersByHostname: this.getUsersByHostname, getInstitutionUsersByHostname: this.getInstitutionUsersByHostname,
getUserFullEmails: this.getUserFullEmails,
}, },
'../Institutions/InstitutionsAPI': { '../Institutions/InstitutionsAPI': {
addAffiliation: this.addAffiliation, addAffiliation: this.addAffiliation,
@ -103,7 +98,7 @@ describe('InstitutionsController', function () {
it('should add affiliations for matching users', function (done) { it('should add affiliations for matching users', function (done) {
this.res.sendStatus = code => { this.res.sendStatus = code => {
code.should.equal(200) code.should.equal(200)
this.getUsersByHostname.calledOnce.should.equal(true) this.getInstitutionUsersByHostname.calledOnce.should.equal(true)
this.addAffiliation.calledThrice.should.equal(true) this.addAffiliation.calledThrice.should.equal(true)
this.addAffiliation this.addAffiliation
.calledWithMatch( .calledWithMatch(
@ -147,7 +142,7 @@ describe('InstitutionsController', function () {
this.addAffiliation.onCall(2).callsArgWith(3, new Error('error')) this.addAffiliation.onCall(2).callsArgWith(3, new Error('error'))
this.next = error => { this.next = error => {
expect(error).to.exist expect(error).to.exist
this.getUsersByHostname.calledOnce.should.equal(true) this.getInstitutionUsersByHostname.calledOnce.should.equal(true)
return done() return done()
} }
return this.InstitutionsController.confirmDomain( return this.InstitutionsController.confirmDomain(