Straighten a few things out

lib/pecorino/cached_throttle.rb

@@ -17,36 +17,34 @@ def initialize(cache_store, throttle)
 
   # @see Pecorino::Throttle#request!
   def request!(n = 1)
-    blocked_state = cached_blocked_state
-    if blocked_state&.blocked?
-      raise Pecorino::Throttle::Throttled.new(@throttle, blocked_state)
-    else
-      begin
-        @throttle.request!(n)
-      rescue Pecorino::Throttle::Throttled => throttled_ex
-        cache_blocked_state(throttled_ex.state) if throttled_ex.throttle == @throttle
-        raise
-      end
+    blocked_state = read_cached_blocked_state
+    raise Pecorino::Throttle::Throttled.new(@throttle, blocked_state) if blocked_state&.blocked?
+
+    begin
+      @throttle.request!(n)
+    rescue Pecorino::Throttle::Throttled => throttled_ex
+      write_cache_blocked_state(throttled_ex.state) if throttled_ex.throttle == @throttle
+      raise
     end
   end
 
   # Returns cached `state` for the throttle if there is a currently active block for that throttle in the cache. Otherwise forwards to underlying throttle.
   #
   # @see Pecorino::Throttle#request
   def request(n = 1)
-    blocked_state = cached_blocked_state
+    blocked_state = read_cached_blocked_state
     return blocked_state if blocked_state&.blocked?
 
     @throttle.request(n).tap do |state|
-      cache_blocked_state(state) if state.blocked_until
+      write_cache_blocked_state(state) if state.blocked_until
     end
   end
 
   # Returns `false` if there is a currently active block for that throttle in the cache. Otherwise forwards to underlying throttle.
   #
   # @see Pecorino::Throttle#able_to_accept?
   def able_to_accept?(n = 1)
-    blocked_state = cached_blocked_state
+    blocked_state = read_cached_blocked_state
     return false if blocked_state&.blocked?
 
     @throttle.able_to_accept?(n)
@@ -72,21 +70,22 @@ def key
   #
   # @see Pecorino::Throttle#able_to_accept?
   def state
-    blocked_state = cached_blocked_state
+    blocked_state = read_cached_blocked_state
+    warn "Read blocked state #{blocked_state.inspect}"
     return blocked_state if blocked_state&.blocked?
 
     @throttle.state.tap do |state|
-      cache_blocked_state(state) if state.blocked?
+      write_cache_blocked_state(state) if state.blocked?
     end
   end
 
   private
 
-  def cache_blocked_state(state)
+  def write_cache_blocked_state(state)
     @cache_store.write("pecorino-cached-throttle-state-#{@throttle.key}", state, expires_after: state.blocked_until)
   end
 
-  def cached_blocked_state
+  def read_cached_blocked_state
     @cache_store.read("pecorino-cached-throttle-state-#{@throttle.key}")
   end
 end

test/cached_throttle_test.rb

@@ -13,7 +13,7 @@ def teardown
 
   test "caches results of request! and correctly raises Throttled until the block is lifted" do
     store = ActiveSupport::Cache::MemoryStore.new
-    throttle = Pecorino::Throttle.new(key: Random.uuid, capacity: 2, over_time: 1.second, block_for: 2.seconds)
+    throttle = Pecorino::Throttle.new(key: Random.uuid, capacity: 2, over_time: 1.second, block_for: 10.seconds)
     cached_throttle = Pecorino::CachedThrottle.new(store, throttle)
 
     state1 = cached_throttle.request!
@@ -38,7 +38,7 @@ class << throttle
 
   test "caches results of able_to_accept? until the block is lifted" do
     store = ActiveSupport::Cache::MemoryStore.new
-    throttle = Pecorino::Throttle.new(key: Random.uuid, capacity: 2, over_time: 1.second, block_for: 2.seconds)
+    throttle = Pecorino::Throttle.new(key: Random.uuid, capacity: 2, over_time: 1.second, block_for: 10.seconds)
     cached_throttle = Pecorino::CachedThrottle.new(store, throttle)
 
     cached_throttle.request(1)
@@ -57,7 +57,7 @@ class << throttle
 
   test "caches results of request() and correctly returns cached state until the block is lifted" do
     store = ActiveSupport::Cache::MemoryStore.new
-    throttle = Pecorino::Throttle.new(key: Random.uuid, capacity: 2, over_time: 1.second, block_for: 2.seconds)
+    throttle = Pecorino::Throttle.new(key: Random.uuid, capacity: 2, over_time: 1.second, block_for: 10.seconds)
     cached_throttle = Pecorino::CachedThrottle.new(store, throttle)
 
     state1 = cached_throttle.request(1)
@@ -78,9 +78,16 @@ class << throttle
     assert_predicate state_from_cache, :blocked?
   end
 
+  test "returns the key of the contained throttle" do
+    store = ActiveSupport::Cache::MemoryStore.new
+    throttle = Pecorino::Throttle.new(key: Random.uuid, capacity: 2, over_time: 1.second, block_for: 10.seconds)
+    cached_throttle = Pecorino::CachedThrottle.new(store, throttle)
+    assert_equal cached_throttle.key, throttle.key
+  end
+
   test "does not run block in throttled() until the block is lifted" do
     store = ActiveSupport::Cache::MemoryStore.new
-    throttle = Pecorino::Throttle.new(key: Random.uuid, capacity: 2, over_time: 1.second, block_for: 2.seconds)
+    throttle = Pecorino::Throttle.new(key: Random.uuid, capacity: 2, over_time: 1.second, block_for: 10.seconds)
     cached_throttle = Pecorino::CachedThrottle.new(store, throttle)
 
     assert_equal 123, cached_throttle.throttled { 123 }
@@ -94,28 +101,4 @@ class << throttle
 
     assert_nil cached_throttle.throttled { 345 }
   end
-
-  test "caches results of state() and correctly returns cached state until the block is lifted" do
-    store = ActiveSupport::Cache::MemoryStore.new
-    throttle = Pecorino::Throttle.new(key: Random.uuid, capacity: 2, over_time: 1.second, block_for: 2.seconds)
-    cached_throttle = Pecorino::CachedThrottle.new(store, throttle)
-
-    cached_throttle.request(2)
-    blocked_state = cached_throttle.request(1)
-    blocked_state_from_cache = cached_throttle.state
-
-    assert_kind_of Pecorino::Throttle::State, blocked_state
-    assert_kind_of Pecorino::Throttle::State, blocked_state_from_cache
-    assert_predicate blocked_state, :blocked?
-    assert_predicate blocked_state_from_cache, :blocked?
-
-    # Delete the method on the actual throttle as it should not be called anymore until the block is lifted
-    class << throttle
-      undef :state
-    end
-
-    state_from_cache = cached_throttle.state
-    assert_kind_of Pecorino::Throttle::State, state_from_cache
-    assert_predicate state_from_cache, :blocked?
-  end
 end