Skip to content

Commit

Permalink
Routes specifying 'to:' must be a string that contains a "#" or a rack
Browse files Browse the repository at this point in the history
application.  Use of a symbol should be replaced with `action: symbol`.
Use of a string without a "#" should be replaced with `controller: string`.
  • Loading branch information
tenderlove committed Jun 3, 2014
1 parent 9b09e60 commit cc26b6b
Show file tree
Hide file tree
Showing 6 changed files with 149 additions and 27 deletions.
4 changes: 4 additions & 0 deletions actionpack/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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*
Expand Down
44 changes: 44 additions & 0 deletions actionpack/aaron_path.dot
Original file line number Diff line number Diff line change
@@ -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;
}
48 changes: 48 additions & 0 deletions actionpack/ebi_path.dot
Original file line number Diff line number Diff line change
@@ -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;
}
24 changes: 21 additions & 3 deletions actionpack/lib/action_dispatch/routing/mapper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
require 'active_support/inflector'
require 'action_dispatch/routing/redirection'
require 'action_dispatch/routing/endpoint'
require 'active_support/deprecation'

module ActionDispatch
module Routing
Expand Down Expand Up @@ -284,9 +285,13 @@ def check_part(name, part, path_params, hash)

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)
Expand Down Expand Up @@ -1458,7 +1463,20 @@ def match(path, *rest)
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ def teardown
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
Expand Down Expand Up @@ -174,7 +174,7 @@ def parse_multipart(name)
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
Expand Down
52 changes: 30 additions & 22 deletions actionpack/test/dispatch/routing_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -361,8 +361,8 @@ def test_global
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
Expand Down Expand Up @@ -730,8 +730,8 @@ def test_replies
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
Expand Down Expand Up @@ -1188,7 +1188,7 @@ def test_articles_with_id
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
Expand Down Expand Up @@ -1435,7 +1435,7 @@ def test_dynamically_generated_helpers_on_collection_do_not_clobber_resources_ur
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

Expand Down Expand Up @@ -1492,7 +1492,7 @@ def test_redirect_with_port
def test_normalize_namespaced_matches
draw do
namespace :account do
get 'description', :to => :description, :as => "description"
get 'description', :action => :description, :as => "description"
end
end

Expand Down Expand Up @@ -2154,7 +2154,7 @@ def test_custom_resource_routes_are_scoped
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
Expand Down Expand Up @@ -2996,7 +2996,9 @@ def test_controller_name_with_leading_slash_raise_error
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
Expand Down Expand Up @@ -3284,20 +3286,24 @@ def test_mix_string_to_action
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
Expand Down Expand Up @@ -3567,16 +3573,18 @@ def draw(&block)
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)
end

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)
Expand Down Expand Up @@ -4083,7 +4091,7 @@ def show
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

Expand Down

0 comments on commit cc26b6b

Please sign in to comment.