Skip to content

Commit

Permalink
Merge pull request #2129 from sikachu/3-1-stable-dynamic_finder
Browse files Browse the repository at this point in the history
Show a deprecation warning if user passing less number of argument in the
  • Loading branch information
jonleighton committed Jul 18, 2011
2 parents 1b97f6c + fa7dd34 commit 3d7ed11
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 5 deletions.
2 changes: 2 additions & 0 deletions activerecord/CHANGELOG
Original file line number Diff line number Diff line change
@@ -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.

Expand Down
15 changes: 15 additions & 0 deletions activerecord/lib/active_record/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
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'
Expand Down Expand Up @@ -1056,6 +1057,13 @@ 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
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
Expand All @@ -1066,6 +1074,13 @@ 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
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)
Expand Down
8 changes: 8 additions & 0 deletions activerecord/test/cases/finder_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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_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")
Expand Down Expand Up @@ -859,6 +863,10 @@ def test_find_or_initialize_from_one_attribute
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
Expand Down
20 changes: 15 additions & 5 deletions activerecord/test/cases/named_scope_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -489,14 +489,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_deprecated { @test_klass.scoped_by_author_id_and_title(1) }
end
end

0 comments on commit 3d7ed11

Please sign in to comment.