diff options
| -rw-r--r-- | activerecord/CHANGELOG | 2 | ||||
| -rwxr-xr-x | activerecord/lib/active_record/base.rb | 19 | ||||
| -rw-r--r-- | activerecord/test/cases/finder_respond_to_test.rb | 76 | ||||
| -rw-r--r-- | activerecord/test/cases/finder_test.rb | 22 |
4 files changed, 106 insertions, 13 deletions
diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index cb8b4ceaca..2a782f78ce 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -1,5 +1,7 @@ *SVN* +* Ensure that respond_to? considers dynamic finder methods. Closes #11538. [floehopper] + * Ensure that save on parent object fails for invalid has_one association. Closes #10518. [Pratik] * Remove duplicate code from associations. [Pratik] diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 92417a429f..8bef5ed2ae 100755 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -1263,6 +1263,13 @@ module ActiveRecord #:nodoc: defined?(@abstract_class) && @abstract_class == true end + def respond_to?(method_id, include_private = false) + if match = matches_dynamic_finder?(method_id) || matches_dynamic_finder_with_initialize_or_create?(method_id) + return true if all_attributes_exists?(extract_attribute_names_from_match(match)) + end + super + end + private def find_initial(options) options.update(:limit => 1) unless options[:include] @@ -1568,7 +1575,7 @@ module ActiveRecord #:nodoc: # Each dynamic finder or initializer/creator is also defined in the class after it is first invoked, so that future # attempts to use it do not run through method_missing. def method_missing(method_id, *arguments) - if match = /^find_(all_by|by)_([_a-zA-Z]\w*)$/.match(method_id.to_s) + if match = matches_dynamic_finder?(method_id) finder = determine_finder(match) attribute_names = extract_attribute_names_from_match(match) @@ -1592,7 +1599,7 @@ module ActiveRecord #:nodoc: end }, __FILE__, __LINE__ send(method_id, *arguments) - elsif match = /^find_or_(initialize|create)_by_([_a-zA-Z]\w*)$/.match(method_id.to_s) + elsif match = matches_dynamic_finder_with_initialize_or_create?(method_id) instantiator = determine_instantiator(match) attribute_names = extract_attribute_names_from_match(match) super unless all_attributes_exists?(attribute_names) @@ -1630,6 +1637,14 @@ module ActiveRecord #:nodoc: end end + def matches_dynamic_finder?(method_id) + /^find_(all_by|by)_([_a-zA-Z]\w*)$/.match(method_id.to_s) + end + + def matches_dynamic_finder_with_initialize_or_create?(method_id) + /^find_or_(initialize|create)_by_([_a-zA-Z]\w*)$/.match(method_id.to_s) + end + def determine_finder(match) match.captures.first == 'all_by' ? :find_every : :find_initial end diff --git a/activerecord/test/cases/finder_respond_to_test.rb b/activerecord/test/cases/finder_respond_to_test.rb new file mode 100644 index 0000000000..4e6fecf11a --- /dev/null +++ b/activerecord/test/cases/finder_respond_to_test.rb @@ -0,0 +1,76 @@ +require "cases/helper" +require 'models/topic' + +class FinderRespondToTest < ActiveRecord::TestCase + + fixtures :topics + + def test_should_preserve_normal_respond_to_behaviour_and_respond_to_newly_added_method + class << Topic; self; end.send(:define_method, :method_added_for_finder_respond_to_test) { } + assert Topic.respond_to?(:method_added_for_finder_respond_to_test) + ensure + class << Topic; self; end.send(:remove_method, :method_added_for_finder_respond_to_test) + end + + def test_should_preserve_normal_respond_to_behaviour_and_respond_to_standard_object_method + assert Topic.respond_to?(:to_s) + end + + def test_should_respond_to_find_by_one_attribute_before_caching + ensure_topic_method_is_not_cached(:find_by_title) + assert Topic.respond_to?(:find_by_title) + end + + def test_should_respond_to_find_all_by_one_attribute + ensure_topic_method_is_not_cached(:find_all_by_title) + assert Topic.respond_to?(:find_all_by_title) + end + + def test_should_respond_to_find_all_by_two_attributes + ensure_topic_method_is_not_cached(:find_all_by_title_and_author_name) + assert Topic.respond_to?(:find_all_by_title_and_author_name) + end + + def test_should_respond_to_find_by_two_attributes + ensure_topic_method_is_not_cached(:find_by_title_and_author_name) + assert Topic.respond_to?(:find_by_title_and_author_name) + end + + def test_should_respond_to_find_or_initialize_from_one_attribute + ensure_topic_method_is_not_cached(:find_or_initialize_by_title) + assert Topic.respond_to?(:find_or_initialize_by_title) + end + + def test_should_respond_to_find_or_initialize_from_two_attributes + ensure_topic_method_is_not_cached(:find_or_initialize_by_title_and_author_name) + assert Topic.respond_to?(:find_or_initialize_by_title_and_author_name) + end + + def test_should_respond_to_find_or_create_from_one_attribute + ensure_topic_method_is_not_cached(:find_or_create_by_title) + assert Topic.respond_to?(:find_or_create_by_title) + end + + def test_should_respond_to_find_or_create_from_two_attributes + ensure_topic_method_is_not_cached(:find_or_create_by_title_and_author_name) + assert Topic.respond_to?(:find_or_create_by_title_and_author_name) + end + + def test_should_not_respond_to_find_by_one_missing_attribute + assert !Topic.respond_to?(:find_by_undertitle) + end + + def test_should_not_respond_to_find_by_invalid_method_syntax + assert !Topic.respond_to?(:fail_to_find_by_title) + assert !Topic.respond_to?(:find_by_title?) + assert !Topic.respond_to?(:fail_to_find_or_create_by_title) + assert !Topic.respond_to?(:find_or_create_by_title?) + end + + private + + def ensure_topic_method_is_not_cached(method_id) + class << Topic; self; end.send(:remove_method, method_id) if Topic.public_methods.any? { |m| m.to_s == method_id.to_s } + end + +end
\ No newline at end of file diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index 79907c9c64..b7f87fe6e8 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -416,15 +416,15 @@ class FinderTest < ActiveRecord::TestCase 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.respond_to?(:find_by_title) - assert !Topic.respond_to?(:find_by_title) + 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.respond_to?(:find_by_title) + 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.respond_to?(:find_by_title) + 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 @@ -474,15 +474,15 @@ class FinderTest < ActiveRecord::TestCase 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.respond_to?(:find_by_credit_limit) - assert !Account.respond_to?(:find_by_credit_limit) + 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.respond_to?(:find_by_credit_limit) + 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.respond_to?(:find_by_credit_limit) + class << Account; self; end.send(:remove_method, :find_by_credit_limit) if Account.public_methods.any? { |m| m.to_s == 'find_by_credit_limit' } a = Account.find_by_credit_limit(50, :conditions => ['firm_id = ?', 6]) assert_equal a, Account.find_by_credit_limit(50, :conditions => ['firm_id = ?', 6]) # find_by_credit_limit has been cached end @@ -702,10 +702,10 @@ class FinderTest < ActiveRecord::TestCase end def test_dynamic_find_or_initialize_from_one_attribute_caches_method - class << Company; self; end.send(:remove_method, :find_or_initialize_by_name) if Company.respond_to?(:find_or_initialize_by_name) - assert !Company.respond_to?(:find_or_initialize_by_name) + class << Company; self; end.send(:remove_method, :find_or_initialize_by_name) if Company.public_methods.any? { |m| m.to_s == 'find_or_initialize_by_name' } + assert !Company.public_methods.any? { |m| m.to_s == 'find_or_initialize_by_name' } sig38 = Company.find_or_initialize_by_name("38signals") - assert Company.respond_to?(:find_or_initialize_by_name) + assert Company.public_methods.any? { |m| m.to_s == 'find_or_initialize_by_name' } end def test_find_or_initialize_from_two_attributes |