diff --git a/services/web/app/coffee/Features/Dropbox/DropboxWebhookController.coffee b/services/web/app/coffee/Features/Dropbox/DropboxWebhookController.coffee index 3509d58687..5726b51bc4 100644 --- a/services/web/app/coffee/Features/Dropbox/DropboxWebhookController.coffee +++ b/services/web/app/coffee/Features/Dropbox/DropboxWebhookController.coffee @@ -10,6 +10,10 @@ module.exports = DropboxWebhookController = logger.log dropbox_uids: dropbox_uids, "received webhook request from Dropbox" if !dropbox_uids? return res.send(400) # Bad Request + + # Do this in the background so as not to keep Dropbox waiting DropboxWebhookHandler.pollDropboxUids dropbox_uids, (error) -> - return next(error) if error? - res.send(200) \ No newline at end of file + if error? + logger.error err: error, dropbox_uids: dropbox_uids, "error in webhook" + + res.send(200) \ No newline at end of file diff --git a/services/web/app/coffee/Features/Dropbox/DropboxWebhookHandler.coffee b/services/web/app/coffee/Features/Dropbox/DropboxWebhookHandler.coffee index 8e48489bed..d29a0d3aa5 100644 --- a/services/web/app/coffee/Features/Dropbox/DropboxWebhookHandler.coffee +++ b/services/web/app/coffee/Features/Dropbox/DropboxWebhookHandler.coffee @@ -1,8 +1,13 @@ logger = require("logger-sharelatex") +settings = require("settings-sharelatex") async = require "async" User = require("../../models/User").User TpdsUpdateSender = require "../ThirdPartyDataStore/TpdsUpdateSender" +redis = require('redis') +rclient = redis.createClient(settings.redis.web.port, settings.redis.web.host) +rclient.auth(settings.redis.web.password) + module.exports = DropboxWebhookHandler = pollDropboxUids: (dropbox_uids, callback = (error) ->) -> jobs = [] @@ -13,13 +18,32 @@ module.exports = DropboxWebhookHandler = async.series jobs, callback pollDropboxUid: (dropbox_uid, callback = (error) ->) -> - User.find { - "dropbox.access_token.uid": dropbox_uid.toString() - "features.dropbox": true - }, (error, users = []) -> + DropboxWebhookHandler._delayAndBatchPoll dropbox_uid, (error, shouldPoll) -> return callback(error) if error? - user = users[0] - if !user? - logger.log dropbox_uid: dropbox_uid, "no sharelatex user found" - return callback() - TpdsUpdateSender.pollDropboxForUser user._id, callback \ No newline at end of file + return callback() if !shouldPoll + User.find { + "dropbox.access_token.uid": dropbox_uid.toString() + "features.dropbox": true + }, (error, users = []) -> + return callback(error) if error? + user = users[0] + if !user? + logger.log dropbox_uid: dropbox_uid, "no sharelatex user found" + return callback() + TpdsUpdateSender.pollDropboxForUser user._id, callback + + POLL_DELAY_IN_MS: 5000 # 5 seconds + _delayAndBatchPoll: (dropbox_uid, callback = (error, shouldPoll) ->) -> + rclient.set( + "dropbox-poll-lock:#{dropbox_uid}", "LOCK", + "PX", DropboxWebhookHandler.POLL_DELAY_IN_MS, + "NX", + (error, gotLock) -> + return callback(error) if error? + if gotLock + setTimeout () -> + callback(null, true) + , DropboxWebhookHandler.POLL_DELAY_IN_MS + else + callback(null, false) + ) \ No newline at end of file diff --git a/services/web/test/UnitTests/coffee/Dropbox/DropboxWebhookHandlerTests.coffee b/services/web/test/UnitTests/coffee/Dropbox/DropboxWebhookHandlerTests.coffee index 82a0220858..60519532e2 100644 --- a/services/web/test/UnitTests/coffee/Dropbox/DropboxWebhookHandlerTests.coffee +++ b/services/web/test/UnitTests/coffee/Dropbox/DropboxWebhookHandlerTests.coffee @@ -1,6 +1,7 @@ SandboxedModule = require('sandboxed-module') assert = require('assert') require('chai').should() +expect = require("chai").expect sinon = require('sinon') modulePath = require('path').join __dirname, '../../../../app/js/Features/Dropbox/DropboxWebhookHandler.js' @@ -9,6 +10,10 @@ describe 'DropboxWebhookHandler', -> @DropboxWebhookHandler = SandboxedModule.require modulePath, requires: "../../models/User": User: @User = {} "../ThirdPartyDataStore/TpdsUpdateSender": @TpdsUpdateSender = {} + "redis": + createClient: () => @rclient = + auth: sinon.stub() + 'settings-sharelatex': redis: web: {} 'logger-sharelatex': log:-> err:-> @@ -35,17 +40,60 @@ describe 'DropboxWebhookHandler', -> @user_id = "sharelatex-user-id" @User.find = sinon.stub().callsArgWith(1, null, [ _id: @user_id ]) @TpdsUpdateSender.pollDropboxForUser = sinon.stub().callsArg(1) - @DropboxWebhookHandler.pollDropboxUid @dropbox_uid, @callback - it "should look up the user", -> - @User.find - .calledWith({ "dropbox.access_token.uid": @dropbox_uid, "features.dropbox": true }) - .should.equal true + describe "when there is already a poll in progress", () -> + beforeEach -> + @DropboxWebhookHandler._delayAndBatchPoll = sinon.stub().callsArgWith(1, null, false) + @DropboxWebhookHandler.pollDropboxUid @dropbox_uid, @callback - it "should poll the user's Dropbox", -> - @TpdsUpdateSender.pollDropboxForUser - .calledWith(@user_id) - .should.equal true + it "should not go ahead with the poll", -> + @TpdsUpdateSender.pollDropboxForUser.called.should.equal false + + describe "when we are the one to do the delayed poll", () -> + beforeEach -> + @DropboxWebhookHandler._delayAndBatchPoll = sinon.stub().callsArgWith(1, null, true) + @DropboxWebhookHandler.pollDropboxUid @dropbox_uid, @callback + + it "should look up the user", -> + @User.find + .calledWith({ "dropbox.access_token.uid": @dropbox_uid, "features.dropbox": true }) + .should.equal true + + it "should poll the user's Dropbox", -> + @TpdsUpdateSender.pollDropboxForUser + .calledWith(@user_id) + .should.equal true + + it "should call the callback", -> + @callback.called.should.equal true + + describe "_delayAndBatchPoll", () -> + beforeEach -> + @dropbox_uid = "dropbox-uid-123" + @DropboxWebhookHandler.POLL_DELAY_IN_MS = 100 + + describe "when no one else is polling yet", -> + beforeEach (done) -> + @rclient.set = sinon.stub().callsArgWith(5, null, "OK") + @start = Date.now() + @DropboxWebhookHandler._delayAndBatchPoll @dropbox_uid, (error, @shouldPoll) => + @end = Date.now() + done() + + it "should set the lock", -> + @rclient.set + .calledWith("dropbox-poll-lock:#{@dropbox_uid}", "LOCK", "PX", @DropboxWebhookHandler.POLL_DELAY_IN_MS, "NX") + .should.equal true + + it "should return the callback after the delay with shouldPoll=true", -> + @shouldPoll.should.equal true + expect(@end - @start).to.be.at.least(@DropboxWebhookHandler.POLL_DELAY_IN_MS) + + describe "when someone else is already polling", -> + beforeEach -> + @rclient.set = sinon.stub().callsArgWith(5, null, null) + @DropboxWebhookHandler._delayAndBatchPoll @dropbox_uid, @callback + + it "should return the callback immediately with shouldPoll=false", -> + @callback.calledWith(null, false).should.equal true - it "should call the callback", -> - @callback.called.should.equal true