Move from proxies to well-defined adapters

Author: fatkodima

.rubocop.yml

@@ -97,3 +97,7 @@ Style/SpecialGlobalVars:
 
 Style/UnneededPercentQ:
   Enabled: true
+
+Lint/UnusedMethodArgument:
+  Exclude:
+    - "lib/rack/attack/store_adapter.rb"

lib/rack/attack.rb

@@ -6,12 +6,12 @@
 require 'rack/attack/configuration'
 require 'rack/attack/path_normalizer'
 require 'rack/attack/request'
-require 'rack/attack/store_proxy/dalli_proxy'
-require 'rack/attack/store_proxy/mem_cache_store_proxy'
-require 'rack/attack/store_proxy/redis_proxy'
-require 'rack/attack/store_proxy/redis_store_proxy'
-require 'rack/attack/store_proxy/redis_cache_store_proxy'
-require 'rack/attack/store_proxy/active_support_redis_store_proxy'
+require 'rack/attack/store_adapters/dalli_adapter'
+require 'rack/attack/store_adapters/mem_cache_store_adapter'
+require 'rack/attack/store_adapters/redis_adapter'
+require 'rack/attack/store_adapters/redis_store_adapter'
+require 'rack/attack/store_adapters/redis_cache_store_adapter'
+require 'rack/attack/store_adapters/active_support_redis_store_adapter'
 
 require 'rack/attack/railtie' if defined?(::Rails)
 

lib/rack/attack/base_proxy.rb

@@ -13,12 +13,16 @@ def initialize
 
       attr_reader :store
       def store=(store)
-        @store =
-          if (proxy = BaseProxy.lookup(store))
-            proxy.new(store)
-          else
-            store
-          end
+        raise Rack::Attack::MissingStoreError if store.nil?
+
+        adapter = StoreAdapter.lookup(store)
+        if adapter
+          @store = adapter.new(store)
+        elsif store?(store)
+          @store = store
+        else
+          raise Rack::Attack::MisconfiguredStoreError
+        end
       end
 
       def count(unprefixed_key, period)
@@ -27,9 +31,6 @@ def count(unprefixed_key, period)
       end
 
       def read(unprefixed_key)
-        enforce_store_presence!
-        enforce_store_method_presence!(:read)
-
         store.read("#{prefix}:#{unprefixed_key}")
       end
 
@@ -67,33 +68,19 @@ def key_and_expiry(unprefixed_key, period)
       end
 
       def do_count(key, expires_in)
-        enforce_store_presence!
-        enforce_store_method_presence!(:increment)
-
         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
+      STORE_METHODS = [:read, :write, :increment, :delete].freeze
 
-      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"
-          )
-        end
+      def store?(object)
+        STORE_METHODS.all? { |meth| object.respond_to?(meth) }
       end
     end
   end

lib/rack/attack/cache.rb

@@ -0,0 +1,47 @@
+# frozen_string_literal: true
+
+module Rack
+  class Attack
+    class StoreAdapter
+      class << self
+        def adapters
+          @@adapters ||= []
+        end
+
+        def inherited(klass)
+          adapters << klass
+        end
+
+        def lookup(store)
+          adapters.find { |adapter| adapter.handle?(store) }
+        end
+
+        def handle?(store)
+          raise NotImplementedError
+        end
+      end
+
+      attr_reader :store
+
+      def initialize(store)
+        @store = store
+      end
+
+      def read(key)
+        raise NotImplementedError
+      end
+
+      def write(key, value, options = {})
+        raise NotImplementedError
+      end
+
+      def increment(key, amount, options = {})
+        raise NotImplementedError
+      end
+
+      def delete(key, options = {})
+        raise NotImplementedError
+      end
+    end
+  end
+end

lib/rack/attack/store_adapter.rb

@@ -0,0 +1,47 @@
+# frozen_string_literal: true
+
+require 'rack/attack/store_adapter'
+
+module Rack
+  class Attack
+    module StoreAdapters
+      class ActiveSupportRedisStoreAdapter < StoreAdapter
+        def self.handle?(store)
+          defined?(::Redis) &&
+            defined?(::ActiveSupport::Cache::RedisStore) &&
+            store.is_a?(::ActiveSupport::Cache::RedisStore)
+        end
+
+        def read(key, options = {})
+          store.read(key, options.merge!(raw: true))
+        end
+
+        def write(key, value, options = {})
+          store.write(key, value, options.merge!(raw: true))
+        end
+
+        def increment(key, amount = 1, options = {})
+          # #increment ignores options[:expires_in].
+          #
+          # So in order to workaround this we use #write (which sets expiration) to initialize
+          # the counter. After that we continue using the original #increment.
+          if options[:expires_in] && !read(key)
+            write(key, amount, options)
+
+            amount
+          else
+            store.increment(key, amount, options)
+          end
+        end
+
+        def delete(key, options = {})
+          store.delete(key, options)
+        end
+
+        def delete_matched(matcher, options = nil)
+          store.delete_matched(matcher, options)
+        end
+      end
+    end
+  end
+end

lib/rack/attack/store_adapters/active_support_redis_store_adapter.rb

@@ -1,11 +1,11 @@
 # frozen_string_literal: true
 
