Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Merge pull request #2128 from sikachu/master-dynamic_finder

Raise an ArgumentError if user passing less number of argument in the dynamic finder
  • Loading branch information...
commit 50941ec07ccea65757c0c9884bedfc9ff8af193a 2 parents 1e452f1 + 4443905
@jonleighton jonleighton 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
23 activerecord/lib/active_record/base.rb
@@ -1052,20 +1052,15 @@ def compute_table_name
# Each dynamic finder using <tt>scoped_by_*</tt> 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, &block)
- if match = DynamicFinderMatch.match(method_id)
+ if match = (DynamicFinderMatch.match(method_id) || DynamicScopeMatch.match(method_id))
attribute_names = match.attribute_names
super unless all_attributes_exists?(attribute_names)
- if match.finder?
- options = arguments.extract_options!
- relation = options.any? ? scoped(options) : scoped
- relation.send :find_by_attributes, match, attribute_names, *arguments, &block
- elsif match.instantiator?
- scoped.send :find_or_instantiator_by_attributes, match, attribute_names, *arguments, &block
+ 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
- elsif match = DynamicScopeMatch.match(method_id)
- attribute_names = match.attribute_names
- super unless all_attributes_exists?(attribute_names)
- if match.scope?
+ if match.respond_to?(:scope?) && match.scope?
self.class_eval <<-METHOD, __FILE__, __LINE__ + 1
def self.#{method_id}(*args) # def self.scoped_by_user_name_and_password(*args)
attributes = Hash[[:#{attribute_names.join(',:')}].zip(args)] # attributes = Hash[[:user_name, :password].zip(args)]
@@ -1074,6 +1069,12 @@ def self.#{method_id}(*args) # def self.scope
end # end
METHOD
send(method_id, *arguments)
+ elsif match.finder?
+ options = arguments.extract_options!
+ relation = options.any? ? scoped(options) : scoped
+ relation.send :find_by_attributes, match, attribute_names, *arguments, &block
+ elsif match.instantiator?
+ scoped.send :find_or_instantiator_by_attributes, match, attribute_names, *arguments, &block
end
else
super
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
20 activerecord/test/cases/named_scope_test.rb
@@ -497,14 +497,24 @@ def test_scoped_by
class DynamicScopeTest < ActiveRecord::TestCase
fixtures :posts
+ def setup
+ @test_klass = Class.new(Post) do
+ def self.name; "Post"; end
+ 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"})
+ assert_equal @test_klass.scoped_by_author_id(1).find(1), @test_klass.find(1)
+ assert_equal @test_klass.scoped_by_author_id_and_title(1, "Welcome to the weblog").first, @test_klass.find(:first, :conditions => { :author_id => 1, :title => "Welcome to the weblog"})
end
def test_dynamic_scope_should_create_methods_after_hitting_method_missing
- assert_blank Developer.methods.grep(/scoped_by_created_at/)
- Developer.scoped_by_created_at(nil)
- assert_present Developer.methods.grep(/scoped_by_created_at/)
+ assert_blank @test_klass.methods.grep(/scoped_by_type/)
+ @test_klass.scoped_by_type(nil)
+ assert_present @test_klass.methods.grep(/scoped_by_type/)
+ end
+
+ def test_dynamic_scope_with_less_number_of_arguments
+ assert_raise(ArgumentError){ @test_klass.scoped_by_author_id_and_title(1) }
end
end

0 comments on commit 50941ec

Please sign in to comment.
Something went wrong with that request. Please try again.