diff --git a/services/docstore/app/coffee/DocManager.coffee b/services/docstore/app/coffee/DocManager.coffee index c37b68152f..216002f8ee 100644 --- a/services/docstore/app/coffee/DocManager.coffee +++ b/services/docstore/app/coffee/DocManager.coffee @@ -9,7 +9,7 @@ module.exports = DocManager = # collection (which is all that this collection contains). In future, we should # migrate this version property to be part of the docs collection, to guarantee # consitency between lines and version when writing/reading, and for a simpler schema. - getDoc: (project_id, doc_id, callback = (error, doc) ->) -> + getDoc: (project_id, doc_id, filter = { version: false }, callback = (error, doc) ->) -> MongoManager.findDoc project_id, doc_id, (err, doc)-> if err? return callback(err) @@ -20,11 +20,14 @@ module.exports = DocManager = if err? logger.err err:err, project_id:project_id, doc_id:doc_id, "error unarchiving doc" return callback(err) - DocManager.getDoc project_id, doc_id, callback + DocManager.getDoc project_id, doc_id, filter, callback else - MongoManager.getDocVersion doc_id, (error, version) -> - return callback(error) if error? - doc.version = version + if filter.version + MongoManager.getDocVersion doc_id, (error, version) -> + return callback(error) if error? + doc.version = version + callback err, doc + else callback err, doc getAllDocs: (project_id, callback = (error, docs) ->) -> @@ -38,7 +41,7 @@ module.exports = DocManager = return callback(null, docs) updateDoc: (project_id, doc_id, lines, version, callback = (error, modified, rev) ->) -> - DocManager.getDoc project_id, doc_id, (err, doc)-> + DocManager.getDoc project_id, doc_id, {version: true}, (err, doc)-> if err? and !(err instanceof Errors.NotFoundError) logger.err project_id: project_id, doc_id: doc_id, err:err, "error getting document for update" return callback(err) @@ -78,7 +81,7 @@ module.exports = DocManager = callback null, true, oldRev + 1 # rev will have been incremented in mongo by MongoManager.updateDoc deleteDoc: (project_id, doc_id, callback = (error) ->) -> - DocManager.getDoc project_id, doc_id, (error, doc) -> + DocManager.getDoc project_id, doc_id, {}, (error, doc) -> return callback(error) if error? return callback new Errors.NotFoundError("No such project/doc to delete: #{project_id}/#{doc_id}") if !doc? MongoManager.upsertIntoDocCollection project_id, doc_id, doc.lines, (error) -> diff --git a/services/docstore/app/coffee/HttpController.coffee b/services/docstore/app/coffee/HttpController.coffee index 0dfb30c31e..e87d89ea64 100644 --- a/services/docstore/app/coffee/HttpController.coffee +++ b/services/docstore/app/coffee/HttpController.coffee @@ -10,7 +10,7 @@ module.exports = HttpController = doc_id = req.params.doc_id include_deleted = req.query?.include_deleted == "true" logger.log project_id: project_id, doc_id: doc_id, "getting doc" - DocManager.getDoc project_id, doc_id, (error, doc) -> + DocManager.getDoc project_id, doc_id, {version: true}, (error, doc) -> return next(error) if error? logger.log doc: doc, "got doc" if !doc? @@ -24,7 +24,7 @@ module.exports = HttpController = project_id = req.params.project_id doc_id = req.params.doc_id logger.log project_id: project_id, doc_id: doc_id, "getting raw doc" - DocManager.getDoc project_id, doc_id, (error, doc) -> + DocManager.getDoc project_id, doc_id, {version: false}, (error, doc) -> return next(error) if error? if !doc? res.send 404 diff --git a/services/docstore/test/unit/coffee/DocManagerTests.coffee b/services/docstore/test/unit/coffee/DocManagerTests.coffee index d16c30206b..58f26abaf3 100644 --- a/services/docstore/test/unit/coffee/DocManagerTests.coffee +++ b/services/docstore/test/unit/coffee/DocManagerTests.coffee @@ -33,7 +33,7 @@ describe "DocManager", -> describe "when the doc is in the doc collection", -> beforeEach -> @MongoManager.findDoc.yields(null, @doc) - @DocManager.getDoc @project_id, @doc_id, @callback + @DocManager.getDoc @project_id, @doc_id, {version: true}, @callback it "should get the doc from the doc collection", -> @MongoManager.findDoc @@ -50,11 +50,19 @@ describe "DocManager", -> doc = @callback.args[0][1] doc.lines.should.equal @doc.lines doc.version.should.equal @version + + describe "without the version filter", -> + beforeEach -> + @MongoManager.findDoc.yields(null, @doc) + @DocManager.getDoc @project_id, @doc_id, {version: false}, @callback + + it "should not get the doc version from the docOps collection", -> + @MongoManager.getDocVersion.called.should.equal false describe "when MongoManager.findDoc errors", -> beforeEach -> @MongoManager.findDoc.yields(@stubbedError) - @DocManager.getDoc @project_id, @doc_id, @callback + @DocManager.getDoc @project_id, @doc_id, {version: true}, @callback it "should return the error", -> @callback.calledWith(@stubbedError).should.equal true @@ -67,7 +75,7 @@ describe "DocManager", -> @doc.inS3 = false callback() sinon.spy @DocArchiveManager, "unarchiveDoc" - @DocManager.getDoc @project_id, @doc_id, @callback + @DocManager.getDoc @project_id, @doc_id, {version: true}, @callback it "should call the DocArchive to unarchive the doc", -> @DocArchiveManager.unarchiveDoc.calledWith(@project_id, @doc_id).should.equal true @@ -81,7 +89,7 @@ describe "DocManager", -> describe "when the doc does not exist in the docs collection", -> beforeEach -> @MongoManager.findDoc = sinon.stub().callsArgWith(2, null, null) - @DocManager.getDoc @project_id, @doc_id, @callback + @DocManager.getDoc @project_id, @doc_id, {version: true}, @callback it "should return a NotFoundError", -> @callback @@ -120,7 +128,7 @@ describe "DocManager", -> beforeEach -> @lines = ["mock", "doc", "lines"] @rev = 77 - @DocManager.getDoc = sinon.stub().callsArgWith(2, null, {lines: @lines, rev:@rev}) + @DocManager.getDoc = sinon.stub().callsArgWith(3, null, {lines: @lines, rev:@rev}) @MongoManager.upsertIntoDocCollection = sinon.stub().callsArg(3) @MongoManager.markDocAsDeleted = sinon.stub().callsArg(1) @DocManager.deleteDoc @project_id, @doc_id, @callback @@ -145,7 +153,7 @@ describe "DocManager", -> describe "when the doc does not exist", -> beforeEach -> - @DocManager.getDoc = sinon.stub().callsArgWith(2, null, null) + @DocManager.getDoc = sinon.stub().callsArgWith(3, null, null) @MongoManager.upsertIntoDocCollection = sinon.stub() @DocManager.deleteDoc @project_id, @doc_id, @callback @@ -170,7 +178,7 @@ describe "DocManager", -> describe "when the doc lines have changed", -> beforeEach -> - @DocManager.getDoc = sinon.stub().callsArgWith(2, null, @doc) + @DocManager.getDoc = sinon.stub().callsArgWith(3, null, @doc) @DocManager.updateDoc @project_id, @doc_id, @newDocLines, @version, @callback it "should get the existing doc", -> @@ -207,12 +215,12 @@ describe "DocManager", -> describe "when the version has changed", -> beforeEach -> - @DocManager.getDoc = sinon.stub().callsArgWith(2, null, @doc) + @DocManager.getDoc = sinon.stub().callsArgWith(3, null, @doc) @DocManager.updateDoc @project_id, @doc_id, @oldDocLines, @version + 1, @callback - it "should get the existing doc", -> + it "should get the existing doc with the version", -> @DocManager.getDoc - .calledWith(@project_id, @doc_id) + .calledWith(@project_id, @doc_id, {version: true}) .should.equal true it "should upsert the document to the doc collection", -> @@ -230,12 +238,12 @@ describe "DocManager", -> describe "when the version is null and the lines are different", -> beforeEach -> - @DocManager.getDoc = sinon.stub().callsArgWith(2, null, @doc) + @DocManager.getDoc = sinon.stub().callsArgWith(3, null, @doc) @DocManager.updateDoc @project_id, @doc_id, @newDocLines, null, @callback - it "should get the existing doc", -> + it "should get the existing doc with the version", -> @DocManager.getDoc - .calledWith(@project_id, @doc_id) + .calledWith(@project_id, @doc_id, {version: true}) .should.equal true it "should upsert the document to the doc collection", -> @@ -253,7 +261,7 @@ describe "DocManager", -> describe "when the version is null and the lines are the same", -> beforeEach -> - @DocManager.getDoc = sinon.stub().callsArgWith(2, null, @doc) + @DocManager.getDoc = sinon.stub().callsArgWith(3, null, @doc) @DocManager.updateDoc @project_id, @doc_id, @oldDocLines, null, @callback it "should not update the version", -> @@ -268,7 +276,7 @@ describe "DocManager", -> describe "when there is a generic error getting the doc", -> beforeEach -> @error = new Error("doc could not be found") - @DocManager.getDoc = sinon.stub().callsArgWith(2, @error, null, null) + @DocManager.getDoc = sinon.stub().callsArgWith(3, @error, null, null) @DocManager.updateDoc @project_id, @doc_id, @newDocLines, @version, @callback it "should not upsert the document to the doc collection", -> @@ -279,7 +287,7 @@ describe "DocManager", -> describe "when the doc lines have not changed", -> beforeEach -> - @DocManager.getDoc = sinon.stub().callsArgWith(2, null, @doc) + @DocManager.getDoc = sinon.stub().callsArgWith(3, null, @doc) @DocManager.updateDoc @project_id, @doc_id, @oldDocLines.slice(), @version, @callback it "should not update the doc", -> @@ -291,7 +299,7 @@ describe "DocManager", -> describe "when the doc lines are an empty array", -> beforeEach -> @doc.lines = [] - @DocManager.getDoc = sinon.stub().callsArgWith(2, null, @doc) + @DocManager.getDoc = sinon.stub().callsArgWith(3, null, @doc) @DocManager.updateDoc @project_id, @doc_id, @doc.lines, @callback it "should upsert the document to the doc collection", -> @@ -301,7 +309,7 @@ describe "DocManager", -> describe "when the doc does not exist", -> beforeEach -> - @DocManager.getDoc = sinon.stub().callsArgWith(2, null, null, null) + @DocManager.getDoc = sinon.stub().callsArgWith(3, null, null, null) @DocManager.updateDoc @project_id, @doc_id, @newDocLines, @version, @callback it "should upsert the document to the doc collection", -> diff --git a/services/docstore/test/unit/coffee/HttpControllerTests.coffee b/services/docstore/test/unit/coffee/HttpControllerTests.coffee index 4d9ae795ab..a34237982e 100644 --- a/services/docstore/test/unit/coffee/HttpControllerTests.coffee +++ b/services/docstore/test/unit/coffee/HttpControllerTests.coffee @@ -40,12 +40,12 @@ describe "HttpController", -> @req.params = project_id: @project_id doc_id: @doc_id - @DocManager.getDoc = sinon.stub().callsArgWith(2, null, @doc) + @DocManager.getDoc = sinon.stub().callsArgWith(3, null, @doc) @HttpController.getDoc @req, @res, @next - it "should get the document (including deleted)", -> + it "should get the document with the version (including deleted)", -> @DocManager.getDoc - .calledWith(@project_id, @doc_id) + .calledWith(@project_id, @doc_id, {version: true}) .should.equal true it "should return the doc as JSON", -> @@ -64,11 +64,11 @@ describe "HttpController", -> @req.params = project_id: @project_id doc_id: @doc_id - @DocManager.getDoc = sinon.stub().callsArgWith(2, null, @deletedDoc) + @DocManager.getDoc = sinon.stub().callsArgWith(3, null, @deletedDoc) it "should get the doc from the doc manager", -> @HttpController.getDoc @req, @res, @next - @DocManager.getDoc.calledWith(@project_id, @doc_id).should.equal true + @DocManager.getDoc.calledWith(@project_id, @doc_id, {version: true}).should.equal true it "should return 404 if the query string delete is not set ", -> @HttpController.getDoc @req, @res, @next @@ -92,12 +92,12 @@ describe "HttpController", -> @req.params = project_id: @project_id doc_id: @doc_id - @DocManager.getDoc = sinon.stub().callsArgWith(2, null, @doc) + @DocManager.getDoc = sinon.stub().callsArgWith(3, null, @doc) @HttpController.getRawDoc @req, @res, @next - it "should get the document", -> + it "should get the document without the version", -> @DocManager.getDoc - .calledWith(@project_id, @doc_id) + .calledWith(@project_id, @doc_id, {version: false}) .should.equal true it "should set the content type header", ->