added tests for uniqueness feature

test_app/app/controllers/dummy_controller.rb

@@ -0,0 +1,6 @@
+# This controller imports the impressionist module to make the modules methods available for testing
+class DummyController < ActionController::Base
+
+  impressionist
+  
+end

test_app/app/controllers/widgets_controller.rb

@@ -1,5 +1,6 @@
 class WidgetsController < ApplicationController
-  impressionist :actions=>[:show,:index], :unique => [:action_name, :impressionable_id]
+  impressionist :actions=>[:show,:index], :unique => [:controller_name,:action_name,:impressionable_id]
+  
   def show
   end
   

test_app/spec/controllers/controller_spec.rb

@@ -78,28 +78,6 @@ describe WidgetsController do
     Impression.all.size.should eq 13
   end
   
-  it "should log unique impressions at the per action" do
-    get "show", :id=> 1
-    Impression.all.size.should eq 12
-    get "show", :id=> 2
-    Impression.all.size.should eq 13
-    get "show", :id => 2
-    Impression.all.size.should eq 13
-    get "index"
-    Impression.all.size.should eq 14
-  end
-  
-  it "should log unique impressions only once per id" do
-    get "show", :id=> 1
-    Impression.all.size.should eq 12
-    get "show", :id=> 2
-    Impression.all.size.should eq 13
-    get "show", :id => 2
-    Impression.all.size.should eq 13
-    get "index"
-    Impression.all.size.should eq 14
-  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
@@ -111,5 +89,32 @@ describe WidgetsController do
     get "show", :id=> 1
     Impression.all.size.should eq 11    
   end
+  
+  describe "impressionist unique options" do
+    
+    it "should log unique impressions at the per action level" do
+      get "show", :id=> 1
+      Impression.all.size.should eq 12
+      get "show", :id=> 2
+      Impression.all.size.should eq 13
+      get "show", :id => 2
+      Impression.all.size.should eq 13
+      get "index"
+      Impression.all.size.should eq 14
+    end
+    
+    it "should log unique impressions only once per id" do
+      get "show", :id=> 1
+      Impression.all.size.should eq 12
+      get "show", :id=> 2
+      Impression.all.size.should eq 13
+      get "show", :id => 2
+      Impression.all.size.should eq 13
+      get "index"
+      Impression.all.size.should eq 14
+    end
+  
+  end
+  
 end
   

test_app/spec/controllers/impressionist_uniqueness_spec.rb

