From 3da439aa1ed3156ef8182111859ea8afb302cd86 Mon Sep 17 00:00:00 2001 From: Konstantin Gredeskoul Date: Thu, 5 Jan 2017 16:04:55 -0800 Subject: [PATCH 01/16] Updating Ruby test matrix --- .travis.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index f628758..db299c2 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,10 +1,12 @@ language: ruby rvm: - 1.9.3 + - 2.3.3 + - 2.4.0 script: "bundle exec rspec" notifications: email: recipients: - - dev-info@wanelo.com + - kigster@gmail.com on_success: never on_failure: always From 5c063c1d3db30b88914cdd338d9ce495627b6eb1 Mon Sep 17 00:00:00 2001 From: Konstantin Gredeskoul Date: Thu, 5 Jan 2017 16:05:16 -0800 Subject: [PATCH 02/16] Use gem spec as required by the specification --- Gemfile | 8 -------- pause.gemspec | 7 +++++++ 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/Gemfile b/Gemfile index 26df920..288ca27 100644 --- a/Gemfile +++ b/Gemfile @@ -5,11 +5,3 @@ source 'https://rubygems.org' # Specify your gem's dependencies in pause.gemspec gemspec - -gem 'fakeredis' -gem 'guard-rspec' -gem 'pry-nav' -gem 'rake' -gem 'rb-fsevent' -gem 'rspec' -gem 'timecop' diff --git a/pause.gemspec b/pause.gemspec index a7d9388..64a3733 100644 --- a/pause.gemspec +++ b/pause.gemspec @@ -18,4 +18,11 @@ Gem::Specification.new do |gem| gem.require_paths = ["lib"] gem.add_dependency 'redis' + + gem.add_development_dependency 'rspec' + gem.add_development_dependency 'guard-rspec' + gem.add_development_dependency 'pry-nav' + gem.add_development_dependency 'rb-fsevent' + gem.add_development_dependency 'timecop' + gem.add_development_dependency 'rake' end From a289bbfb217166ef2944000cb3b9443565878d62 Mon Sep 17 00:00:00 2001 From: Konstantin Gredeskoul Date: Thu, 5 Jan 2017 16:05:44 -0800 Subject: [PATCH 03/16] Simplify method overriding for ShardedAdapter --- lib/pause/redis/adapter.rb | 38 ++++++++++++++++-------- lib/pause/redis/sharded_adapter.rb | 23 ++++++-------- spec/pause/redis/sharded_adapter_spec.rb | 14 +++++++++ 3 files changed, 49 insertions(+), 26 deletions(-) diff --git a/lib/pause/redis/adapter.rb b/lib/pause/redis/adapter.rb index f799184..f423f3b 100644 --- a/lib/pause/redis/adapter.rb +++ b/lib/pause/redis/adapter.rb @@ -10,20 +10,27 @@ class Adapter attr_accessor :resolution, :time_blocks_to_keep, :history def initialize(config) - @resolution = config.resolution + @resolution = config.resolution @time_blocks_to_keep = config.history / @resolution - @history = config.history + @history = config.history + end + + # Override in subclasses to disable + def with_multi + redis.multi do |redis| + yield(redis) if block_given? + end end def increment(scope, identifier, timestamp, count = 1) k = tracked_key(scope, identifier) - redis.multi do |redis| + with_multi do |redis| redis.zincrby k, count, period_marker(resolution, timestamp) redis.expire k, history end if redis.zcard(k) > time_blocks_to_keep - list = extract_set_elements(k) + list = extract_set_elements(k) to_remove = list.slice(0, (list.size - time_blocks_to_keep)) redis.zrem(k, to_remove.map(&:ts)) end @@ -78,7 +85,7 @@ def enable(scope) end def disabled?(scope) - ! enabled?(scope) + !enabled?(scope) end def enabled?(scope) @@ -89,18 +96,25 @@ def expire_block_list(scope) redis.zremrangebyscore rate_limited_list(scope), '-inf', Time.now.to_i end + protected + + def redis + @redis_conn ||= ::Redis.new(redis_connection_opts) + end + + def redis_connection_opts + { host: Pause.config.redis_host, + port: Pause.config.redis_port, + db: Pause.config.redis_db } + end + private def delete_tracking_keys(scope, ids) - increment_keys = ids.map{ |key| tracked_key(scope, key) } + increment_keys = ids.map { |key| tracked_key(scope, key) } redis.del(increment_keys) end - def redis - @redis_conn ||= ::Redis.new(host: Pause.config.redis_host, - port: Pause.config.redis_port, - db: Pause.config.redis_db) - end def tracked_scope(scope) ['i', scope].join(':') @@ -117,7 +131,7 @@ def rate_limited_list(scope) def keys(key_scope) redis.keys("#{key_scope}:*").map do |key| - key.gsub(/^#{key_scope}:/, "").tr('|','') + key.gsub(/^#{key_scope}:/, "").tr('|', '') end end diff --git a/lib/pause/redis/sharded_adapter.rb b/lib/pause/redis/sharded_adapter.rb index 17ea6ae..88383bb 100644 --- a/lib/pause/redis/sharded_adapter.rb +++ b/lib/pause/redis/sharded_adapter.rb @@ -7,26 +7,21 @@ class OperationNotSupported < StandardError # Operations that are not possible when data is sharded # raise an error. class ShardedAdapter < Adapter - def increment(scope, identifier, timestamp, count = 1) - k = tracked_key(scope, identifier) - redis.zincrby k, count, period_marker(resolution, timestamp) - redis.expire k, history - if redis.zcard(k) > time_blocks_to_keep - list = extract_set_elements(k) - to_remove = list.slice(0, (list.size - time_blocks_to_keep)) - redis.zrem(k, to_remove.map(&:ts)) - end + # Overrides real multi which is not possible when sharded. + def with_multi + yield(redis) if block_given? end + protected - private - - def redis - @redis_conn ||= ::Redis.new(host: Pause.config.redis_host, - port: Pause.config.redis_port) + def redis_connection_opts + { host: Pause.config.redis_host, + port: Pause.config.redis_port } end + private + def keys(_key_scope) raise OperationNotSupported.new('Can not be executed when Pause is configured in sharded mode') end diff --git a/spec/pause/redis/sharded_adapter_spec.rb b/spec/pause/redis/sharded_adapter_spec.rb index 14804a3..cbccbf9 100644 --- a/spec/pause/redis/sharded_adapter_spec.rb +++ b/spec/pause/redis/sharded_adapter_spec.rb @@ -21,4 +21,18 @@ expect { adapter.all_keys('cake') }.to raise_error(Pause::Redis::OperationNotSupported) end end + + describe '#with_multi' do + let(:redis) { adapter.send(:redis) } + it 'should not call redis.multi' do + expect(redis).to_not receive(:multi) + expect { adapter.increment(:scope, 123, Time.now) }.to_not raise_error + end + end + + describe '#redis' do + it 'should not use redis db when connecting' do + expect(adapter.send(:redis_connection_opts)).to_not include(:db) + end + end end From 4990e2bf92a673c3b00908c0e202d72114a5d5e3 Mon Sep 17 00:00:00 2001 From: Konstantin Gredeskoul Date: Thu, 5 Jan 2017 16:05:56 -0800 Subject: [PATCH 04/16] =?UTF-8?q?Kill=20fakeredis=20as=20it=E2=80=99s=20ma?= =?UTF-8?q?sking=20real=20issues.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- spec/spec_helper.rb | 1 - spec/support/fakeredis.rb | 2 -- 2 files changed, 3 deletions(-) delete mode 100644 spec/support/fakeredis.rb diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index b14d901..56a5ec4 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -10,7 +10,6 @@ require 'bundler/setup' if File.exists?(ENV['BUNDLE_GEMFILE']) require 'pause' require 'pry' -require 'support/fakeredis' RSpec.configure do |config| config.run_all_when_everything_filtered = true diff --git a/spec/support/fakeredis.rb b/spec/support/fakeredis.rb deleted file mode 100644 index 0049433..0000000 --- a/spec/support/fakeredis.rb +++ /dev/null @@ -1,2 +0,0 @@ -require 'fakeredis/rspec' - From c531b514529719cd5210b8a9efc86b90bce9515c Mon Sep 17 00:00:00 2001 From: Konstantin Gredeskoul Date: Thu, 5 Jan 2017 16:06:29 -0800 Subject: [PATCH 05/16] Add redis-server to Travis --- .travis.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.travis.yml b/.travis.yml index db299c2..a83fa91 100644 --- a/.travis.yml +++ b/.travis.yml @@ -4,6 +4,8 @@ rvm: - 2.3.3 - 2.4.0 script: "bundle exec rspec" +services: + - redis-server notifications: email: recipients: From a679035aff938a7b960db358574537a57c8ec355 Mon Sep 17 00:00:00 2001 From: Konstantin Gredeskoul Date: Mon, 9 Jan 2017 15:48:01 -0800 Subject: [PATCH 06/16] Fix rspecs to work with real redis * refactored Action class-level methods * got rid of a horrendous class level variable * derive default action scope from the class name * flush redis db before each test. * clean up gemspec * bump version to 0.3.0 --- lib/pause.rb | 6 +- lib/pause/action.rb | 183 +++++++++++++++++++------------------ lib/pause/analyzer.rb | 2 +- lib/pause/configuration.rb | 2 +- lib/pause/redis/adapter.rb | 23 +++-- lib/pause/version.rb | 2 +- pause.gemspec | 8 +- spec/pause/action_spec.rb | 25 ++--- spec/spec_helper.rb | 4 + 9 files changed, 135 insertions(+), 120 deletions(-) diff --git a/lib/pause.rb b/lib/pause.rb index a913b7f..f509cfc 100644 --- a/lib/pause.rb +++ b/lib/pause.rb @@ -37,11 +37,11 @@ def adapter=(adapter) end def configure(&block) - @configuration = Pause::Configuration.new.configure(&block) + @configuration ||= Pause::Configuration.new.configure(&block) end - def config - @configuration + def config(&block) + configure(&block) end end end diff --git a/lib/pause/action.rb b/lib/pause/action.rb index 379181d..6405934 100644 --- a/lib/pause/action.rb +++ b/lib/pause/action.rb @@ -3,60 +3,111 @@ class Action attr_accessor :identifier def initialize(identifier) - @identifier = identifier + @identifier = identifier self.class.checks = [] unless self.class.instance_variable_get(:@checks) end - # Action subclasses should define their scope as follows - # - # class MyAction < Pause::Action - # scope "my:scope" - # end - # def scope - raise 'Should implement scope. (Ex: ipn:follow)' - end - - def self.scope(scope_identifier = nil) - class_variable_set(:@@class_scope, scope_identifier) - define_method(:scope) { scope_identifier } - end - - # Action subclasses should define their checks as follows - # - # period_seconds - compare all activity by an identifier within the time period - # max_allowed - if the number of actions by an identifier exceeds max_allowed for the time period marked - # by period_seconds, it is no longer ok. - # block_ttl - time to mark identifier as not ok - # - # class MyAction < Pause::Action - # check period_seconds: 60, max_allowed: 100, block_ttl: 3600 - # check period_seconds: 1800, max_allowed: 2000, block_ttl: 3600 - # end - # - def self.check(*args) - @checks ||= [] - period_seconds, max_allowed, block_ttl = - if args.first.is_a?(Hash) - [args.first[:period_seconds], args.first[:max_allowed], args.first[:block_ttl]] - else - args + self.class.scope + end + + class << self + attr_accessor :checks + + def inherited(klass) + klass.instance_eval do + # Action subclasses should define their scope as follows + # + # class MyAction < Pause::Action + # scope "my:scope" + # end + # + @scope = klass.name.downcase.gsub(/::/, '.') + class << self + + # @param [String] args + def scope(*args) + @scope = args.first if args && args.size == 1 + @scope + end + end end - @checks << Pause::PeriodCheck.new(period_seconds, max_allowed, block_ttl) - end + end + + + # Actions can be globally disabled or re-enabled in a persistent + # way. + # + # MyAction.disable + # MyAction.enabled? => false + # MyAction.disabled? => true + # + # MyAction.enable + # MyAction.enabled? => true + # MyAction.disabled? => false + # + def enable + adapter.enable(scope) + end + + def disable + adapter.disable(scope) + end + + def enabled? + adapter.enabled?(scope) + end + + def disabled? + !enabled? + end + + + # Action subclasses should define their checks as follows + # + # period_seconds - compare all activity by an identifier within the time period + # max_allowed - if the number of actions by an identifier exceeds max_allowed for the time period marked + # by period_seconds, it is no longer ok. + # block_ttl - time to mark identifier as not ok + # + # class MyAction < Pause::Action + # check period_seconds: 60, max_allowed: 100, block_ttl: 3600 + # check period_seconds: 1800, max_allowed: 2000, block_ttl: 3600 + # end + # + def check(*args) + self.checks ||= [] + params = + if args.first.is_a?(Hash) + [args.first[:period_seconds], args.first[:max_allowed], args.first[:block_ttl]] + else + args + end + self.checks << Pause::PeriodCheck.new(*params) + end + + def tracked_identifiers + adapter.all_keys(scope) + end + + def rate_limited_identifiers + adapter.rate_limited_keys(scope) + end + + def unblock_all + adapter.delete_rate_limited_keys(scope) + end + + def adapter + Pause.adapter + end - def self.checks - @checks end def checks self.class.checks end - def self.checks=(period_checks) - @checks = period_checks - end - def block_for(ttl) adapter.rate_limit!(scope, identifier, ttl) end @@ -66,7 +117,7 @@ def increment!(count = 1, timestamp = Time.now.to_i) end def rate_limited? - ! ok? + !ok? end def ok? @@ -80,61 +131,15 @@ def analyze Pause.analyzer.check(self) end - def self.tracked_identifiers - adapter.all_keys(self.class_scope) - end - - def self.rate_limited_identifiers - adapter.rate_limited_keys(self.class_scope) - end - - def self.unblock_all - adapter.delete_rate_limited_keys(self.class_scope) - end - def unblock adapter.delete_rate_limited_key(scope, identifier) end - # Actions can be globally disabled or re-enabled in a persistent - # way. - # - # MyAction.disable - # MyAction.enabled? => false - # MyAction.disabled? => true - # - # MyAction.enable - # MyAction.enabled? => true - # MyAction.disabled? => false - # - def self.enable - adapter.enable(class_scope) - end - - def self.disable - adapter.disable(class_scope) - end - - def self.enabled? - adapter.enabled?(class_scope) - end - - def self.disabled? - ! enabled? - end - private - def self.adapter - Pause.adapter - end - def adapter self.class.adapter end - def self.class_scope - class_variable_get:@@class_scope if class_variable_defined?(:@@class_scope) - end end end diff --git a/lib/pause/analyzer.rb b/lib/pause/analyzer.rb index dfb14ca..8e8529b 100644 --- a/lib/pause/analyzer.rb +++ b/lib/pause/analyzer.rb @@ -13,7 +13,7 @@ class Analyzer def check(action) return false if adapter.rate_limited?(action.scope, action.identifier) timestamp = period_marker(Pause.config.resolution, Time.now.to_i) - set = adapter.key_history(action.scope, action.identifier) + set = adapter.key_history(action.scope, action.identifier) action.checks.each do |period_check| start_time = timestamp - period_check.period_seconds set.reverse.inject(0) do |sum, element| diff --git a/lib/pause/configuration.rb b/lib/pause/configuration.rb index 48c07c9..42a34ac 100644 --- a/lib/pause/configuration.rb +++ b/lib/pause/configuration.rb @@ -3,7 +3,7 @@ class Configuration attr_writer :redis_host, :redis_port, :redis_db, :resolution, :history, :sharded def configure - yield self + yield self if block_given? self end diff --git a/lib/pause/redis/adapter.rb b/lib/pause/redis/adapter.rb index f423f3b..35b5bb2 100644 --- a/lib/pause/redis/adapter.rb +++ b/lib/pause/redis/adapter.rb @@ -5,6 +5,17 @@ module Redis # This class encapsulates Redis operations used by Pause class Adapter + class << self + def redis + @redis_conn ||= ::Redis.new(redis_connection_opts) + end + + def redis_connection_opts + { host: Pause.config.redis_host, + port: Pause.config.redis_port, + db: Pause.config.redis_db } + end + end include Pause::Helper::Timing attr_accessor :resolution, :time_blocks_to_keep, :history @@ -96,20 +107,12 @@ def expire_block_list(scope) redis.zremrangebyscore rate_limited_list(scope), '-inf', Time.now.to_i end - protected + private def redis - @redis_conn ||= ::Redis.new(redis_connection_opts) + self.class.redis end - def redis_connection_opts - { host: Pause.config.redis_host, - port: Pause.config.redis_port, - db: Pause.config.redis_db } - end - - private - def delete_tracking_keys(scope, ids) increment_keys = ids.map { |key| tracked_key(scope, key) } redis.del(increment_keys) diff --git a/lib/pause/version.rb b/lib/pause/version.rb index ff83373..27acadd 100644 --- a/lib/pause/version.rb +++ b/lib/pause/version.rb @@ -1,3 +1,3 @@ module Pause - VERSION = '0.2.1' + VERSION = '0.3.0' end diff --git a/pause.gemspec b/pause.gemspec index 64a3733..1459240 100644 --- a/pause.gemspec +++ b/pause.gemspec @@ -4,13 +4,13 @@ $LOAD_PATH.unshift(lib) unless $LOAD_PATH.include?(lib) require 'pause/version' Gem::Specification.new do |gem| - gem.name = "pause" + gem.name = 'pause' gem.version = Pause::VERSION - gem.authors = ["Atasay Gokkaya", "Paul Henry", "Eric Saxby", "Konstantin Gredeskoul"] - gem.email = %w(atasay@wanelo.com paul@wanelo.com sax@wanelo.com kig@wanelo.com) + gem.authors = ['Atasay Gokkaya', 'Paul Henry', 'Eric Saxby', 'Konstantin Gredeskoul'] + gem.email = %w(atasay@wanelo.com paul@wanelo.com sax@ericsaxby.com kigster@gmail.com) gem.description = %q(Real time rate limiting for multi-process ruby environments based on Redis) gem.summary = %q(RReal time rate limiting for multi-process ruby environments based on Redis) - gem.homepage = "https://github.com/wanelo/pause" + gem.homepage = 'https://github.com/wanelo/pause' gem.files = `git ls-files`.split($/) gem.executables = gem.files.grep(%r{^bin/}).map{ |f| File.basename(f) } diff --git a/spec/pause/action_spec.rb b/spec/pause/action_spec.rb index 8ec682c..72bb578 100644 --- a/spec/pause/action_spec.rb +++ b/spec/pause/action_spec.rb @@ -81,13 +81,14 @@ class MyNotification < Pause::Action describe '#analyze' do context 'action should not be rate limited' do it 'returns nil' do + expect(adapter.rate_limited?(action.scope, action.identifier)).to be false expect(action.analyze).to be nil end end context 'action should be rate limited' do it 'returns a RateLimitedEvent object' do - time = Time.now + time = Time.now rate_limit = nil Timecop.freeze time do @@ -188,12 +189,13 @@ class ActionWithHashChecks < Pause::Action it 'should define a period check on new instances' do expect(ActionWithCheck.new('id').checks).to eq([ - Pause::PeriodCheck.new(100, 150, 200) - ]) + Pause::PeriodCheck.new(100, 150, 200) + ]) end it 'should define a period check on new instances' do - expect(ActionWithMultipleChecks.new('id').checks).to eq([ + expect(ActionWithMultipleChecks.new('id').checks).to \ + eq([ Pause::PeriodCheck.new(100, 150, 200), Pause::PeriodCheck.new(200, 150, 200), Pause::PeriodCheck.new(300, 150, 200) @@ -202,20 +204,21 @@ class ActionWithHashChecks < Pause::Action it 'should accept hash arguments' do expect(ActionWithHashChecks.new('id').checks).to eq([ - Pause::PeriodCheck.new(50, 100, 60) - ]) + Pause::PeriodCheck.new(50, 100, 60) + ]) end end describe Pause::Action, '.scope' do - class UndefinedScopeAction < Pause::Action + module MyApp + class NoScope < ::Pause::Action + end end + it 'should raise if scope is not defined' do - expect { - UndefinedScopeAction.new('1.2.3.4').scope - }.to raise_error('Should implement scope. (Ex: ipn:follow)') + expect(MyApp::NoScope.new('1.2.3.4').scope).to eq 'myapp.noscope' end class DefinedScopeAction < Pause::Action @@ -236,7 +239,7 @@ class BlockedAction < Pause::Action before do Pause.configure do |c| c.resolution = 10 - c.history = 10 + c.history = 10 end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 56a5ec4..0fe92e3 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -10,6 +10,7 @@ require 'bundler/setup' if File.exists?(ENV['BUNDLE_GEMFILE']) require 'pause' require 'pry' +require 'pause/redis/adapter' RSpec.configure do |config| config.run_all_when_everything_filtered = true @@ -20,4 +21,7 @@ # the seed, which is printed after each run. # --seed 1234 config.order = 'random' + config.before(:example) do + Pause::Redis::Adapter.redis.flushdb + end end From 57c893e35447e3404a0941782aee65be14685f80 Mon Sep 17 00:00:00 2001 From: Konstantin Gredeskoul Date: Mon, 9 Jan 2017 23:33:43 -0800 Subject: [PATCH 07/16] Enabling both fakeredis and real redis tests --- .travis.yml | 3 ++- README.md | 20 ++++++++++++++++++++ Rakefile | 15 ++++++++++++++- pause.gemspec | 6 ++++-- spec/spec_helper.rb | 13 ++++++++++--- 5 files changed, 50 insertions(+), 7 deletions(-) diff --git a/.travis.yml b/.travis.yml index a83fa91..4f7e843 100644 --- a/.travis.yml +++ b/.travis.yml @@ -3,12 +3,13 @@ rvm: - 1.9.3 - 2.3.3 - 2.4.0 -script: "bundle exec rspec" +script: "bundle exec rake" services: - redis-server notifications: email: recipients: + - dev-info@wanelo.com - kigster@gmail.com on_success: never on_failure: always diff --git a/README.md b/README.md index 59e0f7c..b31b8b7 100644 --- a/README.md +++ b/README.md @@ -222,6 +222,26 @@ tracked identifiers. The action block list is implemented as a sorted set, so it should still be usable when sharding. +## Testing + +By default, `fakeredis` gem is used to emulate Redis in development. However, the same test-suite should be able to run against a real redis — however, be aware that it will flush the current db during spec run. In order to run specs against real redis, make sure you have Redis running locally on the default port, and that you are able to connect to it using `redis-cli`. + +Please note that Travis suite, as well as the default rake task, run both. + +### Unit Testing with Fakeredis + +Fakeredis is the default, and is also run whenever `bundle exec rspec` is executed, or `rake spec` task invoked. + +```bash +bundle exec rake spec:unit +``` + +### Integration Testing with Redis + +```bash +bundle exec rake spec:integration +``` + ## Contributing Want to make it better? Cool. Here's how: diff --git a/Rakefile b/Rakefile index 8754e40..68cc91e 100644 --- a/Rakefile +++ b/Rakefile @@ -3,4 +3,17 @@ require 'rspec/core/rake_task' RSpec::Core::RakeTask.new(:spec) -task :default => :spec +task :default => [ 'spec:unit', 'spec:integration' ] + +namespace :spec do + desc 'Run specs using fakeredis' + task :unit do + ENV['PAUSE_REAL_REDIS'] = nil + Rake::Task["spec"].execute + end + desc 'Run specs against a local Redis server' + task :integration do + ENV['PAUSE_REAL_REDIS'] = 'true' + Rake::Task["spec"].execute + end +end diff --git a/pause.gemspec b/pause.gemspec index 1459240..c5d062c 100644 --- a/pause.gemspec +++ b/pause.gemspec @@ -8,8 +8,8 @@ Gem::Specification.new do |gem| gem.version = Pause::VERSION gem.authors = ['Atasay Gokkaya', 'Paul Henry', 'Eric Saxby', 'Konstantin Gredeskoul'] gem.email = %w(atasay@wanelo.com paul@wanelo.com sax@ericsaxby.com kigster@gmail.com) - gem.description = %q(Real time rate limiting for multi-process ruby environments based on Redis) - gem.summary = %q(RReal time rate limiting for multi-process ruby environments based on Redis) + gem.summary = %q(Fast and efficient real time rate limiting library for multi-process ruby environments based on Redis) + gem.description = %q(This gem provides flexible and easy to use interface to define rate checks, register events as they come, and verify if the rate limit is reached. Multiple checks for the same metric are easily supported.) gem.homepage = 'https://github.com/wanelo/pause' gem.files = `git ls-files`.split($/) @@ -18,8 +18,10 @@ Gem::Specification.new do |gem| gem.require_paths = ["lib"] gem.add_dependency 'redis' + gem.add_dependency 'hiredis' gem.add_development_dependency 'rspec' + gem.add_development_dependency 'fakeredis' gem.add_development_dependency 'guard-rspec' gem.add_development_dependency 'pry-nav' gem.add_development_dependency 'rb-fsevent' diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 0fe92e3..46b4662 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -10,7 +10,12 @@ require 'bundler/setup' if File.exists?(ENV['BUNDLE_GEMFILE']) require 'pause' require 'pry' -require 'pause/redis/adapter' + +if ENV['PAUSE_REAL_REDIS'] + require 'pause/redis/adapter' +else + require 'fakeredis/rspec' +end RSpec.configure do |config| config.run_all_when_everything_filtered = true @@ -21,7 +26,9 @@ # the seed, which is printed after each run. # --seed 1234 config.order = 'random' - config.before(:example) do - Pause::Redis::Adapter.redis.flushdb + if ENV['PAUSE_REAL_REDIS'] + config.before(:example) do + Pause::Redis::Adapter.redis.flushdb + end end end From b73af92d5158614a0835a6a86a9cd2012da39d57 Mon Sep 17 00:00:00 2001 From: Konstantin Gredeskoul Date: Tue, 10 Jan 2017 00:01:32 -0800 Subject: [PATCH 08/16] Add a message about redis-server --- spec/spec_helper.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 46b4662..524f515 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -13,6 +13,7 @@ if ENV['PAUSE_REAL_REDIS'] require 'pause/redis/adapter' + puts "\n==> Using real Redis-server at #{Pause::Redis::Adapter.redis.inspect}\n\n" else require 'fakeredis/rspec' end From ca707b2196414e09af14725980f6d36f9e2db257 Mon Sep 17 00:00:00 2001 From: Konstantin Gredeskoul Date: Tue, 10 Jan 2017 13:18:40 -0800 Subject: [PATCH 09/16] Remove 1.9.3 --- .travis.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 4f7e843..f68970f 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,6 +1,5 @@ language: ruby rvm: - - 1.9.3 - 2.3.3 - 2.4.0 script: "bundle exec rake" From d314e577bfccee7644f8aecaf98b596f1306c9d1 Mon Sep 17 00:00:00 2001 From: Konstantin Gredeskoul Date: Tue, 10 Jan 2017 15:32:44 -0800 Subject: [PATCH 10/16] More defensive zrem() command; more tests. The issue I faced is a race condition when two processes increment at the same time; first one performs zrem on two old elements, second does not have any elements to zrem (even though zcard > value) was satisified. This race condition must be simply coded around. --- lib/pause/action.rb | 2 +- lib/pause/redis/adapter.rb | 15 +-- spec/pause/action_spec.rb | 177 ++++++++++++++++--------------- spec/pause/redis/adapter_spec.rb | 30 ++++-- 4 files changed, 125 insertions(+), 99 deletions(-) diff --git a/lib/pause/action.rb b/lib/pause/action.rb index 6405934..2a59a2d 100644 --- a/lib/pause/action.rb +++ b/lib/pause/action.rb @@ -4,7 +4,7 @@ class Action def initialize(identifier) @identifier = identifier - self.class.checks = [] unless self.class.instance_variable_get(:@checks) + self.class.checks ||= [] end def scope diff --git a/lib/pause/redis/adapter.rb b/lib/pause/redis/adapter.rb index 35b5bb2..3290c81 100644 --- a/lib/pause/redis/adapter.rb +++ b/lib/pause/redis/adapter.rb @@ -40,11 +40,7 @@ def increment(scope, identifier, timestamp, count = 1) redis.expire k, history end - if redis.zcard(k) > time_blocks_to_keep - list = extract_set_elements(k) - to_remove = list.slice(0, (list.size - time_blocks_to_keep)) - redis.zrem(k, to_remove.map(&:ts)) - end + truncate_set_for(k) end def key_history(scope, identifier) @@ -113,12 +109,19 @@ def redis self.class.redis end + def truncate_set_for(k) + if redis.zcard(k) > time_blocks_to_keep + list = extract_set_elements(k) + to_remove = list.slice(0, (list.size - time_blocks_to_keep)).map(&:ts) + redis.zrem(k, to_remove) if k && to_remove && to_remove.size > 0 + end + end + def delete_tracking_keys(scope, ids) increment_keys = ids.map { |key| tracked_key(scope, key) } redis.del(increment_keys) end - def tracked_scope(scope) ['i', scope].join(':') end diff --git a/spec/pause/action_spec.rb b/spec/pause/action_spec.rb index 72bb578..298efdc 100644 --- a/spec/pause/action_spec.rb +++ b/spec/pause/action_spec.rb @@ -22,87 +22,119 @@ class MyNotification < Pause::Action allow(Pause).to receive(:adapter).and_return(adapter) end - let(:action) { MyNotification.new('1237612') } - let(:other_action) { MyNotification.new('1237613') } - - describe '#increment!' do - it 'should increment' do - time = Time.now - Timecop.freeze time do - expect(Pause.adapter).to receive(:increment).with(action.scope, '1237612', time.to_i, 1) - action.increment! + let(:identifier) { '11112222' } + let(:action) { MyNotification.new(identifier) } + + let(:other_identifier) { '8798734' } + let(:other_action) { MyNotification.new(other_identifier) } + + RSpec.shared_examples 'an action' do + describe '#increment!' do + it 'should increment' do + time = Time.now + Timecop.freeze time do + expect(Pause.adapter).to receive(:increment).with(action.scope, identifier, time.to_i, 1) + action.increment! + end end end - end - describe '#ok?' do - it 'should successfully return if the action is blocked or not' do - time = Time.now - Timecop.freeze time do - 4.times do + describe '#ok?' do + it 'should successfully return if the action is blocked or not' do + time = Time.now + Timecop.freeze time do + 4.times do + action.increment! + expect(action.ok?).to be true + end action.increment! - expect(action.ok?).to be true + expect(action.ok?).to be false end - action.increment! - expect(action.ok?).to be false end - end - it 'should successfully consider different period checks' do - time = Time.parse('Sept 22, 11:34:00') + it 'should successfully consider different period checks' do + time = Time.parse('Sept 22, 11:34:00') - Timecop.freeze time - 30 do - action.increment! 4 - expect(action.ok?).to be true - end + Timecop.freeze time - 30 do + action.increment! 4 + expect(action.ok?).to be true + end - Timecop.freeze time do - action.increment! 2 - expect(action.ok?).to be true + Timecop.freeze time do + action.increment! 2 + expect(action.ok?).to be true + end + + Timecop.freeze time do + action.increment! 1 + expect(action.ok?).to be false + end end - Timecop.freeze time do - action.increment! 1 + it 'should return false and silently fail if redis is not available' do + allow(Pause::Logger).to receive(:fatal) + allow_any_instance_of(Redis).to receive(:zrange).and_raise Redis::CannotConnectError + time = period_marker(resolution, Time.now.to_i) + + action.increment! 4, time - 25 + expect(action.ok?).to be false end end - it 'should return false and silently fail if redis is not available' do - allow(Pause::Logger).to receive(:fatal) - allow_any_instance_of(Redis).to receive(:zrange).and_raise Redis::CannotConnectError - time = period_marker(resolution, Time.now.to_i) + describe '#analyze' do + context 'action should not be rate limited' do + it 'returns nil' do + expect(adapter.rate_limited?(action.scope, action.identifier)).to be false + expect(action.analyze).to be nil + end + end - action.increment! 4, time - 25 + context 'action should be rate limited' do + it 'returns a RateLimitedEvent object' do + time = Time.now + rate_limit = nil - expect(action.ok?).to be false - end - end + Timecop.freeze time do + 7.times { action.increment! } + rate_limit = action.analyze + end + + expected_rate_limit = Pause::RateLimitedEvent.new(action, action.checks[0], 7, time.to_i) - describe '#analyze' do - context 'action should not be rate limited' do - it 'returns nil' do - expect(adapter.rate_limited?(action.scope, action.identifier)).to be false - expect(action.analyze).to be nil + expect(rate_limit).to be_a(Pause::RateLimitedEvent) + expect(rate_limit.identifier).to eq(expected_rate_limit.identifier) + expect(rate_limit.sum).to eq(expected_rate_limit.sum) + expect(rate_limit.period_check).to eq(expected_rate_limit.period_check) + expect(rate_limit.timestamp).to eq(expected_rate_limit.timestamp) + end end end - context 'action should be rate limited' do - it 'returns a RateLimitedEvent object' do - time = Time.now - rate_limit = nil - - Timecop.freeze time do - 7.times { action.increment! } - rate_limit = action.analyze - end + describe '#unblock' do + it 'unblocks the specified id' do + 10.times { action.increment! } + expect(action.ok?).to be false + action.unblock + expect(action.ok?).to be true + end + end - expected_rate_limit = Pause::RateLimitedEvent.new(action, action.checks[0], 7, time.to_i) + describe '#block_for' do + it 'blocks the IP for N seconds' do + expect(adapter).to receive(:rate_limit!).with(action.scope, action.identifier, 10).and_call_original + action.block_for(10) + expect(action.ok?).to be false + end + end + end - expect(rate_limit).to be_a(Pause::RateLimitedEvent) - expect(rate_limit.identifier).to eq(expected_rate_limit.identifier) - expect(rate_limit.sum).to eq(expected_rate_limit.sum) - expect(rate_limit.period_check).to eq(expected_rate_limit.period_check) - expect(rate_limit.timestamp).to eq(expected_rate_limit.timestamp) + context 'actions under test' do + ['123456', 'hello', 0, 999999].each do |id| + let(:identifier) { id } + let(:action) { MyNotification.new(identifier) } + describe "action with identifier #{id}" do + it_behaves_like 'an action' end end end @@ -151,25 +183,6 @@ class MyNotification < Pause::Action end end - describe '#unblock' do - it 'unblocks the specified id' do - 10.times { action.increment! } - - expect(action.ok?).to be false - - action.unblock - - expect(action.ok?).to be true - end - end - - describe '#block_for' do - it 'blocks the IP for N seconds' do - expect(adapter).to receive(:rate_limit!).with(action.scope, action.identifier, 10).and_call_original - action.block_for(10) - expect(action.ok?).to be false - end - end end describe Pause::Action, '.check' do @@ -196,10 +209,10 @@ class ActionWithHashChecks < Pause::Action it 'should define a period check on new instances' do expect(ActionWithMultipleChecks.new('id').checks).to \ eq([ - Pause::PeriodCheck.new(100, 150, 200), - Pause::PeriodCheck.new(200, 150, 200), - Pause::PeriodCheck.new(300, 150, 200) - ]) + Pause::PeriodCheck.new(100, 150, 200), + Pause::PeriodCheck.new(200, 150, 200), + Pause::PeriodCheck.new(300, 150, 200) + ]) end it 'should accept hash arguments' do @@ -207,7 +220,6 @@ class ActionWithHashChecks < Pause::Action Pause::PeriodCheck.new(50, 100, 60) ]) end - end describe Pause::Action, '.scope' do @@ -216,7 +228,6 @@ class NoScope < ::Pause::Action end end - it 'should raise if scope is not defined' do expect(MyApp::NoScope.new('1.2.3.4').scope).to eq 'myapp.noscope' end diff --git a/spec/pause/redis/adapter_spec.rb b/spec/pause/redis/adapter_spec.rb index 594dca2..86c4231 100644 --- a/spec/pause/redis/adapter_spec.rb +++ b/spec/pause/redis/adapter_spec.rb @@ -31,19 +31,31 @@ expect(set[0].size).to eql(2) end - it 'should remove old key from a redis set' do - time = Time.now - expect(redis_conn).to receive(:zrem).with(tracked_key, [adapter.period_marker(resolution, time)]) - - adapter.time_blocks_to_keep = 1 - Timecop.freeze time do - adapter.increment(scope, identifier, Time.now.to_i) + RSpec.shared_examples 'removes old elements' do + let(:time) { Time.now } + before do + to_delete.times do |t| + expect(redis_conn).to receive(:zrem).with(tracked_key, [adapter.period_marker(resolution, time + t)]).once + end + adapter.time_blocks_to_keep = 1 end - Timecop.freeze time + (adapter.resolution + 1) do - adapter.increment(scope, identifier, Time.now.to_i) + it 'should remove old elements' do + Timecop.freeze time do + adapter.increment(scope, identifier, Time.now.to_i) + end + to_delete.times do |t| + Timecop.freeze time + (adapter.resolution + t + 1) do + adapter.increment(scope, identifier, Time.now.to_i) + end + end end end + context 'removing two elements' do + let(:to_delete) { 2 } + it_behaves_like 'removes old elements' + end + it 'sets expiry on key' do expect(redis_conn).to receive(:expire).with(tracked_key, history) adapter.increment(scope, identifier, Time.now.to_i) From 2c2fdd2d54beac3e7507b32c98d508a257223113 Mon Sep 17 00:00:00 2001 From: Konstantin Gredeskoul Date: Wed, 28 Mar 2018 20:15:58 -0700 Subject: [PATCH 11/16] Introducing block helpers to simplify the API. - together with tests - updating gems, removing unused gems - migrating gem to /kigster for maintenance - bump version to 0.4.0 --- .gitignore | 2 + .travis.yml | 6 +- README.md | 127 ++++++++++++++++++++++++++----- Rakefile | 36 ++++++++- lib/pause/action.rb | 32 ++++++-- lib/pause/analyzer.rb | 4 +- lib/pause/version.rb | 2 +- pause.gemspec | 12 +-- spec/pause/action_spec.rb | 50 ++++++++++++ spec/pause/redis/adapter_spec.rb | 18 ++--- spec/spec_helper.rb | 16 ++-- 11 files changed, 239 insertions(+), 66 deletions(-) diff --git a/.gitignore b/.gitignore index d6b49a9..ee14490 100644 --- a/.gitignore +++ b/.gitignore @@ -17,3 +17,5 @@ test/version_tmp tmp .idea .DS_Store +.ruby-version +.spec diff --git a/.travis.yml b/.travis.yml index f68970f..9ef53a0 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,14 +1,14 @@ language: ruby rvm: - 2.3.3 - - 2.4.0 -script: "bundle exec rake" + - 2.4.3 + - 2.5.0 +script: "bundle exec rake || bundle exec rspec --only-failures" services: - redis-server notifications: email: recipients: - - dev-info@wanelo.com - kigster@gmail.com on_success: never on_failure: always diff --git a/README.md b/README.md index b31b8b7..f5540cc 100644 --- a/README.md +++ b/README.md @@ -1,17 +1,105 @@ -Pause -====== +# Pause [![Gem Version](https://badge.fury.io/rb/pause.png)](http://badge.fury.io/rb/pause) -[![Build status](https://secure.travis-ci.org/wanelo/pause.png)](http://travis-ci.org/wanelo/pause) +[![Build Status](https://travis-ci.org/kigster/pause.svg?branch=master)](https://travis-ci.org/kigster/pause) -Pause is a flexible Redis-backed rate-limiting client. Use it to track events, with +## In a Nutshell + +**Pause** is a fast and very flexible Redis-backed rate-limiter. You can use it to track events, with rules around how often they are allowed to occur within configured time checks. -Because Pause is Redis-based, multiple ruby processes (even distributed across multiple servers) can track and report -events together, and then query whether a particular identifier should be rate limited or not. +Sample applications include: + + * throttling notifications sent to a user as to not overwhelm them with too much frequency, + * IP-based blocking based on HTTP request volume (see the related gem [spanx](https://github.com/wanelo/spanx)) that uses Pause, + * ensuring you do not exceed API rate limits when calling external web APIs. + * etc. + +Pause currently does not offer a CLI client, and can only be used from within a Ruby application. + +Additionally: + + * Pause is pure-ruby gem and does not depend on Rails or Rack + * Pause can be used across multiple ruby processes, since it uses a distributed Redis backend + * Pause is currently in use by a web application receiving 6K-10K web requests per second + * Pause will work with a horizontally sharded multi-Redis-backend by using Twitter's [Twemproxy](https://github.com/twitter/twemproxy). This way, millions of concurrent users can be handled with ease. + +### Quick Start + +This section is meant to give you a rapid introduction, so that you can start using Pause immediately. + +Our use case: we want to rate limit notifications sent to users, identified by their `user_id`, to: + + * no more than 1 in any 2-hour period + * no more than 3 per day + * no more than 7 per week + +Here is how we could set this up using Pause: + +#### Configuration + +We need to setup Pause with a Redis instance. Here is how we do it: + +```ruby +require 'pause' + +# First, lets point Pause to a Redis instance +Pause.configure do |config| + # Redis connection parameters + config.redis_host = '127.0.0.1' + config.redis_port = 6379 + config.redis_db = 1 + + # aggregate all events into 10 minute blocks. + # Larger blocks require less RAM and CPU, smaller blocks are more + # computationally expensive. + config.resolution = 600 + + # discard all events older than 1 day + config.history = 86400 +end +``` + +#### Define Rate Limited "Action" + +Next we must define the rate limited action based on the specification above. This is how easy it is: + +```ruby +module MyApp + class UserNotificationLimiter < ::Pause::Action + # this is a redis key namespace added to all data in this action + scope 'un' + check period_seconds: 120, max_allowed: 1 + check period_seconds: 86400, max_allowed: 3 + check period_seconds: 7 * 86400, max_allowed: 7 + end +end +``` + +#### Perform operation, but only if the user is not rate-limited + +Now we simply instantiate this limiter by passing user ID (any unique identifier works). We can then ask the limiter, `ok?` or `rate_limited?`, or we can use two convenient methods that only execute enclosed block if the described condition is satisfied: + +```ruby +class NotificationsWorker + def perform(user_id) + limiter = MyApp::UserNotificationLimiter.new(user_id) + + limiter.unless_rate_limited do + user = User.find(user_id) + user.send_push_notification! + end + + # You can also do something in case the user is rate limited: + limiter.if_rate_limited do |rate_limit_event| + Rails.logger.info("user #{user.id} has exceeded rate limit: #{rate_limit_event}") + end + end +end +``` + +That's it! Using these two methods you can pretty much ensure that your rate limits are always in check. -Sample applications include IP-based blocking based on HTTP request volume (see related gem "spanx"), -throttling push notifications as to not overwhelm the user with too much frequency, etc. ## Installation @@ -86,47 +174,44 @@ In other words, if your shortest check is 1 minute, you could set resolution to require 'pause' class FollowAction < Pause::Action - scope "f" + scope 'fa' # keep those short check period_seconds: 60, max_allowed: 100, block_ttl: 3600 check period_seconds: 1800, max_allowed: 2000, block_ttl: 3600 end ``` -When an event occurs, you increment an instance of your action, optionally with a timestamp and count. This saves -data into a redis store, so it can be checked later by other processes. Timestamps should be in unix epoch format. +When an event occurs, you increment an instance of your action, optionally with a timestamp and count. This saves data into a redis store, so it can be checked later by other processes. Timestamps should be in unix epoch format. + +In the example at the top of the README you saw how we used `#unless_rate_limited` and `#if_rate_limited` methods. These are the recommended API methods, but if you must get a finer-grained control over the actions, you can also use methods such as `#ok?`, `#rate_limited?`, `#increment!` to do manually what the block methods do already. Below is an example of this "manual" implementation: ```ruby class FollowsController < ApplicationController def create action = FollowAction.new(user.id) if action.ok? - # do stuff! - # and track it... + user.follow! + # and don't forget to track the "success" action.increment! - else - # action is rate limited, either skip - # or show error, depending on the context. end end end class OtherController < ApplicationController def index - action = OtherAction.new(params[:thing]) + action = OtherAction.new(params[:thing])d unless action.rate_limited? # perform business logic - .... - # track it + # but in this action.increment!(params[:count].to_i, Time.now.to_i) end end end ``` -If more data is needed about why the action is blocked, the `analyze` can be called +If more data is needed about why the action is blocked, the `analyze` can be called: ```ruby -action = NotifyViaEmailAction.new("thing") +action = NotifyViaEmailAction.new(:thing) while true action.increment! diff --git a/Rakefile b/Rakefile index 68cc91e..c80f41e 100644 --- a/Rakefile +++ b/Rakefile @@ -1,19 +1,47 @@ -require "bundler/gem_tasks" +require 'bundler/gem_tasks' require 'rspec/core/rake_task' +require 'yard' RSpec::Core::RakeTask.new(:spec) -task :default => [ 'spec:unit', 'spec:integration' ] +task :default => %w(spec:unit spec:integration) namespace :spec do desc 'Run specs using fakeredis' task :unit do ENV['PAUSE_REAL_REDIS'] = nil - Rake::Task["spec"].execute + Rake::Task['spec'].execute end desc 'Run specs against a local Redis server' task :integration do ENV['PAUSE_REAL_REDIS'] = 'true' - Rake::Task["spec"].execute + Rake::Task['spec'].execute end end + +def shell(*args) + puts "running: #{args.join(' ')}" + system(args.join(' ')) +end + +task :clean do + shell('rm -rf pkg/ tmp/ coverage/ doc/ ' ) +end + +task :gem => [:build] do + shell('gem install pkg/*') +end + +task :permissions => [ :clean ] do + shell('chmod -v o+r,g+r * */* */*/* */*/*/* */*/*/*/* */*/*/*/*/*') + shell("find . -type d -exec chmod o+x,g+x {} \\;") +end + +task :build => :permissions + +YARD::Rake::YardocTask.new(:doc) do |t| + t.files = %w(lib/**/*.rb exe/*.rb - README.md LICENSE.txt) + t.options.unshift('--title','"Pause - Redis-backed Rate Limiter"') + t.after = ->() { exec('open doc/index.html') } +end + diff --git a/lib/pause/action.rb b/lib/pause/action.rb index 2a59a2d..3750cf1 100644 --- a/lib/pause/action.rb +++ b/lib/pause/action.rb @@ -34,7 +34,6 @@ def scope(*args) end end - # Actions can be globally disabled or re-enabled in a persistent # way. # @@ -62,7 +61,6 @@ def disabled? !enabled? end - # Action subclasses should define their checks as follows # # period_seconds - compare all activity by an identifier within the time period @@ -75,14 +73,18 @@ def disabled? # check period_seconds: 1800, max_allowed: 2000, block_ttl: 3600 # end # - def check(*args) + def check(*args, **opts) self.checks ||= [] - params = - if args.first.is_a?(Hash) - [args.first[:period_seconds], args.first[:max_allowed], args.first[:block_ttl]] + + params = + if args.empty? + # if block_ttl is not provided, just default to the period + opts[:block_ttl] ||= opts[:period_seconds] + [opts[:period_seconds], opts[:max_allowed], opts[:block_ttl]] else args end + self.checks << Pause::PeriodCheck.new(*params) end @@ -101,7 +103,21 @@ def unblock_all def adapter Pause.adapter end + end + + def unless_rate_limited(count: 1, timestamp: Time.now.to_i, &_block) + check_result = analyze + if check_result.nil? + yield + increment!(count, timestamp) + else + check_result + end + end + def if_rate_limited(&_block) + check_result = analyze(recalculate: true) + yield(check_result) unless check_result.nil? end def checks @@ -127,8 +143,8 @@ def ok? false end - def analyze - Pause.analyzer.check(self) + def analyze(recalculate: false) + Pause.analyzer.check(self, recalculate: recalculate) end def unblock diff --git a/lib/pause/analyzer.rb b/lib/pause/analyzer.rb index 8e8529b..016b4f9 100644 --- a/lib/pause/analyzer.rb +++ b/lib/pause/analyzer.rb @@ -10,8 +10,8 @@ class Analyzer # @return [nil] everything is fine # @return [false] this action is already blocked # @return [Pause::RateLimitedEvent] the action was blocked as a result of this check - def check(action) - return false if adapter.rate_limited?(action.scope, action.identifier) + def check(action, recalculate: false) + return false if adapter.rate_limited?(action.scope, action.identifier) && !recalculate timestamp = period_marker(Pause.config.resolution, Time.now.to_i) set = adapter.key_history(action.scope, action.identifier) action.checks.each do |period_check| diff --git a/lib/pause/version.rb b/lib/pause/version.rb index 27acadd..06effb7 100644 --- a/lib/pause/version.rb +++ b/lib/pause/version.rb @@ -1,3 +1,3 @@ module Pause - VERSION = '0.3.0' + VERSION = '0.4.0' end diff --git a/pause.gemspec b/pause.gemspec index c5d062c..4f2c321 100644 --- a/pause.gemspec +++ b/pause.gemspec @@ -8,23 +8,23 @@ Gem::Specification.new do |gem| gem.version = Pause::VERSION gem.authors = ['Atasay Gokkaya', 'Paul Henry', 'Eric Saxby', 'Konstantin Gredeskoul'] gem.email = %w(atasay@wanelo.com paul@wanelo.com sax@ericsaxby.com kigster@gmail.com) - gem.summary = %q(Fast and efficient real time rate limiting library for multi-process ruby environments based on Redis) - gem.description = %q(This gem provides flexible and easy to use interface to define rate checks, register events as they come, and verify if the rate limit is reached. Multiple checks for the same metric are easily supported.) - gem.homepage = 'https://github.com/wanelo/pause' + gem.summary = %q(Fast, scalable, and flexible real time rate limiting library for distributed Ruby environments backed by Redis.) + gem.description = %q(This gem provides highly flexible and easy to use interface to define rate limit checks, register events as they come, and verify if the rate limit is reached. Multiple checks for the same metric are easily supported. This gem is used at very high scale on several popular web sites.) + gem.homepage = 'https://github.com/kigster/pause' gem.files = `git ls-files`.split($/) gem.executables = gem.files.grep(%r{^bin/}).map{ |f| File.basename(f) } gem.test_files = gem.files.grep(%r{^(test|spec|features)/}) - gem.require_paths = ["lib"] + gem.require_paths = ['lib'] gem.add_dependency 'redis' gem.add_dependency 'hiredis' + gem.add_development_dependency 'simplecov' + gem.add_development_dependency 'yard' gem.add_development_dependency 'rspec' gem.add_development_dependency 'fakeredis' gem.add_development_dependency 'guard-rspec' - gem.add_development_dependency 'pry-nav' - gem.add_development_dependency 'rb-fsevent' gem.add_development_dependency 'timecop' gem.add_development_dependency 'rake' end diff --git a/spec/pause/action_spec.rb b/spec/pause/action_spec.rb index 298efdc..7948217 100644 --- a/spec/pause/action_spec.rb +++ b/spec/pause/action_spec.rb @@ -139,6 +139,56 @@ class MyNotification < Pause::Action end end + context 'DSL usage' do + class CowRateLimited < Pause::Action + scope 'cow:moo' + check period_seconds: 10, max_allowed: 2, block_ttl: 40 + check period_seconds: 20, max_allowed: 4, block_ttl: 40 + end + + let(:identifier) { 'cow-moo' } + let(:action) { CowRateLimited.new(identifier) } + let(:bogus) { Struct.new(:name, :event).new } + + describe '#unless_rate_limited' do + before do + expect(bogus).to receive(:name).exactly(2).times + end + it 'should call through the block' do + action.unless_rate_limited { bogus.name } + action.unless_rate_limited { bogus.name } + result = action.unless_rate_limited { bogus.name } + expect(result).to be_a_kind_of(::Pause::RateLimitedEvent) + end + end + + describe '#unless_rate_limited' do + before { expect(bogus).to receive(:name).exactly(2).times } + + it 'should call through the block' do + 3.times { action.unless_rate_limited { bogus.name } } + end + + describe '#if_rate_limited' do + before { 2.times { action.unless_rate_limited { bogus.name } } } + + it 'it should not analyze during method call' do + bogus.event = 1 + action.if_rate_limited { |event| bogus.event = event } + expect(bogus.event).to be_a_kind_of(::Pause::RateLimitedEvent) + expect(bogus.event.identifier).to eq(identifier) + end + + it 'should analyze if requested' do + action.unless_rate_limited { bogus.name } + result = action.if_rate_limited { |event| bogus.event = event } + expect(bogus.event).to be_a_kind_of(::Pause::RateLimitedEvent) + expect(result).to eq(bogus.event) + end + end + end + end + describe '.tracked_identifiers' do it 'should return all the identifiers tracked (but not blocked) so far' do action.increment! diff --git a/spec/pause/redis/adapter_spec.rb b/spec/pause/redis/adapter_spec.rb index 86c4231..2c93384 100644 --- a/spec/pause/redis/adapter_spec.rb +++ b/spec/pause/redis/adapter_spec.rb @@ -31,31 +31,25 @@ expect(set[0].size).to eql(2) end - RSpec.shared_examples 'removes old elements' do + context 'removing two elements' do + let(:to_delete) { 2 } let(:time) { Time.now } before do + adapter to_delete.times do |t| expect(redis_conn).to receive(:zrem).with(tracked_key, [adapter.period_marker(resolution, time + t)]).once end adapter.time_blocks_to_keep = 1 end it 'should remove old elements' do - Timecop.freeze time do - adapter.increment(scope, identifier, Time.now.to_i) - end + adapter.increment(scope, identifier, time.to_i) to_delete.times do |t| - Timecop.freeze time + (adapter.resolution + t + 1) do - adapter.increment(scope, identifier, Time.now.to_i) - end + next_time = time + (adapter.resolution + t + 1) + adapter.increment(scope, identifier, next_time.to_i) end end end - context 'removing two elements' do - let(:to_delete) { 2 } - it_behaves_like 'removes old elements' - end - it 'sets expiry on key' do expect(redis_conn).to receive(:expire).with(tracked_key, history) adapter.increment(scope, identifier, Time.now.to_i) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 524f515..0c6ea69 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -5,15 +5,12 @@ # # See http://rubydoc.info/gems/rspec-core/RSpec/Core/Configuration -ENV['BUNDLE_GEMFILE'] ||= File.expand_path('../../Gemfile', __FILE__) -require 'rubygems' -require 'bundler/setup' if File.exists?(ENV['BUNDLE_GEMFILE']) require 'pause' -require 'pry' +require 'fileutils' if ENV['PAUSE_REAL_REDIS'] require 'pause/redis/adapter' - puts "\n==> Using real Redis-server at #{Pause::Redis::Adapter.redis.inspect}\n\n" + puts ; puts "NOTE: Using real Redis-server at #{Pause::Redis::Adapter.redis.inspect}\n\n" else require 'fakeredis/rspec' end @@ -22,11 +19,12 @@ config.run_all_when_everything_filtered = true config.filter_run :focus - # Run specs in random order to surface order dependencies. If you find an - # order dependency and want to debug it, you can fix the order by providing - # the seed, which is printed after each run. - # --seed 1234 + rspec_dir = './.spec'.freeze + FileUtils.mkdir_p(rspec_dir) + config.example_status_persistence_file_path = "#{rspec_dir}/results.txt" + config.order = 'random' + if ENV['PAUSE_REAL_REDIS'] config.before(:example) do Pause::Redis::Adapter.redis.flushdb From 40f7c4c531ff0720645023c700746cf9c699c522 Mon Sep 17 00:00:00 2001 From: Konstantin Gredeskoul Date: Wed, 28 Mar 2018 20:29:11 -0700 Subject: [PATCH 12/16] Making Travis more robust --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 9ef53a0..28977a1 100644 --- a/.travis.yml +++ b/.travis.yml @@ -3,7 +3,7 @@ rvm: - 2.3.3 - 2.4.3 - 2.5.0 -script: "bundle exec rake || bundle exec rspec --only-failures" +script: "bundle exec rake || ( sleep 1; bundle exec rspec --only-failures ) " services: - redis-server notifications: From f7c8bb6e3aea178442fe17e46c72d519f5f8acde Mon Sep 17 00:00:00 2001 From: Konstantin Gredeskoul Date: Wed, 28 Mar 2018 20:37:01 -0700 Subject: [PATCH 13/16] Adding SimpleCov text coverage (currently at 99.13%) --- spec/spec_helper.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 0c6ea69..1e53cf7 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -5,9 +5,13 @@ # # See http://rubydoc.info/gems/rspec-core/RSpec/Core/Configuration -require 'pause' require 'fileutils' +require 'simplecov' +SimpleCov.start + +require 'pause' + if ENV['PAUSE_REAL_REDIS'] require 'pause/redis/adapter' puts ; puts "NOTE: Using real Redis-server at #{Pause::Redis::Adapter.redis.inspect}\n\n" From 033147e5185671c2c9e5d300cf30a2e5976a6d9c Mon Sep 17 00:00:00 2001 From: Konstantin Gredeskoul Date: Thu, 29 Mar 2018 10:32:52 -0700 Subject: [PATCH 14/16] Block usage for Action constructor - also adding CodeCimate, new tests - updating README - version bump 0.4.1 --- .travis.yml | 23 ++++++++++---- LICENSE.txt | 2 +- README.md | 63 +++++++++++++++++++++++++-------------- lib/pause.rb | 1 + lib/pause/action.rb | 3 +- lib/pause/logger.rb | 6 ++-- lib/pause/version.rb | 2 +- pause.gemspec | 1 + spec/pause/action_spec.rb | 21 +++++++++---- spec/pause/logger_spec.rb | 13 ++++++++ 10 files changed, 94 insertions(+), 41 deletions(-) create mode 100644 spec/pause/logger_spec.rb diff --git a/.travis.yml b/.travis.yml index 28977a1..f22361f 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,14 +1,25 @@ language: ruby rvm: - - 2.3.3 - - 2.4.3 - - 2.5.0 -script: "bundle exec rake || ( sleep 1; bundle exec rspec --only-failures ) " +- 2.3.3 +- 2.4.3 +- 2.5.0 +cache: +- bundler +before_script: + - curl -L https://codeclimate.com/downloads/test-reporter/test-reporter-latest-linux-amd64 > ./cc-test-reporter + - chmod +x ./cc-test-reporter + - ./cc-test-reporter before-build +after_script: + - ./cc-test-reporter after-build --exit-code $TRAVIS_TEST_RESULT +script: 'bundle exec rake || ( sleep 1; bundle exec rspec --only-failures ) ' services: - - redis-server +- redis-server notifications: email: recipients: - - kigster@gmail.com + - kigster@gmail.com on_success: never on_failure: always +env: + global: + secure: bkq6cFUhMqn2ppqUPNax5biIivGw7uuOf1+pK4o9EFsi2qkuzLMQMulwmKC+RMLUMsaITkvec3Lp+kHwRYiXr4bxiFaBg+Q278W9o+mRWcBEh6mGnDVCgR13xdMfDZFrafEm44jEnOWJICkBlmdfMkMOriJUTowc8g745jpGEUNEu7ZqIsVFSflb+GcdYXhlouEThhkcwcdmRwkXqfq7pp8AEhiji2V5PNKiVY+Zu/lq9APAqMqFmXDZ0+SAjkeagSSCtLiXYQqc4Z1PU2Jvyov7nfDJ72VYqvfqevSe9+rqitOleR/BvIoIsGO+et7Dq94liK964fzP+spp1ODUMdhbC7tmBuYqYr3lxsK5S6bHZ9/LABHKOMbpKVJefrxmyh/QaQpjA5w3vuSkNZXD/OsZ0ddmHOvya6cv5sTX//muJVmba88IMCcmQSAZUIYK8796ACnnvDhlQ9n/ilOYzP69W+RmkyX09SH2VR9AeMSjwRESCOh0XYMevHNIjfOk24nPnsH5OT317p8dyCfn9Z+dif1iTEujRAqAzGh6AmWQNQQx6HEC8QDQGOMpLmOzlLWeQMgo90KOe78JA4iGGmVfbFvB/qJcZ4ZacG+s8tlLU8ADcq7yj2sYCHZOEatpQZbNAMjJwoZMyqojhCZgtH039LxwDXoD4/u0epk+RuY= diff --git a/LICENSE.txt b/LICENSE.txt index 1259baf..60de880 100644 --- a/LICENSE.txt +++ b/LICENSE.txt @@ -1,4 +1,4 @@ -Copyright (c) 2012 Wanelo, Inc +Copyright © 2018 Konstantin Gredeskoul, Atasay Gokkaya, Eric Saxby, Paul Henry MIT License diff --git a/README.md b/README.md index f5540cc..c382772 100644 --- a/README.md +++ b/README.md @@ -50,16 +50,21 @@ Pause.configure do |config| config.redis_port = 6379 config.redis_db = 1 - # aggregate all events into 10 minute blocks. - # Larger blocks require less RAM and CPU, smaller blocks are more - # computationally expensive. - config.resolution = 600 - - # discard all events older than 1 day - config.history = 86400 + config.resolution = 600 + config.history = 7 * 86400 # discard events older than 7 days end ``` +> NOTE: **resolution** is an setting that's key to understanding how Pause works. It represents the length of time during which similar events are aggregated into a Hash-like object, where the key is the identifier, and the value is the count within that period. +> +> Because of this, +> +> * _Larger resolution requires less RAM and CPU and are faster to compute_ +> * _Smaller resolution is more computationally expensive, but provides higher granularity_. +> +> The resolution setting must set to the smallest rate-limit period across all of your checks. Below it is set to 10 minutes, meaning that you can use Pause to **rate limit any event to no more than N times within a period of 10 minutes or more.** + + #### Define Rate Limited "Action" Next we must define the rate limited action based on the specification above. This is how easy it is: @@ -69,13 +74,22 @@ module MyApp class UserNotificationLimiter < ::Pause::Action # this is a redis key namespace added to all data in this action scope 'un' - check period_seconds: 120, max_allowed: 1 - check period_seconds: 86400, max_allowed: 3 - check period_seconds: 7 * 86400, max_allowed: 7 + + check period_seconds: 120, + max_allowed: 1, + block_ttl: 240 + + check period_seconds: 86400, + max_allowed: 3 + + check period_seconds: 7 *86400, + max_allowed: 7 end end ``` +> NOTE: for each check, `block_ttl` defaults to `period_seconds`, and represents the duration of time the action will consider itself as "rate limited" after a particular check reaches the limit. Note, that all actions will automatically leave the "rate limited" state after `block_ttl` seconds have passed. + #### Perform operation, but only if the user is not rate-limited Now we simply instantiate this limiter by passing user ID (any unique identifier works). We can then ask the limiter, `ok?` or `rate_limited?`, or we can use two convenient methods that only execute enclosed block if the described condition is satisfied: @@ -83,17 +97,18 @@ Now we simply instantiate this limiter by passing user ID (any unique identifier ```ruby class NotificationsWorker def perform(user_id) - limiter = MyApp::UserNotificationLimiter.new(user_id) - - limiter.unless_rate_limited do - user = User.find(user_id) - user.send_push_notification! - end - - # You can also do something in case the user is rate limited: - limiter.if_rate_limited do |rate_limit_event| - Rails.logger.info("user #{user.id} has exceeded rate limit: #{rate_limit_event}") - end + MyApp::UserNotificationLimiter.new(user_id) do + unless_rate_limited do + # this block ONLY runs if rate limit is not reached + user = User.find(user_id) + user.send_push_notification! + end + + if_rate_limited do |rate_limit_event| + # this block ONLY runs if the action has reached it's rate limit. + Rails.logger.info("user #{user.id} has exceeded rate limit: #{rate_limit_event}") + end + end end end ``` @@ -339,8 +354,10 @@ Want to make it better? Cool. Here's how: ## Authors -This gem was written by Eric Saxby, Atasay Gokkaya and Konstantin Gredeskoul at Wanelo, Inc. + * This gem was written by Eric Saxby, Atasay Gokkaya and Konstantin Gredeskoul at Wanelo, Inc. + * It's been updated and refreshed by Konstantin Gredeskoul. + -Please see the LICENSE.txt file for further details. +Please see the [LICENSE.txt](LICENSE.txt) file for further details. diff --git a/lib/pause.rb b/lib/pause.rb index f509cfc..33a2291 100644 --- a/lib/pause.rb +++ b/lib/pause.rb @@ -1,4 +1,5 @@ require 'redis' +require 'colored2' require 'pause/version' require 'pause/configuration' require 'pause/action' diff --git a/lib/pause/action.rb b/lib/pause/action.rb index 3750cf1..7509df6 100644 --- a/lib/pause/action.rb +++ b/lib/pause/action.rb @@ -2,9 +2,10 @@ module Pause class Action attr_accessor :identifier - def initialize(identifier) + def initialize(identifier, &block) @identifier = identifier self.class.checks ||= [] + instance_exec(&block) if block end def scope diff --git a/lib/pause/logger.rb b/lib/pause/logger.rb index 60dd7d8..30aa56d 100644 --- a/lib/pause/logger.rb +++ b/lib/pause/logger.rb @@ -1,11 +1,11 @@ module Pause class Logger - def self.puts message + def self.puts(message) STDOUT.puts message end - def self.fatal message - STDERR.puts message + def self.fatal(message) + STDERR.puts message.red end end end diff --git a/lib/pause/version.rb b/lib/pause/version.rb index 06effb7..bda026a 100644 --- a/lib/pause/version.rb +++ b/lib/pause/version.rb @@ -1,3 +1,3 @@ module Pause - VERSION = '0.4.0' + VERSION = '0.4.1' end diff --git a/pause.gemspec b/pause.gemspec index 4f2c321..b56ad4b 100644 --- a/pause.gemspec +++ b/pause.gemspec @@ -19,6 +19,7 @@ Gem::Specification.new do |gem| gem.add_dependency 'redis' gem.add_dependency 'hiredis' + gem.add_dependency 'colored2' gem.add_development_dependency 'simplecov' gem.add_development_dependency 'yard' diff --git a/spec/pause/action_spec.rb b/spec/pause/action_spec.rb index 7948217..160fe54 100644 --- a/spec/pause/action_spec.rb +++ b/spec/pause/action_spec.rb @@ -151,19 +151,28 @@ class CowRateLimited < Pause::Action let(:bogus) { Struct.new(:name, :event).new } describe '#unless_rate_limited' do - before do - expect(bogus).to receive(:name).exactly(2).times + before { expect(bogus).to receive(:name).exactly(2).times } + + describe '#initialize' do + before { expect(bogus).to receive(:event).exactly(1).times } + it 'should be able to use methods inside the #new block' do + b = bogus + CowRateLimited.new(identifier) do + unless_rate_limited { b.name } # this executes + unless_rate_limited { b.name } # this also executes + unless_rate_limited { b.name } # and this will be rate limited + if_rate_limited { b.event } + end + end end + + it 'should call through the block' do action.unless_rate_limited { bogus.name } action.unless_rate_limited { bogus.name } result = action.unless_rate_limited { bogus.name } expect(result).to be_a_kind_of(::Pause::RateLimitedEvent) end - end - - describe '#unless_rate_limited' do - before { expect(bogus).to receive(:name).exactly(2).times } it 'should call through the block' do 3.times { action.unless_rate_limited { bogus.name } } diff --git a/spec/pause/logger_spec.rb b/spec/pause/logger_spec.rb new file mode 100644 index 0000000..77037ae --- /dev/null +++ b/spec/pause/logger_spec.rb @@ -0,0 +1,13 @@ +require 'spec_helper' + +describe Pause::Logger do + before do + expect(STDOUT).to receive(:puts).with('hello') + expect(STDERR).to receive(:puts).with('whoops'.red) + end + + it 'will call through STDOUT/STDERR' do + described_class.puts('hello') + described_class.fatal('whoops') + end +end From 15abc63998c9d9d6e39e14e2dd5888ec12629229 Mon Sep 17 00:00:00 2001 From: Konstantin Gredeskoul Date: Thu, 29 Mar 2018 10:34:34 -0700 Subject: [PATCH 15/16] Add badges to README --- README.md | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index c382772..a4ab60c 100644 --- a/README.md +++ b/README.md @@ -1,10 +1,14 @@ -# Pause - -[![Gem Version](https://badge.fury.io/rb/pause.png)](http://badge.fury.io/rb/pause) [![Build Status](https://travis-ci.org/kigster/pause.svg?branch=master)](https://travis-ci.org/kigster/pause) +[![Test Coverage](https://api.codeclimate.com/v1/badges/af443a25cc902e629c8f/test_coverage)](https://codeclimate.com/github/kigster/pause/test_coverage) + +[![Gem Version](https://badge.fury.io/rb/pause.png)](https://badge.fury.io/rb/pause) +[![Maintainability](https://api.codeclimate.com/v1/badges/af443a25cc902e629c8f/maintainability)](https://codeclimate.com/github/kigster/pause/maintainability) + +# Pause ## In a Nutshell + **Pause** is a fast and very flexible Redis-backed rate-limiter. You can use it to track events, with rules around how often they are allowed to occur within configured time checks. From 96662d53bb06f1281a7d64a61f64f65d71b88598 Mon Sep 17 00:00:00 2001 From: Konstantin Gredeskoul Date: Thu, 29 Mar 2018 10:41:12 -0700 Subject: [PATCH 16/16] Make builds more robust --- .travis.yml | 2 +- README.md | 2 +- bin/spec | 14 ++++++++++++++ 3 files changed, 16 insertions(+), 2 deletions(-) create mode 100755 bin/spec diff --git a/.travis.yml b/.travis.yml index f22361f..30324cd 100644 --- a/.travis.yml +++ b/.travis.yml @@ -11,7 +11,7 @@ before_script: - ./cc-test-reporter before-build after_script: - ./cc-test-reporter after-build --exit-code $TRAVIS_TEST_RESULT -script: 'bundle exec rake || ( sleep 1; bundle exec rspec --only-failures ) ' +script: bin/spec services: - redis-server notifications: diff --git a/README.md b/README.md index a4ab60c..3733576 100644 --- a/README.md +++ b/README.md @@ -1,7 +1,7 @@ [![Build Status](https://travis-ci.org/kigster/pause.svg?branch=master)](https://travis-ci.org/kigster/pause) [![Test Coverage](https://api.codeclimate.com/v1/badges/af443a25cc902e629c8f/test_coverage)](https://codeclimate.com/github/kigster/pause/test_coverage) -[![Gem Version](https://badge.fury.io/rb/pause.png)](https://badge.fury.io/rb/pause) +[![Gem Version](https://badge.fury.io/rb/pause.svg)](https://badge.fury.io/rb/pause.svg) [![Maintainability](https://api.codeclimate.com/v1/badges/af443a25cc902e629c8f/maintainability)](https://codeclimate.com/github/kigster/pause/maintainability) # Pause diff --git a/bin/spec b/bin/spec new file mode 100755 index 0000000..0d21836 --- /dev/null +++ b/bin/spec @@ -0,0 +1,14 @@ +#!/usr/bin/env bash + +retry-errors() { + sleep 1 + bundle exec rspec --only-failures +} + +specs() { + bundle exec rspec && \ + PAUSE_REAL_REDIS=true bundle exec rspec +} + +specs || retry-errors +