From 9d0b22515bff541f6c8e39e77edf18aea77005ee Mon Sep 17 00:00:00 2001
From: Cory Schires <coryschires@gmail.com>
Date: Sat, 29 Oct 2011 17:24:57 -0500
Subject: [PATCH 1/4] Added support for counter cache column.

---
 README.md                                  | 37 +++++++++++++---------
 app/models/impression.rb                   |  9 ++++++
 app/models/impressionist/impressionable.rb | 23 ++++++++++----
 3 files changed, 48 insertions(+), 21 deletions(-)

diff --git a/README.md b/README.md
index 7b74ff3..743c7ef 100644
--- a/README.md
+++ b/README.md
@@ -5,20 +5,14 @@ impressionist
 
 A lightweight plugin that logs impressions per action or manually per model
 
+I would not call this a stable plugin yet, although I have been running it in prod with no problems.  Use at your own risk ;-)
 ------------------------------------------------------------------------------------------------------------------------------
 
-NOTE: If you are upgrading from a version prior to 0.4.0, you will need to run this migration after the upgrade:
-
-https://github.com/charlotte-ruby/impressionist/blob/master/upgrade_migrations/version_0_4_0.rb
-
-If you don't run this migration you will receive this error: Unknown attribute : referrer
-
-
 What does this thing do?
 ------------------------
-Logs an impression... and I use that term loosely.  It can log page impressions (technically action impressions), but it is not limited to that.  
-You can log impressions multiple times per request.  And you can also attach it to a model.  The goal of this project is to provide customizable 
-stats that are immediately accessible in your application as opposed to using G Analytics and pulling data using their API.  You can attach custom 
+Logs an impression... and I use that term loosely.  It can log page impressions (technically action impressions), but it is not limited to that.
+You can log impressions multiple times per request.  And you can also attach it to a model.  The goal of this project is to provide customizable
+stats that are immediately accessible in your application as opposed to using G Analytics and pulling data using their API.  You can attach custom
 messages to impressions.  No reporting yet.. this thingy just creates the data.
 
 What about bots?
@@ -28,7 +22,7 @@ http://www.user-agents.org/allagents.xml
 
 Which versions of Rails and Ruby is this compatible with?
 ---------------------------------------------------------
-Rails 3.0.x and Ruby 1.9.2 (also tested on REE 1.8.7) - Sorry, but you need to upgrade if you are using Rails 2.  You know you want to anyways.. all the cool kids are doing it ;-)
+Rails 3.0.4 and Ruby 1.9.2 (also tested on REE 1.8.7) - Sorry, but you need to upgrade if you are using Rails 2.  You know you want to anyways.. all the cool kids are doing it ;-)
 
 Installation
 ------------
@@ -101,17 +95,30 @@ Usage
 6. Get the unique impression count from a model filtered by IP address.  This in turn will give you impressions with unique request_hash, since rows with the same request_hash will have the same IP address.
 
         @widget.impressionist_count(:filter=>:ip_address)
-        
+
 7. Get the unique impression count from a model filtered by session hash.  Same as #6 regarding request hash.  This may be more desirable than filtering by IP address depending on your situation, since filtering by IP may ignore visitors that use the same IP.  The downside to this filtering is that a user could clear session data in their browser and skew the results.
 
         @widget.impressionist_count(:filter=>:session_hash)
-    
+
 8. Get total impression count.  This may return more than 1 impression per http request, depending on how you are logging impressions
 
         @widget.impressionist_count(:filter=>:all)
 
 Logging impressions for authenticated users happens automatically.  If you have a current_user helper or use @current_user in your before_filter to set your authenticated user, current_user.id will be written to the user_id field in the impressions table.
 
+Adding a counter cache
+----------------------
+Impressionist makes it easy to add a `counter_cache` column to your model. The most basic configuration looks like:
+
+  is_impressionable :counter_cache => true
+
+This will automatically increment the `impressions_count` column in the included model. Note: You'll need to add that column to your model. If you'd like specific a different column name, you can:
+
+  is_impressionable :counter_cache => { :column_name => :my_column }
+
+If you'd like to include only unique impressions in your count:
+
+  is_impressionable :counter_cache => { :column_name => :my_column, :unique => true }
 
 Development Roadmap
 -------------------
