Merge pull request rails#47340 from aledustet/deprecate-memcache-store-using-a-dalli-client-instance

Deprecate initialize memcache store with dalli client

Author: byroot

activesupport/CHANGELOG.md

@@ -1,3 +1,9 @@
+*   Deprecated initializing a `ActiveSupport::Cache::MemCacheStore` with an instance of `Dalli::Client`.
+
+    Deprecate the undocumented option of providing an already-initialized instance of `Dalli::Client` to `ActiveSupport::Cache::MemCacheStore`. Such clients could be configured with unrecognized options, which could lead to unexpected behavior. Instead, provide addresses as documented.
+
+    *aledustet*
+
 *   Stub `Time.new()` in `TimeHelpers#travel_to`
 
       ```ruby

activesupport/lib/active_support/cache/mem_cache_store.rb

@@ -28,6 +28,10 @@ module Cache
     # MemCacheStore implements the Strategy::LocalCache strategy which implements
     # an in-memory cache inside of a block.
     class MemCacheStore < Store
+      # These options represent behavior overridden by this implementation and should
+      # not be allowed to get down to the Dalli client
+      OVERRIDDEN_OPTIONS = UNIVERSAL_OPTIONS
+
       # Advertise cache versioning support.
       def self.supports_cache_versioning?
         true
@@ -108,6 +112,7 @@ def self.build_mem_cache(*addresses) # :nodoc:
       #
       # If no addresses are provided, but <tt>ENV['MEMCACHE_SERVERS']</tt> is defined, it will be used instead. Otherwise,
       # MemCacheStore will connect to localhost:11211 (the default memcached port).
+      # Passing a +Dalli::Client+ instance is deprecated and will be removed. Please pass an address instead.
       def initialize(*addresses)
         addresses = addresses.flatten
         options = addresses.extract_options!
@@ -117,15 +122,19 @@ def initialize(*addresses)
         super(options)
 
         unless [String, Dalli::Client, NilClass].include?(addresses.first.class)
-          raise ArgumentError, "First argument must be an empty array, an array of hosts or a Dalli::Client instance."
+          raise ArgumentError, "First argument must be an empty array, address, or array of addresses."
         end
         if addresses.first.is_a?(Dalli::Client)
+          ActiveSupport.deprecator.warn(<<~MSG)
+            Initializing MemCacheStore with a Dalli::Client is deprecated and will be removed in Rails 7.2.
+            Use memcached server addresses instead.
+          MSG
           @data = addresses.first
         else
           mem_cache_options = options.dup
           # The value "compress: false" prevents duplicate compression within Dalli.
           mem_cache_options[:compress] = false
-          (UNIVERSAL_OPTIONS - %i(compress)).each { |name| mem_cache_options.delete(name) }
+          (OVERRIDDEN_OPTIONS - %i(compress)).each { |name| mem_cache_options.delete(name) }
           @data = self.class.build_mem_cache(*(addresses + [mem_cache_options]))
         end
       end

activesupport/test/cache/stores/mem_cache_store_test.rb

@@ -271,9 +271,11 @@ def test_unless_exist_expires_when_configured
   end
 
   def test_uses_provided_dalli_client_if_present
-    cache = lookup_store(Dalli::Client.new("custom_host"))
-
-    assert_equal ["custom_host"], servers(cache)
+    assert_deprecated(ActiveSupport.deprecator) do
+      host = "custom_host"
+      cache = lookup_store(Dalli::Client.new(host))
+      assert_equal [host], servers(cache)
+    end
   end
 
   def test_forwards_string_addresses_if_present