Dynamic column name that uses model primary key causes all updates to increment the counter

[dvandersluis]

Consider this dynamic column name:

counter_culture :user,
    column_name: -> (model) { MyModel.processed.exists?(model.id) ? 'processed_count' : nil },

When counter_culture tries to determine if the counter cache needs to be changed on update, it collects the column name for the previous state of the model and the current state and then checks if they're the same:

counter_cache_name_was = counter.counter_cache_name_for(counter.previous_model(self))
counter_cache_name = counter.counter_cache_name_for(self)

if ... || counter_cache_name != counter_cache_name_was
  # increment the counter ...
end

However, counter.previous_model(self) calls obj.dup, which for an ActiveRecord model clears the primary key. Which means that a column_name proc that relies on the model's primary key will always set counter_cache_name_was to nil, which means even a no-op update (model.update({})) will cause the counter cache to increment.

[dvandersluis]

I'm not sure if this is something to solve or not but I can see two options:

1. resetting the primary key values in previous_model. This appears like it should be safe, since this model is never used for anything other than determining the column name.
2. communicating not to rely on primary key values inside the column_name proc

[magnusvk]

Fascinating. This makes sense to me. I have two thoughts:

1. It feels like you're misusing the column_name proc. It was intended to just check the state of the current model. There are performance implications of triggering another query each time this is called.
2. That being said, I agree with you that setting the id on the dup'ed model should be safe, so I'd be happy to accept a PR to add that if you want to put one together.