@@ -121,14 +128,14 @@ Development Roadmap
 * AB testing integration
 
 Contributing to impressionist
------------------------------ 
+-----------------------------
 * Check out the latest master to make sure the feature hasn't been implemented or the bug hasn't been fixed yet
 * Check out the issue tracker to make sure someone already hasn't requested it and/or contributed it
 * Fork the project
 * Start a feature/bugfix branch
 * Commit and push until you are happy with your contribution
 * Make sure to add rpsec tests for it. Patches or features without tests will be ignored.  Also, try to write better tests than I do ;-)
-* If adding engine controller or view functionality, use HAML and Inherited Resources.  
+* If adding engine controller or view functionality, use HAML and Inherited Resources.
 * All testing is done inside a small Rails app (test_app).  You will find specs within this app.
 
 Copyright (c) 2011 John McAliley. See LICENSE.txt for further details.
diff --git a/app/models/impression.rb b/app/models/impression.rb
index 30f26ea..a7c00d6 100644
--- a/app/models/impression.rb
+++ b/app/models/impression.rb
@@ -1,3 +1,12 @@
 class Impression < ActiveRecord::Base
   belongs_to :impressionable, :polymorphic=>true
+
+  after_save :update_impressions_counter_cache
+
+  private
+
+  def update_impressions_counter_cache
+    resouce = self.impressionable_type.constantize.find(self.impressionable_id)
+    resouce.update_counter_cache if resouce.try(:cache_impression_count?)
+  end
 end
\ No newline at end of file
diff --git a/app/models/impressionist/impressionable.rb b/app/models/impressionist/impressionable.rb
index 57accfb..c60d4dd 100644
--- a/app/models/impressionist/impressionable.rb
+++ b/app/models/impressionist/impressionable.rb
@@ -1,15 +1,16 @@
 module Impressionist
   module Impressionable
-    def is_impressionable
+    def is_impressionable(options={})
       has_many :impressions, :as=>:impressionable
+      @counter_cache_options = options[:counter_cache] ? options[:counter_cache] : nil
       include InstanceMethods
     end
-    
+
     module InstanceMethods
       def impressionable?
         true
       end
-      
+
       def impressionist_count(options={})
         options.reverse_merge!(:filter=>:request_hash, :start_date=>nil, :end_date=>Time.now)
         imps = options[:start_date].blank? ? impressions : impressions.where("created_at>=? and created_at<=?",options[:start_date],options[:end_date])
@@ -18,7 +19,17 @@ module Impressionist
         end
         imps.all.size
       end
-      
+
+      def cache_impression_count?
+        ! @counter_cache_options.nil?
+      end
+
+      def update_counter_cache
+        column_name = @counter_cache_options[:column_name] || :impressions_count
+        impression_count = @counter_cache_options[:unique] ?  impressionist_count(filter: :ip_address) : impressionist_count
+        self.class.update_attribute(column_name.to_sym, impression_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})
@@ -27,11 +38,11 @@ module Impressionist
       def unique_impression_count(start_date=nil,end_date=Time.now)
         impressionist_count({:start_date=>start_date, :end_date=>end_date, :filter=> :request_hash})
       end
-      
+
       def unique_impression_count_ip(start_date=nil,end_date=Time.now)
         impressionist_count({:start_date=>start_date, :end_date=>end_date, :filter=> :ip_address})
       end
-      
+
       def unique_impression_count_session(start_date=nil,end_date=Time.now)
         impressionist_count({:start_date=>start_date, :end_date=>end_date, :filter=> :session_hash})
       end

From 77219d23a581ad8e0600822f34df63f0a9a29246 Mon Sep 17 00:00:00 2001
From: Cory Schires <coryschires@gmail.com>
Date: Mon, 31 Oct 2011 12:02:31 -0500
Subject: [PATCH 2/4] Added tests for counter cache logic

