diff options
Diffstat (limited to 'actionpack')
19 files changed, 365 insertions, 32 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 67c9e04ab2..be793d905f 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,12 +1,14 @@ ## Rails 4.0.0 (unreleased) ## +* Show routes in exception page while debugging a `RoutingError` in development. *Richard Schneeman and Mattt Thompson* + * Add `ActionController::Flash.add_flash_types` method to allow people to register their own flash types. e.g.: class ApplicationController add_flash_types :error, :warning end - If you add the above code, you can use `<%= error %>` in an erb, and `rediect_to /foo, :error => 'message'` in a controller. + If you add the above code, you can use `<%= error %>` in an erb, and `redirect_to /foo, :error => 'message'` in a controller. *kennyj* diff --git a/actionpack/lib/action_controller/metal/http_authentication.rb b/actionpack/lib/action_controller/metal/http_authentication.rb index a0d1064094..0050ede806 100644 --- a/actionpack/lib/action_controller/metal/http_authentication.rb +++ b/actionpack/lib/action_controller/metal/http_authentication.rb @@ -436,10 +436,12 @@ module ActionController values = Hash[$1.split(',').map do |value| value.strip! # remove any spaces between commas and values key, value = value.split(/\=\"?/) # split key=value pairs - value.chomp!('"') # chomp trailing " in value - value.gsub!(/\\\"/, '"') # unescape remaining quotes - [key, value] - end] + if value + value.chomp!('"') # chomp trailing " in value + value.gsub!(/\\\"/, '"') # unescape remaining quotes + [key, value] + end + end.compact] [values.delete("token"), values.with_indifferent_access] end end diff --git a/actionpack/lib/action_controller/metal/responder.rb b/actionpack/lib/action_controller/metal/responder.rb index 83407846dc..d9c89a74f1 100644 --- a/actionpack/lib/action_controller/metal/responder.rb +++ b/actionpack/lib/action_controller/metal/responder.rb @@ -90,7 +90,7 @@ module ActionController #:nodoc: # # def create # @project = Project.find(params[:project_id]) - # @task = @project.comments.build(params[:task]) + # @task = @project.tasks.build(params[:task]) # flash[:notice] = 'Task was successfully created.' if @task.save # respond_with(@project, @task, :status => 201) # end diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index 0498b9d138..5f50bf5de6 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -358,7 +358,7 @@ module ActionController # Use AS::TestCase for the base class when describing a model register_spec_type(self) do |desc| - desc < ActionController::Base + Class === desc && desc < ActionController::Base end module Behavior diff --git a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb index b903f98761..0f0589a844 100644 --- a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb +++ b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb @@ -1,5 +1,7 @@ require 'action_dispatch/http/request' require 'action_dispatch/middleware/exception_wrapper' +require 'action_dispatch/routing/inspector' + module ActionDispatch # This middleware is responsible for logging exceptions and @@ -7,8 +9,9 @@ module ActionDispatch class DebugExceptions RESCUES_TEMPLATE_PATH = File.join(File.dirname(__FILE__), 'templates') - def initialize(app) - @app = app + def initialize(app, routes_app = nil) + @app = app + @routes_app = routes_app end def call(env) @@ -39,7 +42,8 @@ module ActionDispatch :exception => wrapper.exception, :application_trace => wrapper.application_trace, :framework_trace => wrapper.framework_trace, - :full_trace => wrapper.full_trace + :full_trace => wrapper.full_trace, + :routes => formatted_routes(exception) ) file = "rescues/#{wrapper.rescue_template}" @@ -78,5 +82,13 @@ module ActionDispatch def stderr_logger @stderr_logger ||= ActiveSupport::Logger.new($stderr) end + + def formatted_routes(exception) + return false unless @routes_app.respond_to?(:routes) + if exception.is_a?(ActionController::RoutingError) || exception.is_a?(ActionView::Template::Error) + inspector = ActionDispatch::Routing::RoutesInspector.new + inspector.format(@routes_app.routes.routes).join("\n") + end + end end end diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/routing_error.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/routing_error.erb index 177d383e94..8c594c1523 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/routing_error.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/routing_error.erb @@ -10,8 +10,14 @@ </ol> </p> <% end %> +<%= render :template => "rescues/_trace" %> + +<h2> + Routes +</h2> + <p> - Try running <code>rake routes</code> for more information on available routes. + Routes match in priority from top to bottom </p> -<%= render :template => "rescues/_trace" %> +<p><pre><%= @routes %></pre></p> diff --git a/actionpack/lib/action_dispatch/routing/inspector.rb b/actionpack/lib/action_dispatch/routing/inspector.rb new file mode 100644 index 0000000000..bc7229b6a1 --- /dev/null +++ b/actionpack/lib/action_dispatch/routing/inspector.rb @@ -0,0 +1,121 @@ +require 'delegate' + +module ActionDispatch + module Routing + class RouteWrapper < SimpleDelegator + def endpoint + rack_app ? rack_app.inspect : "#{controller}##{action}" + end + + def constraints + requirements.except(:controller, :action) + end + + def rack_app(app = self.app) + @rack_app ||= begin + class_name = app.class.name.to_s + if class_name == "ActionDispatch::Routing::Mapper::Constraints" + rack_app(app.app) + elsif ActionDispatch::Routing::Redirect === app || class_name !~ /^ActionDispatch::Routing/ + app + end + end + end + + def verb + super.source.gsub(/[$^]/, '') + end + + def path + super.spec.to_s + end + + def name + super.to_s + end + + def reqs + @reqs ||= begin + reqs = endpoint + reqs += " #{constraints.inspect}" unless constraints.empty? + reqs + end + end + + def controller + requirements[:controller] || ':controller' + end + + def action + requirements[:action] || ':action' + end + + def internal? + path =~ %r{/rails/info.*|^#{Rails.application.config.assets.prefix}} + end + + def engine? + rack_app && rack_app.respond_to?(:routes) + end + end + + ## + # This class is just used for displaying route information when someone + # executes `rake routes`. People should not use this class. + class RoutesInspector # :nodoc: + def initialize + @engines = Hash.new + end + + def format(all_routes, filter = nil) + if filter + all_routes = all_routes.select{ |route| route.defaults[:controller] == filter } + end + + routes = collect_routes(all_routes) + + formatted_routes(routes) + + formatted_routes_for_engines + end + + def collect_routes(routes) + routes = routes.collect do |route| + RouteWrapper.new(route) + end.reject do |route| + route.internal? + end.collect do |route| + collect_engine_routes(route) + + {:name => route.name, :verb => route.verb, :path => route.path, :reqs => route.reqs } + end + end + + def collect_engine_routes(route) + name = route.endpoint + return unless route.engine? + return if @engines[name] + + routes = route.rack_app.routes + if routes.is_a?(ActionDispatch::Routing::RouteSet) + @engines[name] = collect_routes(routes.routes) + end + end + + def formatted_routes_for_engines + @engines.map do |name, routes| + ["\nRoutes for #{name}:"] + formatted_routes(routes) + end.flatten + end + + def formatted_routes(routes) + name_width = routes.map{ |r| r[:name].length }.max + verb_width = routes.map{ |r| r[:verb].length }.max + path_width = routes.map{ |r| r[:path].length }.max + + routes.map do |r| + "#{r[:name].rjust(name_width)} #{r[:verb].ljust(verb_width)} #{r[:path].ljust(path_width)} #{r[:reqs]}" + end + end + end + end +end diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 53a4afecb3..a49b12621e 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -430,6 +430,10 @@ module ActionDispatch if options path = options.delete(:at) else + unless Hash === app + raise ArgumentError, "must be called with mount point" + end + options = app app, path = options.find { |k, v| k.respond_to?(:call) } options.delete(app) if app diff --git a/actionpack/lib/action_view/context.rb b/actionpack/lib/action_view/context.rb index 245849d706..ee263df484 100644 --- a/actionpack/lib/action_view/context.rb +++ b/actionpack/lib/action_view/context.rb @@ -26,11 +26,11 @@ module ActionView # Encapsulates the interaction with the view flow so it # returns the correct buffer on +yield+. This is usually - # overwriten by helpers to add more behavior. + # overwritten by helpers to add more behavior. # :api: plugin def _layout_for(name=nil) name ||= :layout view_flow.get(name).html_safe end end -end
\ No newline at end of file +end diff --git a/actionpack/lib/action_view/helpers/form_options_helper.rb b/actionpack/lib/action_view/helpers/form_options_helper.rb index eef426703d..72fbbd109a 100644 --- a/actionpack/lib/action_view/helpers/form_options_helper.rb +++ b/actionpack/lib/action_view/helpers/form_options_helper.rb @@ -353,7 +353,7 @@ module ActionView html_attributes[:disabled] = 'disabled' if disabled && option_value_selected?(value, disabled) html_attributes[:value] = value - content_tag(:option, text, html_attributes) + content_tag_string(:option, text, html_attributes) end.join("\n").html_safe end diff --git a/actionpack/lib/action_view/helpers/tags/base.rb b/actionpack/lib/action_view/helpers/tags/base.rb index e077cd5b3c..192f5eebaa 100644 --- a/actionpack/lib/action_view/helpers/tags/base.rb +++ b/actionpack/lib/action_view/helpers/tags/base.rb @@ -137,10 +137,10 @@ module ActionView def add_options(option_tags, options, value = nil) if options[:include_blank] - option_tags = content_tag('option', options[:include_blank].kind_of?(String) ? options[:include_blank] : nil, :value => '') + "\n" + option_tags + option_tags = content_tag_string('option', options[:include_blank].kind_of?(String) ? options[:include_blank] : nil, :value => '') + "\n" + option_tags end if value.blank? && options[:prompt] - option_tags = content_tag('option', prompt_text(options[:prompt]), :value => '') + "\n" + option_tags + option_tags = content_tag_string('option', prompt_text(options[:prompt]), :value => '') + "\n" + option_tags end option_tags end diff --git a/actionpack/test/controller/http_token_authentication_test.rb b/actionpack/test/controller/http_token_authentication_test.rb index 3054c1684c..ad4e743be8 100644 --- a/actionpack/test/controller/http_token_authentication_test.rb +++ b/actionpack/test/controller/http_token_authentication_test.rb @@ -79,6 +79,14 @@ class HttpTokenAuthenticationTest < ActionController::TestCase end end + test "authentication request with badly formatted header" do + @request.env['HTTP_AUTHORIZATION'] = "Token foobar" + get :index + + assert_response :unauthorized + assert_equal "HTTP Token: Access denied.\n", @response.body, "Authentication header was not properly parsed" + end + test "authentication request without credential" do get :display diff --git a/actionpack/test/controller/new_base/render_action_test.rb b/actionpack/test/controller/new_base/render_action_test.rb index aa44e0b282..475bf9d3c9 100644 --- a/actionpack/test/controller/new_base/render_action_test.rb +++ b/actionpack/test/controller/new_base/render_action_test.rb @@ -86,8 +86,6 @@ module RenderAction def setup end - describe "Both <controller_path>.html.erb and application.html.erb are missing" - test "rendering with layout => true" do assert_raise(ArgumentError) do get "/render_action/basic/hello_world_with_layout", {}, "action_dispatch.show_exceptions" => false @@ -154,8 +152,6 @@ module RenderActionWithApplicationLayout end class LayoutTest < Rack::TestCase - describe "Only application.html.erb is present and <controller_path>.html.erb is missing" - test "rendering implicit application.html.erb as layout" do get "/render_action_with_application_layout/basic/hello_world" @@ -232,8 +228,6 @@ module RenderActionWithControllerLayout end class ControllerLayoutTest < Rack::TestCase - describe "Only <controller_path>.html.erb is present and application.html.erb is missing" - test "render hello_world and implicitly use <controller_path>.html.erb as a layout." do get "/render_action_with_controller_layout/basic/hello_world" @@ -290,8 +284,6 @@ module RenderActionWithBothLayouts end class ControllerLayoutTest < Rack::TestCase - describe "Both <controller_path>.html.erb and application.html.erb are present" - test "rendering implicitly use <controller_path>.html.erb over application.html.erb as a layout" do get "/render_action_with_both_layouts/basic/hello_world" diff --git a/actionpack/test/controller/new_base/render_template_test.rb b/actionpack/test/controller/new_base/render_template_test.rb index 00c7df2af8..156d87c321 100644 --- a/actionpack/test/controller/new_base/render_template_test.rb +++ b/actionpack/test/controller/new_base/render_template_test.rb @@ -160,8 +160,6 @@ module RenderTemplate end class TestWithLayout < Rack::TestCase - describe "Rendering with :template using implicit or explicit layout" - test "rendering with implicit layout" do with_routing do |set| set.draw { get ':controller', :action => :index } diff --git a/actionpack/test/controller/new_base/render_text_test.rb b/actionpack/test/controller/new_base/render_text_test.rb index f8d02e8b6c..d6c3926a4d 100644 --- a/actionpack/test/controller/new_base/render_text_test.rb +++ b/actionpack/test/controller/new_base/render_text_test.rb @@ -63,8 +63,6 @@ module RenderText end class RenderTextTest < Rack::TestCase - describe "Rendering text using render :text" - test "rendering text from an action with default options renders the text with the layout" do with_routing do |set| set.draw { get ':controller', :action => 'index' } diff --git a/actionpack/test/dispatch/mapper_test.rb b/actionpack/test/dispatch/mapper_test.rb index 8070bdec8a..58457b0c28 100644 --- a/actionpack/test/dispatch/mapper_test.rb +++ b/actionpack/test/dispatch/mapper_test.rb @@ -98,6 +98,15 @@ module ActionDispatch mapper.get '/*path', :to => 'pages#show', :format => true assert_equal '/*path.:format', fakeset.conditions.first[:path_info] end + + def test_raising_helpful_error_on_invalid_arguments + fakeset = FakeSet.new + mapper = Mapper.new fakeset + app = lambda { |env| [200, {}, [""]] } + assert_raises ArgumentError do + mapper.mount app + end + end end end end diff --git a/actionpack/test/dispatch/routing/inspector_test.rb b/actionpack/test/dispatch/routing/inspector_test.rb new file mode 100644 index 0000000000..97fc4f3e4b --- /dev/null +++ b/actionpack/test/dispatch/routing/inspector_test.rb @@ -0,0 +1,170 @@ +require 'minitest/autorun' +require 'action_controller' +require 'rails/engine' +require 'action_dispatch/routing/inspector' + +module ActionDispatch + module Routing + class RoutesInspectorTest < ActiveSupport::TestCase + def setup + @set = ActionDispatch::Routing::RouteSet.new + @inspector = ActionDispatch::Routing::RoutesInspector.new + app = ActiveSupport::OrderedOptions.new + app.config = ActiveSupport::OrderedOptions.new + app.config.assets = ActiveSupport::OrderedOptions.new + app.config.assets.prefix = '/sprockets' + Rails.stubs(:application).returns(app) + Rails.stubs(:env).returns("development") + end + + def draw(&block) + @set.draw(&block) + @inspector.format(@set.routes) + end + + def test_displaying_routes_for_engines + engine = Class.new(Rails::Engine) do + def self.to_s + "Blog::Engine" + end + end + engine.routes.draw do + get '/cart', :to => 'cart#show' + end + + output = draw do + get '/custom/assets', :to => 'custom_assets#show' + mount engine => "/blog", :as => "blog" + end + + expected = [ + "custom_assets GET /custom/assets(.:format) custom_assets#show", + " blog /blog Blog::Engine", + "\nRoutes for Blog::Engine:", + "cart GET /cart(.:format) cart#show" + ] + assert_equal expected, output + end + + def test_cart_inspect + output = draw do + get '/cart', :to => 'cart#show' + end + assert_equal ["cart GET /cart(.:format) cart#show"], output + end + + def test_inspect_shows_custom_assets + output = draw do + get '/custom/assets', :to => 'custom_assets#show' + end + assert_equal ["custom_assets GET /custom/assets(.:format) custom_assets#show"], output + end + + def test_inspect_routes_shows_resources_route + output = draw do + resources :articles + end + expected = [ + " articles GET /articles(.:format) articles#index", + " POST /articles(.:format) articles#create", + " new_article GET /articles/new(.:format) articles#new", + "edit_article GET /articles/:id/edit(.:format) articles#edit", + " article GET /articles/:id(.:format) articles#show", + " PATCH /articles/:id(.:format) articles#update", + " PUT /articles/:id(.:format) articles#update", + " DELETE /articles/:id(.:format) articles#destroy" ] + assert_equal expected, output + end + + def test_inspect_routes_shows_root_route + output = draw do + root :to => 'pages#main' + end + assert_equal ["root GET / pages#main"], output + end + + def test_inspect_routes_shows_dynamic_action_route + output = draw do + get 'api/:action' => 'api' + end + assert_equal [" GET /api/:action(.:format) api#:action"], output + end + + def test_inspect_routes_shows_controller_and_action_only_route + output = draw do + get ':controller/:action' + end + assert_equal [" GET /:controller/:action(.:format) :controller#:action"], output + end + + def test_inspect_routes_shows_controller_and_action_route_with_constraints + output = draw do + get ':controller(/:action(/:id))', :id => /\d+/ + end + assert_equal [" GET /:controller(/:action(/:id))(.:format) :controller#:action {:id=>/\\d+/}"], output + end + + def test_rake_routes_shows_route_with_defaults + output = draw do + get 'photos/:id' => 'photos#show', :defaults => {:format => 'jpg'} + end + assert_equal [%Q[ GET /photos/:id(.:format) photos#show {:format=>"jpg"}]], output + end + + def test_rake_routes_shows_route_with_constraints + output = draw do + get 'photos/:id' => 'photos#show', :id => /[A-Z]\d{5}/ + end + assert_equal [" GET /photos/:id(.:format) photos#show {:id=>/[A-Z]\\d{5}/}"], output + end + + class RackApp + def self.call(env) + end + end + + def test_rake_routes_shows_route_with_rack_app + output = draw do + get 'foo/:id' => RackApp, :id => /[A-Z]\d{5}/ + end + assert_equal [" GET /foo/:id(.:format) #{RackApp.name} {:id=>/[A-Z]\\d{5}/}"], output + end + + def test_rake_routes_shows_route_with_rack_app_nested_with_dynamic_constraints + constraint = Class.new do + def to_s + "( my custom constraint )" + end + end + + output = draw do + scope :constraint => constraint.new do + mount RackApp => '/foo' + end + end + + assert_equal [" /foo #{RackApp.name} {:constraint=>( my custom constraint )}"], output + end + + def test_rake_routes_dont_show_app_mounted_in_assets_prefix + output = draw do + get '/sprockets' => RackApp + end + assert_no_match(/RackApp/, output.first) + assert_no_match(/\/sprockets/, output.first) + end + + def test_redirect + output = draw do + get "/foo" => redirect("/foo/bar"), :constraints => { :subdomain => "admin" } + get "/bar" => redirect(path: "/foo/bar", status: 307) + get "/foobar" => redirect{ "/foo/bar" } + end + + assert_equal " foo GET /foo(.:format) redirect(301, /foo/bar) {:subdomain=>\"admin\"}", output[0] + assert_equal " bar GET /bar(.:format) redirect(307, path: /foo/bar)", output[1] + assert_equal "foobar GET /foobar(.:format) redirect(301)", output[2] + end + end + end +end diff --git a/actionpack/test/template/active_model_helper_test.rb b/actionpack/test/template/active_model_helper_test.rb index 24511df444..86bccdfade 100644 --- a/actionpack/test/template/active_model_helper_test.rb +++ b/actionpack/test/template/active_model_helper_test.rb @@ -41,6 +41,19 @@ class ActiveModelHelperTest < ActionView::TestCase ) end + def test_select_with_errors + assert_dom_equal( + %(<div class="field_with_errors"><select name="post[author_name]" id="post_author_name"><option value="a">a</option>\n<option value="b">b</option></select></div>), + select("post", "author_name", [:a, :b]) + ) + end + + def test_select_with_errors_and_blank_option + expected_dom = %(<div class="field_with_errors"><select name="post[author_name]" id="post_author_name"><option value="">Choose one...</option>\n<option value="a">a</option>\n<option value="b">b</option></select></div>) + assert_dom_equal(expected_dom, select("post", "author_name", [:a, :b], :include_blank => 'Choose one...')) + assert_dom_equal(expected_dom, select("post", "author_name", [:a, :b], :prompt => 'Choose one...')) + end + def test_date_select_with_errors assert_dom_equal( %(<div class="field_with_errors"><select id="post_updated_at_1i" name="post[updated_at(1i)]">\n<option selected="selected" value="2004">2004</option>\n<option value="2005">2005</option>\n</select>\n<input id="post_updated_at_2i" name="post[updated_at(2i)]" type="hidden" value="6" />\n<input id="post_updated_at_3i" name="post[updated_at(3i)]" type="hidden" value="1" />\n</div>), @@ -82,4 +95,5 @@ class ActiveModelHelperTest < ActionView::TestCase ensure ActionView::Base.field_error_proc = old_proc if old_proc end + end diff --git a/actionpack/test/template/erb/tag_helper_test.rb b/actionpack/test/template/erb/tag_helper_test.rb index 1724d6432d..84e328d8be 100644 --- a/actionpack/test/template/erb/tag_helper_test.rb +++ b/actionpack/test/template/erb/tag_helper_test.rb @@ -3,9 +3,6 @@ require "template/erb/helper" module ERBTest class TagHelperTest < BlockTestCase - - extend ActiveSupport::Testing::Declarative - test "percent equals works for content_tag and does not require parenthesis on method call" do assert_equal "<div>Hello world</div>", render_content("content_tag :div", "Hello world") end |