aboutsummaryrefslogtreecommitdiffstats
path: root/activerecord
diff options
context:
space:
mode:
authorPrem Sichanugrist <s@sikachu.com>2011-07-17 18:41:02 -0400
committerPrem Sichanugrist <s@sikachu.com>2011-07-17 18:44:03 -0400
commit6e6994994d9a8edea33720f0da32b04f9a0efa2f (patch)
tree5bf702910dbe0accd9be6c79b5b9112c9dc2aa6a /activerecord
parente9bd83402ed1ab86e70c9ec6ffc913b72fd41f1d (diff)
downloadrails-6e6994994d9a8edea33720f0da32b04f9a0efa2f.tar.gz
rails-6e6994994d9a8edea33720f0da32b04f9a0efa2f.tar.bz2
rails-6e6994994d9a8edea33720f0da32b04f9a0efa2f.zip
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)
Diffstat (limited to 'activerecord')
-rw-r--r--activerecord/CHANGELOG14
-rw-r--r--activerecord/lib/active_record/base.rb10
-rw-r--r--activerecord/test/cases/finder_test.rb8
-rw-r--r--activerecord/test/cases/named_scope_test.rb15
4 files changed, 47 insertions, 0 deletions
diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG
index 4113a16c12..ef202a3573 100644
--- a/activerecord/CHANGELOG
+++ b/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.
diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index deff1c65ef..5808950715 100644
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -1055,6 +1055,11 @@ 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
+ 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 @@ 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
+ 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)
diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb
index 3d2a03d2b9..5dc5f99582 100644
--- a/activerecord/test/cases/finder_test.rb
+++ b/activerecord/test/cases/finder_test.rb
@@ -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_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 @@ class FinderTest < ActiveRecord::TestCase
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
diff --git a/activerecord/test/cases/named_scope_test.rb b/activerecord/test/cases/named_scope_test.rb
index 2afe3c8f32..01893e4dc3 100644
--- a/activerecord/test/cases/named_scope_test.rb
+++ b/activerecord/test/cases/named_scope_test.rb
@@ -497,6 +497,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"})
@@ -507,4 +518,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_raise(ArgumentError){ Post.scoped_by_author_id_and_title(1) }
+ end
end