[misc] mongodb: change findAndModify queries to updateOne

There is no need to fetch mongo data when the call-site does not look at
 it. Also handle mongo errors in `setTTLOnArchivedPack`.

services/track-changes/app/js/PackManager.js

@@ -273,10 +273,7 @@ module.exports = PackManager = {
       'appending updates to existing pack'
     )
     Metrics.inc(`append-pack-${temporary ? 'temporary' : 'permanent'}`)
-    return db.docHistory.findAndModify(
-      { query, update, new: true, fields: { meta: 1, v_end: 1 } },
-      callback
-    )
+    return db.docHistory.updateOne(query, update, callback)
   },
 
   // Retrieve all changes for a document
@@ -497,11 +494,9 @@ module.exports = PackManager = {
   increaseTTL(pack, callback) {
     if (pack.expiresAt < new Date(Date.now() + 6 * DAYS)) {
       // update cache expiry since we are using this pack
-      return db.docHistory.findAndModify(
-        {
-          query: { _id: pack._id },
-          update: { $set: { expiresAt: new Date(Date.now() + 7 * DAYS) } }
-        },
+      return db.docHistory.updateOne(
+        { _id: pack._id },
+        { $set: { expiresAt: new Date(Date.now() + 7 * DAYS) } },
         (err) => callback(err, pack)
       )
     } else {
@@ -727,15 +722,15 @@ module.exports = PackManager = {
   },
 
   _insertPacksIntoIndex(project_id, doc_id, newPacks, callback) {
-    return db.docHistoryIndex.findAndModify(
+    return db.docHistoryIndex.updateOne(
+      { _id: ObjectId(doc_id.toString()) },
+      {
+        $setOnInsert: { project_id: ObjectId(project_id.toString()) },
+        $push: {
+          packs: { $each: newPacks, $sort: { v: 1 } }
+        }
+      },
       {
-        query: { _id: ObjectId(doc_id.toString()) },
-        update: {
-          $setOnInsert: { project_id: ObjectId(project_id.toString()) },
-          $push: {
-            packs: { $each: newPacks, $sort: { v: 1 } }
-          }
-        },
         upsert: true
       },
       callback
@@ -993,11 +988,9 @@ module.exports = PackManager = {
 
   _markPackAsFinalised(project_id, doc_id, pack_id, callback) {
     logger.log({ project_id, doc_id, pack_id }, 'marking pack as finalised')
-    return db.docHistory.findAndModify(
-      {
-        query: { _id: pack_id },
-        update: { $set: { finalised: true } }
-      },
+    return db.docHistory.updateOne(
+      { _id: pack_id },
+      { $set: { finalised: true } },
       callback
     )
   },
@@ -1018,11 +1011,9 @@ module.exports = PackManager = {
 
   markPackAsChecked(project_id, doc_id, pack_id, callback) {
     logger.log({ project_id, doc_id, pack_id }, 'marking pack as checked')
-    return db.docHistory.findAndModify(
-      {
-        query: { _id: pack_id },
-        update: { $currentDate: { last_checked: true } }
-      },
+    return db.docHistory.updateOne(
+      { _id: pack_id },
+      { $currentDate: { last_checked: true } },
       callback
     )
   },
@@ -1115,15 +1106,12 @@ module.exports = PackManager = {
       { project_id, doc_id, pack_id },
       'clearing as archive in progress'
     )
-    return db.docHistoryIndex.findAndModify(
+    return db.docHistoryIndex.updateOne(
       {
-        query: {
-          _id: ObjectId(doc_id.toString()),
-          packs: { $elemMatch: { _id: pack_id, inS3: false } }
-        },
-        fields: { 'packs.$': 1 },
-        update: { $unset: { 'packs.$.inS3': true } }
+        _id: ObjectId(doc_id.toString()),
+        packs: { $elemMatch: { _id: pack_id, inS3: false } }
       },
+      { $unset: { 'packs.$.inS3': true } },
       callback
     )
   },
@@ -1153,12 +1141,13 @@ module.exports = PackManager = {
   },
 
   setTTLOnArchivedPack(project_id, doc_id, pack_id, callback) {
-    return db.docHistory.findAndModify(
-      {
-        query: { _id: pack_id },
-        update: { $set: { expiresAt: new Date(Date.now() + 1 * DAYS) } }
-      },
+    return db.docHistory.updateOne(
+      { _id: pack_id },
+      { $set: { expiresAt: new Date(Date.now() + 1 * DAYS) } },
       function (err) {
+        if (err) {
+          return callback(err)
+        }
         logger.log({ project_id, doc_id, pack_id }, 'set expiry on pack')
         return callback()
       }

services/track-changes/test/unit/js/PackManager/PackManagerTests.js

@@ -68,6 +68,7 @@ describe('PackManager', function () {
       return (this.db.docHistory = {
         save: sinon.stub().callsArg(1),
         insert: sinon.stub().callsArg(1),
+        updateOne: sinon.stub().yields(),
         findAndModify: sinon.stub().callsArg(1)
       })
     })
@@ -443,23 +444,21 @@ describe('PackManager', function () {
 
       return describe('for a small update that will expire', function () {
         it('should append the update in mongo', function () {
-          return this.db.docHistory.findAndModify
-            .calledWithMatch({
-              query: { _id: this.lastUpdate._id },
-              update: {
+          return this.db.docHistory.updateOne
+            .calledWithMatch(
+              { _id: this.lastUpdate._id },
+              {
                 $push: { pack: { $each: this.newUpdates } },
                 $set: { v_end: this.newUpdates[this.newUpdates.length - 1].v }
               }
-            })
+            )
             .should.equal(true)
         })
 
         it('should set an expiry time in the future', function () {
-          return this.db.docHistory.findAndModify
-            .calledWithMatch({
-              update: {
-                $set: { expiresAt: new Date(Date.now() + 7 * 24 * 3600 * 1000) }
-              }
+          return this.db.docHistory.updateOne
+            .calledWithMatch(sinon.match.any, {
+              $set: { expiresAt: new Date(Date.now() + 7 * 24 * 3600 * 1000) }
             })
             .should.equal(true)
         })