Skip to content

Commit

Permalink
Fix: expire_changed_association_uniq_keys and inverse_instance
Browse files Browse the repository at this point in the history
  • Loading branch information
OuYangJinTing committed Apr 2, 2020
1 parent 493243b commit 65482e5
Show file tree
Hide file tree
Showing 7 changed files with 54 additions and 12 deletions.
8 changes: 7 additions & 1 deletion lib/second_level_cache/active_record/preloader.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,13 @@ def records_for(ids, &block)
return super(ids, &block)
end
records_from_cache = ::SecondLevelCache.cache_store.read_multi(*map_cache_keys)
record_marshals = RecordMarshal.load_multi(records_from_cache.values)
record_marshals = RecordMarshal.load_multi(records_from_cache.values) do |record|
# This block is copy from:
# https://github.com/rails/rails/blob/6-0-stable/activerecord/lib/active_record/associations/preloader/association.rb#L101
owner = owners_by_key[convert_key(record[association_key_name])].first
association = owner.association(reflection.name)
association.set_inverse_instance(record)
end

# NOTICE
# Rails.cache.read_multi return hash that has keys only hitted.
Expand Down
4 changes: 2 additions & 2 deletions lib/second_level_cache/mixin.rb
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,8 @@ def write_second_level_cache
alias update_second_level_cache write_second_level_cache

def expire_changed_association_uniq_keys
reflections = klass.reflections.select { |_, reflection| reflection.belongs_to? }
changed_keys = reflections.map { |_, reflection| reflection.foreign_key } & previous_changes.keys
changed_keys = klass.reflect_on_all_associations(:belongs_to).map(&:foreign_key)
.map(&:to_s) & previous_changes.keys
changed_keys.each do |key|
SecondLevelCache.cache_store.delete(klass.send(:cache_uniq_key, key => previous_changes[key][0]))
end
Expand Down
8 changes: 4 additions & 4 deletions lib/second_level_cache/record_marshal.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ def dump(record)
end

# load a cached record
def load(serialized)
def load(serialized, &block)
return unless serialized
# fix issues 19
# fix 2.1.2 object.changed? ActiveRecord::SerializationTypeMismatch: Attribute was supposed to be a Hash, but was a String. -- "{:a=>\"t\", :b=>\"x\"}"
Expand Down Expand Up @@ -45,11 +45,11 @@ def load(serialized)
attributes[key] = value[attributes[key]] if attributes[key].is_a?(String)
end

klass.instantiate(attributes)
klass.instantiate(attributes, &block)
end

def load_multi(serializeds)
serializeds.map { |serialized| load(serialized) }
def load_multi(serializeds, &block)
serializeds.map { |serialized| load(serialized, &block) }
end
end
end
14 changes: 14 additions & 0 deletions test/belongs_to_association_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,18 @@ def test_should_write_belongs_to_association_cache
assert_equal @user, book.user
# assert_not_nil User.read_second_level_cache(@user.id)
end

def test_should_expire_changed_association_uniq_keys_when_foreign_key_is_symbol
reflection = Account.reflect_on_association(:user)
assert reflection.belongs_to?
assert reflection.foreign_key.is_a?(Symbol)

account = @user.create_account(site: "foobar")
@user.reload.account # Write cache
cache_key = Account.send(:cache_uniq_key, reflection.foreign_key => @user.id)
assert_equal @user.id, SecondLevelCache.cache_store.read(cache_key)

account.update_attribute(:user_id, 0)
assert_equal nil, SecondLevelCache.cache_store.read(cache_key)
end
end
2 changes: 1 addition & 1 deletion test/model/account.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,5 @@

class Account < ActiveRecord::Base
second_level_cache expires_in: 3.days
belongs_to :user
belongs_to :user, foreign_key: :user_id
end
2 changes: 1 addition & 1 deletion test/model/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class User < ActiveRecord::Base
serialize :json_options, JSON if ::ActiveRecord::VERSION::STRING >= "4.1.0"
store :extras, accessors: %i[tagline gender]

has_one :account
has_one :account, inverse_of: :user
has_one :forked_user_link, foreign_key: "forked_to_user_id"
has_one :forked_from_user, through: :forked_user_link
has_many :namespaces
Expand Down
28 changes: 25 additions & 3 deletions test/preloader_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,20 +47,20 @@ def test_has_one_preload_caches_includes
])
namespaces = users.map { |user| user.create_namespace(name: user.name) }

assert_queries(2) { User.includes(:namespace).order(id: :asc).to_a } # Write cache
assert_queries(2) { User.includes(:namespace).order(id: :asc).to_a } # Write cache
assert_queries(1) do
assert_equal namespaces, User.includes(:namespace).order(id: :asc).map(&:namespace)
end
end

def test_belongs_to_when_read_multi_missed_from_cache_should_will_fetch_missed_records_from_db
def test_has_one_when_read_multi_missed_from_cache_should_will_fetch_missed_records_from_db
users = User.create([
{ name: "foobar1", email: "[email protected]" },
{ name: "foobar2", email: "[email protected]" },
{ name: "foobar3", email: "[email protected]" }
])
namespaces = users.map { |user| user.create_namespace(name: user.name) }
assert_queries(2) { User.includes(:namespace).order(id: :asc).to_a } # Write cache
assert_queries(2) { User.includes(:namespace).order(id: :asc).to_a } # Write cache
expired_namespace = namespaces.first
expired_namespace.expire_second_level_cache

Expand All @@ -73,6 +73,28 @@ def test_belongs_to_when_read_multi_missed_from_cache_should_will_fetch_missed_r
end
end

def test_has_one_preloader_returns_correct_records_after_modify
user = User.create(name: "foobar", email: "[email protected]")

old_namespace = user.create_namespace(name: "old")
assert_queries(2) { User.includes(:namespace).order(id: :asc).to_a } # Write cache
assert_queries(1) do
assert_equal old_namespace, User.includes(:namespace).first.namespace
end

new_namespace = user.create_namespace(name: "new")
assert_queries(2) { User.includes(:namespace).order(id: :asc).to_a } # Write cache
assert_queries(1) do
assert_equal new_namespace, User.includes(:namespace).first.namespace
end
end

def test_has_one_preloader_caches_includes_tried_set_inverse_instance
User.create(name: "foobar", email: "[email protected]").create_account(site: "foobar")
users = User.includes(:account)
assert_equal users.first.object_id, users.first.account.user.object_id
end

def test_has_many_preloader_returns_correct_results
topic = Topic.create(id: 1)
Post.create(id: 1)
Expand Down

0 comments on commit 65482e5

Please sign in to comment.