From c79a67c5774d5ece25b74c5f8b5137fc2ba99e25 Mon Sep 17 00:00:00 2001 From: Paul Henry Date: Fri, 1 Aug 2014 10:58:29 -0700 Subject: [PATCH 1/7] wip --- .gitignore | 1 - Gemfile.lock | 63 +++++++++++++++++++++++++++++++ lib/pause/analyzer.rb | 46 +++++++++++++++++----- pause.gemspec | 2 +- spec/pause/action_spec.rb | 38 ++++++++++++------- spec/pause/analyzer_spec.rb | 65 ++++++++++++++++++++++++++++++-- spec/pause/redis/adapter_spec.rb | 1 - 7 files changed, 186 insertions(+), 30 deletions(-) create mode 100644 Gemfile.lock diff --git a/.gitignore b/.gitignore index d6b49a9..55f23a1 100644 --- a/.gitignore +++ b/.gitignore @@ -3,7 +3,6 @@ .bundle .config .yardoc -Gemfile.lock InstalledFiles _yardoc coverage diff --git a/Gemfile.lock b/Gemfile.lock new file mode 100644 index 0000000..fba2262 --- /dev/null +++ b/Gemfile.lock @@ -0,0 +1,63 @@ +PATH + remote: . + specs: + pause (0.0.6) + redis + +GEM + remote: https://rubygems.org/ + specs: + celluloid (0.15.2) + timers (~> 1.1.0) + coderay (1.1.0) + diff-lcs (1.2.5) + fakeredis (0.5.0) + redis (~> 3.0) + ffi (1.9.3) + formatador (0.2.5) + guard (2.6.1) + formatador (>= 0.2.4) + listen (~> 2.7) + lumberjack (~> 1.0) + pry (>= 0.9.12) + thor (>= 0.18.1) + guard-rspec (4.2.9) + guard (~> 2.1) + rspec (>= 2.14, < 4.0) + listen (2.7.5) + celluloid (>= 0.15.2) + rb-fsevent (>= 0.9.3) + rb-inotify (>= 0.9) + lumberjack (1.0.5) + method_source (0.8.2) + pry (0.9.12.6) + coderay (~> 1.0) + method_source (~> 0.8) + slop (~> 3.4) + rb-fsevent (0.9.4) + rb-inotify (0.9.4) + ffi (>= 0.5.0) + redis (3.1.0) + rspec (2.99.0) + rspec-core (~> 2.99.0) + rspec-expectations (~> 2.99.0) + rspec-mocks (~> 2.99.0) + rspec-core (2.99.1) + rspec-expectations (2.99.2) + diff-lcs (>= 1.1.3, < 2.0) + rspec-mocks (2.99.2) + slop (3.5.0) + thor (0.19.1) + timecop (0.7.1) + timers (1.1.0) + +PLATFORMS + ruby + +DEPENDENCIES + fakeredis + guard-rspec + pause! + rb-fsevent + rspec (~> 2.0) + timecop diff --git a/lib/pause/analyzer.rb b/lib/pause/analyzer.rb index a89b5ed..7fd0aab 100644 --- a/lib/pause/analyzer.rb +++ b/lib/pause/analyzer.rb @@ -4,30 +4,58 @@ module Pause class Analyzer include Pause::Helper::Timing - def check(action) - timestamp = period_marker(Pause.config.resolution, Time.now.to_i) - set = adapter.key_history(action.key) - action.checks.each do |period_check| + class BlockTTLChecker < Struct.new(:period_check, :timestamp, :set, :action) + def check start_time = timestamp - period_check.period_seconds set.reverse.inject(0) do |sum, element| break if element.ts < start_time sum += element.count if sum >= period_check.max_allowed - adapter.rate_limit!(action.key, period_check.block_ttl) + Pause.adapter.rate_limit!(action.key, period_check.block_ttl) # Note that Time.now is different from period_marker(resolution, Time.now), which # rounds down to the nearest (resolution) seconds return Pause::RateLimitedEvent.new(action, period_check, sum, Time.now.to_i) end sum end + nil end - nil end - private + class PeriodTTLChecker < Struct.new(:period_check, :timestamp, :set, :action) + def check + start_time = timestamp - period_check.period_seconds + set.select!{|element| element.ts >= start_time} + set.inject(0) do |sum, element| + sum += element.count + if sum >= period_check.max_allowed + block_ttl = set.first.ts + period_check.period_seconds - element.ts + Pause.adapter.rate_limit!(action.key, block_ttl) + # Note that Time.now is different from period_marker(resolution, Time.now), which + # rounds down to the nearest (resolution) seconds + return Pause::RateLimitedEvent.new(action, period_check, sum, Time.now.to_i) + end + sum + end + nil + end + end + + def check(action) + timestamp = period_marker(Pause.config.resolution, Time.now.to_i) + set = Pause.adapter.key_history(action.key) + action.checks.each do |period_check| + checker = if period_check.block_ttl + BlockTTLChecker.new(period_check, timestamp, set, action) + else + PeriodTTLChecker.new(period_check, timestamp, set, action) + end - def adapter - Pause.adapter + puts checker.inspect + result = checker.check + return result if result + end + nil end end end diff --git a/pause.gemspec b/pause.gemspec index 0933320..9624c83 100644 --- a/pause.gemspec +++ b/pause.gemspec @@ -19,7 +19,7 @@ Gem::Specification.new do |gem| gem.add_dependency 'redis' - gem.add_development_dependency 'rspec' + gem.add_development_dependency 'rspec', '~> 2.0' gem.add_development_dependency 'fakeredis' gem.add_development_dependency 'timecop' gem.add_development_dependency 'guard-rspec' diff --git a/spec/pause/action_spec.rb b/spec/pause/action_spec.rb index 4d5233b..423a8e6 100644 --- a/spec/pause/action_spec.rb +++ b/spec/pause/action_spec.rb @@ -15,9 +15,9 @@ class MyNotification < Pause::Action let(:configuration) { Pause::Configuration.new } before do - Pause.stub(:config).and_return(configuration) - Pause.config.stub(:resolution).and_return(resolution) - Pause.config.stub(:history).and_return(history) + allow(Pause).to receive(:config).and_return(configuration) + allow(configuration).to receive(:resolution).and_return(resolution) + allow(configuration).to receive(:history).and_return(history) end let(:action) { MyNotification.new("1237612") } @@ -47,17 +47,26 @@ class MyNotification < Pause::Action end it "should successfully consider different period checks" do - time = period_marker(resolution, Time.now.to_i) + time = Time.now.to_i - action.increment! 4, time - 25 - action.ok?.should be_true + puts "incrementing by 4" + Timecop.freeze time - 25 do + action.increment! 4 + action.ok?.should be_true + end - action.increment! 2, time - 3 - action.ok?.should be_true + Timecop.freeze time - 3 do + puts "incrementing by 2" + action.increment! 2 + action.ok?.should be_true + end - action.increment! 1, time + Timecop.freeze time do + puts "incrementing by 1" + action.increment! 1 - action.ok?.should be_false + action.ok?.should be_false + end end it "should return false and silently fail if redis is not available" do @@ -217,11 +226,12 @@ class BlockedAction < Pause::Action check 10, 0, 10 end + let(:configuration) { Pause::Configuration.new } + before do - Pause.configure do |c| - c.resolution = 10 - c.history = 10 - end + allow(Pause).to receive(:config).and_return(configuration) + allow(Pause.config).to receive(:resolution).and_return(10) + allow(Pause.config).to receive(:history).and_return(10) end let(:action) { BlockedAction } diff --git a/spec/pause/analyzer_spec.rb b/spec/pause/analyzer_spec.rb index eed7763..4628ca2 100644 --- a/spec/pause/analyzer_spec.rb +++ b/spec/pause/analyzer_spec.rb @@ -10,14 +10,19 @@ class FollowPushNotification < Pause::Action check 40, 7, 12 end - let(:resolution) { 10 } + class UberSimpleCheck < Pause::Action + scope "usc" + check period_seconds: 10, max_allowed: 3 + end + + let(:resolution) { 1 } let(:history) { 60 } let(:configuration) { Pause::Configuration.new } before do - Pause.stub(:config).and_return(configuration) - Pause.config.stub(:resolution).and_return(resolution) - Pause.config.stub(:history).and_return(history) + allow(Pause).to receive(:config).and_return(configuration) + allow(Pause.config).to receive(:resolution).and_return(resolution) + allow(Pause.config).to receive(:history).and_return(history) end let(:analyzer) { Pause.analyzer } @@ -35,6 +40,58 @@ class FollowPushNotification < Pause::Action end end end + + context "checks without block_ttl" do + let(:action) { UberSimpleCheck.new("1243123") } + + it "blocks only for the amount of time until an action would be allowed again" do + adapter.should_receive(:rate_limit!).once.with(action.key, 3) + + now = Time.now.to_i + + Timecop.freeze now - 10 do + action.increment! + end + + Timecop.freeze now - 5 do + action.increment! + end + + Timecop.freeze now - 3 do + action.increment! + end + + Timecop.freeze now do + analyzer.check(action) + end + end + end + + context "with a set element with a higher count" do + let(:action) { UberSimpleCheck.new("1243123") } + + it "blocks for a period appropriate to when the limit was exceeded" do + adapter.should_receive(:rate_limit!).once.with(action.key, 5) + + now = Time.now.to_i + + Timecop.freeze now - 10 do + action.increment! + end + + Timecop.freeze now - 5 do + action.increment!(2) + end + + Timecop.freeze now - 3 do + action.increment! + end + + Timecop.freeze now do + analyzer.check(action) + end + end + end end describe "#check" do diff --git a/spec/pause/redis/adapter_spec.rb b/spec/pause/redis/adapter_spec.rb index 7c0ff92..bd51782 100644 --- a/spec/pause/redis/adapter_spec.rb +++ b/spec/pause/redis/adapter_spec.rb @@ -9,7 +9,6 @@ let(:configuration) { Pause::Configuration.new } before do - Pause.stub(:config).and_return(configuration) Pause.config.stub(:resolution).and_return(resolution) Pause.config.stub(:history).and_return(history) redis_conn.flushall From c446a07b3828acded9f5702a591f1c1e47e3bc69 Mon Sep 17 00:00:00 2001 From: Max Gurewitz & Paul Henry Date: Fri, 1 Aug 2014 12:05:35 -0700 Subject: [PATCH 2/7] pause should check the rate limited key before analyzing --- lib/pause/action.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/pause/action.rb b/lib/pause/action.rb index c2edfef..e4de2f0 100644 --- a/lib/pause/action.rb +++ b/lib/pause/action.rb @@ -66,7 +66,7 @@ def rate_limited? end def ok? - Pause.analyzer.check(self).nil? + !Pause.adapter.rate_limited?(key) && Pause.analyzer.check(self).nil? rescue ::Redis::CannotConnectError => e $stderr.puts "Error connecting to redis: #{e.inspect}" false From 66c40ee6409bc17d6ccf3eeb5e39eda890bdae27 Mon Sep 17 00:00:00 2001 From: Max Gurewitz & Paul Henry Date: Fri, 1 Aug 2014 12:05:44 -0700 Subject: [PATCH 3/7] use rspec 3.0 --- Gemfile.lock | 22 +++++++++++++--------- pause.gemspec | 2 +- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index fba2262..375ba5b 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -38,14 +38,18 @@ GEM rb-inotify (0.9.4) ffi (>= 0.5.0) redis (3.1.0) - rspec (2.99.0) - rspec-core (~> 2.99.0) - rspec-expectations (~> 2.99.0) - rspec-mocks (~> 2.99.0) - rspec-core (2.99.1) - rspec-expectations (2.99.2) - diff-lcs (>= 1.1.3, < 2.0) - rspec-mocks (2.99.2) + rspec (3.0.0) + rspec-core (~> 3.0.0) + rspec-expectations (~> 3.0.0) + rspec-mocks (~> 3.0.0) + rspec-core (3.0.3) + rspec-support (~> 3.0.0) + rspec-expectations (3.0.3) + diff-lcs (>= 1.2.0, < 2.0) + rspec-support (~> 3.0.0) + rspec-mocks (3.0.3) + rspec-support (~> 3.0.0) + rspec-support (3.0.3) slop (3.5.0) thor (0.19.1) timecop (0.7.1) @@ -59,5 +63,5 @@ DEPENDENCIES guard-rspec pause! rb-fsevent - rspec (~> 2.0) + rspec (~> 3.0) timecop diff --git a/pause.gemspec b/pause.gemspec index 9624c83..edd5112 100644 --- a/pause.gemspec +++ b/pause.gemspec @@ -19,7 +19,7 @@ Gem::Specification.new do |gem| gem.add_dependency 'redis' - gem.add_development_dependency 'rspec', '~> 2.0' + gem.add_development_dependency 'rspec', '~> 3.0' gem.add_development_dependency 'fakeredis' gem.add_development_dependency 'timecop' gem.add_development_dependency 'guard-rspec' From 940d14140d0c752be7416e8762647e746d16b4e5 Mon Sep 17 00:00:00 2001 From: Max Gurewitz & Paul Henry Date: Fri, 1 Aug 2014 12:05:57 -0700 Subject: [PATCH 4/7] remove useless puts --- lib/pause/action.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/pause/action.rb b/lib/pause/action.rb index e4de2f0..59c3cbc 100644 --- a/lib/pause/action.rb +++ b/lib/pause/action.rb @@ -68,7 +68,6 @@ def rate_limited? def ok? !Pause.adapter.rate_limited?(key) && Pause.analyzer.check(self).nil? rescue ::Redis::CannotConnectError => e - $stderr.puts "Error connecting to redis: #{e.inspect}" false end From 218a7b7b5049a10860e7dc99f4767dad7aee7720 Mon Sep 17 00:00:00 2001 From: Max Gurewitz & Paul Henry Date: Fri, 1 Aug 2014 12:06:14 -0700 Subject: [PATCH 5/7] remove once useful puts --- lib/pause/analyzer.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/pause/analyzer.rb b/lib/pause/analyzer.rb index 7fd0aab..777bfa2 100644 --- a/lib/pause/analyzer.rb +++ b/lib/pause/analyzer.rb @@ -51,7 +51,6 @@ def check(action) PeriodTTLChecker.new(period_check, timestamp, set, action) end - puts checker.inspect result = checker.check return result if result end From 7e27e5524c8100cb0d4d204eab1a1d7388cd6895 Mon Sep 17 00:00:00 2001 From: Max Gurewitz & Paul Henry Date: Fri, 1 Aug 2014 12:06:29 -0700 Subject: [PATCH 6/7] reset adapter before every test --- spec/spec_helper.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index b403cd8..880ad5e 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -24,5 +24,6 @@ config.before :each do Redis.new.flushall + Pause.instance_variable_set(:@adapter, nil) end end From 8a5a992fb3ab1f2cbad89308163276ea07cbda37 Mon Sep 17 00:00:00 2001 From: Max Gurewitz & Paul Henry Date: Fri, 1 Aug 2014 12:07:03 -0700 Subject: [PATCH 7/7] remove ALL THE deprecation warnings --- spec/pause/action_spec.rb | 85 +++++++++++++++----------------- spec/pause/analyzer_spec.rb | 12 ++--- spec/pause/configuration_spec.rb | 20 ++++---- spec/pause/redis/adapter_spec.rb | 51 +++++++++---------- spec/spec_helper.rb | 1 - 5 files changed, 83 insertions(+), 86 deletions(-) diff --git a/spec/pause/action_spec.rb b/spec/pause/action_spec.rb index 423a8e6..316947f 100644 --- a/spec/pause/action_spec.rb +++ b/spec/pause/action_spec.rb @@ -5,12 +5,12 @@ include Pause::Helper::Timing class MyNotification < Pause::Action - scope "ipn:follow" + scope "ipn:mynotification" check period_seconds: 20, max_allowed: 5, block_ttl: 40 check period_seconds: 40, max_allowed: 7, block_ttl: 40 end - let(:resolution) { 10 } + let(:resolution) { 1 } let(:history) { 60 } let(:configuration) { Pause::Configuration.new } @@ -27,7 +27,7 @@ class MyNotification < Pause::Action it "should increment" do time = Time.now Timecop.freeze time do - Pause.adapter.should_receive(:increment).with(action.key, time.to_i, 1) + expect(Pause.adapter).to receive(:increment).with(action.key, time.to_i, 1) action.increment! end end @@ -39,54 +39,51 @@ class MyNotification < Pause::Action Timecop.freeze time do 4.times do action.increment! - action.ok?.should be_true + expect(action.ok?).to eq(true) end action.increment! - action.ok?.should be_false + expect(action.ok?).to eq(false) end end it "should successfully consider different period checks" do time = Time.now.to_i - puts "incrementing by 4" Timecop.freeze time - 25 do action.increment! 4 - action.ok?.should be_true + expect(action.ok?).to eq(true) end Timecop.freeze time - 3 do - puts "incrementing by 2" action.increment! 2 - action.ok?.should be_true + expect(action.ok?).to eq(true) end Timecop.freeze time do - puts "incrementing by 1" action.increment! 1 - action.ok?.should be_false + expect(action.ok?).to eq(false) end end it "should return false and silently fail if redis is not available" do - Redis.any_instance.stub(:zrange) { raise Redis::CannotConnectError } + allow_any_instance_of(Redis).to receive(:zrange) { raise Redis::CannotConnectError } time = period_marker(resolution, Time.now.to_i) action.increment! 4, time - 25 - action.ok?.should be_false + expect(action.ok?).to eq(false) end end describe "#analyze" do - context "action should not be rate limited" do + describe "action not to be rate limited" do it "returns nil" do - action.analyze.should be_nil + expect(action.analyze).to be_nil end end - context "action should be rate limited" do + describe "action to be rate limited" do it "returns a RateLimitedEvent object" do time = Time.now rate_limit = nil @@ -98,11 +95,11 @@ class MyNotification < Pause::Action expected_rate_limit = Pause::RateLimitedEvent.new(action, action.checks[0], 7, time.to_i) - rate_limit.should be_a(Pause::RateLimitedEvent) - rate_limit.identifier.should == expected_rate_limit.identifier - rate_limit.sum.should == expected_rate_limit.sum - rate_limit.period_check.should == expected_rate_limit.period_check - rate_limit.timestamp.should == expected_rate_limit.timestamp + 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 @@ -115,8 +112,8 @@ class MyNotification < Pause::Action action.ok? other_action.ok? - MyNotification.tracked_identifiers.should include(action.identifier) - MyNotification.tracked_identifiers.should include(other_action.identifier) + expect(MyNotification.tracked_identifiers).to include(action.identifier) + expect(MyNotification.tracked_identifiers).to include(other_action.identifier) end end @@ -128,8 +125,8 @@ class MyNotification < Pause::Action action.ok? other_action.ok? - MyNotification.rate_limited_identifiers.should include(action.identifier) - MyNotification.rate_limited_identifiers.should include(other_action.identifier) + expect(MyNotification.rate_limited_identifiers).to include(action.identifier) + expect(MyNotification.rate_limited_identifiers).to include(other_action.identifier) end end @@ -141,13 +138,13 @@ class MyNotification < Pause::Action action.ok? other_action.ok? - MyNotification.tracked_identifiers.should include(action.identifier, other_action.identifier) - MyNotification.rate_limited_identifiers.should == [action.identifier] + expect(MyNotification.tracked_identifiers).to include(action.identifier, other_action.identifier) + expect(MyNotification.rate_limited_identifiers).to eq([action.identifier]) MyNotification.unblock_all - MyNotification.rate_limited_identifiers.should be_empty - MyNotification.tracked_identifiers.should == [other_action.identifier] + expect(MyNotification.rate_limited_identifiers).to be_empty + expect(MyNotification.tracked_identifiers).to eq([other_action.identifier]) end end @@ -155,11 +152,11 @@ class MyNotification < Pause::Action it 'unblocks the specified id' do 10.times { action.increment! } - expect(action.ok?).to be_false + expect(action.ok?).to eq(false) action.unblock - expect(action.ok?).to be_true + expect(action.ok?).to eq(true) end end end @@ -180,13 +177,13 @@ class ActionWithHashChecks < Pause::Action end it "should define a period check on new instances" do - ActionWithCheck.new("id").checks.should == [ + expect(ActionWithCheck.new("id").checks).to eq [ Pause::PeriodCheck.new(100, 150, 200) ] end it "should define a period check on new instances" do - ActionWithMultipleChecks.new("id").checks.should == [ + 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) @@ -194,7 +191,7 @@ class ActionWithHashChecks < Pause::Action end it "should accept hash arguments" do - ActionWithHashChecks.new("id").checks.should == [ + expect(ActionWithHashChecks.new("id").checks).to eq [ Pause::PeriodCheck.new(50, 100, 60) ] end @@ -206,9 +203,9 @@ class UndefinedScopeAction < Pause::Action end it "should raise if scope is not defined" do - lambda { + expect { UndefinedScopeAction.new("1.2.3.4").scope - }.should raise_error("Should implement scope. (Ex: ipn:follow)") + }.to raise_error("Should implement scope. (Ex: ipn:follow)") end class DefinedScopeAction < Pause::Action @@ -216,7 +213,7 @@ class DefinedScopeAction < Pause::Action end it "should set scope on class" do - DefinedScopeAction.new("1.2.3.4").scope.should == "my:scope" + expect(DefinedScopeAction.new("1.2.3.4").scope).to eq("my:scope") end end @@ -238,27 +235,27 @@ class BlockedAction < Pause::Action describe "#disable" do before do - action.should be_enabled - action.should_not be_disabled + expect(action).to be_enabled + expect(action).to_not be_disabled action.disable end it "disables the action" do - action.should be_disabled - action.should_not be_enabled + expect(action).to be_disabled + expect(action).to_not be_enabled end end describe "#enable" do before do action.disable - action.should_not be_enabled + expect(action).to_not be_enabled action.enable end it "enables the action" do - action.should be_enabled - action.should_not be_disabled + expect(action).to be_enabled + expect(action).to_not be_disabled end end end diff --git a/spec/pause/analyzer_spec.rb b/spec/pause/analyzer_spec.rb index 4628ca2..28d18aa 100644 --- a/spec/pause/analyzer_spec.rb +++ b/spec/pause/analyzer_spec.rb @@ -5,7 +5,7 @@ include Pause::Helper::Timing class FollowPushNotification < Pause::Action - scope "ipn:follow" + scope "ipn:followpn" check 20, 5, 12 check 40, 7, 12 end @@ -32,7 +32,7 @@ class UberSimpleCheck < Pause::Action describe "#analyze" do it "checks and blocks if max_allowed is reached" do time = Time.now - adapter.should_receive(:rate_limit!).once.with(action.key, 12) + expect(adapter).to receive(:rate_limit!).once.with(action.key, 12) Timecop.freeze time do 5.times do action.increment! @@ -45,7 +45,7 @@ class UberSimpleCheck < Pause::Action let(:action) { UberSimpleCheck.new("1243123") } it "blocks only for the amount of time until an action would be allowed again" do - adapter.should_receive(:rate_limit!).once.with(action.key, 3) + expect(adapter).to receive(:rate_limit!).once.with(action.key, 3) now = Time.now.to_i @@ -71,7 +71,7 @@ class UberSimpleCheck < Pause::Action let(:action) { UberSimpleCheck.new("1243123") } it "blocks for a period appropriate to when the limit was exceeded" do - adapter.should_receive(:rate_limit!).once.with(action.key, 5) + expect(adapter).to receive(:rate_limit!).once.with(action.key, 5) now = Time.now.to_i @@ -96,7 +96,7 @@ class UberSimpleCheck < Pause::Action describe "#check" do it "should return nil if action is NOT blocked" do - analyzer.check(action).should be_nil + expect(analyzer.check(action)).to be_nil end it "should return blocked action if action is blocked" do @@ -104,7 +104,7 @@ class UberSimpleCheck < Pause::Action 5.times do action.increment! end - analyzer.check(action).should be_a(Pause::RateLimitedEvent) + expect(analyzer.check(action)).to be_a(Pause::RateLimitedEvent) end end end diff --git a/spec/pause/configuration_spec.rb b/spec/pause/configuration_spec.rb index b412166..6cf8ca7 100644 --- a/spec/pause/configuration_spec.rb +++ b/spec/pause/configuration_spec.rb @@ -14,12 +14,12 @@ c.history = 6000 end - subject.redis_host.should == "128.23.12.8" - subject.redis_port.should == 2134 - subject.redis_db.should == "13" + expect(subject.redis_host).to eq("128.23.12.8") + expect(subject.redis_port).to eq(2134) + expect(subject.redis_db).to eq("13") - subject.resolution.should == 5000 - subject.history.should == 6000 + expect(subject.resolution).to eq(5000) + expect(subject.history).to eq(6000) end it "should provide redis defaults" do @@ -27,10 +27,10 @@ # do nothing end - subject.redis_host.should == "127.0.0.1" - subject.redis_port.should == 6379 - subject.redis_db.should == "1" - subject.resolution.should == 600 # 10 minutes - subject.history.should == 86400 # one day + expect(subject.redis_host).to eq("127.0.0.1") + expect(subject.redis_port).to eq(6379) + expect(subject.redis_db).to eq("1") + expect(subject.resolution).to eq(600) # 10 minutes + expect(subject.history).to eq(86400) # one day end end diff --git a/spec/pause/redis/adapter_spec.rb b/spec/pause/redis/adapter_spec.rb index bd51782..888b81b 100644 --- a/spec/pause/redis/adapter_spec.rb +++ b/spec/pause/redis/adapter_spec.rb @@ -9,8 +9,9 @@ let(:configuration) { Pause::Configuration.new } before do - Pause.config.stub(:resolution).and_return(resolution) - Pause.config.stub(:history).and_return(history) + allow(Pause).to receive(:config).and_return(configuration) + allow(Pause.config).to receive(:resolution).and_return(resolution) + allow(Pause.config).to receive(:history).and_return(history) redis_conn.flushall end @@ -23,14 +24,14 @@ it "should add key to a redis set" do adapter.increment(key, Time.now.to_i) set = redis_conn.zrange(adapter.send(:white_key, key), 0, -1, :with_scores => true) - set.should_not be_empty - set.size.should eql(1) - set[0].size.should eql(2) + expect(set).to_not be_empty + expect(set.size).to eql(1) + expect(set[0].size).to eql(2) end it "should remove old key from a redis set" do time = Time.now - redis_conn.should_receive(:zrem).with(adapter.send(:white_key, key), [adapter.period_marker(resolution, time)]) + expect(redis_conn).to receive(:zrem).with(adapter.send(:white_key, key), [adapter.period_marker(resolution, time)]) adapter.time_blocks_to_keep = 1 Timecop.freeze time do @@ -42,7 +43,7 @@ end it "sets expiry on key" do - redis_conn.should_receive(:expire).with(adapter.send(:white_key, key), history) + expect(redis_conn).to receive(:expire).with(adapter.send(:white_key, key), history) adapter.increment(key, Time.now.to_i) end end @@ -54,8 +55,8 @@ it "saves ip to redis with expiration" do adapter.rate_limit!(key, ttl) - redis_conn.get(blocked_key).should_not be_nil - redis_conn.ttl(blocked_key).should == ttl + expect(redis_conn.get(blocked_key)).to_not be_nil + expect(redis_conn.ttl(blocked_key)).to eq(ttl) end end @@ -66,38 +67,38 @@ it "should return true if blocked" do adapter.rate_limit!(key, ttl) - (!!redis_conn.get(blocked_key).should) == adapter.rate_limited?(key) + expect(redis_conn.get(blocked_key) ? true : false).to eq(adapter.rate_limited?(key)) end end describe "#white_key" do it "prefixes key" do - adapter.send(:white_key, "abc").should == "i:abc" + expect(adapter.send(:white_key, "abc")).to eq("i:abc") end end describe '#enable' do it 'deletes the disabled flag in redis' do adapter.disable("boom") - expect(adapter.disabled?("boom")).to be_true + expect(adapter.disabled?("boom")).to eq(true) adapter.enable("boom") - expect(adapter.disabled?("boom")).to be_false + expect(adapter.disabled?("boom")).to eq(false) end end describe '#disable' do it 'sets the disabled flag in redis' do - expect(adapter.enabled?("boom")).to be_true + expect(adapter.enabled?("boom")).to eq(true) adapter.disable("boom") - expect(adapter.enabled?("boom")).to be_false + expect(adapter.enabled?("boom")).to eq(false) end end describe '#rate_limit!' do it 'rate limits a key for a specific ttl' do - expect(adapter.rate_limited?('1')).to be_false + expect(adapter.rate_limited?('1')).to eq(false) adapter.rate_limit!('1', 10) - expect(adapter.rate_limited?('1')).to be_true + expect(adapter.rate_limited?('1')).to eq(true) end end @@ -106,13 +107,13 @@ adapter.rate_limit!('boom:1', 10) adapter.rate_limit!('boom:2', 10) - expect(adapter.rate_limited?('boom:1')).to be_true - expect(adapter.rate_limited?('boom:2')).to be_true + expect(adapter.rate_limited?('boom:1')).to eq(true) + expect(adapter.rate_limited?('boom:2')).to eq(true) adapter.delete_rate_limited_keys('boom') - expect(adapter.rate_limited?('boom:1')).to be_false - expect(adapter.rate_limited?('boom:2')).to be_false + expect(adapter.rate_limited?('boom:1')).to eq(false) + expect(adapter.rate_limited?('boom:2')).to eq(false) end end @@ -121,13 +122,13 @@ adapter.rate_limit!('boom:1', 10) adapter.rate_limit!('boom:2', 10) - expect(adapter.rate_limited?('boom:1')).to be_true - expect(adapter.rate_limited?('boom:2')).to be_true + expect(adapter.rate_limited?('boom:1')).to eq(true) + expect(adapter.rate_limited?('boom:2')).to eq(true) adapter.delete_rate_limited_key('boom', '1') - expect(adapter.rate_limited?('boom:1')).to be_false - expect(adapter.rate_limited?('boom:2')).to be_true + expect(adapter.rate_limited?('boom:1')).to eq(false) + expect(adapter.rate_limited?('boom:2')).to eq(true) end end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 880ad5e..596d04b 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -12,7 +12,6 @@ require 'support/fakeredis' RSpec.configure do |config| - config.treat_symbols_as_metadata_keys_with_true_values = true config.run_all_when_everything_filtered = true config.filter_run :focus