From 9f3ec72f8126fb8db2b608f218e986b2bf8f4e16 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Mon, 7 Aug 2017 14:43:28 +0100 Subject: [PATCH] switch to single get/set method for getProjectDocs if project state hasn't changed, return the docs. Otherwise set the hash and return a 409 Conflict response. --- .../document-updater/app/coffee/Errors.coffee | 9 +++- .../app/coffee/HttpController.coffee | 15 ++++--- .../app/coffee/ProjectManager.coffee | 44 +++++++++++-------- .../app/coffee/RedisManager.coffee | 16 ++++++- 4 files changed, 57 insertions(+), 27 deletions(-) diff --git a/services/document-updater/app/coffee/Errors.coffee b/services/document-updater/app/coffee/Errors.coffee index 941bfcc9f1..e5e93fa458 100644 --- a/services/document-updater/app/coffee/Errors.coffee +++ b/services/document-updater/app/coffee/Errors.coffee @@ -12,7 +12,14 @@ OpRangeNotAvailableError = (message) -> return error OpRangeNotAvailableError.prototype.__proto__ = Error.prototype +ProjectStateChangedError = (message) -> + error = new Error(message) + error.name = "ProjectStateChangedError" + error.__proto__ = ProjectStateChangedError.prototype + return error +ProjectStateChangedError.prototype.__proto__ = Error.prototype + module.exports = Errors = NotFoundError: NotFoundError OpRangeNotAvailableError: OpRangeNotAvailableError - + ProjectStateChangedError: ProjectStateChangedError diff --git a/services/document-updater/app/coffee/HttpController.coffee b/services/document-updater/app/coffee/HttpController.coffee index 73d5d24b23..8da4b68c2d 100644 --- a/services/document-updater/app/coffee/HttpController.coffee +++ b/services/document-updater/app/coffee/HttpController.coffee @@ -39,6 +39,7 @@ module.exports = HttpController = getProjectDocs: (req, res, next = (error) ->) -> project_id = req.params.project_id + projectStateHash = req.query?.state # exclude is string of existing docs "id:version,id:version,..." excludeItems = req.query?.exclude?.split(',') or [] logger.log project_id: project_id, exclude: excludeItems, "getting docs via http" @@ -47,12 +48,16 @@ module.exports = HttpController = for item in excludeItems [id,version] = item?.split(':') excludeVersions[id] = version - logger.log {project_id: project_id, excludeVersions: excludeVersions}, "excluding versions" - ProjectManager.getProjectDocs project_id, excludeVersions, (error, result) -> + logger.log {project_id: project_id, projectStateHash: projectStateHash, excludeVersions: excludeVersions}, "excluding versions" + ProjectManager.getProjectDocs project_id, projectStateHash, excludeVersions, (error, result) -> timer.done() - return next(error) if error? - logger.log project_id: project_id, result: ("#{doc._id}:#{doc.rev}" for doc in result), "got docs via http" - res.send result + if error instanceof Errors.ProjectStateChangedError + res.send 409 # conflict + else if error? + return next(error) + else + logger.log project_id: project_id, result: ("#{doc._id}:#{doc.rev}" for doc in result), "got docs via http" + res.send result setDoc: (req, res, next = (error) ->) -> doc_id = req.params.doc_id diff --git a/services/document-updater/app/coffee/ProjectManager.coffee b/services/document-updater/app/coffee/ProjectManager.coffee index e1d346204b..80f9b84a25 100644 --- a/services/document-updater/app/coffee/ProjectManager.coffee +++ b/services/document-updater/app/coffee/ProjectManager.coffee @@ -3,6 +3,7 @@ DocumentManager = require "./DocumentManager" async = require "async" logger = require "logger-sharelatex" Metrics = require "./Metrics" +Errors = require "./Errors" module.exports = ProjectManager = flushProjectWithLocks: (project_id, _callback = (error) ->) -> @@ -57,29 +58,34 @@ module.exports = ProjectManager = else callback(null) - getProjectDocs: (project_id, excludeVersions = {}, _callback = (error) ->) -> + getProjectDocs: (project_id, projectStateHash, excludeVersions = {}, _callback = (error) ->) -> timer = new Metrics.Timer("projectManager.getProjectDocs") callback = (args...) -> timer.done() _callback(args...) - RedisManager.getDocIdsInProject project_id, (error, doc_ids) -> + RedisManager.checkOrSetProjectState project_id, projectStateHash, (error, projectStateChanged) -> return callback(error) if error? - jobs = [] - docs = [] - for doc_id in doc_ids or [] - do (doc_id) -> - jobs.push (cb) -> - # check the doc version first - RedisManager.getDocVersion doc_id, (error, version) -> - return cb(error) if error? - # skip getting the doc if we already have that version - return cb() if version is excludeVersions[doc_id] - # otherwise get the doc lines from redis - RedisManager.getDocLines doc_id, (error, lines) -> - return cb(error) if error? - docs.push {_id: doc_id, lines: lines, rev: version} - cb() - async.series jobs, (error) -> + # we can't return docs if project structure has changed + return callback Errors.ProjectStateChangedError("project state changed") if projectStateChanged + # project structure hasn't changed, return doc content from redis + RedisManager.getDocIdsInProject project_id, (error, doc_ids) -> return callback(error) if error? - callback(null, docs) + jobs = [] + docs = [] + for doc_id in doc_ids or [] + do (doc_id) -> + jobs.push (cb) -> + # check the doc version first + RedisManager.getDocVersion doc_id, (error, version) -> + return cb(error) if error? + # skip getting the doc if we already have that version + return cb() if version is excludeVersions[doc_id] + # otherwise get the doc lines from redis + RedisManager.getDocLines doc_id, (error, lines) -> + return cb(error) if error? + docs.push {_id: doc_id, lines: lines, rev: version} + cb() + async.series jobs, (error) -> + return callback(error) if error? + callback(null, docs) diff --git a/services/document-updater/app/coffee/RedisManager.coffee b/services/document-updater/app/coffee/RedisManager.coffee index 472098d91f..b3e6132e4e 100644 --- a/services/document-updater/app/coffee/RedisManager.coffee +++ b/services/document-updater/app/coffee/RedisManager.coffee @@ -90,10 +90,22 @@ module.exports = RedisManager = return callback(error) if error? multi = rclient.multi() multi.srem keys.docsInProject(project_id:project_id), doc_id - if keys.clsiState? - multi.del keys.clsiState(project_id:project_id) + if keys.projectState? + multi.del keys.projectState(project_id:project_id) multi.exec callback + checkOrSetProjectState: (project_id, newState, callback = (error, stateChanged) ->) -> + if keys.projectState? + multi = rclient.multi() + multi.getset keys.projectState(project_id:project_id), newState + multi.expire keys.projectState(project_id:project_id), 30 * minutes + multi.exec (error, response) -> + return callback(error) if error? + logger.log project_id: project_id, newState:newState, oldState: response[0], "checking project state" + callback(null, response[0] isnt newState) + else + callback(null,true) + getDoc : (project_id, doc_id, callback = (error, lines, version, ranges) ->)-> timer = new metrics.Timer("redis.get-doc") multi = rclient.multi()