diff options
8 files changed, 19 insertions, 79 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 1fd35f68a8..8f82217d7b 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,7 @@ +* Removed support for deprecated `counter_sql` in associations. + + *Neeraj Singh* + * Do not invoke callbacks when `delete_all` is called on collection. Method `delete_all` should not be invoking callbacks and this diff --git a/activerecord/lib/active_record/associations/builder/collection_association.rb b/activerecord/lib/active_record/associations/builder/collection_association.rb index c24432c863..69627893c8 100644 --- a/activerecord/lib/active_record/associations/builder/collection_association.rb +++ b/activerecord/lib/active_record/associations/builder/collection_association.rb @@ -8,7 +8,7 @@ module ActiveRecord::Associations::Builder CALLBACKS = [:before_add, :after_add, :before_remove, :after_remove] def valid_options - super + [:table_name, :finder_sql, :counter_sql, :before_add, + super + [:table_name, :finder_sql, :before_add, :after_add, :before_remove, :after_remove, :extend] end diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index dbe97cabaf..acc7b35e46 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -197,15 +197,15 @@ module ActiveRecord end end - # Count all records using SQL. If the +:counter_sql+ or +:finder_sql+ option is set for the + # Count all records using SQL. If the +:finder_sql+ option is set for the # association, it will be used for the query. Otherwise, construct options and pass them with # scope to the target class's +count+. def count(column_name = nil, count_options = {}) column_name, count_options = nil, column_name if column_name.is_a?(Hash) - if options[:counter_sql] || options[:finder_sql] + if options[:finder_sql] unless count_options.blank? - raise ArgumentError, "If finder_sql/counter_sql is used then options cannot be passed" + raise ArgumentError, "If finder_sql is used then options cannot be passed" end reflection.klass.count_by_sql(custom_counter_sql) @@ -301,14 +301,14 @@ module ActiveRecord # Returns true if the collection is empty. # - # If the collection has been loaded or the <tt>:counter_sql</tt> option - # is provided, it is equivalent to <tt>collection.size.zero?</tt>. If the + # If the collection has been loaded + # it is equivalent to <tt>collection.size.zero?</tt>. If the # collection has not been loaded, it is equivalent to # <tt>collection.exists?</tt>. If the collection has not already been # loaded and you are going to fetch the records anyway it is better to # check <tt>collection.length.zero?</tt>. def empty? - if loaded? || options[:counter_sql] + if loaded? size.zero? else @target.blank? && !scope.exists? @@ -407,15 +407,11 @@ module ActiveRecord private def custom_counter_sql - if options[:counter_sql] - interpolate(options[:counter_sql]) - else - # replace the SELECT clause with COUNT(SELECTS), preserving any hints within /* ... */ - interpolate(options[:finder_sql]).sub(/SELECT\b(\/\*.*?\*\/ )?(.*)\bFROM\b/im) do - count_with = $2.to_s - count_with = '*' if count_with.blank? || count_with =~ /,/ || count_with =~ /\.\*/ - "SELECT #{$1}COUNT(#{count_with}) FROM" - end + # replace the SELECT clause with COUNT(SELECTS), preserving any hints within /* ... */ + interpolate(options[:finder_sql]).sub(/SELECT\b(\/\*.*?\*\/ )?(.*)\bFROM\b/im) do + count_with = $2.to_s + count_with = '*' if count_with.blank? || count_with =~ /,/ || count_with =~ /\.\*/ + "SELECT #{$1}COUNT(#{count_with}) FROM" end end diff --git a/activerecord/lib/active_record/associations/collection_proxy.rb b/activerecord/lib/active_record/associations/collection_proxy.rb index 9381bd4456..6dc2da56d1 100644 --- a/activerecord/lib/active_record/associations/collection_proxy.rb +++ b/activerecord/lib/active_record/associations/collection_proxy.rb @@ -726,7 +726,7 @@ module ActiveRecord end # Returns +true+ if the collection is empty. If the collection has been - # loaded or the <tt>:counter_sql</tt> option is provided, it is equivalent + # loaded it is equivalent # to <tt>collection.size.zero?</tt>. If the collection has not been loaded, # it is equivalent to <tt>collection.exists?</tt>. If the collection has # not already been loaded and you are going to fetch the records anyway it diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb index cf8a589496..b632b0dfed 100644 --- a/activerecord/lib/active_record/associations/has_many_association.rb +++ b/activerecord/lib/active_record/associations/has_many_association.rb @@ -58,7 +58,7 @@ module ActiveRecord def count_records count = if has_cached_counter? owner.send(:read_attribute, cached_counter_attribute_name) - elsif options[:counter_sql] || options[:finder_sql] + elsif options[:finder_sql] reflection.klass.count_by_sql(custom_counter_sql) else scope.count diff --git a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb index e23780174b..7862fe3bf4 100644 --- a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb @@ -65,19 +65,6 @@ class DeveloperWithSymbolsForKeys < ActiveRecord::Base :foreign_key => "developer_id" end -class DeveloperWithCounterSQL < ActiveRecord::Base - self.table_name = 'developers' - - ActiveSupport::Deprecation.silence do - has_and_belongs_to_many :projects, - :class_name => "DeveloperWithCounterSQL", - :join_table => "developers_projects", - :association_foreign_key => "project_id", - :foreign_key => "developer_id", - :counter_sql => proc { "SELECT COUNT(*) AS count_all FROM projects INNER JOIN developers_projects ON projects.id = developers_projects.project_id WHERE developers_projects.developer_id =#{id}" } - end -end - class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase fixtures :accounts, :companies, :categories, :posts, :categories_posts, :developers, :projects, :developers_projects, :parrots, :pirates, :parrots_pirates, :treasures, :price_estimates, :tags, :taggings @@ -791,14 +778,6 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase assert_equal 2, david.projects.count end - def test_count_with_counter_sql - developer = DeveloperWithCounterSQL.create(:name => 'tekin') - developer.project_ids = [projects(:active_record).id] - developer.save - developer.reload - assert_equal 1, developer.projects.count - end - unless current_adapter?(:PostgreSQLAdapter) def test_count_with_finder_sql assert_equal 3, projects(:active_record).developers_with_finder_sql.count diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index 582cde26ed..42e140a831 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -36,19 +36,6 @@ class HasManyAssociationsTestForCountWithFinderSql < ActiveRecord::TestCase end end -class HasManyAssociationsTestForCountWithCountSql < ActiveRecord::TestCase - class Invoice < ActiveRecord::Base - ActiveSupport::Deprecation.silence do - has_many :custom_line_items, :class_name => 'LineItem', :counter_sql => "SELECT COUNT(*) line_items.* from line_items" - end - end - def test_should_fail - assert_raise(ArgumentError) do - Invoice.create.custom_line_items.count(:conditions => {:amount => 0}) - end - end -end - class HasManyAssociationsTestForCountWithVariousFinderSqls < ActiveRecord::TestCase class Invoice < ActiveRecord::Base ActiveSupport::Deprecation.silence do @@ -381,17 +368,6 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_equal client, firm.clients_using_sql.find(client.id, client.id.to_s) end - def test_counting_using_sql - assert_equal 1, Firm.order("id").first.clients_using_counter_sql.size - assert Firm.order("id").first.clients_using_counter_sql.any? - assert_equal 0, Firm.order("id").first.clients_using_zero_counter_sql.size - assert !Firm.order("id").first.clients_using_zero_counter_sql.any? - end - - def test_counting_non_existant_items_using_sql - assert_equal 0, Firm.order("id").first.no_clients_using_counter_sql.size - end - def test_counting_using_finder_sql assert_equal 2, Firm.find(4).clients_using_sql.count end diff --git a/activerecord/test/models/company.rb b/activerecord/test/models/company.rb index b988184f34..12fb183bb3 100644 --- a/activerecord/test/models/company.rb +++ b/activerecord/test/models/company.rb @@ -35,13 +35,7 @@ module Namespaced end class Firm < Company - ActiveSupport::Deprecation.silence do - has_many :clients, -> { order "id" }, :dependent => :destroy, :counter_sql => - "SELECT COUNT(*) FROM companies WHERE firm_id = 1 " + - "AND (#{QUOTED_TYPE} = 'Client' OR #{QUOTED_TYPE} = 'SpecialClient' OR #{QUOTED_TYPE} = 'VerySpecialClient' )", - :before_remove => :log_before_remove, - :after_remove => :log_after_remove - end + has_many :clients, -> { order "id" }, :dependent => :destroy, :before_remove => :log_before_remove, :after_remove => :log_after_remove has_many :unsorted_clients, :class_name => "Client" has_many :unsorted_clients_with_symbol, :class_name => :Client has_many :clients_sorted_desc, -> { order "id DESC" }, :class_name => "Client" @@ -56,15 +50,6 @@ class Firm < Company has_many :clients_like_ms_with_hash_conditions, -> { where(:name => 'Microsoft').order("id") }, :class_name => "Client" ActiveSupport::Deprecation.silence do has_many :clients_using_sql, :class_name => "Client", :finder_sql => proc { "SELECT * FROM companies WHERE client_of = #{id}" } - has_many :clients_using_counter_sql, :class_name => "Client", - :finder_sql => proc { "SELECT * FROM companies WHERE client_of = #{id} " }, - :counter_sql => proc { "SELECT COUNT(*) FROM companies WHERE client_of = #{id}" } - has_many :clients_using_zero_counter_sql, :class_name => "Client", - :finder_sql => proc { "SELECT * FROM companies WHERE client_of = #{id}" }, - :counter_sql => proc { "SELECT 0 FROM companies WHERE client_of = #{id}" } - has_many :no_clients_using_counter_sql, :class_name => "Client", - :finder_sql => 'SELECT * FROM companies WHERE client_of = 1000', - :counter_sql => 'SELECT COUNT(*) FROM companies WHERE client_of = 1000' has_many :clients_using_finder_sql, :class_name => "Client", :finder_sql => 'SELECT * FROM companies WHERE 1=1' end has_many :plain_clients, :class_name => 'Client' |