From 1f3b0a371316ff12ca392d83be158b0b9fea277b Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Tue, 22 Aug 2017 08:26:30 +0100 Subject: [PATCH 1/2] Upgrade mongojs dependency to version 2.4.0 - Upgrade in package.json - Use new connection syntax - Fix unit tests, issues with object equality --- services/docstore/app/coffee/mongojs.coffee | 2 +- services/docstore/package.json | 2 +- .../test/unit/coffee/MongoManagerTests.coffee | 50 ++++++++----------- 3 files changed, 22 insertions(+), 32 deletions(-) diff --git a/services/docstore/app/coffee/mongojs.coffee b/services/docstore/app/coffee/mongojs.coffee index db3d3c00d8..0153c3cfcd 100644 --- a/services/docstore/app/coffee/mongojs.coffee +++ b/services/docstore/app/coffee/mongojs.coffee @@ -1,6 +1,6 @@ Settings = require "settings-sharelatex" mongojs = require "mongojs" -db = mongojs.connect(Settings.mongo.url, ["docs", "docOps"]) +db = mongojs(Settings.mongo.url, ["docs", "docOps"]) module.exports = db: db ObjectId: mongojs.ObjectId diff --git a/services/docstore/package.json b/services/docstore/package.json index dacc42948e..4d6a7e379a 100644 --- a/services/docstore/package.json +++ b/services/docstore/package.json @@ -11,7 +11,7 @@ "settings-sharelatex": "git+https://github.com/sharelatex/settings-sharelatex.git#v1.0.0", "logger-sharelatex": "git+https://github.com/sharelatex/logger-sharelatex.git#v1.4.0", "metrics-sharelatex": "git+https://github.com/sharelatex/metrics-sharelatex.git#v1.7.1", - "mongojs": "0.18.2", + "mongojs": "2.4.0", "express": "~4.1.1", "underscore": "~1.6.0", "body-parser": "~1.0.2", diff --git a/services/docstore/test/unit/coffee/MongoManagerTests.coffee b/services/docstore/test/unit/coffee/MongoManagerTests.coffee index 1a264bf0d2..8b65402ac3 100644 --- a/services/docstore/test/unit/coffee/MongoManagerTests.coffee +++ b/services/docstore/test/unit/coffee/MongoManagerTests.coffee @@ -26,12 +26,10 @@ describe "MongoManager", -> @MongoManager.findDoc @project_id, @doc_id, @filter, @callback it "should find the doc", -> - @db.docs.find - .calledWith({ - _id: ObjectId(@doc_id) - project_id: ObjectId(@project_id) - }, @filter) - .should.equal true + @db.docs.find.lastCall.args.slice(0,2).should.deep.equal([ + {_id: ObjectId(@doc_id), project_id: ObjectId(@project_id)}, + @filter + ]) it "should call the callback with the doc", -> @callback.calledWith(null, @doc).should.equal true @@ -50,12 +48,9 @@ describe "MongoManager", -> @MongoManager.getProjectsDocs @project_id, include_deleted: false, @filter, @callback it "should find the non-deleted docs via the project_id", -> - @db.docs.find - .calledWith({ - project_id: ObjectId(@project_id) - deleted: { $ne: true } - }, @filter) - .should.equal true + @db.docs.find.lastCall.args.slice(0,1).should.deep.equal([ + {project_id: ObjectId(@project_id), deleted: {$ne: true}} + ]) it "should call the callback with the docs", -> @callback.calledWith(null, [@doc, @doc3, @doc4]).should.equal true @@ -65,11 +60,10 @@ describe "MongoManager", -> @MongoManager.getProjectsDocs @project_id, include_deleted: true, @filter, @callback it "should find all via the project_id", -> - @db.docs.find - .calledWith({ - project_id: ObjectId(@project_id) - }, @filter) - .should.equal true + @db.docs.find.lastCall.args.slice(0,2).should.deep.equal([ + {project_id: ObjectId(@project_id)}, + @filter + ]) it "should call the callback with the docs", -> @callback.calledWith(null, [@doc, @doc3, @doc4]).should.equal true @@ -119,9 +113,10 @@ describe "MongoManager", -> @MongoManager.getDocVersion @doc_id, @callback it "should look for the doc in the database", -> - @db.docOps.find - .calledWith({ doc_id: ObjectId(@doc_id) }, {version: 1}) - .should.equal true + @db.docOps.find.lastCall.args.slice(0,2).should.deep.equal([ + { doc_id: ObjectId(@doc_id) }, + {version: 1} + ]) it "should call the callback with the version", -> @callback.calledWith(null, @version).should.equal true @@ -141,16 +136,11 @@ describe "MongoManager", -> @MongoManager.setDocVersion @doc_id, @version, @callback it "should update the doc version", -> - @db.docOps.update - .calledWith({ - doc_id: ObjectId(@doc_id) - }, { - $set: - version: @version - }, { - upsert: true - }) - .should.equal true + @db.docOps.update.lastCall.args.slice(0,3).should.deep.equal([ + {doc_id: ObjectId(@doc_id)}, + {$set: {version: @version}}, + {upsert: true} + ]) it "should call the callback", -> @callback.called.should.equal true From 65697cb2f6f322882bfbf23e0938ae89ffbb1932 Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Tue, 22 Aug 2017 15:20:43 +0100 Subject: [PATCH 2/2] Upgrade sinonjs, and fix unit tests. - use `sinon.match.has` for error matching - restore previous use of `calledWith` --- services/docstore/package.json | 2 +- .../test/unit/coffee/DocManagerTests.coffee | 12 ++--- .../unit/coffee/HttpControllerTests.coffee | 4 +- .../test/unit/coffee/MongoManagerTests.coffee | 50 +++++++++++-------- 4 files changed, 39 insertions(+), 29 deletions(-) diff --git a/services/docstore/package.json b/services/docstore/package.json index 4d6a7e379a..40a866c3cb 100644 --- a/services/docstore/package.json +++ b/services/docstore/package.json @@ -26,7 +26,7 @@ "grunt": "~0.4.4", "bunyan": "~0.22.3", "grunt-bunyan": "~0.5.0", - "sinon": "~1.5.2", + "sinon": "~3.2.1", "sandboxed-module": "~0.3.0", "chai": "~1.9.1", "grunt-forever": "~0.4.4", diff --git a/services/docstore/test/unit/coffee/DocManagerTests.coffee b/services/docstore/test/unit/coffee/DocManagerTests.coffee index 082badd0f3..70cea2e2c8 100644 --- a/services/docstore/test/unit/coffee/DocManagerTests.coffee +++ b/services/docstore/test/unit/coffee/DocManagerTests.coffee @@ -184,7 +184,7 @@ describe "DocManager", -> it "should return a NotFoundError", -> @callback - .calledWith(new Errors.NotFoundError("No such doc: #{@doc_id} in project #{@project_id}")) + .calledWith(sinon.match.has('message', "No such doc: #{@doc_id} in project #{@project_id}")) .should.equal true describe "getAllNonDeletedDocs", -> @@ -212,7 +212,7 @@ describe "DocManager", -> it "should return a NotFoundError", -> @callback - .calledWith(new Errors.NotFoundError("No docs for project #{@project_id}")) + .calledWith(sinon.match.has('message', "No docs for project #{@project_id}")) .should.equal true describe "deleteDoc", -> @@ -244,7 +244,7 @@ describe "DocManager", -> it "should return a NotFoundError", -> @callback - .calledWith(new Errors.NotFoundError("No such doc: #{@doc_id}")) + .calledWith(sinon.match.has('message', "No such project/doc to delete: #{@project_id}/#{@doc_id}")) .should.equal true describe "updateDoc", -> @@ -349,21 +349,21 @@ describe "DocManager", -> @DocManager.updateDoc @project_id, @doc_id, @newDocLines, null, @originalRanges, @callback it "should return an error", -> - @callback.calledWith(new Error("no lines, version or ranges provided")).should.equal true + @callback.calledWith(sinon.match.has('message', "no lines, version or ranges provided")).should.equal true describe "when the lines are null", -> beforeEach -> @DocManager.updateDoc @project_id, @doc_id, null, @version, @originalRanges, @callback it "should return an error", -> - @callback.calledWith(new Error("no lines, version or ranges provided")).should.equal true + @callback.calledWith(sinon.match.has('message', "no lines, version or ranges provided")).should.equal true describe "when the ranges are null", -> beforeEach -> @DocManager.updateDoc @project_id, @doc_id, @newDocLines, @version, null, @callback it "should return an error", -> - @callback.calledWith(new Error("no lines, version or ranges provided")).should.equal true + @callback.calledWith(sinon.match.has('message', "no lines, version or ranges provided")).should.equal true describe "when there is a generic error getting the doc", -> beforeEach -> diff --git a/services/docstore/test/unit/coffee/HttpControllerTests.coffee b/services/docstore/test/unit/coffee/HttpControllerTests.coffee index 0f32f4b9e3..80ec0c64fe 100644 --- a/services/docstore/test/unit/coffee/HttpControllerTests.coffee +++ b/services/docstore/test/unit/coffee/HttpControllerTests.coffee @@ -174,7 +174,7 @@ describe "HttpController", -> it "should log out an error", -> @logger.error .calledWith( - err: new Error("null doc") + err: sinon.match.has('message', "null doc") project_id: @project_id "encountered null doc" ) @@ -325,4 +325,4 @@ describe "HttpController", -> it "should return a 204 (No Content)", -> @res.send .calledWith(204) - .should.equal true \ No newline at end of file + .should.equal true diff --git a/services/docstore/test/unit/coffee/MongoManagerTests.coffee b/services/docstore/test/unit/coffee/MongoManagerTests.coffee index 8b65402ac3..1a264bf0d2 100644 --- a/services/docstore/test/unit/coffee/MongoManagerTests.coffee +++ b/services/docstore/test/unit/coffee/MongoManagerTests.coffee @@ -26,10 +26,12 @@ describe "MongoManager", -> @MongoManager.findDoc @project_id, @doc_id, @filter, @callback it "should find the doc", -> - @db.docs.find.lastCall.args.slice(0,2).should.deep.equal([ - {_id: ObjectId(@doc_id), project_id: ObjectId(@project_id)}, - @filter - ]) + @db.docs.find + .calledWith({ + _id: ObjectId(@doc_id) + project_id: ObjectId(@project_id) + }, @filter) + .should.equal true it "should call the callback with the doc", -> @callback.calledWith(null, @doc).should.equal true @@ -48,9 +50,12 @@ describe "MongoManager", -> @MongoManager.getProjectsDocs @project_id, include_deleted: false, @filter, @callback it "should find the non-deleted docs via the project_id", -> - @db.docs.find.lastCall.args.slice(0,1).should.deep.equal([ - {project_id: ObjectId(@project_id), deleted: {$ne: true}} - ]) + @db.docs.find + .calledWith({ + project_id: ObjectId(@project_id) + deleted: { $ne: true } + }, @filter) + .should.equal true it "should call the callback with the docs", -> @callback.calledWith(null, [@doc, @doc3, @doc4]).should.equal true @@ -60,10 +65,11 @@ describe "MongoManager", -> @MongoManager.getProjectsDocs @project_id, include_deleted: true, @filter, @callback it "should find all via the project_id", -> - @db.docs.find.lastCall.args.slice(0,2).should.deep.equal([ - {project_id: ObjectId(@project_id)}, - @filter - ]) + @db.docs.find + .calledWith({ + project_id: ObjectId(@project_id) + }, @filter) + .should.equal true it "should call the callback with the docs", -> @callback.calledWith(null, [@doc, @doc3, @doc4]).should.equal true @@ -113,10 +119,9 @@ describe "MongoManager", -> @MongoManager.getDocVersion @doc_id, @callback it "should look for the doc in the database", -> - @db.docOps.find.lastCall.args.slice(0,2).should.deep.equal([ - { doc_id: ObjectId(@doc_id) }, - {version: 1} - ]) + @db.docOps.find + .calledWith({ doc_id: ObjectId(@doc_id) }, {version: 1}) + .should.equal true it "should call the callback with the version", -> @callback.calledWith(null, @version).should.equal true @@ -136,11 +141,16 @@ describe "MongoManager", -> @MongoManager.setDocVersion @doc_id, @version, @callback it "should update the doc version", -> - @db.docOps.update.lastCall.args.slice(0,3).should.deep.equal([ - {doc_id: ObjectId(@doc_id)}, - {$set: {version: @version}}, - {upsert: true} - ]) + @db.docOps.update + .calledWith({ + doc_id: ObjectId(@doc_id) + }, { + $set: + version: @version + }, { + upsert: true + }) + .should.equal true it "should call the callback", -> @callback.called.should.equal true