diff options
| -rwxr-xr-x | activerecord/lib/active_record/base.rb | 76 | ||||
| -rw-r--r-- | activerecord/lib/active_record/named_scope.rb | 18 | ||||
| -rw-r--r-- | activerecord/lib/active_record/relation.rb | 22 | ||||
| -rw-r--r-- | activerecord/test/cases/finder_test.rb | 46 |
4 files changed, 38 insertions, 124 deletions
diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index b6d73265a8..31737043eb 100755 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -671,20 +671,10 @@ module ActiveRecord #:nodoc: 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 @@ -1687,6 +1677,8 @@ module ActiveRecord #:nodoc: 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)). @@ -1701,6 +1693,21 @@ module ActiveRecord #:nodoc: 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 @@ -1853,48 +1860,9 @@ module ActiveRecord #:nodoc: 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) diff --git a/activerecord/lib/active_record/named_scope.rb b/activerecord/lib/active_record/named_scope.rb index 96d361093f..a6336e762a 100644 --- a/activerecord/lib/active_record/named_scope.rb +++ b/activerecord/lib/active_record/named_scope.rb @@ -26,23 +26,11 @@ module ActiveRecord 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 diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index f9ca6cbbd2..bcae352cc6 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -129,7 +129,7 @@ module ActiveRecord self end - private + protected def method_missing(method, *args, &block) if @relation.respond_to?(method) @@ -141,20 +141,24 @@ module ActiveRecord 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 diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index efc850cffc..59c016d19c 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -571,21 +571,6 @@ class FinderTest < ActiveRecord::TestCase 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!") @@ -596,21 +581,6 @@ class FinderTest < ActiveRecord::TestCase 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') @@ -654,14 +624,6 @@ class FinderTest < ActiveRecord::TestCase 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' } @@ -694,14 +656,6 @@ class FinderTest < ActiveRecord::TestCase 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") } |