Added tests for counter cache logic

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)
+    impressionable_class = self.impressionable_type.constantize
-    resouce.update_counter_cache if resouce.try(:cache_impression_count?)
+
+    if impressionable_class.counter_caching?
+      resouce = impressionable_class.find(self.impressionable_id)
+      resouce.try(:update_counter_cache)
+    end
   end
 end

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

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"

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)
+    ZenTest (4.6.2)
-    actionmailer (3.1.0.rc1)
+    actionmailer (3.1.0)
-      actionpack (= 3.1.0.rc1)
+      actionpack (= 3.1.0)
       mail (~> 2.3.0)
-    actionpack (3.1.0.rc1)
+    actionpack (3.1.0)
-      activemodel (= 3.1.0.rc1)
+      activemodel (= 3.1.0)
-      activesupport (= 3.1.0.rc1)
+      activesupport (= 3.1.0)
       builder (~> 3.0.0)
       erubis (~> 2.7.0)
-      i18n (~> 0.6.0beta1)
+      i18n (~> 0.6)
-      rack (~> 1.3.0.beta2)
+      rack (~> 1.3.2)
-      rack-cache (~> 1.0.1)
+      rack-cache (~> 1.0.3)
-      rack-mount (~> 0.8.1)
+      rack-mount (~> 0.8.2)
-      rack-test (~> 0.6.0)
+      rack-test (~> 0.6.1)
-      sprockets (~> 2.0.0.beta.5)
+      sprockets (~> 2.0.0)
-      tzinfo (~> 0.3.27)
+    activemodel (3.1.0)
-    activemodel (3.1.0.rc1)
+      activesupport (= 3.1.0)
-      activesupport (= 3.1.0.rc1)
+      bcrypt-ruby (~> 3.0.0)
-      bcrypt-ruby (~> 2.1.4)
       builder (~> 3.0.0)
-      i18n (~> 0.6.0beta1)
+      i18n (~> 0.6)
-    activerecord (3.1.0.rc1)
+    activerecord (3.1.0)
-      activemodel (= 3.1.0.rc1)
+      activemodel (= 3.1.0)
-      activesupport (= 3.1.0.rc1)
+      activesupport (= 3.1.0)
-      arel (~> 2.1.1)
+      arel (~> 2.2.1)
-      tzinfo (~> 0.3.27)
+      tzinfo (~> 0.3.29)
-    activeresource (3.1.0.rc1)
+    activeresource (3.1.0)
-      activemodel (= 3.1.0.rc1)
+      activemodel (= 3.1.0)
-      activesupport (= 3.1.0.rc1)
+      activesupport (= 3.1.0)
-    activesupport (3.1.0.rc1)
+    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-notification (2.3.3)
-      autotest (~> 4.3)
+      autotest-standalone (~> 4.5)
-    bcrypt-ruby (2.1.4)
+    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 (1.1.0)
-    cucumber (0.10.3)
       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)
+      term-ansicolor (>= 1.0.6)
-    cucumber-rails (0.5.1)
+    cucumber-rails (1.1.1)
-      capybara (>= 1.0.0.beta1)
+      capybara (>= 1.1.1)
-      cucumber (>= 0.10.3)
+      cucumber (>= 1.1.0)
-      nokogiri (>= 1.4.4)
+      nokogiri (>= 1.5.0)
-      rack-test (>= 0.5.7)
     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 (1.6.1)
-    json_pure (1.5.1)
+    json_pure (1.6.1)
-    launchy (0.4.0)
+    launchy (2.0.5)
-      configuration (>= 0.0.5)
+      addressable (~> 2.2.6)
-      rake (>= 0.8.1)
     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)
+    polyglot (0.3.2)
-    rack (1.3.0)
+    rack (1.3.5)
-    rack-cache (1.0.2)
+    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)
+    rails (3.1.0)
-      actionmailer (= 3.1.0.rc1)
+      actionmailer (= 3.1.0)
-      actionpack (= 3.1.0.rc1)
+      actionpack (= 3.1.0)
-      activerecord (= 3.1.0.rc1)
+      activerecord (= 3.1.0)
-      activeresource (= 3.1.0.rc1)
+      activeresource (= 3.1.0)
-      activesupport (= 3.1.0.rc1)
+      activesupport (= 3.1.0)
       bundler (~> 1.0)
-      railties (= 3.1.0.rc1)
+      railties (= 3.1.0)
-    railties (3.1.0.rc1)
+    railties (3.1.0)
-      actionpack (= 3.1.0.rc1)
+      actionpack (= 3.1.0)
-      activesupport (= 3.1.0.rc1)
+      activesupport (= 3.1.0)
       rack-ssl (~> 1.3.2)
       rake (>= 0.8.7)
+      rdoc (~> 3.4)
       thor (~> 0.14.6)
-    rake (0.9.1)
+    rake (0.9.2.2)
-    rspec (2.6.0)
+    rdoc (3.11)
-      rspec-core (~> 2.6.0)
+      json (~> 1.4)
-      rspec-expectations (~> 2.6.0)
+    rspec (2.7.0)
-      rspec-mocks (~> 2.6.0)
+      rspec-core (~> 2.7.0)
-    rspec-core (2.6.3)
+      rspec-expectations (~> 2.7.0)
-    rspec-expectations (2.6.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-mocks (2.7.0)
-    rspec-rails (2.6.1)
+    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)
+    selenium-webdriver (2.10.0)
-      childprocess (>= 0.1.7)
+      childprocess (>= 0.2.1)
-      ffi (>= 1.0.7)
+      ffi (= 1.0.9)
       json_pure
       rubyzip
     spork (0.8.5)
-    sprockets (2.0.0.beta.9)
+    sprockets (2.0.3)
-      hike (~> 1.0)
+      hike (~> 1.2)
       rack (~> 1.0)
-      tilt (~> 1.1, != 1.3.0)
+      tilt (!= 1.3.0, ~> 1.1)
-    sqlite3 (1.3.3)
+    sqlite3 (1.3.4)
     sqlite3-ruby (1.3.3)
       sqlite3 (>= 1.3.3)
-    systemu (2.2.0)
+    systemu (2.4.0)
-    term-ansicolor (1.0.5)
+    term-ansicolor (1.0.7)
     thor (0.14.6)
-    tilt (1.3.2)
+    tilt (1.3.3)
-    treetop (1.4.9)
+    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

test_app/app/models/widget.rb

@@ -0,0 +1,3 @@
+class Widget < ActiveRecord::Base
+  is_impressionable :counter_cache => true
+end

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
-  

test_app/spec/fixtures/widgets.yml

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

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

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