From 9f47b1dc9226595a1831f5b1657af13c3e35913a Mon Sep 17 00:00:00 2001 From: VolodyaAll Date: Wed, 30 Sep 2026 14:05:27 +0300 Subject: [PATCH] Fix cache_belongs_to prefetch with owners sharing a foreign key Cached::BelongsTo#fetch_async indexed the owners to load by foreign key with a plain hash, so when several owners pointed at the same record only the last one kept its entry. The prefetched record was written into that owner only, and every other owner fell back to fetch_by_id, one cache round trip each. Collect every owner per foreign key and write the record into all of them, as the polymorphic branch already does. The collection yielded to nested includes is unchanged. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 2 ++ lib/identity_cache/cached/belongs_to.rb | 13 ++++---- test/prefetch_associations_test.rb | 41 +++++++++++++++++++++++++ 3 files changed, 50 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2c71a69f..774e8227 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## Unreleased +- Fix `cache_belongs_to` prefetch assigning the record to only one of several owners that share a foreign key. + ## 1.6.5 - Improve MemCacheStoreCAS patch to properly pass serialized entries to Dalli. diff --git a/lib/identity_cache/cached/belongs_to.rb b/lib/identity_cache/cached/belongs_to.rb index a8bbb5ca..551b4a20 100644 --- a/lib/identity_cache/cached/belongs_to.rb +++ b/lib/identity_cache/cached/belongs_to.rb @@ -70,21 +70,22 @@ def fetch_async(load_strategy, records) yield batch_records end else - ids_to_owner_record = records.each_with_object({}) do |owner_record, hash| + ids_to_owner_records = records.each_with_object(Hash.new { |h, k| h[k] = [] }) do |owner_record, hash| associated_id = owner_record.send(reflection.foreign_key) if associated_id && !owner_record.instance_variable_defined?(records_variable_name) - hash[associated_id] = owner_record + hash[associated_id] << owner_record end end - if ids_to_owner_record.any? + if ids_to_owner_records.any? load_strategy.load_multi( reflection.klass.cached_primary_index, - ids_to_owner_record.keys + ids_to_owner_records.keys ) do |associated_records_by_id| associated_records_by_id.each do |id, associated_record| - owner_record = ids_to_owner_record.fetch(id) - write(owner_record, associated_record) + ids_to_owner_records.fetch(id).each do |owner_record| + write(owner_record, associated_record) + end end yield associated_records_by_id.values.compact diff --git a/test/prefetch_associations_test.rb b/test/prefetch_associations_test.rb index b0218911..424b06c3 100644 --- a/test/prefetch_associations_test.rb +++ b/test/prefetch_associations_test.rb @@ -100,6 +100,24 @@ def test_prefetch_associations_cached_belongs_to end end + def test_prefetch_associations_cached_belongs_to_with_shared_foreign_key + Item.send(:cache_belongs_to, :item) + @bob.update!(item_id: @fred.id) + @joe.update!(item_id: @fred.id) + @bob.fetch_item + @joe.fetch_item + items = [@bob, @joe].map(&:reload) + + assert_no_queries do + assert_memcache_operations(1) do + prefetch(Item, :item, items) + end + assert_memcache_operations(0) do + assert_equal([@fred, @fred], items.map(&:fetch_item)) + end + end + end + def test_prefetch_associations_notifies_about_hydration Item.send(:cache_belongs_to, :item) @bob.update!(item_id: @joe.id) @@ -346,6 +364,29 @@ def test_fetch_multi_batch_fetches_non_embedded_second_level_belongs_to_associat end end + def test_fetch_multi_batch_fetches_non_embedded_second_level_belongs_to_associations_with_shared_foreign_key + Item.send(:cache_has_many, :associated_records, embed: :ids) + AssociatedRecord.send(:cache_belongs_to, :item_two) + + shared_item_two = ItemTwo.create!(title: "shared") + child_records = [@bob, @bob, @joe].map do |parent| + parent.associated_records.create!(item_two: shared_item_two) + end + + # populate the cache entries and associated children ID variables + ItemTwo.fetch(shared_item_two.id) + AssociatedRecord.fetch_multi(child_records.map(&:id)) + Item.fetch_multi(@bob.id, @joe.id) + + assert_memcache_operations(3) do + cached_bob, cached_joe = Item.fetch_multi( + @bob.id, @joe.id, includes: { associated_records: :item_two } + ) + cached_child_records = cached_bob.fetch_associated_records + cached_joe.fetch_associated_records + assert_equal([shared_item_two] * 3, cached_child_records.map(&:fetch_item_two)) + end + end + def test_fetch_multi_doesnt_batch_fetches_belongs_to_associations_if_the_foreign_key_isnt_present AssociatedRecord.send(:cache_belongs_to, :item) @child = AssociatedRecord.create!(name: "bob child")