From 5d2c09bf758b27b73cfe43d0036ee01a041b96cb Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Thu, 22 Jan 2015 15:05:48 +0000 Subject: [PATCH] read docs from doc collection first - removes include deleted flag as it is irrelivant now --- .../docstore/app/coffee/DocManager.coffee | 31 +++++------- .../docstore/app/coffee/HttpController.coffee | 3 +- .../acceptance/coffee/GettingDocsTests.coffee | 20 +++----- .../coffee/helpers/DocstoreClient.coffee | 7 +-- .../test/unit/coffee/DocManagerTests.coffee | 47 ++++++++++++------- .../unit/coffee/HttpControllerTests.coffee | 8 ++-- 6 files changed, 56 insertions(+), 60 deletions(-) diff --git a/services/docstore/app/coffee/DocManager.coffee b/services/docstore/app/coffee/DocManager.coffee index dedf3998f5..f1c856eb9b 100644 --- a/services/docstore/app/coffee/DocManager.coffee +++ b/services/docstore/app/coffee/DocManager.coffee @@ -5,24 +5,19 @@ _ = require "underscore" async = require "async" module.exports = DocManager = - getDoc: (project_id, doc_id, options, callback = (error, doc, mongoPath) ->) -> - if typeof(options) == "function" - callback = options - options.include_deleted = false + getDoc: (project_id, doc_id, callback = (error, doc, mongoPath) ->) -> - MongoManager.findProject project_id, (error, project) -> - return callback(error) if error? - return callback new Errors.NotFoundError("No such project: #{project_id}") if !project? - DocManager.findDocInProject project, doc_id, (error, doc, mongoPath) -> + MongoManager.findDoc doc_id, (err, docFromDocCollection)-> + return callback(err) if err? + MongoManager.findProject project_id, (error, project) -> return callback(error) if error? - if doc? - return callback null, doc, mongoPath - else - if options.include_deleted - MongoManager.findDoc doc_id, (error, doc) -> - return callback(error) if error? - return callback new Errors.NotFoundError("No such doc: #{project_id}") if !doc? - return callback null, doc + return callback new Errors.NotFoundError("No such project: #{project_id}") if !project? + DocManager.findDocInProject project, doc_id, (error, doc, mongoPath) -> + return callback(error) if error? + if docFromDocCollection? + return callback null, docFromDocCollection, mongoPath + else if doc? + return callback null, doc, mongoPath else return callback new Errors.NotFoundError("No such doc: #{project_id}") @@ -37,7 +32,7 @@ module.exports = DocManager = updateDoc: (project_id, doc_id, lines, callback = (error, modified, rev) ->) -> DocManager.getDoc project_id, doc_id, (error, doc, mongoPath) -> return callback(error) if error? - return callback new Errors.NotFoundError("No such project/doc: #{project_id}/#{doc_id}") if !doc? or !mongoPath? + return callback new Errors.NotFoundError("No such project/doc to update: #{project_id}/#{doc_id}") if !doc? or !mongoPath? if _.isEqual(doc.lines, lines) logger.log { @@ -65,7 +60,7 @@ module.exports = DocManager = deleteDoc: (project_id, doc_id, callback = (error) ->) -> DocManager.getDoc project_id, doc_id, (error, doc) -> return callback(error) if error? - return callback new Errors.NotFoundError("No such project/doc: #{project_id}/#{doc_id}") if !doc? + 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, doc.rev, (error) -> return callback(error) if error? MongoManager.markDocAsDeleted doc_id, (error) -> diff --git a/services/docstore/app/coffee/HttpController.coffee b/services/docstore/app/coffee/HttpController.coffee index 39a14620f7..2d54e69e3a 100644 --- a/services/docstore/app/coffee/HttpController.coffee +++ b/services/docstore/app/coffee/HttpController.coffee @@ -5,9 +5,8 @@ module.exports = HttpController = getDoc: (req, res, next = (error) ->) -> project_id = req.params.project_id 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, include_deleted: include_deleted, (error, doc) -> + DocManager.getDoc project_id, doc_id, (error, doc) -> return next(error) if error? logger.log doc: doc, "got doc" if !doc? diff --git a/services/docstore/test/acceptance/coffee/GettingDocsTests.coffee b/services/docstore/test/acceptance/coffee/GettingDocsTests.coffee index 406f300f4f..fe92f44d72 100644 --- a/services/docstore/test/acceptance/coffee/GettingDocsTests.coffee +++ b/services/docstore/test/acceptance/coffee/GettingDocsTests.coffee @@ -34,19 +34,13 @@ describe "Getting a doc", -> describe "when the doc is a deleted doc", -> beforeEach (done) -> - @deleted_doc_id = ObjectId() + @deleted_doc_id = ObjectId() DocstoreClient.createDeletedDoc @project_id, @deleted_doc_id, @lines, done - describe "with include_deleted=true", -> - it "should return the doc", (done) -> - DocstoreClient.getDoc @project_id, @deleted_doc_id, include_deleted: true, (error, res, doc) => - doc.lines.should.deep.equal @lines - doc.deleted.should.equal true - done() - - describe "without include_deleted=true", -> - it "should return 404", (done) -> - DocstoreClient.getDoc @project_id, @deleted_doc_id, (error, res, doc) => - res.statusCode.should.equal 404 - done() + it "should return the doc", (done) -> + DocstoreClient.getDoc @project_id, @deleted_doc_id, (error, res, doc) => + doc.lines.should.deep.equal @lines + doc.deleted.should.equal true + done() + \ No newline at end of file diff --git a/services/docstore/test/acceptance/coffee/helpers/DocstoreClient.coffee b/services/docstore/test/acceptance/coffee/helpers/DocstoreClient.coffee index 1ca3cc2aff..9bf763bbf0 100644 --- a/services/docstore/test/acceptance/coffee/helpers/DocstoreClient.coffee +++ b/services/docstore/test/acceptance/coffee/helpers/DocstoreClient.coffee @@ -31,12 +31,9 @@ module.exports = DocstoreClient = deleteProject: (project_id, callback = (error, res, body) ->) -> db.projects.remove _id: project_id, callback - getDoc: (project_id, doc_id, options, callback = (error, res, body) ->) -> - if typeof(options) == "function" - callback = options - options = { include_deleted: false } + getDoc: (project_id, doc_id, callback = (error, res, body) ->) -> request.get { - url: "http://localhost:3016/project/#{project_id}/doc/#{doc_id}?include_deleted=#{options.include_deleted}" + url: "http://localhost:3016/project/#{project_id}/doc/#{doc_id}" json: true }, callback diff --git a/services/docstore/test/unit/coffee/DocManagerTests.coffee b/services/docstore/test/unit/coffee/DocManagerTests.coffee index 14b342fb99..3172e8ea5b 100644 --- a/services/docstore/test/unit/coffee/DocManagerTests.coffee +++ b/services/docstore/test/unit/coffee/DocManagerTests.coffee @@ -15,14 +15,38 @@ describe "DocManager", -> @doc_id = ObjectId().toString() @project_id = ObjectId().toString() @callback = sinon.stub() + @stubbedError = new Error("blew up") describe "getDoc", -> + beforeEach -> + @project = { name: "mock-project" } + @doc = { _id: @doc_id, lines: ["mock-lines"] } + @docCollectionDoc = { _id: @doc_id, lines: ["mock-lines"] } + + describe "when the doc is in the doc collection not projects collection", -> + + beforeEach -> + @MongoManager.findDoc = sinon.stub() + @MongoManager.findProject = sinon.stub().callsArgWith(1, null, @project) + @DocManager.findDocInProject = sinon.stub().callsArgWith(2, null, @doc, @mongoPath) + + it "should get the doc from the doc collection when it is present there", (done)-> + @MongoManager.findDoc.callsArgWith(1, null, @docCollectionDoc) + @DocManager.getDoc @project_id, @doc_id, (err, doc)=> + doc.should.equal @docCollectionDoc + done() + + it "should return the error from find doc", (done)-> + @MongoManager.findDoc.callsArgWith(1, @stubbedError) + @DocManager.getDoc @project_id, @doc_id, (err, doc)=> + err.should.equal @stubbedError + done() + describe "when the project exists and the doc is in it", -> beforeEach -> - @project = { name: "mock-project" } - @doc = { _id: @doc_id, lines: ["mock-lines"] } @mongoPath = "mock.mongo.path" @MongoManager.findProject = sinon.stub().callsArgWith(1, null, @project) + @MongoManager.findDoc = sinon.stub().callsArg(1) @DocManager.findDocInProject = sinon.stub().callsArgWith(2, null, @doc, @mongoPath) @DocManager.getDoc @project_id, @doc_id, @callback @@ -42,6 +66,7 @@ describe "DocManager", -> describe "when the project does not exist", -> beforeEach -> @MongoManager.findProject = sinon.stub().callsArgWith(1, null, null) + @MongoManager.findDoc = sinon.stub().callsArg(1) @DocManager.findDocInProject = sinon.stub() @DocManager.getDoc @project_id, @doc_id, @callback @@ -59,9 +84,7 @@ describe "DocManager", -> @DocManager.findDocInProject = sinon.stub().callsArgWith(2, null, null, null) @MongoManager.findDoc = sinon.stub().callsArgWith(1, null, @doc) - describe "when include_deleted = true", -> - beforeEach -> - @DocManager.getDoc @project_id, @doc_id, include_deleted: true, @callback + @DocManager.getDoc @project_id, @doc_id, @callback it "should try to find the doc in the docs collection", -> @MongoManager.findDoc @@ -73,17 +96,7 @@ describe "DocManager", -> .calledWith(null, @doc) .should.equal true - describe "when include_deleted is not set", -> - beforeEach -> - @DocManager.getDoc @project_id, @doc_id, @callback - - it "should not try to find the doc in the docs collection", -> - @MongoManager.findDoc.called.should.equal false - - it "should return a NotFoundError", -> - @callback - .calledWith(new Errors.NotFoundError("No such doc: #{@doc_id}")) - .should.equal true + describe "when the doc does not exist anywhere", -> beforeEach -> @@ -124,7 +137,7 @@ describe "DocManager", -> beforeEach -> @MongoManager.findProject = sinon.stub().callsArgWith(1, null, null) @DocManager.findAllDocsInProject = sinon.stub() - @DocManager.getDoc @project_id, @doc_id, @callback + @DocManager.getAllDocs @project_id, @callback it "should not try to find the doc in the project", -> @DocManager.findAllDocsInProject.called.should.equal false diff --git a/services/docstore/test/unit/coffee/HttpControllerTests.coffee b/services/docstore/test/unit/coffee/HttpControllerTests.coffee index a41e6ced82..7ec4d046ce 100644 --- a/services/docstore/test/unit/coffee/HttpControllerTests.coffee +++ b/services/docstore/test/unit/coffee/HttpControllerTests.coffee @@ -29,7 +29,7 @@ describe "HttpController", -> @req.params = project_id: @project_id doc_id: @doc_id - @DocManager.getDoc = sinon.stub().callsArgWith(3, null, @doc) + @DocManager.getDoc = sinon.stub().callsArgWith(2, null, @doc) describe "without deleted docs", -> beforeEach -> @@ -37,7 +37,7 @@ describe "HttpController", -> it "should get the document (including deleted)", -> @DocManager.getDoc - .calledWith(@project_id, @doc_id, include_deleted: false) + .calledWith(@project_id, @doc_id) .should.equal true it "should return the doc as JSON", -> @@ -52,13 +52,11 @@ describe "HttpController", -> describe "with deleted docs", -> beforeEach -> - @req.query = - include_deleted: 'true' @HttpController.getDoc @req, @res, @next it "should get the document (without deleted)", -> @DocManager.getDoc - .calledWith(@project_id, @doc_id, include_deleted: true) + .calledWith(@project_id, @doc_id) .should.equal true describe "getRawDoc", ->