From d0bf073d6661a7401d202209ca20c9c7630146f1 Mon Sep 17 00:00:00 2001 From: Julik Tarkhanov Date: Sun, 11 Feb 2024 19:21:12 +0100 Subject: [PATCH] Straighten a few things out --- lib/pecorino/cached_throttle.rb | 33 ++++++++++++++-------------- test/cached_throttle_test.rb | 39 ++++++++++----------------------- 2 files changed, 27 insertions(+), 45 deletions(-) diff --git a/lib/pecorino/cached_throttle.rb b/lib/pecorino/cached_throttle.rb index 1ac77e6..ae23a4d 100644 --- a/lib/pecorino/cached_throttle.rb +++ b/lib/pecorino/cached_throttle.rb @@ -17,16 +17,14 @@ 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 @@ -34,11 +32,11 @@ def request!(n = 1) # # @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 @@ -46,7 +44,7 @@ def request(n = 1) # # @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 diff --git a/test/cached_throttle_test.rb b/test/cached_throttle_test.rb index 7c51f2e..434c875 100644 --- a/test/cached_throttle_test.rb +++ b/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