Database Access optimisations (#14544)

* Optimize database access during spell casting

* load less data when casting spells

* Begin migrating update calls to updateOne and updateMany

* Only update user objects that don’t have notification yet

* fix test

* fix spy

* Don’t unnecessarily update user when requesting invalid guild

* fix sort order for middlewares to not load user twice every request

* fix tests

* fix integration test

* fix skill usage not always deducting mp

* addtest case for blessing spell

* fix healAll

* fix lint

* Fix error for when some spells are used outside of party

* Add check to not run bulk spells in web client

* fix(tags): change const to let

---------

Co-authored-by: SabreCat <sabe@habitica.com>

test/api/unit/middlewares/cronMiddleware.js

@@ -242,7 +242,7 @@ describe('cron middleware', () => {
 
     sandbox.spy(cronLib, 'recoverCron');
 
-    sandbox.stub(User, 'update')
+    sandbox.stub(User, 'updateOne')
       .withArgs({
         _id: user._id,
         $or: [

test/api/unit/models/group.test.js

@@ -1732,7 +1732,7 @@ describe('Group Model', () => {
         });
 
         it('updates participting members (not including user)', async () => {
-          sandbox.spy(User, 'update');
+          sandbox.spy(User, 'updateMany');
 
           await party.startQuest(nonParticipatingMember);
 
@@ -1740,7 +1740,7 @@ describe('Group Model', () => {
             questLeader._id, participatingMember._id, sleepingParticipatingMember._id,
           ];
 
-          expect(User.update).to.be.calledWith(
+          expect(User.updateMany).to.be.calledWith(
             { _id: { $in: members } },
             {
               $set: {
@@ -1753,11 +1753,11 @@ describe('Group Model', () => {
         });
 
         it('updates non-user quest leader and decrements quest scroll', async () => {
-          sandbox.spy(User, 'update');
+          sandbox.spy(User, 'updateOne');
 
           await party.startQuest(participatingMember);
 
-          expect(User.update).to.be.calledWith(
+          expect(User.updateOne).to.be.calledWith(
             { _id: questLeader._id },
             {
               $inc: {
@@ -1819,29 +1819,29 @@ describe('Group Model', () => {
         };
 
         it('doesn\'t retry successful operations', async () => {
-          sandbox.stub(User, 'update').returns(successfulMock);
+          sandbox.stub(User, 'updateOne').returns(successfulMock);
 
           await party.finishQuest(quest);
 
-          expect(User.update).to.be.calledThrice;
+          expect(User.updateOne).to.be.calledThrice;
         });
 
         it('stops retrying when a successful update has occurred', async () => {
-          const updateStub = sandbox.stub(User, 'update');
+          const updateStub = sandbox.stub(User, 'updateOne');
           updateStub.onCall(0).returns(failedMock);
           updateStub.returns(successfulMock);
 
           await party.finishQuest(quest);
 
-          expect(User.update.callCount).to.equal(4);
+          expect(User.updateOne.callCount).to.equal(4);
         });
 
         it('retries failed updates at most five times per user', async () => {
-          sandbox.stub(User, 'update').returns(failedMock);
+          sandbox.stub(User, 'updateOne').returns(failedMock);
 
           await expect(party.finishQuest(quest)).to.eventually.be.rejected;
 
-          expect(User.update.callCount).to.eql(15); // for 3 users
+          expect(User.updateOne.callCount).to.eql(15); // for 3 users
         });
       });
 
@@ -2088,17 +2088,17 @@ describe('Group Model', () => {
 
       context('Party quests', () => {
         it('updates participating members with rewards', async () => {
-          sandbox.spy(User, 'update');
+          sandbox.spy(User, 'updateOne');
           await party.finishQuest(quest);
 
-          expect(User.update).to.be.calledThrice;
-          expect(User.update).to.be.calledWithMatch({
+          expect(User.updateOne).to.be.calledThrice;
+          expect(User.updateOne).to.be.calledWithMatch({
             _id: questLeader._id,
           });
-          expect(User.update).to.be.calledWithMatch({
+          expect(User.updateOne).to.be.calledWithMatch({
             _id: participatingMember._id,
           });
-          expect(User.update).to.be.calledWithMatch({
+          expect(User.updateOne).to.be.calledWithMatch({
             _id: sleepingParticipatingMember._id,
           });
         });
@@ -2173,11 +2173,11 @@ describe('Group Model', () => {
         });
 
         it('updates all users with rewards', async () => {
-          sandbox.spy(User, 'update');
+          sandbox.spy(User, 'updateMany');
           await party.finishQuest(tavernQuest);
 
-          expect(User.update).to.be.calledOnce;
-          expect(User.update).to.be.calledWithMatch({});
+          expect(User.updateMany).to.be.calledOnce;
+          expect(User.updateMany).to.be.calledWithMatch({});
         });
 
         it('sets quest completed to the world quest key', async () => {

test/api/v4/user/POST-user_class_cast_spellId.test.js

@@ -202,18 +202,86 @@ describe('POST /user/class/cast/:spellId', () => {
     await group.groupLeader.post('/user/class/cast/mpheal');
 
     promises = [];
+    promises.push(group.groupLeader.sync());
     promises.push(group.members[0].sync());
     promises.push(group.members[1].sync());
     promises.push(group.members[2].sync());
     promises.push(group.members[3].sync());
     await Promise.all(promises);
 
+    expect(group.groupLeader.stats.mp).to.be.equal(170); // spell caster
     expect(group.members[0].stats.mp).to.be.greaterThan(0); // warrior
     expect(group.members[1].stats.mp).to.equal(0); // wizard
     expect(group.members[2].stats.mp).to.be.greaterThan(0); // rogue
     expect(group.members[3].stats.mp).to.be.greaterThan(0); // healer
   });
 
+  const spellList = [
+    {
+      className: 'warrior',
+      spells: [['smash', 'task'], ['defensiveStance'], ['valorousPresence'], ['intimidate']],
+    },
+    {
+      className: 'wizard',
+      spells: [['fireball', 'task'], ['mpheal'], ['earth'], ['frost']],
+    },
+    {
+      className: 'healer',
+      spells: [['heal'], ['brightness'], ['protectAura'], ['healAll']],
+    },
+    {
+      className: 'rogue',
+      spells: [['pickPocket', 'task'], ['backStab', 'task'], ['toolsOfTrade'], ['stealth']],
+    },
+  ];
+
+  spellList.forEach(async habitClass => {
+    describe(`For a ${habitClass.className}`, async () => {
+      habitClass.spells.forEach(async spell => {
+        describe(`Using ${spell[0]}`, async () => {
+          it('Deducts MP from spell caster', async () => {
+            const { groupLeader } = await createAndPopulateGroup({
+              groupDetails: { type: 'party', privacy: 'private' },
+              members: 3,
+            });
+            await groupLeader.update({
+              'stats.mp': 200, 'stats.class': habitClass.className, 'stats.lvl': 20, 'stats.hp': 40,
+            });
+            // need this for task spells and for stealth
+            const task = await groupLeader.post('/tasks/user', {
+              text: 'test habit',
+              type: 'daily',
+            });
+            if (spell.length === 2 && spell[1] === 'task') {
+              await groupLeader.post(`/user/class/cast/${spell[0]}?targetId=${task._id}`);
+            } else {
+              await groupLeader.post(`/user/class/cast/${spell[0]}`);
+            }
+            await groupLeader.sync();
+            expect(groupLeader.stats.mp).to.be.lessThan(200);
+          });
+          it('works without a party', async () => {
+            await user.update({
+              'stats.mp': 200, 'stats.class': habitClass.className, 'stats.lvl': 20, 'stats.hp': 40,
+            });
+            // need this for task spells and for stealth
+            const task = await user.post('/tasks/user', {
+              text: 'test habit',
+              type: 'daily',
+            });
+            if (spell.length === 2 && spell[1] === 'task') {
+              await user.post(`/user/class/cast/${spell[0]}?targetId=${task._id}`);
+            } else {
+              await user.post(`/user/class/cast/${spell[0]}`);
+            }
+            await user.sync();
+            expect(user.stats.mp).to.be.lessThan(200);
+          });
+        });
+      });
+    });
+  });
+
   it('cast bulk', async () => {
     let { group, groupLeader } = await createAndPopulateGroup({ // eslint-disable-line prefer-const
       groupDetails: { type: 'party', privacy: 'private' },

website/client/src/mixins/spells.js

@@ -114,7 +114,7 @@ export default {
       this.castCancel();
 
       // the selected member doesn't have the flags property which sets `cardReceived`
-      if (spell.pinType !== 'card') {
+      if (spell.pinType !== 'card' && spell.bulk !== true) {
         try {
           spell.cast(this.user, target, {});
         } catch (e) {

website/common/script/content/spells.js

@@ -77,13 +77,11 @@ spells.wizard = {
     lvl: 12,
     target: 'party',
     notes: t('spellWizardMPHealNotes'),
-    cast (user, target) {
-      each(target, member => {
+    bulk: true,
+    cast (user, data) {
       const bonus = statsComputed(user).int;
-        if (user._id !== member._id && member.stats.class !== 'wizard') {
-          member.stats.mp += Math.ceil(diminishingReturns(bonus, 25, 125));
-        }
-      });
+      data.query['stats.class'] = { $ne: 'wizard' };
+      data.update = { $inc: { 'stats.mp': Math.ceil(diminishingReturns(bonus, 25, 125)) } };
     },
   },
   earth: { // Earthquake
@@ -92,12 +90,10 @@ spells.wizard = {
     lvl: 13,
     target: 'party',
     notes: t('spellWizardEarthNotes'),
-    cast (user, target) {
-      each(target, member => {
+    bulk: true,
+    cast (user, data) {
       const bonus = statsComputed(user).int - user.stats.buffs.int;
-        if (!member.stats.buffs.int) member.stats.buffs.int = 0;
-        member.stats.buffs.int += Math.ceil(diminishingReturns(bonus, 30, 200));
-      });
+      data.update = { $inc: { 'stats.buffs.int': Math.ceil(diminishingReturns(bonus, 30, 200)) } };
     },
   },
   frost: { // Chilling Frost
@@ -147,12 +143,10 @@ spells.warrior = {
     lvl: 13,
     target: 'party',
     notes: t('spellWarriorValorousPresenceNotes'),
-    cast (user, target) {
-      each(target, member => {
+    bulk: true,
+    cast (user, data) {
       const bonus = statsComputed(user).str - user.stats.buffs.str;
-        if (!member.stats.buffs.str) member.stats.buffs.str = 0;
-        member.stats.buffs.str += Math.ceil(diminishingReturns(bonus, 20, 200));
-      });
+      data.update = { $inc: { 'stats.buffs.str': Math.ceil(diminishingReturns(bonus, 20, 200)) } };
     },
   },
   intimidate: { // Intimidating Gaze
@@ -161,12 +155,10 @@ spells.warrior = {
     lvl: 14,
     target: 'party',
     notes: t('spellWarriorIntimidateNotes'),
-    cast (user, target) {
-      each(target, member => {
+    bulk: true,
+    cast (user, data) {
       const bonus = statsComputed(user).con - user.stats.buffs.con;
-        if (!member.stats.buffs.con) member.stats.buffs.con = 0;
-        member.stats.buffs.con += Math.ceil(diminishingReturns(bonus, 24, 200));
-      });
+      data.update = { $inc: { 'stats.buffs.con': Math.ceil(diminishingReturns(bonus, 24, 200)) } };
     },
   },
 };
@@ -203,12 +195,10 @@ spells.rogue = {
     lvl: 13,
     target: 'party',
     notes: t('spellRogueToolsOfTradeNotes'),
-    cast (user, target) {
-      each(target, member => {
+    bulk: true,
+    cast (user, data) {
       const bonus = statsComputed(user).per - user.stats.buffs.per;
-        if (!member.stats.buffs.per) member.stats.buffs.per = 0;
-        member.stats.buffs.per += Math.ceil(diminishingReturns(bonus, 100, 50));
-      });
+      data.update = { $inc: { 'stats.buffs.per': Math.ceil(diminishingReturns(bonus, 100, 50)) } };
     },
   },
   stealth: { // Stealth
@@ -257,12 +247,10 @@ spells.healer = {
     lvl: 13,
     target: 'party',
     notes: t('spellHealerProtectAuraNotes'),
-    cast (user, target) {
-      each(target, member => {
+    bulk: true,
+    cast (user, data) {
       const bonus = statsComputed(user).con - user.stats.buffs.con;
-        if (!member.stats.buffs.con) member.stats.buffs.con = 0;
-        member.stats.buffs.con += Math.ceil(diminishingReturns(bonus, 200, 200));
-      });
+      data.update = { $inc: { 'stats.buffs.con': Math.ceil(diminishingReturns(bonus, 200, 200)) } };
     },
   },
   healAll: { // Blessing

website/server/controllers/api-v3/quests.js

@@ -93,7 +93,7 @@ api.inviteToQuest = {
     user.party.quest.RSVPNeeded = false;
     user.party.quest.key = questKey;
 
-    await User.update({
+    await User.updateMany({
       'party._id': group._id,
       _id: { $ne: user._id },
     }, {
@@ -101,7 +101,7 @@ api.inviteToQuest = {
         'party.quest.RSVPNeeded': true,
         'party.quest.key': questKey,
       },
-    }, { multi: true }).exec();
+    }).exec();
 
     _.each(members, member => {
       group.quest.members[member._id] = null;
@@ -409,10 +409,9 @@ api.cancelQuest = {
     const [savedGroup] = await Promise.all([
       group.save(),
       newChatMessage.save(),
-      User.update(
+      User.updateMany(
         { 'party._id': groupId },
         Group.cleanQuestParty(),
-        { multi: true },
       ).exec(),
     ]);
 
@@ -467,12 +466,11 @@ api.abortQuest = {
     });
     await newChatMessage.save();
 
-    const memberUpdates = User.update({
+    const memberUpdates = User.updateMany({
       'party._id': groupId,
-    }, Group.cleanQuestParty(),
-    { multi: true }).exec();
+    }, Group.cleanQuestParty()).exec();
 
-    const questLeaderUpdate = User.update({
+    const questLeaderUpdate = User.updateOne({
       _id: group.quest.leader,
     }, {
       $inc: {

website/server/controllers/api-v3/tags.js

@@ -227,7 +227,7 @@ api.deleteTag = {
     const tagFound = find(user.tags, tag => tag.id === req.params.tagId);
     if (!tagFound) throw new NotFound(res.t('tagNotFound'));
 
-    await user.update({
+    await user.updateOne({
       $pull: { tags: { id: tagFound.id } },
     }).exec();
 
@@ -237,13 +237,13 @@ api.deleteTag = {
     user._v += 1;
 
     // Remove from all the tasks TODO test
-    await Tasks.Task.update({
+    await Tasks.Task.updateMany({
       userId: user._id,
     }, {
       $pull: {
         tags: tagFound.id,
       },
-    }, { multi: true }).exec();
+    }).exec();
 
     res.respond(200, {});
   },

website/server/controllers/api-v3/tasks.js

@@ -840,7 +840,7 @@ api.moveTask = {
     // Cannot send $pull and $push on same field in one single op
     const pullQuery = { $pull: {} };
     pullQuery.$pull[`tasksOrder.${task.type}s`] = task.id;
-    await owner.update(pullQuery).exec();
+    await owner.updateOne(pullQuery).exec();
 
     let position = to;
     if (to === -1) position = order.length - 1; // push to bottom
@@ -850,7 +850,7 @@ api.moveTask = {
       $each: [task._id],
       $position: position,
     };
-    await owner.update(updateQuery).exec();
+    await owner.updateOne(updateQuery).exec();
 
     // Update the user version field manually,
     // it cannot be updated in the pre update hook
@@ -1434,7 +1434,7 @@ api.deleteTask = {
 
       const pullQuery = { $pull: {} };
       pullQuery.$pull[`tasksOrder.${task.type}s`] = task._id;
-      const taskOrderUpdate = (challenge || user).update(pullQuery).exec();
+      const taskOrderUpdate = (challenge || user).updateOne(pullQuery).exec();
 
       // Update the user version field manually,
       // it cannot be updated in the pre update hook

website/server/libs/baseModel.js

@@ -37,7 +37,15 @@ export default function baseModel (schema, options = {}) {
     });
 
     schema.pre('update', function preUpdateModel () {
-      this.update({}, { $set: { updatedAt: new Date() } });
+      this.set({}, { $set: { updatedAt: new Date() } });
+    });
+
+    schema.pre('updateOne', function preUpdateModel () {
+      this.set({}, { $set: { updatedAt: new Date() } });
+    });
+
+    schema.pre('updateMany', function preUpdateModel () {
+      this.set({}, { $set: { updatedAt: new Date() } });
     });
   }
 

website/server/libs/pushNotifications.js

@@ -21,7 +21,7 @@ const apnProvider = APN_ENABLED ? new apn.Provider({
 }) : undefined;
 
 function removePushDevice (user, pushDevice) {
-  return User.update({ _id: user._id }, {
+  return User.updateOne({ _id: user._id }, {
     $pull: { pushDevices: { regId: pushDevice.regId } },
   }).exec().catch(err => {
     logger.error(err, `Error removing pushDevice ${pushDevice.regId} for user ${user._id}`);

website/server/libs/routes.js

@@ -24,7 +24,7 @@ export function readController (router, controller, overrides = []) {
 
     // If an authentication middleware is used run getUserLanguage after it, otherwise before
     // for cron instead use it only if an authentication middleware is present
-    const authMiddlewareIndex = _.findIndex(middlewares, middleware => {
+    let authMiddlewareIndex = _.findIndex(middlewares, middleware => {
       if (middleware.name.indexOf('authWith') === 0) { // authWith{Headers|Session|Url|...}
         return true;
       }
@@ -36,6 +36,7 @@ export function readController (router, controller, overrides = []) {
     // disable caching for all routes with mandatory or optional authentication
     if (authMiddlewareIndex !== -1) {
       middlewares.unshift(disableCache);
+      authMiddlewareIndex += 1;
     }
 
     if (action.noLanguage !== true) { // unless getting the language is explictly disabled

website/server/libs/spells.js

@@ -11,7 +11,7 @@ import {
 } from '../models/group';
 import apiError from './apiError';
 
-const partyMembersFields = 'profile.name stats achievements items.special notifications flags pinnedItems';
+const partyMembersFields = 'profile.name stats achievements items.special pinnedItems notifications flags';
 // Excluding notifications and flags from the list of public fields to return.
 const partyMembersPublicFields = 'profile.name stats achievements items.special';
 
@@ -74,12 +74,13 @@ async function castSelfSpell (req, user, spell, quantity = 1) {
   await user.save();
 }
 
-async function castPartySpell (req, party, partyMembers, user, spell, quantity = 1) {
+async function getPartyMembers (user, party) {
+  let partyMembers;
   if (!party) {
     // Act as solo party
-    partyMembers = [user]; // eslint-disable-line no-param-reassign
+    partyMembers = [user];
   } else {
-    partyMembers = await User // eslint-disable-line no-param-reassign
+    partyMembers = await User
       .find({
         'party._id': party._id,
         _id: { $ne: user._id }, // add separately
@@ -89,22 +90,40 @@ async function castPartySpell (req, party, partyMembers, user, spell, quantity =
 
     partyMembers.unshift(user);
   }
+  return partyMembers;
+}
 
+async function castPartySpell (req, party, user, spell, quantity = 1) {
+  let partyMembers;
+  if (spell.bulk) {
+    const data = { };
+    if (party) {
+      data.query = { 'party._id': party._id };
+    } else {
+      data.query = { _id: user._id };
+    }
+    spell.cast(user, data);
+    await User.updateMany(data.query, data.update);
+    await user.save();
+    partyMembers = await getPartyMembers(user, party);
+  } else {
+    partyMembers = await getPartyMembers(user, party);
     for (let i = 0; i < quantity; i += 1) {
       spell.cast(user, partyMembers, req);
     }
     await Promise.all(partyMembers.map(m => m.save()));
-
+  }
   return partyMembers;
 }
 
-async function castUserSpell (res, req, party, partyMembers, targetId, user, spell, quantity = 1) {
+async function castUserSpell (res, req, party, targetId, user, spell, quantity = 1) {
+  let partyMembers;
   if (!party && (!targetId || user._id === targetId)) {
-    partyMembers = user; // eslint-disable-line no-param-reassign
+    partyMembers = user;
   } else {
     if (!targetId) throw new BadRequest(res.t('targetIdUUID'));
     if (!party) throw new NotFound(res.t('partyNotFound'));
-    partyMembers = await User // eslint-disable-line no-param-reassign
+    partyMembers = await User
       .findOne({ _id: targetId, 'party._id': party._id })
       .select(partyMembersFields)
       .exec();
@@ -195,10 +214,10 @@ async function castSpell (req, res, { isV3 = false }) {
     let partyMembers;
 
     if (targetType === 'party') {
-      partyMembers = await castPartySpell(req, party, partyMembers, user, spell, quantity);
+      partyMembers = await castPartySpell(req, party, user, spell, quantity);
     } else {
       partyMembers = await castUserSpell(
-        res, req, party, partyMembers,
+        res, req, party,
         targetId, user, spell, quantity,
       );
     }

website/server/libs/tasks/index.js

@@ -114,7 +114,7 @@ async function createTasks (req, res, options = {}) {
     };
   }
 
-  await owner.update(taskOrderUpdateQuery).exec();
+  await owner.updateOne(taskOrderUpdateQuery).exec();
 
   // tasks with aliases need to be validated asynchronously
   await validateTaskAlias(toSave, res);

website/server/libs/user/index.js

@@ -121,7 +121,7 @@ async function checkNewInputForProfanity (user, res, newValue) {
 export async function update (req, res, { isV3 = false }) {
   const { user } = res.locals;
 
-  const promisesForTagsRemoval = [];
+  let promisesForTagsRemoval = [];
 
   if (req.body['party.seeking'] !== undefined && req.body['party.seeking'] !== null) {
     user.invitations.party = {};
@@ -218,13 +218,13 @@ export async function update (req, res, { isV3 = false }) {
       // Remove from all the tasks
       // NOTE each tag to remove requires a query
 
-      promisesForTagsRemoval.push(removedTagsIds.map(tagId => Tasks.Task.update({
+      promisesForTagsRemoval = removedTagsIds.map(tagId => Tasks.Task.updateMany({
         userId: user._id,
       }, {
         $pull: {
           tags: tagId,
         },
-      }, { multi: true }).exec()));
+      }).exec());
     } else if (key === 'flags.newStuff' && val === false) {
       // flags.newStuff was removed from the user schema and is only returned for compatibility
       // reasons but we're keeping the ability to set it in API v3

website/server/libs/webhook.js

@@ -61,7 +61,7 @@ function sendWebhook (webhook, body, user) {
       };
     }
 
-    return User.update({
+    return User.updateOne({
       _id: user._id,
       'webhooks.id': webhook.id,
     }, update).exec();

website/server/middlewares/cron.js

@@ -16,7 +16,7 @@ async function checkForActiveCron (user, now) {
 
   // To avoid double cron we first set _cronSignature
   // and then check that it's not changed while processing
-  const userUpdateResult = await User.update({
+  const userUpdateResult = await User.updateOne({
     _id: user._id,
     $or: [ // Make sure last cron was successful or failed before cronRetryTime
       { _cronSignature: 'NOT_RUNNING' },
@@ -36,7 +36,7 @@ async function checkForActiveCron (user, now) {
 }
 
 async function updateLastCron (user, now) {
-  await User.update({
+  await User.updateOne({
     _id: user._id,
   }, {
     lastCron: now, // setting lastCron now so we don't risk re-running parts of cron if it fails
@@ -44,7 +44,7 @@ async function updateLastCron (user, now) {
 }
 
 async function unlockUser (user) {
-  await User.update({
+  await User.updateOne({
     _id: user._id,
   }, {
     _cronSignature: 'NOT_RUNNING',
@@ -125,7 +125,7 @@ async function cronAsync (req, res) {
     await Group.processQuestProgress(user, progress);
 
     // Set _cronSignature, lastCron and auth.timestamps.loggedin to signal end of cron
-    await User.update({
+    await User.updateOne({
       _id: user._id,
     }, {
       $set: {
@@ -153,7 +153,7 @@ async function cronAsync (req, res) {
       // For any other error make sure to reset _cronSignature
       // so that it doesn't prevent cron from running
       // at the next request
-      await User.update({
+      await User.updateOne({
         _id: user._id,
       }, {
         _cronSignature: 'NOT_RUNNING',

website/server/models/challenge.js

@@ -102,7 +102,7 @@ schema.methods.addToUser = async function addChallengeToUser (user) {
   // Add challenge to users challenges atomically (with a condition that checks that it
   // is not there already) to prevent multiple concurrent requests from passing through
   // see https://github.com/HabitRPG/habitica/issues/11295
-  const result = await User.update(
+  const result = await User.updateOne(
     {
       _id: user._id,
       challenges: { $nin: [this._id] },
@@ -249,7 +249,7 @@ async function _addTaskFn (challenge, tasks, memberId) {
     },
   };
   const updateUserParams = { ...updateTasksOrderQ, ...addToChallengeTagSet };
-  toSave.unshift(User.update({ _id: memberId }, updateUserParams).exec());
+  toSave.unshift(User.updateOne({ _id: memberId }, updateUserParams).exec());
 
   return Promise.all(toSave);
 }
@@ -278,11 +278,11 @@ schema.methods.updateTask = async function challengeUpdateTask (task) {
   const taskSchema = Tasks[task.type];
   // Updating instead of loading and saving for performances,
   // risks becoming a problem if we introduce more complexity in tasks
-  await taskSchema.update({
+  await taskSchema.updateMany({
     userId: { $exists: true },
     'challenge.id': challenge.id,
     'challenge.taskId': task._id,
-  }, updateCmd, { multi: true }).exec();
+  }, updateCmd).exec();
 };
 
 // Remove a task from challenge members
@@ -290,13 +290,13 @@ schema.methods.removeTask = async function challengeRemoveTask (task) {
   const challenge = this;
 
   // Set the task as broken
-  await Tasks.Task.update({
+  await Tasks.Task.updateMany({
     userId: { $exists: true },
     'challenge.id': challenge.id,
     'challenge.taskId': task._id,
   }, {
     $set: { 'challenge.broken': 'TASK_DELETED' },
-  }, { multi: true }).exec();
+  }).exec();
 };
 
 // Unlink challenges tasks (and the challenge itself) from user. TODO rename to 'leave'
@@ -311,9 +311,9 @@ schema.methods.unlinkTasks = async function challengeUnlinkTasks (user, keep, sa
   this.memberCount -= 1;
 
   if (keep === 'keep-all') {
-    await Tasks.Task.update(findQuery, {
+    await Tasks.Task.updateMany(findQuery, {
       $set: { challenge: {} },
-    }, { multi: true }).exec();
+    }).exec();
 
     const promises = [this.save()];
 
@@ -356,11 +356,12 @@ schema.methods.closeChal = async function closeChal (broken = {}) {
 
   // Refund the leader if the challenge is deleted (no winner chosen)
   if (brokenReason === 'CHALLENGE_DELETED') {
-    await User.update({ _id: challenge.leader }, { $inc: { balance: challenge.prize / 4 } }).exec();
+    await User.updateOne({ _id: challenge.leader }, { $inc: { balance: challenge.prize / 4 } })
+      .exec();
   }
 
   // Update the challengeCount on the group
-  await Group.update({ _id: challenge.group }, { $inc: { challengeCount: -1 } }).exec();
+  await Group.updateOne({ _id: challenge.group }, { $inc: { challengeCount: -1 } }).exec();
 
   // Award prize to winner and notify
   if (winner) {
@@ -370,7 +371,7 @@ schema.methods.closeChal = async function closeChal (broken = {}) {
     // reimburse the leader
     const winnerCanGetGems = await winner.canGetGems();
     if (!winnerCanGetGems) {
-      await User.update(
+      await User.updateOne(
         { _id: challenge.leader },
         { $inc: { balance: challenge.prize / 4 } },
       ).exec();
@@ -408,22 +409,22 @@ schema.methods.closeChal = async function closeChal (broken = {}) {
     Tasks.Task.remove({ 'challenge.id': challenge._id, userId: { $exists: false } }).exec(),
     // Set the challenge tag to non-challenge status
     // and remove the challenge from the user's challenges
-    User.update({
+    User.updateMany({
       challenges: challenge._id,
       'tags.id': challenge._id,
     }, {
       $set: { 'tags.$.challenge': false },
       $pull: { challenges: challenge._id },
-    }, { multi: true }).exec(),
+    }).exec(),
     // Break users' tasks
-    Tasks.Task.update({
+    Tasks.Task.updateMany({
       'challenge.id': challenge._id,
     }, {
       $set: {
         'challenge.broken': brokenReason,
         'challenge.winner': winner && winner.profile.name,
       },
-    }, { multi: true }).exec(),
+    }).exec(),
   ];
 
   Promise.all(backgroundTasks);

website/server/models/group.js

@@ -268,10 +268,13 @@ schema.statics.getGroup = async function getGroup (options = {}) {
     if (groupId === user.party._id) {
       // reset party object to default state
       user.party = {};
-    } else {
-      removeFromArray(user.guilds, groupId);
-    }
       await user.save();
+    } else {
+      const item = removeFromArray(user.guilds, groupId);
+      if (item) {
+        await user.save();
+      }
+    }
   }
 
   return group;
@@ -659,7 +662,7 @@ schema.methods.handleQuestInvitation = async function handleQuestInvitation (use
   // to prevent multiple concurrent requests overriding updates
   // see https://github.com/HabitRPG/habitica/issues/11398
   const Group = this.constructor;
-  const result = await Group.update(
+  const result = await Group.updateOne(
     {
       _id: this._id,
       [`quest.members.${user._id}`]: { $type: 10 }, // match BSON Type Null (type number 10)
@@ -707,7 +710,7 @@ schema.methods.startQuest = async function startQuest (user) {
 
   // Persist quest.members early to avoid simultaneous handling of accept/reject
   // while processing the rest of this script
-  await this.update({ $set: { 'quest.members': this.quest.members } }).exec();
+  await this.updateOne({ $set: { 'quest.members': this.quest.members } }).exec();
 
   const nonUserQuestMembers = _.keys(this.quest.members);
   removeFromArray(nonUserQuestMembers, user._id);
@@ -747,7 +750,7 @@ schema.methods.startQuest = async function startQuest (user) {
     user.markModified('items.quests');
     promises.push(user.save());
   } else { // another user is starting the quest, update the leader separately
-    promises.push(User.update({ _id: this.quest.leader }, {
+    promises.push(User.updateOne({ _id: this.quest.leader }, {
       $inc: {
         [`items.quests.${this.quest.key}`]: -1,
       },
@@ -755,7 +758,7 @@ schema.methods.startQuest = async function startQuest (user) {
   }
 
   // update the remaining users
-  promises.push(User.update({
+  promises.push(User.updateMany({
     _id: { $in: nonUserQuestMembers },
   }, {
     $set: {
@@ -763,16 +766,15 @@ schema.methods.startQuest = async function startQuest (user) {
       'party.quest.progress.down': 0,
       'party.quest.completed': null,
     },
-  }, { multi: true }).exec());
+  }).exec());
 
   await Promise.all(promises);
 
   // update the users who are not participating
   // Do not block updates
-  User.update({
+  User.updateMany({
     _id: { $in: nonMembers },
-  }, _cleanQuestParty(),
-  { multi: true }).exec();
+  }, _cleanQuestParty()).exec();
 
   const newMessage = this.sendChat({
     message: `\`${shared.i18n.t('chatQuestStarted', { questName: quest.text('en') }, 'en')}\``,
@@ -903,7 +905,7 @@ function _getUserUpdateForQuestReward (itemToAward, allAwardedItems) {
 async function _updateUserWithRetries (userId, updates, numTry = 1, query = {}) {
   query._id = userId;
   try {
-    return await User.update(query, updates).exec();
+    return await User.updateOne(query, updates).exec();
   } catch (err) {
     if (numTry < MAX_UPDATE_RETRIES) {
       numTry += 1; // eslint-disable-line no-param-reassign
@@ -949,7 +951,7 @@ schema.methods.finishQuest = async function finishQuest (quest) {
   this.markModified('quest');
 
   if (this._id === TAVERN_ID) {
-    return User.update({}, updates, { multi: true }).exec();
+    return User.updateMany({}, updates).exec();
   }
 
   const promises = participants.map(userId => {
@@ -1389,10 +1391,10 @@ schema.methods.leave = async function leaveGroup (user, keep = 'keep-all', keepC
   const userUpdate = { $pull: { 'preferences.tasks.mirrorGroupTasks': group._id } };
   if (group.type === 'guild') {
     userUpdate.$pull.guilds = group._id;
-    promises.push(User.update({ _id: user._id }, userUpdate).exec());
+    promises.push(User.updateOne({ _id: user._id }, userUpdate).exec());
   } else {
     userUpdate.$set = { party: {} };
-    promises.push(User.update({ _id: user._id }, userUpdate).exec());
+    promises.push(User.updateOne({ _id: user._id }, userUpdate).exec());
 
     update.$unset = { [`quest.members.${user._id}`]: 1 };
   }
@@ -1508,7 +1510,7 @@ schema.methods.unlinkTask = async function groupUnlinkTask (
   const promises = [unlinkingTask.save()];
 
   if (keep === 'keep-all') {
-    await Tasks.Task.update(findQuery, {
+    await Tasks.Task.updateOne(findQuery, {
       $set: { group: {} },
     }).exec();
 

website/server/models/user/hooks.js

@@ -392,6 +392,13 @@ schema.pre('update', function preUpdateUser () {
   this.update({}, { $inc: { _v: 1 } });
 });
 
+schema.pre('updateOne', function preUpdateUser () {
+  this.updateOne({}, { $inc: { _v: 1 } });
+});
+schema.pre('updateMany', function preUpdateUser () {
+  this.updateMany({}, { $inc: { _v: 1 } });
+});
+
 schema.post('save', function postSaveUser () {
   // Send a webhook notification when the user has leveled up
   if (this._tmp && this._tmp.leveledUp && this._tmp.leveledUp.length > 0) {

website/server/models/user/methods.js

@@ -225,10 +225,9 @@ schema.statics.pushNotification = async function pushNotification (
     throw validationResult;
   }
 
-  await this.update(
+  await this.updateMany(
     query,
     { $push: { notifications: newNotification.toObject() } },
-    { multi: true },
   ).exec();
 };
 
@@ -274,13 +273,12 @@ schema.statics.addAchievementUpdate = async function addAchievementUpdate (query
   const validationResult = newNotification.validateSync();
   if (validationResult) throw validationResult;
 
-  await this.update(
+  await this.updateMany(
     query,
     {
       $push: { notifications: newNotification.toObject() },
       $set: { [`achievements.${achievement}`]: true },
     },
-    { multi: true },
   ).exec();
 };