Merge pull request #66 from overleaf/jpa-mongodb-native

[misc] migrate to the native mongo driver

services/docstore/app.js

@@ -10,6 +10,7 @@ const Settings = require('settings-sharelatex')
 const logger = require('logger-sharelatex')
 const express = require('express')
 const bodyParser = require('body-parser')
+const mongodb = require('./app/js/mongodb')
 const Errors = require('./app/js/Errors')
 const HttpController = require('./app/js/HttpController')
 
@@ -76,12 +77,21 @@ const { host } = Settings.internal.docstore
 
 if (!module.parent) {
   // Called directly
-  app.listen(port, host, function (error) {
-    if (error != null) {
-      throw error
-    }
-    return logger.info(`Docstore starting up, listening on ${host}:${port}`)
-  })
+  mongodb
+    .waitForDb()
+    .then(() => {
+      app.listen(port, host, function (err) {
+        if (err) {
+          logger.fatal({ err }, `Cannot bind to ${host}:${port}. Exiting.`)
+          process.exit(1)
+        }
+        return logger.info(`Docstore starting up, listening on ${host}:${port}`)
+      })
+    })
+    .catch((err) => {
+      logger.fatal({ err }, 'Cannot connect to mongo. Exiting.')
+      process.exit(1)
+    })
 }
 
 module.exports = app

services/docstore/app/js/HealthChecker.js

@@ -10,7 +10,7 @@
  * DS207: Consider shorter variations of null checks
  * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
  */
-const { db, ObjectId } = require('./mongojs')
+const { getCollection, ObjectId } = require('./mongodb')
 const request = require('request')
 const async = require('async')
 const _ = require('underscore')
@@ -19,6 +19,9 @@ const settings = require('settings-sharelatex')
 const { port } = settings.internal.docstore
 const logger = require('logger-sharelatex')
 
+const docsCollectionPromise = getCollection('docs')
+const docOpsCollectionPromise = getCollection('docOps')
+
 module.exports = {
   check(callback) {
     const doc_id = ObjectId()
@@ -60,8 +63,14 @@ module.exports = {
           }
         })
       },
-      (cb) => db.docs.remove({ _id: doc_id, project_id }, cb),
-      (cb) => db.docOps.remove({ doc_id }, cb)
+      (cb) =>
+        docsCollectionPromise.then((docs) =>
+          docs.deleteOne({ _id: doc_id, project_id }, cb)
+        ),
+      (cb) =>
+        docOpsCollectionPromise.then((docOps) =>
+          docOps.deleteOne({ doc_id }, cb)
+        )
     ]
     return async.series(jobs, callback)
   }

services/docstore/app/js/MongoManager.js

@@ -11,28 +11,28 @@
  * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
  */
 let MongoManager
-const { db, ObjectId } = require('./mongojs')
+const { getCollection, ObjectId } = require('./mongodb')
 const logger = require('logger-sharelatex')
 const metrics = require('metrics-sharelatex')
 const { promisify } = require('util')
 
