Fallback to $MEMCACHE_SERVERS if no servers given

By default, Dalli has two fallbacks if no server addresses are given:

- $MEMCACHE_SERVERS
- "127.0.0.1:11211"

However, MemCacheStore does its own check for addresses, and falls back
to "localhost:11211" if none are present.

This can lead to bugs in migrations from the deprecated :dalli_store
(provided by the Dalli) to :mem_cache_store:

```diff
-config.cache_store = :dalli_store     # could be implicitly relying on $MEMCACHE_SERVERS
+config.cache_store = :mem_cache_store # ignores $MEMCACHE_SERVERS
```

By removing our own fallback and simply passing `nil` to Dalli::Client,
we get its fallback logic for free. Tests are added so we can detect if
this ever changes.

activesupport/CHANGELOG.md

@@ -1,3 +1,19 @@
+*   `ActiveSupport::Cache::MemCacheStore` now checks `ENV["MEMCACHE_SERVERS"]` before falling back to `"localhost:11211"` if configured without any addresses.
+
+    ```ruby
+    config.cache_store = :mem_cache_store
+
+    # is now equivalent to
+
+    config.cache_store = :mem_cache_store, ENV["MEMCACHE_SERVERS"] || "localhost:11211"
+
+    # instead of
+
+    config.cache_store = :mem_cache_store, "localhost:11211" # ignores ENV["MEMCACHE_SERVERS"]
+    ```
+
+    *Sam Bostock*
+
 *   `ActiveSupport::Subscriber#attach_to` now accepts an `inherit_all:` argument. When set to true,
     it allows a subscriber to receive events for methods defined in the subscriber's ancestor class(es).
 

activesupport/lib/active_support/cache/mem_cache_store.rb

@@ -53,16 +53,18 @@ module ActiveSupport
       ESCAPE_KEY_CHARS = /[\x00-\x20%\x7F-\xFF]/n
 
       # Creates a new Dalli::Client instance with specified addresses and options.
-      # By default address is equal localhost:11211.
+      # If no addresses are provided, we give nil to Dalli::Client, so it uses its fallbacks:
+      # - ENV["MEMCACHE_SERVERS"] (if defined)
+      # - "127.0.0.1:11211"        (otherwise)
       #
       #   ActiveSupport::Cache::MemCacheStore.build_mem_cache
-      #     # => #<Dalli::Client:0x007f98a47d2028 @servers=["localhost:11211"], @options={}, @ring=nil>
+      #     # => #<Dalli::Client:0x007f98a47d2028 @servers=["127.0.0.1:11211"], @options={}, @ring=nil>
       #   ActiveSupport::Cache::MemCacheStore.build_mem_cache('localhost:10290')
       #     # => #<Dalli::Client:0x007f98a47b3a60 @servers=["localhost:10290"], @options={}, @ring=nil>
       def self.build_mem_cache(*addresses) # :nodoc:
         addresses = addresses.flatten
         options = addresses.extract_options!
-        addresses = ["localhost:11211"] if addresses.empty?
+        addresses = nil if addresses.empty?
         pool_options = retrieve_pool_options(options)
 
         if pool_options.empty?
@@ -79,8 +81,8 @@ module ActiveSupport
       #
       #   ActiveSupport::Cache::MemCacheStore.new("localhost", "server-downstairs.localnetwork:8229")
       #
-      # If no addresses are specified, then MemCacheStore will connect to
-      # localhost port 11211 (the default memcached port).
+      # If no addresses are provided, but ENV['MEMCACHE_SERVERS'] is defined, it will be used instead. Otherwise,
+      # MemCacheStore will connect to localhost:11211 (the default memcached port).
       def initialize(*addresses)
         addresses = addresses.flatten
         options = addresses.extract_options!

activesupport/test/cache/stores/mem_cache_store_test.rb

@@ -180,10 +180,20 @@ class MemCacheStoreTest < ActiveSupport::TestCase
     assert_equal expected_addresses, servers(cache)
   end
 
