Eagerly check for store validity

lib/rack/attack/cache.rb

@@ -26,6 +26,9 @@ def store=(store)
           else
             store
           end
+        if @store
+          check_store_methods_presence(:read, :write, :delete, :increment)
+        end
       end
 
       def count(unprefixed_key, period)
@@ -34,13 +37,14 @@ def count(unprefixed_key, period)
       end
 
       def read(unprefixed_key)
-        enforce_store_presence!
-        enforce_store_method_presence!(:read)
+        raise Rack::Attack::MissingStoreError if store.nil?
 
         store.read("#{prefix}:#{unprefixed_key}")
       end
 
       def write(unprefixed_key, value, expires_in)
+        raise Rack::Attack::MissingStoreError if store.nil?
+
         store.write("#{prefix}:#{unprefixed_key}", value, expires_in: expires_in)
       end
 
@@ -74,32 +78,22 @@ def key_and_expiry(unprefixed_key, period)
       end
 
       def do_count(key, expires_in)
-        enforce_store_presence!
-        enforce_store_method_presence!(:increment)
+        raise Rack::Attack::MissingStoreError if store.nil?
 
         result = store.increment(key, 1, expires_in: expires_in)
 
         # NB: Some stores return nil when incrementing uninitialized values
         if result.nil?
-          enforce_store_method_presence!(:write)
-
           store.write(key, 1, expires_in: expires_in)
         end
         result || 1
       end
 
-      def enforce_store_presence!
-        if store.nil?
-          raise Rack::Attack::MissingStoreError
-        end
-      end
-
-      def enforce_store_method_presence!(method_name)
-        if !store.respond_to?(method_name)
-          raise(
-            Rack::Attack::MisconfiguredStoreError,
-            "Configured store #{store.class.name} doesn't respond to ##{method_name} method"
-          )
+      def check_store_methods_presence(*method_names)
+        missing = method_names.reject { |m| store.respond_to?(m) }
+        unless missing.empty?
+          missing = missing.map { |m| "##{m}" }.join(", ")
+          warn "[rack-attack] Configured store #{store.class.name} doesn't respond to #{missing}"
         end
       end
     end

spec/acceptance/cache_store_config_for_allow2ban_spec.rb

@@ -20,67 +20,14 @@
     end
   end
 
-  it "gives semantic error if store is missing #read method" do
-    raised_exception = nil
-
-    fake_store_class = Class.new do
-      def write(key, value); end
-
-      def increment(key, count, options = {}); end
+  it "display warning if store is missing methods" do
+    warning = "[rack-attack] Configured store Object doesn't respond to #read, #write, #delete, #increment\n"
+    assert_output("", warning) do
+      Rack::Attack.cache.store = Object.new
     end
-
-    Object.stub_const(:FakeStore, fake_store_class) do
-      Rack::Attack.cache.store = FakeStore.new
-
-      raised_exception = assert_raises(Rack::Attack::MisconfiguredStoreError) do
-        get "/scarce-resource"
-      end
-    end
-
-    assert_equal "Configured store FakeStore doesn't respond to #read method", raised_exception.message
   end
 
-  it "gives semantic error if store is missing #write method" do
-    raised_exception = nil
-
-    fake_store_class = Class.new do
-      def read(key); end
-
-      def increment(key, count, options = {}); end
-    end
-
-    Object.stub_const(:FakeStore, fake_store_class) do
-      Rack::Attack.cache.store = FakeStore.new
-
-      raised_exception = assert_raises(Rack::Attack::MisconfiguredStoreError) do
-        get "/scarce-resource"
-      end
-    end
-
-    assert_equal "Configured store FakeStore doesn't respond to #write method", raised_exception.message
-  end
-
-  it "gives semantic error if store is missing #increment method" do
-    raised_exception = nil
-
-    fake_store_class = Class.new do
-      def read(key); end
-
-      def write(key, value); end
-    end
-
-    Object.stub_const(:FakeStore, fake_store_class) do
-      Rack::Attack.cache.store = FakeStore.new
-
-      raised_exception = assert_raises(Rack::Attack::MisconfiguredStoreError) do
-        get "/scarce-resource"
-      end
-    end
-
-    assert_equal "Configured store FakeStore doesn't respond to #increment method", raised_exception.message
-  end
-
-  it "works with any object that responds to #read, #write and #increment" do
+  it "works with any object that responds to #read, #write, #delete and #increment" do
     fake_store_class = Class.new do
       attr_accessor :backend
 
@@ -100,6 +47,10 @@ def increment(key, _count, _options = {})
         @backend[key] ||= 0
         @backend[key] += 1
       end