@@ -0,0 +1,312 @@
+require "spec_helper.rb"
+
+# we use the posts controller as it uses the impressionsist module. any such controller would do.
+describe DummyController do
+  
+  before do
+      @impression_count = Impression.all.size
+    end
+  
+  describe "impressionist filter uniqueness" do  
+  
+    it "should ignore uniqueness if not requested" do
+      controller.impressionist_subapp_filter(nil, nil)
+      controller.impressionist_subapp_filter(nil, nil)
+      Impression.should have(@impression_count + 2).records
+    end
+    
+    it "should recognize unique session" do
+      # the following line was necessary as session hash returned a binary string (ASCII-8BIT encoded)
+      controller.stub!(:session_hash).and_return(request.session_options[:id].encode("ISO-8859-1"))
+      controller.impressionist_subapp_filter(nil, [:session_hash])
+      controller.impressionist_subapp_filter(nil, [:session_hash])
+      Impression.should have(@impression_count + 1).records
+    end
+    
+    it "should recognize unique ip" do
+      controller.request.stub!(:remote_ip).and_return("1.2.3.4")
+      controller.impressionist_subapp_filter(nil, [:ip_address])
+      controller.impressionist_subapp_filter(nil, [:ip_address])
+      Impression.should have(@impression_count + 1).records
+    end
+    
+    it "should recognize unique request" do
+      controller.impressionist_subapp_filter(nil, [:request_hash])
+      controller.impressionist_subapp_filter(nil, [:request_hash])
+      Impression.should have(@impression_count + 1).records
+    end
+    
+    it "should recognize unique action" do
+      controller.stub!(:action_name).and_return("test_action")
+      controller.impressionist_subapp_filter(nil, [:action_name])
+      controller.impressionist_subapp_filter(nil, [:action_name])
+      Impression.should have(@impression_count + 1).records
+    end
+    
+    it "should recognize unique controller" do
+      controller.stub!(:controller_name).and_return("test_controller")
+      controller.impressionist_subapp_filter(nil, [:controller_name])
+      controller.impressionist_subapp_filter(nil, [:controller_name])
+      Impression.should have(@impression_count + 1).records
+    end
+    
+     it "should recognize unique user" do
+      controller.stub!(:user_id).and_return(42)
+      controller.impressionist_subapp_filter(nil, [:user_id])
+      controller.impressionist_subapp_filter(nil, [:user_id])
+      Impression.should have(@impression_count + 1).records
+    end
+    
+    it "should recognize unique referer" do
+      controller.request.stub!(:referer).and_return("http://foo/bar")
+      controller.impressionist_subapp_filter(nil, [:referrer])
+      controller.impressionist_subapp_filter(nil, [:referrer])
+      Impression.should have(@impression_count + 1).records
+    end
+    
+    it "should recognize unique id" do
+      controller.stub!(:params).and_return({:id => "666"}) # for correct impressionable id in filter
+      controller.impressionist_subapp_filter(nil, [:impressionable_id])
+      controller.impressionist_subapp_filter(nil, [:impressionable_id])
+      Impression.should have(@impression_count + 1).records
+    end
+    
+    # extra redundant test for important controller and action combination.
+    it "should recognize different controller and action" do
+      controller.stub!(:controller_name).and_return("test_controller")
+      controller.stub!(:action_name).and_return("test_action")
+      controller.impressionist_subapp_filter(nil, [:controller_name, :action_name])
+      controller.impressionist_subapp_filter(nil, [:controller_name, :action_name])
+      Impression.should have(@impression_count + 1).records
+      controller.stub!(:action_name).and_return("another_action")
+      controller.impressionist_subapp_filter(nil, [:controller_name, :action_name])
+      controller.impressionist_subapp_filter(nil, [:controller_name, :action_name])
+      Impression.should have(@impression_count + 2).records
+      controller.stub!(:controller_name).and_return("another_controller")
+      controller.impressionist_subapp_filter(nil, [:controller_name, :action_name])
+      controller.impressionist_subapp_filter(nil, [:controller_name, :action_name])
+      Impression.should have(@impression_count + 3).records
+    end
+    
+    it "should recognize different action" do
+      controller.stub!(:action_name).and_return("test_action")
+      controller.impressionist_subapp_filter(nil, [:action_name])
+      controller.impressionist_subapp_filter(nil, [:action_name])
+      Impression.should have(@impression_count + 1).records
+      controller.stub!(:action_name).and_return("another_action")
+      controller.impressionist_subapp_filter(nil, [:action_name])
+      controller.impressionist_subapp_filter(nil, [:action_name])
+      Impression.should have(@impression_count + 2).records
+    end
+    
+    it "should recognize different controller" do
+      controller.stub!(:controller_name).and_return("test_controller")
+      controller.impressionist_subapp_filter(nil, [:controller_name])
+      controller.impressionist_subapp_filter(nil, [:controller_name])
+      Impression.should have(@impression_count + 1).records
+      controller.stub!(:controller_name).and_return("another_controller")
+      controller.impressionist_subapp_filter(nil, [:controller_name])
+      controller.impressionist_subapp_filter(nil, [:controller_name])
+      Impression.should have(@impression_count + 2).records
+    end
+    
+    it "should recognize different session" do
+      controller.stub!(:session_hash).and_return("foo")
+      controller.impressionist_subapp_filter(nil, [:session_hash])
+      controller.impressionist_subapp_filter(nil, [:session_hash])
+      Impression.should have(@impression_count + 1).records
+      controller.stub!(:session_hash).and_return("bar")
+      controller.impressionist_subapp_filter(nil, [:session_hash])
+      controller.impressionist_subapp_filter(nil, [:session_hash])
+      Impression.should have(@impression_count + 2).records
+    end
+    
+    it "should recognize different ip" do
+      controller.request.stub!(:remote_ip).and_return("1.2.3.4")
+      controller.impressionist_subapp_filter(nil, [:ip_address])
+      controller.impressionist_subapp_filter(nil, [:ip_address])
+      Impression.should have(@impression_count + 1).records
+      controller.request.stub!(:remote_ip).and_return("5.6.7.8")
+      controller.impressionist_subapp_filter(nil, [:ip_address])
+      controller.impressionist_subapp_filter(nil, [:ip_address])
+      Impression.should have(@impression_count + 2).records
+    end
+    
+    it "should recognize different referer" do
+      controller.request.stub!(:referer).and_return("http://foo/bar")
+      controller.impressionist_subapp_filter(nil, [:referrer])
+      controller.impressionist_subapp_filter(nil, [:referrer])
+      Impression.should have(@impression_count + 1).records
+      controller.request.stub!(:referer).and_return("http://bar/fo")
+      controller.impressionist_subapp_filter(nil, [:referrer])
+      controller.impressionist_subapp_filter(nil, [:referrer])
+      Impression.should have(@impression_count + 2).records
+    end
+    
+    it "should recognize different id" do
+      controller.stub!(:params).and_return({:id => "666"}) # for correct impressionable id in filter
+      controller.impressionist_subapp_filter(nil, [:impressionable_type, :impressionable_id])
+      controller.impressionist_subapp_filter(nil, [:impressionable_type, :impressionable_id])
+      controller.stub!(:params).and_return({:id => "42"}) # for correct impressionable id in filter
+      controller.impressionist_subapp_filter(nil, [:impressionable_type, :impressionable_id])
+      controller.impressionist_subapp_filter(nil, [:impressionable_type, :impressionable_id])
+      Impression.should have(@impression_count + 2).records
+    end
+    
+    it "should recognize combined uniqueness" do
+      controller.stub!(:action_name).and_return("test_action")
+      controller.impressionist_subapp_filter(nil, [:ip_address, :request_hash, :action_name])
+      controller.impressionist_subapp_filter(nil, [:request_hash, :ip_address, :action_name])
+      controller.impressionist_subapp_filter(nil, [:request_hash, :action_name])
+      controller.impressionist_subapp_filter(nil, [:ip_address, :action_name])
+      controller.impressionist_subapp_filter(nil, [:ip_address, :request_hash])
+      controller.impressionist_subapp_filter(nil, [:action_name])
+      controller.impressionist_subapp_filter(nil, [:ip_address])
+      controller.impressionist_subapp_filter(nil, [:request_hash])
+      Impression.should have(@impression_count + 1).records
+    end
+    
+    it "should recognize combined non-uniqueness" do
+      controller.stub!(:action_name).and_return(nil)
+      controller.impressionist_subapp_filter(nil, [:ip_address, :action_name])
+      controller.stub!(:action_name).and_return("test_action")
+      controller.impressionist_subapp_filter(nil, [:ip_address, :action_name])
+      controller.stub!(:action_name).and_return("another_action")
+      controller.impressionist_subapp_filter(nil, [:ip_address, :action_name])
+      Impression.should have(@impression_count + 3).records
+    end     
+    
+  end
+  
+  describe "impressionist method uniqueness for impressionables" do  
+    
+    # in this test we reuse the post model. might break if model changes.
+    
+    it "should ignore uniqueness if not requested" do
+      impressionable = Post.create
+      controller.impressionist impressionable
+      controller.impressionist impressionable
+      Impression.should have(@impression_count + 2).records
+    end
+    
+    it "should recognize unique session" do
+      # the following line was necessary as session hash returned a binary string (ASCII-8BIT encoded)
+      controller.stub!(:session_hash).and_return(request.session_options[:id].encode("ISO-8859-1"))
+      impressionable = Post.create
+      controller.impressionist(impressionable, nil, :unique => [:session_hash])
+      controller.impressionist(impressionable, nil, :unique => [:session_hash])
+      Impression.should have(@impression_count + 1).records
+    end
+    
+    it "should recognize unique ip" do
+      controller.request.stub!(:remote_ip).and_return("1.2.3.4")
+      impressionable = Post.create
+      controller.impressionist(impressionable, nil, :unique => [:ip_address])
+      controller.impressionist(impressionable, nil, :unique => [:ip_address])
+      Impression.should have(@impression_count + 1).records
+    end
+    
+    it "should recognize unique request" do
+      impressionable = Post.create
+      controller.impressionist(impressionable, nil, :unique => [:request_hash])
+      controller.impressionist(impressionable, nil, :unique => [:request_hash])
+      Impression.should have(@impression_count + 1).records
+    end
+    
+     it "should recognize unique user" do
+      controller.stub!(:user_id).and_return(666)
+      impressionable = Post.create
+      controller.impressionist(impressionable, nil, :unique => [:user_id])
+      controller.impressionist(impressionable, nil, :unique => [:user_id])
+      Impression.should have(@impression_count + 1).records
+    end
+    
+    it "should recognize unique referer" do
+      controller.request.stub!(:referer).and_return("http://foo/bar")
+      impressionable = Post.create
+      controller.impressionist(impressionable, nil, :unique => [:referrer])
+      controller.impressionist(impressionable, nil, :unique => [:referrer])
+      Impression.should have(@impression_count + 1).records
+    end
+    
+    it "should recognize different session" do
+      impressionable = Post.create
+      controller.stub!(:session_hash).and_return("foo")
+      controller.impressionist(impressionable, nil, :unique => [:session_hash])
+      controller.impressionist(impressionable, nil, :unique => [:session_hash])
+      Impression.should have(@impression_count + 1).records
+      controller.stub!(:session_hash).and_return("bar")
+      controller.impressionist(impressionable, nil, :unique => [:session_hash])
+      controller.impressionist(impressionable, nil, :unique => [:session_hash])
+      Impression.should have(@impression_count + 2).records
+    end
+    
+    it "should recognize different ip" do
+      controller.request.stub!(:remote_ip).and_return("1.2.3.4")
+      impressionable = Post.create
+      controller.impressionist(impressionable, nil, :unique => [:ip_address])
+      controller.impressionist(impressionable, nil, :unique => [:ip_address])
+      Impression.should have(@impression_count + 1).records
+      controller.request.stub!(:remote_ip).and_return("5.6.7.8")
+      controller.impressionist(impressionable, nil, :unique => [:ip_address])
+      controller.impressionist(impressionable, nil, :unique => [:ip_address])
+      Impression.should have(@impression_count + 2).records
+    end
+    
+    it "should recognize different user" do
+      impressionable = Post.create
+      controller.stub!(:user_id).and_return(666)
+      controller.impressionist(impressionable, nil, :unique => [:user_id])
+      controller.impressionist(impressionable, nil, :unique => [:user_id])
+      Impression.should have(@impression_count + 1).records
+      controller.stub!(:user_id).and_return(42)
+      controller.impressionist(impressionable, nil, :unique => [:user_id])
+      controller.impressionist(impressionable, nil, :unique => [:user_id])
+      Impression.should have(@impression_count + 2).records
+    end
+    
+    it "should recognize combined uniqueness" do
+      impressionable = Post.create
+      controller.stub!(:session_hash).and_return("foo")
+      controller.impressionist(impressionable, nil, :unique => [:ip_address, :request_hash, :session_hash])
+      controller.impressionist(impressionable, nil, :unique => [:request_hash, :ip_address, :session_hash])
+      controller.impressionist(impressionable, nil, :unique => [:request_hash, :session_hash])
+      controller.impressionist(impressionable, nil, :unique => [:ip_address, :session_hash])
+      controller.impressionist(impressionable, nil, :unique => [:ip_address, :request_hash])
+      controller.impressionist(impressionable, nil, :unique => [:session_hash])
+      controller.impressionist(impressionable, nil, :unique => [:ip_address])
+      controller.impressionist(impressionable, nil, :unique => [:request_hash])
+      Impression.should have(@impression_count + 1).records
+    end
+    
+    it "should recognize combined non-uniqueness" do
+      impressionable = Post.create
+      controller.stub!(:session_hash).and_return(nil)
+      controller.impressionist(impressionable, nil, :unique => [:ip_address, :session_hash])
+      controller.stub!(:session_hash).and_return("foo")
+      controller.impressionist(impressionable, nil, :unique => [:ip_address, :session_hash])
+      controller.stub!(:session_hash).and_return("bar")
+      controller.impressionist(impressionable, nil, :unique => [:ip_address, :session_hash])
+      Impression.should have(@impression_count + 3).records
+    end
+    
+  end
+  
+  describe "impressionist filter and method uniqueness" do
+  
+    it "should recognize uniqueness" do
+      impressionable = Post.create
+      controller.stub!(:controller_name).and_return("posts") # for correct impressionable type in filter
+      controller.stub!(:params).and_return({:id => impressionable.id.to_s}) # for correct impressionable id in filter
+      controller.stub!(:session_hash).and_return("foo")
+      controller.request.stub!(:remote_ip).and_return("1.2.3.4")
+      # order of the following methods is important for the test!
+      controller.impressionist_subapp_filter(nil, [:ip_address, :request_hash, :session_hash])
+      controller.impressionist(impressionable, nil, :unique => [:ip_address, :request_hash, :session_hash])
+      Impression.should have(@impression_count + 1).records
+    end
+    
+  end
+  
+end
+

