Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Raise an ArgumentError if user passing less number of argument in the…

… dynamic finder

The previous behavior was unintentional, and some people was relying on it. Now the dynamic finder will always expecting the number of arguments to be equal or greater (so you can still pass the options to it.)

So if you were doing this and expecting the second argument to be nil:

    User.find_by_username_and_group("sikachu")

You'll now get `ArgumentError: wrong number of arguments (1 for 2).` You'll then have to do this:

    User.find_by_username_and_group("sikachu", nil)
  • Loading branch information...
commit 6e6994994d9a8edea33720f0da32b04f9a0efa2f 1 parent e9bd834
@sikachu sikachu authored
View
14 activerecord/CHANGELOG
@@ -1,3 +1,17 @@
+*Rails 3.2.0 (unreleased)*
+
+* Active Record's dynamic finder will now raise the error if you passing in less number of arguments than what you call in method signature.
+
+ So if you were doing this and expecting the second argument to be nil:
+
+ User.find_by_username_and_group("sikachu")
+
+ You'll now get `ArgumentError: wrong number of arguments (1 for 2).` You'll then have to do this:
+
+ User.find_by_username_and_group("sikachu", nil)
+
+ [Prem Sichanugrist]
+
*Rails 3.1.0 (unreleased)*
* ActiveRecord::MacroReflection::AssociationReflection#build_record has a new method signature.
View
10 activerecord/lib/active_record/base.rb
@@ -1055,6 +1055,11 @@ def method_missing(method_id, *arguments, &block)
if match = DynamicFinderMatch.match(method_id)
attribute_names = match.attribute_names
super unless all_attributes_exists?(attribute_names)
+ if arguments.size < attribute_names.size
+ method_trace = "#{__FILE__}:#{__LINE__}:in `#{method_id}'"
+ backtrace = [method_trace] + caller
+ raise ArgumentError, "wrong number of arguments (#{arguments.size} for #{attribute_names.size})", backtrace
+ end
if match.finder?
options = arguments.extract_options!
relation = options.any? ? scoped(options) : scoped
@@ -1065,6 +1070,11 @@ def method_missing(method_id, *arguments, &block)
elsif match = DynamicScopeMatch.match(method_id)
attribute_names = match.attribute_names
super unless all_attributes_exists?(attribute_names)
+ if arguments.size < attribute_names.size
+ method_trace = "#{__FILE__}:#{__LINE__}:in `#{method_id}'"
+ backtrace = [method_trace] + caller
+ raise ArgumentError, "wrong number of arguments (#{arguments.size} for #{attribute_names.size})", backtrace
+ 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
8 activerecord/test/cases/finder_test.rb
@@ -666,6 +666,10 @@ def test_find_by_two_attributes
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_raise(ArgumentError) { 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")
@@ -947,6 +951,10 @@ def test_find_or_initialize_from_two_attributes
assert !another.persisted?
end
+ def test_find_or_initialize_from_two_attributes_but_passing_only_one
+ assert_raise(ArgumentError) { Topic.find_or_initialize_by_title_and_author_name("Another topic") }
+ end
+
def test_find_or_initialize_from_one_aggregate_attribute_and_one_not
new_customer = Customer.find_or_initialize_by_balance_and_name(Money.new(123), "Elizabeth")
assert_equal 123, new_customer.balance.amount
View
15 activerecord/test/cases/named_scope_test.rb
@@ -497,6 +497,17 @@ def test_scoped_by
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"})
@@ -507,4 +518,8 @@ def test_dynamic_scope_should_create_methods_after_hitting_method_missing
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_raise(ArgumentError){ Post.scoped_by_author_id_and_title(1) }
+ end
end
Please sign in to comment.
Something went wrong with that request. Please try again.