From 5394be374dd20c34ddf128cfd04d6b80096a4acb Mon Sep 17 00:00:00 2001 From: Jason Lee Date: Sat, 30 Jun 2018 12:52:29 +0800 Subject: [PATCH] Fix Cache :redis_store increment/decrement ttl check and add more tests. --- .../active_support/cache/redis_cache_store.rb | 4 ++-- .../cache/stores/redis_cache_store_test.rb | 24 +++++++++++++++---- 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/activesupport/lib/active_support/cache/redis_cache_store.rb b/activesupport/lib/active_support/cache/redis_cache_store.rb index 95f8f639e8..a05718b663 100644 --- a/activesupport/lib/active_support/cache/redis_cache_store.rb +++ b/activesupport/lib/active_support/cache/redis_cache_store.rb @@ -264,7 +264,7 @@ def increment(name, amount = 1, options = nil) failsafe :increment do redis.with do |c| val = c.incrby key, amount - if expires_in > 0 && c.ttl(key) == -2 + if expires_in > 0 && c.ttl(key) < 0 c.expire key, expires_in end val @@ -290,7 +290,7 @@ def decrement(name, amount = 1, options = nil) failsafe :decrement do redis.with do |c| val = c.decrby key, amount - if expires_in > 0 && c.ttl(key) == -2 + if expires_in > 0 && c.ttl(key) < 0 c.expire key, expires_in end val diff --git a/activesupport/test/cache/stores/redis_cache_store_test.rb b/activesupport/test/cache/stores/redis_cache_store_test.rb index a2165e1978..845254ac69 100644 --- a/activesupport/test/cache/stores/redis_cache_store_test.rb +++ b/activesupport/test/cache/stores/redis_cache_store_test.rb @@ -145,24 +145,40 @@ def test_fetch_multi_uses_redis_mget def test_increment_expires_in assert_called_with @cache.redis, :incrby, [ "#{@namespace}:foo", 1 ] do assert_called_with @cache.redis, :expire, [ "#{@namespace}:foo", 60 ] do - @cache.increment("foo", 1, expires_in: 60) + @cache.increment "foo", 1, expires_in: 60 end end + # key and ttl exist + @cache.redis.setex "#{@namespace}:bar", 120, 1 assert_not_called @cache.redis, :expire do - @cache.decrement("foo", 1, expires_in: 60) + @cache.increment "bar", 1, expires_in: 120 + end + + # key exist but not have expire + @cache.redis.set "#{@namespace}:dar", 10 + assert_called_with @cache.redis, :expire, [ "#{@namespace}:dar", 60 ] do + @cache.increment "dar", 1, expires_in: 60 end end def test_decrement_expires_in assert_called_with @cache.redis, :decrby, [ "#{@namespace}:foo", 1 ] do assert_called_with @cache.redis, :expire, [ "#{@namespace}:foo", 60 ] do - @cache.decrement("foo", 1, expires_in: 60) + @cache.decrement "foo", 1, expires_in: 60 end end + # key and ttl exist + @cache.redis.setex "#{@namespace}:bar", 120, 1 assert_not_called @cache.redis, :expire do - @cache.decrement("foo", 1, expires_in: 60) + @cache.decrement "bar", 1, expires_in: 120 + end + + # key exist but not have expire + @cache.redis.set "#{@namespace}:dar", 10 + assert_called_with @cache.redis, :expire, [ "#{@namespace}:dar", 60 ] do + @cache.decrement "dar", 1, expires_in: 60 end end end -- GitLab