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

Merge pull request #14696 from overleaf/jpa-lean-mongo-queries

[web] use lean mongo queries

GitOrigin-RevId: 5c9e2cddb2c45835dd9bb87c31b6e9d2b91873fd
Jakob Ackermann 3 лет назад
Родитель
Сommit
3bc7407ba9

+ 1 - 1
services/web/app/src/Features/BetaProgram/BetaProgramController.js

@@ -37,7 +37,7 @@ const BetaProgramController = {
   optInPage(req, res, next) {
     const userId = SessionManager.getLoggedInUserId(req.session)
     logger.debug({ userId }, 'showing beta participation page for user')
-    UserGetter.getUser(userId, function (err, user) {
+    UserGetter.getUser(userId, { betaProgram: 1 }, function (err, user) {
       if (err) {
         OError.tag(err, 'error fetching user', {
           userId,

+ 1 - 1
services/web/app/src/Features/Institutions/InstitutionsManager.js

@@ -322,7 +322,7 @@ function refreshFeaturesAndNotify(affiliation, callback) {
 const getUserInfo = (userId, callback) =>
   async.waterfall(
     [
-      cb => UserGetter.getUser(userId, cb),
+      cb => UserGetter.getUser(userId, { _id: 1 }, cb),
       (user, cb) =>
         SubscriptionLocator.getUsersSubscription(user, (err, subscription) =>
           cb(err, user, subscription)

+ 7 - 4
services/web/app/src/Features/Subscription/ManagedUsersHandler.js

@@ -116,10 +116,13 @@ async function getEnrollmentForUser(requestedUser) {
 
 async function enrollInSubscription(userId, subscription) {
   // check whether the user is already enrolled in a subscription
-  const user = await User.findOne({
-    _id: userId,
-    'enrollment.managedBy': { $exists: true },
-  }).exec()
+  const user = await User.findOne(
+    {
+      _id: userId,
+      'enrollment.managedBy': { $exists: true },
+    },
+    { _id: 1 }
+  ).exec()
   if (user != null) {
     throw new OError('User is already enrolled in a subscription', {
       userId,

+ 31 - 23
services/web/app/src/Features/User/UserController.js

@@ -288,44 +288,52 @@ const UserController = {
 
   subscribe(req, res, next) {
     const userId = SessionManager.getLoggedInUserId(req.session)
-    UserGetter.getUser(userId, (err, user) => {
-      if (err != null) {
-        return next(err)
-      }
-      NewsletterManager.subscribe(user, err => {
+    UserGetter.getUser(
+      userId,
+      { _id: 1, email: 1, first_name: 1, last_name: 1 },
+      (err, user) => {
         if (err != null) {
-          OError.tag(err, 'error subscribing to newsletter')
           return next(err)
         }
-        return res.json({
-          message: req.i18n.translate('thanks_settings_updated'),
+        NewsletterManager.subscribe(user, err => {
+          if (err != null) {
+            OError.tag(err, 'error subscribing to newsletter')
+            return next(err)
+          }
+          return res.json({
+            message: req.i18n.translate('thanks_settings_updated'),
+          })
         })
-      })
-    })
+      }
+    )
   },
 
   unsubscribe(req, res, next) {
     const userId = SessionManager.getLoggedInUserId(req.session)
-    UserGetter.getUser(userId, (err, user) => {
-      if (err != null) {
-        return next(err)
-      }
-      NewsletterManager.unsubscribe(user, err => {
+    UserGetter.getUser(
+      userId,
+      { _id: 1, email: 1, first_name: 1, last_name: 1 },
+      (err, user) => {
         if (err != null) {
-          OError.tag(err, 'error unsubscribing to newsletter')
           return next(err)
         }
-        Modules.hooks.fire('newsletterUnsubscribed', user, err => {
-          if (err) {
-            OError.tag(err, 'error firing "newsletterUnsubscribed" hook')
+        NewsletterManager.unsubscribe(user, err => {
+          if (err != null) {
+            OError.tag(err, 'error unsubscribing to newsletter')
             return next(err)
           }
-          return res.json({
-            message: req.i18n.translate('thanks_settings_updated'),
+          Modules.hooks.fire('newsletterUnsubscribed', user, err => {
+            if (err) {
+              OError.tag(err, 'error firing "newsletterUnsubscribed" hook')
+              return next(err)
+            }
+            return res.json({
+              message: req.i18n.translate('thanks_settings_updated'),
+            })
           })
         })
-      })
-    })
+      }
+    )
   },
 
   updateUserSettings(req, res, next) {

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

@@ -90,7 +90,7 @@ const UserEmailsConfirmationHandler = {
         if (!userId || email !== EmailHelper.parseEmail(email)) {
           return callback(new Errors.NotFoundError('invalid data'))
         }
-        UserGetter.getUser(userId, {}, function (error, user) {
+        UserGetter.getUser(userId, { emails: 1 }, function (error, user) {
           if (error) {
             return callback(error)
           }

+ 8 - 0
services/web/app/src/Features/User/UserGetter.js

@@ -111,6 +111,14 @@ const UserGetter = {
     }
   },
 
+  getUserFeatures(userId, callback) {
+    this.getUser(userId, { features: 1 }, (error, user) => {
+      if (error) return callback(error)
+      if (!user) return callback(new Errors.NotFoundError('user not found'))
+      callback(null, user.features)
+    })
+  },
+
   getUserEmail(userId, callback) {
     this.getUser(userId, { email: 1 }, (error, user) =>
       callback(error, user && user.email)

+ 15 - 11
services/web/app/src/Features/User/UserPagesController.js

@@ -223,21 +223,25 @@ const UserPagesController = {
 
   emailPreferencesPage(req, res, next) {
     const userId = SessionManager.getLoggedInUserId(req.session)
-    UserGetter.getUser(userId, (err, user) => {
-      if (err != null) {
-        return next(err)
-      }
-      NewsletterManager.subscribed(user, (err, subscribed) => {
+    UserGetter.getUser(
+      userId,
+      { _id: 1, email: 1, first_name: 1, last_name: 1 },
+      (err, user) => {
         if (err != null) {
-          OError.tag(err, 'error getting newsletter subscription status')
           return next(err)
         }
-        res.render('user/email-preferences', {
-          title: 'newsletter_info_title',
-          subscribed,
+        NewsletterManager.subscribed(user, (err, subscribed) => {
+          if (err != null) {
+            OError.tag(err, 'error getting newsletter subscription status')
+            return next(err)
+          }
+          res.render('user/email-preferences', {
+            title: 'newsletter_info_title',
+            subscribed,
+          })
         })
-      })
-    })
+      }
+    )
   },
 
   _restructureThirdPartyIds(user) {

+ 1 - 1
services/web/modules/server-ce-scripts/scripts/delete-user.js

@@ -12,7 +12,7 @@ async function main() {
   }
 
   await new Promise((resolve, reject) => {
-    UserGetter.getUser({ email }, function (error, user) {
+    UserGetter.getUser({ email }, { _id: 1 }, function (error, user) {
       if (error) {
         return reject(error)
       }

+ 2 - 2
services/web/test/acceptance/src/ModelTests.js

@@ -9,7 +9,7 @@ describe('mongoose', function () {
 
     it('allows the creation of a user', async function () {
       await expect(User.create({ email })).to.be.fulfilled
-      await expect(User.findOne({ email })).to.eventually.exist
+      await expect(User.findOne({ email }, { _id: 1 })).to.eventually.exist
     })
 
     it('does not allow the creation of multiple users with the same email', async function () {
@@ -41,7 +41,7 @@ describe('mongoose', function () {
         })
       ).to.be.fulfilled
 
-      const user = await User.findOne({ email })
+      const user = await User.findOne({ email }, { splitTests: 1 })
       expect(user.splitTests['some-test'][0].assignedAt).to.be.a('date')
       expect(user.splitTests['some-test'][1].assignedAt).to.be.a('date')
     })

+ 2 - 2
services/web/test/unit/src/BetaProgram/BetaProgramControllerTests.js

@@ -127,7 +127,7 @@ describe('BetaProgramController', function () {
 
   describe('optInPage', function () {
     beforeEach(function () {
-      this.UserGetter.getUser.callsArgWith(1, null, this.user)
+      this.UserGetter.getUser.yields(null, this.user)
     })
 
     it('should render the opt-in page', function () {
@@ -139,7 +139,7 @@ describe('BetaProgramController', function () {
 
     describe('when UserGetter.getUser produces an error', function () {
       beforeEach(function () {
-        this.UserGetter.getUser.callsArgWith(1, new Error('woops'))
+        this.UserGetter.getUser.yields(new Error('woops'))
       })
 
       it('should not render the opt-in page', function () {

+ 5 - 13
services/web/test/unit/src/Institutions/InstitutionsManagerTests.js

@@ -45,7 +45,7 @@ describe('InstitutionsManager', function () {
 
     this.UserGetter = {
       getUsersByAnyConfirmedEmail: sinon.stub().yields(),
-      getUser: sinon.stub().callsArgWith(1, null, this.user),
+      getUser: sinon.stub().yields(null, this.user),
       promises: {
         getUsers: sinon.stub().resolves(this.users),
         getUsersByAnyConfirmedEmail: sinon.stub().resolves(),
@@ -164,18 +164,10 @@ describe('InstitutionsManager', function () {
       this.user3 = { _id: this.user3Id }
       this.user4 = { _id: this.user4Id }
 
-      this.UserGetter.getUser
-        .withArgs(this.user1Id)
-        .callsArgWith(1, null, this.user1)
-      this.UserGetter.getUser
-        .withArgs(this.user2Id)
-        .callsArgWith(1, null, this.user2)
-      this.UserGetter.getUser
-        .withArgs(this.user3Id)
-        .callsArgWith(1, null, this.user3)
-      this.UserGetter.getUser
-        .withArgs(this.user4Id)
-        .callsArgWith(1, null, this.user4)
+      this.UserGetter.getUser.withArgs(this.user1Id).yields(null, this.user1)
+      this.UserGetter.getUser.withArgs(this.user2Id).yields(null, this.user2)
+      this.UserGetter.getUser.withArgs(this.user3Id).yields(null, this.user3)
+      this.UserGetter.getUser.withArgs(this.user4Id).yields(null, this.user4)
 
       this.SubscriptionLocator.getUsersSubscription
         .withArgs(this.user2)

+ 2 - 2
services/web/test/unit/src/User/UserControllerTests.js

@@ -38,10 +38,10 @@ describe('UserController', function () {
 
     this.UserDeleter = { deleteUser: sinon.stub().yields() }
     this.UserGetter = {
-      getUser: sinon.stub().callsArgWith(1, null, this.user),
+      getUser: sinon.stub().yields(null, this.user),
       promises: { getUser: sinon.stub().resolves(this.user) },
     }
-    this.User = { findById: sinon.stub().callsArgWith(1, null, this.user) }
+    this.User = { findById: sinon.stub().yields(null, this.user) }
     this.NewsLetterManager = {
       subscribe: sinon.stub().yields(),
       unsubscribe: sinon.stub().yields(),