From cc26b6b7bccf0eea2e2c1a9ebdcc9d30ca7390d9 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Tue, 3 Jun 2014 14:05:16 -0700 Subject: Routes specifying 'to:' must be a string that contains a "#" or a rack application. Use of a symbol should be replaced with `action: symbol`. Use of a string without a "#" should be replaced with `controller: string`. --- actionpack/CHANGELOG.md | 4 ++ actionpack/aaron_path.dot | 44 ++++++++++++++++++ actionpack/ebi_path.dot | 48 ++++++++++++++++++++ actionpack/lib/action_dispatch/routing/mapper.rb | 24 ++++++++-- .../request/multipart_params_parsing_test.rb | 4 +- actionpack/test/dispatch/routing_test.rb | 52 +++++++++++++--------- 6 files changed, 149 insertions(+), 27 deletions(-) create mode 100644 actionpack/aaron_path.dot create mode 100644 actionpack/ebi_path.dot (limited to 'actionpack') diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 6a50565de5..fb633f11d5 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,7 @@ +* Routes specifying 'to:' must be a string that contains a "#" or a rack + application. Use of a symbol should be replaced with `action: symbol`. + Use of a string without a "#" should be replaced with `controller: string`. + * Deprecate all *_filter callbacks in favor of *_action callbacks. *Rafael Mendonça França* diff --git a/actionpack/aaron_path.dot b/actionpack/aaron_path.dot new file mode 100644 index 0000000000..bee01a9a9d --- /dev/null +++ b/actionpack/aaron_path.dot @@ -0,0 +1,44 @@ + digraph parse_tree { + size="8,5" + node [shape = none]; + edge [dir = none]; + 70293339236320 [label=""]; +70293339236820 [label=""]; +70293339237000 [label=""]; +70293339237340 [label=""]; +70293339239200 [label=""]; +70293339239520 [label="/"]; +70293339239300 [label="foo"]; +70293339237420 [label="()"]; +70293339237520 [label=""]; +70293339238380 [label=""]; +70293339238820 [label="/"]; +70293339238520 [label=":bar"]; +70293339237620 [label="()"]; +70293339237860 [label=":baz"]; +70293339237080 [label="/"]; +70293339236920 [label=":aaron"]; +70293339236360 [label="()"]; +70293339236520 [label=""]; +70293339236700 [label="."]; +70293339236560 [label=":format"]; + 70293339236320 -> 70293339236820; +70293339236320 -> 70293339236360; +70293339236820 -> 70293339237000; +70293339236820 -> 70293339236920; +70293339237000 -> 70293339237340; +70293339237000 -> 70293339237080; +70293339237340 -> 70293339239200; +70293339237340 -> 70293339237420; +70293339239200 -> 70293339239520; +70293339239200 -> 70293339239300; +70293339237420 -> 70293339237520; +70293339237520 -> 70293339238380; +70293339237520 -> 70293339237620; +70293339238380 -> 70293339238820; +70293339238380 -> 70293339238520; +70293339237620 -> 70293339237860; +70293339236360 -> 70293339236520; +70293339236520 -> 70293339236700; +70293339236520 -> 70293339236560; + } diff --git a/actionpack/ebi_path.dot b/actionpack/ebi_path.dot new file mode 100644 index 0000000000..258192b908 --- /dev/null +++ b/actionpack/ebi_path.dot @@ -0,0 +1,48 @@ + digraph parse_tree { + size="8,5" + node [shape = none]; + edge [dir = none]; + 70293330639600 [label=""]; +70293330640700 [label=""]; +70293330655800 [label=""]; +70293330649320 [label=""]; +70293330650540 [label=""]; +70293330650820 [label="/"]; +70293330650620 [label="foo"]; +70293330649420 [label="()"]; +70293330649540 [label=""]; +70293330650120 [label=""]; +70293330650340 [label="/"]; +70293330650180 [label=":bar"]; +70293330649600 [label="()"]; +70293330649720 [label=""]; +70293330649920 [label="/"]; +70293330649760 [label=":baz"]; +70293330655220 [label="/"]; +70293330657120 [label=":aaron"]; +70293330639740 [label="()"]; +70293330639980 [label=""]; +70293330640400 [label="."]; +70293330640100 [label=":format"]; + 70293330639600 -> 70293330640700; +70293330639600 -> 70293330639740; +70293330640700 -> 70293330655800; +70293330640700 -> 70293330657120; +70293330655800 -> 70293330649320; +70293330655800 -> 70293330655220; +70293330649320 -> 70293330650540; +70293330649320 -> 70293330649420; +70293330650540 -> 70293330650820; +70293330650540 -> 70293330650620; +70293330649420 -> 70293330649540; +70293330649540 -> 70293330650120; +70293330649540 -> 70293330649600; +70293330650120 -> 70293330650340; +70293330650120 -> 70293330650180; +70293330649600 -> 70293330649720; +70293330649720 -> 70293330649920; +70293330649720 -> 70293330649760; +70293330639740 -> 70293330639980; +70293330639980 -> 70293330640400; +70293330639980 -> 70293330640100; + } diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 0b13738dcb..a32e4ee0d1 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -7,6 +7,7 @@ require 'active_support/core_ext/module/remove_method' require 'active_support/inflector' require 'action_dispatch/routing/redirection' require 'action_dispatch/routing/endpoint' +require 'active_support/deprecation' module ActionDispatch module Routing @@ -284,9 +285,13 @@ module ActionDispatch def get_controller_and_action(controller, action, to, modyoule) case to - when Symbol then action = to.to_s + when Symbol + ActiveSupport::Deprecation.warn "defining a route where `to` is a symbol is deprecated. Please change \"to: :#{to}\" to \"action: :#{to}\"" + action = to.to_s when /#/ then controller, action = to.split('#') - when String then controller = to + when String + ActiveSupport::Deprecation.warn "defining a route where `to` is a controller without an action is deprecated. Please change \"to: :#{to}\" to \"controller: :#{to}\"" + controller = to end if modyoule && !controller.is_a?(Regexp) @@ -1458,7 +1463,20 @@ module ActionDispatch if rest.empty? && Hash === path options = path path, to = options.find { |name, _value| name.is_a?(String) } - options[:to] = to + + case to + when Symbol + options[:action] = to + when String + if to =~ /#/ + options[:to] = to + else + options[:controller] = to + end + else + options[:to] = to + end + options.delete(path) paths = [path] else diff --git a/actionpack/test/dispatch/request/multipart_params_parsing_test.rb b/actionpack/test/dispatch/request/multipart_params_parsing_test.rb index 2a2f92b5b3..2db3fee6bb 100644 --- a/actionpack/test/dispatch/request/multipart_params_parsing_test.rb +++ b/actionpack/test/dispatch/request/multipart_params_parsing_test.rb @@ -145,7 +145,7 @@ class MultipartParamsParsingTest < ActionDispatch::IntegrationTest test "does not raise EOFError on GET request with multipart content-type" do with_routing do |set| set.draw do - get ':action', to: 'multipart_params_parsing_test/test' + get ':action', controller: 'multipart_params_parsing_test/test' end headers = { "CONTENT_TYPE" => "multipart/form-data; boundary=AaB03x" } get "/parse", {}, headers @@ -174,7 +174,7 @@ class MultipartParamsParsingTest < ActionDispatch::IntegrationTest def with_test_routing with_routing do |set| set.draw do - post ':action', :to => 'multipart_params_parsing_test/test' + post ':action', :controller => 'multipart_params_parsing_test/test' end yield end diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index ba9d015258..778dbfc74d 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -361,8 +361,8 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest draw do controller(:global) do get 'global/hide_notice' - get 'global/export', :to => :export, :as => :export_request - get '/export/:id/:file', :to => :export, :as => :export_download, :constraints => { :file => /.*/ } + get 'global/export', :action => :export, :as => :export_request + get '/export/:id/:file', :action => :export, :as => :export_download, :constraints => { :file => /.*/ } get 'global/:action' end end @@ -730,8 +730,8 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest draw do resources :replies do member do - put :answer, :to => :mark_as_answer - delete :answer, :to => :unmark_as_answer + put :answer, :action => :mark_as_answer + delete :answer, :action => :unmark_as_answer end end end @@ -1188,7 +1188,7 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest controller :articles do scope '/articles', :as => 'article' do scope :path => '/:title', :title => /[a-z]+/, :as => :with_title do - get '/:id', :to => :with_id, :as => "" + get '/:id', :action => :with_id, :as => "" end end end @@ -1435,7 +1435,7 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest def test_scoped_controller_with_namespace_and_action draw do namespace :account do - get ':action/callback', :action => /twitter|github/, :to => "callbacks", :as => :callback + get ':action/callback', :action => /twitter|github/, :controller => "callbacks", :as => :callback end end @@ -1492,7 +1492,7 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest def test_normalize_namespaced_matches draw do namespace :account do - get 'description', :to => :description, :as => "description" + get 'description', :action => :description, :as => "description" end end @@ -2154,7 +2154,7 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end resources :invoices do get "outstanding" => "invoices#outstanding", :on => :collection - get "overdue", :to => :overdue, :on => :collection + get "overdue", :action => :overdue, :on => :collection get "print" => "invoices#print", :as => :print, :on => :member post "preview" => "invoices#preview", :as => :preview, :on => :new end @@ -2996,7 +2996,9 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end assert_raise(ArgumentError) do - draw { controller("/feeds") { get '/feeds/:service', :to => :show } } + assert_deprecated do + draw { controller("/feeds") { get '/feeds/:service', :to => :show } } + end end assert_raise(ArgumentError) do @@ -3284,20 +3286,24 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_mix_symbol_to_controller_action - draw do - get '/projects', controller: 'project_files', - action: 'index', - to: :show + assert_deprecated do + draw do + get '/projects', controller: 'project_files', + action: 'index', + to: :show + end end get '/projects' assert_equal 'project_files#show', @response.body end - def test_mix_string_to_controller_action - draw do - get '/projects', controller: 'project_files', - action: 'index', - to: 'show' + def test_mix_string_to_controller_action_no_hash + assert_deprecated do + draw do + get '/projects', controller: 'project_files', + action: 'index', + to: 'show' + end end get '/projects' assert_equal 'show#index', @response.body @@ -3567,7 +3573,7 @@ class TestNamespaceWithControllerOption < ActionDispatch::IntegrationTest def test_missing_controller ex = assert_raises(ArgumentError) { draw do - get '/foo/bar', :to => :index + get '/foo/bar', :action => :index end } assert_match(/Missing :controller/, ex.message) @@ -3575,8 +3581,10 @@ class TestNamespaceWithControllerOption < ActionDispatch::IntegrationTest def test_missing_action ex = assert_raises(ArgumentError) { - draw do - get '/foo/bar', :to => 'foo' + assert_deprecated do + draw do + get '/foo/bar', :to => 'foo' + end end } assert_match(/Missing :action/, ex.message) @@ -4083,7 +4091,7 @@ class TestInvalidUrls < ActionDispatch::IntegrationTest set.draw do get "/bar/:id", :to => redirect("/foo/show/%{id}") get "/foo/show(/:id)", :to => "test_invalid_urls/foo#show" - get "/foo(/:action(/:id))", :to => "test_invalid_urls/foo" + get "/foo(/:action(/:id))", :controller => "test_invalid_urls/foo" get "/:controller(/:action(/:id))" end -- cgit v1.2.3