Fix counter_cache create/concat with overlapping counter_cache_column

Fix when multiple `belongs_to` maps to the same counter_cache column.
In such situation `inverse_which_updates_counter_cache` may find the
wrong relation which leads into an invalid increment of the
counter_cache.

This is done by releying on the `inverse_of` property of the relation
as well as comparing the models the association points two.

Note however that this second check doesn't work for polymorphic
associations.

Fixes #41250

Co-Authored-By: Jean Boussier <jean.boussier@gmail.com>
This commit is contained in:
Jacopo 2021-02-03 11:57:53 +01:00 committed by Jean Boussier
parent f043f74cdb
commit 186474f273
5 changed files with 60 additions and 3 deletions

View File

@ -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:

View File

@ -275,8 +275,11 @@ module ActiveRecord
# 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

View File

@ -43,6 +43,7 @@ require "models/interest"
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 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
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")

View File

@ -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

View File

@ -376,8 +376,14 @@ ActiveRecord::Schema.define do
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 @@ ActiveRecord::Schema.define do
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 @@ ActiveRecord::Schema.define do
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