diff options
| -rw-r--r-- | activerecord/CHANGELOG | 2 | ||||
| -rw-r--r-- | activerecord/lib/active_record/associations/has_many_association.rb | 2 | ||||
| -rw-r--r-- | activerecord/lib/active_record/calculations.rb | 37 | ||||
| -rwxr-xr-x | activerecord/test/associations_test.rb | 4 | ||||
| -rwxr-xr-x | activerecord/test/base_test.rb | 2 | ||||
| -rw-r--r-- | activerecord/test/calculations_test.rb | 12 | ||||
| -rwxr-xr-x | activerecord/test/deprecated_finder_test.rb | 6 | ||||
| -rw-r--r-- | activerecord/test/finder_test.rb | 6 | ||||
| -rw-r--r-- | activerecord/test/method_scoping_test.rb | 4 |
9 files changed, 50 insertions, 25 deletions
diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index da4e0a59d2..5d437a3e70 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -1,5 +1,7 @@ *SVN* +* Deprecation: count class method should be called with an options hash rather than two args for conditions and joins. #6287 [Bob Silva] + * has_one associations with a nil target may be safely marshaled. #6279 [norbauer, Jeremy Kemper] * Duplicate the hash provided to AR::Base#to_xml to prevent unexpected side effects [Koz] diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb index 226698611c..d0af68eea7 100644 --- a/activerecord/lib/active_record/associations/has_many_association.rb +++ b/activerecord/lib/active_record/associations/has_many_association.rb @@ -137,7 +137,7 @@ module ActiveRecord elsif @reflection.options[:counter_sql] @reflection.klass.count_by_sql(@counter_sql) else - @reflection.klass.count(@counter_sql) + @reflection.klass.count(:conditions => @counter_sql) end @target = [] and loaded if count == 0 diff --git a/activerecord/lib/active_record/calculations.rb b/activerecord/lib/active_record/calculations.rb index e0ac96c879..aab0fa2a74 100644 --- a/activerecord/lib/active_record/calculations.rb +++ b/activerecord/lib/active_record/calculations.rb @@ -9,7 +9,7 @@ module ActiveRecord # Count operates using three different approaches. # # * Count all: By not passing any parameters to count, it will return a count of all the rows for the model. - # * Count by conditions or joins: For backwards compatibility, you can pass in +conditions+ and +joins+ as individual parameters. + # * Count by conditions or joins: This API has been deprecated and will be removed in Rails 2.0 # * Count using options will find the row count matched by the options used. # # The last approach, count using options, accepts an option hash as the only parameter. The options are: @@ -29,7 +29,7 @@ module ActiveRecord # Examples for counting all: # Person.count # returns the total count of all people # - # Examples for count by +conditions+ and +joins+ (for backwards compatibility): + # Examples for count by +conditions+ and +joins+ (this has been deprecated): # Person.count("age > 26") # returns the number of people older than 26 # Person.find("age > 26 AND job.salary > 60000", "LEFT JOIN jobs on jobs.person_id = person.id") # returns the total number of rows matching the conditions and joins fetched by SELECT COUNT(*). # @@ -128,24 +128,33 @@ module ActiveRecord def construct_count_options_from_legacy_args(*args) options = {} column_name = :all - # For backwards compatibility, we need to handle both count(conditions=nil, joins=nil) or count(options={}) or count(column_name=:all, options={}). - if args.size >= 0 && args.size <= 2 - if args.first.is_a?(Hash) - options = args.first + + # We need to handle + # count() + # count(options={}) + # count(column_name=:all, options={}) + # count(conditions=nil, joins=nil) # deprecated + if args.size > 2 + raise ArgumentError, "Unexpected parameters passed to count(options={}): #{args.inspect}" + elsif args.size > 0 + if args[0].is_a?(Hash) + options = args[0] elsif args[1].is_a?(Hash) - options = args[1] - column_name = args.first + column_name, options = args else - # Handle legacy paramter options: def count(conditions=nil, joins=nil) - options.merge!(:conditions => args[0]) if args.length > 0 - options.merge!(:joins => args[1]) if args.length > 1 + # Deprecated count(conditions, joins=nil) + ActiveSupport::Deprecation.warn( + "You called count(#{args[0].inspect}, #{args[1].inspect}), which is a deprecated API call. Instead you should use " + + "count(column_name, options). Passing the conditions and joins as string parameters will be removed in Rails 2.0." + ) + options.merge!(:conditions => args[0]) + options.merge!(:joins => args[1]) if args[1] end - else - raise(ArgumentError, "Unexpected parameters passed to count(*args): expected either count(conditions=nil, joins=nil) or count(options={})") end + [column_name, options] end - + def construct_calculation_sql(operation, column_name, options) #:nodoc: scope = scope(:find) merged_includes = merge_includes(scope ? scope[:include] : [], options[:include]) diff --git a/activerecord/test/associations_test.rb b/activerecord/test/associations_test.rb index 5e9a39873d..b843f5bdf5 100755 --- a/activerecord/test/associations_test.rb +++ b/activerecord/test/associations_test.rb @@ -395,7 +395,9 @@ class HasManyAssociationsTest < Test::Unit::TestCase end def test_counting_with_single_conditions - assert_equal 2, Firm.find(:first).plain_clients.count('1=1') + assert_deprecated 'count' do + assert_equal 2, Firm.find(:first).plain_clients.count('1=1') + end end def test_counting_with_single_hash diff --git a/activerecord/test/base_test.rb b/activerecord/test/base_test.rb index c1dc89a611..055f5e8670 100755 --- a/activerecord/test/base_test.rb +++ b/activerecord/test/base_test.rb @@ -1159,7 +1159,7 @@ class BasicsTest < Test::Unit::TestCase def test_count_with_join res = Post.count_by_sql "SELECT COUNT(*) FROM posts LEFT JOIN comments ON posts.id=comments.post_id WHERE posts.#{QUOTED_TYPE} = 'Post'" res2 = nil - assert_nothing_raised do + assert_deprecated 'count' do res2 = Post.count("posts.#{QUOTED_TYPE} = 'Post'", "LEFT JOIN comments ON posts.id=comments.post_id") end diff --git a/activerecord/test/calculations_test.rb b/activerecord/test/calculations_test.rb index 397b7a9026..23c45f8637 100644 --- a/activerecord/test/calculations_test.rb +++ b/activerecord/test/calculations_test.rb @@ -196,4 +196,16 @@ class CalculationsTest < Test::Unit::TestCase assert_equal 6, Account.count(:distinct => true, :include => :firm) assert_equal 4, Account.count(:distinct => true, :include => :firm, :select => :credit_limit) end + + def test_deprecated_count_with_string_parameters + assert_deprecated('count') { Account.count('credit_limit > 50') } + end + + def test_count_with_no_parameters_isnt_deprecated + assert_not_deprecated { Account.count } + end + + def test_count_with_too_many_parameters_raises + assert_raise(ArgumentError) { Account.count(1, 2, 3) } + end end diff --git a/activerecord/test/deprecated_finder_test.rb b/activerecord/test/deprecated_finder_test.rb index fa56e8ee04..796e46f8c2 100755 --- a/activerecord/test/deprecated_finder_test.rb +++ b/activerecord/test/deprecated_finder_test.rb @@ -88,9 +88,9 @@ class DeprecatedFinderTest < Test::Unit::TestCase end def test_count - assert_equal(0, Entrant.count("id > 3")) - assert_equal(1, Entrant.count(["id > ?", 2])) - assert_equal(2, Entrant.count(["id > ?", 1])) + assert_equal(0, Entrant.count(:conditions => "id > 3")) + assert_equal(1, Entrant.count(:conditions => ["id > ?", 2])) + assert_equal(2, Entrant.count(:conditions => ["id > ?", 1])) end def test_count_by_sql diff --git a/activerecord/test/finder_test.rb b/activerecord/test/finder_test.rb index bd72348916..ebc21b5e0c 100644 --- a/activerecord/test/finder_test.rb +++ b/activerecord/test/finder_test.rb @@ -249,9 +249,9 @@ class FinderTest < Test::Unit::TestCase end def test_count - assert_equal(0, Entrant.count("id > 3")) - assert_equal(1, Entrant.count(["id > ?", 2])) - assert_equal(2, Entrant.count(["id > ?", 1])) + assert_equal(0, Entrant.count(:conditions => "id > 3")) + assert_equal(1, Entrant.count(:conditions => ["id > ?", 2])) + assert_equal(2, Entrant.count(:conditions => ["id > ?", 1])) end def test_count_by_sql diff --git a/activerecord/test/method_scoping_test.rb b/activerecord/test/method_scoping_test.rb index bceb3869a6..ee4a52df1d 100644 --- a/activerecord/test/method_scoping_test.rb +++ b/activerecord/test/method_scoping_test.rb @@ -57,7 +57,7 @@ class MethodScopingTest < Test::Unit::TestCase Developer.with_scope(:find => { :conditions => 'salary = 100000' }) do assert_equal 8, Developer.count - assert_equal 1, Developer.count("name LIKE 'fixture_1%'") + assert_equal 1, Developer.count(:conditions => "name LIKE 'fixture_1%'") end end @@ -74,7 +74,7 @@ class MethodScopingTest < Test::Unit::TestCase def test_scoped_count_include # with the include, will retrieve only developers for the given project Developer.with_scope(:find => { :include => :projects }) do - assert_equal 1, Developer.count('projects.id = 2') + assert_equal 1, Developer.count(:conditions => 'projects.id = 2') end end |