Merge pull request #34700 from gmcgibbon/fetch_multi_key_order

Preserve key order of #fetch_multi

3 files changed, 24 insertions, 10 deletions

diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md
index e678f48244..360cef2b41 100644
--- a/activesupport/CHANGELOG.md
+++ b/activesupport/CHANGELOG.md
@@ -1,3 +1,9 @@
+*   Preserve key order passed to `ActiveSupport::CacheStore#fetch_multi`.
+
+    `fetch_multi(*names)` now returns its results in the same order as the `*names` requested, rather than returning cache hits followed by cache misses.
+
+    *Gannon McGibbon*
+
 *   If the same block is `included` multiple times for a Concern, an exception is no longer raised.
 
     *Mark J. Titorenko*, *Vlad Bokov*
diff --git a/activesupport/lib/active_support/cache.rb b/activesupport/lib/active_support/cache.rb
index e8518645d9..30a69c550b 100644
--- a/activesupport/lib/active_support/cache.rb
+++ b/activesupport/lib/active_support/cache.rb
@@ -438,18 +438,18 @@ module ActiveSupport
         options = merged_options(options)
 
         instrument :read_multi, names, options do |payload|
-          read_multi_entries(names, options).tap do |results|
-            payload[:hits] = results.keys
-            payload[:super_operation] = :fetch_multi
+          reads   = read_multi_entries(names, options)
+          writes  = {}
+          ordered = names.each_with_object({}) do |name, hash|
+            hash[name] = reads.fetch(name) { writes[name] = yield(name) }
+          end
 
-            writes = {}
+          payload[:hits] = reads.keys
+          payload[:super_operation] = :fetch_multi
 
-            (names - results.keys).each do |name|
-              results[name] = writes[name] = yield(name)
-            end
+          write_multi(writes, options)
 
-            write_multi writes, options
-          end
+          ordered
         end
       end
 
diff --git a/activesupport/test/cache/behaviors/cache_store_behavior.rb b/activesupport/test/cache/behaviors/cache_store_behavior.rb
index 9f54b1e7de..a696760bb2 100644
--- a/activesupport/test/cache/behaviors/cache_store_behavior.rb
+++ b/activesupport/test/cache/behaviors/cache_store_behavior.rb
@@ -130,7 +130,7 @@ module CacheStoreBehavior
     assert_equal("fufu", @cache.read("fu"))
   end
 
-  def test_multi_with_objects
+  def test_fetch_multi_with_objects
     cache_struct = Struct.new(:cache_key, :title)
     foo = cache_struct.new("foo", "FOO!")
     bar = cache_struct.new("bar")
@@ -142,6 +142,14 @@ module CacheStoreBehavior
     assert_equal({ foo => "FOO!", bar => "BAM!" }, values)
   end
 
+  def test_fetch_multi_returns_ordered_names
+    @cache.write("bam", "BAM")
+
+    values = @cache.fetch_multi("foo", "bar", "bam") { |key| key.upcase }
+
+    assert_equal(%w(foo bar bam), values.keys)
+  end
+
   def test_fetch_multi_without_block
     assert_raises(ArgumentError) do
       @cache.fetch_multi("foo")