Better tests for counter caching.

app/models/impression.rb

@@ -8,7 +8,7 @@ class Impression < ActiveRecord::Base
   def update_impressions_counter_cache
     impressionable_class = self.impressionable_type.constantize
 
-    if impressionable_class.counter_caching?
+    if impressionable_class.counter_cache_options
       resouce = impressionable_class.find(self.impressionable_id)
       resouce.try(:update_counter_cache)
     end

app/models/impressionist/impressionable.rb

@@ -1,27 +1,31 @@
 module Impressionist
   module Impressionable
 
-    def is_impressionable(options={})
+    def self.included(base)
-      has_many :impressions, :as=>:impressionable
+      base.extend ClassMethods
+      base.send(:include, InstanceMethods)
+    end
 
-      @cache_options = options[:counter_cache]
+    module ClassMethods
+      attr_accessor :cache_options
+      @cache_options = nil
 
-      if @cache_options.present?
+      def is_impressionable(options={})
-        @cache_options = { :column_name => :impressions_count, :unique => false }
+        has_many :impressions, :as=>:impressionable
-        @cache_options.merge!(options[:counter_cache]) if options[:counter_cache].is_a?(Hash)
+        @cache_options = options[:counter_cache]
       end
 
-      def update_counter_cache
+      def counter_cache_options
-        column_name = cache_options[:column_name].to_sym
+        if @cache_options
-        count = @cache_options[:unique] ? impressionist_count(:filter => :ip_address) : impressionist_count
+          options = { :column_name => :impressions_count, :unique => false }
-        update_attribute(column_name, count)
+          options.merge!(@cache_options) if @cache_options.is_a?(Hash)
+          options
+        end
       end
 
-      def self.counter_caching?
+      def counter_caching?
-        @cache_options.present?
+        counter_cache_options.present?
       end
-
-      include InstanceMethods
     end
 
     module InstanceMethods
@@ -38,6 +42,13 @@ module Impressionist
         imps.all.size
       end
 
+      def update_counter_cache
+        cache_options = self.class.counter_cache_options
+        column_name = cache_options[:column_name].to_sym
+        count = cache_options[:unique] ? impressionist_count(:filter => :ip_address) : impressionist_count
+        update_attribute(column_name, count)
+      end
+
       # OLD METHODS - DEPRECATE IN V0.5
       def impression_count(start_date=nil,end_date=Time.now)
         impressionist_count({:start_date=>start_date, :end_date=>end_date, :filter=>:all})
@@ -58,3 +69,5 @@ module Impressionist
 
   end
 end
+
+ActiveRecord::Base.send(:include, Impressionist::Impressionable)

test_app/spec/fixtures/widgets.yml

@@ -1,3 +1,4 @@
 one:
   id: 1
   name: A Widget
+  impressions_count: 0

test_app/spec/models/counter_caching.rb

@@ -1,23 +0,0 @@
-require 'spec_helper'
-
-describe Impression do
-  fixtures :widgets
-
-  before(:each) do
-    @widget = Widget.find(1)
-  end
-
-  context "when configured to use counter caching" do
-    it "should know counter caching is enabled" do
-      Widget.should be_counter_caching
-    end
-  end
-
-  context "when not configured to use counter caching" do
-    it "should know that counter caching is disabled" do
-      Article.should_not be_counter_caching
-    end
-  end
-
-
-end

test_app/spec/models/counter_caching_spec.rb

@@ -0,0 +1,30 @@
+require 'spec_helper'
+
+describe Impression do
+  fixtures :widgets
+
+  before(:each) do
+    @widget = Widget.find(1)
+    Impression.destroy_all
+  end
+
+  describe "self#counter_caching?" do
+    it "should know when counter caching is enabled" do
+      Widget.should be_counter_caching
+    end
+
+    it "should know when counter caching is disabled" do
+      Article.should_not be_counter_caching
+    end
+  end
+
+  describe "#update_counter_cache" do
+    it "should update the counter cache column to reflect the correct number of impressions" do
+      lambda {
+         Impression.create(:impressionable_type => @widget.class.name, :impressionable_id => @widget.id)
+         @widget.reload
+       }.should change(@widget, :impressions_count).from(0).to(1)
+    end
+  end
+
+end