Skip to content

Cached counter updated twice when reassigning association on persisted model #23265

@domcleal

Description

@domcleal

Cached counters on a belongs_to association of a model (e.g. Comment) are being updated twice when the model is reassigned after first being created and loaded back from the database.

Example:

class Post < ActiveRecord::Base  #domain
  has_many :posts
end

class Comment < ActiveRecord::Base  #hg
  belongs_to :post, :counter_cache => :comments_count
end

class BugTest < Minitest::Test
  def test_counter_update
    post1, post2 = Post.create!, Post.create!

    comment1 = Comment.create!(:post => post1)
    assert_equal 1, post1.reload.comments_count
    assert_equal 0, post2.reload.comments_count

    # Remove this finder to prevent a reload, which exposes the bug.  Using the same
    # newly created object prevents the counter being incremented again.
    comment1 = Comment.find_by_id(comment1.id)

    # The counter of both models are updated twice in this call instead of once.
    #
    # It's the model's association setter that's incrementing the counter, as well as
    # a model callback monitoring post_id for changes.
    comment1.update_attribute(:post, post2)

    assert_equal 0, post1.reload.comments_count
    assert_equal 1, post2.reload.comments_count
  end
end

The last two assertions will fail (the following on line 66 is the post1.reload.comments_count assertion):

  1) Failure:
BugTest#test_counter_update [reproducer.rb:66]:
Expected: 0
  Actual: -1

1 runs, 3 assertions, 1 failures, 0 errors, 0 skips

The existing AR tests for reassignment run correctly, but they use a newly created model which has slightly different behaviour. The commit above changes the test to reload the model to show the problem.

An instance variable on the belongs_to association stops counters being updated twice for newly created models:

https://github.com/rails/rails/blob/v4.2.5/activerecord/lib/active_record/associations/builder/belongs_to.rb#L36

and so on a model that isn't newly created, causes it to increment/decrement counters here:

https://github.com/rails/rails/blob/v4.2.5/activerecord/lib/active_record/associations/builder/belongs_to.rb#L44

The second counter update appears to come from here:

https://github.com/rails/rails/blob/v4.2.5/activerecord/lib/active_record/associations/belongs_to_association.rb#L13

The reproducer seems to pass OK on 3-2-stable, but fails on 4-1-stable and above (I didn't try 4-0-stable).

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions