diff options
Diffstat (limited to 'actionpack/lib/action_dispatch')
12 files changed, 187 insertions, 120 deletions
diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb index a519d6c1fc..8c035c3c6c 100644 --- a/actionpack/lib/action_dispatch/http/request.rb +++ b/actionpack/lib/action_dispatch/http/request.rb @@ -225,7 +225,7 @@ module ActionDispatch @remote_ip ||= (@env["action_dispatch.remote_ip"] || ip).to_s end - # Returns the unique request id, which is based off either the X-Request-Id header that can + # Returns the unique request id, which is based on either the X-Request-Id header that can # be generated by a firewall, load balancer, or web server or by the RequestId middleware # (which sets the action_dispatch.request_id environment variable). # diff --git a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb index 0ca1a87645..274f6f2f22 100644 --- a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb +++ b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb @@ -38,9 +38,7 @@ module ActionDispatch template = ActionView::Base.new([RESCUES_TEMPLATE_PATH], request: request, exception: wrapper.exception, - application_trace: wrapper.application_trace, - framework_trace: wrapper.framework_trace, - full_trace: wrapper.full_trace, + traces: traces_from_wrapper(wrapper), routes_inspector: routes_inspector(exception), source_extract: wrapper.source_extract, line_number: wrapper.line_number, @@ -95,5 +93,36 @@ module ActionDispatch ActionDispatch::Routing::RoutesInspector.new(@routes_app.routes.routes) end end + + # Augment the exception traces by providing ids for all unique stack frame + def traces_from_wrapper(wrapper) + application_trace = wrapper.application_trace + framework_trace = wrapper.framework_trace + full_trace = wrapper.full_trace + + if application_trace && framework_trace + id_counter = 0 + + application_trace = application_trace.map do |trace| + prev = id_counter + id_counter += 1 + { id: prev, trace: trace } + end + + framework_trace = framework_trace.map do |trace| + prev = id_counter + id_counter += 1 + { id: prev, trace: trace } + end + + full_trace = application_trace + framework_trace + end + + { + "Application Trace" => application_trace, + "Framework Trace" => framework_trace, + "Full Trace" => full_trace + } + end end end diff --git a/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb b/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb index 2326bb043a..b98b553c38 100644 --- a/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb +++ b/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb @@ -61,12 +61,15 @@ module ActionDispatch end def source_extract - if application_trace && trace = application_trace.first - file, line, _ = trace.split(":") - @file = file - @line_number = line.to_i - source_fragment(@file, @line_number) - end + exception.backtrace.map do |trace| + file, line = trace.split(":") + line_number = line.to_i + { + code: source_fragment(file, line_number), + file: file, + line_number: line_number + } + end if exception.backtrace end private @@ -110,7 +113,7 @@ module ActionDispatch def expand_backtrace @exception.backtrace.unshift( @exception.to_s.split("\n") - ).flatten! + ).flatten! end end end diff --git a/actionpack/lib/action_dispatch/middleware/request_id.rb b/actionpack/lib/action_dispatch/middleware/request_id.rb index 5d1740d0d4..25658bac3d 100644 --- a/actionpack/lib/action_dispatch/middleware/request_id.rb +++ b/actionpack/lib/action_dispatch/middleware/request_id.rb @@ -5,7 +5,7 @@ module ActionDispatch # Makes a unique request id available to the action_dispatch.request_id env variable (which is then accessible through # ActionDispatch::Request#uuid) and sends the same id to the client via the X-Request-Id header. # - # The unique request id is either based off the X-Request-Id header in the request, which would typically be generated + # The unique request id is either based on the X-Request-Id header in the request, which would typically be generated # by a firewall, load balancer, or the web server, or, if this header is not available, a random uuid. If the # header is accepted from the outside world, we sanitize it to a max of 255 chars and alphanumeric and dashes only. # diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/_source.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/_source.erb index 38429cb78e..51660a619b 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/_source.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/_source.erb @@ -1,25 +1,29 @@ <% if @source_extract %> -<div class="source"> -<div class="info"> - Extracted source (around line <strong>#<%= @line_number %></strong>): -</div> -<div class="data"> - <table cellpadding="0" cellspacing="0" class="lines"> - <tr> - <td> - <pre class="line_numbers"> - <% @source_extract.keys.each do |line_number| %> + <% @source_extract.each_with_index do |extract_source, index| %> + <% if extract_source[:code] %> + <div class="source <%="hidden" if index != 0%>" id="frame-source-<%=index%>"> + <div class="info"> + Extracted source (around line <strong>#<%= extract_source[:line_number] %></strong>): + </div> + <div class="data"> + <table cellpadding="0" cellspacing="0" class="lines"> + <tr> + <td> + <pre class="line_numbers"> + <% extract_source[:code].keys.each do |line_number| %> <span><%= line_number -%></span> - <% end %> - </pre> - </td> + <% end %> + </pre> + </td> <td width="100%"> <pre> -<% @source_extract.each do |line, source| -%><div class="line<%= " active" if line == @line_number -%>"><%= source -%></div><% end -%> +<% extract_source[:code].each do |line, source| -%><div class="line<%= " active" if line == extract_source[:line_number] -%>"><%= source -%></div><% end -%> </pre> </td> - </tr> - </table> -</div> -</div> + </tr> + </table> + </div> + </div> + <% end %> + <% end %> <% end %> diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/_trace.html.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/_trace.html.erb index b181909bff..f62caf51d7 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/_trace.html.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/_trace.html.erb @@ -1,9 +1,4 @@ -<% - traces = { "Application Trace" => @application_trace, - "Framework Trace" => @framework_trace, - "Full Trace" => @full_trace } - names = traces.keys -%> +<% names = @traces.keys %> <p><code>Rails.root: <%= defined?(Rails) && Rails.respond_to?(:root) ? Rails.root : "unset" %></code></p> @@ -16,9 +11,42 @@ <a href="#" onclick="<%= hide.join %><%= show %>; return false;"><%= name %></a> <%= '|' unless names.last == name %> <% end %> - <% traces.each do |name, trace| %> + <% @traces.each do |name, trace| %> <div id="<%= name.gsub(/\s/, '-') %>" style="display: <%= (name == "Application Trace") ? 'block' : 'none' %>;"> - <pre><code><%= trace.join "\n" %></code></pre> + <pre><code><% trace.each do |frame| %><a class="trace-frames" data-frame-id="<%= frame[:id] %>" href="#"><%= frame[:trace] %></a><br><% end %></code></pre> </div> <% end %> + + <script type="text/javascript"> + var traceFrames = document.getElementsByClassName('trace-frames'); + var selectedFrame, currentSource = document.getElementById('frame-source-0'); + + // Add click listeners for all stack frames + for (var i = 0; i < traceFrames.length; i++) { + traceFrames[i].addEventListener('click', function(e) { + e.preventDefault(); + var target = e.target; + var frame_id = target.dataset.frameId; + + if (selectedFrame) { + selectedFrame.className = selectedFrame.className.replace("selected", ""); + } + + target.className += " selected"; + selectedFrame = target; + + // Change the extracted source code + changeSourceExtract(frame_id); + }); + + function changeSourceExtract(frame_id) { + var el = document.getElementById('frame-source-' + frame_id); + if (currentSource && el) { + currentSource.className += " hidden"; + el.className = el.className.replace(" hidden", ""); + currentSource = el; + } + } + } + </script> </div> diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/_trace.text.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/_trace.text.erb index d4af5c9b06..c0b53068f7 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/_trace.text.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/_trace.text.erb @@ -1,15 +1,9 @@ -<% - traces = { "Application Trace" => @application_trace, - "Framework Trace" => @framework_trace, - "Full Trace" => @full_trace } -%> - Rails.root: <%= defined?(Rails) && Rails.respond_to?(:root) ? Rails.root : "unset" %> -<% traces.each do |name, trace| %> +<% @traces.each do |name, trace| %> <% if trace.any? %> <%= name %> -<%= trace.join("\n") %> +<%= trace.map { |t| t[:trace] }.join("\n") %> <% end %> <% end %> diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/layout.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/layout.erb index bc5d03dc10..e0509f56f4 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/layout.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/layout.erb @@ -116,9 +116,15 @@ background-color: #FFCCCC; } + .hidden { + display: none; + } + a { color: #980905; } a:visited { color: #666; } + a.trace-frames { color: #666; } a:hover { color: #C52F24; } + a.trace-frames.selected { color: #C52F24 } <%= yield :style %> </style> diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/template_error.html.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/template_error.html.erb index 027a0f5b3e..c1e8b6cae3 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/template_error.html.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/template_error.html.erb @@ -1,4 +1,3 @@ -<% @source_extract = @exception.source_extract(0, :html) %> <header> <h1> <%= @exception.original_exception.class.to_s %> in @@ -12,29 +11,7 @@ </p> <pre><code><%= h @exception.message %></code></pre> - <div class="source"> - <div class="info"> - <p>Extracted source (around line <strong>#<%= @exception.line_number %></strong>):</p> - </div> - <div class="data"> - <table cellpadding="0" cellspacing="0" class="lines"> - <tr> - <td> - <pre class="line_numbers"> - <% @source_extract.keys.each do |line_number| %> -<span><%= line_number -%></span> - <% end %> - </pre> - </td> -<td width="100%"> -<pre> -<% @source_extract.each do |line, source| -%><div class="line<%= " active" if line == @exception.line_number -%>"><%= source -%></div><% end -%> -</pre> -</td> - </tr> - </table> -</div> -</div> + <%= render template: "rescues/_source" %> <p><%= @exception.sub_template_message %></p> diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/template_error.text.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/template_error.text.erb index 5da21d9784..77bcd26726 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/template_error.text.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/template_error.text.erb @@ -1,4 +1,3 @@ -<% @source_extract = @exception.source_extract(0, :html) %> <%= @exception.original_exception.class.to_s %> in <%= @request.parameters["controller"].camelize if @request.parameters["controller"] %>#<%= @request.parameters["action"] %> Showing <%= @exception.file_name %> where line #<%= @exception.line_number %> raised: diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index cd94f35e8f..e92baa5aa7 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -63,7 +63,7 @@ module ActionDispatch attr_reader :requirements, :conditions, :defaults attr_reader :to, :default_controller, :default_action, :as, :anchor - def self.build(scope, set, path, options) + def self.build(scope, set, path, as, options) options = scope[:options].merge(options) if scope[:options] options.delete :only @@ -74,10 +74,10 @@ module ActionDispatch defaults = (scope[:defaults] || {}).merge options.delete(:defaults) || {} - new scope, set, path, defaults, options + new scope, set, path, defaults, as, options end - def initialize(scope, set, path, defaults, options) + def initialize(scope, set, path, defaults, as, options) @requirements, @conditions = {}, {} @defaults = defaults @set = set @@ -85,7 +85,7 @@ module ActionDispatch @to = options.delete :to @default_controller = options.delete(:controller) || scope[:controller] @default_action = options.delete(:action) || scope[:action] - @as = options.delete :as + @as = as @anchor = options.delete :anchor formatted = options.delete :format @@ -1046,8 +1046,6 @@ module ActionDispatch VALID_ON_OPTIONS = [:new, :collection, :member] RESOURCE_OPTIONS = [:as, :controller, :path, :only, :except, :param, :concerns] CANONICAL_ACTIONS = %w(index create new show update destroy) - RESOURCE_METHOD_SCOPES = [:collection, :member, :new] - RESOURCE_SCOPES = [:resource, :resources] class Resource #:nodoc: attr_reader :controller, :path, :options, :param @@ -1521,7 +1519,7 @@ module ActionDispatch if on = options.delete(:on) send(on) { decomposed_match(path, options) } else - case @scope[:scope_level] + case @scope.scope_level when :resources nested { decomposed_match(path, options) } when :resource @@ -1544,13 +1542,13 @@ module ActionDispatch action = nil end - if !options.fetch(:as, true) # if it's set to nil or false - options.delete(:as) - else - options[:as] = name_for_action(options[:as], action) - end + as = if !options.fetch(:as, true) # if it's set to nil or false + options.delete(:as) + else + name_for_action(options.delete(:as), action) + end - mapping = Mapping.build(@scope, @set, URI.parser.escape(path), options) + mapping = Mapping.build(@scope, @set, URI.parser.escape(path), as, options) app, conditions, requirements, defaults, as, anchor = mapping.to_route @set.add_route(app, conditions, requirements, defaults, as, anchor) end @@ -1564,7 +1562,7 @@ module ActionDispatch raise ArgumentError, "must be called with a path and/or options" end - if @scope[:scope_level] == :resources + if @scope.resources? with_scope_level(:root) do scope(parent_resource.path) do super(options) @@ -1631,15 +1629,15 @@ module ActionDispatch end def resource_scope? #:nodoc: - RESOURCE_SCOPES.include? @scope[:scope_level] + @scope.resource_scope? end def resource_method_scope? #:nodoc: - RESOURCE_METHOD_SCOPES.include? @scope[:scope_level] + @scope.resource_method_scope? end def nested_scope? #:nodoc: - @scope[:scope_level] == :nested + @scope.nested? end def with_exclusive_scope @@ -1655,7 +1653,7 @@ module ActionDispatch end def with_scope_level(kind) - @scope = @scope.new(:scope_level => kind) + @scope = @scope.new_level(kind) yield ensure @scope = @scope.parent @@ -1699,8 +1697,8 @@ module ActionDispatch @scope[:constraints][parent_resource.param] end - def canonical_action?(action, flag) #:nodoc: - flag && resource_method_scope? && CANONICAL_ACTIONS.include?(action.to_s) + def canonical_action?(action) #:nodoc: + resource_method_scope? && CANONICAL_ACTIONS.include?(action.to_s) end def shallow_scope(path, options = {}) #:nodoc: @@ -1714,7 +1712,7 @@ module ActionDispatch end def path_for_action(action, path) #:nodoc: - if canonical_action?(action, path.blank?) + if path.blank? && canonical_action?(action) @scope[:path].to_s else "#{@scope[:path]}/#{action_path(action, path)}" @@ -1729,15 +1727,17 @@ module ActionDispatch def prefix_name_for_action(as, action) #:nodoc: if as prefix = as - elsif !canonical_action?(action, @scope[:scope_level]) + elsif !canonical_action?(action) prefix = action end - prefix.to_s.tr('-', '_') if prefix + + if prefix && prefix != '/' && !prefix.empty? + Mapper.normalize_name prefix.to_s.tr('-', '_') + end end def name_for_action(as, action) #:nodoc: prefix = prefix_name_for_action(as, action) - prefix = Mapper.normalize_name(prefix) if prefix name_prefix = @scope[:as] if parent_resource @@ -1747,22 +1747,9 @@ module ActionDispatch member_name = parent_resource.member_name end - name = case @scope[:scope_level] - when :nested - [name_prefix, prefix] - when :collection - [prefix, name_prefix, collection_name] - when :new - [prefix, :new, name_prefix, member_name] - when :member - [prefix, name_prefix, member_name] - when :root - [name_prefix, collection_name, prefix] - else - [name_prefix, member_name, prefix] - end + name = @scope.action_name(name_prefix, prefix, collection_name, member_name) - if candidate = name.select(&:present?).join("_").presence + if candidate = name.compact.join("_").presence # If a name was not explicitly given, we check if it is valid # and return nil in case it isn't. Otherwise, we pass the invalid name # forward so the underlying router engine treats it and raises an exception. @@ -1897,11 +1884,48 @@ module ActionDispatch :controller, :action, :path_names, :constraints, :shallow, :blocks, :defaults, :options] - attr_reader :parent + RESOURCE_SCOPES = [:resource, :resources] + RESOURCE_METHOD_SCOPES = [:collection, :member, :new] + + attr_reader :parent, :scope_level - def initialize(hash, parent = {}) + def initialize(hash, parent = {}, scope_level = nil) @hash = hash @parent = parent + @scope_level = scope_level + end + + def nested? + scope_level == :nested + end + + def resources? + scope_level == :resources + end + + def resource_method_scope? + RESOURCE_METHOD_SCOPES.include? scope_level + end + + def action_name(name_prefix, prefix, collection_name, member_name) + case scope_level + when :nested + [name_prefix, prefix] + when :collection + [prefix, name_prefix, collection_name] + when :new + [prefix, :new, name_prefix, member_name] + when :member + [prefix, name_prefix, member_name] + when :root + [name_prefix, collection_name, prefix] + else + [name_prefix, member_name, prefix] + end + end + + def resource_scope? + RESOURCE_SCOPES.include? scope_level end def options @@ -1909,7 +1933,15 @@ module ActionDispatch end def new(hash) - self.class.new hash, self + self.class.new hash, self, scope_level + end + + def new_level(level) + self.class.new(self, self, level) + end + + def fetch(key, &block) + @hash.fetch(key, &block) end def [](key) diff --git a/actionpack/lib/action_dispatch/testing/assertions/response.rb b/actionpack/lib/action_dispatch/testing/assertions/response.rb index 0adc6c84ff..13a72220b3 100644 --- a/actionpack/lib/action_dispatch/testing/assertions/response.rb +++ b/actionpack/lib/action_dispatch/testing/assertions/response.rb @@ -73,13 +73,8 @@ module ActionDispatch if Regexp === fragment fragment else - handle = @controller || Class.new(ActionController::Metal) do - include ActionController::Redirecting - def initialize(request) - @_request = request - end - end.new(@request) - handle._compute_redirect_to_location(fragment) + handle = @controller || ActionController::Redirecting + handle._compute_redirect_to_location(@request, fragment) end end end |