introduce ControllerDataExtractor

lib/exceptional/controller_exception_data.rb

@@ -1,33 +1,64 @@
 require 'digest/md5'
 
 module Exceptional
-  class ControllerExceptionData < ExceptionData
-    def initialize(exception, controller=nil, request=nil)
-      super(exception)
+  class ControllerDataExtractor
+    def initialize(controller, request)
       @request = request
       @controller = controller
     end
 
-    def framework
-      "rails"
+    def controller
+      "#{@controller.class}"
     end
 
-    def extra_stuff                                                                                               
-      return {} if @request.nil?
-      {
-        'request' => {
-          'url' => (@request.respond_to?(:url) ? @request.url : "#{@request.protocol}#{@request.host}#{@request.request_uri}"),
-          'controller' => @controller.class.to_s,
-          'action' => (@request.respond_to?(:parameters) ? @request.parameters['action'] : @request.params['action']),
-          'parameters' => filter_parameters(@request.respond_to?(:parameters) ? @request.parameters : @request.params),
-          'request_method' => @request.request_method.to_s,
-          'remote_ip' => (@request.respond_to?(:remote_ip) ? @request.remote_ip : @request.ip),
-          'headers' => extract_http_headers(@request.env),
-          'session' => self.class.sanitize_session(@request)
-        }
-      }
+    def url
+      if @request.respond_to?(:url) 
+        @request.url 
+      else
+        "#{@request.protocol}#{@request.host}#{@request.request_uri}"
+      end
+    end
+
+    def action
+      if @request.respond_to?(:parameters)
+        @request.parameters['action'] 
+      else
+        @request.params['action']
+      end
+    end
+
+    def parameters
+      parameters = if @request.respond_to?(:parameters)
+                     @request.parameters
+                   else
+                     @request.params
+                   end
+
+      filter_parameters(parameters)
+    end
+
+    def request_method
+      "#{@request.request_method}"
+    end
+
+    def remote_ip
+      if @request.respond_to?(:remote_ip) 
+        @request.remote_ip 
+      else 
+        @request.ip
+      end
+    end
+
+    def env
+      @request.env
     end
 
+    def request
+      @request
+    end
+
+    private
+
     def filter_hash(keys_to_filter, hash)
       keys_to_filter.map! {|x| x.to_s}
       if keys_to_filter.is_a?(Array) && !keys_to_filter.empty?
@@ -62,4 +93,32 @@ def filter_parameters(hash)
       end
     end
   end
+
+  class ControllerExceptionData < ExceptionData
+    def initialize(exception, controller=nil, request=nil)
+      super(exception)
+      @data = ControllerDataExtractor.new(controller, request) unless request.nil?
+    end
+
+    def framework
+      "rails"
+    end
+
+    def extra_stuff
+      return {} if @data.nil?
+      {
+        'request' =>
+        {
+            'url' => @data.url,
+            'controller' => @data.controller,
+            'action' => @data.action,
+            'parameters' => @data.parameters,
+            'request_method' => @data.request_method,
+            'remote_ip' => @data.remote_ip,
+            'headers' => extract_http_headers(@data.env),
+            'session' => self.class.sanitize_session(@data.request)
+        }
+      }
+    end
+  end
 end

spec/integrations/controller_exception_data_spec.rb

@@ -24,3 +24,101 @@ def to_json
     JSON.parse(data.to_json)['request']['session']['data'].should == {"boom" => brokenJson.to_s}
   end
 end
