Ensure that respond_to? considers dynamic finder methods. Closes #11538. [floehopper]

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@9235 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

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]

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

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

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