From 083f657c0f1990e980d33f89f44d8943a9270475 Mon Sep 17 00:00:00 2001
From: Kir Shatrov <shatrov@me.com>
Date: Mon, 1 Dec 2014 17:45:33 -0500
Subject: Refactor debug view Avoid logic in ERB and use helpers

---
 .../action_dispatch/middleware/debug_exceptions.rb | 30 +++++++++++++++++++++-
 .../middleware/exception_wrapper.rb                |  1 -
 .../rescues/_request_and_response.html.erb         | 23 ++---------------
 actionpack/test/dispatch/debug_exceptions_test.rb  | 23 +++++++++++++++++
 4 files changed, 54 insertions(+), 23 deletions(-)

(limited to 'actionpack')

diff --git a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb
index c1562fcc0d..9f27187aa7 100644
--- a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb
+++ b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb
@@ -2,12 +2,40 @@ require 'action_dispatch/http/request'
 require 'action_dispatch/middleware/exception_wrapper'
 require 'action_dispatch/routing/inspector'
 
+require 'pp'
+
 module ActionDispatch
   # This middleware is responsible for logging exceptions and
   # showing a debugging page in case the request is local.
   class DebugExceptions
     RESCUES_TEMPLATE_PATH = File.expand_path('../templates', __FILE__)
 
+    class DebugView < ActionView::Base
+      def debug_params(params)
+        clean_params = params.clone
+        clean_params.delete("action")
+        clean_params.delete("controller")
+
+        if clean_params.empty?
+          'None'
+        else
+          PP.pp(clean_params, "", 200)
+        end
+      end
+
+      def debug_headers(headers)
+        if headers.present?
+          headers.inspect.gsub(',', ",\n")
+        else
+          'None'
+        end
+      end
+
+      def debug_hash(object)
+        object.to_hash.sort_by { |k, _| k.to_s }.map { |k, v| "#{k}: #{v.inspect rescue $!.message}" }.join("\n")
+      end
+    end
+
     def initialize(app, routes_app = nil)
       @app        = app
       @routes_app = routes_app
@@ -46,7 +74,7 @@ module ActionDispatch
           source_to_show_id = source_to_show[:id]
         end
 
-        template = ActionView::Base.new([RESCUES_TEMPLATE_PATH],
+        template = DebugView.new([RESCUES_TEMPLATE_PATH],
           request: request,
           exception: wrapper.exception,
           traces: traces,
diff --git a/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb b/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb
index 8edeeaf2af..a4862e33aa 100644
--- a/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb
+++ b/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb
@@ -1,6 +1,5 @@
 require 'action_controller/metal/exceptions'
 require 'active_support/core_ext/module/attribute_accessors'
-require 'pp'
 
 module ActionDispatch
   class ExceptionWrapper
diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/_request_and_response.html.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/_request_and_response.html.erb
index 761cd0ffcc..49b1e83551 100644
--- a/actionpack/lib/action_dispatch/middleware/templates/rescues/_request_and_response.html.erb
+++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/_request_and_response.html.erb
@@ -5,27 +5,8 @@
   <pre id="blame_trace" <%='style="display:none"' if hide %>><code><%= @exception.describe_blame %></code></pre>
 <% end %>
 
-<%
-  clean_params = @request.filtered_parameters.clone
-  clean_params.delete("action")
-  clean_params.delete("controller")
-
-  request_dump = if clean_params.empty?
-    'None'
-  else
-    pretty_params = ""
-    PP.pp(clean_params, pretty_params, 200)
-
-    pretty_params
-  end
-
-  def debug_hash(object)
-    object.to_hash.sort_by { |k, _| k.to_s }.map { |k, v| "#{k}: #{v.inspect rescue $!.message}" }.join("\n")
-  end unless self.class.method_defined?(:debug_hash)
-%>
-
 <h2 style="margin-top: 30px">Request</h2>
-<p><b>Parameters</b>:</p> <pre><%= request_dump %></pre>
+<p><b>Parameters</b>:</p> <pre><%= debug_params(@request.filtered_parameters) %></pre>
 
 <div class="details">
   <div class="summary"><a href="#" onclick="return toggleSessionDump()">Toggle session dump</a></div>
@@ -38,4 +19,4 @@
 </div>
 
 <h2 style="margin-top: 30px">Response</h2>
-<p><b>Headers</b>:</p> <pre><%= defined?(@response) ? @response.headers.inspect.gsub(',', ",\n") : 'None' %></pre>
+<p><b>Headers</b>:</p> <pre><%= debug_headers(defined?(@response) ? @response.headers : {}) %></pre>
diff --git a/actionpack/test/dispatch/debug_exceptions_test.rb b/actionpack/test/dispatch/debug_exceptions_test.rb
index 5e87744f6b..1e5ed60b09 100644
--- a/actionpack/test/dispatch/debug_exceptions_test.rb
+++ b/actionpack/test/dispatch/debug_exceptions_test.rb
@@ -241,6 +241,29 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest
     assert_match(/RuntimeError\n\s+in FeaturedTileController/, body)
   end
 
+  test "show formatted params" do
+    @app = DevelopmentApp
+
+    params = {
+      'id' => 'unknown',
+      'someparam' => {
+        'foo' => 'bar',
+        'abc' => 'goo'
+      }
+    }
+
+    get("/runtime_error", {}, {
+      'action_dispatch.show_exceptions' => true,
+      'action_dispatch.request.parameters' => {
+        'action' => 'show',
+        'controller' => 'featured_tile'
+      }.merge(params)
+    })
+    assert_response 500
+
+    assert_includes(body, CGI.escapeHTML(PP.pp(params, "", 200)))
+  end
+
   test "sets the HTTP charset parameter" do
     @app = DevelopmentApp
 
-- 
cgit v1.2.3