Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use loaded model if possible to prevent extra queries #23

Merged
merged 7 commits into from
May 13, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 20 additions & 3 deletions lib/active_model_cachers/active_record/attr_model.rb
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,26 @@ def collection?
return @reflect.collection?
end

def query_model(id)
return @klass.find_by(id: id) if @column == nil # Cache self
return @klass.where(id: id).limit(1).pluck(@column).first if not association? # Cache attributes
def query_model(binding, id)
return query_self(binding, id) if @column == nil
return query_association(binding, id) if association?
return query_attribute(binding, id)
end

private

def query_self(binding, id)
return binding if binding.is_a?(::ActiveRecord::Base)
return @klass.find_by(id: id)
end

def query_attribute(binding, id)
return binding.send(@column) if binding.is_a?(::ActiveRecord::Base) and binding.has_attribute?(@column)
return @klass.where(id: id).limit(1).pluck(@column).first
end

def query_association(binding, id)
return binding.send(@column) if binding.is_a?(::ActiveRecord::Base)
id = @reflect.active_record.where(id: id).limit(1).pluck(foreign_key).first if foreign_key != 'id'
if @reflect.collection?
return id ? @reflect.klass.where(@reflect.foreign_key => id).to_a : []
Expand Down
9 changes: 6 additions & 3 deletions lib/active_model_cachers/active_record/cacher.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,16 +37,19 @@ def initialize(id: nil, model: nil)
private

def exec_by(attr, primary_key, service_klasses, method)
bindings = [@model]
if @model and attr.association?
if attr.has_one?
data = @model.send(attr.column).try(primary_key)
else
data = @model.send(attr.foreign_key)
service_klasses = [service_klasses.last]
bindings << @model.send(attr.column) if @model.is_a?(::ActiveRecord::Base)
end
end
data ||= (@model ? @model.send(primary_key) : nil) || @id
service_klasses.all?{|s| (data = s.instance(data).send(method, binding: @model)) != nil }
service_klasses.each_with_index do |service_klass, index|
data = service_klass.instance(data).send(method, binding: bindings[index])
return if data == nil
end
return data
end
end
Expand Down
2 changes: 1 addition & 1 deletion lib/active_model_cachers/active_record/extension.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ def cache_at(column, query = nil, expire_by: nil, on: nil, foreign_key: nil, pri
attr = AttrModel.new(self, column)
return cache_belongs_to(attr) if attr.belongs_to?

query ||= ->(id){ attr.query_model(id) }
query ||= ->(id){ attr.query_model(self, id) }
service_klass = CacheServiceFactory.create_for_active_model(attr, query)
Cacher.define_cacher_method(attr, primary_key, [service_klass])

Expand Down
7 changes: 7 additions & 0 deletions test/cache_at_attribute_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,13 @@ def test_basic_usage_of_instance_cacher
assert_cache('active_model_cachers_Profile_at_point_1' => 10)
end

def test_instance_cacher_to_use_loaded_associations
profile = Profile.first

assert_queries(0){ assert_equal 10, profile.cacher.point }
assert_cache('active_model_cachers_Profile_at_point_1' => 10)
end

# ----------------------------------------------------------------
# ● Create
# ----------------------------------------------------------------
Expand Down
17 changes: 15 additions & 2 deletions test/cache_at_belongs_to_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,26 @@ def test_basic_usage

def test_basic_usage_of_instance_cacher
user = User.find_by(name: 'John1')
language = user.language

assert_queries(1){ assert_equal 'zh-tw', user.cacher.language.name }
assert_queries(0){ assert_equal 'zh-tw', user.cacher.language.name }
assert_cache('active_model_cachers_Language_2' => language)
assert_cache('active_model_cachers_User_at_language_id_1' => 2, 'active_model_cachers_Language_2' => user.language)
end

def test_instance_cacher_to_use_loaded_associations
user = User.find_by(name: 'John1')
language = user.language

assert_queries(0){ assert_equal 'zh-tw', user.cacher.language.name }
assert_cache('active_model_cachers_User_at_language_id_1' => 2, 'active_model_cachers_Language_2' => language)
end

def test_instance_cacher_to_use_preloaded_associations
user = User.includes(:language).find_by(name: 'John1')

assert_queries(0){ assert_equal 'zh-tw', user.cacher.language.name }
assert_cache('active_model_cachers_User_at_language_id_1' => 2, 'active_model_cachers_Language_2' => user.language)
end
# ----------------------------------------------------------------
# ● Create
# ----------------------------------------------------------------
Expand Down
15 changes: 15 additions & 0 deletions test/cache_at_has_many_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,25 @@ def test_basic_usage_of_instance_cacher
posts = user.posts

assert_queries(1){ assert_equal 3, user.cacher.posts.size }
assert_queries(0){ assert_equal 3, user.cacher.posts.size }
assert_cache('active_model_cachers_User_at_posts_1' => user.posts)
end

def test_instance_cacher_to_use_loaded_associations
user = User.find_by(name: 'John1')
posts = user.posts.to_a # to_a to make sure posts is loaded

assert_queries(0){ assert_equal 3, user.cacher.posts.size }
assert_cache('active_model_cachers_User_at_posts_1' => posts)
end

def test_instance_cacher_to_use_preloaded_associations
user = User.includes(:posts).find_by(name: 'John1')

assert_queries(0){ assert_equal 3, user.cacher.posts.size }
assert_cache('active_model_cachers_User_at_posts_1' => user.posts)
end

# ----------------------------------------------------------------
# ● Create
# ----------------------------------------------------------------
Expand Down
16 changes: 15 additions & 1 deletion test/cache_at_has_one_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,27 @@ def test_basic_usage

def test_basic_usage_of_instance_cacher
user = User.find_by(name: 'John2')
profile = user.profile

assert_queries(1){ assert_equal 10, user.cacher.profile.point }
assert_queries(0){ assert_equal 10, user.cacher.profile.point }
assert_cache('active_model_cachers_Profile_1' => user.profile)
end

def test_instance_cacher_to_use_loaded_associations
user = User.find_by(name: 'John2')
profile = user.profile

assert_queries(0){ assert_equal 10, user.cacher.profile.point }
assert_cache('active_model_cachers_Profile_1' => profile)
end

def test_instance_cacher_to_use_preloaded_associations
user = User.includes(:profile).find_by(name: 'John2')

assert_queries(0){ assert_equal 10, user.cacher.profile.point }
assert_cache('active_model_cachers_Profile_1' => user.profile)
end

# ----------------------------------------------------------------
# ● Create
# ----------------------------------------------------------------
Expand Down