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")