Merge pull request #4 from gewo/extract_session_data

Extract session data

.rubocop.yml

@@ -0,0 +1,14 @@
+Documentation:
+  Enabled: false
+AlignArray:
+  Enabled: false
+AlignHash:
+  Enabled: false
+AlignParameters:
+  Enabled: false
+ModuleFunction:
+  Enabled: false
+AllCops:
+  Excludes:
+    - spec/dummy/**
+    - script/**

Gemfile.lock

@@ -1,7 +1,7 @@
 PATH
   remote: .
   specs:
-    re_track (0.5.0)
+    re_track (0.5.1)
       activesupport
       mongoid (>= 3)
       rails
@@ -40,6 +40,7 @@ GEM
       bundler
       rake
     arel (3.0.3)
+    ast (1.1.0)
     builder (3.0.4)
     coderay (1.1.0)
     coveralls (0.7.0)
@@ -72,7 +73,11 @@ GEM
     moped (1.5.1)
     multi_json (1.8.2)
     origin (1.1.0)
+    parser (2.0.0)
+      ast (~> 1.1)
+      slop (~> 3.4, >= 3.4.5)
     polyglot (0.3.3)
+    powerpack (0.0.9)
     pry (0.9.12.3)
       coderay (~> 1.0)
       method_source (~> 0.8)
@@ -99,6 +104,7 @@ GEM
       rake (>= 0.8.7)
       rdoc (~> 3.4)
       thor (>= 0.14.6, < 2.0)
+    rainbow (1.1.4)
     rake (10.1.0)
     rdoc (3.12.2)
       json (~> 1.4)
@@ -115,6 +121,10 @@ GEM
       rspec-core (~> 2.14.0)
       rspec-expectations (~> 2.14.0)
       rspec-mocks (~> 2.14.0)
+    rubocop (0.15.0)
+      parser (~> 2.0)
+      powerpack (~> 0.0.6)
+      rainbow (>= 1.1.4)
     simplecov (0.8.2)
       docile (~> 1.1.0)
       multi_json
@@ -147,4 +157,5 @@ DEPENDENCIES
   pry
   re_track!
   rspec-rails
+  rubocop
   simplecov

app/models/re_track/referer_tracking.rb

@@ -16,7 +16,7 @@ class RefererTracking
     field :accept_language,  type: String
 
     [:referer_url, :first_url, :user_agent, :first_visited_at, :ip,
-     :forwarded_ip, :accept_language].each { |field| index({ field => 1 }) }
+     :forwarded_ip, :accept_language].each { |field| index(field => 1) }
 
     # Extract query parameters from referer_url and first_url.
     #
@@ -32,18 +32,19 @@ class RefererTracking
     #
     # @return [String] The value for the given query parameter or nil.
     def query(parameter, url_field_name = 'first_url')
-      return nil unless url = value_for(url_field_name)
+      url = value_for(url_field_name)
+      return unless url
       query_hash(url)[parameter.to_s]
     end
 
     private
 
       def query_hash(url)
-        Rack::Utils.parse_query URI.parse(CGI::unescape(url)).query rescue {}
+        Rack::Utils.parse_query URI.parse(CGI.unescape(url)).query rescue {}
       end
 
       def value_for(field)
-        return nil unless ['referer_url', 'first_url'].include? field.to_s
+        return unless %w(referer_url first_url).include? field.to_s
         public_send("#{field}")
       end
   end

lib/re_track.rb

@@ -1,4 +1,5 @@
 require 're_track/engine'
+require 're_track/session_data'
 require 're_track/tracker'
 require 're_track/sweeper'
 require 're_track/trackable'

lib/re_track/session_data.rb

@@ -0,0 +1,45 @@
+module ReTrack
+  module SessionData
+    ATTRIBUTES = [
+      :referer_url, :first_url, :user_agent, :first_visited_at, :ip,
+      :accept_language, :forwarded_ip
+    ]
+
+    def to_hash(request)
+      @request = request
+      Hash[ATTRIBUTES.map { |k, v| [k, send(k)] }]
+    end
+
+    private
+
+      def referer_url
+        @request.headers['HTTP_REFERER'].presence || 'none'
+      end
+
+      def first_url
+        @request.url
+      end
+
+      def user_agent
+        @request.env['HTTP_USER_AGENT']
+      end
+
+      def first_visited_at
+        Time.now
+      end
+
+      def ip
+        @request.remote_ip
+      end
+
+      def accept_language
+        @request.env['HTTP_ACCEPT_LANGUAGE']
+      end
+
+      def forwarded_ip
+        @request.env['HTTP_X_FORWARDED_FOR'] || @request.env['HTTP_CLIENT_IP']
+      end
+
+    extend self
+  end
+end

lib/re_track/sweeper.rb

@@ -44,9 +44,7 @@ def rt_create_referer_tracking!(record)
 
       def rt_after_create(record)
         return unless record.persisted?
-        if session && session[:retrack]
-          rt_create_referer_tracking! record
-        end
+        rt_create_referer_tracking!(record) if session && session[:retrack]
       rescue => e
         Rails.logger.info(
           "ReTrack::Sweeper.after_create error saving record: #{e}")

lib/re_track/tracker.rb

@@ -8,19 +8,9 @@ module Tracker
 
     private
 
-      TRACK = {
-        referer_url: ->(request) { request.headers['HTTP_REFERER'].presence || 'none' },
-        first_url: ->(request) { request.url },
-        user_agent: ->(request) { request.env['HTTP_USER_AGENT'] },
-        first_visited_at: ->(request) { Time.now },
-        ip: ->(request) { request.remote_ip },
-        accept_language: ->(request) { request.env['HTTP_ACCEPT_LANGUAGE'] },
-        forwarded_ip: ->(request) { request.env['HTTP_X_FORWARDED_FOR'] || request.env['HTTP_CLIENT_IP'] }
-      }
-
       def rt_track_referer
         session[:retrack].nil? && !request_from_a_known_bot? &&
-          session[:retrack] = Hash[TRACK.map { |k, v| [k, v.call(request)] }]
+          session[:retrack] = SessionData.to_hash(request)
       end
 
       def request_from_a_known_bot?

lib/re_track/version.rb

@@ -1,3 +1,3 @@
 module ReTrack
-  VERSION = '0.5.0'
+  VERSION = '0.5.1'
 end

re_track.gemspec

@@ -26,4 +26,5 @@ Gem::Specification.new do |s|
   s.add_development_dependency 'database_cleaner'
   s.add_development_dependency 'pry'
   s.add_development_dependency 'appraisal'
+  s.add_development_dependency 'rubocop'
 end

spec/controllers/referer_tracking_spec.rb

@@ -1,86 +1,93 @@
 require 'spec_helper'
 
-describe UsersController, "RefererTracking" do
+describe UsersController, 'RefererTracking' do
   before :each do
-    @request.env['HTTP_REFERER'] = (@referer = "awesome.pa.ge/believe/me")
-    @request.env['HTTP_USER_AGENT'] = @ua = "Fancy new UA"
+    @request.env['HTTP_REFERER'] = @referer = 'awesome.pa.ge/believe/me'
+    @request.env['HTTP_USER_AGENT'] = @ua = 'Fancy new UA'
     @request.env['REMOTE_ADDR'] = @ip = '102.97.107.101'
+    @request.env['HTTP_ACCEPT_LANGUAGE'] = @accept_language = 'eo'
+    @request.env['HTTP_X_FORWARDED_FOR'] = @forwarded_ip = '10.0.23.42'
     get :new
   end
 
-  it "saves referer_url to session" do
+  it 'saves referer_url to session' do
     session[:retrack].should_not be_blank
     session[:retrack][:referer_url].should == @referer
   end
 
-  it "saves first_url to session" do
+  it 'saves first_url to session' do
     session[:retrack].should_not be_blank
-    session[:retrack][:first_url].should =~ %r(users/new$)
+    session[:retrack][:first_url].should =~ /users\/new$/
   end
 
-  context "on the second request" do
+  context 'on the second request' do
     before :each do
-      @request.env['HTTP_REFERER'] = ("this.is.the.new/referer")
+      @request.env['HTTP_REFERER'] = 'this.is.the.new/referer'
     end
 
-    it "doesn't update referer_url" do
+    it 'does not update referer_url' do
       get :index
       session[:retrack][:referer_url].should == @referer
     end
 
-    it "doesn't update first_url" do
+    it 'does not update first_url' do
       get :index
-      session[:retrack][:first_url].should =~ %r(users/new$)
+      session[:retrack][:first_url].should =~ /users\/new$/
     end
   end
 
-  describe "POST 'create'" do
-    it "saves a user" do
-      expect {
-        post :create
-      }.to change(User.all, :count).by(1)
+  describe 'POST create' do
+    subject { -> { post :create } }
+    it 'saves a user' do
+      should change(User.all, :count).by(1)
     end
 
-    it "saves a RefererTracking" do
-      expect {
-        post :create
-      }.to change(ReTrack::RefererTracking.all, :count).by(1)
+    it 'saves a RefererTracking' do
+      should change(ReTrack::RefererTracking.all, :count).by(1)
     end
 
-    context "when saving the RefererTracking" do
+    context 'when saving the RefererTracking' do
       before :each do
         post :create
         @rt = ReTrack::RefererTracking.last
       end
 
-      it "saves the referer_url in RefererTracking" do
+      it 'saves the referer_url in RefererTracking' do
         @rt.referer_url.should == @referer
       end
 
-      it "saves the first_url in RefererTracking" do
-        @rt.first_url.should =~ %r(users/new$)
+      it 'saves the first_url in RefererTracking' do
+        @rt.first_url.should =~ /users\/new$/
       end
 
-      it "saves the user_agent in RefererTracking" do
+      it 'saves the user_agent in RefererTracking' do
         @rt.user_agent.should == @ua
       end
 
-      it "saves the first_visited_at in RefererTracking" do
+      it 'saves the first_visited_at in RefererTracking' do
         # TODO: use Timecop
         @rt.first_visited_at.should be_within(5.seconds).of(DateTime.now)
       end
 
-      it "saves the correct user in RefererTracking" do
+      it 'saves the correct user in RefererTracking' do
         user = User.last
         @rt.trackable.should == user
       end
 
       it 'saves the ip in RefererTracking' do
         @rt.ip.should == @ip
       end
+
+      it 'saves the accept_language in RefererTracking' do
+        expect(@rt.accept_language).to eq @accept_language
+      end
+
+      it 'saves forwarded_ip in RefererTracking' do
+        expect(@rt.forwarded_ip).to eq @forwarded_ip
+      end
     end
 
-    it "completes the response when there are errors in the sweeper" do
+    it 'completes the response when there are errors in the sweeper' do
       ReTrack::RefererTracking.any_instance.stub(:save).and_raise
       post :create
       response.should be_a_redirect # not a 500

spec/models/re_track/referer_tracking_spec.rb

@@ -27,7 +27,7 @@ module ReTrack
 
       context 'when first_url is url_encoded' do
         before do
-          referer_tracking.first_url = CGI::escape 'http://google.de/?q=query'
+          referer_tracking.first_url = CGI.escape 'http://google.de/?q=query'
         end
         it { should == 'query' }
       end