Merge pull request #31710 from eugeneius/indestructible_through_record

Don't update counter cache when through record was not destroyed

6 files changed, 24 insertions, 5 deletions

diff --git a/activerecord/lib/active_record/associations/has_many_through_association.rb b/activerecord/lib/active_record/associations/has_many_through_association.rb
index 7a3bef969b..24766dd315 100644
--- a/activerecord/lib/active_record/associations/has_many_through_association.rb
+++ b/activerecord/lib/active_record/associations/has_many_through_association.rb
@@ -145,7 +145,7 @@ module ActiveRecord
           case method
           when :destroy
             if scope.klass.primary_key
-              count = scope.destroy_all.length
+              count = scope.destroy_all.count(&:destroyed?)
             else
               scope.each(&:_run_destroy_callbacks)
               count = scope.delete_all
diff --git a/activerecord/test/cases/associations/has_many_through_associations_test.rb b/activerecord/test/cases/associations/has_many_through_associations_test.rb
index 56a4b7c4d1..dabeeff1be 100644
--- a/activerecord/test/cases/associations/has_many_through_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb
@@ -538,6 +538,16 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase
     end
   end
 
+  def test_update_counter_caches_on_destroy_with_indestructible_through_record
+    post = posts(:welcome)
+    tag  = post.indestructible_tags.create!(name: "doomed")
+    post.update_columns(indestructible_tags_count: post.indestructible_tags.count)
+
+    assert_no_difference "post.reload.indestructible_tags_count" do
+      posts(:welcome).indestructible_tags.destroy(tag)
+    end
+  end
+
   def test_replace_association
     assert_queries(4) { posts(:welcome);people(:david);people(:michael); posts(:welcome).people.reload }
 
diff --git a/activerecord/test/cases/connection_adapters/schema_cache_test.rb b/activerecord/test/cases/connection_adapters/schema_cache_test.rb
index 006be9e65d..67496381d1 100644
--- a/activerecord/test/cases/connection_adapters/schema_cache_test.rb
+++ b/activerecord/test/cases/connection_adapters/schema_cache_test.rb
@@ -22,8 +22,8 @@ module ActiveRecord
 
         new_cache = YAML.load(YAML.dump(@cache))
         assert_no_queries do
-          assert_equal 11, new_cache.columns("posts").size
-          assert_equal 11, new_cache.columns_hash("posts").size
+          assert_equal 12, new_cache.columns("posts").size
+          assert_equal 12, new_cache.columns_hash("posts").size
           assert new_cache.data_sources("posts")
           assert_equal "id", new_cache.primary_keys("posts")
         end
@@ -75,8 +75,8 @@ module ActiveRecord
         @cache = Marshal.load(Marshal.dump(@cache))
 
         assert_no_queries do
-          assert_equal 11, @cache.columns("posts").size
-          assert_equal 11, @cache.columns_hash("posts").size
+          assert_equal 12, @cache.columns("posts").size
+          assert_equal 12, @cache.columns_hash("posts").size
           assert @cache.data_sources("posts")
           assert_equal "id", @cache.primary_keys("posts")
         end
diff --git a/activerecord/test/models/post.rb b/activerecord/test/models/post.rb
index 780a2c17f5..b552f66787 100644
--- a/activerecord/test/models/post.rb
+++ b/activerecord/test/models/post.rb
@@ -106,6 +106,9 @@ class Post < ActiveRecord::Base
     end
   end
 
+  has_many :indestructible_taggings, as: :taggable, counter_cache: :indestructible_tags_count
+  has_many :indestructible_tags, through: :indestructible_taggings, source: :tag
+
   has_many :taggings_with_delete_all, class_name: "Tagging", as: :taggable, dependent: :delete_all, counter_cache: :taggings_with_delete_all_count
   has_many :taggings_with_destroy, class_name: "Tagging", as: :taggable, dependent: :destroy, counter_cache: :taggings_with_destroy_count
 
diff --git a/activerecord/test/models/tagging.rb b/activerecord/test/models/tagging.rb
index 861fde633f..6d4230f6f4 100644
--- a/activerecord/test/models/tagging.rb
+++ b/activerecord/test/models/tagging.rb
@@ -14,3 +14,7 @@ class Tagging < ActiveRecord::Base
   belongs_to :taggable, polymorphic: true, counter_cache: :tags_count
   has_many :things, through: :taggable
 end
+
+class IndestructibleTagging < Tagging
+  before_destroy { throw :abort }
+end
diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb
index 3205c4c20a..7d008eecd5 100644
--- a/activerecord/test/schema/schema.rb
+++ b/activerecord/test/schema/schema.rb
@@ -690,6 +690,7 @@ ActiveRecord::Schema.define do
     t.integer :taggings_with_delete_all_count, default: 0
     t.integer :taggings_with_destroy_count, default: 0
     t.integer :tags_count, default: 0
+    t.integer :indestructible_tags_count, default: 0
     t.integer :tags_with_destroy_count, default: 0
     t.integer :tags_with_nullify_count, default: 0
   end
@@ -847,6 +848,7 @@ ActiveRecord::Schema.define do
     t.column :taggable_type, :string
     t.column :taggable_id, :integer
     t.string :comment
+    t.string :type
   end
 
   create_table :tasks, force: true do |t|