diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index 4c58664a19..3d7b814daa 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -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. diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 83c9d37140..c7e35029bd 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -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) diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index 3d2a03d2b9..528ee0ce0a 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_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 diff --git a/activerecord/test/cases/named_scope_test.rb b/activerecord/test/cases/named_scope_test.rb index 34188e4915..0cc73e0409 100644 --- a/activerecord/test/cases/named_scope_test.rb +++ b/activerecord/test/cases/named_scope_test.rb @@ -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