Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Define dynamic finders as real methods after first usage. Close #9317

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@7510 5ecf4fe2-1ee6-0310-87b1-e25e094e27de
  • Loading branch information...
commit 4a1388adeab6aaf0ef28f975e480e22061bb1aad 1 parent 7495b5c
Tobias Lütke authored
View
2  activerecord/CHANGELOG
@@ -1,5 +1,7 @@
*SVN*
+* Define dynamic finders as real methods after first usage. [bscofield]
+
* Deprecation: remove deprecated threaded_connections methods. Use allow_concurrency instead. [Jeremy Kemper]
* Associations macros accept extension blocks alongside modules. #9346 [Josh
View
59 activerecord/lib/active_record/base.rb
@@ -1220,6 +1220,9 @@ def undecorated_table_name(class_name = base_class.name)
#
# This also enables you to initialize a record if it is not found, such as find_or_initialize_by_amount(amount)
# or find_or_create_by_user_and_password(user, password).
+ #
+ # 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)
finder = determine_finder(match)
@@ -1227,45 +1230,45 @@ def method_missing(method_id, *arguments)
attribute_names = extract_attribute_names_from_match(match)
super unless all_attributes_exists?(attribute_names)
- attributes = construct_attributes_from_arguments(attribute_names, arguments)
-
- case extra_options = arguments[attribute_names.size]
- when nil
- options = { :conditions => attributes }
+ self.class_eval %{
+ def self.#{method_id}(*args)
+ options = args.last.is_a?(Hash) ? args.pop : {}
+ attributes = construct_attributes_from_arguments([:#{attribute_names.join(',:')}], args)
+ finder_options = { :conditions => attributes }
+ validate_find_options(options)
set_readonly_option!(options)
- ActiveSupport::Deprecation.silence { send(finder, options) }
- when Hash
- finder_options = extra_options.merge(:conditions => attributes)
- validate_find_options(finder_options)
- set_readonly_option!(finder_options)
-
- if extra_options[:conditions]
- with_scope(:find => { :conditions => extra_options[:conditions] }) do
- ActiveSupport::Deprecation.silence { send(finder, finder_options) }
+ if options[:conditions]
+ with_scope(:find => finder_options) do
+ ActiveSupport::Deprecation.silence { send(:#{finder}, options) }
end
else
- ActiveSupport::Deprecation.silence { send(finder, finder_options) }
+ ActiveSupport::Deprecation.silence { send(:#{finder}, options.merge(finder_options)) }
end
-
- else
- raise ArgumentError, "Unrecognized arguments for #{method_id}: #{extra_options.inspect}"
- end
+ end
+ }
+ send(method_id, *arguments)
elsif match = /^find_or_(initialize|create)_by_([_a-zA-Z]\w*)$/.match(method_id.to_s)
instantiator = determine_instantiator(match)
attribute_names = extract_attribute_names_from_match(match)
super unless all_attributes_exists?(attribute_names)
- if arguments[0].is_a?(Hash)
- attributes = arguments[0].with_indifferent_access
- find_attributes = attributes.slice(*attribute_names)
- else
- find_attributes = attributes = construct_attributes_from_arguments(attribute_names, arguments)
- end
- options = { :conditions => find_attributes }
- set_readonly_option!(options)
+ self.class_eval %{
+ def self.#{method_id}(*args)
+ if args[0].is_a?(Hash)
+ attributes = args[0].with_indifferent_access
+ find_attributes = attributes.slice(*[:#{attribute_names.join(',:')}])
+ else
+ find_attributes = attributes = construct_attributes_from_arguments([:#{attribute_names.join(',:')}], args)
+ end
+
+ options = { :conditions => find_attributes }
+ set_readonly_option!(options)
- find_initial(options) || send(instantiator, attributes)
+ find_initial(options) || send(:#{instantiator}, attributes)
+ end
+ }
+ send(method_id, *arguments)
else
super
end
View
41 activerecord/test/finder_test.rb
@@ -324,6 +324,21 @@ 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!")
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.respond_to?(:find_by_title)
+ assert !Topic.respond_to?(:find_by_title)
+ t = Topic.find_by_title("The First Topic")
+ assert Topic.respond_to?(: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)
+ 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')
@@ -334,6 +349,21 @@ def test_find_by_one_attribute_with_conditions
assert_equal accounts(:rails_core_account), Account.find_by_credit_limit(50, :conditions => ['firm_id = ?', 6])
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.respond_to?(:find_by_credit_limit)
+ assert !Account.respond_to?(:find_by_credit_limit)
+ a = Account.find_by_credit_limit(50, :conditions => ['firm_id = ?', 6])
+ assert Account.respond_to?(: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)
+ 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
+
def test_find_by_one_attribute_with_several_options
assert_equal accounts(:unknown), Account.find_by_credit_limit(50, :order => 'id DESC', :conditions => ['id != ?', 3])
end
@@ -440,6 +470,13 @@ def test_find_or_initialize_from_one_attribute
assert sig38.new_record?
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)
+ sig38 = Company.find_or_initialize_by_name("38signals")
+ assert Company.respond_to?(:find_or_initialize_by_name)
+ end
+
def test_find_or_initialize_from_two_attributes
another = Topic.find_or_initialize_by_title_and_author_name("Another topic","John")
assert_equal "Another topic", another.title
@@ -464,6 +501,10 @@ def test_find_with_invalid_params
assert_raises(ArgumentError) { Topic.find :first, :conditions => '1 = 1', :join => "It should be `joins'" }
end
+ def test_dynamic_finder_with_invalid_params
+ assert_raises(ArgumentError) { Topic.find_by_title 'No Title', :join => "It should be `joins'" }
+ end
+
def test_find_all_with_limit
first_five_developers = Developer.find :all, :order => 'id ASC', :limit => 5
assert_equal 5, first_five_developers.length
Please sign in to comment.
Something went wrong with that request. Please try again.