Delay Dropbox polling by 5 seconds and dispatch only a single request

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?
+			if error?
-			res.send(200)
+				logger.error err: error, dropbox_uids: dropbox_uids, "error in webhook"
+		
+		res.send(200)

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 {
+		DropboxWebhookHandler._delayAndBatchPoll dropbox_uid, (error, shouldPoll) ->
-			"dropbox.access_token.uid": dropbox_uid.toString()
-			"features.dropbox": true
-		}, (error, users = []) ->
 			return callback(error) if error?
-			user = users[0]
+			return callback() if !shouldPoll
-			if !user?
+			User.find {
-				logger.log dropbox_uid: dropbox_uid, "no sharelatex user found"
+				"dropbox.access_token.uid": dropbox_uid.toString()
-				return callback()
+				"features.dropbox": true
-			TpdsUpdateSender.pollDropboxForUser user._id, callback
+			}, (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)
+		)

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", ->
+		describe "when there is already a poll in progress", () ->
-			@User.find
+			beforeEach ->
-				.calledWith({ "dropbox.access_token.uid": @dropbox_uid, "features.dropbox": true })
+				@DropboxWebhookHandler._delayAndBatchPoll = sinon.stub().callsArgWith(1, null, false)
-				.should.equal true
+				@DropboxWebhookHandler.pollDropboxUid @dropbox_uid, @callback
 				
-		it "should poll the user's Dropbox", ->
+			it "should not go ahead with the poll", ->
-			@TpdsUpdateSender.pollDropboxForUser
+				@TpdsUpdateSender.pollDropboxForUser.called.should.equal false
-				.calledWith(@user_id)
+				
-				.should.equal true
+		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