From 79de0d53e1a333f2c469d24008d68d229a1ae037 Mon Sep 17 00:00:00 2001 From: Gonzalo Rodriguez Date: Fri, 22 Jun 2018 17:30:06 -0300 Subject: [PATCH 1/7] Only require dalli when running dalli appraisal --- rack-attack.gemspec | 1 - .../stores/active_support_dalli_store_spec.rb | 58 +++++++++--------- .../active_support_mem_cache_store_spec.rb | 56 ++++++++--------- .../connection_pool_dalli_client_spec.rb | 60 ++++++++++--------- spec/acceptance/stores/dalli_client_spec.rb | 58 +++++++++--------- spec/integration/offline_spec.rb | 21 +++---- spec/spec_helper.rb | 5 ++ 7 files changed, 136 insertions(+), 123 deletions(-) diff --git a/rack-attack.gemspec b/rack-attack.gemspec index 9a724f7..588024e 100644 --- a/rack-attack.gemspec +++ b/rack-attack.gemspec @@ -49,6 +49,5 @@ Gem::Specification.new do |s| s.add_development_dependency 'actionpack', '>= 3.0.0' s.add_development_dependency 'activesupport', '>= 3.0.0' s.add_development_dependency 'connection_pool' - s.add_development_dependency 'dalli' s.add_development_dependency 'redis-activesupport' end diff --git a/spec/acceptance/stores/active_support_dalli_store_spec.rb b/spec/acceptance/stores/active_support_dalli_store_spec.rb index b3be95f..440a860 100644 --- a/spec/acceptance/stores/active_support_dalli_store_spec.rb +++ b/spec/acceptance/stores/active_support_dalli_store_spec.rb @@ -1,39 +1,41 @@ require_relative "../../spec_helper" -require_relative "../../support/cache_store_helper" -require "active_support/cache/dalli_store" -require "timecop" +if defined?(::Dalli) + require_relative "../../support/cache_store_helper" + require "active_support/cache/dalli_store" + require "timecop" -describe "ActiveSupport::Cache::DalliStore as a cache backend" do - before do - Rack::Attack.cache.store = ActiveSupport::Cache::DalliStore.new - end - - after do - Rack::Attack.cache.store.clear - end - - it_works_for_cache_backed_features - - it "doesn't leak keys" do - Rack::Attack.throttle("by ip", limit: 1, period: 1) do |request| - request.ip + describe "ActiveSupport::Cache::DalliStore as a cache backend" do + before do + Rack::Attack.cache.store = ActiveSupport::Cache::DalliStore.new end - key = nil - - # Freeze time during these statement to be sure that the key used by rack attack is the same - # we pre-calculate in local variable `key` - Timecop.freeze do - key = "rack::attack:#{Time.now.to_i}:by ip:1.2.3.4" - - get "/", {}, "REMOTE_ADDR" => "1.2.3.4" + after do + Rack::Attack.cache.store.clear end - assert Rack::Attack.cache.store.fetch(key) + it_works_for_cache_backed_features - sleep 2.1 + it "doesn't leak keys" do + Rack::Attack.throttle("by ip", limit: 1, period: 1) do |request| + request.ip + end - assert_nil Rack::Attack.cache.store.fetch(key) + key = nil + + # Freeze time during these statement to be sure that the key used by rack attack is the same + # we pre-calculate in local variable `key` + Timecop.freeze do + key = "rack::attack:#{Time.now.to_i}:by ip:1.2.3.4" + + get "/", {}, "REMOTE_ADDR" => "1.2.3.4" + end + + assert Rack::Attack.cache.store.fetch(key) + + sleep 2.1 + + assert_nil Rack::Attack.cache.store.fetch(key) + end end end diff --git a/spec/acceptance/stores/active_support_mem_cache_store_spec.rb b/spec/acceptance/stores/active_support_mem_cache_store_spec.rb index 1a4e7f7..4a3ced6 100644 --- a/spec/acceptance/stores/active_support_mem_cache_store_spec.rb +++ b/spec/acceptance/stores/active_support_mem_cache_store_spec.rb @@ -1,38 +1,40 @@ require_relative "../../spec_helper" -require_relative "../../support/cache_store_helper" -require "timecop" +if defined?(::Dalli) + require_relative "../../support/cache_store_helper" + require "timecop" -describe "ActiveSupport::Cache::MemCacheStore as a cache backend" do - before do - Rack::Attack.cache.store = ActiveSupport::Cache::MemCacheStore.new - end - - after do - Rack::Attack.cache.store.flush_all - end - - it_works_for_cache_backed_features - - it "doesn't leak keys" do - Rack::Attack.throttle("by ip", limit: 1, period: 1) do |request| - request.ip + describe "ActiveSupport::Cache::MemCacheStore as a cache backend" do + before do + Rack::Attack.cache.store = ActiveSupport::Cache::MemCacheStore.new end - key = nil - - # Freeze time during these statement to be sure that the key used by rack attack is the same - # we pre-calculate in local variable `key` - Timecop.freeze do - key = "rack::attack:#{Time.now.to_i}:by ip:1.2.3.4" - - get "/", {}, "REMOTE_ADDR" => "1.2.3.4" + after do + Rack::Attack.cache.store.flush_all end - assert Rack::Attack.cache.store.get(key) + it_works_for_cache_backed_features - sleep 2.1 + it "doesn't leak keys" do + Rack::Attack.throttle("by ip", limit: 1, period: 1) do |request| + request.ip + end - assert_nil Rack::Attack.cache.store.get(key) + key = nil + + # Freeze time during these statement to be sure that the key used by rack attack is the same + # we pre-calculate in local variable `key` + Timecop.freeze do + key = "rack::attack:#{Time.now.to_i}:by ip:1.2.3.4" + + get "/", {}, "REMOTE_ADDR" => "1.2.3.4" + end + + assert Rack::Attack.cache.store.get(key) + + sleep 2.1 + + assert_nil Rack::Attack.cache.store.get(key) + end end end diff --git a/spec/acceptance/stores/connection_pool_dalli_client_spec.rb b/spec/acceptance/stores/connection_pool_dalli_client_spec.rb index a852d03..bb8bf66 100644 --- a/spec/acceptance/stores/connection_pool_dalli_client_spec.rb +++ b/spec/acceptance/stores/connection_pool_dalli_client_spec.rb @@ -1,40 +1,42 @@ require_relative "../../spec_helper" -require_relative "../../support/cache_store_helper" -require "connection_pool" -require "dalli" -require "timecop" +if defined?(::Dalli) + require_relative "../../support/cache_store_helper" + require "connection_pool" + require "dalli" + require "timecop" -describe "ConnectionPool with Dalli::Client as a cache backend" do - before do - Rack::Attack.cache.store = ConnectionPool.new { Dalli::Client.new } - end - - after do - Rack::Attack.cache.store.with { |client| client.flush_all } - end - - it_works_for_cache_backed_features - - it "doesn't leak keys" do - Rack::Attack.throttle("by ip", limit: 1, period: 1) do |request| - request.ip + describe "ConnectionPool with Dalli::Client as a cache backend" do + before do + Rack::Attack.cache.store = ConnectionPool.new { Dalli::Client.new } end - key = nil - - # Freeze time during these statement to be sure that the key used by rack attack is the same - # we pre-calculate in local variable `key` - Timecop.freeze do - key = "rack::attack:#{Time.now.to_i}:by ip:1.2.3.4" - - get "/", {}, "REMOTE_ADDR" => "1.2.3.4" + after do + Rack::Attack.cache.store.with { |client| client.flush_all } end - assert(Rack::Attack.cache.store.with { |client| client.fetch(key) }) + it_works_for_cache_backed_features - sleep 2.1 + it "doesn't leak keys" do + Rack::Attack.throttle("by ip", limit: 1, period: 1) do |request| + request.ip + end - assert_nil(Rack::Attack.cache.store.with { |client| client.fetch(key) }) + key = nil + + # Freeze time during these statement to be sure that the key used by rack attack is the same + # we pre-calculate in local variable `key` + Timecop.freeze do + key = "rack::attack:#{Time.now.to_i}:by ip:1.2.3.4" + + get "/", {}, "REMOTE_ADDR" => "1.2.3.4" + end + + assert(Rack::Attack.cache.store.with { |client| client.fetch(key) }) + + sleep 2.1 + + assert_nil(Rack::Attack.cache.store.with { |client| client.fetch(key) }) + end end end diff --git a/spec/acceptance/stores/dalli_client_spec.rb b/spec/acceptance/stores/dalli_client_spec.rb index e01ab3f..48a2d9f 100644 --- a/spec/acceptance/stores/dalli_client_spec.rb +++ b/spec/acceptance/stores/dalli_client_spec.rb @@ -1,39 +1,41 @@ require_relative "../../spec_helper" -require_relative "../../support/cache_store_helper" -require "dalli" -require "timecop" +if defined?(::Dalli) + require_relative "../../support/cache_store_helper" + require "dalli" + require "timecop" -describe "Dalli::Client as a cache backend" do - before do - Rack::Attack.cache.store = Dalli::Client.new - end - - after do - Rack::Attack.cache.store.flush_all - end - - it_works_for_cache_backed_features - - it "doesn't leak keys" do - Rack::Attack.throttle("by ip", limit: 1, period: 1) do |request| - request.ip + describe "Dalli::Client as a cache backend" do + before do + Rack::Attack.cache.store = Dalli::Client.new end - key = nil - - # Freeze time during these statement to be sure that the key used by rack attack is the same - # we pre-calculate in local variable `key` - Timecop.freeze do - key = "rack::attack:#{Time.now.to_i}:by ip:1.2.3.4" - - get "/", {}, "REMOTE_ADDR" => "1.2.3.4" + after do + Rack::Attack.cache.store.flush_all end - assert Rack::Attack.cache.store.fetch(key) + it_works_for_cache_backed_features - sleep 2.1 + it "doesn't leak keys" do + Rack::Attack.throttle("by ip", limit: 1, period: 1) do |request| + request.ip + end - assert_nil Rack::Attack.cache.store.fetch(key) + key = nil + + # Freeze time during these statement to be sure that the key used by rack attack is the same + # we pre-calculate in local variable `key` + Timecop.freeze do + key = "rack::attack:#{Time.now.to_i}:by ip:1.2.3.4" + + get "/", {}, "REMOTE_ADDR" => "1.2.3.4" + end + + assert Rack::Attack.cache.store.fetch(key) + + sleep 2.1 + + assert_nil Rack::Attack.cache.store.fetch(key) + end end end diff --git a/spec/integration/offline_spec.rb b/spec/integration/offline_spec.rb index ebaf16a..8d1f670 100644 --- a/spec/integration/offline_spec.rb +++ b/spec/integration/offline_spec.rb @@ -1,6 +1,5 @@ require 'active_support/cache' require 'redis-activesupport' -require 'dalli' require_relative '../spec_helper' OfflineExamples = Minitest::SharedExamples.new do @@ -27,17 +26,19 @@ describe 'when Redis is offline' do end end -describe 'when Memcached is offline' do - include OfflineExamples +if defined?(::Dalli) + describe 'when Memcached is offline' do + include OfflineExamples - before do - Dalli.logger.level = Logger::FATAL + before do + Dalli.logger.level = Logger::FATAL - @cache = Rack::Attack::Cache.new - @cache.store = Dalli::Client.new('127.0.0.1:22122') - end + @cache = Rack::Attack::Cache.new + @cache.store = Dalli::Client.new('127.0.0.1:22122') + end - after do - Dalli.logger.level = Logger::INFO + after do + Dalli.logger.level = Logger::INFO + end end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 7d2c851..71ff76b 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -13,6 +13,11 @@ if RUBY_ENGINE == "ruby" require "byebug" end +begin + require "dalli" +rescue LoadError +end + class MiniTest::Spec include Rack::Test::Methods From bd2ade8977a284f2d419a15defb55d02e2a5e13b Mon Sep 17 00:00:00 2001 From: Gonzalo Rodriguez Date: Fri, 22 Jun 2018 17:45:58 -0300 Subject: [PATCH 2/7] Only require connection_pool running connection_pool appraisal --- Appraisals | 5 ++ gemfiles/connection_pool_dalli.gemfile | 8 +++ rack-attack.gemspec | 1 - ...e_support_redis_cache_store_pooled_spec.rb | 58 ++++++++++--------- .../connection_pool_dalli_client_spec.rb | 2 +- spec/spec_helper.rb | 5 ++ 6 files changed, 49 insertions(+), 30 deletions(-) create mode 100644 gemfiles/connection_pool_dalli.gemfile diff --git a/Appraisals b/Appraisals index 2ccb781..55b47f0 100644 --- a/Appraisals +++ b/Appraisals @@ -32,3 +32,8 @@ end appraise 'dalli2' do gem 'dalli', '~> 2.0' end + +appraise "connection_pool_dalli" do + gem "connection_pool", "~> 2.2" + gem "dalli", "~> 2.7" +end diff --git a/gemfiles/connection_pool_dalli.gemfile b/gemfiles/connection_pool_dalli.gemfile new file mode 100644 index 0000000..69dc887 --- /dev/null +++ b/gemfiles/connection_pool_dalli.gemfile @@ -0,0 +1,8 @@ +# This file was generated by Appraisal + +source "https://rubygems.org" + +gem "connection_pool", "~> 2.2" +gem "dalli", "~> 2.7" + +gemspec path: "../" diff --git a/rack-attack.gemspec b/rack-attack.gemspec index 588024e..59ba7df 100644 --- a/rack-attack.gemspec +++ b/rack-attack.gemspec @@ -48,6 +48,5 @@ Gem::Specification.new do |s| # which rack-attack uses only for testing compatibility in test suite. s.add_development_dependency 'actionpack', '>= 3.0.0' s.add_development_dependency 'activesupport', '>= 3.0.0' - s.add_development_dependency 'connection_pool' s.add_development_dependency 'redis-activesupport' end diff --git a/spec/acceptance/stores/active_support_redis_cache_store_pooled_spec.rb b/spec/acceptance/stores/active_support_redis_cache_store_pooled_spec.rb index 8c006bb..3447981 100644 --- a/spec/acceptance/stores/active_support_redis_cache_store_pooled_spec.rb +++ b/spec/acceptance/stores/active_support_redis_cache_store_pooled_spec.rb @@ -1,40 +1,42 @@ require_relative "../../spec_helper" -require_relative "../../support/cache_store_helper" -require "timecop" +if defined?(::ConnectionPool) + require_relative "../../support/cache_store_helper" + require "timecop" -if ActiveSupport.version >= Gem::Version.new("5.2.0") - describe "ActiveSupport::Cache::RedisCacheStore (pooled) as a cache backend" do - before do - Rack::Attack.cache.store = ActiveSupport::Cache::RedisCacheStore.new(pool_size: 2) - end - - after do - Rack::Attack.cache.store.clear - end - - it_works_for_cache_backed_features - - it "doesn't leak keys" do - Rack::Attack.throttle("by ip", limit: 1, period: 1) do |request| - request.ip + if ActiveSupport.version >= Gem::Version.new("5.2.0") + describe "ActiveSupport::Cache::RedisCacheStore (pooled) as a cache backend" do + before do + Rack::Attack.cache.store = ActiveSupport::Cache::RedisCacheStore.new(pool_size: 2) end - key = nil - - # Freeze time during these statement to be sure that the key used by rack attack is the same - # we pre-calculate in local variable `key` - Timecop.freeze do - key = "rack::attack:#{Time.now.to_i}:by ip:1.2.3.4" - - get "/", {}, "REMOTE_ADDR" => "1.2.3.4" + after do + Rack::Attack.cache.store.clear end - assert Rack::Attack.cache.store.fetch(key) + it_works_for_cache_backed_features - sleep 2.1 + it "doesn't leak keys" do + Rack::Attack.throttle("by ip", limit: 1, period: 1) do |request| + request.ip + end - assert_nil Rack::Attack.cache.store.fetch(key) + key = nil + + # Freeze time during these statement to be sure that the key used by rack attack is the same + # we pre-calculate in local variable `key` + Timecop.freeze do + key = "rack::attack:#{Time.now.to_i}:by ip:1.2.3.4" + + get "/", {}, "REMOTE_ADDR" => "1.2.3.4" + end + + assert Rack::Attack.cache.store.fetch(key) + + sleep 2.1 + + assert_nil Rack::Attack.cache.store.fetch(key) + end end end end diff --git a/spec/acceptance/stores/connection_pool_dalli_client_spec.rb b/spec/acceptance/stores/connection_pool_dalli_client_spec.rb index bb8bf66..15f4564 100644 --- a/spec/acceptance/stores/connection_pool_dalli_client_spec.rb +++ b/spec/acceptance/stores/connection_pool_dalli_client_spec.rb @@ -1,6 +1,6 @@ require_relative "../../spec_helper" -if defined?(::Dalli) +if defined?(::Dalli) && defined?(::ConnectionPool) require_relative "../../support/cache_store_helper" require "connection_pool" require "dalli" diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 71ff76b..4bac5f8 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -18,6 +18,11 @@ begin rescue LoadError end +begin + require "connection_pool" +rescue LoadError +end + class MiniTest::Spec include Rack::Test::Methods From a72bfb5fc782b69afdebfc5bd92871056a181a1b Mon Sep 17 00:00:00 2001 From: Gonzalo Rodriguez Date: Fri, 22 Jun 2018 18:55:27 -0300 Subject: [PATCH 3/7] Only require redis stores when running their respective appraisal --- Appraisals | 19 +++++++ .../active_support_redis_cache_store.gemfile | 8 +++ ...e_support_redis_cache_store_pooled.gemfile | 9 +++ gemfiles/active_support_redis_store.gemfile | 7 +++ gemfiles/redis_store.gemfile | 7 +++ rack-attack.gemspec | 1 - ...e_support_redis_cache_store_pooled_spec.rb | 54 +++++++++--------- .../active_support_redis_cache_store_spec.rb | 6 +- .../stores/active_support_redis_store_spec.rb | 57 ++++++++++--------- spec/acceptance/stores/redis_store_spec.rb | 55 +++++++++--------- spec/integration/offline_spec.rb | 15 ++--- spec/spec_helper.rb | 15 +++++ 12 files changed, 159 insertions(+), 94 deletions(-) create mode 100644 gemfiles/active_support_redis_cache_store.gemfile create mode 100644 gemfiles/active_support_redis_cache_store_pooled.gemfile create mode 100644 gemfiles/active_support_redis_store.gemfile create mode 100644 gemfiles/redis_store.gemfile diff --git a/Appraisals b/Appraisals index 55b47f0..e9d2e21 100644 --- a/Appraisals +++ b/Appraisals @@ -37,3 +37,22 @@ appraise "connection_pool_dalli" do gem "connection_pool", "~> 2.2" gem "dalli", "~> 2.7" end + +appraise "active_support_redis_cache_store" do + gem "activesupport", "~> 5.2.0" + gem "redis", "~> 4.0" +end + +appraise "active_support_redis_cache_store_pooled" do + gem "activesupport", "~> 5.2.0" + gem "connection_pool", "~> 2.2" + gem "redis", "~> 4.0" +end + +appraise "redis_store" do + gem "redis-store", "~> 1.5" +end + +appraise "active_support_redis_store" do + gem "redis-activesupport", "~> 5.0" +end diff --git a/gemfiles/active_support_redis_cache_store.gemfile b/gemfiles/active_support_redis_cache_store.gemfile new file mode 100644 index 0000000..30e1e38 --- /dev/null +++ b/gemfiles/active_support_redis_cache_store.gemfile @@ -0,0 +1,8 @@ +# This file was generated by Appraisal + +source "https://rubygems.org" + +gem "activesupport", "~> 5.2.0" +gem "redis", "~> 4.0" + +gemspec path: "../" diff --git a/gemfiles/active_support_redis_cache_store_pooled.gemfile b/gemfiles/active_support_redis_cache_store_pooled.gemfile new file mode 100644 index 0000000..9232a9b --- /dev/null +++ b/gemfiles/active_support_redis_cache_store_pooled.gemfile @@ -0,0 +1,9 @@ +# This file was generated by Appraisal + +source "https://rubygems.org" + +gem "activesupport", "~> 5.2.0" +gem "connection_pool", "~> 2.2" +gem "redis", "~> 4.0" + +gemspec path: "../" diff --git a/gemfiles/active_support_redis_store.gemfile b/gemfiles/active_support_redis_store.gemfile new file mode 100644 index 0000000..517c70f --- /dev/null +++ b/gemfiles/active_support_redis_store.gemfile @@ -0,0 +1,7 @@ +# This file was generated by Appraisal + +source "https://rubygems.org" + +gem "redis-activesupport", "~> 5.0" + +gemspec path: "../" diff --git a/gemfiles/redis_store.gemfile b/gemfiles/redis_store.gemfile new file mode 100644 index 0000000..8aafc6d --- /dev/null +++ b/gemfiles/redis_store.gemfile @@ -0,0 +1,7 @@ +# This file was generated by Appraisal + +source "https://rubygems.org" + +gem "redis-store", "~> 1.5" + +gemspec path: "../" diff --git a/rack-attack.gemspec b/rack-attack.gemspec index 59ba7df..c1180c7 100644 --- a/rack-attack.gemspec +++ b/rack-attack.gemspec @@ -48,5 +48,4 @@ Gem::Specification.new do |s| # which rack-attack uses only for testing compatibility in test suite. s.add_development_dependency 'actionpack', '>= 3.0.0' s.add_development_dependency 'activesupport', '>= 3.0.0' - s.add_development_dependency 'redis-activesupport' end diff --git a/spec/acceptance/stores/active_support_redis_cache_store_pooled_spec.rb b/spec/acceptance/stores/active_support_redis_cache_store_pooled_spec.rb index 3447981..5295ba5 100644 --- a/spec/acceptance/stores/active_support_redis_cache_store_pooled_spec.rb +++ b/spec/acceptance/stores/active_support_redis_cache_store_pooled_spec.rb @@ -1,42 +1,40 @@ require_relative "../../spec_helper" -if defined?(::ConnectionPool) +if defined?(::ConnectionPool) && defined?(::Redis) && defined?(::ActiveSupport::Cache::RedisCacheStore) require_relative "../../support/cache_store_helper" require "timecop" - if ActiveSupport.version >= Gem::Version.new("5.2.0") - describe "ActiveSupport::Cache::RedisCacheStore (pooled) as a cache backend" do - before do - Rack::Attack.cache.store = ActiveSupport::Cache::RedisCacheStore.new(pool_size: 2) + describe "ActiveSupport::Cache::RedisCacheStore (pooled) as a cache backend" do + before do + Rack::Attack.cache.store = ActiveSupport::Cache::RedisCacheStore.new(pool_size: 2) + end + + after do + Rack::Attack.cache.store.clear + end + + it_works_for_cache_backed_features + + it "doesn't leak keys" do + Rack::Attack.throttle("by ip", limit: 1, period: 1) do |request| + request.ip end - after do - Rack::Attack.cache.store.clear + key = nil + + # Freeze time during these statement to be sure that the key used by rack attack is the same + # we pre-calculate in local variable `key` + Timecop.freeze do + key = "rack::attack:#{Time.now.to_i}:by ip:1.2.3.4" + + get "/", {}, "REMOTE_ADDR" => "1.2.3.4" end - it_works_for_cache_backed_features + assert Rack::Attack.cache.store.fetch(key) - it "doesn't leak keys" do - Rack::Attack.throttle("by ip", limit: 1, period: 1) do |request| - request.ip - end + sleep 2.1 - key = nil - - # Freeze time during these statement to be sure that the key used by rack attack is the same - # we pre-calculate in local variable `key` - Timecop.freeze do - key = "rack::attack:#{Time.now.to_i}:by ip:1.2.3.4" - - get "/", {}, "REMOTE_ADDR" => "1.2.3.4" - end - - assert Rack::Attack.cache.store.fetch(key) - - sleep 2.1 - - assert_nil Rack::Attack.cache.store.fetch(key) - end + assert_nil Rack::Attack.cache.store.fetch(key) end end end diff --git a/spec/acceptance/stores/active_support_redis_cache_store_spec.rb b/spec/acceptance/stores/active_support_redis_cache_store_spec.rb index 495a6a2..e6ecdda 100644 --- a/spec/acceptance/stores/active_support_redis_cache_store_spec.rb +++ b/spec/acceptance/stores/active_support_redis_cache_store_spec.rb @@ -1,9 +1,9 @@ require_relative "../../spec_helper" -require_relative "../../support/cache_store_helper" -require "timecop" +if defined?(::Redis) && defined?(::ActiveSupport::Cache::RedisCacheStore) + require_relative "../../support/cache_store_helper" + require "timecop" -if ActiveSupport.version >= Gem::Version.new("5.2.0") describe "ActiveSupport::Cache::RedisCacheStore as a cache backend" do before do Rack::Attack.cache.store = ActiveSupport::Cache::RedisCacheStore.new diff --git a/spec/acceptance/stores/active_support_redis_store_spec.rb b/spec/acceptance/stores/active_support_redis_store_spec.rb index 0b446e4..840a7d4 100644 --- a/spec/acceptance/stores/active_support_redis_store_spec.rb +++ b/spec/acceptance/stores/active_support_redis_store_spec.rb @@ -1,39 +1,40 @@ require_relative "../../spec_helper" -require_relative "../../support/cache_store_helper" -require "redis-activesupport" -require "timecop" +if defined?(::ActiveSupport::Cache::RedisStore) + require_relative "../../support/cache_store_helper" + require "timecop" -describe "ActiveSupport::Cache::RedisStore as a cache backend" do - before do - Rack::Attack.cache.store = ActiveSupport::Cache::RedisStore.new - end - - after do - Rack::Attack.cache.store.flushdb - end - - it_works_for_cache_backed_features - - it "doesn't leak keys" do - Rack::Attack.throttle("by ip", limit: 1, period: 1) do |request| - request.ip + describe "ActiveSupport::Cache::RedisStore as a cache backend" do + before do + Rack::Attack.cache.store = ActiveSupport::Cache::RedisStore.new end - key = nil - - # Freeze time during these statement to be sure that the key used by rack attack is the same - # we pre-calculate in local variable `key` - Timecop.freeze do - key = "rack::attack:#{Time.now.to_i}:by ip:1.2.3.4" - - get "/", {}, "REMOTE_ADDR" => "1.2.3.4" + after do + Rack::Attack.cache.store.flushdb end - assert Rack::Attack.cache.store.read(key) + it_works_for_cache_backed_features - sleep 2.1 + it "doesn't leak keys" do + Rack::Attack.throttle("by ip", limit: 1, period: 1) do |request| + request.ip + end - assert_nil Rack::Attack.cache.store.read(key) + key = nil + + # Freeze time during these statement to be sure that the key used by rack attack is the same + # we pre-calculate in local variable `key` + Timecop.freeze do + key = "rack::attack:#{Time.now.to_i}:by ip:1.2.3.4" + + get "/", {}, "REMOTE_ADDR" => "1.2.3.4" + end + + assert Rack::Attack.cache.store.read(key) + + sleep 2.1 + + assert_nil Rack::Attack.cache.store.read(key) + end end end diff --git a/spec/acceptance/stores/redis_store_spec.rb b/spec/acceptance/stores/redis_store_spec.rb index 17e5a0a..9c9fba5 100644 --- a/spec/acceptance/stores/redis_store_spec.rb +++ b/spec/acceptance/stores/redis_store_spec.rb @@ -1,39 +1,40 @@ require_relative "../../spec_helper" require_relative "../../support/cache_store_helper" -require "redis-store" -require "timecop" +if defined?(::Redis::Store) + require "timecop" -describe "ActiveSupport::Cache::RedisStore as a cache backend" do - before do - Rack::Attack.cache.store = ::Redis::Store.new - end - - after do - Rack::Attack.cache.store.flushdb - end - - it_works_for_cache_backed_features - - it "doesn't leak keys" do - Rack::Attack.throttle("by ip", limit: 1, period: 1) do |request| - request.ip + describe "ActiveSupport::Cache::RedisStore as a cache backend" do + before do + Rack::Attack.cache.store = ::Redis::Store.new end - key = nil - - # Freeze time during these statement to be sure that the key used by rack attack is the same - # we pre-calculate in local variable `key` - Timecop.freeze do - key = "rack::attack:#{Time.now.to_i}:by ip:1.2.3.4" - - get "/", {}, "REMOTE_ADDR" => "1.2.3.4" + after do + Rack::Attack.cache.store.flushdb end - assert Rack::Attack.cache.store.read(key) + it_works_for_cache_backed_features - sleep 2.1 + it "doesn't leak keys" do + Rack::Attack.throttle("by ip", limit: 1, period: 1) do |request| + request.ip + end - assert_nil Rack::Attack.cache.store.read(key) + key = nil + + # Freeze time during these statement to be sure that the key used by rack attack is the same + # we pre-calculate in local variable `key` + Timecop.freeze do + key = "rack::attack:#{Time.now.to_i}:by ip:1.2.3.4" + + get "/", {}, "REMOTE_ADDR" => "1.2.3.4" + end + + assert Rack::Attack.cache.store.read(key) + + sleep 2.1 + + assert_nil Rack::Attack.cache.store.read(key) + end end end diff --git a/spec/integration/offline_spec.rb b/spec/integration/offline_spec.rb index 8d1f670..6a6e925 100644 --- a/spec/integration/offline_spec.rb +++ b/spec/integration/offline_spec.rb @@ -1,5 +1,4 @@ require 'active_support/cache' -require 'redis-activesupport' require_relative '../spec_helper' OfflineExamples = Minitest::SharedExamples.new do @@ -16,13 +15,15 @@ OfflineExamples = Minitest::SharedExamples.new do end end -describe 'when Redis is offline' do - include OfflineExamples +if defined?(::ActiveSupport::Cache::RedisStore) + describe 'when Redis is offline' do + include OfflineExamples - before do - @cache = Rack::Attack::Cache.new - # Use presumably unused port for Redis client - @cache.store = ActiveSupport::Cache::RedisStore.new(:host => '127.0.0.1', :port => 3333) + before do + @cache = Rack::Attack::Cache.new + # Use presumably unused port for Redis client + @cache.store = ActiveSupport::Cache::RedisStore.new(:host => '127.0.0.1', :port => 3333) + end end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 4bac5f8..420f193 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -23,6 +23,21 @@ begin rescue LoadError end +begin + require "redis" +rescue LoadError +end + +begin + require "redis-activesupport" +rescue LoadError +end + +begin + require "redis-store" +rescue LoadError +end + class MiniTest::Spec include Rack::Test::Methods From a1ea2f9aefdec5414660e05216e0609711a8247d Mon Sep 17 00:00:00 2001 From: Gonzalo Rodriguez Date: Fri, 22 Jun 2018 19:04:47 -0300 Subject: [PATCH 4/7] Avoid repetition in spec_helper --- spec/spec_helper.rb | 32 ++++++++++---------------------- 1 file changed, 10 insertions(+), 22 deletions(-) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 420f193..20c6d47 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -13,30 +13,18 @@ if RUBY_ENGINE == "ruby" require "byebug" end -begin - require "dalli" -rescue LoadError +def safe_require(name) + begin + require name + rescue LoadError + end end -begin - require "connection_pool" -rescue LoadError -end - -begin - require "redis" -rescue LoadError -end - -begin - require "redis-activesupport" -rescue LoadError -end - -begin - require "redis-store" -rescue LoadError -end +safe_require "connection_pool" +safe_require "dalli" +safe_require "redis" +safe_require "redis-activesupport" +safe_require "redis-store" class MiniTest::Spec include Rack::Test::Methods From 5d48addd6ebc91817298e79d1033ac7df95d28c2 Mon Sep 17 00:00:00 2001 From: Gonzalo Rodriguez Date: Fri, 22 Jun 2018 19:20:46 -0300 Subject: [PATCH 5/7] Make TravisCI run new appraisal scenarios --- .travis.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.travis.yml b/.travis.yml index 612ece6..24a4474 100644 --- a/.travis.yml +++ b/.travis.yml @@ -27,6 +27,11 @@ gemfile: - gemfiles/rails_5_1.gemfile - gemfiles/rails_4_2.gemfile - gemfiles/dalli2.gemfile + - gemfiles/active_support_redis_cache_store + - gemfiles/active_support_redis_cache_store_pooled + - gemfiles/active_support_redis_store + - gemfiles/connection_pool_dalli + - gemfiles/redis_store matrix: allow_failures: From e3213ee746f61432f8ba99ec42679e1d8df8785e Mon Sep 17 00:00:00 2001 From: Gonzalo Rodriguez Date: Mon, 25 Jun 2018 16:15:02 -0300 Subject: [PATCH 6/7] Fix .travis.yml gemfile paths --- .travis.yml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/.travis.yml b/.travis.yml index 24a4474..7cce90c 100644 --- a/.travis.yml +++ b/.travis.yml @@ -27,11 +27,11 @@ gemfile: - gemfiles/rails_5_1.gemfile - gemfiles/rails_4_2.gemfile - gemfiles/dalli2.gemfile - - gemfiles/active_support_redis_cache_store - - gemfiles/active_support_redis_cache_store_pooled - - gemfiles/active_support_redis_store - - gemfiles/connection_pool_dalli - - gemfiles/redis_store + - gemfiles/connection_pool_dalli.gemfile + - gemfiles/active_support_redis_cache_store.gemfile + - gemfiles/active_support_redis_cache_store_pooled.gemfile + - gemfiles/redis_store.gemfile + - gemfiles/active_support_redis_store.gemfile matrix: allow_failures: From ee84079768b619784a8e9b98084f263173ed10eb Mon Sep 17 00:00:00 2001 From: Gonzalo Rodriguez Date: Fri, 22 Jun 2018 19:10:35 -0300 Subject: [PATCH 7/7] Fix 'redis is not part of the bundle' exception when using :memory_store When RedisCacheStore constant is referenced, activesupport autoloads and rails tries to require redis, throwing exception if not present --- lib/rack/attack/store_proxy/redis_cache_store_proxy.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/rack/attack/store_proxy/redis_cache_store_proxy.rb b/lib/rack/attack/store_proxy/redis_cache_store_proxy.rb index f99de36..d6cbdfb 100644 --- a/lib/rack/attack/store_proxy/redis_cache_store_proxy.rb +++ b/lib/rack/attack/store_proxy/redis_cache_store_proxy.rb @@ -5,7 +5,7 @@ module Rack module StoreProxy class RedisCacheStoreProxy < SimpleDelegator def self.handle?(store) - defined?(::ActiveSupport::Cache::RedisCacheStore) && store.is_a?(::ActiveSupport::Cache::RedisCacheStore) + defined?(::Redis) && defined?(::ActiveSupport::Cache::RedisCacheStore) && store.is_a?(::ActiveSupport::Cache::RedisCacheStore) end def increment(name, amount = 1, options = {})