From 62a10f25f27802343bfac135275a8720226eac2e Mon Sep 17 00:00:00 2001 From: Hayden Faulds Date: Tue, 5 Jun 2018 13:54:05 +0100 Subject: [PATCH 1/8] move tk call to before sandboxed module call --- .../coffee/DocumentManager/DocumentManagerTests.coffee | 8 ++++---- .../ProjectHistoryRedisManagerTests.coffee | 4 ++-- .../unit/coffee/RedisManager/RedisManagerTests.coffee | 6 +++--- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/services/document-updater/test/unit/coffee/DocumentManager/DocumentManagerTests.coffee b/services/document-updater/test/unit/coffee/DocumentManager/DocumentManagerTests.coffee index d7cd18630a..c52bb4b30d 100644 --- a/services/document-updater/test/unit/coffee/DocumentManager/DocumentManagerTests.coffee +++ b/services/document-updater/test/unit/coffee/DocumentManager/DocumentManagerTests.coffee @@ -8,6 +8,7 @@ tk = require "timekeeper" describe "DocumentManager", -> beforeEach -> + tk.freeze(new Date()) @DocumentManager = SandboxedModule.require modulePath, requires: "./RedisManager": @RedisManager = {} "./ProjectHistoryRedisManager": @ProjectHistoryRedisManager = {} @@ -35,6 +36,9 @@ describe "DocumentManager", -> @pathname = '/a/b/c.tex' @unflushedTime = Date.now() + afterEach -> + tk.reset() + describe "flushAndDeleteDoc", -> describe "successfully", -> beforeEach -> @@ -394,12 +398,8 @@ describe "DocumentManager", -> describe "getDocAndFlushIfOld", -> beforeEach -> - tk.freeze(new Date()) @DocumentManager.flushDocIfLoaded = sinon.stub().callsArg(2) - afterEach -> - tk.reset() - describe "when the doc is in Redis", -> describe "and has changes to be flushed", -> beforeEach -> diff --git a/services/document-updater/test/unit/coffee/ProjectHistoryRedisManager/ProjectHistoryRedisManagerTests.coffee b/services/document-updater/test/unit/coffee/ProjectHistoryRedisManager/ProjectHistoryRedisManagerTests.coffee index 349d3623e6..aa777b40d6 100644 --- a/services/document-updater/test/unit/coffee/ProjectHistoryRedisManager/ProjectHistoryRedisManagerTests.coffee +++ b/services/document-updater/test/unit/coffee/ProjectHistoryRedisManager/ProjectHistoryRedisManagerTests.coffee @@ -26,8 +26,8 @@ describe "ProjectHistoryRedisManager", -> globals: JSON: @JSON = JSON - afterEach -> - tk.reset() + afterEach -> + tk.reset() describe "queueOps", -> beforeEach -> diff --git a/services/document-updater/test/unit/coffee/RedisManager/RedisManagerTests.coffee b/services/document-updater/test/unit/coffee/RedisManager/RedisManagerTests.coffee index 42d06d743a..4f6c24720e 100644 --- a/services/document-updater/test/unit/coffee/RedisManager/RedisManagerTests.coffee +++ b/services/document-updater/test/unit/coffee/RedisManager/RedisManagerTests.coffee @@ -55,14 +55,14 @@ describe "RedisManager", -> globals: JSON: @JSON = JSON - afterEach -> - tk.reset() - @doc_id = "doc-id-123" @project_id = "project-id-123" @projectHistoryId = 123 @callback = sinon.stub() + afterEach -> + tk.reset() + describe "getDoc", -> beforeEach -> @lines = ["one", "two", "three", "これは"] # include some utf8 From 6237577f875fc3b00b041b06b1b0a1bfce014f42 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Fri, 20 Jul 2018 10:43:31 +0100 Subject: [PATCH 2/8] set a timestamp for the first entry in the projectHistory:Ops queue --- .../coffee/ProjectHistoryRedisManager.coffee | 14 +++++++++-- .../config/settings.defaults.coffee | 1 + .../coffee/ApplyingUpdatesToADocTests.coffee | 23 ++++++++++++++++++- .../ProjectHistoryRedisManagerTests.coffee | 17 +++++++++++--- 4 files changed, 49 insertions(+), 6 deletions(-) diff --git a/services/document-updater/app/coffee/ProjectHistoryRedisManager.coffee b/services/document-updater/app/coffee/ProjectHistoryRedisManager.coffee index 42b9f16df2..1cc80ea722 100644 --- a/services/document-updater/app/coffee/ProjectHistoryRedisManager.coffee +++ b/services/document-updater/app/coffee/ProjectHistoryRedisManager.coffee @@ -4,8 +4,18 @@ rclient = require("redis-sharelatex").createClient(Settings.redis.documentupdate logger = require('logger-sharelatex') module.exports = ProjectHistoryRedisManager = - queueOps: (project_id, ops..., callback) -> - rclient.rpush projectHistoryKeys.projectHistoryOps({project_id}), ops..., callback + queueOps: (project_id, ops..., callback = (error, projectUpdateCount) ->) -> + multi = rclient.multi() + # Push the ops onto the project history queue + multi.rpush projectHistoryKeys.projectHistoryOps({project_id}), ops... + # To record the age of the oldest op on the queue set a timestamp if not + # already present (SETNX). + multi.setnx projectHistoryKeys.projectHistoryFirstOpTimestamp({project_id}), Date.now() + multi.exec (error, result) -> + return callback(error) if error? + # return the number of entries pushed onto the project history queue + callback null, result[0] + queueRenameEntity: (project_id, projectHistoryId, entity_type, entity_id, user_id, projectUpdate, callback) -> projectUpdate = diff --git a/services/document-updater/config/settings.defaults.coffee b/services/document-updater/config/settings.defaults.coffee index 18f2b1570b..dfce36f6e3 100755 --- a/services/document-updater/config/settings.defaults.coffee +++ b/services/document-updater/config/settings.defaults.coffee @@ -74,6 +74,7 @@ module.exports = project_history: key_schema: projectHistoryOps: ({project_id}) -> "ProjectHistory:Ops:#{project_id}" + projectHistoryFirstOpTimestamp: ({project_id}) -> "ProjectHistory:FirstOpTimestamp:#{project_id}" # cluster: [{ # port: "7000" # host: "localhost" diff --git a/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToADocTests.coffee b/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToADocTests.coffee index cb560a26bb..c6aa6fe856 100644 --- a/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToADocTests.coffee +++ b/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToADocTests.coffee @@ -33,7 +33,7 @@ describe "Applying updates to a doc", -> before (done) -> [@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()] sinon.spy MockWebApi, "getDocument" - + @startTime = Date.now() MockWebApi.insertDoc @project_id, @doc_id, {lines: @lines, version: @version} DocUpdaterClient.sendUpdate @project_id, @doc_id, @update, (error) -> throw error if error? @@ -67,6 +67,27 @@ describe "Applying updates to a doc", -> JSON.parse(updates[0]).op.should.deep.equal @update.op done() + it "should set the first op timestamp", (done) -> + rclient_history.get ProjectHistoryKeys.projectHistoryFirstOpTimestamp({@project_id}), (error, result) => + throw error if error? + result.should.be.within(@startTime, Date.now()) + @firstOpTimestamp = result + done() + + describe "when sending another update", -> + before (done) -> + @second_update = Object.create(@update) + @second_update.v = @version + 1 + DocUpdaterClient.sendUpdate @project_id, @doc_id, @second_update, (error) -> + throw error if error? + setTimeout done, 200 + + it "should not change the first op timestamp", (done) -> + rclient_history.get ProjectHistoryKeys.projectHistoryFirstOpTimestamp({@project_id}), (error, result) => + throw error if error? + result.should.equal @firstOpTimestamp + done() + describe "when the document is loaded", -> before (done) -> [@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()] diff --git a/services/document-updater/test/unit/coffee/ProjectHistoryRedisManager/ProjectHistoryRedisManagerTests.coffee b/services/document-updater/test/unit/coffee/ProjectHistoryRedisManager/ProjectHistoryRedisManagerTests.coffee index aa777b40d6..8c6ff5c7d7 100644 --- a/services/document-updater/test/unit/coffee/ProjectHistoryRedisManager/ProjectHistoryRedisManagerTests.coffee +++ b/services/document-updater/test/unit/coffee/ProjectHistoryRedisManager/ProjectHistoryRedisManagerTests.coffee @@ -20,6 +20,7 @@ describe "ProjectHistoryRedisManager", -> project_history: key_schema: projectHistoryOps: ({project_id}) -> "ProjectHistory:Ops:#{project_id}" + projectHistoryFirstOpTimestamp: ({project_id}) -> "ProjectHistory:FirstOpTimestamp:#{project_id}" } "redis-sharelatex": createClient: () => @rclient @@ -32,16 +33,26 @@ describe "ProjectHistoryRedisManager", -> describe "queueOps", -> beforeEach -> @ops = ["mock-op-1", "mock-op-2"] - @rclient.rpush = sinon.stub() + @multi = exec: sinon.stub() + @multi.rpush = sinon.stub() + @multi.setnx = sinon.stub() + @rclient.multi = () => @multi + # @rclient = multi: () => @multi @ProjectHistoryRedisManager.queueOps @project_id, @ops..., @callback it "should queue an update", -> - @rclient.rpush + @multi.rpush .calledWithExactly( "ProjectHistory:Ops:#{@project_id}" @ops[0] @ops[1] - @callback + ).should.equal true + + it "should set the queue timestamp if not present", -> + @multi.setnx + .calledWithExactly( + "ProjectHistory:FirstOpTimestamp:#{@project_id}" + Date.now() ).should.equal true describe "queueRenameEntity", -> From 33e898f67184425e9c4f14b11d41576fda1acb50 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Mon, 23 Jul 2018 15:32:33 +0100 Subject: [PATCH 3/8] update to 1.1.9 build scripts --- .../.github/ISSUE_TEMPLATE.md | 38 ++++++++++++++++ .../.github/PULL_REQUEST_TEMPLATE.md | 45 +++++++++++++++++++ services/document-updater/Jenkinsfile | 45 +++++++++++++++++-- services/document-updater/Makefile | 20 ++++++--- .../document-updater/docker-compose.ci.yml | 13 ++++-- services/document-updater/docker-compose.yml | 18 +++++--- services/document-updater/nodemon.json | 5 ++- services/document-updater/package.json | 21 ++++----- 8 files changed, 175 insertions(+), 30 deletions(-) create mode 100644 services/document-updater/.github/ISSUE_TEMPLATE.md create mode 100644 services/document-updater/.github/PULL_REQUEST_TEMPLATE.md diff --git a/services/document-updater/.github/ISSUE_TEMPLATE.md b/services/document-updater/.github/ISSUE_TEMPLATE.md new file mode 100644 index 0000000000..e0093aa90c --- /dev/null +++ b/services/document-updater/.github/ISSUE_TEMPLATE.md @@ -0,0 +1,38 @@ + + +## Steps to Reproduce + + + +1. +2. +3. + +## Expected Behaviour + + +## Observed Behaviour + + + +## Context + + +## Technical Info + + +* URL: +* Browser Name and version: +* Operating System and version (desktop or mobile): +* Signed in as: +* Project and/or file: + +## Analysis + + +## Who Needs to Know? + + + +- +- diff --git a/services/document-updater/.github/PULL_REQUEST_TEMPLATE.md b/services/document-updater/.github/PULL_REQUEST_TEMPLATE.md new file mode 100644 index 0000000000..ed25ee83c1 --- /dev/null +++ b/services/document-updater/.github/PULL_REQUEST_TEMPLATE.md @@ -0,0 +1,45 @@ + + +### Description + + + +#### Screenshots + + + +#### Related Issues / PRs + + + +### Review + + + +#### Potential Impact + + + +#### Manual Testing Performed + +- [ ] +- [ ] + +#### Accessibility + + + +### Deployment + + + +#### Deployment Checklist + +- [ ] Update documentation not included in the PR (if any) +- [ ] + +#### Metrics and Monitoring + + + +#### Who Needs to Know? diff --git a/services/document-updater/Jenkinsfile b/services/document-updater/Jenkinsfile index ab90aaae29..fa0f5beedc 100644 --- a/services/document-updater/Jenkinsfile +++ b/services/document-updater/Jenkinsfile @@ -3,6 +3,13 @@ String cron_string = BRANCH_NAME == "master" ? "@daily" : "" pipeline { agent any + environment { + GIT_PROJECT = "document-updater-sharelatex" + JENKINS_WORKFLOW = "document-updater-sharelatex" + TARGET_URL = "${env.JENKINS_URL}blue/organizations/jenkins/${JENKINS_WORKFLOW}/detail/$BRANCH_NAME/$BUILD_NUMBER/pipeline" + GIT_API_URL = "https://api.github.com/repos/sharelatex/${GIT_PROJECT}/statuses/$GIT_COMMIT" + } + triggers { pollSCM('* * * * *') cron(cron_string) @@ -18,16 +25,24 @@ pipeline { } } steps { - // we need to disable logallrefupdates, else git clones - // during the npm install will require git to lookup the - // user id which does not exist in the container's + withCredentials([usernamePassword(credentialsId: 'GITHUB_INTEGRATION', usernameVariable: 'GH_AUTH_USERNAME', passwordVariable: 'GH_AUTH_PASSWORD')]) { + sh "curl $GIT_API_URL \ + --data '{ \ + \"state\" : \"pending\", \ + \"target_url\": \"$TARGET_URL\", \ + \"description\": \"Your build is underway\", \ + \"context\": \"ci/jenkins\" }' \ + -u $GH_AUTH_USERNAME:$GH_AUTH_PASSWORD" + } + // we need to disable logallrefupdates, else git clones + // during the npm install will require git to lookup the + // user id which does not exist in the container's // /etc/passwd file, causing the clone to fail. sh 'git config --global core.logallrefupdates false' sh 'rm -rf node_modules' sh 'npm install && npm rebuild' } } - stage('Compile') { agent { docker { @@ -77,6 +92,19 @@ pipeline { post { always { sh 'DOCKER_COMPOSE_FLAGS="-f docker-compose.ci.yml" make test_clean' + sh 'make clean' + } + + success { + withCredentials([usernamePassword(credentialsId: 'GITHUB_INTEGRATION', usernameVariable: 'GH_AUTH_USERNAME', passwordVariable: 'GH_AUTH_PASSWORD')]) { + sh "curl $GIT_API_URL \ + --data '{ \ + \"state\" : \"success\", \ + \"target_url\": \"$TARGET_URL\", \ + \"description\": \"Your build succeeded!\", \ + \"context\": \"ci/jenkins\" }' \ + -u $GH_AUTH_USERNAME:$GH_AUTH_PASSWORD" + } } failure { @@ -84,6 +112,15 @@ pipeline { to: "${EMAIL_ALERT_TO}", subject: "Jenkins build failed: ${JOB_NAME}:${BUILD_NUMBER}", body: "Build: ${BUILD_URL}") + withCredentials([usernamePassword(credentialsId: 'GITHUB_INTEGRATION', usernameVariable: 'GH_AUTH_USERNAME', passwordVariable: 'GH_AUTH_PASSWORD')]) { + sh "curl $GIT_API_URL \ + --data '{ \ + \"state\" : \"failure\", \ + \"target_url\": \"$TARGET_URL\", \ + \"description\": \"Your build failed\", \ + \"context\": \"ci/jenkins\" }' \ + -u $GH_AUTH_USERNAME:$GH_AUTH_PASSWORD" + } } } diff --git a/services/document-updater/Makefile b/services/document-updater/Makefile index 10adbe3866..8b3646251d 100644 --- a/services/document-updater/Makefile +++ b/services/document-updater/Makefile @@ -1,13 +1,18 @@ # This file was auto-generated, do not edit it directly. # Instead run bin/update_build_scripts from # https://github.com/sharelatex/sharelatex-dev-environment -# Version: 1.0.1 +# Version: 1.1.9 BUILD_NUMBER ?= local BRANCH_NAME ?= $(shell git rev-parse --abbrev-ref HEAD) PROJECT_NAME = document-updater DOCKER_COMPOSE_FLAGS ?= -f docker-compose.yml -DOCKER_COMPOSE := docker-compose ${DOCKER_COMPOSE_FLAGS} +DOCKER_COMPOSE := BUILD_NUMBER=$(BUILD_NUMBER) \ + BRANCH_NAME=$(BRANCH_NAME) \ + PROJECT_NAME=$(PROJECT_NAME) \ + MOCHA_GREP=${MOCHA_GREP} \ + docker-compose ${DOCKER_COMPOSE_FLAGS} + clean: rm -f app.js @@ -18,12 +23,15 @@ clean: test: test_unit test_acceptance test_unit: - @[ -d test/unit ] && $(DOCKER_COMPOSE) run --rm test_unit -- ${MOCHA_ARGS} || echo "document-updater has no unit tests" + @[ ! -d test/unit ] && echo "document-updater has no unit tests" || $(DOCKER_COMPOSE) run --rm test_unit -test_acceptance: test_clean # clear the database before each acceptance test run - @[ -d test/acceptance ] && $(DOCKER_COMPOSE) run --rm test_acceptance -- ${MOCHA_ARGS} || echo "document-updater has no acceptance tests" +test_acceptance: test_clean test_acceptance_pre_run # clear the database before each acceptance test run + @[ ! -d test/acceptance ] && echo "document-updater has no acceptance tests" || $(DOCKER_COMPOSE) run --rm test_acceptance test_clean: - $(DOCKER_COMPOSE) down -t 0 + $(DOCKER_COMPOSE) down -v -t 0 + +test_acceptance_pre_run: + @[ ! -f test/acceptance/scripts/pre-run ] && echo "document-updater has no pre acceptance tests task" || $(DOCKER_COMPOSE) run --rm test_acceptance test/acceptance/scripts/pre-run .PHONY: clean test test_unit test_acceptance test_clean build publish diff --git a/services/document-updater/docker-compose.ci.yml b/services/document-updater/docker-compose.ci.yml index 9f40ba879d..4f00796ee6 100644 --- a/services/document-updater/docker-compose.ci.yml +++ b/services/document-updater/docker-compose.ci.yml @@ -1,7 +1,7 @@ # This file was auto-generated, do not edit it directly. # Instead run bin/update_build_scripts from # https://github.com/sharelatex/sharelatex-dev-environment -# Version: 1.0.1 +# Version: 1.1.9 version: "2" @@ -11,7 +11,8 @@ services: volumes: - .:/app working_dir: /app - entrypoint: npm run test:unit:_run + user: node + command: npm run test:unit:_run test_acceptance: image: node:6.9.5 @@ -19,12 +20,16 @@ services: - .:/app working_dir: /app environment: + ELASTIC_SEARCH_DSN: es:9200 REDIS_HOST: redis MONGO_HOST: mongo + POSTGRES_HOST: postgres + MOCHA_GREP: ${MOCHA_GREP} depends_on: - - redis - mongo - entrypoint: npm run test:acceptance:_run + - redis + user: node + command: npm run test:acceptance:_run redis: image: redis diff --git a/services/document-updater/docker-compose.yml b/services/document-updater/docker-compose.yml index d55ca56580..899e2d6a62 100644 --- a/services/document-updater/docker-compose.yml +++ b/services/document-updater/docker-compose.yml @@ -1,7 +1,7 @@ # This file was auto-generated, do not edit it directly. # Instead run bin/update_build_scripts from # https://github.com/sharelatex/sharelatex-dev-environment -# Version: 1.0.1 +# Version: 1.1.9 version: "2" @@ -11,23 +11,31 @@ services: volumes: - .:/app working_dir: /app - entrypoint: npm run test:unit + environment: + MOCHA_GREP: ${MOCHA_GREP} + command: npm run test:unit + user: node test_acceptance: image: node:6.9.5 volumes: - .:/app + working_dir: /app environment: + ELASTIC_SEARCH_DSN: es:9200 REDIS_HOST: redis MONGO_HOST: mongo + POSTGRES_HOST: postgres + MOCHA_GREP: ${MOCHA_GREP} + user: node depends_on: - - redis - mongo - working_dir: /app - entrypoint: npm run test:acceptance + - redis + command: npm run test:acceptance redis: image: redis mongo: image: mongo:3.4 + diff --git a/services/document-updater/nodemon.json b/services/document-updater/nodemon.json index 9a3be8d966..98db38d71b 100644 --- a/services/document-updater/nodemon.json +++ b/services/document-updater/nodemon.json @@ -8,9 +8,12 @@ "execMap": { "js": "npm run start" }, + "watch": [ "app/coffee/", - "app.coffee" + "app.coffee", + "config/" ], "ext": "coffee" + } diff --git a/services/document-updater/package.json b/services/document-updater/package.json index ce5aa9d18c..7d13caa763 100644 --- a/services/document-updater/package.json +++ b/services/document-updater/package.json @@ -7,16 +7,17 @@ "url": "https://github.com/sharelatex/document-updater-sharelatex.git" }, "scripts": { - "compile:app": "coffee -o app/js -c app/coffee && coffee -c app.coffee", - "start": "npm run compile:app && node app.js", - "test:acceptance:_run": "mocha --recursive --reporter spec --timeout 15000 --exit $@ test/acceptance/js", - "test:acceptance": "npm run compile:app && npm run compile:acceptance_tests && npm run test:acceptance:_run -- $@", - "test:unit:_run": "mocha --recursive --reporter spec $@ test/unit/js", - "test:unit": "npm run compile:app && npm run compile:unit_tests && npm run test:unit:_run -- $@", - "compile:unit_tests": "[ -e test/unit ] && coffee -o test/unit/js -c test/unit/coffee || echo 'No unit tests to compile'", - "compile:acceptance_tests": "[ -e test/acceptance ] && coffee -o test/acceptance/js -c test/acceptance/coffee || echo 'No acceptance tests to compile'", - "compile:all": "npm run compile:app && npm run compile:unit_tests && npm run compile:acceptance_tests", - "nodemon": "nodemon --config nodemon.json" + "compile:app": "([ -e app/coffee ] && coffee $COFFEE_OPTIONS -o app/js -c app/coffee || echo 'No CoffeeScript folder to compile') && ( [ -e app.coffee ] && coffee $COFFEE_OPTIONS -c app.coffee || echo 'No CoffeeScript app to compile')", + "start": "npm run compile:app && node $NODE_APP_OPTIONS app.js", + "test:acceptance:_run": "mocha --recursive --reporter spec --timeout 30000 --exit $@ test/acceptance/js", + "test:acceptance": "npm run compile:app && npm run compile:acceptance_tests && npm run test:acceptance:_run -- --grep=$MOCHA_GREP", + "test:unit:_run": "mocha --recursive --reporter spec --exit $@ test/unit/js", + "test:unit": "npm run compile:app && npm run compile:unit_tests && npm run test:unit:_run -- --grep=$MOCHA_GREP", + "compile:unit_tests": "[ ! -e test/unit/coffee ] && echo 'No unit tests to compile' || coffee -o test/unit/js -c test/unit/coffee", + "compile:acceptance_tests": "[ ! -e test/acceptance/coffee ] && echo 'No acceptance tests to compile' || coffee -o test/acceptance/js -c test/acceptance/coffee", + "compile:all": "npm run compile:app && npm run compile:unit_tests && npm run compile:acceptance_tests && npm run compile:smoke_tests", + "nodemon": "nodemon --config nodemon.json", + "compile:smoke_tests": "[ ! -e test/smoke/coffee ] && echo 'No smoke tests to compile' || coffee -o test/smoke/js -c test/smoke/coffee" }, "dependencies": { "async": "^2.5.0", From e471730efbb82922ba91b471e8aa4f61d601d6a8 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Mon, 23 Jul 2018 16:05:04 +0100 Subject: [PATCH 4/8] Revert "Merge pull request #33 from sharelatex/bg-add-timestamp-marker-to-project-history-queue" This reverts commit 8ec398b133aa75c45384dd7ceb630cd743f4a15c, reversing changes made to e3a6c934cf53fd245d7d0df737463cc90f954885. --- .../coffee/ProjectHistoryRedisManager.coffee | 14 ++--------- .../config/settings.defaults.coffee | 1 - .../coffee/ApplyingUpdatesToADocTests.coffee | 23 +------------------ .../ProjectHistoryRedisManagerTests.coffee | 17 +++----------- 4 files changed, 6 insertions(+), 49 deletions(-) diff --git a/services/document-updater/app/coffee/ProjectHistoryRedisManager.coffee b/services/document-updater/app/coffee/ProjectHistoryRedisManager.coffee index 1cc80ea722..42b9f16df2 100644 --- a/services/document-updater/app/coffee/ProjectHistoryRedisManager.coffee +++ b/services/document-updater/app/coffee/ProjectHistoryRedisManager.coffee @@ -4,18 +4,8 @@ rclient = require("redis-sharelatex").createClient(Settings.redis.documentupdate logger = require('logger-sharelatex') module.exports = ProjectHistoryRedisManager = - queueOps: (project_id, ops..., callback = (error, projectUpdateCount) ->) -> - multi = rclient.multi() - # Push the ops onto the project history queue - multi.rpush projectHistoryKeys.projectHistoryOps({project_id}), ops... - # To record the age of the oldest op on the queue set a timestamp if not - # already present (SETNX). - multi.setnx projectHistoryKeys.projectHistoryFirstOpTimestamp({project_id}), Date.now() - multi.exec (error, result) -> - return callback(error) if error? - # return the number of entries pushed onto the project history queue - callback null, result[0] - + queueOps: (project_id, ops..., callback) -> + rclient.rpush projectHistoryKeys.projectHistoryOps({project_id}), ops..., callback queueRenameEntity: (project_id, projectHistoryId, entity_type, entity_id, user_id, projectUpdate, callback) -> projectUpdate = diff --git a/services/document-updater/config/settings.defaults.coffee b/services/document-updater/config/settings.defaults.coffee index dfce36f6e3..18f2b1570b 100755 --- a/services/document-updater/config/settings.defaults.coffee +++ b/services/document-updater/config/settings.defaults.coffee @@ -74,7 +74,6 @@ module.exports = project_history: key_schema: projectHistoryOps: ({project_id}) -> "ProjectHistory:Ops:#{project_id}" - projectHistoryFirstOpTimestamp: ({project_id}) -> "ProjectHistory:FirstOpTimestamp:#{project_id}" # cluster: [{ # port: "7000" # host: "localhost" diff --git a/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToADocTests.coffee b/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToADocTests.coffee index c6aa6fe856..cb560a26bb 100644 --- a/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToADocTests.coffee +++ b/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToADocTests.coffee @@ -33,7 +33,7 @@ describe "Applying updates to a doc", -> before (done) -> [@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()] sinon.spy MockWebApi, "getDocument" - @startTime = Date.now() + MockWebApi.insertDoc @project_id, @doc_id, {lines: @lines, version: @version} DocUpdaterClient.sendUpdate @project_id, @doc_id, @update, (error) -> throw error if error? @@ -67,27 +67,6 @@ describe "Applying updates to a doc", -> JSON.parse(updates[0]).op.should.deep.equal @update.op done() - it "should set the first op timestamp", (done) -> - rclient_history.get ProjectHistoryKeys.projectHistoryFirstOpTimestamp({@project_id}), (error, result) => - throw error if error? - result.should.be.within(@startTime, Date.now()) - @firstOpTimestamp = result - done() - - describe "when sending another update", -> - before (done) -> - @second_update = Object.create(@update) - @second_update.v = @version + 1 - DocUpdaterClient.sendUpdate @project_id, @doc_id, @second_update, (error) -> - throw error if error? - setTimeout done, 200 - - it "should not change the first op timestamp", (done) -> - rclient_history.get ProjectHistoryKeys.projectHistoryFirstOpTimestamp({@project_id}), (error, result) => - throw error if error? - result.should.equal @firstOpTimestamp - done() - describe "when the document is loaded", -> before (done) -> [@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()] diff --git a/services/document-updater/test/unit/coffee/ProjectHistoryRedisManager/ProjectHistoryRedisManagerTests.coffee b/services/document-updater/test/unit/coffee/ProjectHistoryRedisManager/ProjectHistoryRedisManagerTests.coffee index 8c6ff5c7d7..aa777b40d6 100644 --- a/services/document-updater/test/unit/coffee/ProjectHistoryRedisManager/ProjectHistoryRedisManagerTests.coffee +++ b/services/document-updater/test/unit/coffee/ProjectHistoryRedisManager/ProjectHistoryRedisManagerTests.coffee @@ -20,7 +20,6 @@ describe "ProjectHistoryRedisManager", -> project_history: key_schema: projectHistoryOps: ({project_id}) -> "ProjectHistory:Ops:#{project_id}" - projectHistoryFirstOpTimestamp: ({project_id}) -> "ProjectHistory:FirstOpTimestamp:#{project_id}" } "redis-sharelatex": createClient: () => @rclient @@ -33,26 +32,16 @@ describe "ProjectHistoryRedisManager", -> describe "queueOps", -> beforeEach -> @ops = ["mock-op-1", "mock-op-2"] - @multi = exec: sinon.stub() - @multi.rpush = sinon.stub() - @multi.setnx = sinon.stub() - @rclient.multi = () => @multi - # @rclient = multi: () => @multi + @rclient.rpush = sinon.stub() @ProjectHistoryRedisManager.queueOps @project_id, @ops..., @callback it "should queue an update", -> - @multi.rpush + @rclient.rpush .calledWithExactly( "ProjectHistory:Ops:#{@project_id}" @ops[0] @ops[1] - ).should.equal true - - it "should set the queue timestamp if not present", -> - @multi.setnx - .calledWithExactly( - "ProjectHistory:FirstOpTimestamp:#{@project_id}" - Date.now() + @callback ).should.equal true describe "queueRenameEntity", -> From fdcb806518761c9e015559e072fe87c25bbfae71 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Fri, 20 Jul 2018 10:43:31 +0100 Subject: [PATCH 5/8] set a timestamp for the first entry in the projectHistory:Ops queue --- .../coffee/ProjectHistoryRedisManager.coffee | 14 +++++++++-- .../config/settings.defaults.coffee | 1 + .../coffee/ApplyingUpdatesToADocTests.coffee | 23 ++++++++++++++++++- .../ProjectHistoryRedisManagerTests.coffee | 17 +++++++++++--- 4 files changed, 49 insertions(+), 6 deletions(-) diff --git a/services/document-updater/app/coffee/ProjectHistoryRedisManager.coffee b/services/document-updater/app/coffee/ProjectHistoryRedisManager.coffee index 42b9f16df2..1cc80ea722 100644 --- a/services/document-updater/app/coffee/ProjectHistoryRedisManager.coffee +++ b/services/document-updater/app/coffee/ProjectHistoryRedisManager.coffee @@ -4,8 +4,18 @@ rclient = require("redis-sharelatex").createClient(Settings.redis.documentupdate logger = require('logger-sharelatex') module.exports = ProjectHistoryRedisManager = - queueOps: (project_id, ops..., callback) -> - rclient.rpush projectHistoryKeys.projectHistoryOps({project_id}), ops..., callback + queueOps: (project_id, ops..., callback = (error, projectUpdateCount) ->) -> + multi = rclient.multi() + # Push the ops onto the project history queue + multi.rpush projectHistoryKeys.projectHistoryOps({project_id}), ops... + # To record the age of the oldest op on the queue set a timestamp if not + # already present (SETNX). + multi.setnx projectHistoryKeys.projectHistoryFirstOpTimestamp({project_id}), Date.now() + multi.exec (error, result) -> + return callback(error) if error? + # return the number of entries pushed onto the project history queue + callback null, result[0] + queueRenameEntity: (project_id, projectHistoryId, entity_type, entity_id, user_id, projectUpdate, callback) -> projectUpdate = diff --git a/services/document-updater/config/settings.defaults.coffee b/services/document-updater/config/settings.defaults.coffee index 18f2b1570b..dfce36f6e3 100755 --- a/services/document-updater/config/settings.defaults.coffee +++ b/services/document-updater/config/settings.defaults.coffee @@ -74,6 +74,7 @@ module.exports = project_history: key_schema: projectHistoryOps: ({project_id}) -> "ProjectHistory:Ops:#{project_id}" + projectHistoryFirstOpTimestamp: ({project_id}) -> "ProjectHistory:FirstOpTimestamp:#{project_id}" # cluster: [{ # port: "7000" # host: "localhost" diff --git a/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToADocTests.coffee b/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToADocTests.coffee index cb560a26bb..c6aa6fe856 100644 --- a/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToADocTests.coffee +++ b/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToADocTests.coffee @@ -33,7 +33,7 @@ describe "Applying updates to a doc", -> before (done) -> [@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()] sinon.spy MockWebApi, "getDocument" - + @startTime = Date.now() MockWebApi.insertDoc @project_id, @doc_id, {lines: @lines, version: @version} DocUpdaterClient.sendUpdate @project_id, @doc_id, @update, (error) -> throw error if error? @@ -67,6 +67,27 @@ describe "Applying updates to a doc", -> JSON.parse(updates[0]).op.should.deep.equal @update.op done() + it "should set the first op timestamp", (done) -> + rclient_history.get ProjectHistoryKeys.projectHistoryFirstOpTimestamp({@project_id}), (error, result) => + throw error if error? + result.should.be.within(@startTime, Date.now()) + @firstOpTimestamp = result + done() + + describe "when sending another update", -> + before (done) -> + @second_update = Object.create(@update) + @second_update.v = @version + 1 + DocUpdaterClient.sendUpdate @project_id, @doc_id, @second_update, (error) -> + throw error if error? + setTimeout done, 200 + + it "should not change the first op timestamp", (done) -> + rclient_history.get ProjectHistoryKeys.projectHistoryFirstOpTimestamp({@project_id}), (error, result) => + throw error if error? + result.should.equal @firstOpTimestamp + done() + describe "when the document is loaded", -> before (done) -> [@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()] diff --git a/services/document-updater/test/unit/coffee/ProjectHistoryRedisManager/ProjectHistoryRedisManagerTests.coffee b/services/document-updater/test/unit/coffee/ProjectHistoryRedisManager/ProjectHistoryRedisManagerTests.coffee index aa777b40d6..8c6ff5c7d7 100644 --- a/services/document-updater/test/unit/coffee/ProjectHistoryRedisManager/ProjectHistoryRedisManagerTests.coffee +++ b/services/document-updater/test/unit/coffee/ProjectHistoryRedisManager/ProjectHistoryRedisManagerTests.coffee @@ -20,6 +20,7 @@ describe "ProjectHistoryRedisManager", -> project_history: key_schema: projectHistoryOps: ({project_id}) -> "ProjectHistory:Ops:#{project_id}" + projectHistoryFirstOpTimestamp: ({project_id}) -> "ProjectHistory:FirstOpTimestamp:#{project_id}" } "redis-sharelatex": createClient: () => @rclient @@ -32,16 +33,26 @@ describe "ProjectHistoryRedisManager", -> describe "queueOps", -> beforeEach -> @ops = ["mock-op-1", "mock-op-2"] - @rclient.rpush = sinon.stub() + @multi = exec: sinon.stub() + @multi.rpush = sinon.stub() + @multi.setnx = sinon.stub() + @rclient.multi = () => @multi + # @rclient = multi: () => @multi @ProjectHistoryRedisManager.queueOps @project_id, @ops..., @callback it "should queue an update", -> - @rclient.rpush + @multi.rpush .calledWithExactly( "ProjectHistory:Ops:#{@project_id}" @ops[0] @ops[1] - @callback + ).should.equal true + + it "should set the queue timestamp if not present", -> + @multi.setnx + .calledWithExactly( + "ProjectHistory:FirstOpTimestamp:#{@project_id}" + Date.now() ).should.equal true describe "queueRenameEntity", -> From aa013f0bee14441462fc9a8fe372c9abe5b23b7d Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Thu, 16 Aug 2018 11:13:11 +0100 Subject: [PATCH 6/8] limit parallel resync doc requests to web --- services/document-updater/app/coffee/HistoryManager.coffee | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/services/document-updater/app/coffee/HistoryManager.coffee b/services/document-updater/app/coffee/HistoryManager.coffee index 9d39166681..802d944ced 100644 --- a/services/document-updater/app/coffee/HistoryManager.coffee +++ b/services/document-updater/app/coffee/HistoryManager.coffee @@ -65,10 +65,12 @@ module.exports = HistoryManager = newBlock = Math.floor(length / threshold) return newBlock != prevBlock + MAX_PARALLEL_REQUESTS: 4 + resyncProjectHistory: (project_id, projectHistoryId, docs, files, callback) -> ProjectHistoryRedisManager.queueResyncProjectStructure project_id, projectHistoryId, docs, files, (error) -> return callback(error) if error? DocumentManager = require "./DocumentManager" resyncDoc = (doc, cb) -> DocumentManager.resyncDocContentsWithLock project_id, doc.doc, cb - async.each docs, resyncDoc, callback + async.eachLimit docs, HistoryManager.MAX_PARALLEL_REQUESTS, resyncDoc, callback From 910b27357da6995c890c3b42a44837aae6d1f32f Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Thu, 16 Aug 2018 11:14:11 +0100 Subject: [PATCH 7/8] add more logging to resync operations --- .../document-updater/app/coffee/DocumentManager.coffee | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/services/document-updater/app/coffee/DocumentManager.coffee b/services/document-updater/app/coffee/DocumentManager.coffee index 0c50d9b1f3..39713a1981 100644 --- a/services/document-updater/app/coffee/DocumentManager.coffee +++ b/services/document-updater/app/coffee/DocumentManager.coffee @@ -181,14 +181,19 @@ module.exports = DocumentManager = callback(null, lines, version) resyncDocContents: (project_id, doc_id, callback) -> + logger.log {project_id: project_id, doc_id: doc_id}, "start resyncing doc contents" RedisManager.getDoc project_id, doc_id, (error, lines, version, ranges, pathname, projectHistoryId) -> return callback(error) if error? if !lines? or !version? + logger.log {project_id: project_id, doc_id: doc_id}, "resyncing doc contents - not found in redis - retrieving from web" PersistenceManager.getDoc project_id, doc_id, (error, lines, version, ranges, pathname, projectHistoryId) -> - return callback(error) if error? + if error? + logger.error {project_id: project_id, doc_id: doc_id, getDocError: error}, "resyncing doc contents - error retrieving from web" + return callback(error) ProjectHistoryRedisManager.queueResyncDocContent project_id, projectHistoryId, doc_id, lines, version, pathname, callback else + logger.log {project_id: project_id, doc_id: doc_id}, "resyncing doc contents - doc in redis - will queue in redis" ProjectHistoryRedisManager.queueResyncDocContent project_id, projectHistoryId, doc_id, lines, version, pathname, callback getDocWithLock: (project_id, doc_id, callback = (error, lines, version) ->) -> From c128e0ab1c4859e562c5600049a8d8327780c826 Mon Sep 17 00:00:00 2001 From: Alasdair Smith Date: Tue, 9 Oct 2018 11:46:27 +0100 Subject: [PATCH 8/8] Use setting instead of hard-coding port --- services/document-updater/config/settings.defaults.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/document-updater/config/settings.defaults.coffee b/services/document-updater/config/settings.defaults.coffee index dfce36f6e3..02de30553a 100755 --- a/services/document-updater/config/settings.defaults.coffee +++ b/services/document-updater/config/settings.defaults.coffee @@ -10,7 +10,7 @@ module.exports = apis: web: - url: "http://#{process.env["WEB_HOST"] or "localhost"}:3000" + url: "http://#{process.env["WEB_HOST"] or "localhost"}:#{process.env['WEB_PORT'] or 3000}" user: "sharelatex" pass: "password" trackchanges: