Show a deprecation warning if user passing less number of argument in the dynamic finder

The previous behavior was unintentional, and some people was relying on it. In the next version of Rails, the dynamic finder will always expecting the number of arguments to be equal or greater (so you can still pass the options to it.) It will raise the ArgumentError otherwise.
This commit is contained in:
Prem Sichanugrist
2011-07-17 19:30:11 -04:00
parent 567930882f
commit b2cd66ff9c
4 changed files with 40 additions and 0 deletions

View File

@@ -1,5 +1,7 @@
*Rails 3.1.0 (unreleased)*
* Active Record's dynamic finder will now show a deprecation warning if you passing in less number of arguments than what you call in method signature. This behavior will raise ArgumentError in the next version of Rails [Prem Sichanugrist]
* Deprecated the AssociationCollection constant. CollectionProxy is now the appropriate constant
to use, though be warned that this is not really a public API.

View File

@@ -23,6 +23,7 @@ require 'active_support/core_ext/module/delegation'
require 'active_support/core_ext/module/introspection'
require 'active_support/core_ext/object/duplicable'
require 'active_support/core_ext/object/blank'
require 'active_support/deprecation'
require 'arel'
require 'active_record/errors'
require 'active_record/log_subscriber'
@@ -1056,6 +1057,13 @@ module ActiveRecord #:nodoc:
if match = DynamicFinderMatch.match(method_id)
attribute_names = match.attribute_names
super unless all_attributes_exists?(attribute_names)
if arguments.size < attribute_names.size
ActiveSupport::Deprecation.warn(
"Calling dynamic finder with less number of arguments than the number of attributes in " \
"method name is deprecated and will raise an ArguementError in the next version of Rails. " \
"Please passing `nil' to the argument you want it to be nil."
)
end
if match.finder?
options = arguments.extract_options!
relation = options.any? ? scoped(options) : scoped
@@ -1066,6 +1074,13 @@ module ActiveRecord #:nodoc:
elsif match = DynamicScopeMatch.match(method_id)
attribute_names = match.attribute_names
super unless all_attributes_exists?(attribute_names)
if arguments.size < attribute_names.size
ActiveSupport::Deprecation.warn(
"Calling dynamic scope with less number of arguments than the number of attributes in " \
"method name is deprecated and will raise an ArguementError in the next version of Rails. " \
"Please passing `nil' to the argument you want it to be nil."
)
end
if match.scope?
self.class_eval <<-METHOD, __FILE__, __LINE__ + 1
def self.#{method_id}(*args) # def self.scoped_by_user_name_and_password(*args)

View File

@@ -666,6 +666,10 @@ class FinderTest < ActiveRecord::TestCase
assert_nil Topic.find_by_title_and_author_name("The First Topic", "Mary")
end
def test_find_by_two_attributes_but_passing_only_one
assert_deprecated { Topic.find_by_title_and_author_name("The First Topic") }
end
def test_find_last_by_one_attribute
assert_equal Topic.last, Topic.find_last_by_title(Topic.last.title)
assert_nil Topic.find_last_by_title("A title with no matches")
@@ -859,6 +863,10 @@ class FinderTest < ActiveRecord::TestCase
assert !sig38.persisted?
end
def test_find_or_initialize_from_two_attributes_but_passing_only_one
assert_deprecated { Topic.find_or_initialize_by_title_and_author_name("Another topic") }
end
def test_find_or_initialize_from_one_aggregate_attribute
new_customer = Customer.find_or_initialize_by_balance(Money.new(123))
assert_equal 123, new_customer.balance.amount

View File

@@ -489,6 +489,17 @@ end
class DynamicScopeTest < ActiveRecord::TestCase
fixtures :posts
def setup
# Ensure we start with a clean model with no generated class method
Post.methods.select{ |c| c =~ /scoped_by_/ }.each do |method_name|
Post.class_eval <<-RUBY
class << self
remove_method :#{method_name}
end
RUBY
end
end
def test_dynamic_scope
assert_equal Post.scoped_by_author_id(1).find(1), Post.find(1)
assert_equal Post.scoped_by_author_id_and_title(1, "Welcome to the weblog").first, Post.find(:first, :conditions => { :author_id => 1, :title => "Welcome to the weblog"})
@@ -499,4 +510,8 @@ class DynamicScopeTest < ActiveRecord::TestCase
Developer.scoped_by_created_at(nil)
assert_present Developer.methods.grep(/scoped_by_created_at/)
end
def test_dynamic_scope_with_less_number_of_arguments
assert_deprecated { Post.scoped_by_author_id_and_title(1) }
end
end