Skip to content

Commit

Permalink
Fix counter_cache create/concat with overlapping counter_cache_column
Browse files Browse the repository at this point in the history
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 improving the inverse relation detection in 2 ways:
- by comparing `inverse.klass` with the current
  `active_record` (doesn't work for polymorphic)
- by relying on `inverse_of`

The above adjustments allows to correctly map the
`inverse_which_updates_counter_cache` for both non-polymorphic/polymorphic
relations.

Fixes rails#41250
  • Loading branch information
intrip committed Aug 9, 2021
1 parent 6983a89 commit b25f3e5
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 2 deletions.
9 changes: 8 additions & 1 deletion activerecord/lib/active_record/reflection.rb
Expand Up @@ -249,7 +249,9 @@ def check_validity_of_inverse!
# Hence this method.
def inverse_which_updates_counter_cache
return @inverse_which_updates_counter_cache if defined?(@inverse_which_updates_counter_cache)
@inverse_which_updates_counter_cache = klass.reflect_on_all_associations(:belongs_to).find do |inverse|
inverse_candidates = inverse_of ? [inverse_of] : klass.reflect_on_all_associations(:belongs_to)
@inverse_which_updates_counter_cache = inverse_candidates.find do |inverse|
next false if inverse.klass_suppress_errors && inverse.klass_suppress_errors != active_record
inverse.counter_cache_column == counter_cache_column
end
end
Expand Down Expand Up @@ -298,6 +300,11 @@ def actual_source_reflection # FIXME: this is a horrible name
self
end

def klass_suppress_errors
klass
rescue NameError, ArgumentError
end

private
def predicate_builder(table)
PredicateBuilder.new(TableMetadata.new(klass, table))
Expand Down
18 changes: 18 additions & 0 deletions activerecord/test/cases/associations/has_many_associations_test.rb
Expand Up @@ -41,6 +41,7 @@
require "models/zine"
require "models/interest"
require "models/human"
require "models/comment_overlapping_counter_cache"

class HasManyAssociationsTestForReorderWithJoinDependency < ActiveRecord::TestCase
fixtures :authors, :author_addresses, :posts, :comments
Expand Down Expand Up @@ -1361,6 +1362,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")

Expand Down
15 changes: 15 additions & 0 deletions 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
16 changes: 15 additions & 1 deletion activerecord/test/schema/schema.rb
Expand Up @@ -244,8 +244,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
Expand Down Expand Up @@ -843,6 +849,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
Expand Down Expand Up @@ -1265,6 +1275,10 @@
t.string :auth_token
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
Expand Down

0 comments on commit b25f3e5

Please sign in to comment.