Skip to content

Commit

Permalink
Make Model.find_by_* and Model.find_all_by_* use relations and remove…
Browse files Browse the repository at this point in the history 
… dynamic method caching
  • Loading branch information
@lifo
lifo committed Dec 27, 2009
1 parent 51d84ef commit 8829d6e
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 124 deletions.
76 changes: 22 additions & 54 deletions activerecord/lib/active_record/base.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -671,20 +671,10 @@ def all(*args)
options = args.extract_options!

if options.empty? && !scoped?(:find)
relation = arel_table
arel_table
else
relation = construct_finder_arel(options)
include_associations = merge_includes(scope(:find, :include), options[:include])

if include_associations.any?
if references_eager_loaded_tables?(options)
relation.eager_load(include_associations)
else
relation.preload(include_associations)
end
end
construct_finder_arel_with_includes(options)
end
relation
end

# Executes a custom SQL query against your database and returns all the results. The results will
Expand Down Expand Up @@ -1687,6 +1677,8 @@ def default_select(qualified)

def construct_finder_arel(options = {}, scope = scope(:find))
# TODO add lock to Arel
validate_find_options(options)

relation = arel_table(options[:from]).
joins(construct_join(options[:joins], scope)).
where(construct_conditions(options[:conditions], scope)).
Expand All @@ -1701,6 +1693,21 @@ def construct_finder_arel(options = {}, scope = scope(:find))
relation
end

def construct_finder_arel_with_includes(options = {})
relation = construct_finder_arel(options)
include_associations = merge_includes(scope(:find, :include), options[:include])

if include_associations.any?
if references_eager_loaded_tables?(options)
relation = relation.eager_load(include_associations)
else
relation = relation.preload(include_associations)
end
end

relation
end