---
 app/models/impression.rb                     |   8 +-
 app/models/impressionist/impressionable.rb   |  31 ++--
 test_app/Gemfile                             |   2 +-
 test_app/Gemfile.lock                        | 186 ++++++++++---------
 test_app/app/models/widget.rb                |   3 +
 test_app/spec/controllers/controller_spec.rb |  33 ++--
 test_app/spec/fixtures/widgets.yml           |   3 +
 test_app/spec/models/counter_caching.rb      |  23 +++
 test_app/spec/models/model_spec.rb           |   2 +
 9 files changed, 171 insertions(+), 120 deletions(-)
 create mode 100644 test_app/app/models/widget.rb
 create mode 100644 test_app/spec/fixtures/widgets.yml
 create mode 100644 test_app/spec/models/counter_caching.rb

diff --git a/app/models/impression.rb b/app/models/impression.rb
index a7c00d6..95b82b7 100644
--- a/app/models/impression.rb
+++ b/app/models/impression.rb
@@ -6,7 +6,11 @@ class Impression < ActiveRecord::Base
   private
 
   def update_impressions_counter_cache
-    resouce = self.impressionable_type.constantize.find(self.impressionable_id)
-    resouce.update_counter_cache if resouce.try(:cache_impression_count?)
+    impressionable_class = self.impressionable_type.constantize
+
+    if impressionable_class.counter_caching?
+      resouce = impressionable_class.find(self.impressionable_id)
+      resouce.try(:update_counter_cache)
+    end
   end
 end
\ No newline at end of file
diff --git a/app/models/impressionist/impressionable.rb b/app/models/impressionist/impressionable.rb
index c60d4dd..0540f06 100644
--- a/app/models/impressionist/impressionable.rb
+++ b/app/models/impressionist/impressionable.rb
@@ -1,8 +1,26 @@
 module Impressionist
   module Impressionable
+
     def is_impressionable(options={})
       has_many :impressions, :as=>:impressionable
-      @counter_cache_options = options[:counter_cache] ? options[:counter_cache] : nil
+
+      @cache_options = options[:counter_cache]
+
+      if @cache_options.present?
+        @cache_options = { :column_name => :impressions_count, :unique => false }
+        @cache_options.merge!(options[:counter_cache]) if options[:counter_cache].is_a?(Hash)
+      end
+
+      def update_counter_cache
+        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
+
+      def self.counter_caching?
+        @cache_options.present?
+      end
+
       include InstanceMethods
     end
 
@@ -20,16 +38,6 @@ module Impressionist
         imps.all.size
       end
 
-      def cache_impression_count?
-        ! @counter_cache_options.nil?
-      end
-
-      def update_counter_cache
-        column_name = @counter_cache_options[:column_name] || :impressions_count
-        impression_count = @counter_cache_options[:unique] ?  impressionist_count(filter: :ip_address) : impressionist_count
-        self.class.update_attribute(column_name.to_sym, impression_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})
@@ -47,5 +55,6 @@ module Impressionist
         impressionist_count({:start_date=>start_date, :end_date=>end_date, :filter=> :session_hash})
       end
     end
+
   end
 end
diff --git a/test_app/Gemfile b/test_app/Gemfile
index cfc3afe..b69c41b 100644
--- a/test_app/Gemfile
+++ b/test_app/Gemfile
@@ -1,6 +1,6 @@
 source 'http://rubygems.org'
 
-gem 'rails', '3.1.0.rc1'
+gem 'rails', '3.1'
 gem 'sqlite3-ruby', :require => 'sqlite3'
 gem 'impressionist', :path=>"#{File.dirname(__FILE__)}/../"
 gem "pg"
diff --git a/test_app/Gemfile.lock b/test_app/Gemfile.lock
index b5676b2..24b0d29 100644
--- a/test_app/Gemfile.lock
+++ b/test_app/Gemfile.lock
@@ -1,155 +1,157 @@
 PATH
