From 980eade00d00c508c68ee1cff146dd70ffb72f1d Mon Sep 17 00:00:00 2001 From: Kir Shatrov Date: Thu, 18 Jan 2024 10:11:23 -0800 Subject: [PATCH] Do not call should_use_cache? on prefetched/embedded assocs --- lib/identity_cache/cached/belongs_to.rb | 2 +- lib/identity_cache/cached/primary_index.rb | 5 ++- .../cached/recursive/association.rb | 2 +- .../cached/reference/has_many.rb | 2 +- lib/identity_cache/encoder.rb | 1 + lib/identity_cache/should_use_cache.rb | 10 +++++ test/denormalized_has_many_test.rb | 11 ------ test/fetch_test.rb | 37 +++++++++++++++++++ 8 files changed, 55 insertions(+), 15 deletions(-) diff --git a/lib/identity_cache/cached/belongs_to.rb b/lib/identity_cache/cached/belongs_to.rb index e5133469..86a6b749 100644 --- a/lib/identity_cache/cached/belongs_to.rb +++ b/lib/identity_cache/cached/belongs_to.rb @@ -9,7 +9,7 @@ def build reflection.active_record.class_eval(<<-RUBY, __FILE__, __LINE__ + 1) def #{cached_accessor_name} association_klass = association(:#{name}).klass - if association_klass.should_use_cache? && #{reflection.foreign_key}.present? && !association(:#{name}).loaded? + if (loaded_by_idc? || association_klass.should_use_cache?) && #{reflection.foreign_key}.present? && !association(:#{name}).loaded? if defined?(#{records_variable_name}) #{records_variable_name} else diff --git a/lib/identity_cache/cached/primary_index.rb b/lib/identity_cache/cached/primary_index.rb index 0f96aad8..d6daa8fe 100644 --- a/lib/identity_cache/cached/primary_index.rb +++ b/lib/identity_cache/cached/primary_index.rb @@ -54,7 +54,10 @@ def cache_key(id) def load_one_from_db(id) record = build_query(id).take - model.send(:setup_embedded_associations_on_miss, [record]) if record + if record + model.send(:setup_embedded_associations_on_miss, [record]) + record.send(:mark_as_loaded_by_idc) + end record end diff --git a/lib/identity_cache/cached/recursive/association.rb b/lib/identity_cache/cached/recursive/association.rb index e762d81c..7459b3e2 100644 --- a/lib/identity_cache/cached/recursive/association.rb +++ b/lib/identity_cache/cached/recursive/association.rb @@ -25,7 +25,7 @@ def build def read(record) assoc = record.association(name) - if assoc.klass.should_use_cache? && !assoc.loaded? && assoc.target.blank? + if (record.send(:loaded_by_idc?) || assoc.klass.should_use_cache?) && !assoc.loaded? && assoc.target.blank? if record.instance_variable_defined?(records_variable_name) record.instance_variable_get(records_variable_name) elsif record.instance_variable_defined?(dehydrated_variable_name) diff --git a/lib/identity_cache/cached/reference/has_many.rb b/lib/identity_cache/cached/reference/has_many.rb index a9980803..62ab68b6 100644 --- a/lib/identity_cache/cached/reference/has_many.rb +++ b/lib/identity_cache/cached/reference/has_many.rb @@ -22,7 +22,7 @@ def #{cached_ids_name} def #{cached_accessor_name} assoc = association(:#{name}) - if assoc.klass.should_use_cache? && !assoc.loaded? && assoc.target.blank? + if (loaded_by_idc? || assoc.klass.should_use_cache?) && !assoc.loaded? && assoc.target.blank? #{records_variable_name} ||= #{reflection.class_name}.fetch_multi(#{cached_ids_name}) else #{name}.to_a diff --git a/lib/identity_cache/encoder.rb b/lib/identity_cache/encoder.rb index 27b4f669..f6dc671d 100644 --- a/lib/identity_cache/encoder.rb +++ b/lib/identity_cache/encoder.rb @@ -69,6 +69,7 @@ def embedded_coder(record, _association, cached_association) def record_from_coder(coder, klass) # :nodoc: record = klass.instantiate(coder[:attributes].dup) + record.send(:mark_as_loaded_by_idc) if coder.key?(:associations) coder[:associations].each do |name, value| diff --git a/lib/identity_cache/should_use_cache.rb b/lib/identity_cache/should_use_cache.rb index 47363be8..b56fd682 100644 --- a/lib/identity_cache/should_use_cache.rb +++ b/lib/identity_cache/should_use_cache.rb @@ -9,5 +9,15 @@ def should_use_cache? IdentityCache.should_use_cache? end end + + private + + def mark_as_loaded_by_idc + @loaded_by_idc = true + end + + def loaded_by_idc? + defined?(@loaded_by_idc) + end end end diff --git a/test/denormalized_has_many_test.rb b/test/denormalized_has_many_test.rb index a14ceeb5..45b4ecac 100644 --- a/test/denormalized_has_many_test.rb +++ b/test/denormalized_has_many_test.rb @@ -171,17 +171,6 @@ def test_returned_records_with_open_transactions_should_not_be_readonly end end - def test_respect_should_use_cache_from_embedded_records - Item.fetch(@record.id) - AssociatedRecord.stubs(:should_use_cache?).returns(false) - - assert_memcache_operations(1) do - assert_queries(1) do - Item.fetch(@record.id).fetch_associated_records - end - end - end - def test_fetch_association_after_adding_to_it item = Item.fetch(@record.id) item.associated_records.create!(name: "foo") diff --git a/test/fetch_test.rb b/test/fetch_test.rb index 7c11297c..0fcb7c11 100644 --- a/test/fetch_test.rb +++ b/test/fetch_test.rb @@ -342,6 +342,43 @@ def test_returned_records_are_not_readonly_with_open_transactions end end + def test_should_use_cache_not_called_on_prefetched + Item.send(:cache_belongs_to, :item) + + bob = Item.create!(title: "bob") + john = Item.create!(title: "john") + bob.update_column(:item_id, john.id) + + item = Item.fetch(bob.id, includes: [:item]) + IdentityCache.expects(:should_use_cache?).never + item.fetch_item + end + + def test_should_use_cache_not_called_on_embedded_has_many + Item.send(:cache_has_many, :associated_records, embed: true) + + bob = Item.create!(title: "bob") + + bob.associated_records.create!(name: "foo") + bob.associated_records.create!(name: "bar") + + item = Item.fetch(bob.id) + IdentityCache.expects(:should_use_cache?).never + records = item.fetch_associated_records + assert_equal 2, records.size + end + + def test_should_use_cache_not_called_on_embedded_has_one + Item.send(:cache_has_one, :related_item, embed: true) + + bob = Item.create!(title: "bob") + bob.build_related_item.save! + + item = Item.fetch(bob.id) + IdentityCache.expects(:should_use_cache?).never + assert_predicate item.fetch_related_item, :present? + end + def test_respects_should_use_cache_on_record @record.save Item.stubs(:should_use_cache?).returns(false)