def construct_finder_sql(options, scope = scope(:find))
construct_finder_arel(options, scope).to_sql
end
Expand Down Expand Up @@ -1853,48 +1860,9 @@ def method_missing(method_id, *arguments, &block)
attribute_names = match.attribute_names
super unless all_attributes_exists?(attribute_names)
if match.finder?
finder = match.finder
bang = match.bang?
# def self.find_by_login_and_activated(*args)
# options = args.extract_options!
# attributes = construct_attributes_from_arguments(
# [:login,:activated],
# args
# )
# finder_options = { :conditions => attributes }
# validate_find_options(options)
# set_readonly_option!(options)
#
# if options[:conditions]
# with_scope(:find => finder_options) do
# find(:first, options)
# end
# else
# find(:first, options.merge(finder_options))
# end
# end
self.class_eval %{
def self.#{method_id}(*args)
options = args.extract_options!
attributes = construct_attributes_from_arguments(
[:#{attribute_names.join(',:')}],
args
)
finder_options = { :conditions => attributes }
validate_find_options(options)
set_readonly_option!(options)
#{'result = ' if bang}if options[:conditions]
with_scope(:find => finder_options) do
find(:#{finder}, options)
end
else
find(:#{finder}, options.merge(finder_options))
end
#{'result || raise(RecordNotFound, "Couldn\'t find #{name} with #{attributes.to_a.collect { |pair| pair.join(\' = \') }.join(\', \')}")' if bang}
end
}, __FILE__, __LINE__
send(method_id, *arguments)
options = arguments.extract_options!
relation = options.any? ? construct_finder_arel_with_includes(options) : scoped
relation.send :find_by_attributes, match, attribute_names, *arguments
elsif match.instantiator?
instantiator = match.instantiator
# def self.find_or_create_by_user_id(*args)
Expand Down
18 changes: 3 additions & 15 deletions activerecord/lib/active_record/named_scope.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,23 +26,11 @@ def scoped(options = {}, &block)
if options.present?
Scope.new(self, options, &block)
else
if !scoped?(:find)
relation = arel_table
relation = relation.where(type_condition) if finder_needs_type_condition?
unless scoped?(:find)
finder_needs_type_condition? ? arel_table.where(type_condition) : arel_table
else
relation = construct_finder_arel
include_associations = scope(:find, :include)

if include_associations.present?
if references_eager_loaded_tables?(options)
relation.eager_load(include_associations)
else
relation.preload(include_associations)
end
end
construct_finder_arel_with_includes
end

relation
end
end

Expand Down
22 changes: 13 additions & 9 deletions activerecord/lib/active_record/relation.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ def reload
self
end

private
protected

def method_missing(method, *args, &block)
if @relation.respond_to?(method)
Expand All @@ -141,20 +141,24 @@ def method_missing(method, *args, &block)
super unless @klass.send(:all_attributes_exists?, attributes)

if match.finder?
conditions = attributes.inject({}) {|h, a| h[a] = args[attributes.index(a)]; h}
result = where(conditions).send(match.finder)

if match.bang? && result.blank?
raise RecordNotFound, "Couldn't find #{@klass.name} with #{conditions.to_a.collect {|p| p.join(' = ')}.join(', ')}"
else
result
end
find_by_attributes(match, attributes, *args)
end
else
super
end
end

def find_by_attributes(match, attributes, *args)
conditions = attributes.inject({}) {|h, a| h[a] = args[attributes.index(a)]; h}
result = where(conditions).send(match.finder)

if match.bang? && result.blank?
raise RecordNotFound, "Couldn't find #{@klass.name} with #{conditions.to_a.collect {|p| p.join(' = ')}.join(', ')}"
else
result
end
end

def create_new_relation(relation, readonly = @readonly, preload = @associations_to_preload, eager_load = @eager_load_associations)
Relation.new(@klass, relation, readonly, preload, eager_load)
end
Expand Down
46 changes: 0 additions & 46 deletions activerecord/test/cases/finder_test.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -571,21 +571,6 @@ def test_count_by_sql
assert_equal(2, Entrant.count_by_sql(["SELECT COUNT(*) FROM entrants WHERE id > ?", 1]))
end

def test_dynamic_finders_should_go_through_the_find_class_method
Topic.expects(:find).with(:first, :conditions => { :title => 'The First Topic!' })
Topic.find_by_title("The First Topic!")

Topic.expects(:find).with(:last, :conditions => { :title => 'The Last Topic!' })
Topic.find_last_by_title("The Last Topic!")

Topic.expects(:find).with(:all, :conditions => { :title => 'A Topic.' })
Topic.find_all_by_title("A Topic.")

Topic.expects(:find).with(:first, :conditions => { :title => 'Does not exist yet for sure!' }).times(2)
Topic.find_or_initialize_by_title('Does not exist yet for sure!')
Topic.find_or_create_by_title('Does not exist yet for sure!')
end

def test_find_by_one_attribute
assert_equal topics(:first), Topic.find_by_title("The First Topic")
assert_nil Topic.find_by_title("The First Topic!")
Expand All @@ -596,21 +581,6 @@ def test_find_by_one_attribute_bang
assert_raise(ActiveRecord::RecordNotFound) { Topic.find_by_title!("The First Topic!") }
end

def test_find_by_one_attribute_caches_dynamic_finder
# ensure this test can run independently of order
class << Topic; self; end.send(:remove_method, :find_by_title) if Topic.public_methods.any? { |m| m.to_s == 'find_by_title' }
assert !Topic.public_methods.any? { |m| m.to_s == 'find_by_title' }
t = Topic.find_by_title("The First Topic")
assert Topic.public_methods.any? { |m| m.to_s == 'find_by_title' }
end

def test_dynamic_finder_returns_same_results_after_caching
# ensure this test can run independently of order
class << Topic; self; end.send(:remove_method, :find_by_title) if Topic.public_method_defined?(:find_by_title)
t = Topic.find_by_title("The First Topic")
assert_equal t, Topic.find_by_title("The First Topic") # find_by_title has been cached
end

def test_find_by_one_attribute_with_order_option
assert_equal accounts(:signals37), Account.find_by_credit_limit(50, :order => 'id')
assert_equal accounts(:rails_core_account), Account.find_by_credit_limit(50, :order => 'id DESC')
Expand Down Expand Up @@ -654,14 +624,6 @@ def test_find_by_two_attributes_with_one_being_an_aggregate
assert_equal customers(:david), found_customer
end

def test_dynamic_finder_on_one_attribute_with_conditions_caches_method
# ensure this test can run independently of order
class << Account; self; end.send(:remove_method, :find_by_credit_limit) if Account.public_methods.any? { |m| m.to_s == 'find_by_credit_limit' }
assert !Account.public_methods.any? { |m| m.to_s == 'find_by_credit_limit' }
a = Account.find_by_credit_limit(50, :conditions => ['firm_id = ?', 6])
assert Account.public_methods.any? { |m| m.to_s == 'find_by_credit_limit' }
end

def test_dynamic_finder_on_one_attribute_with_conditions_returns_same_results_after_caching
# ensure this test can run independently of order
class << Account; self; end.send(:remove_method, :find_by_credit_limit) if Account.public_methods.any? { |m| m.to_s == 'find_by_credit_limit' }
Expand Down Expand Up @@ -694,14 +656,6 @@ def test_find_last_by_one_attribute
assert_nil Topic.find_last_by_title("A title with no matches")
end

def test_find_last_by_one_attribute_caches_dynamic_finder
# ensure this test can run independently of order
class << Topic; self; end.send(:remove_method, :find_last_by_title) if Topic.public_methods.any? { |m| m.to_s == 'find_last_by_title' }
assert !Topic.public_methods.any? { |m| m.to_s == 'find_last_by_title' }
t = Topic.find_last_by_title(Topic.last.title)
assert Topic.public_methods.any? { |m| m.to_s == 'find_last_by_title' }
end

def test_find_last_by_invalid_method_syntax
assert_raise(NoMethodError) { Topic.fail_to_find_last_by_title("The First Topic") }
assert_raise(NoMethodError) { Topic.find_last_by_title?("The First Topic") }
Expand Down

0 comments on commit 8829d6e

Please sign in to comment.