-  remote: /rails_plugins/mine/impressionist
+  remote: /Users/coryschires/Desktop/work/applications/impressionist
   specs:
-    impressionist (0.3.2)
+    impressionist (0.4.0)
 
 GEM
   remote: http://rubygems.org/
   specs:
-    ZenTest (4.5.0)
-    actionmailer (3.1.0.rc1)
-      actionpack (= 3.1.0.rc1)
+    ZenTest (4.6.2)
+    actionmailer (3.1.0)
+      actionpack (= 3.1.0)
       mail (~> 2.3.0)
-    actionpack (3.1.0.rc1)
-      activemodel (= 3.1.0.rc1)
-      activesupport (= 3.1.0.rc1)
+    actionpack (3.1.0)
+      activemodel (= 3.1.0)
+      activesupport (= 3.1.0)
       builder (~> 3.0.0)
       erubis (~> 2.7.0)
-      i18n (~> 0.6.0beta1)
-      rack (~> 1.3.0.beta2)
-      rack-cache (~> 1.0.1)
-      rack-mount (~> 0.8.1)
-      rack-test (~> 0.6.0)
-      sprockets (~> 2.0.0.beta.5)
-      tzinfo (~> 0.3.27)
-    activemodel (3.1.0.rc1)
-      activesupport (= 3.1.0.rc1)
-      bcrypt-ruby (~> 2.1.4)
+      i18n (~> 0.6)
+      rack (~> 1.3.2)
+      rack-cache (~> 1.0.3)
+      rack-mount (~> 0.8.2)
+      rack-test (~> 0.6.1)
+      sprockets (~> 2.0.0)
+    activemodel (3.1.0)
+      activesupport (= 3.1.0)
+      bcrypt-ruby (~> 3.0.0)
       builder (~> 3.0.0)
-      i18n (~> 0.6.0beta1)
-    activerecord (3.1.0.rc1)
-      activemodel (= 3.1.0.rc1)
-      activesupport (= 3.1.0.rc1)
-      arel (~> 2.1.1)
-      tzinfo (~> 0.3.27)
-    activeresource (3.1.0.rc1)
-      activemodel (= 3.1.0.rc1)
-      activesupport (= 3.1.0.rc1)
-    activesupport (3.1.0.rc1)
+      i18n (~> 0.6)
+    activerecord (3.1.0)
+      activemodel (= 3.1.0)
+      activesupport (= 3.1.0)
+      arel (~> 2.2.1)
+      tzinfo (~> 0.3.29)
+    activeresource (3.1.0)
+      activemodel (= 3.1.0)
+      activesupport (= 3.1.0)
+    activesupport (3.1.0)
       multi_json (~> 1.0)
-    arel (2.1.1)
+    addressable (2.2.6)
+    arel (2.2.1)
     autotest (4.4.6)
       ZenTest (>= 4.4.1)
-    autotest-notification (2.3.1)
-      autotest (~> 4.3)
-    bcrypt-ruby (2.1.4)
+    autotest-notification (2.3.3)
+      autotest-standalone (~> 4.5)
+    autotest-standalone (4.5.8)
+    bcrypt-ruby (3.0.1)
     builder (3.0.0)
-    capybara (1.0.0.beta1)
+    capybara (1.1.1)
       mime-types (>= 1.16)
       nokogiri (>= 1.3.3)
       rack (>= 1.0.0)
       rack-test (>= 0.5.4)
-      selenium-webdriver (>= 0.0.27)
+      selenium-webdriver (~> 2.0)
       xpath (~> 0.1.4)
-    childprocess (0.1.9)
+    childprocess (0.2.2)
       ffi (~> 1.0.6)
-    configuration (1.2.0)
-    cucumber (0.10.3)
+    cucumber (1.1.0)
       builder (>= 2.1.2)
       diff-lcs (>= 1.1.2)
