rails/rails — pull request #11594 by jimjh (2013-07-25 12:40:19+00:00)
Title: Added test cases and a fix for rails/rails#10865 - double incrementing of counter cache.

Assuming Topic has_many Replies and Reply belongs_to Topic, the counter cache was incremented twice if reply is saved after reassignment, as follows:

reply.topic = topic # triggers BelongsToAssocation#replace
reply.save # triggers BelongsTo belongs_to_counter_cache_after_update

The problem is caused by duplicate work.

Consider the following various ways to update a foreign key:

### Case 1: new, assignment, save

[example](https://github.com/rails/rails/blob/4-0-stable/activerecord/test/cases/associations/belongs_to_associations_test.rb#L273)

This is not affected because it is not an update and BelongsTo `belongs_to_counter_cache_after_update` is not triggered.

### Case 2: new, assignment, save, assignment, save

[example](https://github.com/rails/rails/blob/4-0-stable/activerecord/test/cases/associations/belongs_to_associations_test.rb#L283)

This is not affected because the `@_after_create_counter_called` flag does its work in [BelongsTo](https://github.com/rails/rails/blob/4-0-stable/activerecord/lib/active_record/associations/builder/belongs_to.rb#L44).

### Case 3: find/create, assignment, save

(Refer to new test case in this pull request.)

In the simplistic case, the save is superfluous, but because `BelongsToAssocation#replace` and `belongs_to_counter_cache_after_update` are both triggered, the counter is incremented twice.

### Case 4: find/create, append, save

(Refer to new test case in this pull request.)

```ruby
reply = Reply.find(1)
topic.replies << reply
reply.save
```

Similarly, the save is superfluous. This is not affected because `<<` does not trigger `BelongsToAssocation#replace`.

### Case 5: find/create, append, assignment, save

(Refer to new test case in this pull request.)

```ruby
reply.topic = t1 # not saved, but counter updated
t2.replies << t2 # save! called, previous change needs to be undone
```

### Fix

This commit adds `@_dirty_but_updated_counter_cache` flag to `reply` if it has already triggered an increment, but has not been saved yet. Thus, when the reply is actually saved, the superfluous increment does not occur.

Note that because of Case 5, the flag cannot be a simple boolean flag.