Add support for connection pooling on RedisCacheStore

4 files changed, 91 insertions, 50 deletions

diff --git a/activesupport/test/cache/behaviors.rb b/activesupport/test/cache/behaviors.rb
index cb08a10bba..0e07e2319d 100644
--- a/activesupport/test/cache/behaviors.rb
+++ b/activesupport/test/cache/behaviors.rb
@@ -5,5 +5,6 @@ require_relative "behaviors/cache_delete_matched_behavior"
 require_relative "behaviors/cache_increment_decrement_behavior"
 require_relative "behaviors/cache_store_behavior"
 require_relative "behaviors/cache_store_version_behavior"
+require_relative "behaviors/connection_pool_behavior"
 require_relative "behaviors/encoded_key_cache_behavior"
 require_relative "behaviors/local_cache_behavior"
diff --git a/activesupport/test/cache/behaviors/connection_pool_behavior.rb b/activesupport/test/cache/behaviors/connection_pool_behavior.rb
new file mode 100644
index 0000000000..500d51a134
--- /dev/null
+++ b/activesupport/test/cache/behaviors/connection_pool_behavior.rb
@@ -0,0 +1,53 @@
+# frozen_string_literal: true
+
+module ConnectionPoolBehavior
+  def test_connection_pool
+    emulating_latency do
+      begin
+        cache = ActiveSupport::Cache.lookup_store(store, pool_size: 2, pool_timeout: 1)
+        cache.clear
+
+        threads = []
+
+        assert_raises Timeout::Error do
+          # One of the three threads will fail in 1 second because our pool size
+          # is only two.
+          3.times do
+            threads << Thread.new do
+              cache.read("latency")
+            end
+          end
+
+          threads.each(&:join)
+        end
+      ensure
+        threads.each(&:kill)
+      end
+    end
+  end
+
+  def test_no_connection_pool
+    emulating_latency do
+      begin
+        cache = ActiveSupport::Cache.lookup_store(store)
+        cache.clear
+
+        threads = []
+
+        assert_nothing_raised do
+          # Default connection pool size is 5, assuming 10 will make sure that
+          # the connection pool isn't used at all.
+          10.times do
+            threads << Thread.new do
+              cache.read("latency")
+            end
+          end
+
+          threads.each(&:join)
+        end
+      ensure
+        threads.each(&:kill)
+      end
+    end
+  end
+end
diff --git a/activesupport/test/cache/stores/mem_cache_store_test.rb b/activesupport/test/cache/stores/mem_cache_store_test.rb
index 7f537c3bbf..ccb3b7403f 100644
--- a/activesupport/test/cache/stores/mem_cache_store_test.rb
+++ b/activesupport/test/cache/stores/mem_cache_store_test.rb
@@ -45,56 +45,7 @@ class MemCacheStoreTest < ActiveSupport::TestCase
   include CacheIncrementDecrementBehavior
   include EncodedKeyCacheBehavior
   include AutoloadingCacheBehavior
-
-  def test_connection_pool
-    emulating_latency do
-      begin
-        cache = ActiveSupport::Cache.lookup_store(:mem_cache_store, pool_size: 2, pool_timeout: 1)
-        cache.clear
-
-        threads = []
-
-        assert_raises Timeout::Error do
-          # One of the three threads will fail in 1 second because our pool size
-          # is only two.
-          3.times do
-            threads << Thread.new do
-              cache.read("latency")
-            end
-          end
-
-          threads.each(&:join)
-        end
-      ensure
-        threads.each(&:kill)
-      end
-    end
-  end
-
-  def test_no_connection_pool
-    emulating_latency do
-      begin
-        cache = ActiveSupport::Cache.lookup_store(:mem_cache_store)
-        cache.clear
-
-        threads = []
-
-        assert_nothing_raised do
-          # Default connection pool size is 5, assuming 10 will make sure that
-          # the connection pool isn't used at all.
-          10.times do
-            threads << Thread.new do
-              cache.read("latency")
-            end
-          end
-
-          threads.each(&:join)
-        end
-      ensure
-        threads.each(&:kill)
-      end
-    end
-  end
+  include ConnectionPoolBehavior
 
   def test_raw_values
     cache = ActiveSupport::Cache.lookup_store(:mem_cache_store, raw: true)
@@ -154,6 +105,10 @@ class MemCacheStoreTest < ActiveSupport::TestCase
 
   private
 
+    def store
+      :mem_cache_store
+    end
+
     def emulating_latency
       old_client = Dalli.send(:remove_const, :Client)
       Dalli.const_set(:Client, SlowDalliClient)
diff --git a/activesupport/test/cache/stores/redis_cache_store_test.rb b/activesupport/test/cache/stores/redis_cache_store_test.rb
index 3635703002..cdbb5dc4c6 100644
--- a/activesupport/test/cache/stores/redis_cache_store_test.rb
+++ b/activesupport/test/cache/stores/redis_cache_store_test.rb
@@ -8,6 +8,18 @@ require_relative "../behaviors"
 module ActiveSupport::Cache::RedisCacheStoreTests
   DRIVER = %w[ ruby hiredis ].include?(ENV["REDIS_DRIVER"]) ? ENV["REDIS_DRIVER"] : "hiredis"
 
+  # Emulates a latency on Redis's back-end for the key latency to facilitate
+  # connection pool testing.
+  class SlowRedis < Redis
+    def get(key, options = {})
+      if key =~ /latency/
+        sleep 3
+      else
+        super
+      end
+    end
+  end
+
   class LookupTest < ActiveSupport::TestCase
     test "may be looked up as :redis_cache_store" do
       assert_kind_of ActiveSupport::Cache::RedisCacheStore,
@@ -110,6 +122,26 @@ module ActiveSupport::Cache::RedisCacheStoreTests
     include AutoloadingCacheBehavior
   end
 
+  class RedisCacheStoreConnectionPoolBehaviourTest < StoreTest
+    include ConnectionPoolBehavior
+
+    private
+
+      def store
+        :redis_cache_store
+      end
+
+      def emulating_latency
+        old_redis = Object.send(:remove_const, :Redis)
+        Object.const_set(:Redis, SlowRedis)
+
+        yield
+      ensure
+        Object.send(:remove_const, :Redis)
+        Object.const_set(:Redis, old_redis)
+      end
+  end
+
   # Separate test class so we can omit the namespace which causes expected,
   # appropriate complaints about incompatible string encodings.
   class KeyEncodingSafetyTest < StoreTest