-      gherkin (>= 2.3.8)
+      gherkin (~> 2.5.0)
       json (>= 1.4.6)
-      term-ansicolor (>= 1.0.5)
-    cucumber-rails (0.5.1)
-      capybara (>= 1.0.0.beta1)
-      cucumber (>= 0.10.3)
-      nokogiri (>= 1.4.4)
-      rack-test (>= 0.5.7)
+      term-ansicolor (>= 1.0.6)
+    cucumber-rails (1.1.1)
+      capybara (>= 1.1.1)
+      cucumber (>= 1.1.0)
+      nokogiri (>= 1.5.0)
     daemons (1.0.10)
     database_cleaner (0.6.7)
-    diff-lcs (1.1.2)
+    diff-lcs (1.1.3)
     erubis (2.7.0)
     ffi (1.0.9)
     gem_plugin (0.2.3)
-    gherkin (2.3.10)
+    gherkin (2.5.4)
       json (>= 1.4.6)
-    hike (1.0.0)
+    hike (1.2.1)
     i18n (0.6.0)
-    json (1.5.1)
-    json_pure (1.5.1)
-    launchy (0.4.0)
-      configuration (>= 0.0.5)
-      rake (>= 0.8.1)
+    json (1.6.1)
+    json_pure (1.6.1)
+    launchy (2.0.5)
+      addressable (~> 2.2.6)
     mail (2.3.0)
       i18n (>= 0.4.0)
       mime-types (~> 1.16)
       treetop (~> 1.4.8)
-    mime-types (1.16)
+    mime-types (1.17.2)
     mongrel (1.2.0.pre2)
       daemons (~> 1.0.10)
       gem_plugin (~> 0.2.3)
     multi_json (1.0.3)
-    nokogiri (1.4.4)
+    nokogiri (1.5.0)
     pg (0.11.0)
-    polyglot (0.3.1)
-    rack (1.3.0)
-    rack-cache (1.0.2)
+    polyglot (0.3.2)
+    rack (1.3.5)
+    rack-cache (1.0.3)
       rack (>= 0.4)
-    rack-mount (0.8.1)
+    rack-mount (0.8.3)
       rack (>= 1.0.0)
     rack-ssl (1.3.2)
       rack
-    rack-test (0.6.0)
+    rack-test (0.6.1)
       rack (>= 1.0)
-    rails (3.1.0.rc1)
-      actionmailer (= 3.1.0.rc1)
-      actionpack (= 3.1.0.rc1)
-      activerecord (= 3.1.0.rc1)
-      activeresource (= 3.1.0.rc1)
-      activesupport (= 3.1.0.rc1)
+    rails (3.1.0)
+      actionmailer (= 3.1.0)
+      actionpack (= 3.1.0)
+      activerecord (= 3.1.0)
+      activeresource (= 3.1.0)
+      activesupport (= 3.1.0)
       bundler (~> 1.0)
-      railties (= 3.1.0.rc1)
-    railties (3.1.0.rc1)
-      actionpack (= 3.1.0.rc1)
-      activesupport (= 3.1.0.rc1)
+      railties (= 3.1.0)
+    railties (3.1.0)
+      actionpack (= 3.1.0)
+      activesupport (= 3.1.0)
       rack-ssl (~> 1.3.2)
       rake (>= 0.8.7)
+      rdoc (~> 3.4)
       thor (~> 0.14.6)
-    rake (0.9.1)
-    rspec (2.6.0)
-      rspec-core (~> 2.6.0)
-      rspec-expectations (~> 2.6.0)
-      rspec-mocks (~> 2.6.0)
-    rspec-core (2.6.3)
-    rspec-expectations (2.6.0)
+    rake (0.9.2.2)
+    rdoc (3.11)
+      json (~> 1.4)
+    rspec (2.7.0)
+      rspec-core (~> 2.7.0)
+      rspec-expectations (~> 2.7.0)
+      rspec-mocks (~> 2.7.0)
+    rspec-core (2.7.1)
+    rspec-expectations (2.7.0)
       diff-lcs (~> 1.1.2)
