diff --git a/services/web/app/coffee/Features/Compile/CompileManager.coffee b/services/web/app/coffee/Features/Compile/CompileManager.coffee index 8a5ee7bd4f..e9e3177097 100644 --- a/services/web/app/coffee/Features/Compile/CompileManager.coffee +++ b/services/web/app/coffee/Features/Compile/CompileManager.coffee @@ -8,8 +8,7 @@ ProjectRootDocManager = require "../Project/ProjectRootDocManager" ClsiManager = require "./ClsiManager" Metrics = require('../../infrastructure/Metrics') logger = require("logger-sharelatex") -RateLimiter = require("ratelimiter") - +rateLimiter = require("../../infrastructure/RateLimiter") module.exports = CompileManager = compile: (project_id, user_id, opt = {}, _callback = (error) ->) -> @@ -56,17 +55,15 @@ module.exports = CompileManager = _checkIfAutoCompileLimitHasBeenHit: (isAutoCompile, callback = (err, canCompile)->)-> if !isAutoCompile return callback(null, true) - key = "auto_compile_rate_limit" - ten_seconds = (10 * 1000) - limit = new RateLimiter(db:rclient, id:key, max:7, duration:ten_seconds) - limit.get (err, limit)-> - Metrics.inc("compile.autocompile.rateLimitCheck") - if limit.remaining > 0 and !err? - canCompile = true - else + opts = + endpointName:"auto_compile" + timeInterval:10 + subjectName:"everyone" + throttle: 5 + rateLimiter.addCount opts, (err, canCompile)-> + if err? canCompile = false - Metrics.inc("compile.autocompile.rateLimitHit") - logger.log canCompile:canCompile, limit:limit, "checking if auto compile limit has been hit" + logger.log canCompile:canCompile, opts:opts, "checking if auto compile limit has been hit" callback err, canCompile _ensureRootDocumentIsSet: (project_id, callback = (error) ->) -> diff --git a/services/web/app/coffee/infrastructure/RateLimiter.coffee b/services/web/app/coffee/infrastructure/RateLimiter.coffee new file mode 100644 index 0000000000..2860e70656 --- /dev/null +++ b/services/web/app/coffee/infrastructure/RateLimiter.coffee @@ -0,0 +1,9 @@ +redback = require("redback").createClient() + +module.exports = + + addCount: (opts, callback = (opts, shouldProcess)->)-> + ratelimit = redback.createRateLimit(opts.endpointName) + ratelimit.addCount opts.subjectName, opts.timeInterval, (err, callCount)-> + shouldProcess = callCount < opts.throttle + callback(err, shouldProcess) \ No newline at end of file diff --git a/services/web/package.json b/services/web/package.json index 1261713176..46830b5875 100644 --- a/services/web/package.json +++ b/services/web/package.json @@ -23,21 +23,21 @@ "lynx": "0.0.11", "session.socket.io": "0.1.4", "socket.io": "0.9.15", - "mimelib": "~0.2.8", - "bufferedstream": "~1.4.1", + "mimelib": "0.2.8", + "bufferedstream": "1.4.1", "mixpanel": "0.0.18", "settings-sharelatex": "git+https://github.com/sharelatex/settings-sharelatex.git#master", "logger-sharelatex": "git+https://github.com/sharelatex/logger-sharelatex.git#master", "soa-req-id": "git+https://github.com/sharelatex/soa-req-id.git#master", "fairy": "0.0.2", - "node-uuid": "~1.4.0", + "node-uuid": "1.4.0", "mongojs": "0.9.8", "node-ses": "0.0.3", "bcrypt": "0.7.5", - "archiver": "~0.5.1", - "ratelimiter": "~1.0.0", - "nodetime": "~0.8.15", - "mocha": "~1.17.1" + "archiver": "0.5.1", + "nodetime": "0.8.15", + "mocha": "1.17.1", + "redback": "0.3.7" }, "devDependencies": { "chai": "", @@ -45,16 +45,16 @@ "sandboxed-module": "0.2.0", "timekeeper": "", "sinon": "", - "grunt-concurrent": "~0.4.3", - "grunt-contrib-clean": "~0.5.0", - "grunt-contrib-coffee": "~0.10.0", - "grunt-nodemon": "~0.2.0", - "grunt-contrib-less": "~0.9.0", - "grunt-mocha-test": "~0.9.0", - "grunt-available-tasks": "~0.4.1", - "grunt-contrib-requirejs": "~0.4.1", - "grunt-execute": "~0.1.5", - "bunyan": "~0.22.1", - "grunt-bunyan": "~0.5.0" + "grunt-concurrent": "0.4.3", + "grunt-contrib-clean": "0.5.0", + "grunt-contrib-coffee": "0.10.0", + "grunt-nodemon": "0.2.0", + "grunt-contrib-less": "0.9.0", + "grunt-mocha-test": "0.9.0", + "grunt-available-tasks": "0.4.1", + "grunt-contrib-requirejs": "0.4.1", + "grunt-execute": "0.1.5", + "bunyan": "0.22.1", + "grunt-bunyan": "0.5.0" } } diff --git a/services/web/test/UnitTests/coffee/Compile/CompileManagerTests.coffee b/services/web/test/UnitTests/coffee/Compile/CompileManagerTests.coffee index 4a9dfa8425..09561959f4 100644 --- a/services/web/test/UnitTests/coffee/Compile/CompileManagerTests.coffee +++ b/services/web/test/UnitTests/coffee/Compile/CompileManagerTests.coffee @@ -10,11 +10,8 @@ describe "CompileManager", -> beforeEach -> @rateLimitGetStub = sinon.stub() rateLimitGetStub = @rateLimitGetStub - @ratelimiter = class RateLimiter - constructor: -> - return { - get: rateLimitGetStub - } + @ratelimiter = + addCount: sinon.stub() @CompileManager = SandboxedModule.require modulePath, requires: "settings-sharelatex": @settings = redis: web: {host: "localhost", port: 42} @@ -25,7 +22,7 @@ describe "CompileManager", -> "../../models/Project": Project: @Project = {} "./ClsiManager": @ClsiManager = {} "../../managers/LatexManager": @LatexManager = {} - "ratelimiter":@ratelimiter + "../../infrastructure/RateLimiter": @ratelimiter "../../infrastructure/Metrics": @Metrics = Timer: class Timer done: sinon.stub() @@ -53,7 +50,6 @@ describe "CompileManager", -> .calledWith(@project_id, @user_id) .should.equal true - it "should flush the project to the database", -> @DocumentUpdaterHandler.flushProjectToMongo .calledWith(@project_id) @@ -203,29 +199,30 @@ describe "CompileManager", -> describe "_checkIfAutoCompileLimitHasBeenHit", -> it "should be able to compile if it is not an autocompile", (done)-> - limit = {remaining:-1} - @rateLimitGetStub.callsArgWith(0, null, limit) + @ratelimiter.addCount.callsArgWith(1, null, true) @CompileManager._checkIfAutoCompileLimitHasBeenHit false, (err, canCompile)=> canCompile.should.equal true done() it "should be able to compile if rate limit has remianing", (done)-> - limit = {remaining:3} - @rateLimitGetStub.callsArgWith(0, null, limit) + @ratelimiter.addCount.callsArgWith(1, null, true) @CompileManager._checkIfAutoCompileLimitHasBeenHit true, (err, canCompile)=> + args = @ratelimiter.addCount.args[0][0] + args.throttle.should.equal 5 + args.subjectName.should.equal "everyone" + args.timeInterval.should.equal 10 + args.endpointName.should.equal "auto_compile" canCompile.should.equal true done() it "should be not able to compile if rate limit has no remianing", (done)-> - limit = {remaining:0} - @rateLimitGetStub.callsArgWith(0, null, limit) + @ratelimiter.addCount.callsArgWith(1, null, false) @CompileManager._checkIfAutoCompileLimitHasBeenHit true, (err, canCompile)=> canCompile.should.equal false done() it "should return false if there is an error in the rate limit", (done)-> - limit = {remaining:4} - @rateLimitGetStub.callsArgWith(0, "Err", limit) + @ratelimiter.addCount.callsArgWith(1, "error") @CompileManager._checkIfAutoCompileLimitHasBeenHit true, (err, canCompile)=> canCompile.should.equal false done() diff --git a/services/web/test/UnitTests/coffee/infrastructure/RateLimterTests.coffee b/services/web/test/UnitTests/coffee/infrastructure/RateLimterTests.coffee new file mode 100644 index 0000000000..e513c11cdf --- /dev/null +++ b/services/web/test/UnitTests/coffee/infrastructure/RateLimterTests.coffee @@ -0,0 +1,73 @@ +assert = require("chai").assert +sinon = require('sinon') +chai = require('chai') +should = chai.should() +expect = chai.expect +modulePath = "../../../../app/js/infrastructure/RateLimiter.js" +SandboxedModule = require('sandboxed-module') + +describe "FileStoreHandler", -> + + beforeEach -> + @settings = apis:{filestore:{url:"http//filestore.sharelatex.test"}} + @redbackInstance = + addCount: sinon.stub() + + @redback = + createRateLimit: sinon.stub().returns(@redbackInstance) + + @limiter = SandboxedModule.require modulePath, requires: + "settings-sharelatex":@settings + "logger-sharelatex" : @logger = {log:sinon.stub(), err:sinon.stub()} + "redback": createClient: => @redback + + @endpointName = "compiles" + @subject = "some project id" + @timeInterval = 20 + @throttleLimit = 5 + + @details = + endpointName: @endpointName + subjectName: @subject + throttle: @throttleLimit + timeInterval: @timeInterval + + + describe "addCount", -> + + beforeEach -> + @redbackInstance.addCount.callsArgWith(2, null, 10) + + it "should use correct namespace", (done)-> + @limiter.addCount @details, => + @redback.createRateLimit.calledWith(@endpointName).should.equal true + done() + + it "should only call it once", (done)-> + @limiter.addCount @details, => + @redbackInstance.addCount.callCount.should.equal 1 + done() + + it "should use the subjectName", (done)-> + @limiter.addCount @details, => + @redbackInstance.addCount.calledWith(@details.subjectName, @details.timeInterval).should.equal true + done() + + it "should return true if the count is less than throttle", (done)-> + @details.throttle = 100 + @limiter.addCount @details, (err, canProcess)=> + canProcess.should.equal true + done() + + it "should return true if the count is less than throttle", (done)-> + @details.throttle = 1 + @limiter.addCount @details, (err, canProcess)=> + canProcess.should.equal false + done() + + it "should return false if the limit is matched", (done)-> + @details.throttle = 10 + @limiter.addCount @details, (err, canProcess)=> + canProcess.should.equal false + done() +