From 5c70e5c5340e526acc2202a34cfb3dbb05df890a Mon Sep 17 00:00:00 2001 From: Christopher Hoskin Date: Wed, 13 Dec 2023 09:38:54 +0000 Subject: [PATCH] Merge pull request #15475 from overleaf/csh-issue-11625-mongo-ug-5-docstore Upgrade mongodb module for docstore from 4.11.0 to 6.2.0 GitOrigin-RevId: 443bdcc80398f7cd21bc78a801af3033d2b8e921 --- package-lock.json | 147 ++++++++++-------- services/docstore/app/js/HealthChecker.js | 4 +- services/docstore/app/js/MongoManager.js | 49 +++--- services/docstore/app/js/RangeManager.js | 2 +- services/docstore/app/js/mongodb.js | 2 +- services/docstore/package.json | 3 +- .../test/acceptance/js/ArchiveDocsTests.js | 50 +++--- .../test/acceptance/js/DeletingDocsTests.js | 35 +++-- .../test/acceptance/js/GettingAllDocsTests.js | 10 +- .../js/GettingDocsFromArchiveTest.js | 8 +- .../test/acceptance/js/GettingDocsTests.js | 12 +- .../test/acceptance/js/UpdatingDocsTests.js | 14 +- .../test/unit/js/DocArchiveManagerTests.js | 18 +-- .../docstore/test/unit/js/DocManagerTests.js | 16 +- .../test/unit/js/HttpControllerTests.js | 16 +- .../test/unit/js/MongoManagerTests.js | 44 +++--- .../test/unit/js/RangeManagerTests.js | 38 ++--- 17 files changed, 245 insertions(+), 223 deletions(-) diff --git a/package-lock.json b/package-lock.json index 79cbe9c9da..0201a14f05 100644 --- a/package-lock.json +++ b/package-lock.json @@ -40753,7 +40753,8 @@ "celebrate": "^13.0.4", "express": "^4.18.2", "lodash": "^4.17.21", - "mongodb": "^4.11.0", + "mongodb": "^6.2.0", + "mongodb-legacy": "^6.0.1", "p-map": "^4.0.0", "request": "^2.88.2" }, @@ -40767,29 +40768,6 @@ "sinon-chai": "^3.7.0" } }, - "services/docstore/node_modules/buffer": { - "version": "5.7.1", - "resolved": "https://registry.npmjs.org/buffer/-/buffer-5.7.1.tgz", - "integrity": "sha512-EHcyIPBQ4BSGlvjB16k5KgAJ27CIsHY/2JBmCRReo48y9rQ3MaUzWX3KVlBa4U7MyX02HdVj0K7C3WaB3ju7FQ==", - "funding": [ - { - "type": "github", - "url": "https://github.com/sponsors/feross" - }, - { - "type": "patreon", - "url": "https://www.patreon.com/feross" - }, - { - "type": "consulting", - "url": "https://feross.org/support" - } - ], - "dependencies": { - "base64-js": "^1.3.1", - "ieee754": "^1.1.13" - } - }, "services/docstore/node_modules/celebrate": { "version": "13.0.4", "resolved": "https://registry.npmjs.org/celebrate/-/celebrate-13.0.4.tgz", @@ -40809,32 +40787,71 @@ "node": ">=0.3.1" } }, - "services/docstore/node_modules/mongodb": { - "version": "4.17.1", - "resolved": "https://registry.npmjs.org/mongodb/-/mongodb-4.17.1.tgz", - "integrity": "sha512-MBuyYiPUPRTqfH2dV0ya4dcr2E5N52ocBuZ8Sgg/M030nGF78v855B3Z27mZJnp8PxjnUquEnAtjOsphgMZOlQ==", + "services/docstore/node_modules/gcp-metadata": { + "version": "5.3.0", + "resolved": "https://registry.npmjs.org/gcp-metadata/-/gcp-metadata-5.3.0.tgz", + "integrity": "sha512-FNTkdNEnBdlqF2oatizolQqNANMrcqJt6AAYt99B3y1aLLC8Hc5IOBb+ZnnzllodEEf6xMBp6wRcBbc16fa65w==", + "optional": true, + "peer": true, "dependencies": { - "bson": "^4.7.2", - "mongodb-connection-string-url": "^2.6.0", - "socks": "^2.7.1" + "gaxios": "^5.0.0", + "json-bigint": "^1.0.0" }, "engines": { - "node": ">=12.9.0" + "node": ">=12" + } + }, + "services/docstore/node_modules/mongodb": { + "version": "6.2.0", + "resolved": "https://registry.npmjs.org/mongodb/-/mongodb-6.2.0.tgz", + "integrity": "sha512-d7OSuGjGWDZ5usZPqfvb36laQ9CPhnWkAGHT61x5P95p/8nMVeH8asloMwW6GcYFeB0Vj4CB/1wOTDG2RA9BFA==", + "dependencies": { + "@mongodb-js/saslprep": "^1.1.0", + "bson": "^6.2.0", + "mongodb-connection-string-url": "^2.6.0" }, - "optionalDependencies": { - "@aws-sdk/credential-providers": "^3.186.0", - "@mongodb-js/saslprep": "^1.1.0" + "engines": { + "node": ">=16.20.1" + }, + "peerDependencies": { + "@aws-sdk/credential-providers": "^3.188.0", + "@mongodb-js/zstd": "^1.1.0", + "gcp-metadata": "^5.2.0", + "kerberos": "^2.0.1", + "mongodb-client-encryption": ">=6.0.0 <7", + "snappy": "^7.2.2", + "socks": "^2.7.1" + }, + "peerDependenciesMeta": { + "@aws-sdk/credential-providers": { + "optional": true + }, + "@mongodb-js/zstd": { + "optional": true + }, + "gcp-metadata": { + "optional": true + }, + "kerberos": { + "optional": true + }, + "mongodb-client-encryption": { + "optional": true + }, + "snappy": { + "optional": true + }, + "socks": { + "optional": true + } } }, "services/docstore/node_modules/mongodb/node_modules/bson": { - "version": "4.7.2", - "resolved": "https://registry.npmjs.org/bson/-/bson-4.7.2.tgz", - "integrity": "sha512-Ry9wCtIZ5kGqkJoi6aD8KjxFZEx78guTQDnpXWiNthsxzrxAK/i8E6pCHAIZTbaEFWcOCvbecMukfK7XUvyLpQ==", - "dependencies": { - "buffer": "^5.6.0" - }, + "version": "6.2.0", + "resolved": "https://registry.npmjs.org/bson/-/bson-6.2.0.tgz", + "integrity": "sha512-ID1cI+7bazPDyL9wYy9GaQ8gEEohWvcUl/Yf0dIdutJxnmInEEyCsb4awy/OiBfall7zBA179Pahi3vCdFze3Q==", "engines": { - "node": ">=6.9.0" + "node": ">=16.20.1" } }, "services/docstore/node_modules/sinon": { @@ -50804,7 +50821,8 @@ "express": "^4.18.2", "lodash": "^4.17.21", "mocha": "^10.2.0", - "mongodb": "^4.11.0", + "mongodb": "^6.2.0", + "mongodb-legacy": "^6.0.1", "p-map": "^4.0.0", "request": "^2.88.2", "sandboxed-module": "~2.0.4", @@ -50812,15 +50830,6 @@ "sinon-chai": "^3.7.0" }, "dependencies": { - "buffer": { - "version": "5.7.1", - "resolved": "https://registry.npmjs.org/buffer/-/buffer-5.7.1.tgz", - "integrity": "sha512-EHcyIPBQ4BSGlvjB16k5KgAJ27CIsHY/2JBmCRReo48y9rQ3MaUzWX3KVlBa4U7MyX02HdVj0K7C3WaB3ju7FQ==", - "requires": { - "base64-js": "^1.3.1", - "ieee754": "^1.1.13" - } - }, "celebrate": { "version": "13.0.4", "resolved": "https://registry.npmjs.org/celebrate/-/celebrate-13.0.4.tgz", @@ -50837,25 +50846,31 @@ "integrity": "sha512-58lmxKSA4BNyLz+HHMUzlOEpg09FV+ev6ZMe3vJihgdxzgcwZ8VoEEPmALCZG9LmqfVoNMMKpttIYTVG6uDY7A==", "dev": true }, - "mongodb": { - "version": "4.17.1", - "resolved": "https://registry.npmjs.org/mongodb/-/mongodb-4.17.1.tgz", - "integrity": "sha512-MBuyYiPUPRTqfH2dV0ya4dcr2E5N52ocBuZ8Sgg/M030nGF78v855B3Z27mZJnp8PxjnUquEnAtjOsphgMZOlQ==", + "gcp-metadata": { + "version": "5.3.0", + "resolved": "https://registry.npmjs.org/gcp-metadata/-/gcp-metadata-5.3.0.tgz", + "integrity": "sha512-FNTkdNEnBdlqF2oatizolQqNANMrcqJt6AAYt99B3y1aLLC8Hc5IOBb+ZnnzllodEEf6xMBp6wRcBbc16fa65w==", + "optional": true, + "peer": true, + "requires": { + "gaxios": "^5.0.0", + "json-bigint": "^1.0.0" + } + }, + "mongodb": { + "version": "6.2.0", + "resolved": "https://registry.npmjs.org/mongodb/-/mongodb-6.2.0.tgz", + "integrity": "sha512-d7OSuGjGWDZ5usZPqfvb36laQ9CPhnWkAGHT61x5P95p/8nMVeH8asloMwW6GcYFeB0Vj4CB/1wOTDG2RA9BFA==", "requires": { - "@aws-sdk/credential-providers": "^3.186.0", "@mongodb-js/saslprep": "^1.1.0", - "bson": "^4.7.2", - "mongodb-connection-string-url": "^2.6.0", - "socks": "^2.7.1" + "bson": "^6.2.0", + "mongodb-connection-string-url": "^2.6.0" }, "dependencies": { "bson": { - "version": "4.7.2", - "resolved": "https://registry.npmjs.org/bson/-/bson-4.7.2.tgz", - "integrity": "sha512-Ry9wCtIZ5kGqkJoi6aD8KjxFZEx78guTQDnpXWiNthsxzrxAK/i8E6pCHAIZTbaEFWcOCvbecMukfK7XUvyLpQ==", - "requires": { - "buffer": "^5.6.0" - } + "version": "6.2.0", + "resolved": "https://registry.npmjs.org/bson/-/bson-6.2.0.tgz", + "integrity": "sha512-ID1cI+7bazPDyL9wYy9GaQ8gEEohWvcUl/Yf0dIdutJxnmInEEyCsb4awy/OiBfall7zBA179Pahi3vCdFze3Q==" } } }, diff --git a/services/docstore/app/js/HealthChecker.js b/services/docstore/app/js/HealthChecker.js index 9e54a30d6d..2462725fb0 100644 --- a/services/docstore/app/js/HealthChecker.js +++ b/services/docstore/app/js/HealthChecker.js @@ -17,8 +17,8 @@ const logger = require('@overleaf/logger') module.exports = { check(callback) { - const docId = ObjectId() - const projectId = ObjectId(settings.docstore.healthCheck.project_id) + const docId = new ObjectId() + const projectId = new ObjectId(settings.docstore.healthCheck.project_id) const url = `http://localhost:${port}/project/${projectId}/doc/${docId}` const lines = [ 'smoke test - delete me', diff --git a/services/docstore/app/js/MongoManager.js b/services/docstore/app/js/MongoManager.js index acce8dbea6..1d2935428f 100644 --- a/services/docstore/app/js/MongoManager.js +++ b/services/docstore/app/js/MongoManager.js @@ -11,8 +11,8 @@ const ARCHIVING_LOCK_DURATION_MS = Settings.archivingLockDurationMs function findDoc(projectId, docId, filter, callback) { db.docs.findOne( { - _id: ObjectId(docId.toString()), - project_id: ObjectId(projectId.toString()), + _id: new ObjectId(docId.toString()), + project_id: new ObjectId(projectId.toString()), }, { projection: filter, @@ -25,7 +25,7 @@ function getProjectsDeletedDocs(projectId, filter, callback) { db.docs .find( { - project_id: ObjectId(projectId.toString()), + project_id: new ObjectId(projectId.toString()), deleted: true, }, { @@ -38,7 +38,7 @@ function getProjectsDeletedDocs(projectId, filter, callback) { } function getProjectsDocs(projectId, options, filter, callback) { - const query = { project_id: ObjectId(projectId.toString()) } + const query = { project_id: new ObjectId(projectId.toString()) } if (!options.include_deleted) { query.deleted = { $ne: true } } @@ -53,7 +53,7 @@ function getProjectsDocs(projectId, options, filter, callback) { function getArchivedProjectDocs(projectId, maxResults, callback) { const query = { - project_id: ObjectId(projectId.toString()), + project_id: new ObjectId(projectId.toString()), inS3: true, } db.docs @@ -65,7 +65,7 @@ function getNonArchivedProjectDocIds(projectId, callback) { db.docs .find( { - project_id: ObjectId(projectId), + project_id: new ObjectId(projectId), inS3: { $ne: true }, }, { projection: { _id: 1 } } @@ -76,7 +76,7 @@ function getNonArchivedProjectDocIds(projectId, callback) { function getNonDeletedArchivedProjectDocs(projectId, maxResults, callback) { const query = { - project_id: ObjectId(projectId.toString()), + project_id: new ObjectId(projectId.toString()), deleted: { $ne: true }, inS3: true, } @@ -102,8 +102,8 @@ function upsertIntoDocCollection( } db.docs.updateOne( { - _id: ObjectId(docId), - project_id: ObjectId(projectId), + _id: new ObjectId(docId), + project_id: new ObjectId(projectId), rev: previousRev, }, update, @@ -118,8 +118,8 @@ function upsertIntoDocCollection( } else { db.docs.insertOne( { - _id: ObjectId(docId), - project_id: ObjectId(projectId), + _id: new ObjectId(docId), + project_id: new ObjectId(projectId), rev: 1, ...updates, }, @@ -140,8 +140,8 @@ function upsertIntoDocCollection( function patchDoc(projectId, docId, meta, callback) { db.docs.updateOne( { - _id: ObjectId(docId), - project_id: ObjectId(projectId), + _id: new ObjectId(docId), + project_id: new ObjectId(projectId), }, { $set: meta }, callback @@ -158,13 +158,16 @@ function getDocForArchiving(projectId, docId, callback) { const archivingUntil = new Date(Date.now() + ARCHIVING_LOCK_DURATION_MS) db.docs.findOneAndUpdate( { - _id: ObjectId(docId), - project_id: ObjectId(projectId), + _id: new ObjectId(docId), + project_id: new ObjectId(projectId), inS3: { $ne: true }, $or: [{ archivingUntil: null }, { archivingUntil: { $lt: new Date() } }], }, { $set: { archivingUntil } }, - { projection: { lines: 1, ranges: 1, rev: 1 } }, + { + projection: { lines: 1, ranges: 1, rev: 1 }, + includeResultMetadata: true, + }, (err, result) => { if (err) { return callback(err) @@ -179,7 +182,7 @@ function getDocForArchiving(projectId, docId, callback) { */ function markDocAsArchived(projectId, docId, rev, callback) { db.docs.updateOne( - { _id: ObjectId(docId), rev }, + { _id: new ObjectId(docId), rev }, { $set: { inS3: true }, $unset: { lines: 1, ranges: 1, archivingUntil: 1 }, @@ -195,8 +198,8 @@ function markDocAsArchived(projectId, docId, rev, callback) { */ function restoreArchivedDoc(projectId, docId, archivedDoc, callback) { const query = { - _id: ObjectId(docId), - project_id: ObjectId(projectId), + _id: new ObjectId(docId), + project_id: new ObjectId(projectId), rev: archivedDoc.rev, } const update = { @@ -231,7 +234,7 @@ function restoreArchivedDoc(projectId, docId, archivedDoc, callback) { function getDocVersion(docId, callback) { db.docOps.findOne( { - doc_id: ObjectId(docId), + doc_id: new ObjectId(docId), }, { projection: { @@ -250,7 +253,7 @@ function getDocVersion(docId, callback) { function getDocRev(docId, callback) { db.docs.findOne( { - _id: ObjectId(docId.toString()), + _id: new ObjectId(docId.toString()), }, { projection: { rev: 1 }, @@ -297,7 +300,7 @@ function checkRevUnchanged(doc, callback) { function destroyProject(projectId, callback) { db.docs - .find({ project_id: ObjectId(projectId) }, { projection: { _id: 1 } }) + .find({ project_id: new ObjectId(projectId) }, { projection: { _id: 1 } }) .toArray((err, records) => { const docIds = records.map(r => r._id) if (err) { @@ -307,7 +310,7 @@ function destroyProject(projectId, callback) { if (err) { return callback(err) } - db.docs.deleteMany({ project_id: ObjectId(projectId) }, callback) + db.docs.deleteMany({ project_id: new ObjectId(projectId) }, callback) }) }) } diff --git a/services/docstore/app/js/RangeManager.js b/services/docstore/app/js/RangeManager.js index 7bd2156da2..f36f68fe35 100644 --- a/services/docstore/app/js/RangeManager.js +++ b/services/docstore/app/js/RangeManager.js @@ -60,7 +60,7 @@ module.exports = RangeManager = { _safeObjectId(data) { try { - return ObjectId(data) + return new ObjectId(data) } catch (error) { return data } diff --git a/services/docstore/app/js/mongodb.js b/services/docstore/app/js/mongodb.js index 8daf41fb98..9774866956 100644 --- a/services/docstore/app/js/mongodb.js +++ b/services/docstore/app/js/mongodb.js @@ -1,6 +1,6 @@ const Metrics = require('@overleaf/metrics') const Settings = require('@overleaf/settings') -const { MongoClient, ObjectId } = require('mongodb') +const { MongoClient, ObjectId } = require('mongodb-legacy') const mongoClient = new MongoClient(Settings.mongo.url) const mongoDb = mongoClient.db() diff --git a/services/docstore/package.json b/services/docstore/package.json index 030baf0fdd..5d867a4709 100644 --- a/services/docstore/package.json +++ b/services/docstore/package.json @@ -30,7 +30,8 @@ "celebrate": "^13.0.4", "express": "^4.18.2", "lodash": "^4.17.21", - "mongodb": "^4.11.0", + "mongodb": "^6.2.0", + "mongodb-legacy": "^6.0.1", "p-map": "^4.0.0", "request": "^2.88.2" }, diff --git a/services/docstore/test/acceptance/js/ArchiveDocsTests.js b/services/docstore/test/acceptance/js/ArchiveDocsTests.js index fd564589b5..f85b845a18 100644 --- a/services/docstore/test/acceptance/js/ArchiveDocsTests.js +++ b/services/docstore/test/acceptance/js/ArchiveDocsTests.js @@ -50,16 +50,16 @@ describe('Archiving', function () { describe('multiple docs in a project', function () { before(function (done) { - this.project_id = ObjectId() + this.project_id = new ObjectId() this.docs = [ { - _id: ObjectId(), + _id: new ObjectId(), lines: ['one', 'two', 'three'], ranges: {}, version: 2, }, { - _id: ObjectId(), + _id: new ObjectId(), lines: ['aaa', 'bbb', 'ccc'], ranges: {}, version: 4, @@ -177,9 +177,9 @@ describe('Archiving', function () { describe('a deleted doc', function () { beforeEach(function (done) { - this.project_id = ObjectId() + this.project_id = new ObjectId() this.doc = { - _id: ObjectId(), + _id: new ObjectId(), lines: ['one', 'two', 'three'], ranges: {}, version: 2, @@ -332,10 +332,10 @@ describe('Archiving', function () { describe('archiving a single doc', function () { before(function (done) { - this.project_id = ObjectId() + this.project_id = new ObjectId() this.timeout(1000 * 30) this.doc = { - _id: ObjectId(), + _id: new ObjectId(), lines: ['foo', 'bar'], ranges: {}, version: 2, @@ -396,14 +396,14 @@ describe('Archiving', function () { describe('a doc with large lines', function () { before(function (done) { - this.project_id = ObjectId() + this.project_id = new ObjectId() this.timeout(1000 * 30) const quarterMegInBytes = 250000 const bigLine = require('crypto') .randomBytes(quarterMegInBytes) .toString('hex') this.doc = { - _id: ObjectId(), + _id: new ObjectId(), lines: [bigLine, bigLine, bigLine, bigLine], ranges: {}, version: 2, @@ -491,9 +491,9 @@ describe('Archiving', function () { describe('a doc with naughty strings', function () { before(function (done) { - this.project_id = ObjectId() + this.project_id = new ObjectId() this.doc = { - _id: ObjectId(), + _id: new ObjectId(), lines: [ '', 'undefined', @@ -965,35 +965,35 @@ describe('Archiving', function () { describe('a doc with ranges', function () { before(function (done) { - this.project_id = ObjectId() + this.project_id = new ObjectId() this.doc = { - _id: ObjectId(), + _id: new ObjectId(), lines: ['one', 'two', 'three'], ranges: { changes: [ { - id: ObjectId(), + id: new ObjectId(), op: { i: 'foo', p: 24 }, metadata: { - user_id: ObjectId(), + user_id: new ObjectId(), ts: new Date('2017-01-27T16:10:44.194Z'), }, }, { - id: ObjectId(), + id: new ObjectId(), op: { d: 'bar', p: 50 }, metadata: { - user_id: ObjectId(), + user_id: new ObjectId(), ts: new Date('2017-01-27T18:10:44.194Z'), }, }, ], comments: [ { - id: ObjectId(), - op: { c: 'comment', p: 284, t: ObjectId() }, + id: new ObjectId(), + op: { c: 'comment', p: 284, t: new ObjectId() }, metadata: { - user_id: ObjectId(), + user_id: new ObjectId(), ts: new Date('2017-01-26T14:22:04.869Z'), }, }, @@ -1085,9 +1085,9 @@ describe('Archiving', function () { describe('a doc that is archived twice', function () { before(function (done) { - this.project_id = ObjectId() + this.project_id = new ObjectId() this.doc = { - _id: ObjectId(), + _id: new ObjectId(), lines: ['abc', 'def', 'ghi'], ranges: {}, version: 2, @@ -1181,9 +1181,9 @@ describe('Archiving', function () { return describe('a doc with the old schema (just an array of lines)', function () { before(function (done) { - this.project_id = ObjectId() + this.project_id = new ObjectId() this.doc = { - _id: ObjectId(), + _id: new ObjectId(), lines: ['abc', 'def', 'ghi'], ranges: {}, version: 2, @@ -1193,7 +1193,7 @@ describe('Archiving', function () { this.doc.lines, error => { expect(error).not.to.exist - db.docs.insert( + db.docs.insertOne( { project_id: this.project_id, _id: this.doc._id, diff --git a/services/docstore/test/acceptance/js/DeletingDocsTests.js b/services/docstore/test/acceptance/js/DeletingDocsTests.js index 8b4cfd7e24..fd06cbf344 100644 --- a/services/docstore/test/acceptance/js/DeletingDocsTests.js +++ b/services/docstore/test/acceptance/js/DeletingDocsTests.js @@ -25,8 +25,8 @@ function deleteTestSuite(deleteDoc) { }) beforeEach(function (done) { - this.project_id = ObjectId() - this.doc_id = ObjectId() + this.project_id = new ObjectId() + this.doc_id = new ObjectId() this.lines = ['original', 'lines'] this.version = 42 this.ranges = [] @@ -70,7 +70,7 @@ function deleteTestSuite(deleteDoc) { }) afterEach(function (done) { - db.docs.remove({ _id: this.doc_id }, done) + db.docs.deleteOne({ _id: this.doc_id }, done) }) it('should mark the doc as deleted on /deleted', function (done) { @@ -129,7 +129,7 @@ function deleteTestSuite(deleteDoc) { }) afterEach(function cleanupDoc(done) { - db.docs.remove({ _id: this.doc_id }, done) + db.docs.deleteOne({ _id: this.doc_id }, done) }) it('should set the deleted flag in the doc', function (done) { @@ -167,7 +167,7 @@ function deleteTestSuite(deleteDoc) { }) describe('when the doc exists in another project', function () { - const otherProjectId = ObjectId() + const otherProjectId = new ObjectId() it('should show as not existing on /deleted', function (done) { DocstoreClient.isDocDeleted(otherProjectId, this.doc_id, (error, res) => { @@ -188,7 +188,7 @@ function deleteTestSuite(deleteDoc) { describe('when the doc does not exist', function () { it('should show as not existing on /deleted', function (done) { - const missingDocId = ObjectId() + const missingDocId = new ObjectId() DocstoreClient.isDocDeleted( this.project_id, missingDocId, @@ -201,7 +201,7 @@ function deleteTestSuite(deleteDoc) { }) it('should return a 404', function (done) { - const missingDocId = ObjectId() + const missingDocId = new ObjectId() deleteDoc(this.project_id, missingDocId, (error, res, doc) => { if (error) return done(error) res.statusCode.should.equal(404) @@ -338,7 +338,7 @@ describe('Delete via PATCH', function () { describe('after deleting multiple docs', function () { beforeEach('create doc2', function (done) { - this.doc_id2 = ObjectId() + this.doc_id2 = new ObjectId() DocstoreClient.createDoc( this.project_id, this.doc_id2, @@ -359,7 +359,7 @@ describe('Delete via PATCH', function () { ) }) beforeEach('create doc3', function (done) { - this.doc_id3 = ObjectId() + this.doc_id3 = new ObjectId() DocstoreClient.createDoc( this.project_id, this.doc_id3, @@ -447,8 +447,8 @@ describe('Delete via PATCH', function () { describe("Destroying a project's documents", function () { beforeEach(function (done) { - this.project_id = ObjectId() - this.doc_id = ObjectId() + this.project_id = new ObjectId() + this.doc_id = new ObjectId() this.lines = ['original', 'lines'] this.version = 42 this.ranges = [] @@ -471,12 +471,15 @@ describe("Destroying a project's documents", function () { describe('when the doc exists', function () { beforeEach(function (done) { - db.docOps.insert({ doc_id: ObjectId(this.doc_id), version: 1 }, err => { - if (err) { - return done(err) + db.docOps.insertOne( + { doc_id: new ObjectId(this.doc_id), version: 1 }, + err => { + if (err) { + return done(err) + } + DocstoreClient.destroyAllDoc(this.project_id, done) } - DocstoreClient.destroyAllDoc(this.project_id, done) - }) + ) }) it('should remove the doc from the docs collection', function (done) { diff --git a/services/docstore/test/acceptance/js/GettingAllDocsTests.js b/services/docstore/test/acceptance/js/GettingAllDocsTests.js index 8b668ad6bf..d6994cf7d4 100644 --- a/services/docstore/test/acceptance/js/GettingAllDocsTests.js +++ b/services/docstore/test/acceptance/js/GettingAllDocsTests.js @@ -19,29 +19,29 @@ const DocstoreClient = require('./helpers/DocstoreClient') describe('Getting all docs', function () { beforeEach(function (done) { - this.project_id = ObjectId() + this.project_id = new ObjectId() this.docs = [ { - _id: ObjectId(), + _id: new ObjectId(), lines: ['one', 'two', 'three'], ranges: { mock: 'one' }, rev: 2, }, { - _id: ObjectId(), + _id: new ObjectId(), lines: ['aaa', 'bbb', 'ccc'], ranges: { mock: 'two' }, rev: 4, }, { - _id: ObjectId(), + _id: new ObjectId(), lines: ['111', '222', '333'], ranges: { mock: 'three' }, rev: 6, }, ] this.deleted_doc = { - _id: ObjectId(), + _id: new ObjectId(), lines: ['deleted'], ranges: { mock: 'four' }, rev: 8, diff --git a/services/docstore/test/acceptance/js/GettingDocsFromArchiveTest.js b/services/docstore/test/acceptance/js/GettingDocsFromArchiveTest.js index dd1e7fff41..8448f26280 100644 --- a/services/docstore/test/acceptance/js/GettingDocsFromArchiveTest.js +++ b/services/docstore/test/acceptance/js/GettingDocsFromArchiveTest.js @@ -26,10 +26,10 @@ describe('Getting A Doc from Archive', function () { describe('for an archived doc', function () { before(function (done) { - this.project_id = ObjectId() + this.project_id = new ObjectId() this.timeout(1000 * 30) this.doc = { - _id: ObjectId(), + _id: new ObjectId(), lines: ['foo', 'bar'], ranges: {}, version: 2, @@ -100,10 +100,10 @@ describe('Getting A Doc from Archive', function () { describe('for an non-archived doc', function () { before(function (done) { - this.project_id = ObjectId() + this.project_id = new ObjectId() this.timeout(1000 * 30) this.doc = { - _id: ObjectId(), + _id: new ObjectId(), lines: ['foo', 'bar'], ranges: {}, version: 2, diff --git a/services/docstore/test/acceptance/js/GettingDocsTests.js b/services/docstore/test/acceptance/js/GettingDocsTests.js index 134072fae7..4710e447dc 100644 --- a/services/docstore/test/acceptance/js/GettingDocsTests.js +++ b/services/docstore/test/acceptance/js/GettingDocsTests.js @@ -17,17 +17,17 @@ const DocstoreClient = require('./helpers/DocstoreClient') describe('Getting a doc', function () { beforeEach(function (done) { - this.project_id = ObjectId() - this.doc_id = ObjectId() + this.project_id = new ObjectId() + this.doc_id = new ObjectId() this.lines = ['original', 'lines'] this.version = 42 this.ranges = { changes: [ { - id: ObjectId().toString(), + id: new ObjectId().toString(), op: { i: 'foo', p: 3 }, meta: { - user_id: ObjectId().toString(), + user_id: new ObjectId().toString(), ts: new Date().toString(), }, }, @@ -69,7 +69,7 @@ describe('Getting a doc', function () { describe('when the doc does not exist', function () { return it('should return a 404', function (done) { - const missingDocId = ObjectId() + const missingDocId = new ObjectId() return DocstoreClient.getDoc( this.project_id, missingDocId, @@ -85,7 +85,7 @@ describe('Getting a doc', function () { return describe('when the doc is a deleted doc', function () { beforeEach(function (done) { - this.deleted_doc_id = ObjectId() + this.deleted_doc_id = new ObjectId() return DocstoreClient.createDoc( this.project_id, this.deleted_doc_id, diff --git a/services/docstore/test/acceptance/js/UpdatingDocsTests.js b/services/docstore/test/acceptance/js/UpdatingDocsTests.js index db631db83b..a1e9573b28 100644 --- a/services/docstore/test/acceptance/js/UpdatingDocsTests.js +++ b/services/docstore/test/acceptance/js/UpdatingDocsTests.js @@ -17,17 +17,17 @@ const DocstoreClient = require('./helpers/DocstoreClient') describe('Applying updates to a doc', function () { beforeEach(function (done) { - this.project_id = ObjectId() - this.doc_id = ObjectId() + this.project_id = new ObjectId() + this.doc_id = new ObjectId() this.originalLines = ['original', 'lines'] this.newLines = ['new', 'lines'] this.originalRanges = { changes: [ { - id: ObjectId().toString(), + id: new ObjectId().toString(), op: { i: 'foo', p: 3 }, meta: { - user_id: ObjectId().toString(), + user_id: new ObjectId().toString(), ts: new Date().toString(), }, }, @@ -36,10 +36,10 @@ describe('Applying updates to a doc', function () { this.newRanges = { changes: [ { - id: ObjectId().toString(), + id: new ObjectId().toString(), op: { i: 'bar', p: 6 }, meta: { - user_id: ObjectId().toString(), + user_id: new ObjectId().toString(), ts: new Date().toString(), }, }, @@ -258,7 +258,7 @@ describe('Applying updates to a doc', function () { describe('when the doc does not exist', function () { beforeEach(function (done) { - this.missing_doc_id = ObjectId() + this.missing_doc_id = new ObjectId() return DocstoreClient.updateDoc( this.project_id, this.missing_doc_id, diff --git a/services/docstore/test/unit/js/DocArchiveManagerTests.js b/services/docstore/test/unit/js/DocArchiveManagerTests.js index cfa0cc34f1..13f8e0eadd 100644 --- a/services/docstore/test/unit/js/DocArchiveManagerTests.js +++ b/services/docstore/test/unit/js/DocArchiveManagerTests.js @@ -46,47 +46,47 @@ describe('DocArchiveManager', function () { ReadableString: sinon.stub().returns({ stream: 'readStream' }), } - projectId = ObjectId() + projectId = new ObjectId() archivedDocs = [ { - _id: ObjectId(), + _id: new ObjectId(), inS3: true, rev: 2, }, { - _id: ObjectId(), + _id: new ObjectId(), inS3: true, rev: 4, }, { - _id: ObjectId(), + _id: new ObjectId(), inS3: true, rev: 6, }, ] mongoDocs = [ { - _id: ObjectId(), + _id: new ObjectId(), lines: ['one', 'two', 'three'], rev: 2, }, { - _id: ObjectId(), + _id: new ObjectId(), lines: ['aaa', 'bbb', 'ccc'], rev: 4, }, { - _id: ObjectId(), + _id: new ObjectId(), inS3: true, rev: 6, }, { - _id: ObjectId(), + _id: new ObjectId(), inS3: true, rev: 6, }, { - _id: ObjectId(), + _id: new ObjectId(), lines: ['111', '222', '333'], rev: 6, }, diff --git a/services/docstore/test/unit/js/DocManagerTests.js b/services/docstore/test/unit/js/DocManagerTests.js index 852ac8fbaa..d426b84c6f 100644 --- a/services/docstore/test/unit/js/DocManagerTests.js +++ b/services/docstore/test/unit/js/DocManagerTests.js @@ -7,9 +7,9 @@ const Errors = require('../../../app/js/Errors') describe('DocManager', function () { beforeEach(function () { - this.doc_id = ObjectId().toString() - this.project_id = ObjectId().toString() - this.another_project_id = ObjectId().toString() + this.doc_id = new ObjectId().toString() + this.project_id = new ObjectId().toString() + this.another_project_id = new ObjectId().toString() this.stubbedError = new Error('blew up') this.version = 42 @@ -343,7 +343,7 @@ describe('DocManager', function () { this.lines = ['mock', 'doc', 'lines'] this.rev = 77 this.MongoManager.promises.findDoc.resolves({ - _id: ObjectId(this.doc_id), + _id: new ObjectId(this.doc_id), }) this.meta = {} }) @@ -478,10 +478,10 @@ describe('DocManager', function () { this.originalRanges = { changes: [ { - id: ObjectId().toString(), + id: new ObjectId().toString(), op: { i: 'foo', p: 3 }, meta: { - user_id: ObjectId().toString(), + user_id: new ObjectId().toString(), ts: new Date().toString(), }, }, @@ -490,10 +490,10 @@ describe('DocManager', function () { this.newRanges = { changes: [ { - id: ObjectId().toString(), + id: new ObjectId().toString(), op: { i: 'bar', p: 6 }, meta: { - user_id: ObjectId().toString(), + user_id: new ObjectId().toString(), ts: new Date().toString(), }, }, diff --git a/services/docstore/test/unit/js/HttpControllerTests.js b/services/docstore/test/unit/js/HttpControllerTests.js index 53cc121c1c..8a08d7757d 100644 --- a/services/docstore/test/unit/js/HttpControllerTests.js +++ b/services/docstore/test/unit/js/HttpControllerTests.js @@ -158,12 +158,12 @@ describe('HttpController', function () { this.req.params = { project_id: this.projectId } this.docs = [ { - _id: ObjectId(), + _id: new ObjectId(), lines: ['mock', 'lines', 'one'], rev: 2, }, { - _id: ObjectId(), + _id: new ObjectId(), lines: ['mock', 'lines', 'two'], rev: 4, }, @@ -203,12 +203,12 @@ describe('HttpController', function () { this.req.params = { project_id: this.projectId } this.docs = [ { - _id: ObjectId(), + _id: new ObjectId(), lines: null, rev: 2, }, { - _id: ObjectId(), + _id: new ObjectId(), lines: ['mock', 'lines', 'two'], rev: 4, }, @@ -242,13 +242,13 @@ describe('HttpController', function () { this.req.params = { project_id: this.projectId } this.docs = [ { - _id: ObjectId(), + _id: new ObjectId(), lines: ['mock', 'lines', 'one'], rev: 2, }, null, { - _id: ObjectId(), + _id: new ObjectId(), lines: ['mock', 'lines', 'two'], rev: 4, }, @@ -296,11 +296,11 @@ describe('HttpController', function () { this.req.params = { project_id: this.projectId } this.docs = [ { - _id: ObjectId(), + _id: new ObjectId(), ranges: { mock_ranges: 'one' }, }, { - _id: ObjectId(), + _id: new ObjectId(), ranges: { mock_ranges: 'two' }, }, ] diff --git a/services/docstore/test/unit/js/MongoManagerTests.js b/services/docstore/test/unit/js/MongoManagerTests.js index 8eeda93f82..ed369bb02c 100644 --- a/services/docstore/test/unit/js/MongoManagerTests.js +++ b/services/docstore/test/unit/js/MongoManagerTests.js @@ -31,8 +31,8 @@ describe('MongoManager', function () { './Errors': Errors, }, }) - this.projectId = ObjectId().toString() - this.docId = ObjectId().toString() + this.projectId = new ObjectId().toString() + this.docId = new ObjectId().toString() this.rev = 42 this.callback = sinon.stub() this.stubbedErr = new Error('hello world') @@ -56,8 +56,8 @@ describe('MongoManager', function () { this.db.docs.findOne .calledWith( { - _id: ObjectId(this.docId), - project_id: ObjectId(this.projectId), + _id: new ObjectId(this.docId), + project_id: new ObjectId(this.projectId), }, { projection: this.filter, @@ -86,8 +86,8 @@ describe('MongoManager', function () { it('should pass the parameter along', function () { this.db.docs.updateOne.should.have.been.calledWith( { - _id: ObjectId(this.docId), - project_id: ObjectId(this.projectId), + _id: new ObjectId(this.docId), + project_id: new ObjectId(this.projectId), }, { $set: this.meta, @@ -125,7 +125,7 @@ describe('MongoManager', function () { this.db.docs.find .calledWith( { - project_id: ObjectId(this.projectId), + project_id: new ObjectId(this.projectId), deleted: { $ne: true }, }, { @@ -156,7 +156,7 @@ describe('MongoManager', function () { this.db.docs.find .calledWith( { - project_id: ObjectId(this.projectId), + project_id: new ObjectId(this.projectId), }, { projection: this.filter, @@ -193,7 +193,7 @@ describe('MongoManager', function () { it('should find the deleted docs via the project_id', function () { this.db.docs.find .calledWith({ - project_id: ObjectId(this.projectId), + project_id: new ObjectId(this.projectId), deleted: true, }) .should.equal(true) @@ -231,8 +231,8 @@ describe('MongoManager', function () { assert.equal(err, null) const args = this.db.docs.updateOne.args[0] assert.deepEqual(args[0], { - _id: ObjectId(this.docId), - project_id: ObjectId(this.projectId), + _id: new ObjectId(this.docId), + project_id: new ObjectId(this.projectId), rev: this.oldRev, }) assert.equal(args[1].$set.lines, this.lines) @@ -264,8 +264,8 @@ describe('MongoManager', function () { { lines: this.lines, ranges: this.ranges }, err => { expect(this.db.docs.insertOne).to.have.been.calledWith({ - _id: ObjectId(this.docId), - project_id: ObjectId(this.projectId), + _id: new ObjectId(this.docId), + project_id: new ObjectId(this.projectId), rev: 1, lines: this.lines, ranges: this.ranges, @@ -307,8 +307,8 @@ describe('MongoManager', function () { describe('destroyProject', function () { beforeEach(function (done) { - this.projectId = ObjectId() - this.docIds = [ObjectId(), ObjectId()] + this.projectId = new ObjectId() + this.docIds = [new ObjectId(), new ObjectId()] this.db.docs.deleteMany = sinon.stub().yields() this.db.docOps.deleteMany = sinon.stub().yields() this.db.docs.find = sinon @@ -349,7 +349,7 @@ describe('MongoManager', function () { it('should look for the doc in the database', function () { this.db.docOps.findOne .calledWith( - { doc_id: ObjectId(this.docId) }, + { doc_id: new ObjectId(this.docId) }, { projection: { version: 1 }, } @@ -376,7 +376,7 @@ describe('MongoManager', function () { describe('checkRevUnchanged', function () { this.beforeEach(function () { - this.doc = { _id: ObjectId(), name: 'mock-doc', rev: 1 } + this.doc = { _id: new ObjectId(), name: 'mock-doc', rev: 1 } }) it('should call the callback when the rev has not changed', function (done) { @@ -397,7 +397,7 @@ describe('MongoManager', function () { it('should return a value error if incoming rev is NaN', function (done) { this.db.docs.findOne = sinon.stub().callsArgWith(2, null, { rev: 2 }) - this.doc = { _id: ObjectId(), name: 'mock-doc', rev: NaN } + this.doc = { _id: new ObjectId(), name: 'mock-doc', rev: NaN } this.MongoManager.checkRevUnchanged(this.doc, err => { err.should.be.instanceof(Errors.DocRevValueError) done() @@ -435,8 +435,8 @@ describe('MongoManager', function () { it('updates Mongo', function () { expect(this.db.docs.updateOne).to.have.been.calledWith( { - _id: ObjectId(this.docId), - project_id: ObjectId(this.projectId), + _id: new ObjectId(this.docId), + project_id: new ObjectId(this.projectId), rev: this.archivedDoc.rev, }, { @@ -466,8 +466,8 @@ describe('MongoManager', function () { it('sets ranges to an empty object', function () { expect(this.db.docs.updateOne).to.have.been.calledWith( { - _id: ObjectId(this.docId), - project_id: ObjectId(this.projectId), + _id: new ObjectId(this.docId), + project_id: new ObjectId(this.projectId), rev: this.archivedDoc.rev, }, { diff --git a/services/docstore/test/unit/js/RangeManagerTests.js b/services/docstore/test/unit/js/RangeManagerTests.js index fdba72292a..68f85bc882 100644 --- a/services/docstore/test/unit/js/RangeManagerTests.js +++ b/services/docstore/test/unit/js/RangeManagerTests.js @@ -31,10 +31,10 @@ describe('RangeManager', function () { describe('jsonRangesToMongo', function () { it('should convert ObjectIds and dates to proper objects', function () { - const changeId = ObjectId().toString() - const commentId = ObjectId().toString() - const userId = ObjectId().toString() - const threadId = ObjectId().toString() + const changeId = new ObjectId().toString() + const commentId = new ObjectId().toString() + const userId = new ObjectId().toString() + const threadId = new ObjectId().toString() const ts = new Date().toJSON() return this.RangeManager.jsonRangesToMongo({ changes: [ @@ -56,18 +56,18 @@ describe('RangeManager', function () { }).should.deep.equal({ changes: [ { - id: ObjectId(changeId), + id: new ObjectId(changeId), op: { i: 'foo', p: 3 }, metadata: { - user_id: ObjectId(userId), + user_id: new ObjectId(userId), ts: new Date(ts), }, }, ], comments: [ { - id: ObjectId(commentId), - op: { c: 'foo', p: 3, t: ObjectId(threadId) }, + id: new ObjectId(commentId), + op: { c: 'foo', p: 3, t: new ObjectId(threadId) }, }, ], }) @@ -109,10 +109,10 @@ describe('RangeManager', function () { }) return it('should be consistent when transformed through json -> mongo -> json', function () { - const changeId = ObjectId().toString() - const commentId = ObjectId().toString() - const userId = ObjectId().toString() - const threadId = ObjectId().toString() + const changeId = new ObjectId().toString() + const commentId = new ObjectId().toString() + const userId = new ObjectId().toString() + const threadId = new ObjectId().toString() const ts = new Date().toJSON() const ranges1 = { changes: [ @@ -145,18 +145,18 @@ describe('RangeManager', function () { this.ranges = { changes: [ { - id: ObjectId(), + id: new ObjectId(), op: { i: 'foo', p: 3 }, metadata: { - user_id: ObjectId(), + user_id: new ObjectId(), ts: new Date(), }, }, ], comments: [ { - id: ObjectId(), - op: { c: 'foo', p: 3, t: ObjectId() }, + id: new ObjectId(), + op: { c: 'foo', p: 3, t: new ObjectId() }, }, ], } @@ -194,7 +194,7 @@ describe('RangeManager', function () { return describe('with changes', function () { it('should return true when the change id changes', function () { - this.ranges_copy.changes[0].id = ObjectId() + this.ranges_copy.changes[0].id = new ObjectId() return this.RangeManager.shouldUpdateRanges( this.ranges, this.ranges_copy @@ -202,7 +202,7 @@ describe('RangeManager', function () { }) it('should return true when the change user id changes', function () { - this.ranges_copy.changes[0].metadata.user_id = ObjectId() + this.ranges_copy.changes[0].metadata.user_id = new ObjectId() return this.RangeManager.shouldUpdateRanges( this.ranges, this.ranges_copy @@ -226,7 +226,7 @@ describe('RangeManager', function () { }) it('should return true when the comment id changes', function () { - this.ranges_copy.comments[0].id = ObjectId() + this.ranges_copy.comments[0].id = new ObjectId() return this.RangeManager.shouldUpdateRanges( this.ranges, this.ranges_copy