diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 30cbbf3ee1067..bf2de28d525bc 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,10 @@ +* Fix incrementation of in memory counter caches when associations overlap + + When two associations had a similarly named counter cache column, Active Record + could sometime increment the wrong one. + + *Jacopo Beschi*, *Jean Boussier* + * Don't show secrets for Active Record's `Cipher::Aes256Gcm#inspect`. Before: diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index 4c193fe8e45c7..815621b6bf83b 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -275,8 +275,11 @@ def check_validity_of_inverse! # Hence this method. def inverse_which_updates_counter_cache unless @inverse_which_updates_counter_cache_defined - @inverse_which_updates_counter_cache = klass.reflect_on_all_associations(:belongs_to).find do |inverse| - inverse.counter_cache_column == counter_cache_column + if counter_cache_column + inverse_candidates = inverse_of ? [inverse_of] : klass.reflect_on_all_associations(:belongs_to) + @inverse_which_updates_counter_cache = inverse_candidates.find do |inverse| + inverse.counter_cache_column == counter_cache_column && (inverse.polymorphic? || inverse.klass == active_record) + end end @inverse_which_updates_counter_cache_defined = true end diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index 28c50d8242bd2..dec2c4539d384 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -43,6 +43,7 @@ require "models/human" require "models/sharded" require "models/cpk" +require "models/comment_overlapping_counter_cache" class HasManyAssociationsTestForReorderWithJoinDependency < ActiveRecord::TestCase fixtures :authors, :author_addresses, :posts, :comments @@ -1403,6 +1404,23 @@ def test_counter_cache_updates_in_memory_after_update_with_inverse_of_disabled assert_equal 2, topic.reload.replies_count end + def test_counter_cache_updates_in_memory_after_create_with_overlapping_counter_cache_columns + user = UserCommentsCount.create! + post = PostCommentsCount.create! + + assert_difference "user.comments_count", +1 do + assert_no_difference "post.comments_count" do + post.comments << CommentOverlappingCounterCache.create!(user_comments_count: user) + end + end + + assert_difference "user.comments_count", +1 do + assert_no_difference "post.comments_count" do + user.comments << CommentOverlappingCounterCache.create!(post_comments_count: post) + end + end + end + def test_counter_cache_updates_in_memory_after_update_with_inverse_of_enabled category = Category.create!(name: "Counter Cache") diff --git a/activerecord/test/models/comment_overlapping_counter_cache.rb b/activerecord/test/models/comment_overlapping_counter_cache.rb new file mode 100644 index 0000000000000..60c3c7de29d69 --- /dev/null +++ b/activerecord/test/models/comment_overlapping_counter_cache.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class CommentOverlappingCounterCache < ActiveRecord::Base + belongs_to :user_comments_count, counter_cache: :comments_count + belongs_to :post_comments_count, class_name: "PostCommentsCount" + belongs_to :commentable, polymorphic: true, counter_cache: :comments_count +end + +class UserCommentsCount < ActiveRecord::Base + has_many :comments, as: :commentable, class_name: "CommentOverlappingCounterCache" +end + +class PostCommentsCount < ActiveRecord::Base + has_many :comments, class_name: "CommentOverlappingCounterCache" +end diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index 9b7c4cf40e032..df8816f182678 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -376,8 +376,14 @@ t.integer :company end + create_table :comment_overlapping_counter_caches, force: true do |t| + t.integer :user_comments_count_id + t.integer :post_comments_count_id + t.references :commentable, polymorphic: true, index: false + end + create_table :companies, force: true do |t| - t.string :type + t.string :type t.references :firm, index: false t.string :firm_name t.string :name @@ -981,6 +987,10 @@ t.string :author_id end + create_table :post_comments_counts, force: true do |t| + t.integer :comments_count, default: 0 + end + create_table :serialized_posts, force: true do |t| t.integer :author_id t.string :title, null: false @@ -1419,6 +1429,10 @@ t.timestamps null: true end + create_table :user_comments_counts, force: true do |t| + t.integer :comments_count, default: 0 + end + create_table :test_with_keyword_column_name, force: true do |t| t.string :desc end