diff --git a/services/track-changes/app/coffee/MongoManager.coffee b/services/track-changes/app/coffee/MongoManager.coffee index 330a903a8a..0ded71fb63 100644 --- a/services/track-changes/app/coffee/MongoManager.coffee +++ b/services/track-changes/app/coffee/MongoManager.coffee @@ -75,6 +75,12 @@ module.exports = MongoManager = cursor.toArray callback + deleteOldProjectUpdates: (project_id, before, callback = (error) ->) -> + db.docHistory.remove { + project_id: ObjectId(project_id) + "meta.end_ts": { $lt: before } + }, callback + backportProjectId: (project_id, doc_id, callback = (error) ->) -> db.docHistory.update { doc_id: ObjectId(doc_id.toString()) @@ -85,11 +91,28 @@ module.exports = MongoManager = multi: true }, callback - ensureIndices: (callback = (error) ->) -> - # For finding all updates that go into a diff for a doc - db.docHistory.ensureIndex { doc_id: 1, v: 1 }, callback - # For finding all updates that affect a project - db.docHistory.ensureIndex { project_id: 1, "meta.end_ts": 1 }, callback - # For finding updates that don't yet have a project_id and need it inserting - db.docHistory.ensureIndex { doc_id: 1, project_id: 1 }, callback + getProjectMetaData: (project_id, callback = (error, metadata) ->) -> + db.projectHistoryMetaData.find { + project_id: ObjectId(project_id.toString()) + }, (error, results) -> + return callback(error) if error? + callback null, results[0] + setProjectMetaData: (project_id, metadata, callback = (error) ->) -> + db.projectHistoryMetaData.update { + project_id: ObjectId(project_id) + }, { + $set: metadata + }, { + upsert: true + }, callback + + ensureIndices: () -> + # For finding all updates that go into a diff for a doc + db.docHistory.ensureIndex { doc_id: 1, v: 1 } + # For finding all updates that affect a project + db.docHistory.ensureIndex { project_id: 1, "meta.end_ts": 1 } + # For finding updates that don't yet have a project_id and need it inserting + db.docHistory.ensureIndex { doc_id: 1, project_id: 1 } + # For finding project meta-data + db.projectHistoryMetaData.ensureIndex { project_id: 1 } diff --git a/services/track-changes/app/coffee/UpdateTrimmer.coffee b/services/track-changes/app/coffee/UpdateTrimmer.coffee new file mode 100644 index 0000000000..12a673eddc --- /dev/null +++ b/services/track-changes/app/coffee/UpdateTrimmer.coffee @@ -0,0 +1,32 @@ +MongoManager = require "./MongoManager" +WebApiManager = require "./WebApiManager" +logger = require "logger-sharelatex" + +module.exports = UpdateTrimmer = + _shouldTrimUpdates: (project_id, callback = (error, shouldTrim) ->) -> + MongoManager.getProjectMetaData project_id, (error, metadata) -> + return callback(error) if error? + if metadata?.preserveHistory + return callback null, false + else + WebApiManager.getProjectDetails project_id, (error, details) -> + return callback(error) if error? + logger.log project_id: project_id, details: details, "got details" + if details?.features?.versioning + MongoManager.setProjectMetaData project_id, preserveHistory: true, (error) -> + return callback(error) if error? + callback null, false + else + callback null, true + + deleteOldProjectUpdates: (project_id, callback = (error) ->) -> + UpdateTrimmer._shouldTrimUpdates project_id, (error, shouldTrim) -> + return callback(error) if error? + if shouldTrim + logger.log project_id: project_id, "deleting old updates" + oneWeek = 7 * 24 * 60 * 60 * 1000 + before = Date.now() - oneWeek + MongoManager.deleteOldProjectUpdates project_id, before, callback + else + logger.log project_id: project_id, "not deleting old updates" + callback() \ No newline at end of file diff --git a/services/track-changes/app/coffee/UpdatesManager.coffee b/services/track-changes/app/coffee/UpdatesManager.coffee index c13b42d326..e0e531e957 100644 --- a/services/track-changes/app/coffee/UpdatesManager.coffee +++ b/services/track-changes/app/coffee/UpdatesManager.coffee @@ -3,6 +3,7 @@ RedisManager = require "./RedisManager" UpdateCompressor = require "./UpdateCompressor" LockManager = require "./LockManager" WebApiManager = require "./WebApiManager" +UpdateTrimmer = require "./UpdateTrimmer" logger = require "logger-sharelatex" async = require "async" @@ -74,7 +75,9 @@ module.exports = UpdatesManager = do (doc_id) -> jobs.push (callback) -> UpdatesManager.processUncompressedUpdatesWithLock project_id, doc_id, callback - async.parallelLimit jobs, 5, callback + async.parallelLimit jobs, 5, (error) -> + return callback(error) if error? + UpdateTrimmer.deleteOldProjectUpdates project_id, callback getDocUpdates: (project_id, doc_id, options = {}, callback = (error, updates) ->) -> UpdatesManager.processUncompressedUpdatesWithLock project_id, doc_id, (error) -> diff --git a/services/track-changes/app/coffee/mongojs.coffee b/services/track-changes/app/coffee/mongojs.coffee index ff56198fa7..8c2971945c 100644 --- a/services/track-changes/app/coffee/mongojs.coffee +++ b/services/track-changes/app/coffee/mongojs.coffee @@ -1,6 +1,6 @@ Settings = require "settings-sharelatex" mongojs = require "mongojs" -db = mongojs.connect(Settings.mongo.url, ["docHistory"]) +db = mongojs.connect(Settings.mongo.url, ["docHistory", "projectHistoryMetaData"]) module.exports = db: db ObjectId: mongojs.ObjectId diff --git a/services/track-changes/package.json b/services/track-changes/package.json index fdde105959..0eb4ba9cac 100644 --- a/services/track-changes/package.json +++ b/services/track-changes/package.json @@ -22,6 +22,7 @@ "grunt-contrib-coffee": "~0.10.1", "bunyan": "~0.22.1", "grunt-bunyan": "~0.5.0", - "grunt-forever": "~0.4.2" + "grunt-forever": "~0.4.2", + "timekeeper": "0.0.4" } } diff --git a/services/track-changes/test/acceptance/coffee/FlushingUpdatesTests.coffee b/services/track-changes/test/acceptance/coffee/FlushingUpdatesTests.coffee index 70ee40307e..ee225858ee 100644 --- a/services/track-changes/test/acceptance/coffee/FlushingUpdatesTests.coffee +++ b/services/track-changes/test/acceptance/coffee/FlushingUpdatesTests.coffee @@ -9,6 +9,7 @@ request = require "request" rclient = require("redis").createClient() # Only works locally for now TrackChangesClient = require "./helpers/TrackChangesClient" +MockWebApi = require "./helpers/MockWebApi" describe "Flushing updates", -> describe "flushing a doc's updates", -> @@ -16,6 +17,7 @@ describe "Flushing updates", -> @project_id = ObjectId().toString() @doc_id = ObjectId().toString() @user_id = ObjectId().toString() + TrackChangesClient.pushRawUpdates @project_id, @doc_id, [{ op: [{ i: "f", p: 3 }] meta: { ts: Date.now(), user_id: @user_id } @@ -34,23 +36,103 @@ describe "Flushing updates", -> done() describe "flushing a project's updates", -> - before (done) -> - @project_id = ObjectId().toString() - @doc_id = ObjectId().toString() - @user_id = ObjectId().toString() - TrackChangesClient.pushRawUpdates @project_id, @doc_id, [{ - op: [{ i: "f", p: 3 }] - meta: { ts: Date.now(), user_id: @user_id } - v: 3 - }], (error) => - throw error if error? - TrackChangesClient.flushProject @project_id, (error) -> + describe "with versioning enabled", -> + before (done) -> + @project_id = ObjectId().toString() + @doc_id = ObjectId().toString() + @user_id = ObjectId().toString() + + @weeks = 7 * 24 * 60 * 60 * 1000 + + MockWebApi.projects[@project_id] = + features: + versioning: true + + TrackChangesClient.pushRawUpdates @project_id, @doc_id, [{ + op: [{ i: "g", p: 2 }] + meta: { ts: Date.now() - 2 * @weeks, user_id: @user_id } + v: 2 + }, { + op: [{ i: "f", p: 3 }] + meta: { ts: Date.now(), user_id: @user_id } + v: 3 + }], (error) => throw error if error? + TrackChangesClient.flushProject @project_id, (error) -> + throw error if error? + done() + + it "should not delete the old updates", (done) -> + TrackChangesClient.getCompressedUpdates @doc_id, (error, updates) -> + expect(updates.length).to.equal 2 done() - it "should flush the op into mongo", (done) -> - TrackChangesClient.getCompressedUpdates @doc_id, (error, updates) -> - expect(updates[0].op).to.deep.equal [{ - p: 3, i: "f" - }] - done() + it "should preserve history forever", (done) -> + TrackChangesClient.getProjectMetaData @project_id, (error, project) -> + expect(project.preserveHistory).to.equal true + done() + + describe "without versioning enabled", -> + before (done) -> + @project_id = ObjectId().toString() + @doc_id = ObjectId().toString() + @user_id = ObjectId().toString() + + @weeks = 7 * 24 * 60 * 60 * 1000 + + MockWebApi.projects[@project_id] = + features: + versioning: false + + TrackChangesClient.pushRawUpdates @project_id, @doc_id, [{ + op: [{ i: "g", p: 2 }] + meta: { ts: Date.now() - 2 * @weeks, user_id: @user_id } + v: 2 + }, { + op: [{ i: "f", p: 3 }] + meta: { ts: Date.now(), user_id: @user_id } + v: 3 + }], (error) => + throw error if error? + TrackChangesClient.flushProject @project_id, (error) -> + throw error if error? + done() + + it "should delete the older update, but the newer update", (done) -> + TrackChangesClient.getCompressedUpdates @doc_id, (error, updates) -> + expect(updates.length).to.equal 1 + expect(updates[0].op).to.deep.equal [{ i: "f", p: 3 }] + done() + + describe "without versioning enabled but with preserveHistory set to true", -> + before (done) -> + @project_id = ObjectId().toString() + @doc_id = ObjectId().toString() + @user_id = ObjectId().toString() + + @weeks = 7 * 24 * 60 * 60 * 1000 + + MockWebApi.projects[@project_id] = + features: + versioning: false + + TrackChangesClient.setPreserveHistoryForProject @project_id, (error) => + throw error if error? + TrackChangesClient.pushRawUpdates @project_id, @doc_id, [{ + op: [{ i: "g", p: 2 }] + meta: { ts: Date.now() - 2 * @weeks, user_id: @user_id } + v: 2 + }, { + op: [{ i: "f", p: 3 }] + meta: { ts: Date.now(), user_id: @user_id } + v: 3 + }], (error) => + throw error if error? + TrackChangesClient.flushProject @project_id, (error) -> + throw error if error? + done() + + it "should not delete the old updates", (done) -> + TrackChangesClient.getCompressedUpdates @doc_id, (error, updates) -> + expect(updates.length).to.equal 2 + done() diff --git a/services/track-changes/test/acceptance/coffee/GettingUpdatesTests.coffee b/services/track-changes/test/acceptance/coffee/GettingUpdatesTests.coffee index cbf93f105a..6452649fc4 100644 --- a/services/track-changes/test/acceptance/coffee/GettingUpdatesTests.coffee +++ b/services/track-changes/test/acceptance/coffee/GettingUpdatesTests.coffee @@ -19,7 +19,11 @@ describe "Getting updates", -> @project_id = ObjectId().toString() @minutes = 60 * 1000 - @days = 24 * 60 * @minutes + @hours = 60 * @minutes + + MockWebApi.projects[@project_id] = + features: + versioning: true MockWebApi.users[@user_id] = @user = email: "user@sharelatex.com" @@ -32,12 +36,12 @@ describe "Getting updates", -> for i in [0..9] @updates.push { op: [{ i: "a", p: 0 }] - meta: { ts: @now - (9 - i) * @days - 2 * @minutes, user_id: @user_id } + meta: { ts: @now - (9 - i) * @hours - 2 * @minutes, user_id: @user_id } v: 2 * i + 1 } @updates.push { op: [{ i: "b", p: 0 }] - meta: { ts: @now - (9 - i) * @days, user_id: @user_id } + meta: { ts: @now - (9 - i) * @hours, user_id: @user_id } v: 2 * i + 2 } @@ -76,21 +80,21 @@ describe "Getting updates", -> }, { docs: docs2 meta: - start_ts: @to - 1 * @days - 2 * @minutes - end_ts: @to - 1 * @days + start_ts: @to - 1 * @hours - 2 * @minutes + end_ts: @to - 1 * @hours users: [@user] }, { docs: docs3 meta: - start_ts: @to - 2 * @days - 2 * @minutes - end_ts: @to - 2 * @days + start_ts: @to - 2 * @hours - 2 * @minutes + end_ts: @to - 2 * @hours users: [@user] }] describe "getting updates beyond the end of the database", -> before (done) -> - TrackChangesClient.getUpdates @project_id, { before: @to - 8 * @days + 1, min_count: 30 }, (error, body) => + TrackChangesClient.getUpdates @project_id, { before: @to - 8 * @hours + 1, min_count: 30 }, (error, body) => throw error if error? @updates = body.updates done() @@ -103,14 +107,14 @@ describe "Getting updates", -> expect(@updates).to.deep.equal [{ docs: docs1 meta: - start_ts: @to - 8 * @days - 2 * @minutes - end_ts: @to - 8 * @days + start_ts: @to - 8 * @hours - 2 * @minutes + end_ts: @to - 8 * @hours users: [@user] }, { docs: docs2 meta: - start_ts: @to - 9 * @days - 2 * @minutes - end_ts: @to - 9 * @days + start_ts: @to - 9 * @hours - 2 * @minutes + end_ts: @to - 9 * @hours users: [@user] }] diff --git a/services/track-changes/test/acceptance/coffee/helpers/MockWebApi.coffee b/services/track-changes/test/acceptance/coffee/helpers/MockWebApi.coffee index 04ee281541..f7cfc9e2e9 100644 --- a/services/track-changes/test/acceptance/coffee/helpers/MockWebApi.coffee +++ b/services/track-changes/test/acceptance/coffee/helpers/MockWebApi.coffee @@ -4,9 +4,14 @@ app = express() module.exports = MockWebApi = users: {} + projects: {} + getUser: (user_id, callback = (error) ->) -> callback null, @users[user_id] + getProject: (project_id, callback = (error, project) ->) -> + callback null, @projects[project_id] + run: () -> app.get "/user/:user_id/personal_info", (req, res, next) => @getUser req.params.user_id, (error, user) -> @@ -17,6 +22,15 @@ module.exports = MockWebApi = else res.send JSON.stringify user + app.get "/project/:project_id/details", (req, res, next) => + @getProject req.params.project_id, (error, project) -> + if error? + res.send 500 + if !project? + res.send 404 + else + res.send JSON.stringify project + app.listen 3000, (error) -> throw error if error? diff --git a/services/track-changes/test/acceptance/coffee/helpers/TrackChangesClient.coffee b/services/track-changes/test/acceptance/coffee/helpers/TrackChangesClient.coffee index de75cce0b0..661b0eafb8 100644 --- a/services/track-changes/test/acceptance/coffee/helpers/TrackChangesClient.coffee +++ b/services/track-changes/test/acceptance/coffee/helpers/TrackChangesClient.coffee @@ -28,6 +28,23 @@ module.exports = TrackChangesClient = .sort("meta.end_ts": 1) .toArray callback + getProjectMetaData: (project_id, callback = (error, updates) ->) -> + db.projectHistoryMetaData + .find { + project_id: ObjectId(project_id) + }, + (error, projects) -> + callback error, projects[0] + + setPreserveHistoryForProject: (project_id, callback = (error) ->) -> + db.projectHistoryMetaData.update { + project_id: ObjectId(project_id) + }, { + $set: { preserveHistory: true } + }, { + upsert: true + }, callback + pushRawUpdates: (project_id, doc_id, updates, callback = (error) ->) -> rclient.sadd "DocsWithHistoryOps:#{project_id}", doc_id, (error) -> return callback(error) if error? diff --git a/services/track-changes/test/unit/coffee/MongoManager/MongoManagerTests.coffee b/services/track-changes/test/unit/coffee/MongoManager/MongoManagerTests.coffee index b3a8fc04ca..2228147b75 100644 --- a/services/track-changes/test/unit/coffee/MongoManager/MongoManagerTests.coffee +++ b/services/track-changes/test/unit/coffee/MongoManager/MongoManagerTests.coffee @@ -274,3 +274,56 @@ describe "MongoManager", -> it "should call the callback", -> @callback.called.should.equal true + describe "getProjectMetaData", -> + beforeEach -> + @metadata = { "mock": "metadata" } + @db.projectHistoryMetaData = + find: sinon.stub().callsArgWith(1, null, [@metadata]) + @MongoManager.getProjectMetaData @project_id, @callback + + it "should look up the meta data in the db", -> + @db.projectHistoryMetaData.find + .calledWith({ project_id: ObjectId(@project_id) }) + .should.equal true + + it "should return the metadata", -> + @callback.calledWith(null, @metadata).should.equal true + + describe "setProjectMetaData", -> + beforeEach -> + @metadata = { "mock": "metadata" } + @db.projectHistoryMetaData = + update: sinon.stub().callsArgWith(3, null, [@metadata]) + @MongoManager.setProjectMetaData @project_id, @metadata, @callback + + it "should upsert the metadata into the DB", -> + @db.projectHistoryMetaData.update + .calledWith({ + project_id: ObjectId(@project_id) + }, { + $set: @metadata + }, { + upsert: true + }) + .should.equal true + + it "should call the callback", -> + @callback.called.should.equal true + + describe "deleteOldProjectUpdates", -> + beforeEach -> + @before = Date.now() - 10000 + @db.docHistory = + remove: sinon.stub().callsArg(1) + @MongoManager.deleteOldProjectUpdates @project_id, @before, @callback + + it "should delete updates before the 'before' time", -> + @db.docHistory.remove + .calledWith({ + project_id: ObjectId(@project_id) + "meta.end_ts": { "$lt": @before } + }) + .should.equal true + + it "should return the callback", -> + @callback.called.should.equal true diff --git a/services/track-changes/test/unit/coffee/UpdateTrimmer/UpdateTrimmerTests.coffee b/services/track-changes/test/unit/coffee/UpdateTrimmer/UpdateTrimmerTests.coffee new file mode 100644 index 0000000000..aa0ac00f78 --- /dev/null +++ b/services/track-changes/test/unit/coffee/UpdateTrimmer/UpdateTrimmerTests.coffee @@ -0,0 +1,142 @@ +sinon = require('sinon') +chai = require('chai') +should = chai.should() +expect = chai.expect +modulePath = "../../../../app/js/UpdateTrimmer.js" +SandboxedModule = require('sandboxed-module') +tk = require "timekeeper" + +describe "UpdateTrimmer", -> + beforeEach -> + @now = new Date() + tk.freeze(@now) + + @UpdateTrimmer = SandboxedModule.require modulePath, requires: + "logger-sharelatex": @logger = { log: sinon.stub(), error: sinon.stub() } + "./WebApiManager": @WebApiManager = {} + "./MongoManager": @MongoManager = {} + + @callback = sinon.stub() + @project_id = "mock-project-id" + + afterEach -> + tk.reset() + + describe "_shouldTrimUpdates", -> + beforeEach -> + @metadata = {} + @details = + features: {} + @MongoManager.getProjectMetaData = sinon.stub().callsArgWith(1, null, @metadata) + @MongoManager.setProjectMetaData = sinon.stub().callsArgWith(2) + @WebApiManager.getProjectDetails = sinon.stub().callsArgWith(1, null, @details) + + describe "with preserveHistory set in the project meta data", -> + beforeEach -> + @metadata.preserveHistory = true + @UpdateTrimmer._shouldTrimUpdates @project_id, @callback + + it "should look up the meta data", -> + @MongoManager.getProjectMetaData + .calledWith(@project_id) + .should.equal true + + it "should not look up the project details", -> + @WebApiManager.getProjectDetails + .called + .should.equal false + + it "should return false", -> + @callback.calledWith(null, false).should.equal true + + describe "without preserveHistory set in the project meta data", -> + beforeEach -> + @metadata.preserveHistory = false + + describe "when the project has the versioning feature", -> + beforeEach -> + @details.features.versioning = true + @UpdateTrimmer._shouldTrimUpdates @project_id, @callback + + it "should look up the meta data", -> + @MongoManager.getProjectMetaData + .calledWith(@project_id) + .should.equal true + + it "should look up the project details", -> + @WebApiManager.getProjectDetails + .calledWith(@project_id) + .should.equal true + + it "should insert preserveHistory into the metadata", -> + @MongoManager.setProjectMetaData + .calledWith(@project_id, {preserveHistory: true}) + .should.equal true + + it "should return false", -> + @callback.calledWith(null, false).should.equal true + + describe "when the project does not have the versioning feature", -> + beforeEach -> + @details.features.versioning = false + @UpdateTrimmer._shouldTrimUpdates @project_id, @callback + + it "should return true", -> + @callback.calledWith(null, true).should.equal true + + describe "without any meta data", -> + beforeEach -> + @MongoManager.getProjectMetaData = sinon.stub().callsArgWith(1, null, null) + + describe "when the project has the versioning feature", -> + beforeEach -> + @details.features.versioning = true + @UpdateTrimmer._shouldTrimUpdates @project_id, @callback + + it "should insert preserveHistory into the metadata", -> + @MongoManager.setProjectMetaData + .calledWith(@project_id, {preserveHistory: true}) + .should.equal true + + it "should return false", -> + @callback.calledWith(null, false).should.equal true + + describe "when the project does not have the versioning feature", -> + beforeEach -> + @details.features.versioning = false + @UpdateTrimmer._shouldTrimUpdates @project_id, @callback + + it "should return true", -> + @callback.calledWith(null, true).should.equal true + + describe "deleteOldProjectUpdates", -> + beforeEach -> + @oneWeek = 7 * 24 * 60 * 60 * 1000 + @MongoManager.deleteOldProjectUpdates = sinon.stub().callsArg(2) + + describe "when the updates should be trimmed", -> + beforeEach -> + @UpdateTrimmer._shouldTrimUpdates = sinon.stub().callsArgWith(1, null, true) + @UpdateTrimmer.deleteOldProjectUpdates @project_id, @callback + + it "should delete week old updates in mongo", -> + before = Date.now() - @oneWeek + @MongoManager.deleteOldProjectUpdates + .calledWith(@project_id, before) + .should.equal true + + it 'should call the callback', -> + @callback.called.should.equal true + + describe "when the updates should not be trimmed", -> + beforeEach -> + @UpdateTrimmer._shouldTrimUpdates = sinon.stub().callsArgWith(1, null, false) + @UpdateTrimmer.deleteOldProjectUpdates @project_id, @callback + + it "should not delete any updates in mongo", -> + @MongoManager.deleteOldProjectUpdates + .called + .should.equal false + + it 'should call the callback', -> + @callback.called.should.equal true diff --git a/services/track-changes/test/unit/coffee/UpdatesManager/UpdatesManagerTests.coffee b/services/track-changes/test/unit/coffee/UpdatesManager/UpdatesManagerTests.coffee index 6ff9f64e11..0e50ee3e20 100644 --- a/services/track-changes/test/unit/coffee/UpdatesManager/UpdatesManagerTests.coffee +++ b/services/track-changes/test/unit/coffee/UpdatesManager/UpdatesManagerTests.coffee @@ -13,6 +13,7 @@ describe "UpdatesManager", -> "./RedisManager" : @RedisManager = {} "./LockManager" : @LockManager = {} "./WebApiManager": @WebApiManager = {} + "./UpdateTrimmer": @UpdateTrimmer = {} "logger-sharelatex": { log: sinon.stub(), error: sinon.stub() } @doc_id = "doc-id-123" @project_id = "project-id-123" @@ -277,6 +278,7 @@ describe "UpdatesManager", -> @doc_ids = ["mock-id-1", "mock-id-2"] @UpdatesManager.processUncompressedUpdatesWithLock = sinon.stub().callsArg(2) @RedisManager.getDocIdsWithHistoryOps = sinon.stub().callsArgWith(1, null, @doc_ids) + @UpdateTrimmer.deleteOldProjectUpdates = sinon.stub().callsArg(1) @UpdatesManager.processUncompressedUpdatesForProject @project_id, () => @callback() done() @@ -292,6 +294,11 @@ describe "UpdatesManager", -> .calledWith(@project_id, doc_id) .should.equal true + it "should delete old updates for the project", -> + @UpdateTrimmer.deleteOldProjectUpdates + .calledWith(@project_id) + .should.equal true + it "should call the callback", -> @callback.called.should.equal true