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

[misc] migrate to the native mongo driver

services/notifications/app.js

@@ -15,6 +15,7 @@ const app = express()
 const methodOverride = require('method-override')
 const bodyParser = require('body-parser')
 const errorHandler = require('errorhandler')
+const mongodb = require('./app/js/mongodb')
 const controller = require('./app/js/NotificationsController')
 
 metrics.memory.monitor(logger)
@@ -62,9 +63,18 @@ const port =
     Settings.internal != null ? Settings.internal.notifications : undefined,
     (x1) => x1.port
   ) || 3042
-app.listen(port, host, () =>
+
+mongodb
+  .waitForDb()
+  .then(() => {
+    app.listen(port, host, () =>
       logger.info(`notifications starting up, listening on ${host}:${port}`)
-)
+    )
+  })
+  .catch((err) => {
+    logger.fatal({ err }, 'Cannot connect to mongo. Exiting.')
+    process.exit(1)
+  })
 
 function __guard__(value, transform) {
   return typeof value !== 'undefined' && value !== null

services/notifications/app/js/HealthCheckController.js

@@ -11,7 +11,7 @@
  * DS207: Consider shorter variations of null checks
  * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
  */
-const { ObjectId } = require('mongojs')
+const { db, ObjectId } = require('./mongodb')
 const request = require('request')
 const async = require('async')
 const _ = require('underscore')
@@ -19,12 +19,6 @@ const settings = require('settings-sharelatex')
 const { port } = settings.internal.notifications
 const logger = require('logger-sharelatex')
 
-const mongojs = require('mongojs')
-const Settings = require('settings-sharelatex')
-const db = mongojs(Settings.mongo != null ? Settings.mongo.url : undefined, [
-  'notifications'
-])
-
 module.exports = {
   check(callback) {
     const user_id = ObjectId()

services/notifications/app/js/Notifications.js

@@ -12,13 +12,8 @@
  * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
  */
 let Notifications
-const Settings = require('settings-sharelatex')
 const logger = require('logger-sharelatex')
-const mongojs = require('mongojs')
-const db = mongojs(Settings.mongo != null ? Settings.mongo.url : undefined, [
-  'notifications'
-])
-const { ObjectId } = require('mongojs')
+const { db, ObjectId } = require('./mongodb')
 const metrics = require('metrics-sharelatex')
 
 module.exports = Notifications = {
@@ -30,9 +25,7 @@ module.exports = Notifications = {
       user_id: ObjectId(user_id),
       templateKey: { $exists: true }
     }
-    return db.notifications.find(query, (err, notifications) =>
-      callback(err, notifications)
-    )
+    db.notifications.find(query).toArray(callback)
   },
 
   _countExistingNotifications(user_id, notification, callback) {
@@ -85,7 +78,7 @@ module.exports = Notifications = {
           return callback(err)
         }
       }
-      return db.notifications.update(
+      db.notifications.updateOne(
         { user_id: doc.user_id, key: notification.key },
         { $set: doc },
         { upsert: true },
@@ -100,7 +93,7 @@ module.exports = Notifications = {
       _id: ObjectId(notification_id)
     }
     const updateOperation = { $unset: { templateKey: true, messageOpts: true } }
-    return db.notifications.update(searchOps, updateOperation, callback)
+    db.notifications.updateOne(searchOps, updateOperation, callback)
   },
 
   removeNotificationKey(user_id, notification_key, callback) {
@@ -109,19 +102,19 @@ module.exports = Notifications = {
       key: notification_key
     }
     const updateOperation = { $unset: { templateKey: true } }
-    return db.notifications.update(searchOps, updateOperation, callback)
+    db.notifications.updateOne(searchOps, updateOperation, callback)
   },
 
   removeNotificationByKeyOnly(notification_key, callback) {
     const searchOps = { key: notification_key }
     const updateOperation = { $unset: { templateKey: true } }
-    return db.notifications.update(searchOps, updateOperation, callback)
+    db.notifications.updateOne(searchOps, updateOperation, callback)
   },
 
   // hard delete of doc, rather than removing the templateKey
   deleteNotificationByKeyOnly(notification_key, callback) {
     const searchOps = { key: notification_key }
-    return db.notifications.remove(searchOps, { justOne: true }, callback)
+    db.notifications.deleteOne(searchOps, callback)
   }
 }
 ;['getUserNotifications', 'addNotification'].map((method) =>

services/notifications/app/js/mongodb.js

@@ -0,0 +1,28 @@
+const Settings = require('settings-sharelatex')
+const { MongoClient, ObjectId } = require('mongodb')
+
+const clientPromise = MongoClient.connect(
+  Settings.mongo.url,
+  Settings.mongo.options
+)
+
+let setupDbPromise
+async function waitForDb() {
+  if (!setupDbPromise) {
+    setupDbPromise = setupDb()
+  }
+  await setupDbPromise
+}
+
+const db = {}
+async function setupDb() {
+  const internalDb = (await clientPromise).db()
+
+  db.notifications = internalDb.collection('notifications')
+}
+
+module.exports = {
+  db,
+  ObjectId,
+  waitForDb
+}

services/notifications/config/settings.defaults.js

@@ -7,6 +7,10 @@ module.exports = {
   },
 
   mongo: {
+    options: {
+      useUnifiedTopology:
+        (process.env.MONGO_USE_UNIFIED_TOPOLOGY || 'true') === 'true'
+    },
     url:
       process.env.MONGO_CONNECTION_STRING ||
       `mongodb://${process.env.MONGO_HOST || 'localhost'}/sharelatex`

services/notifications/package.json

@@ -25,7 +25,7 @@
     "logger-sharelatex": "^2.2.0",
     "method-override": "^3.0.0",
     "metrics-sharelatex": "^2.6.2",
-    "mongojs": "^3.1.0",
+    "mongodb": "^3.6.0",
     "node-statsd": "0.1.1",
     "request": "^2.88.2",
     "settings-sharelatex": "^1.1.0",

services/notifications/test/unit/js/NotificationsTests.js

@@ -19,7 +19,7 @@ const should = chai.should()
 const modulePath = '../../../app/js/Notifications.js'
 const SandboxedModule = require('sandboxed-module')
 const assert = require('assert')
-const { ObjectId } = require('mongojs')
+const { ObjectId } = require('mongodb')
 
 const user_id = '51dc93e6fb625a261300003b'
 const notification_id = 'fb625a26f09d'
@@ -27,25 +27,19 @@ const notification_key = 'notification-key'
 
 describe('Notifications Tests', function () {
   beforeEach(function () {
-    const self = this
-    this.findStub = sinon.stub()
-    this.insertStub = sinon.stub()
+    this.findToArrayStub = sinon.stub()
+    this.findStub = sinon.stub().returns({ toArray: this.findToArrayStub })
     this.countStub = sinon.stub()
-    this.updateStub = sinon.stub()
-    this.removeStub = sinon.stub()
-    this.mongojs = () => {
-      return {
+    this.updateOneStub = sinon.stub()
+    this.deleteOneStub = sinon.stub()
+    this.db = {
       notifications: {
-          update: self.mongojsUpdate,
         find: this.findStub,
-          insert: this.insertStub,
         count: this.countStub,
-          update: this.updateStub,
-          remove: this.removeStub
+        updateOne: this.updateOneStub,
+        deleteOne: this.deleteOneStub
       }
     }
-    }
-    this.mongojs.ObjectId = ObjectId
 
     this.notifications = SandboxedModule.require(modulePath, {
       requires: {
@@ -54,7 +48,7 @@ describe('Notifications Tests', function () {
           error() {}
         },
         'settings-sharelatex': {},
-        mongojs: this.mongojs,
+        './mongodb': { db: this.db, ObjectId },
         'metrics-sharelatex': { timeAsyncMethod: sinon.stub() }
       },
       globals: {
@@ -73,7 +67,7 @@ describe('Notifications Tests', function () {
 
   describe('getUserNotifications', function () {
     return it('should find all notifications and return i', function (done) {
-      this.findStub.callsArgWith(1, null, this.stubbedNotificationArray)
+      this.findToArrayStub.callsArgWith(0, null, this.stubbedNotificationArray)
       return this.notifications.getUserNotifications(
         user_id,
         (err, notifications) => {
@@ -106,7 +100,7 @@ describe('Notifications Tests', function () {
         user_id: this.stubbedNotification.user_id,
         key: 'notification-key'
       }
-      this.updateStub.yields()
+      this.updateOneStub.yields()
       return this.countStub.yields(null, 0)
     })
 
@@ -117,7 +111,7 @@ describe('Notifications Tests', function () {
         (err) => {
           expect(err).not.exists
           sinon.assert.calledWith(
-            this.updateStub,
+            this.updateOneStub,
             this.expectedQuery,
             { $set: this.expectedDocument },
             { upsert: true }
@@ -138,7 +132,7 @@ describe('Notifications Tests', function () {
           this.stubbedNotification,
           (err) => {
             expect(err).not.exists
-            sinon.assert.notCalled(this.updateStub)
+            sinon.assert.notCalled(this.updateOneStub)
             return done()
           }
         )
@@ -152,7 +146,7 @@ describe('Notifications Tests', function () {
           (err) => {
             expect(err).not.exists
             sinon.assert.calledWith(
-              this.updateStub,
+              this.updateOneStub,
               this.expectedQuery,
               { $set: this.expectedDocument },
               { upsert: true }
@@ -192,7 +186,7 @@ describe('Notifications Tests', function () {
           (err) => {
             expect(err).not.exists
             sinon.assert.calledWith(
-              this.updateStub,
+              this.updateOneStub,
               this.expectedQuery,
               { $set: this.expectedDocument },
               { upsert: true }
@@ -227,7 +221,7 @@ describe('Notifications Tests', function () {
           this.stubbedNotification,
           (err) => {
             ;(err instanceof Error).should.equal(true)
-            sinon.assert.notCalled(this.updateStub)
+            sinon.assert.notCalled(this.updateOneStub)
             return done()
           }
         )
@@ -237,7 +231,7 @@ describe('Notifications Tests', function () {
 
   describe('removeNotificationId', function () {
     return it('should mark the notification id as read', function (done) {
-      this.updateStub.callsArgWith(2, null)
+      this.updateOneStub.callsArgWith(2, null)
 
       return this.notifications.removeNotificationId(
         user_id,
@@ -250,8 +244,8 @@ describe('Notifications Tests', function () {
           const updateOperation = {
             $unset: { templateKey: true, messageOpts: true }
           }
-          assert.deepEqual(this.updateStub.args[0][0], searchOps)
-          assert.deepEqual(this.updateStub.args[0][1], updateOperation)
+          assert.deepEqual(this.updateOneStub.args[0][0], searchOps)
+          assert.deepEqual(this.updateOneStub.args[0][1], updateOperation)
           return done()
         }
       )
@@ -260,7 +254,7 @@ describe('Notifications Tests', function () {
 
   describe('removeNotificationKey', function () {
     return it('should mark the notification key as read', function (done) {
-      this.updateStub.callsArgWith(2, null)
+      this.updateOneStub.callsArgWith(2, null)
 
       return this.notifications.removeNotificationKey(
         user_id,
@@ -273,8 +267,8 @@ describe('Notifications Tests', function () {
           const updateOperation = {
             $unset: { templateKey: true }
           }
-          assert.deepEqual(this.updateStub.args[0][0], searchOps)
-          assert.deepEqual(this.updateStub.args[0][1], updateOperation)
+          assert.deepEqual(this.updateOneStub.args[0][0], searchOps)
+          assert.deepEqual(this.updateOneStub.args[0][1], updateOperation)
           return done()
         }
       )
@@ -283,15 +277,15 @@ describe('Notifications Tests', function () {
 
   describe('removeNotificationByKeyOnly', function () {
     return it('should mark the notification key as read', function (done) {
-      this.updateStub.callsArgWith(2, null)
+      this.updateOneStub.callsArgWith(2, null)
 
       return this.notifications.removeNotificationByKeyOnly(
         notification_key,
         (err) => {
           const searchOps = { key: notification_key }
           const updateOperation = { $unset: { templateKey: true } }
-          assert.deepEqual(this.updateStub.args[0][0], searchOps)
-          assert.deepEqual(this.updateStub.args[0][1], updateOperation)
+          assert.deepEqual(this.updateOneStub.args[0][0], searchOps)
+          assert.deepEqual(this.updateOneStub.args[0][1], updateOperation)
           return done()
         }
       )
@@ -300,15 +294,13 @@ describe('Notifications Tests', function () {
 
   return describe('deleteNotificationByKeyOnly', function () {
     return it('should completely remove the notification', function (done) {
-      this.removeStub.callsArgWith(2, null)
+      this.deleteOneStub.callsArgWith(1, null)
 
       return this.notifications.deleteNotificationByKeyOnly(
         notification_key,
         (err) => {
           const searchOps = { key: notification_key }
-          const opts = { justOne: true }
-          assert.deepEqual(this.removeStub.args[0][0], searchOps)
-          assert.deepEqual(this.removeStub.args[0][1], opts)
+          assert.deepEqual(this.deleteOneStub.args[0][0], searchOps)
           return done()
         }
       )