From 77219d23a581ad8e0600822f34df63f0a9a29246 Mon Sep 17 00:00:00 2001 From: Cory Schires Date: Mon, 31 Oct 2011 12:02:31 -0500 Subject: [PATCH] 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