-  def test_falls_back_to_localhost_if_no_address_provided
-    cache = ActiveSupport::Cache.lookup_store(:mem_cache_store)
+  def test_falls_back_to_localhost_if_no_address_provided_and_memcache_servers_undefined
+    with_memcache_servers_environment_variable(nil) do
+      cache = ActiveSupport::Cache.lookup_store(:mem_cache_store)
 
-    assert_equal ["localhost:11211"], servers(cache)
+      assert_equal ["127.0.0.1:11211"], servers(cache)
+    end
+  end
+
+  def test_falls_back_to_localhost_if_no_address_provided_and_memcache_servers_defined
+    with_memcache_servers_environment_variable("custom_host") do
+      cache = ActiveSupport::Cache.lookup_store(:mem_cache_store)
+
+      assert_equal ["custom_host"], servers(cache)
+    end
   end
 
   private
@@ -222,4 +232,16 @@ class MemCacheStoreTest < ActiveSupport::TestCase
     def client(cache = @cache)
       cache.instance_variable_get(:@data)
     end
+
+    def with_memcache_servers_environment_variable(value)
+      original_value = ENV["MEMCACHE_SERVERS"]
+      ENV["MEMCACHE_SERVERS"] = value
+      yield
+    ensure
+      if original_value.nil?
+        ENV.delete("MEMCACHE_SERVERS")
+      else
+        ENV["MEMCACHE_SERVERS"] = original_value
+      end
+    end
 end

guides/CHANGELOG.md

@@ -1,3 +1,7 @@
+*   Updated `ActiveSupport::Cache::MemCacheStore` docs to reflect support for `$MEMCACHE_SERVERS`.
+
+    *Sam Bostock*
+
 *   Use Bookstore as a unified use-case for all examples in Active Record Query Interface Guide.
 
     *Ashley Engelund*, *Vipul A M*

guides/source/caching_with_rails.md

@@ -453,17 +453,22 @@ no explicit `config.cache_store` is supplied.
 
 This cache store uses Danga's `memcached` server to provide a centralized cache for your application. Rails uses the bundled `dalli` gem by default. This is currently the most popular cache store for production websites. It can be used to provide a single, shared cache cluster with very high performance and redundancy.
 
-When initializing the cache, you need to specify the addresses for all
-memcached servers in your cluster. If none are specified, it will assume
-memcached is running on localhost on the default port, but this is not an ideal
-setup for larger sites.
-
-The `write` and `fetch` methods on this cache accept two additional options that take advantage of features specific to memcached. You can specify `:raw` to send a value directly to the server with no serialization. The value must be a string or number. You can use memcached direct operations like `increment` and `decrement` only on raw values. You can also specify `:unless_exist` if you don't want memcached to overwrite an existing entry.
+When initializing the cache, you should specify the addresses for all memcached servers in your cluster, or ensure the `MEMCACHE_SERVERS` environment variable has been set appropriately.
 
 ```ruby
 config.cache_store = :mem_cache_store, "cache-1.example.com", "cache-2.example.com"
 ```
 
+If neither are specified, it will assume memcached is running on localhost on the default port (`127.0.0.1:11211`), but this is not an ideal setup for larger sites.
+
+```ruby
+config.cache_store = :mem_cache_store # Will fallback to $MEMCACHE_SERVERS, then 127.0.0.1:11211
+```
+
+See the [`Dalli::Client` documentation](https://www.rubydoc.info/github/mperham/dalli/Dalli%2FClient:initialize) for supported address types.
+
+The `write` and `fetch` methods on this cache accept two additional options that take advantage of features specific to memcached. You can specify `:raw` to send a value directly to the server with no serialization. The value must be a string or number. You can use memcached direct operations like `increment` and `decrement` only on raw values. You can also specify `:unless_exist` if you don't want memcached to overwrite an existing entry.
+
 ### ActiveSupport::Cache::RedisCacheStore
 
 The Redis cache store takes advantage of Redis support for automatic eviction