Rails counter_cache not updating correctly

Question

Using Rails 3.1.3 and I'm trying to figure out why our counter caches aren't being updated correctly when changing the parent record id via update_attributes.

class ExhibitorRegistration < ActiveRecord::Base
  belongs_to :event, :counter_cache => true
end

class Event < ActiveRecord::Base
  has_many :exhibitor_registrations, :dependent => :destroy
end

describe ExhibitorRegistration do
  it 'correctly maintains the counter cache on events' do
    event = Factory(:event)
    other_event = Factory(:event)
    registration = Factory(:exhibitor_registration, :event => event)

    event.reload
    event.exhibitor_registrations_count.should == 1

    registration.update_attributes(:event_id => other_event.id)

    event.reload
    event.exhibitor_registrations_count.should == 0

    other_event.reload
    other_event.exhibitor_registrations_count.should == 1
  end
end

This spec fails indicating that the counter cache on event is not being decremented.

1) ExhibitorRegistration correctly maintains the counter cache on events
   Failure/Error: event.exhibitor_registrations_count.should == 0
     expected: 0
          got: 1 (using ==)

Should I even expect this to work or do I need to manually track the changes and update the counter myself?

Malignancy

Answer 1

From the fine manual:

:counter_cache

Caches the number of belonging objects on the associate class through the use of increment_counter and decrement_counter. The counter cache is incremented when an object of this class is created and decremented when it’s destroyed.

There's no mention of updating the cache when an object is moved from one owner to another. Of course, the Rails documentation is often incomplete so we'll have to look at the source for confirmation. When you say :counter_cache => true, you trigger a call to the private add_counter_cache_callbacks method and add_counter_cache_callbacks does this:

1. Adds an after_create callback which calls increment_counter.
2. Adds an before_destroy callback which calls decrement_counter.
3. Calls attr_readonly to make the counter column readonly.

I don't think you're expecting too much, you're just expecting ActiveRecord to be more complete than it is.

All is not lost though, you can fill in the missing pieces yourself without too much effort. If you want to allow reparenting and have your counters updated, you can add a before_save callback to your ExhibitorRegistration that adjusts the counters itself, something like this (untested demo code):

class ExhibitorRegistration < ActiveRecord::Base
  belongs_to :event, :counter_cache => true
  before_save :fix_counter_cache, :if => ->(er) { !er.new_record? && er.event_id_changed? }

private

  def fix_counter_cache
    Event.decrement_counter(:exhibitor_registration_count, self.event_id_was)
    Event.increment_counter(:exhibitor_registration_count, self.event_id)
  end

end

If you were adventurous, you could patch something like that into ActiveRecord::Associations::Builder#add_counter_cache_callbacks and submit a patch. The behavior you're expecting is reasonable and I think it would make sense for ActiveRecord to support it.

Evangel

Thanks @mu-is-too-short this definitely fixes the issue. I think this certainly does deserve attention in ActiveRecord itself, I'll look into submitting a patch. – Malignancy

@MichaelGuterl: Cool, don't forget to include a documentation update with your patch :) – Evangel

@MichaelGuterl: You might want to try Ben's approach as well. I'm going through the Rails code again to see if I missed anything. This could just be a bug and poor/incomplete documentation. – Evangel

what does the ->(er) syntax mean? haven't been able to find it on SO or Google. thx – Salangi

When Google can't help, the Pickaxe book can! Page 67, -> is another syntax to create a Proc object – Salangi

@Pierre: There's also lambda { |er| ... }. symbolhound.com can sometimes help (but not always). – Evangel

