From 23767c0c41fa51ebef1dfb11bc166c07e93897ac Mon Sep 17 00:00:00 2001 From: Matt Wilde Date: Wed, 11 Mar 2015 14:08:19 -0700 Subject: [PATCH] Skip the `:race_condition_ttl` branch if the option is 0 or nil. This fixes an issue with the redis cache, where this code will sometimes throw an error out of SETEX when passing 0 as the `expires_at`. --- activesupport/lib/active_support/cache.rb | 4 ++-- activesupport/test/caching_test.rb | 28 ++++++++++++++++------- 2 files changed, 22 insertions(+), 10 deletions(-) diff --git a/activesupport/lib/active_support/cache.rb b/activesupport/lib/active_support/cache.rb index 625be2c959..837974bc85 100644 --- a/activesupport/lib/active_support/cache.rb +++ b/activesupport/lib/active_support/cache.rb @@ -566,8 +566,8 @@ module ActiveSupport def handle_expired_entry(entry, key, options) if entry && entry.expired? race_ttl = options[:race_condition_ttl].to_i - if race_ttl && (Time.now.to_f - entry.expires_at <= race_ttl) - # When an entry has :race_condition_ttl defined, put the stale entry back into the cache + if (race_ttl > 0) && (Time.now.to_f - entry.expires_at <= race_ttl) + # When an entry has a positive :race_condition_ttl defined, put the stale entry back into the cache # for a brief period while the entry is being recalculated. entry.expires_at = Time.now + race_ttl write_entry(key, entry, :expires_in => race_ttl * 2) diff --git a/activesupport/test/caching_test.rb b/activesupport/test/caching_test.rb index 7f5f8feb0d..4953550c45 100644 --- a/activesupport/test/caching_test.rb +++ b/activesupport/test/caching_test.rb @@ -399,15 +399,16 @@ module CacheStoreBehavior assert_nil @cache.read('foo') end - def test_race_condition_protection - time = Time.now - @cache.write('foo', 'bar', :expires_in => 60) - Time.stubs(:now).returns(time + 61) - result = @cache.fetch('foo', :race_condition_ttl => 10) do - assert_equal 'bar', @cache.read('foo') - "baz" + def test_race_condition_protection_skipped_if_not_defined + @cache.write('foo', 'bar') + time = @cache.send(:read_entry, 'foo', {}).expires_at + Time.stubs(:now).returns(Time.at(time)) + + result = @cache.fetch('foo') do + assert_equal nil, @cache.read('foo') + 'baz' end - assert_equal "baz", result + assert_equal 'baz', result end def test_race_condition_protection_is_limited @@ -437,6 +438,17 @@ module CacheStoreBehavior assert_nil @cache.read('foo') end + def test_race_condition_protection + time = Time.now + @cache.write('foo', 'bar', :expires_in => 60) + Time.stubs(:now).returns(time + 61) + result = @cache.fetch('foo', :race_condition_ttl => 10) do + assert_equal 'bar', @cache.read('foo') + "baz" + end + assert_equal "baz", result + end + def test_crazy_key_characters crazy_key = "#/:*(<+=> )&$%@?;'\"\'`~-" assert @cache.write(crazy_key, "1", :raw => true)