added draft for impressionist(...) uniqueness

app/controllers/impressionist_controller.rb

@@ -12,11 +12,14 @@ module ImpressionistController
       base.before_filter :impressionist_app_filter
     end
 
-    def impressionist(obj,message=nil)
+    def impressionist(obj,message=nil,opts={})
       unless bypass
         if obj.respond_to?("impressionable?")
-          obj.impressions.create(create_statement({:message => message}))
+          if unique_instance?(obj, opts[:unique])
+            obj.impressions.create(associative_create_statement({:message => message}))
+          end
         else
+          # we could create an impression anyway. for classes, too. why not?
           raise "#{obj.class.to_s} is not impressionable!"
         end
       end
@@ -26,17 +29,11 @@ module ImpressionistController
       @impressionist_hash = Digest::SHA2.hexdigest(Time.now.to_f.to_s+rand(10000).to_s)
     end
 
-    def impressionist_subapp_filter(actions=nil, unique_opts=nil)
+    def impressionist_subapp_filter(actions=nil,unique_opts=nil)
       unless bypass
         actions.collect!{|a|a.to_s} unless actions.blank?
-        if (actions.blank? || actions.include?(action_name)) && (unique_opts.blank? || is_unique(unique_opts))
+        if (actions.blank? || actions.include?(action_name)) && unique?(unique_opts)
-          if (!actions.blank? && !unique_opts.blank?)
+          Impression.create(direct_create_statement)
-            logger.info "Restricted to actions #{actions.inspect} and uniqueness for #{unique_opts.inspect}"
-          end
-          Impression.create(create_statement(
-            :impressionable_type => controller_name.singularize.camelize,
-            :impressionable_id=> params[:id]
-          ))
         end
       end
     end
@@ -50,52 +47,55 @@ module ImpressionistController
       Impressionist::Bots::LIST.include? request.user_agent
     end
     
-    def is_unique(unique_opts)
+    def unique_instance?(impressionable, unique_opts)
-      # FIXME think about uniqueness in relation to impressionable_id, impressionable_type and controller_name
+      return unique_opts.blank? || impressionable.impressions.where(unique_query(unique_ops)).size == 0
-      # is controller name redundant? does the controller name always have to match?
-      default_statement = create_statement(
-        :impressionable_type => controller_name.singularize.camelize,
-        :impressionable_id=> params[:id]
-        )
-      statement = unique_opts.reduce({}) do |query, param|
-        query[param] = default_statement[param]
-        query
-      end
-      #logger.debug "Statement params: #{statement.inspect}."
-      # always use impressionable type?
-      statement[:impressionable_type] = controller_name.singularize.camelize
-      #statement[:impressionable_id] = params[:id]
-      return Impression.where(statement).size == 0
     end
     
-    # creates a statment hash that contains default values for creating an impression (without
+    def unique?(unique_opts)
-    # :impressionable_type and impressionable_id as they are not needed for creating via association).
+      return unique_opts.blank? || Impression.where(unique_query(unique_opts)).size == 0
-    def create_statement(query_params={})
+    end
+    
+    # creates the query to check for uniqueness
+    def unique_query(unique_opts)
+      full_statement = direct_create_statement
+      # reduce the full statement to the params we need for the specified unique options
+      unique_opts.reduce({}) do |query, param|
+        query[param] = full_statement[param]
+        query
+      end
+    end
+    
+    # creates a statment hash that contains default values for creating an impression via an AR relation.
+    def associative_create_statement(query_params={})
       query_params.reverse_merge!(
         :controller_name => controller_name,
         :action_name => action_name,
         :user_id => user_id,
         :request_hash => @impressionist_hash,
         :session_hash => session_hash,
-        :ip_address => remote_ip,
+        :ip_address => request.remote_ip,
         :referrer => request.referer
-      )
+        )
+    end
+    
+    # creates a statment hash that contains default values for creating an impression.
+    def direct_create_statement(query_params={})
+      query_params.reverse_merge!(
+        :impressionable_type => controller_name.singularize.camelize,
+        :impressionable_id=> params[:id]
+        )
+      associative_create_statement(query_params)
     end
     
     def session_hash
       # # careful: request.session_options[:id] encoding in rspec test was ASCII-8BIT
-      # # that broke the database query for uniqueness. not sure how to solve this issue
+      # # that broke the database query for uniqueness. not sure if this is a testing only issue.
-      # # seems to depend on app setup/config
       # str = request.session_options[:id]
-      # # probably this isn't a fix: request.session_options[:id].encode("ISO-8859-1")
       # logger.debug "Encoding: #{str.encoding.inspect}"
+      # # request.session_options[:id].encode("ISO-8859-1")
       request.session_options[:id]
     end
     
-    def remote_ip
-      request.remote_ip
-    end
-    
     #use both @current_user and current_user helper
     def user_id
       user_id = @current_user ? @current_user.id : nil rescue nil

test_app/spec/controllers/unique_impression_controller_spec.rb

@@ -57,7 +57,8 @@ describe PostsController do
   end
   
   it "should recognize referrer uniqueness" do
-    controller.stub!(:referrer).and_return("http://somehost.someurl.somdomain/some/path")
+    @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