magnusvk/counter_culture — pull request #85 by dontfidget (2014-12-06 22:42:05+00:00)
Title: changes to make counter_culture handle counter updates a little more like rails

Here a refactor that doesn't really change any existing behavior, but does expose some interfaces to monkey patching and illustrates a problem or two with the current code. I started looking at this because I've been using the second_level_cache gem to cache activerecord models in memcache but I was anticipating some problems with counter caches. second_level_cache monkey patches `update_counters` to trigger a cache invalidation on, so it will work with the activerecord counter cache mechanism, but it won't work with counter_culture because second_level_cache doesn't invalidate every instance of a model when `update_all` is called on a scope for a model. And to invalidate every instance would probably be overkill, because, among other users of `update_all`, rails itself uses `update_all` to implement it's own counter cache mechanism. The reason that counter_culture doesn't use rails' `update_counters` method appears to be the addition of the `touch` flag. The goal of my refactor was to use `update_column` where possible, because `second_level_cache` does invalidate the model in cache when this method is used. However, while this is used during the pass that fixes all the counters, it isn't used when updating the counter caches during normal runtime. This is presumably because we don't want to load the models (this is presumably also why `update_counters` is a class method that takes an `id` and is not an instance method). So I ended up creating a counter_culture parallel to rails' `update_counters` class method, which could easily be monkey patched to invalidate the cache. Obviously, exposing code to monkey patching isn't a good reason in itself to reorganize code, but I think that update counters in a class method is appropriate.

In the process of doing this refactor, I did discover a couple of problems:

1. `timestamp_attributes_for_update_in_model` appears determining the timestamp attributes of the class triggering the counter cache update, not the model with the counter cache columns. While these are probably the same for most applications, it isn't guaranteed. To extract this I had to inline some code that I would have preferred to get from the model.
2. At some point in the rails code, `unscoped` was added as a prefix on the `update_all` call when updating the counter caches. Presumably this was to fix a bug where only the counter caches on the models within the default_scope were being called.

I actually don't know that I'm going to try to integrate this gem in the same models second_level_cache, but I was just curious what would be needed, so here it is. Thanks.

Andrew