Merge pull request rails#50071 from tgwizard/fix-cache-fetch-multi-read-write-order-of-operations

Adjust instr. for Cache::Store#fetch_multi so writes are after reads

Author: byroot

activesupport/CHANGELOG.md

@@ -1,3 +1,7 @@
+*   Make the order of read_multi and write_multi notifications for `Cache::Store#fetch_multi` operations match the order they are executed in.
+
+    *Adam Renberg Tamm*
+
 *   Make return values of `Cache::Store#write` consistent.
 
     The return value was not specified before. Now it returns `true` on a successful write,

activesupport/lib/active_support/cache.rb

@@ -605,14 +605,14 @@ def fetch_multi(*names)
         options = names.extract_options!
         options = merged_options(options)
 
-        instrument_multi :read_multi, names, options do |payload|
+        writes  = {}
+        ordered = instrument_multi :read_multi, names, options do |payload|
           if options[:force]
             reads = {}
           else
             reads = read_multi_entries(names, **options)
           end
 
-          writes  = {}
           ordered = names.index_with do |name|
             reads.fetch(name) { writes[name] = yield(name) }
           end
@@ -621,10 +621,12 @@ def fetch_multi(*names)
           payload[:hits] = reads.keys
           payload[:super_operation] = :fetch_multi
 
-          write_multi(writes, options)
-
           ordered
         end
+
+        write_multi(writes, options)
+
+        ordered
       end
 
       # Writes the value to the cache with the key. The value must be supported

activesupport/test/cache/behaviors/cache_instrumentation_behavior.rb

@@ -34,6 +34,20 @@ def test_instrumentation_with_fetch_multi_as_super_operation
     assert_equal @cache.class.name, events[0].payload[:store]
   end
 
+  def test_fetch_multi_instrumentation_order_of_operations
+    operations = []
+    callback = ->(name, *) { operations << name }
+
+    key_1 = SecureRandom.uuid
+    key_2 = SecureRandom.uuid
+
+    ActiveSupport::Notifications.subscribed(callback, /^cache_(read_multi|write_multi)\.active_support$/) do
+      @cache.fetch_multi(key_1, key_2) { |key| key * 2 }
+    end
+
+    assert_equal %w[ cache_read_multi.active_support cache_write_multi.active_support ], operations
+  end
+
   def test_read_multi_instrumentation
     key_1 = SecureRandom.uuid
     @cache.write(key_1, SecureRandom.alphanumeric)