From 704bf53fe533c7e82cd80fd401b3b75d68e07393 Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Tue, 20 Jan 2015 17:09:51 +0000 Subject: [PATCH 1/2] Write docs to do collection and project collection - docs are written to project collection then doc collection - deletes used to insert into doc collection, now its the same upsert with deleted = true - does not set delted = false for new docs as it is not needed, save space - changed delete to be seperate update on mongo --- .../docstore/app/coffee/DocManager.coffee | 16 ++++-- .../docstore/app/coffee/MongoManager.coffee | 19 +++++-- .../test/unit/coffee/DocManagerTests.coffee | 32 ++++++++---- .../test/unit/coffee/MongoManagerTests.coffee | 51 ++++++++++++++----- 4 files changed, 86 insertions(+), 32 deletions(-) diff --git a/services/docstore/app/coffee/DocManager.coffee b/services/docstore/app/coffee/DocManager.coffee index d789554d1b..a104129525 100644 --- a/services/docstore/app/coffee/DocManager.coffee +++ b/services/docstore/app/coffee/DocManager.coffee @@ -2,6 +2,7 @@ MongoManager = require "./MongoManager" Errors = require "./Errors" logger = require "logger-sharelatex" _ = require "underscore" +async = require "async" module.exports = DocManager = getDoc: (project_id, doc_id, options, callback = (error, doc, mongoPath) ->) -> @@ -51,7 +52,13 @@ module.exports = DocManager = newDocLines: lines rev: doc.rev }, "updating doc lines" - MongoManager.updateDoc project_id, mongoPath, lines, (error) -> + async.series [ + (cb)-> + # project collection is still primary so that needs to be successful first + MongoManager.updateDoc project_id, mongoPath, lines, cb + (cb)-> + MongoManager.upsertIntoDocCollection project_id, doc_id, lines, doc.rev, cb + ], (error)-> return callback(error) if error? callback null, true, doc.rev + 1 # rev will have been incremented in mongo by MongoManager.updateDoc @@ -59,9 +66,10 @@ module.exports = DocManager = 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? - MongoManager.insertDoc project_id, doc_id, { lines: doc.lines, deleted: true }, (error) -> - return callback(error) if error? - callback() + MongoManager.upsertIntoDocCollection project_id, doc_id, doc.lines, doc.rev, (error) -> + MongoManager.markDocAsDeleted doc_id, (error) -> + return callback(error) if error? + callback() findAllDocsInProject: (project, callback = (error, docs) ->) -> callback null, @_findAllDocsInFolder project.rootFolder[0] diff --git a/services/docstore/app/coffee/MongoManager.coffee b/services/docstore/app/coffee/MongoManager.coffee index 82db106d96..05a61e4ccf 100644 --- a/services/docstore/app/coffee/MongoManager.coffee +++ b/services/docstore/app/coffee/MongoManager.coffee @@ -18,7 +18,18 @@ module.exports = MongoManager = db.projects.update _id: ObjectId(project_id), update, callback - insertDoc: (project_id, doc_id, attributes, callback = (error) ->) -> - attributes._id = ObjectId(doc_id) - attributes.project_id = ObjectId(project_id) - db.docs.insert attributes, callback \ No newline at end of file + upsertIntoDocCollection: (project_id, doc_id, lines, oldRev, callback)-> + update = + $set:{} + update.$set["lines"] = lines + update.$set["project_id"] = ObjectId(project_id) + update.$set["rev"] = oldRev + 1 + db.docs.update _id: ObjectId(doc_id), update, {upsert: true}, callback + + + markDocAsDeleted: (doc_id, callback)-> + update = + $set: {} + update.$set["deleted"] = true + db.docs.update _id: ObjectId(doc_id), update, (err)-> + callback(err) diff --git a/services/docstore/test/unit/coffee/DocManagerTests.coffee b/services/docstore/test/unit/coffee/DocManagerTests.coffee index 0a296c4617..14b342fb99 100644 --- a/services/docstore/test/unit/coffee/DocManagerTests.coffee +++ b/services/docstore/test/unit/coffee/DocManagerTests.coffee @@ -138,8 +138,10 @@ describe "DocManager", -> describe "when the doc exists", -> beforeEach -> @lines = ["mock", "doc", "lines"] - @DocManager.getDoc = sinon.stub().callsArgWith(2, null, lines: @lines) - @MongoManager.insertDoc = sinon.stub().callsArg(3) + @rev = 77 + @DocManager.getDoc = sinon.stub().callsArgWith(2, null, {lines: @lines, rev:@rev}) + @MongoManager.upsertIntoDocCollection = sinon.stub().callsArg(4) + @MongoManager.markDocAsDeleted = sinon.stub().callsArg(1) @DocManager.deleteDoc @project_id, @doc_id, @callback it "should get the doc", -> @@ -147,12 +149,14 @@ describe "DocManager", -> .calledWith(@project_id, @doc_id) .should.equal true - it "insert the doc as a deleted doc", -> - @MongoManager.insertDoc - .calledWith(@project_id, @doc_id, { - lines: @lines - deleted: true - }) + it "should update the doc lines", -> + @MongoManager.upsertIntoDocCollection + .calledWith(@project_id, @doc_id, @lines, @rev) + .should.equal true + + it "should mark doc as deleted", -> + @MongoManager.markDocAsDeleted + .calledWith(@doc_id) .should.equal true it "should return the callback", -> @@ -161,11 +165,11 @@ describe "DocManager", -> describe "when the doc does not exist", -> beforeEach -> @DocManager.getDoc = sinon.stub().callsArgWith(2, null, null) - @MongoManager.insertDoc = sinon.stub().callsArg(3) + @MongoManager.upsertIntoDocCollection = sinon.stub() @DocManager.deleteDoc @project_id, @doc_id, @callback it "should not try to insert a deleted doc", -> - @MongoManager.insertDoc.called.should.equal false + @MongoManager.upsertIntoDocCollection.called.should.equal false it "should return a NotFoundError", -> @callback @@ -180,6 +184,7 @@ describe "DocManager", -> @mongoPath = "mock.mongo.path" @MongoManager.updateDoc = sinon.stub().callsArg(3) + @MongoManager.upsertIntoDocCollection = sinon.stub().callsArg(4) describe "when the doc lines have changed", -> beforeEach -> @@ -196,6 +201,11 @@ describe "DocManager", -> .calledWith(@project_id, @mongoPath, @newDocLines) .should.equal true + it "should upsert the document to the doc collection", -> + @MongoManager.upsertIntoDocCollection + .calledWith(@project_id, @doc_id, @newDocLines, @rev) + .should.equal true + it "should log out the old and new doc lines", -> @logger.log .calledWith( @@ -218,6 +228,7 @@ describe "DocManager", -> it "should not update the doc", -> @MongoManager.updateDoc.called.should.equal false + @MongoManager.upsertIntoDocCollection.called.should.equal false it "should return the callback with the existing rev", -> @callback.calledWith(null, false, @rev).should.equal true @@ -229,6 +240,7 @@ describe "DocManager", -> it "should not try to update the doc", -> @MongoManager.updateDoc.called.should.equal false + @MongoManager.upsertIntoDocCollection.called.should.equal false it "should return a NotFoundError", -> @callback diff --git a/services/docstore/test/unit/coffee/MongoManagerTests.coffee b/services/docstore/test/unit/coffee/MongoManagerTests.coffee index c8e8478f58..aac8447a4c 100644 --- a/services/docstore/test/unit/coffee/MongoManagerTests.coffee +++ b/services/docstore/test/unit/coffee/MongoManagerTests.coffee @@ -3,6 +3,7 @@ sinon = require('sinon') require('chai').should() modulePath = require('path').join __dirname, '../../../app/js/MongoManager' ObjectId = require("mongojs").ObjectId +assert = require("chai").assert describe "MongoManager", -> beforeEach -> @@ -13,6 +14,7 @@ describe "MongoManager", -> @project_id = ObjectId().toString() @doc_id = ObjectId().toString() @callback = sinon.stub() + @stubbedErr = new Error("hello world") describe "findProject", -> beforeEach -> @@ -68,20 +70,41 @@ describe "MongoManager", -> it "should call the callback with the project", -> @callback.called.should.equal true - describe "insertDoc", -> + + describe "upsertIntoDocCollection", -> beforeEach -> - @lines = ["mock-lines"] - @db.docs.insert = sinon.stub().callsArg(1) - @MongoManager.insertDoc @project_id, @doc_id, lines: @lines, @callback + @db.docs.update = sinon.stub().callsArgWith(3, @stubbedErr) + @oldRev = 77 - it "should insert the attributes with the given doc and project id", -> - @db.docs.insert - .calledWith({ - _id: ObjectId(@doc_id) - project_id: ObjectId(@project_id) - lines: @lines - }) - .should.equal true + it "should upsert the document", (done)-> + @MongoManager.upsertIntoDocCollection @project_id, @doc_id, @lines, @oldRev, (err)=> + args = @db.docs.update.args[0] + assert.deepEqual args[0], {_id: ObjectId(@doc_id)} + assert.equal args[1]["$set"]["lines"], @lines + assert.equal args[1]["$set"]["rev"], 78 + assert.deepEqual args[1]["$set"]["project_id"], ObjectId(@project_id) + done() - it "should call the callback", -> - @callback.called.should.equal true \ No newline at end of file + it "should return the error", (done)-> + @MongoManager.upsertIntoDocCollection @project_id, @doc_id, @lines, @oldRev, (err)=> + err.should.equal @stubbedErr + done() + + describe "markDocAsDeleted", -> + beforeEach -> + @db.docs.update = sinon.stub().callsArgWith(2, @stubbedErr) + @oldRev = 77 + + it "should process the update", (done)-> + @MongoManager.markDocAsDeleted @doc_id, (err)=> + args = @db.docs.update.args[0] + assert.deepEqual args[0], {_id: ObjectId(@doc_id)} + assert.equal args[1]["$set"]["deleted"], true + done() + + it "should return the error", (done)-> + @MongoManager.markDocAsDeleted @doc_id, (err)=> + err.should.equal @stubbedErr + done() + + \ No newline at end of file From 535d4435bfcc3b3e3159d75e9f30d4986b7b2e1d Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Wed, 21 Jan 2015 12:31:06 +0000 Subject: [PATCH 2/2] added missing err check --- services/docstore/app/coffee/DocManager.coffee | 1 + 1 file changed, 1 insertion(+) diff --git a/services/docstore/app/coffee/DocManager.coffee b/services/docstore/app/coffee/DocManager.coffee index a104129525..dedf3998f5 100644 --- a/services/docstore/app/coffee/DocManager.coffee +++ b/services/docstore/app/coffee/DocManager.coffee @@ -67,6 +67,7 @@ module.exports = DocManager = return callback(error) if error? return callback new Errors.NotFoundError("No such project/doc: #{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) -> return callback(error) if error? callback()