Merge pull request rails#54586 from byroot/local-store-fetch-multi-recorded-miss

Fix LocalStore#read_multi_entries to distinguish recorded misses

Author: byroot

activesupport/lib/active_support/cache/strategy/local_cache.rb

@@ -108,6 +108,40 @@ def decrement(name, amount = 1, **options) # :nodoc:
           value
         end
 
+        def fetch_multi(*names, &block) # :nodoc:
+          return super if local_cache.nil? || names.empty?
+
+          options = names.extract_options!
+          options = merged_options(options)
+
+          keys_to_names = names.index_by { |name| normalize_key(name, options) }
+
+          local_entries = local_cache.read_multi_entries(keys_to_names.keys)
+          results = local_entries.each_with_object({}) do |(key, value), result|
+            # If we recorded a miss in the local cache, `#fetch_multi` will forward
+            # that key to the real store, and the entry will be replaced
+            # local_cache.delete_entry(key)
+            next if value.nil?
+
+            entry = deserialize_entry(value, **options)
+
+            normalized_key = keys_to_names[key]
+            if entry.nil?
+              result[normalized_key] = nil
+            elsif entry.expired? || entry.mismatched?(normalize_version(normalized_key, options))
+              local_cache.delete_entry(key)
+            else
+              result[normalized_key] = entry.value
+            end
+          end
+
+          if results.size < names.size
+            results.merge!(super(*(names - results.keys), options, &block))
+          end
+
+          results
+        end
+
         private
           def read_serialized_entry(key, raw: false, **options)
             if cache = local_cache
@@ -131,6 +165,8 @@ def read_multi_entries(names, **options)
             local_entries = local_cache.read_multi_entries(keys_to_names.keys)
 
             results = local_entries.each_with_object({}) do |(key, value), result|
+              next if value.nil? # recorded cache miss
+
               entry = deserialize_entry(value, **options)
 
               normalized_key = keys_to_names[key]

activesupport/test/cache/behaviors/local_cache_behavior.rb

@@ -124,6 +124,17 @@ def test_local_cache_fetch
     end
   end
 
+  def test_local_cache_fetch_on_miss
+    key = SecureRandom.uuid
+    @cache.with_local_cache do
+      assert_equal false, @cache.exist?(key)
+      value = @cache.fetch(key) { "fetch-yielded" }
+      assert_equal "fetch-yielded", value
+
+      assert_equal "fetch-yielded", @peek.read(key)
+    end
+  end
+
   def test_local_cache_of_write_nil
     key = SecureRandom.uuid
     value = SecureRandom.alphanumeric
@@ -215,14 +226,24 @@ def test_local_cache_of_decrement
   end
 
   def test_local_cache_of_fetch_multi
-    key = SecureRandom.uuid
-    other_key = SecureRandom.uuid
+    existing_key = SecureRandom.uuid
+    known_missing_key = SecureRandom.uuid
+    unknown_key = SecureRandom.uuid
+
     @cache.with_local_cache do
-      @cache.fetch_multi(key, other_key) { |_key| true }
-      @peek.delete(key)
-      @peek.delete(other_key)
-      assert_equal true, @cache.read(key)
-      assert_equal true, @cache.read(other_key)
+      @cache.fetch(existing_key) { "exist" }
+      assert_equal false, @cache.exist?("known-missing")
+
+      results = @cache.fetch_multi(known_missing_key, existing_key, unknown_key) { "fetch-yielded" }
+      expected = {
+        known_missing_key => "fetch-yielded",
+        existing_key => "exist",
+        unknown_key => "fetch-yielded",
+      }
+      assert_equal(expected, results)
+
+      results = @peek.read_multi(known_missing_key, existing_key, unknown_key)
+      assert_equal expected, results
     end
   end
 

activesupport/test/cache/stores/null_store_test.rb

@@ -85,7 +85,7 @@ def test_local_store_repeated_reads
       assert_nil @cache.read("foo")
 
       @cache.read_multi("foo", "bar")
-      assert_equal({ "foo" => nil, "bar" => nil }, @cache.read_multi("foo", "bar"))
+      assert_equal({}, @cache.read_multi("foo", "bar"))
     end
   end
 end