-    rspec-mocks (2.6.0)
-    rspec-rails (2.6.1)
+    rspec-mocks (2.7.0)
+    rspec-rails (2.7.0)
       actionpack (~> 3.0)
       activesupport (~> 3.0)
       railties (~> 3.0)
-      rspec (~> 2.6.0)
+      rspec (~> 2.7.0)
     rubyzip (0.9.4)
-    selenium-webdriver (0.2.1)
-      childprocess (>= 0.1.7)
-      ffi (>= 1.0.7)
+    selenium-webdriver (2.10.0)
+      childprocess (>= 0.2.1)
+      ffi (= 1.0.9)
       json_pure
       rubyzip
     spork (0.8.5)
-    sprockets (2.0.0.beta.9)
-      hike (~> 1.0)
+    sprockets (2.0.3)
+      hike (~> 1.2)
       rack (~> 1.0)
-      tilt (~> 1.1, != 1.3.0)
-    sqlite3 (1.3.3)
+      tilt (!= 1.3.0, ~> 1.1)
+    sqlite3 (1.3.4)
     sqlite3-ruby (1.3.3)
       sqlite3 (>= 1.3.3)
-    systemu (2.2.0)
-    term-ansicolor (1.0.5)
+    systemu (2.4.0)
+    term-ansicolor (1.0.7)
     thor (0.14.6)
-    tilt (1.3.2)
-    treetop (1.4.9)
+    tilt (1.3.3)
+    treetop (1.4.10)
+      polyglot
       polyglot (>= 0.3.1)
-    tzinfo (0.3.27)
+    tzinfo (0.3.30)
     xpath (0.1.4)
       nokogiri (~> 1.3)
 
@@ -168,7 +170,7 @@ DEPENDENCIES
   launchy
   mongrel (= 1.2.0.pre2)
   pg
-  rails (= 3.1.0.rc1)
+  rails (= 3.1)
   rspec
   rspec-rails
   spork
diff --git a/test_app/app/models/widget.rb b/test_app/app/models/widget.rb
new file mode 100644
index 0000000..51bab18
--- /dev/null
+++ b/test_app/app/models/widget.rb
@@ -0,0 +1,3 @@
+class Widget < ActiveRecord::Base
+  is_impressionable :counter_cache => true
+end
diff --git a/test_app/spec/controllers/controller_spec.rb b/test_app/spec/controllers/controller_spec.rb
index c303671..8f7fe09 100644
--- a/test_app/spec/controllers/controller_spec.rb
+++ b/test_app/spec/controllers/controller_spec.rb
@@ -1,14 +1,14 @@
 require 'spec_helper.rb'
 
 describe ArticlesController do
-  fixtures :articles,:impressions,:posts
+  fixtures :articles,:impressions,:posts,:widgets
   render_views
-  
+
   it "should make the impressionable_hash available" do
     get "index"
     response.body.include?("false").should eq true
   end
-  
+
   it "should log an impression with a message" do
     get "index"
     Impression.all.size.should eq 12
@@ -16,7 +16,7 @@ describe ArticlesController do
     Article.first.impressions.last.controller_name.should eq "articles"
     Article.first.impressions.last.action_name.should eq "index"
   end
-  
+
   it "should log an impression without a message" do
     get "show", :id=> 1
     Impression.all.size.should eq 12
@@ -24,18 +24,18 @@ describe ArticlesController do
     Article.first.impressions.last.controller_name.should eq "articles"
     Article.first.impressions.last.action_name.should eq "show"
   end
-  
+
   it "should log the user_id if user is authenticated (@current_user before_filter method)" do
     session[:user_id] = 123
     get "show", :id=> 1
     Article.first.impressions.last.user_id.should eq 123
   end
-  
+
   it "should not log the user_id if user is authenticated" do
     get "show", :id=> 1
     Article.first.impressions.last.user_id.should eq nil
   end
