Skip to content

Commit

Permalink
Do not call should_use_cache? on prefetched/embedded assocs
Browse files Browse the repository at this point in the history
  • Loading branch information
kirs committed Jan 18, 2024
1 parent 7ad52af commit 980eade
Show file tree
Hide file tree
Showing 8 changed files with 55 additions and 15 deletions.
2 changes: 1 addition & 1 deletion lib/identity_cache/cached/belongs_to.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 4 additions & 1 deletion lib/identity_cache/cached/primary_index.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion lib/identity_cache/cached/recursive/association.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion lib/identity_cache/cached/reference/has_many.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions lib/identity_cache/encoder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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|
Expand Down
10 changes: 10 additions & 0 deletions lib/identity_cache/should_use_cache.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
11 changes: 0 additions & 11 deletions test/denormalized_has_many_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
37 changes: 37 additions & 0 deletions test/fetch_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit 980eade

Please sign in to comment.