Kaynağa Gözat

added new rate limit file based on redback. converetd auto compile to
use it.

Henry Oswald 12 yıl önce
ebeveyn
işleme
ff8320bce0

+ 9 - 12
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) ->) ->

+ 9 - 0
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)

+ 18 - 18
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"
   }
 }

+ 12 - 15
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()

+ 73 - 0
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()
+