-require 'rack/attack/base_proxy'
+require 'rack/attack/store_adapter'
 
 module Rack
   class Attack
-    module StoreProxy
-      class DalliProxy < BaseProxy
+    module StoreAdapters
+      class DalliAdapter < StoreAdapter
         def self.handle?(store)
           return false unless defined?(::Dalli)
 
@@ -18,38 +18,38 @@ def self.handle?(store)
           end
         end
 
-        def initialize(client)
-          super(client)
+        def initialize(store)
+          super
           stub_with_if_missing
         end
 
         def read(key)
           rescuing do
-            with do |client|
+            store.with do |client|
               client.get(key)
             end
           end
         end
 
         def write(key, value, options = {})
           rescuing do
-            with do |client|
+            store.with do |client|
               client.set(key, value, options.fetch(:expires_in, 0), raw: true)
             end
           end
         end
 
         def increment(key, amount, options = {})
           rescuing do
-            with do |client|
+            store.with do |client|
               client.incr(key, amount, options.fetch(:expires_in, 0), amount)
             end
           end
         end
 
         def delete(key)
           rescuing do
-            with do |client|
+            store.with do |client|
               client.delete(key)
             end
           end
@@ -58,10 +58,10 @@ def delete(key)
         private
 
         def stub_with_if_missing
-          unless __getobj__.respond_to?(:with)
-            class << self
+          unless store.respond_to?(:with)
+            class << store
               def with
-                yield __getobj__
+                yield store
               end
             end
           end

…b/rack/attack/store_proxy/dalli_proxy.rb …k/attack/store_adapters/dalli_adapter.rblib/rack/attack/store_proxy/dalli_proxy.rb renamed to lib/rack/attack/store_adapters/dalli_adapter.rb lib/rack/attack/store_proxy/dalli_proxy.rb renamed to lib/rack/attack/store_adapters/dalli_adapter.rb

@@ -1,19 +1,22 @@
 # frozen_string_literal: true
 
-require 'rack/attack/base_proxy'
+require 'forwardable'
 
 module Rack
   class Attack
-    module StoreProxy
-      class MemCacheStoreProxy < BaseProxy
+    module StoreAdapters
+      class MemCacheStoreAdapter < StoreAdapter
         def self.handle?(store)
           defined?(::Dalli) &&
             defined?(::ActiveSupport::Cache::MemCacheStore) &&
             store.is_a?(::ActiveSupport::Cache::MemCacheStore)
         end
 
-        def write(name, value, options = {})
-          super(name, value, options.merge!(raw: true))
+        extend Forwardable
+        def_delegators :@store, :read, :increment, :delete
+
+        def write(key, value, options = {})
+          store.write(key, value, options.merge!(raw: true))
         end
       end
     end

…ack/store_proxy/mem_cache_store_proxy.rb …tore_adapters/mem_cache_store_adapter.rblib/rack/attack/store_proxy/mem_cache_store_proxy.rb renamed to lib/rack/attack/store_adapters/mem_cache_store_adapter.rb lib/rack/attack/store_proxy/mem_cache_store_proxy.rb renamed to lib/rack/attack/store_adapters/mem_cache_store_adapter.rb

@@ -1,46 +1,46 @@
 # frozen_string_literal: true
 
-require 'rack/attack/base_proxy'
+require 'rack/attack/store_adapter'
 
 module Rack
   class Attack
-    module StoreProxy
-      class RedisProxy < BaseProxy
-        def initialize(*args)
+    module StoreAdapters
+      class RedisAdapter < StoreAdapter
+        def initialize(store)
           if Gem::Version.new(Redis::VERSION) < Gem::Version.new("3")
             warn 'RackAttack requires Redis gem >= 3.0.0.'
           end
 
-          super(*args)
+          super
         end
 
         def self.handle?(store)
           defined?(::Redis) && store.class == ::Redis
         end
 
         def read(key)
-          rescuing { get(key) }
+          rescuing { store.get(key) }
         end
 
         def write(key, value, options = {})
           if (expires_in = options[:expires_in])
-            rescuing { setex(key, expires_in, value) }
+            rescuing { store.setex(key, expires_in, value) }
           else
-            rescuing { set(key, value) }
+            rescuing { store.set(key, value) }
           end
         end
 
         def increment(key, amount, options = {})
           rescuing do
-            pipelined do
-              incrby(key, amount)
-              expire(key, options[:expires_in]) if options[:expires_in]
+            store.pipelined do
+              store.incrby(key, amount)
+              store.expire(key, options[:expires_in]) if options[:expires_in]
             end.first
           end
         end
 
         def delete(key, _options = {})
-          rescuing { del(key) }
+          rescuing { store.del(key) }
         end
 
         def delete_matched(matcher, _options = nil)
@@ -49,8 +49,8 @@ def delete_matched(matcher, _options = nil)
           rescuing do
             # Fetch keys in batches using SCAN to avoid blocking the Redis server.
             loop do
-              cursor, keys = scan(cursor, match: matcher, count: 1000)
-              del(*keys) unless keys.empty?
+              cursor, keys = store.scan(cursor, match: matcher, count: 1000)
+              store.del(*keys) unless keys.empty?
               break if cursor == "0"
             end
           end