+
+describe Exceptional::ControllerDataExtractor do
+    before do
+      @request = mock(
+        :protocol    => "http://",
+        :host        => "mordor",
+        :request_uri => "/walk_in",
+        :params => 
+          {
+            "action" => "walk",
+            "controller" => "ultimate",
+            "foo"        => "bar"
+          },
+        :request_method => "GET",
+        :ip             => "1.2.3.4",
+        :env            => "fuzzy"
+        )
+        @controller = "Controller"
+    end
+
+    subject { Exceptional::ControllerDataExtractor.new(@controller, @request) }
+
+    it "should extract the controller class" do
+      subject.controller.should == "String"
+    end
+
+    it "should extract the URL" do
+      subject.url.should == "http://mordor/walk_in"
+    end
+
+    it "should extract action" do
+      subject.action.should == "walk"
+    end
+
+    it "should extract parameters" do
+      subject.parameters.should == {
+        "action" => "walk",
+        "controller" => "ultimate",
+        "foo" => "bar"
+      }
+    end
+
+    it "should extract request method" do
+      subject.request_method.should == "GET"
+    end
+
+    it "should extract remote ip" do
+      subject.remote_ip.should == "1.2.3.4"
+    end
+
+    it "should extract env" do
+      subject.env.should == "fuzzy"
+    end
+
+    it "should make request available" do
+      subject.request.should == @request
+    end
+
+    context "with old request" do
+      before do
+        @request = mock(
+          :url    => "http://mordor/walk_in",
+          :parameters => 
+          {
+            "action" => "walk",
+            "controller" => "ultimate",
+            "foo"        => "bar"
+          },
+            :request_method => "GET",
+            :remote_ip      => "1.2.3.4",
+            :env            => "fuzzy"
+        )
+        @controller = "Controller"
+      end
+
+      subject { Exceptional::ControllerDataExtractor.new(@controller, @request) }
+
+      it "should extract the URL" do
+        subject.url.should == "http://mordor/walk_in"
+      end
+
+      it "should extract action" do
+        subject.action.should == "walk"
+      end
+
+      it "should extract parameters" do
+        subject.parameters.should == {
+          "action" => "walk",
+          "controller" => "ultimate",
+          "foo" => "bar"
+        }
+      end
+
+      it "should extract remote ip" do
+        subject.remote_ip.should == "1.2.3.4"
+      end
+    end
+end

spec/integrations/exception_data_spec.rb

@@ -9,16 +9,11 @@ def backtrace
 
 describe Exceptional::ControllerExceptionData do
   it "raises useful error when to_json isn't available on to_hash" do
-    begin
       data = Exceptional::ExceptionData.new(Exceptional::FunkyError.new)
       hash_without_json = {}
       hash_without_json.stub!(:to_json).and_raise(NoMethodError)
       data.stub!(:to_hash).and_return(hash_without_json)
-      data.to_json
-      fail 'expects to raise and error'
-    rescue StandardError => e
-      e.message.should =~ /to_json/
-    end
+      expect { data.to_json }.to raise_exception(/to_json/)
   end
 end
 
@@ -90,32 +85,32 @@ def backtrace
 describe Exceptional::ControllerExceptionData, 'with request/controller/params' do
 
   class Exceptional::SomeController < ActionController::Base
-    # filter_parameter_logging :filter_me # RAILS 2
   end
 
   before :each do
     @controller = Exceptional::SomeController.new
-    @request = ActionDispatch::TestRequest.new({'action' => 'some_action' })
-    @request.request_uri = '/some_path?var1=abc'
-    @request.env["QUERY_STRING"] = "var1=abc"
+    @request = ActionDispatch::TestRequest.new
+    @request.stub!(:parameters).and_return({'var1' => 'abc', 'action' => 'some_action', 'filter_me' => 'private'})
+    @request.stub!(:url).and_return('http://youtube.com/watch?v=oHg5SJYRHA0')
     @request.stub!(:request_method).and_return(:get)
     @request.stub!(:remote_ip).and_return('1.2.3.4')
     @request.stub!(:env).and_return({'SOME_VAR' => 'abc', 'HTTP_CONTENT_TYPE' => 'text/html'})
+    @request.env["action_dispatch.parameter_filter"] = [:filter_me]
     @error = Exceptional::FunkyError.new('some message')
     data = Exceptional::ControllerExceptionData.new(@error, @controller, @request)
     @hash = data.to_hash
   end
 
-  # it "captures request" do
-  #   request_hash = @hash['request']
-  #   request_hash['url'].should == 'http://test.host/some_path?var1=abc'
-  #   request_hash['controller'].should == 'Exceptional::SomeController'
-  #   request_hash['action'].should == 'some_action'
-  #   request_hash['parameters'].should == {'var1' => 'abc', 'action' => 'some_action', 'filter_me' => '[FILTERED]'}
-  #   request_hash['request_method'].should == 'get'
-  #   request_hash['remote_ip'].should == '1.2.3.4'
-  #   request_hash['headers'].should == {'Content-Type' => 'text/html'}
-  # end
+  it "captures request" do
+    request_hash = @hash['request']
+    request_hash['url'].should == 'http://youtube.com/watch?v=oHg5SJYRHA0'
+    request_hash['controller'].should == 'Exceptional::SomeController'
+    request_hash['action'].should == 'some_action'
+    request_hash['parameters'].should == {'var1' => 'abc', 'action' => 'some_action', 'filter_me' => '[FILTERED]'}
+    request_hash['request_method'].should == 'get'
+    request_hash['remote_ip'].should == '1.2.3.4'
+    request_hash['headers'].should == {'Content-Type' => 'text/html'}
+  end
 
   it "filter out objects that aren't jsonable" do
     class Crazy