Merge pull request rails#48967 from jdelStrother/distributed-redis-tests

Add test coverage for RedisCacheStore with Redis::Distributed / ConnectionPool

Author: byroot

activesupport/lib/active_support/cache/redis_cache_store.rb

@@ -10,6 +10,7 @@
 end
 
 require "connection_pool"
+require "active_support/core_ext/array/wrap"
 require "active_support/core_ext/hash/slice"
 require "active_support/core_ext/numeric/time"
 require "active_support/digest"
@@ -455,8 +456,8 @@ def change_counter(key, amount, options)
         def supports_expire_nx?
           return @supports_expire_nx if defined?(@supports_expire_nx)
 
-          redis_version = redis.then { |c| c.info("server").fetch("redis_version") }
-          @supports_expire_nx = Gem::Version.new(redis_version) >= Gem::Version.new("7.0.0")
+          redis_versions = redis.then { |c| Array.wrap(c.info("server")).pluck("redis_version") }
+          @supports_expire_nx = redis_versions.all? { |v| Gem::Version.new(v) >= Gem::Version.new("7.0.0") }
         end
 
         def failsafe(method, returning: nil)

activesupport/test/cache/stores/redis_cache_store_test.rb

@@ -151,7 +151,9 @@ def lookup_store(options = {})
 
     teardown do
       @cache.clear
-      @cache.redis.disconnect!
+      @cache.redis.with do |r|
+        r.respond_to?(:on_each_node, true) ? r.send(:on_each_node, :disconnect!) : r.disconnect!
+      end
     end
   end
 
@@ -169,15 +171,15 @@ class RedisCacheStoreCommonBehaviorTest < StoreTest
     include EncodedKeyCacheBehavior
 
     def test_fetch_multi_uses_redis_mget
-      assert_called(@cache.redis, :mget, returns: []) do
+      assert_called(redis_backend, :mget, returns: []) do
         @cache.fetch_multi("a", "b", "c") do |key|
           key * 2
         end
       end
     end
 
     def test_fetch_multi_with_namespace
-      assert_called_with(@cache.redis, :mget, ["custom-namespace:a", "custom-namespace:b", "custom-namespace:c"], returns: []) do
+      assert_called_with(redis_backend, :mget, ["custom-namespace:a", "custom-namespace:b", "custom-namespace:c"], returns: []) do
         @cache.fetch_multi("a", "b", "c", namespace: "custom-namespace") do |key|
           key * 2
         end
@@ -186,39 +188,53 @@ def test_fetch_multi_with_namespace
 
     def test_write_expires_at
       @cache.write "key_with_expires_at", "bar", expires_at: 30.minutes.from_now
-      assert @cache.redis.ttl("#{@namespace}:key_with_expires_at") > 0
+      redis_backend do |r|
+        assert r.ttl("#{@namespace}:key_with_expires_at") > 0
+      end
     end
 
     def test_increment_expires_in
       @cache.increment "foo", 1, expires_in: 60
-      assert @cache.redis.exists?("#{@namespace}:foo")
-      assert @cache.redis.ttl("#{@namespace}:foo") > 0
+      redis_backend do |r|
+        assert r.exists?("#{@namespace}:foo")
+        assert r.ttl("#{@namespace}:foo") > 0
+      end
 
       # key and ttl exist
-      @cache.redis.setex "#{@namespace}:bar", 120, 1
+      redis_backend { |r| r.setex "#{@namespace}:bar", 120, 1 }
       @cache.increment "bar", 1, expires_in: 60
-      assert @cache.redis.ttl("#{@namespace}:bar") > 60
+      redis_backend do |r|
+        assert r.ttl("#{@namespace}:bar") > 60
+      end
 
       # key exist but not have expire
-      @cache.redis.set "#{@namespace}:dar", 10
+      redis_backend { |r| r.set "#{@namespace}:dar", 10 }
       @cache.increment "dar", 1, expires_in: 60
-      assert @cache.redis.ttl("#{@namespace}:dar") > 0
+      redis_backend do |r|
+        assert r.ttl("#{@namespace}:dar") > 0
+      end
     end
 
     def test_decrement_expires_in
       @cache.decrement "foo", 1, expires_in: 60
-      assert @cache.redis.exists?("#{@namespace}:foo")
-      assert @cache.redis.ttl("#{@namespace}:foo") > 0
+      redis_backend do |r|
+        assert r.exists?("#{@namespace}:foo")
+        assert r.ttl("#{@namespace}:foo") > 0
+      end
 
       # key and ttl exist
-      @cache.redis.setex "#{@namespace}:bar", 120, 1
+      redis_backend { |r| r.setex "#{@namespace}:bar", 120, 1 }
       @cache.decrement "bar", 1, expires_in: 60
-      assert @cache.redis.ttl("#{@namespace}:bar") > 60
+      redis_backend do |r|
+        assert r.ttl("#{@namespace}:bar") > 60
+      end
 
       # key exist but not have expire
-      @cache.redis.set "#{@namespace}:dar", 10
+      redis_backend { |r| r.set "#{@namespace}:dar", 10 }
       @cache.decrement "dar", 1, expires_in: 60
-      assert @cache.redis.ttl("#{@namespace}:dar") > 0
+      redis_backend do |r|
+        assert r.ttl("#{@namespace}:dar") > 0
+      end
     end
 
     test "fetch caches nil" do
@@ -235,6 +251,19 @@ def test_decrement_expires_in
         assert_equal false, @cache.exist?("foo")
       end
     end
+
+    def redis_backend
+      @cache.redis.with do |r|
+        yield r if block_given?
+        return r
+      end
+    end
+  end
+
+  class RedisCacheStoreWithDistributedRedisTest < RedisCacheStoreCommonBehaviorTest
+    def lookup_store(options = {})
+      super(options.merge(pool: { size: 5 }, url: [ENV["REDIS_URL"] || "redis://localhost:6379/0"] * 2))
+    end
   end
 
   class ConnectionPoolBehaviorTest < StoreTest