Merge pull request rails#54292 from zzak/re-54261

Move back to kwargs for (de)increment for cache stores

Author: byroot

activesupport/lib/active_support/cache/file_store.rb

@@ -57,7 +57,7 @@ def cleanup(options = nil)
       #   cache.write("baz", 5)
       #   cache.increment("baz") # => 6
       #
-      def increment(name, amount = 1, options = nil)
+      def increment(name, amount = 1, **options)
         options = merged_options(options)
         key = normalize_key(name, options)
 
@@ -77,7 +77,7 @@ def increment(name, amount = 1, options = nil)
       #   cache.write("baz", 5)
       #   cache.decrement("baz") # => 4
       #
-      def decrement(name, amount = 1, options = nil)
+      def decrement(name, amount = 1, **options)
         options = merged_options(options)
         key = normalize_key(name, options)
 

activesupport/lib/active_support/cache/memory_store.rb

@@ -146,9 +146,9 @@ def pruning?
       #   cache.write("baz", 5)
       #   cache.increment("baz") # => 6
       #
-      def increment(name, amount = 1, options = nil)
+      def increment(name, amount = 1, **options)
         instrument(:increment, name, amount: amount) do
-          modify_value(name, amount, options)
+          modify_value(name, amount, **options)
         end
       end
 
@@ -163,9 +163,9 @@ def increment(name, amount = 1, options = nil)
       #   cache.write("baz", 5)
       #   cache.decrement("baz") # => 4
       #
-      def decrement(name, amount = 1, options = nil)
+      def decrement(name, amount = 1, **options)
         instrument(:decrement, name, amount: amount) do
-          modify_value(name, -amount, options)
+          modify_value(name, -amount, **options)
         end
       end
 
@@ -238,7 +238,7 @@ def delete_entry(key, **options)
 
         # Modifies the amount of an integer value that is stored in the cache.
         # If the key is not found it is created and set to +amount+.
-        def modify_value(name, amount, options)
+        def modify_value(name, amount, **options)
           options = merged_options(options)
           key     = normalize_key(name, options)
           version = normalize_version(name, options)

activesupport/lib/active_support/cache/null_store.rb

@@ -25,10 +25,10 @@ def clear(options = nil)
       def cleanup(options = nil)
       end
 
-      def increment(name, amount = 1, options = nil)
+      def increment(name, amount = 1, **options)
       end
 
-      def decrement(name, amount = 1, options = nil)
+      def decrement(name, amount = 1, **options)
       end
 
       def delete_matched(matcher, options = nil)

activesupport/lib/active_support/cache/strategy/local_cache.rb

@@ -94,25 +94,17 @@ def delete_matched(matcher, options = nil) # :nodoc:
           super
         end
 
-        def increment(name, amount = 1, options = nil) # :nodoc:
+        def increment(name, amount = 1, **options) # :nodoc:
           return super unless local_cache
           value = bypass_local_cache { super }
-          if options
-            write_cache_value(name, value, raw: true, **options)
-          else
-            write_cache_value(name, value, raw: true)
-          end
+          write_cache_value(name, value, raw: true, **options)
           value
         end
 
-        def decrement(name, amount = 1, options = nil) # :nodoc:
+        def decrement(name, amount = 1, **options) # :nodoc:
           return super unless local_cache
           value = bypass_local_cache { super }
-          if options
-            write_cache_value(name, value, raw: true, **options)
-          else
-            write_cache_value(name, value, raw: true)
-          end
+          write_cache_value(name, value, raw: true, **options)
           value
         end
 

activesupport/test/cache/behaviors/cache_increment_decrement_behavior.rb

@@ -34,8 +34,8 @@ def test_decrement
   def test_ttl_isnt_updated
     key = SecureRandom.uuid
 
-    assert_equal 1, @cache.increment(key, 1, expires_in: 1)
-    assert_equal 2, @cache.increment(key, 1, expires_in: 5000)
+    assert_equal 1, @cache.increment(key, expires_in: 1)
+    assert_equal 2, @cache.increment(key, expires_in: 5000)
 
     # having to sleep two seconds in a test is bad, but we're testing
     # a wide range of backends with different TTL mechanisms, most without

activesupport/test/cache/stores/memory_store_test.rb

@@ -38,6 +38,13 @@ def test_increment_preserves_expiry
     Time.stub(:now, Time.now + 1.minute) do
       assert_nil @cache.read("counter", raw: true)
     end
+
+    @cache.write("counter", 1, raw: true)
+    @cache.increment("counter", expires_in: 30)
+    assert_equal 2, @cache.read("counter", raw: true)
+    Time.stub(:now, Time.now + 1.minute) do
+      assert_nil @cache.read("counter2", raw: true)
+    end
   end
 
   def test_cleanup_instrumentation

activesupport/test/cache/stores/null_store_test.rb

@@ -40,9 +40,26 @@ def test_increment
     assert_nil @cache.increment("name")
   end
 
+  def test_increment_with_options
+    time = Time.now
+    @cache.increment("name", expires_in: 1.second)
+    Time.stub(:now, time + 2.seconds) do
+      assert_nil @cache.read("name")
+    end
+  end
+
   def test_decrement
     @cache.write("name", 1, raw: true)
-    assert_nil @cache.increment("name")
+    assert_nil @cache.decrement("name")
+  end
+
+  def test_decrement_with_options
+    time = Time.now
+    @cache.write("name", 1, raw: true)
+    @cache.decrement("name", expires_in: 1.second)
+    Time.stub(:now, time + 2.seconds) do
+      assert_nil @cache.read("name")
+    end
   end
 
   def test_delete_matched

activesupport/test/cache/stores/redis_cache_store_test.rb

@@ -220,22 +220,22 @@ def test_increment_ttl
     end
 
     def test_increment_expires_in
-      @cache.increment "foo", 1, expires_in: 60
+      @cache.increment "foo", expires_in: 60
       redis_backend do |r|
         assert r.exists?("#{@namespace}:foo")
         assert r.ttl("#{@namespace}:foo") > 0
       end
 
       # key and ttl exist
       redis_backend { |r| r.setex "#{@namespace}:bar", 120, 1 }
-      @cache.increment "bar", 1, expires_in: 60
+      @cache.increment "bar", expires_in: 60
       redis_backend do |r|
         assert r.ttl("#{@namespace}:bar") > 60
       end
 
       # key exist but not have expire
       redis_backend(@cache_no_ttl) { |r| r.set "#{@namespace}:dar", 10 }
-      @cache_no_ttl.increment "dar", 1, expires_in: 60
+      @cache_no_ttl.increment "dar", expires_in: 60
       redis_backend(@cache_no_ttl) do |r|
         assert r.ttl("#{@namespace}:dar") > 0
       end