This answer was a big help. I'd also mention that readers should note that if their counter_cache'd relationship (let's call it xyz) is polymorphic, they should: 1. make sure to fix_counter_cache if either the xyz_id or xyz_type columns change, 2. remember to decrement_counter on the Class indicated by xyz_type_was, and increment_counter on the Class indicated by xyz_type. – Litta

Couldn't you call before_update and ditch the first !er.new_record? condition in the if? Always looking for ways to be more brief in my code. – Damalus

@Chris: I think that would work. I tend to be in a constant battle of brief versus "will this still make sense in six months". – Evangel

Is there a pull request for this, I'd like to track it so I can remove this code when it makes it into a release? – Milwaukee

@msaspence: Not that I know of. I don't use counter caches myself and I haven't traced out all the possible side effects. – Evangel

Answer 2

If your counter has been corrupted or you've modified it directly by SQL, you can fix it.

Using:

ModelName.reset_counters(id_of_the_object_having_corrupted_count, one_or_many_counters)

Example 1: Re-compute the cached count on the post with id = 17.

Post.reset_counters(17, :comments)

Source

Example 2: Re-compute the cached count on all your articles.

Article.ids.each { |id| Article.reset_counters(id, :comments) }

Statics

Answer 3

I recently came across this same problem (Rails 3.2.3). Looks like it has yet to be fixed, so I had to go ahead and make a fix. Below is how I amended ActiveRecord::Base and utilize after_update callback to keep my counter_caches in sync.

Extend ActiveRecord::Base

Create a new file lib/fix_counters_update.rb with the following:

module FixUpdateCounters

  def fix_updated_counters
    self.changes.each {|key, value|
      # key should match /master_files_id/ or /bibls_id/
      # value should be an array ['old value', 'new value']
      if key =~ /_id/
        changed_class = key.sub(/_id/, '')
        changed_class.camelcase.constantize.decrement_counter(:"#{self.class.name.underscore.pluralize}_count", value[0]) unless value[0] == nil
        changed_class.camelcase.constantize.increment_counter(:"#{self.class.name.underscore.pluralize}_count", value[1]) unless value[1] == nil
      end
    }
  end 
end

ActiveRecord::Base.send(:include, FixUpdateCounters)

The above code uses the ActiveModel::Dirty method changes which returns a hash containing the attribute changed and an array of both the old value and new value. By testing the attribute to see if it is a relationship (i.e. ends with /_id/), you can conditionally determine whether decrement_counter and/or increment_counter need be run. It is essnetial to test for the presence of nil in the array, otherwise errors will result.

Add to Initializers

Create a new file config/initializers/active_record_extensions.rb with the following:

require 'fix_update_counters'

Add to models

For each model you want the counter caches updated add the callback:

class Comment < ActiveRecord::Base
  after_update :fix_updated_counters
  ....
end

Nigeria

Answer 4

A fix for this has been merged in to active record master

https://github.com/rails/rails/issues/9722

Milwaukee

Of note is that the fix is only in Rails 4.0 and above, and the question is for Rails 3.1. – Courante

Answer 5

The counter_cache function is designed to work through the association name, not the underlying id column. In your test, instead of:

registration.update_attributes(:event_id => other_event.id)

try

registration.update_attributes(:event => other_event)

More information can be found here: http://api.rubyonrails.org/classes/ActiveRecord/Associations/ClassMethods.html

Babettebabeuf

That won't work, the counter updating is tied to create and destroy only so they won't be triggered by a change. – Evangel

I just double checked, and this is incrementing and decrementing the cexhibitor_registrations_count column when modifying the event via update_attributes on an instance of ExhibitorRegistration. I am using Rails 3.0.7 – Babettebabeuf

If you look at ActiveRecord::Associations::Builder::BelongsTo#add_counter_cache_callbacks you'll see what I was talking about. However, you'll see more counter fiddling inside ActiveRecord::Associations::BelongsToAssociation#update_counters. I wonder if there are two separate code paths that aren't quite consistent. – Evangel

@muistooshort once I started to try and apply the patch to Rails I saw the same thing. However, I would like to keep update_attributes working in the controller without having to explicitly find the event and add it to the params. I'm still looking into patching Rails proper, but it's going to be a few days before I can give an accurate assessment of things. – Malignancy

@MichaelGuterl: My take on it is that there are two distinct code paths for the same thing and the id-only path is incomplete. Looks like a 2-part bug: the documentation is incorrect as it says nothing about the "change" case and the id-only path is incomplete as it doesn't handle the "change" case. – Evangel