-  
+
   it "should log the request_hash, ip_address, referrer and session_hash" do
     get "show", :id=> 1
     Impression.last.request_hash.size.should eq 64
@@ -43,13 +43,13 @@ describe ArticlesController do
     Impression.last.session_hash.size.should eq 32
     Impression.last.referrer.should eq nil
   end
-  
+
   it "should log the referrer when you click a link" do
     visit article_url(Article.first)
     click_link "Same Page"
     Impression.last.referrer.should eq "http://test.host/articles/1"
   end
-end  
+end
 
 describe PostsController do
   it "should log impression at the action level" do
@@ -60,7 +60,7 @@ describe PostsController do
     Impression.last.impressionable_type.should eq "Post"
     Impression.last.impressionable_id.should eq 1
   end
-  
+
   it "should log the user_id if user is authenticated (current_user helper method)" do
     session[:user_id] = 123
     get "show", :id=> 1
@@ -69,6 +69,12 @@ describe PostsController do
 end
 
 describe WidgetsController do
+
+  before(:each) do
+    @widget = Widget.find(1)
+    Widget.stub(:find).and_return(@widget)
+  end
+
   it "should log impression at the per action level" do
     get "show", :id=> 1
     Impression.all.size.should eq 12
@@ -77,17 +83,16 @@ describe WidgetsController do
     get "new"
     Impression.all.size.should eq 13
   end
-  
+
   it "should not log impression when user-agent is in wildcard list" do
     request.stub!(:user_agent).and_return('somebot')
     get "show", :id=> 1
     Impression.all.size.should eq 11
   end
-  
+
   it "should not log impression when user-agent is in the bot list" do
     request.stub!(:user_agent).and_return('Acoon Robot v1.50.001')
     get "show", :id=> 1
-    Impression.all.size.should eq 11    
+    Impression.all.size.should eq 11
   end
 end
-  
\ No newline at end of file
diff --git a/test_app/spec/fixtures/widgets.yml b/test_app/spec/fixtures/widgets.yml
new file mode 100644
index 0000000..500a5f4
--- /dev/null
+++ b/test_app/spec/fixtures/widgets.yml
@@ -0,0 +1,3 @@
+one:
+  id: 1
+  name: A Widget
\ No newline at end of file
diff --git a/test_app/spec/models/counter_caching.rb b/test_app/spec/models/counter_caching.rb
new file mode 100644
index 0000000..5eda6cb
--- /dev/null
+++ b/test_app/spec/models/counter_caching.rb
@@ -0,0 +1,23 @@
+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
\ No newline at end of file
diff --git a/test_app/spec/models/model_spec.rb b/test_app/spec/models/model_spec.rb
index 2ed2613..bd9f558 100644
--- a/test_app/spec/models/model_spec.rb
+++ b/test_app/spec/models/model_spec.rb
@@ -49,6 +49,8 @@ describe Impression do
     @article.impressionist_count(:filter=>:session_hash).should eq 7
   end
   
+  
+  
   #OLD COUNT METHODS.  DEPRECATE SOON
   it "should return the impression count with no date range specified" do
     @article.impression_count.should eq 11

From 5386874eca1959ea84751b44a3332ac01f90089a Mon Sep 17 00:00:00 2001
From: Cory Schires <coryschires@gmail.com>
Date: Mon, 31 Oct 2011 13:18:46 -0500
Subject: [PATCH 3/4] Better tests for counter caching.

---
 app/models/impression.rb                     |  2 +-
 app/models/impressionist/impressionable.rb   | 41 +++++++++++++-------
 test_app/spec/fixtures/widgets.yml           |  3 +-
 test_app/spec/models/counter_caching.rb      | 23 -----------
 test_app/spec/models/counter_caching_spec.rb | 30 ++++++++++++++
 5 files changed, 60 insertions(+), 39 deletions(-)
 delete mode 100644 test_app/spec/models/counter_caching.rb
 create mode 100644 test_app/spec/models/counter_caching_spec.rb