test_app/spec/controllers/unique_impression_controller_spec.rb

@@ -1,133 +0,0 @@
-require 'spec_helper.rb'
-
-# we use the posts controller as it uses the impressionsist module. any such controller would do.
-describe PostsController do
-    
-  before do
-    @impression_count = Impression.all.size
-  end
-  
-  it "should ignore uniqueness if not requested" do
-    controller.impressionist_subapp_filter(nil, nil)
-    controller.impressionist_subapp_filter(nil, nil)
-    Impression.should have(@impression_count + 2).records
-  end
-  
-  it "should recognize session uniqueness" do
-    # the following line was necessary as session hash returned a binary string (ASCII-8BIT encoded)
-    # not sure how to 'fix' this. setup/config issue?
-    controller.stub!(:session_hash).and_return(request.session_options[:id].encode("ISO-8859-1"))
-    controller.impressionist_subapp_filter(nil, [:session_hash])
-    controller.impressionist_subapp_filter(nil, [:session_hash])
-    Impression.should have(@impression_count + 1).records
-  end
-  
-  it "should recognize ip uniqueness" do
-    controller.stub!(:action_name).and_return("test_action")
-    controller.impressionist_subapp_filter(nil, [:ip_address])
-    controller.impressionist_subapp_filter(nil, [:ip_address])
-    Impression.should have(@impression_count + 1).records
-  end
-  
-  it "should recognize request uniqueness" do
-    controller.impressionist_subapp_filter(nil, [:request_hash])
-    controller.impressionist_subapp_filter(nil, [:request_hash])
-    Impression.should have(@impression_count + 1).records
-  end
-  
-  it "should recognize action uniqueness" do
-    controller.stub!(:action_name).and_return("test_action")
-    controller.impressionist_subapp_filter(nil, [:action_name])
-    controller.impressionist_subapp_filter(nil, [:action_name])
-    Impression.should have(@impression_count + 1).records
-  end
-  
-  it "should recognize controller uniqueness" do
-    controller.stub!(:controller_name).and_return("test_controller")
-    controller.impressionist_subapp_filter(nil, [:controller_name])
-    controller.impressionist_subapp_filter(nil, [:controller_name])
-    Impression.should have(@impression_count + 1).records
-  end
-  
-   it "should recognize user uniqueness" do
-    controller.stub!(:user_id).and_return(1)
-    controller.impressionist_subapp_filter(nil, [:user_id])
-    controller.impressionist_subapp_filter(nil, [:user_id])
-    Impression.should have(@impression_count + 1).records
-  end
-  
-  it "should recognize referrer uniqueness" do
-    @request.env['HTTP_REFERER'] = 'http://somehost.someurl.somdomain/some/path'
-    #controller.stub!(:referer).and_return("http://somehost.someurl.somdomain/some/path")
-    controller.impressionist_subapp_filter(nil, [:referrer])
-    controller.impressionist_subapp_filter(nil, [:referrer])
-    Impression.should have(@impression_count + 1).records
-  end
-  
-  # extra redundant test for important controller and action combination.
-  it "should recognize difference in controller and action" do
-    controller.stub!(:controller_name).and_return("test_controller")
-    controller.stub!(:action_name).and_return("test_action")
-    controller.impressionist_subapp_filter(nil, [:controller_name, :action_name])
-    Impression.should have(@impression_count + 1).records
-    controller.stub!(:action_name).and_return("another_action")
-    controller.impressionist_subapp_filter(nil, [:controller_name, :action_name])
-    Impression.should have(@impression_count + 2).records
-    controller.stub!(:controller_name).and_return("another_controller")
-    controller.impressionist_subapp_filter(nil, [:controller_name, :action_name])
-    Impression.should have(@impression_count + 3).records
-  end
-  
-  it "should recognize difference in action" do
-    controller.stub!(:action_name).and_return("test_action")
-    controller.impressionist_subapp_filter(nil, [:action_name])
-    Impression.should have(@impression_count + 1).records
-    controller.stub!(:action_name).and_return("another_action")
-    controller.impressionist_subapp_filter(nil, [:action_name])
-    Impression.should have(@impression_count + 2).records
-  end
-  
-  it "should recognize difference in controller" do
-    controller.stub!(:controller_name).and_return("test_controller")
-    controller.impressionist_subapp_filter(nil, [:controller_name])
-    Impression.should have(@impression_count + 1).records
-    controller.stub!(:controller_name).and_return("another_controller")
-    controller.impressionist_subapp_filter(nil, [:controller_name])
-    Impression.should have(@impression_count + 2).records
-  end
-  
-  it "should recognize difference in session" do
-    controller.stub!(:session_hash).and_return(request.session_options[:id].encode("ISO-8859-1"))
-    controller.impressionist_subapp_filter(nil, [:session_hash])
-    Impression.should have(@impression_count + 1).records
-    controller.stub!(:session_hash).and_return("anothersessionhash")
-    controller.impressionist_subapp_filter(nil, [:session_hash])
-    Impression.should have(@impression_count + 2).records
-  end
-  
-  it "should recognize combined uniqueness" do
-    controller.stub!(:action_name).and_return("test_action")
-    controller.impressionist_subapp_filter(nil, [:ip_address, :request_hash, :action_name])
-    controller.impressionist_subapp_filter(nil, [:request_hash, :ip_address, :action_name])
-    controller.impressionist_subapp_filter(nil, [:request_hash, :action_name])
-    controller.impressionist_subapp_filter(nil, [:ip_address, :action_name])
-    controller.impressionist_subapp_filter(nil, [:ip_address, :request_hash])
-    controller.impressionist_subapp_filter(nil, [:action_name])
-    controller.impressionist_subapp_filter(nil, [:ip_address])
-    controller.impressionist_subapp_filter(nil, [:request_hash])
-    Impression.should have(@impression_count + 1).records
-  end
-  
-  it "should recognize combined non-uniqueness" do
-    controller.stub!(:action_name).and_return(nil)
-    controller.impressionist_subapp_filter(nil, [:ip_address, :action_name])
-    controller.stub!(:action_name).and_return("test_action")
-    controller.impressionist_subapp_filter(nil, [:ip_address, :action_name])
-    controller.stub!(:action_name).and_return("another_action")
-    controller.impressionist_subapp_filter(nil, [:ip_address, :action_name])
-    Impression.should have(@impression_count + 3).records
-  end
-    
-end
-
-