+
+      def delete(key)
+        @backend.delete(key)
+      end
     end
 
     Object.stub_const(:FakeStore, fake_store_class) do

spec/acceptance/cache_store_config_for_fail2ban_spec.rb

@@ -20,67 +20,14 @@
     end
   end
 
-  it "gives semantic error if store is missing #read method" do
-    raised_exception = nil
-
-    fake_store_class = Class.new do
-      def write(key, value); end
-
-      def increment(key, count, options = {}); end
+  it "display warning if store is missing methods" do
+    warning = "[rack-attack] Configured store Object doesn't respond to #read, #write, #delete, #increment\n"
+    assert_output("", warning) do
+      Rack::Attack.cache.store = Object.new
     end
-
-    Object.stub_const(:FakeStore, fake_store_class) do
-      Rack::Attack.cache.store = FakeStore.new
-
-      raised_exception = assert_raises(Rack::Attack::MisconfiguredStoreError) do
-        get "/private-place"
-      end
-    end
-
-    assert_equal "Configured store FakeStore doesn't respond to #read method", raised_exception.message
   end
 
-  it "gives semantic error if store is missing #write method" do
-    raised_exception = nil
-
-    fake_store_class = Class.new do
-      def read(key); end
-
-      def increment(key, count, options = {}); end
-    end
-
-    Object.stub_const(:FakeStore, fake_store_class) do
-      Rack::Attack.cache.store = FakeStore.new
-
-      raised_exception = assert_raises(Rack::Attack::MisconfiguredStoreError) do
-        get "/private-place"
-      end
-    end
-
-    assert_equal "Configured store FakeStore doesn't respond to #write method", raised_exception.message
-  end
-
-  it "gives semantic error if store is missing #increment method" do
-    raised_exception = nil
-
-    fake_store_class = Class.new do
-      def read(key); end
-
-      def write(key, value); end
-    end
-
-    Object.stub_const(:FakeStore, fake_store_class) do
-      Rack::Attack.cache.store = FakeStore.new
-
-      raised_exception = assert_raises(Rack::Attack::MisconfiguredStoreError) do
-        get "/private-place"
-      end
-    end
-
-    assert_equal "Configured store FakeStore doesn't respond to #increment method", raised_exception.message
-  end
-
-  it "works with any object that responds to #read, #write and #increment" do
+  it "works with any object that responds to #read, #write, #delete and #increment" do
     fake_store_class = Class.new do
       attr_accessor :backend
 
@@ -100,6 +47,10 @@ def increment(key, _count, _options = {})
         @backend[key] ||= 0
         @backend[key] += 1
       end
+
+      def delete(key)
+        @backend.delete(key)
+      end
     end
 
     Rack::Attack.cache.store = fake_store_class.new

spec/acceptance/cache_store_config_for_throttle_spec.rb

@@ -18,10 +18,9 @@
   end
 
   it "gives semantic error if incompatible store was configured" do
-    Rack::Attack.cache.store = Object.new
-
-    assert_raises(Rack::Attack::MisconfiguredStoreError) do
-      get "/", {}, "REMOTE_ADDR" => "1.2.3.4"
+    warning = "[rack-attack] Configured store Object doesn't respond to #read, #write, #delete, #increment\n"
+    assert_output("", warning) do
+      Rack::Attack.cache.store = Object.new
     end
   end
 
@@ -33,10 +32,22 @@ def initialize
         @counts = {}
       end
 
+      def read(key)
+        @counts[key]
+      end
+
+      def write(key, count)
+        @counts[key] = count
+      end
+
       def increment(key, _count, _options)
         @counts[key] ||= 0
         @counts[key] += 1
       end
+
+      def delete(key)
+        @counts.delete(key)
+      end
     end
 
     Rack::Attack.cache.store = basic_store_class.new

spec/rack_attack_reset_spec.rb

@@ -4,9 +4,21 @@
 
 describe "Rack::Attack.reset!" do
   it "raises an error when is not supported by cache store" do
-    Rack::Attack.cache.store = Class.new
-    assert_raises(Rack::Attack::IncompatibleStoreError) do
-      Rack::Attack.reset!
+    fake_store_class = Class.new do
+      def read(key); end
+
+      def write(key, value); end
+
+      def increment(key, count, options = {}); end
+
+      def delete(key); end
+    end
+
+    Object.stub_const(:FakeStore, fake_store_class) do
+      Rack::Attack.cache.store = FakeStore.new
+      assert_raises(Rack::Attack::IncompatibleStoreError) do
+        Rack::Attack.reset!
+      end
     end
   end