diff --git a/app/models/impression.rb b/app/models/impression.rb
index 95b82b7..8b01ce9 100644
--- a/app/models/impression.rb
+++ b/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
diff --git a/app/models/impressionist/impressionable.rb b/app/models/impressionist/impressionable.rb
index 0540f06..4562af2 100644
--- a/app/models/impressionist/impressionable.rb
+++ b/app/models/impressionist/impressionable.rb
@@ -1,27 +1,31 @@
 module Impressionist
   module Impressionable
 
-    def is_impressionable(options={})
-      has_many :impressions, :as=>:impressionable
+    def self.included(base)
+      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?
-        @cache_options = { :column_name => :impressions_count, :unique => false }
-        @cache_options.merge!(options[:counter_cache]) if options[:counter_cache].is_a?(Hash)
+      def is_impressionable(options={})
+        has_many :impressions, :as=>:impressionable
+        @cache_options = options[:counter_cache]
       end
 
-      def update_counter_cache
-        column_name = cache_options[:column_name].to_sym
-        count = @cache_options[:unique] ? impressionist_count(:filter => :ip_address) : impressionist_count
-        update_attribute(column_name, count)
+      def counter_cache_options
+        if @cache_options
+          options = { :column_name => :impressions_count, :unique => false }
+          options.merge!(@cache_options) if @cache_options.is_a?(Hash)
+          options
+        end
       end
 
-      def self.counter_caching?
-        @cache_options.present?
+      def counter_caching?
+        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)
diff --git a/test_app/spec/fixtures/widgets.yml b/test_app/spec/fixtures/widgets.yml
index 500a5f4..e93905c 100644
--- a/test_app/spec/fixtures/widgets.yml
+++ b/test_app/spec/fixtures/widgets.yml
@@ -1,3 +1,4 @@
 one:
   id: 1
-  name: A Widget
\ No newline at end of file
+  name: A Widget
+  impressions_count: 0
\ No newline at end of file
diff --git a/test_app/spec/models/counter_caching.rb b/test_app/spec/models/counter_caching.rb
deleted file mode 100644
index 5eda6cb..0000000
--- a/test_app/spec/models/counter_caching.rb
+++ /dev/null
@@ -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
\ No newline at end of file
diff --git a/test_app/spec/models/counter_caching_spec.rb b/test_app/spec/models/counter_caching_spec.rb
new file mode 100644
index 0000000..9c27a61
--- /dev/null
+++ b/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
\ No newline at end of file

From aef6acc583db0fdcee0d82bf89b2cf489a84d8f5 Mon Sep 17 00:00:00 2001
From: Cory Schires <coryschires@gmail.com>
Date: Fri, 4 Nov 2011 10:27:13 -0500
Subject: [PATCH 4/4] Move AR include into engine file.

---
 app/models/impressionist/impressionable.rb | 2 --
 lib/impressionist/engine.rb                | 6 +++---
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/app/models/impressionist/impressionable.rb b/app/models/impressionist/impressionable.rb
index 4562af2..9b5d650 100644
--- a/app/models/impressionist/impressionable.rb
+++ b/app/models/impressionist/impressionable.rb
@@ -69,5 +69,3 @@ module Impressionist
 
   end
 end
-
-ActiveRecord::Base.send(:include, Impressionist::Impressionable)
diff --git a/lib/impressionist/engine.rb b/lib/impressionist/engine.rb
index ffa1244..295fcf2 100644
--- a/lib/impressionist/engine.rb
+++ b/lib/impressionist/engine.rb
@@ -2,11 +2,11 @@ require "impressionist"
 require "rails"
 
 module Impressionist
-  class Engine < Rails::Engine    
+  class Engine < Rails::Engine
     initializer 'impressionist.extend_ar' do |app|
-      ActiveRecord::Base.extend Impressionist::Impressionable
+      ActiveRecord::Base.send(:include, Impressionist::Impressionable)
     end
-    
+
     initializer 'impressionist.controller' do
       ActiveSupport.on_load(:action_controller) do
         include ImpressionistController::InstanceMethods