+const docsCollectionPromise = getCollection('docs')
+const docOpsCollectionPromise = getCollection('docOps')
+
 module.exports = MongoManager = {
   findDoc(project_id, doc_id, filter, callback) {
     if (callback == null) {
       callback = function (error, doc) {}
     }
-    return db.docs.find(
-      {
-        _id: ObjectId(doc_id.toString()),
-        project_id: ObjectId(project_id.toString())
-      },
-      filter,
-      function (error, docs) {
-        if (docs == null) {
-          docs = []
-        }
-        return callback(error, docs[0])
-      }
+    docsCollectionPromise.then((docs) =>
+      docs.findOne(
+        {
+          _id: ObjectId(doc_id.toString()),
+          project_id: ObjectId(project_id.toString())
+        },
+        filter,
+        callback
+      )
     )
   },
 
@@ -44,7 +44,9 @@ module.exports = MongoManager = {
     if (!options.include_deleted) {
       query.deleted = { $ne: true }
     }
-    return db.docs.find(query, filter, callback)
+    docsCollectionPromise.then((docs) =>
+      docs.find(query, filter).toArray(callback)
+    )
   },
 
   getArchivedProjectDocs(project_id, callback) {
@@ -52,7 +54,7 @@ module.exports = MongoManager = {
       project_id: ObjectId(project_id.toString()),
       inS3: true
     }
-    return db.docs.find(query, {}, callback)
+    docsCollectionPromise.then((docs) => docs.find(query).toArray(callback))
   },
 
   upsertIntoDocCollection(project_id, doc_id, updates, callback) {
@@ -66,24 +68,28 @@ module.exports = MongoManager = {
       }
     }
     update.$set.project_id = ObjectId(project_id)
-    return db.docs.update(
-      { _id: ObjectId(doc_id) },
-      update,
-      { upsert: true },
-      callback
+    docsCollectionPromise.then((docs) =>
+      docs.updateOne(
+        { _id: ObjectId(doc_id) },
+        update,
+        { upsert: true },
+        callback
+      )
     )
   },
 
   markDocAsDeleted(project_id, doc_id, callback) {
-    return db.docs.update(
-      {
-        _id: ObjectId(doc_id),
-        project_id: ObjectId(project_id)
-      },
-      {
-        $set: { deleted: true }
-      },
-      callback
+    docsCollectionPromise.then((docs) =>
+      docs.updateOne(
+        {
+          _id: ObjectId(doc_id),
+          project_id: ObjectId(project_id)
+        },
+        {
+          $set: { deleted: true }
+        },
+        callback
+      )
     )
   },
 
@@ -99,30 +105,30 @@ module.exports = MongoManager = {
       _id: doc_id,
       rev
     }
-    return db.docs.update(query, update, (err) => callback(err))
+    docsCollectionPromise.then((docs) =>
+      docs.updateOne(query, update, callback)
+    )
   },
 
   getDocVersion(doc_id, callback) {
     if (callback == null) {
       callback = function (error, version) {}
     }
-    return db.docOps.find(
-      {
-        doc_id: ObjectId(doc_id)
-      },
-      {
-        version: 1
-      },
-      function (error, docs) {
-        if (error != null) {
-          return callback(error)
-        }
-        if (docs.length < 1 || docs[0].version == null) {
-          return callback(null, 0)
-        } else {
-          return callback(null, docs[0].version)
+    docOpsCollectionPromise.then((docOps) =>
+      docOps.findOne(
+        {
+          doc_id: ObjectId(doc_id)
+        },
+        {
+          version: 1
+        },
+        function (error, doc) {
+          if (error != null) {
+            return callback(error)
+          }
+          callback(null, (doc && doc.version) || 0)
         }
-      }
+      )
     )
   },
 
@@ -130,36 +136,42 @@ module.exports = MongoManager = {
     if (callback == null) {
       callback = function (error) {}
     }
-    return db.docOps.update(
-      {
-        doc_id: ObjectId(doc_id)
-      },
-      {
-        $set: { version }
-      },
-      {
-        upsert: true
-      },
-      callback
+    docOpsCollectionPromise.then((docOps) =>
+      docOps.updateOne(
+        {
+          doc_id: ObjectId(doc_id)
+        },
+        {
+          $set: { version }
+        },
+        {
+          upsert: true
+        },
+        callback
+      )
     )
   },
 
   destroyDoc(doc_id, callback) {
-    return db.docs.remove(
-      {
-        _id: ObjectId(doc_id)
-      },
-      function (err) {
-        if (err != null) {
-          return callback(err)
+    docsCollectionPromise.then((docs) =>
+      docs.deleteOne(
+        {
+          _id: ObjectId(doc_id)
+        },
+        function (err) {
+          if (err != null) {
+            return callback(err)
+          }
+          docOpsCollectionPromise.then((docOps) =>
+            docOps.deleteOne(
+              {
+                doc_id: ObjectId(doc_id)
+              },
+              callback
+            )
+          )
         }
-        return db.docOps.remove(
-          {
-            doc_id: ObjectId(doc_id)
-          },
-          callback
-        )
-      }
+      )
     )
   }
 }

services/docstore/app/js/RangeManager.js

@@ -13,7 +13,7 @@
  */
 let RangeManager
 const _ = require('underscore')
-const { ObjectId } = require('./mongojs')
+const { ObjectId } = require('./mongodb')
 
 module.exports = RangeManager = {
   shouldUpdateRanges(doc_ranges, incoming_ranges) {

services/docstore/app/js/mongodb.js

@@ -0,0 +1,19 @@
+const Settings = require('settings-sharelatex')
+const { MongoClient, ObjectId } = require('mongodb')
+
+const clientPromise = MongoClient.connect(Settings.mongo.url)
+const dbPromise = clientPromise.then((client) => client.db())
+
+async function getCollection(name) {
+  return (await dbPromise).collection(name)
+}
+
+async function waitForDb() {
+  await clientPromise
+}
+
+module.exports = {
+  ObjectId,
+  getCollection,
+  waitForDb
+}

services/docstore/app/js/mongojs.js

@@ -1,9 +0,0 @@
-// TODO: This file was created by bulk-decaffeinate.
-// Sanity-check the conversion and remove this comment.
-const Settings = require('settings-sharelatex')
-const mongojs = require('mongojs')
-const db = mongojs(Settings.mongo.url, ['docs', 'docOps'])
-module.exports = {
-  db,
-  ObjectId: mongojs.ObjectId
-}

services/docstore/package.json

@@ -25,7 +25,7 @@
     "express": "^4.17.1",
     "logger-sharelatex": "^2.2.0",
     "metrics-sharelatex": "^2.7.0",
-    "mongojs": "3.1.0",
+    "mongodb": "^3.6.0",
     "settings-sharelatex": "^1.1.0",
     "streamifier": "^0.1.1",
     "tiny-async-pool": "^1.1.0",

services/docstore/test/acceptance/js/ArchiveDocsTests.js

@@ -17,7 +17,7 @@ const Settings = require('settings-sharelatex')
 const chai = require('chai')
 const { expect } = chai
 const should = chai.should()
-const { db, ObjectId } = require('../../../app/js/mongojs')
+const { getCollection, ObjectId } = require('../../../app/js/mongodb')
 const async = require('async')
 const DocstoreApp = require('./helpers/DocstoreApp')
 const DocstoreClient = require('./helpers/DocstoreClient')
@@ -32,6 +32,14 @@ function uploadContent(path, json, callback) {
     .catch(callback)
 }
 
+let db
+before(async function () {
+  db = {
+    docs: await getCollection('docs'),
+    docOps: await getCollection('docOps')
+  }
+})
+
 describe('Archiving', function () {
   before(function (done) {
     return DocstoreApp.ensureRunning(done)

services/docstore/test/acceptance/js/DeletingDocsTests.js

@@ -13,13 +13,21 @@
  */
 const chai = require('chai')
 chai.should()
-const { db, ObjectId } = require('../../../app/js/mongojs')
+const { getCollection, ObjectId } = require('../../../app/js/mongodb')
 const { expect } = chai
 const DocstoreApp = require('./helpers/DocstoreApp')
 const Errors = require('../../../app/js/Errors')
 
 const DocstoreClient = require('./helpers/DocstoreClient')
 
+let db
+before(async function () {
+  db = {
+    docs: await getCollection('docs'),
+    docOps: await getCollection('docOps')
+  }
+})
+
 describe('Deleting a doc', function () {
   beforeEach(function (done) {
     this.project_id = ObjectId()
@@ -61,7 +69,7 @@ describe('Deleting a doc', function () {
     })
 
     return it('should insert a deleted doc into the docs collection', function (done) {
-      return db.docs.find({ _id: this.doc_id }, (error, docs) => {
+      return db.docs.find({ _id: this.doc_id }).toArray((error, docs) => {
         docs[0]._id.should.deep.equal(this.doc_id)
         docs[0].lines.should.deep.equal(this.lines)
         docs[0].deleted.should.equal(true)
@@ -100,7 +108,7 @@ describe("Destroying a project's documents", function () {
     })
 
     it('should remove the doc from the docs collection', function (done) {
-      return db.docs.find({ _id: this.doc_id }, (err, docs) => {
+      return db.docs.find({ _id: this.doc_id }).toArray((err, docs) => {
         expect(err).not.to.exist
         expect(docs).to.deep.equal([])
         return done()
@@ -108,7 +116,7 @@ describe("Destroying a project's documents", function () {
     })
 
     return it('should remove the docOps from the docOps collection', function (done) {
-      return db.docOps.find({ doc_id: this.doc_id }, (err, docOps) => {
+      return db.docOps.find({ doc_id: this.doc_id }).toArray((err, docOps) => {
         expect(err).not.to.exist
         expect(docOps).to.deep.equal([])
         return done()
@@ -118,7 +126,7 @@ describe("Destroying a project's documents", function () {
 
   return describe('when the doc is archived', function () {
     beforeEach(function (done) {
-      return DocstoreClient.archiveAllDoc(this.project_id, function (err) {
+      return DocstoreClient.archiveAllDoc(this.project_id, (err) => {
         if (err != null) {
           return done(err)
         }
@@ -127,7 +135,7 @@ describe("Destroying a project's documents", function () {
     })
 
     it('should remove the doc from the docs collection', function (done) {
-      return db.docs.find({ _id: this.doc_id }, (err, docs) => {
+      return db.docs.find({ _id: this.doc_id }).toArray((err, docs) => {
         expect(err).not.to.exist
         expect(docs).to.deep.equal([])
         return done()
@@ -135,7 +143,7 @@ describe("Destroying a project's documents", function () {
     })
 
     it('should remove the docOps from the docOps collection', function (done) {
-      return db.docOps.find({ doc_id: this.doc_id }, (err, docOps) => {
+      return db.docOps.find({ doc_id: this.doc_id }).toArray((err, docOps) => {
         expect(err).not.to.exist
         expect(docOps).to.deep.equal([])
         return done()

services/docstore/test/acceptance/js/GettingAllDocsTests.js

@@ -14,7 +14,7 @@
 const sinon = require('sinon')
 const chai = require('chai')
 chai.should()
-const { ObjectId } = require('mongojs')
+const { ObjectId } = require('mongodb')
 const async = require('async')
 const DocstoreApp = require('./helpers/DocstoreApp')
 

services/docstore/test/acceptance/js/GettingDocsTests.js

@@ -14,7 +14,7 @@
 const sinon = require('sinon')
 const chai = require('chai')
 chai.should()
-const { ObjectId } = require('mongojs')
+const { ObjectId } = require('mongodb')
 const DocstoreApp = require('./helpers/DocstoreApp')
 
 const DocstoreClient = require('./helpers/DocstoreClient')

services/docstore/test/acceptance/js/UpdatingDocsTests.js

@@ -13,7 +13,7 @@
 const sinon = require('sinon')
 const chai = require('chai')
 chai.should()
-const { ObjectId } = require('mongojs')
+const { ObjectId } = require('mongodb')
 const DocstoreApp = require('./helpers/DocstoreApp')
 
 const DocstoreClient = require('./helpers/DocstoreClient')

services/docstore/test/unit/js/DocArchiveManagerTests.js

@@ -3,7 +3,7 @@ const chai = require('chai')
 const { expect } = chai
 const modulePath = '../../../app/js/DocArchiveManager.js'
 const SandboxedModule = require('sandboxed-module')
-const { ObjectId } = require('mongojs')
+const { ObjectId } = require('mongodb')
 const Errors = require('../../../app/js/Errors')
 
 chai.use(require('chai-as-promised'))

services/docstore/test/unit/js/DocManagerTests.js

@@ -19,7 +19,7 @@ const { assert } = require('chai')
 chai.should()
 const { expect } = chai
 const modulePath = require('path').join(__dirname, '../../../app/js/DocManager')
-const { ObjectId } = require('mongojs')
+const { ObjectId } = require('mongodb')
 const Errors = require('../../../app/js/Errors')
 
 describe('DocManager', function () {

services/docstore/test/unit/js/HttpControllerTests.js

@@ -19,7 +19,7 @@ const modulePath = require('path').join(
   __dirname,
   '../../../app/js/HttpController'
 )
-const { ObjectId } = require('mongojs')
+const { ObjectId } = require('mongodb')
 
 describe('HttpController', function () {
   beforeEach(function () {

services/docstore/test/unit/js/MongoManagerTests.js

@@ -16,14 +16,18 @@ const modulePath = require('path').join(
   __dirname,
   '../../../app/js/MongoManager'
 )
-const { ObjectId } = require('mongojs')
+const { ObjectId } = require('mongodb')
 const { assert } = require('chai')
 
 describe('MongoManager', function () {
   beforeEach(function () {
+    this.db = {}
     this.MongoManager = SandboxedModule.require(modulePath, {
       requires: {
-        './mongojs': {
+        './mongodb': {
+          getCollection: sinon.stub().callsFake((name) => {
+            return Promise.resolve((this.db[name] = {}))
+          }),
           db: (this.db = { docs: {}, docOps: {} }),
           ObjectId
         },
@@ -36,14 +40,14 @@ describe('MongoManager', function () {
     })
     this.project_id = ObjectId().toString()
     this.doc_id = ObjectId().toString()
-    this.callback = sinon.stub()
     return (this.stubbedErr = new Error('hello world'))
   })
 
   describe('findDoc', function () {
-    beforeEach(function () {
+    beforeEach(function (done) {
+      this.callback = sinon.stub().callsFake(() => done())
       this.doc = { name: 'mock-doc' }
-      this.db.docs.find = sinon.stub().callsArgWith(2, null, [this.doc])
+      this.db.docs.findOne = sinon.stub().callsArgWith(2, null, this.doc)
       this.filter = { lines: true }
       return this.MongoManager.findDoc(
         this.project_id,
@@ -54,7 +58,7 @@ describe('MongoManager', function () {
     })
 
     it('should find the doc', function () {
-      return this.db.docs.find
+      this.db.docs.findOne
         .calledWith(
           {
             _id: ObjectId(this.doc_id),
@@ -77,13 +81,16 @@ describe('MongoManager', function () {
       this.doc2 = { name: 'mock-doc2' }
       this.doc3 = { name: 'mock-doc3' }
       this.doc4 = { name: 'mock-doc4' }
-      return (this.db.docs.find = sinon
-        .stub()
-        .callsArgWith(2, null, [this.doc, this.doc3, this.doc4]))
+      this.db.docs.find = sinon.stub().returns({
+        toArray: sinon
+          .stub()
+          .callsArgWith(0, null, [this.doc, this.doc3, this.doc4])
+      })
     })
 
     describe('with included_deleted = false', function () {
-      beforeEach(function () {
+      beforeEach(function (done) {
+        this.callback = sinon.stub().callsFake(() => done())
         return this.MongoManager.getProjectsDocs(
           this.project_id,
           { include_deleted: false },
@@ -112,7 +119,8 @@ describe('MongoManager', function () {
     })
 
     return describe('with included_deleted = true', function () {
-      beforeEach(function () {
+      beforeEach(function (done) {
+        this.callback = sinon.stub().callsFake(() => done())
         return this.MongoManager.getProjectsDocs(
           this.project_id,
           { include_deleted: true },
@@ -142,7 +150,7 @@ describe('MongoManager', function () {
 
   describe('upsertIntoDocCollection', function () {
     beforeEach(function () {
-      this.db.docs.update = sinon.stub().callsArgWith(3, this.stubbedErr)
+      this.db.docs.updateOne = sinon.stub().callsArgWith(3, this.stubbedErr)
       return (this.oldRev = 77)
     })
 
@@ -152,7 +160,7 @@ describe('MongoManager', function () {
         this.doc_id,
         { lines: this.lines },
         (err) => {
-          const args = this.db.docs.update.args[0]
+          const args = this.db.docs.updateOne.args[0]
           assert.deepEqual(args[0], { _id: ObjectId(this.doc_id) })
           assert.equal(args[1].$set.lines, this.lines)
           assert.equal(args[1].$inc.rev, 1)
@@ -177,7 +185,7 @@ describe('MongoManager', function () {
 
   describe('markDocAsDeleted', function () {
     beforeEach(function () {
-      this.db.docs.update = sinon.stub().callsArgWith(2, this.stubbedErr)
+      this.db.docs.updateOne = sinon.stub().callsArgWith(2, this.stubbedErr)
       return (this.oldRev = 77)
     })
 
@@ -186,7 +194,7 @@ describe('MongoManager', function () {
         this.project_id,
         this.doc_id,
         (err) => {
-          const args = this.db.docs.update.args[0]
+          const args = this.db.docs.updateOne.args[0]
           assert.deepEqual(args[0], {
             _id: ObjectId(this.doc_id),
             project_id: ObjectId(this.project_id)
@@ -211,19 +219,19 @@ describe('MongoManager', function () {
 
   describe('destroyDoc', function () {
     beforeEach(function (done) {
-      this.db.docs.remove = sinon.stub().yields()
-      this.db.docOps.remove = sinon.stub().yields()
+      this.db.docs.deleteOne = sinon.stub().yields()
+      this.db.docOps.deleteOne = sinon.stub().yields()
       return this.MongoManager.destroyDoc('123456789012', done)
     })
 
     it('should destroy the doc', function () {
-      return sinon.assert.calledWith(this.db.docs.remove, {
+      return sinon.assert.calledWith(this.db.docs.deleteOne, {
         _id: ObjectId('123456789012')
       })
     })
 
     return it('should destroy the docOps', function () {
-      return sinon.assert.calledWith(this.db.docOps.remove, {
+      return sinon.assert.calledWith(this.db.docOps.deleteOne, {
         doc_id: ObjectId('123456789012')
       })
     })
@@ -231,14 +239,15 @@ describe('MongoManager', function () {
 
   describe('getDocVersion', function () {
     describe('when the doc exists', function () {
-      beforeEach(function () {
+      beforeEach(function (done) {
+        this.callback = sinon.stub().callsFake(() => done())
         this.doc = { version: (this.version = 42) }
-        this.db.docOps.find = sinon.stub().callsArgWith(2, null, [this.doc])
+        this.db.docOps.findOne = sinon.stub().callsArgWith(2, null, this.doc)
         return this.MongoManager.getDocVersion(this.doc_id, this.callback)
       })
 
       it('should look for the doc in the database', function () {
-        return this.db.docOps.find
+        return this.db.docOps.findOne
           .calledWith({ doc_id: ObjectId(this.doc_id) }, { version: 1 })
           .should.equal(true)
       })
@@ -249,8 +258,9 @@ describe('MongoManager', function () {
     })
 
     return describe("when the doc doesn't exist", function () {
-      beforeEach(function () {
-        this.db.docOps.find = sinon.stub().callsArgWith(2, null, [])
+      beforeEach(function (done) {
+        this.callback = sinon.stub().callsFake(() => done())
+        this.db.docOps.findOne = sinon.stub().callsArgWith(2, null, null)
         return this.MongoManager.getDocVersion(this.doc_id, this.callback)
       })
 
@@ -261,9 +271,10 @@ describe('MongoManager', function () {
   })
 
   return describe('setDocVersion', function () {
-    beforeEach(function () {
+    beforeEach(function (done) {
+      this.callback = sinon.stub().callsFake(() => done())
       this.version = 42
-      this.db.docOps.update = sinon.stub().callsArg(3)
+      this.db.docOps.updateOne = sinon.stub().callsArg(3)
       return this.MongoManager.setDocVersion(
         this.doc_id,
         this.version,
@@ -272,7 +283,7 @@ describe('MongoManager', function () {
     })
 
     it('should update the doc version', function () {
-      return this.db.docOps.update
+      return this.db.docOps.updateOne
         .calledWith(
           {
             doc_id: ObjectId(this.doc_id)

services/docstore/test/unit/js/RangeManagerTests.js

@@ -18,7 +18,7 @@ const modulePath = require('path').join(
   __dirname,
   '../../../app/js/RangeManager'
 )
-const { ObjectId } = require('mongojs')
+const { ObjectId } = require('mongodb')
 const { assert } = require('chai')
 const _ = require('underscore')
 
@@ -26,7 +26,7 @@ describe('RangeManager', function () {
   beforeEach(function () {
     return (this.RangeManager = SandboxedModule.require(modulePath, {
       requires: {
-        './mongojs': {
+        './mongodb': {
           ObjectId
         }
       }