Skip to content

Commit

Permalink
Merge pull request #449 from Shopify/fix-fetch-assoc-after-add
Browse files Browse the repository at this point in the history
Fix fetch has_many id embedded association after adding to it
  • Loading branch information
dylanahsmith committed Apr 6, 2020
2 parents 684bdc4 + 21634dc commit 6047e4d
Show file tree
Hide file tree
Showing 7 changed files with 21 additions and 7 deletions.
2 changes: 1 addition & 1 deletion dev.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ name: identity-cache
up:
- homebrew:
- postgresql
- ruby: 2.4.1
- ruby: 2.4.10
- railgun
- bundler

Expand Down
4 changes: 2 additions & 2 deletions lib/identity_cache/cached/reference/has_many.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ def #{cached_ids_name}
end
def #{cached_accessor_name}
association_klass = association(:#{name}).klass
if association_klass.should_use_cache? && !#{name}.loaded?
assoc = association(:#{name})
if 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
4 changes: 2 additions & 2 deletions lib/identity_cache/cached/reference/has_one.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ def #{cached_id_name}
end
def #{cached_accessor_name}
association_klass = association(:#{name}).klass
if association_klass.should_use_cache? && !association(:#{name}).loaded?
assoc = association(:#{name})
if assoc.klass.should_use_cache? && !assoc.loaded?
#{records_variable_name} ||= #{reflection.class_name}.fetch(#{cached_id_name}) if #{cached_id_name}
else
#{name}
Expand Down
2 changes: 1 addition & 1 deletion lib/identity_cache/query_api.rb
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ def was_new_record? # :nodoc:
def fetch_recursively_cached_association(ivar_name, dehydrated_ivar_name, association_name) # :nodoc:
assoc = association(association_name)

if assoc.klass.should_use_cache? && !assoc.loaded?
if assoc.klass.should_use_cache? && !assoc.loaded? && assoc.target.blank?
if instance_variable_defined?(ivar_name)
instance_variable_get(ivar_name)
elsif instance_variable_defined?(dehydrated_ivar_name)
Expand Down
7 changes: 7 additions & 0 deletions test/denormalized_has_many_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,13 @@ def test_respect_should_use_cache_from_embedded_records
end
end

def test_fetch_association_after_adding_to_it
item = Item.fetch(@record.id)
item.associated_records.create!(name: 'foo')
fetched_associated_records = item.fetch_associated_records
assert_equal(item.associated_records.length, fetched_associated_records.length)
end

class CheckAssociationTest < IdentityCache::TestCase
def test_unsupported_through_assocation
assert_raises IdentityCache::UnsupportedAssociationError, "caching through associations isn't supported" do
Expand Down
7 changes: 7 additions & 0 deletions test/normalized_has_many_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -244,4 +244,11 @@ def test_respects_should_use_cache_on_association
end
end
end

def test_fetch_association_after_adding_to_it
item = Item.fetch(@record.id)
item.associated_records.create!(name: 'foo')
fetched_associated_records = item.fetch_associated_records
assert_equal(item.associated_records.length, fetched_associated_records.length)
end
end
2 changes: 1 addition & 1 deletion test/prefetch_associations_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ def test_prefetch_associations_on_association

setup_has_many_children_and_grandchildren(@bob)

associated_records = @bob.associated_records
associated_records = Item.find(@bob.id).associated_records

assert_queries(1) do
assert_memcache_operations(1) do
Expand Down

0 comments on commit 6047e4d

Please sign in to comment.