Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Remove deprecated named routes [pixeltrix]

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@7415 5ecf4fe2-1ee6-0310-87b1-e25e094e27de
  • Loading branch information...
commit 68d685056a6a8a6fcb7f922c161d26a54c770213 1 parent 0e6c8e5
Tobias Lütke authored
View
8 actionpack/lib/action_controller/base.rb
@@ -310,6 +310,10 @@ class Base
# Turn on +ignore_missing_templates+ if you want to unit test actions without making the associated templates.
cattr_accessor :ignore_missing_templates
+ # Controls the resource action separator
+ @@resource_action_separator = "/"
+ cattr_accessor :resource_action_separator
+
# Holds the request object that's primarily used to get environment variables through access like
# <tt>request.env["REQUEST_URI"]</tt>.
attr_internal :request
@@ -605,11 +609,11 @@ def controller_name
def controller_path
self.class.controller_path
end
-
+
def session_enabled?
request.session_options && request.session_options[:disabled] != false
end
-
+
# View load paths for controller.
def view_paths
self.class.view_paths
View
24 actionpack/lib/action_controller/resources.rb
@@ -94,6 +94,9 @@ def nesting_name_prefix
"#{name_prefix}#{singular}_"
end
+ def action_separator
+ @action_separator ||= Base.resource_action_separator
+ end
protected
def arrange_actions
@@ -431,10 +434,8 @@ def map_collection_actions(map, resource)
resource.collection_methods.each do |method, actions|
actions.each do |action|
action_options = action_options_for(action, resource, method)
-
- # See http://dev.rubyonrails.org/ticket/8251
- map.deprecated_named_route("#{action}_#{resource.name_prefix}#{resource.plural}", "#{resource.name_prefix}#{action}_#{resource.plural}", "#{resource.path}/#{action}", action_options)
- map.deprecated_named_route("formatted_#{action}_#{resource.name_prefix}#{resource.plural}", "formatted_#{resource.name_prefix}#{action}_#{resource.plural}", "#{resource.path}/#{action}.:format", action_options)
+ map.named_route("#{action}_#{resource.name_prefix}#{resource.plural}", "#{resource.path}#{resource.action_separator}#{action}", action_options)
+ map.named_route("formatted_#{action}_#{resource.name_prefix}#{resource.plural}", "#{resource.path}#{resource.action_separator}#{action}.:format", action_options)
end
end
end
@@ -460,13 +461,11 @@ def map_new_actions(map, resource)
actions.each do |action|
action_options = action_options_for(action, resource, method)
if action == :new
- # See http://dev.rubyonrails.org/ticket/8251
- map.deprecated_named_route("new_#{resource.name_prefix}#{resource.singular}", "#{resource.name_prefix}new_#{resource.singular}", resource.new_path, action_options)
- map.deprecated_named_route("formatted_new_#{resource.name_prefix}#{resource.singular}", "formatted_#{resource.name_prefix}new_#{resource.singular}", "#{resource.new_path}.:format", action_options)
+ map.named_route("new_#{resource.name_prefix}#{resource.singular}", resource.new_path, action_options)
+ map.named_route("formatted_new_#{resource.name_prefix}#{resource.singular}", "#{resource.new_path}.:format", action_options)
else
- # See http://dev.rubyonrails.org/ticket/8251
- map.deprecated_named_route("#{action}_new_#{resource.name_prefix}#{resource.singular}", "#{resource.name_prefix}#{action}_new_#{resource.singular}", "#{resource.new_path}/#{action}", action_options)
- map.deprecated_named_route("formatted_#{action}_new_#{resource.name_prefix}#{resource.singular}", "formatted_#{resource.name_prefix}#{action}_new_#{resource.singular}", "#{resource.new_path}/#{action}.:format", action_options)
+ map.named_route("#{action}_new_#{resource.name_prefix}#{resource.singular}", "#{resource.new_path}#{resource.action_separator}#{action}", action_options)
+ map.named_route("formatted_#{action}_new_#{resource.name_prefix}#{resource.singular}", "#{resource.new_path}#{resource.action_separator}#{action}.:format", action_options)
end
end
end
@@ -476,9 +475,8 @@ def map_member_actions(map, resource)
resource.member_methods.each do |method, actions|
actions.each do |action|
action_options = action_options_for(action, resource, method)
- # See http://dev.rubyonrails.org/ticket/8251
- map.deprecated_named_route("#{action}_#{resource.name_prefix}#{resource.singular}", "#{resource.name_prefix}#{action}_#{resource.singular}", "#{resource.member_path}/#{action}", action_options)
- map.deprecated_named_route("formatted_#{action}_#{resource.name_prefix}#{resource.singular}", "formatted_#{resource.name_prefix}#{action}_#{resource.singular}", "#{resource.member_path}/#{action}.:format",action_options)
+ map.named_route("#{action}_#{resource.name_prefix}#{resource.singular}", "#{resource.member_path}#{resource.action_separator}#{action}", action_options)
+ map.named_route("formatted_#{action}_#{resource.name_prefix}#{resource.singular}", "#{resource.member_path}#{resource.action_separator}#{action}.:format",action_options)
end
end
View
25 actionpack/lib/action_controller/routing.rb
@@ -247,7 +247,7 @@ module ActionController
# end
#
module Routing
- SEPARATORS = %w( / . ? )
+ SEPARATORS = %w( / ; . , ? )
HTTP_METHODS = [:get, :head, :post, :put, :delete]
@@ -548,7 +548,7 @@ def requirement_for(key)
end
class Segment #:nodoc:
- RESERVED_PCHAR = ':@&=+$,;'
+ RESERVED_PCHAR = ':@&=+$'
UNSAFE_PCHAR = Regexp.new("[^#{URI::REGEXP::PATTERN::UNRESERVED}#{RESERVED_PCHAR}]", false, 'N').freeze
attr_accessor :is_optional
@@ -561,7 +561,7 @@ def initialize
def extraction_code
nil
end
-
+
# Continue generating string for the prior segments.
def continue_string_structure(prior_segments)
if prior_segments.empty?
@@ -1020,11 +1020,6 @@ def named_route(name, path, options = {})
@set.add_named_route(name, path, options)
end
- def deprecated_named_route(name, deprecated_name, path, options = {})
- named_route(name, path, options)
- @set.add_deprecated_named_route(name, deprecated_name, path, options) unless deprecated_name == name
- end
-
# Enables the use of resources in a module by setting the name_prefix, path_prefix, and namespace for the model.
# Example:
#
@@ -1057,7 +1052,7 @@ def method_missing(route_name, *args, &proc)
class NamedRouteCollection #:nodoc:
include Enumerable
- attr_reader :routes, :helpers, :deprecated_named_routes
+ attr_reader :routes, :helpers
def initialize
clear!
@@ -1066,7 +1061,6 @@ def initialize
def clear!
@routes = {}
@helpers = []
- @deprecated_named_routes = {}
@module ||= Module.new
@module.instance_methods.each do |selector|
@@ -1080,12 +1074,6 @@ def add(name, route)
end
def get(name)
- if @deprecated_named_routes.has_key?(name.to_sym)
- ActiveSupport::Deprecation.warn(
- "The named route \"#{name}\" uses a format that has been deprecated. " +
- "You should use \"#{@deprecated_named_routes[name]}\" instead", caller
- )
- end
routes[name.to_sym]
end
@@ -1259,11 +1247,6 @@ def add_named_route(name, path, options = {})
named_routes[name.to_sym] = add_route(path, options)
end
- def add_deprecated_named_route(name, deprecated_name, path, options = {})
- add_named_route(deprecated_name, path, options)
- named_routes.deprecated_named_routes[deprecated_name.to_sym] = name
- end
-
def options_as_params(options)
# If an explicit :controller was given, always make :action explicit
# too, so that action expiry works as expected for things like
View
104 actionpack/test/controller/resources_test.rb
@@ -149,7 +149,6 @@ def test_with_collection_actions_and_name_prefix
assert_restful_named_routes_for :messages, :path_prefix => 'threads/1/', :name_prefix => 'thread_', :options => { :thread_id => '1' } do |options|
actions.keys.each do |action|
- assert_deprecated_named_route /thread_#{action}_messages/, "/threads/1/messages/#{action}", "thread_#{action}_messages_path", :action => action
assert_named_route "/threads/1/messages/#{action}", "#{action}_thread_messages_path", :action => action
end
end
@@ -168,7 +167,6 @@ def test_with_collection_action_and_name_prefix_and_formatted
assert_restful_named_routes_for :messages, :path_prefix => 'threads/1/', :name_prefix => 'thread_', :options => { :thread_id => '1' } do |options|
actions.keys.each do |action|
- assert_deprecated_named_route /formatted_thread_#{action}_messages/, "/threads/1/messages/#{action}.xml", "formatted_thread_#{action}_messages_path", :action => action, :format => 'xml'
assert_named_route "/threads/1/messages/#{action}.xml", "formatted_#{action}_thread_messages_path", :action => action, :format => 'xml'
end
end
@@ -232,7 +230,6 @@ def test_with_new_action_with_name_prefix
end
assert_restful_named_routes_for :messages, :path_prefix => 'threads/1/', :name_prefix => 'thread_', :options => { :thread_id => '1' } do |options|
- assert_deprecated_named_route /thread_preview_new_message/, preview_path, :thread_preview_new_message_path, preview_options
assert_named_route preview_path, :preview_new_thread_message_path, preview_options
end
end
@@ -247,7 +244,6 @@ def test_with_formatted_new_action_with_name_prefix
end
assert_restful_named_routes_for :messages, :path_prefix => 'threads/1/', :name_prefix => 'thread_', :options => { :thread_id => '1' } do |options|
- assert_deprecated_named_route /formatted_thread_preview_new_message/, preview_path, :formatted_thread_preview_new_message_path, preview_options
assert_named_route preview_path, :formatted_preview_new_thread_message_path, preview_options
end
end
@@ -313,25 +309,17 @@ def test_nested_restful_routes_with_overwritten_defaults
end
end
- # NOTE from dchelimsky: http://dev.rubyonrails.org/ticket/8251 changes some of the
- # named routes. In order to play nice, I didn't delete the old ones, which means
- # that this test fails because until the old ones are deprecated and/or removed, there
- # must be room for two names for the same route.
- #
- # From what I can tell this shouldn't matter - all the other
- # tests in actionpack pass without this one passing. But I could be wrong ;)
- #
- # def test_restful_routes_dont_generate_duplicates
- # with_restful_routing :messages do
- # routes = ActionController::Routing::Routes.routes
- # routes.each do |route|
- # routes.each do |r|
- # next if route === r # skip the comparison instance
- # assert distinct_routes?(route, r), "Duplicate Route: #{route}"
- # end
- # end
- # end
- # end
+ def test_restful_routes_dont_generate_duplicates
+ with_restful_routing :messages do
+ routes = ActionController::Routing::Routes.routes
+ routes.each do |route|
+ routes.each do |r|
+ next if route === r # skip the comparison instance
+ assert distinct_routes?(route, r), "Duplicate Route: #{route}"
+ end
+ end
+ end
+ end
def test_should_create_singleton_resource_routes
with_singleton_resources :account do
@@ -471,6 +459,50 @@ def test_should_not_allow_delete_or_put_on_collection_path
end
end
+ def test_resource_action_separator
+ with_routing do |set|
+ set.draw do |map|
+ map.resources :messages, :collection => {:search => :get}, :new => {:preview => :any}, :name_prefix => 'thread_', :path_prefix => '/threads/:thread_id'
+ map.resource :account, :member => {:login => :get}, :new => {:preview => :any}, :name_prefix => 'admin_', :path_prefix => '/admin'
+ end
+
+ action_separator = ActionController::Base.resource_action_separator
+
+ assert_simply_restful_for :messages, :name_prefix => 'thread_', :path_prefix => 'threads/1/', :options => { :thread_id => '1' }
+ assert_named_route "/threads/1/messages#{action_separator}search", "search_thread_messages_path", {}
+ assert_named_route "/threads/1/messages/new", "new_thread_message_path", {}
+ assert_named_route "/threads/1/messages/new#{action_separator}preview", "preview_new_thread_message_path", {}
+ assert_singleton_restful_for :account, :name_prefix => 'admin_', :path_prefix => 'admin/'
+ assert_named_route "/admin/account#{action_separator}login", "login_admin_account_path", {}
+ assert_named_route "/admin/account/new", "new_admin_account_path", {}
+ assert_named_route "/admin/account/new#{action_separator}preview", "preview_new_admin_account_path", {}
+ end
+ end
+
+ def test_new_style_named_routes_for_resource
+ with_routing do |set|
+ set.draw do |map|
+ map.resources :messages, :collection => {:search => :get}, :new => {:preview => :any}, :name_prefix => 'thread_', :path_prefix => '/threads/:thread_id'
+ end
+ assert_simply_restful_for :messages, :name_prefix => 'thread_', :path_prefix => 'threads/1/', :options => { :thread_id => '1' }
+ assert_named_route "/threads/1/messages/search", "search_thread_messages_path", {}
+ assert_named_route "/threads/1/messages/new", "new_thread_message_path", {}
+ assert_named_route "/threads/1/messages/new/preview", "preview_new_thread_message_path", {}
+ end
+ end
+
+ def test_new_style_named_routes_for_singleton_resource
+ with_routing do |set|
+ set.draw do |map|
+ map.resource :account, :member => {:login => :get}, :new => {:preview => :any}, :name_prefix => 'admin_', :path_prefix => '/admin'
+ end
+ assert_singleton_restful_for :account, :name_prefix => 'admin_', :path_prefix => 'admin/'
+ assert_named_route "/admin/account/login", "login_admin_account_path", {}
+ assert_named_route "/admin/account/new", "new_admin_account_path", {}
+ assert_named_route "/admin/account/new/preview", "preview_new_admin_account_path", {}
+ end
+ end
+
def test_resources_in_namespace
with_routing do |set|
set.draw do |map|
@@ -624,11 +656,6 @@ def assert_restful_named_routes_for(controller_name, singular_name = nil, option
assert_named_route "#{full_prefix}/1", "#{name_prefix}#{singular_name}_path", options[:options].merge(:id => '1')
assert_named_route "#{full_prefix}/1.xml", "formatted_#{name_prefix}#{singular_name}_path", options[:options].merge(:id => '1', :format => 'xml')
- assert_potentially_deprecated_named_route name_prefix, /#{name_prefix}new_#{singular_name}/, "#{full_prefix}/new", "#{name_prefix}new_#{singular_name}_path", options[:options]
- assert_potentially_deprecated_named_route name_prefix, /formatted_#{name_prefix}new_#{singular_name}/, "#{full_prefix}/new.xml", "formatted_#{name_prefix}new_#{singular_name}_path", options[:options].merge( :format => 'xml')
- assert_potentially_deprecated_named_route name_prefix, /#{name_prefix}edit_#{singular_name}/, "#{full_prefix}/1/edit", "#{name_prefix}edit_#{singular_name}_path", options[:options].merge(:id => '1')
- assert_potentially_deprecated_named_route name_prefix, /formatted_#{name_prefix}edit_#{singular_name}/, "#{full_prefix}/1/edit.xml", "formatted_#{name_prefix}edit_#{singular_name}_path", options[:options].merge(:id => '1', :format => 'xml')
-
assert_named_route "#{full_prefix}/new", "new_#{name_prefix}#{singular_name}_path", options[:options]
assert_named_route "#{full_prefix}/new.xml", "formatted_new_#{name_prefix}#{singular_name}_path", options[:options].merge( :format => 'xml')
assert_named_route "#{full_prefix}/1/edit", "edit_#{name_prefix}#{singular_name}_path", options[:options].merge(:id => '1')
@@ -686,11 +713,6 @@ def assert_singleton_named_routes_for(singleton_name, options = {})
assert_named_route "#{full_path}", "#{name_prefix}#{singleton_name}_path", options[:options]
assert_named_route "#{full_path}.xml", "formatted_#{name_prefix}#{singleton_name}_path", options[:options].merge(:format => 'xml')
- assert_potentially_deprecated_named_route name_prefix, /#{name_prefix}new_#{singleton_name}/, "#{full_path}/new", "#{name_prefix}new_#{singleton_name}_path", options[:options]
- assert_potentially_deprecated_named_route name_prefix, /formatted_#{name_prefix}new_#{singleton_name}/, "#{full_path}/new.xml", "formatted_#{name_prefix}new_#{singleton_name}_path", options[:options].merge(:format => 'xml')
- assert_potentially_deprecated_named_route name_prefix, /#{name_prefix}edit_#{singleton_name}/, "#{full_path}/edit", "#{name_prefix}edit_#{singleton_name}_path", options[:options]
- assert_potentially_deprecated_named_route name_prefix, /formatted_#{name_prefix}edit_#{singleton_name}/, "#{full_path}/edit.xml", "formatted_#{name_prefix}edit_#{singleton_name}_path", options[:options].merge(:format => 'xml')
-
assert_named_route "#{full_path}/new", "new_#{name_prefix}#{singleton_name}_path", options[:options]
assert_named_route "#{full_path}/new.xml", "formatted_new_#{name_prefix}#{singleton_name}_path", options[:options].merge(:format => 'xml')
assert_named_route "#{full_path}/edit", "edit_#{name_prefix}#{singleton_name}_path", options[:options]
@@ -702,22 +724,6 @@ def assert_named_route(expected, route, options)
assert_equal expected, actual, "Error on route: #{route}(#{options.inspect})"
end
- def assert_deprecated_named_route(message, expected, route, options)
- assert_deprecated message do
- assert_named_route expected, route, options
- end
- end
-
- # These are only deprecated if they have a name_prefix.
- # See http://dev.rubyonrails.org/ticket/8251
- def assert_potentially_deprecated_named_route(name_prefix, message, expected, route, options)
- if name_prefix
- assert_deprecated_named_route(message, expected, route, options)
- else
- assert_named_route expected, route, options
- end
- end
-
def assert_resource_methods(expected, resource, action_method, method)
assert_equal expected.length, resource.send("#{action_method}_methods")[method].size, "#{resource.send("#{action_method}_methods")[method].inspect}"
expected.each do |action|
View
54 actionpack/test/controller/routing_test.rb
@@ -22,7 +22,7 @@ def setup
map.connect ':controller/:action/:variable'
end
- safe, unsafe = %w(: @ & = + $ , ;), %w(^ / ? # [ ])
+ safe, unsafe = %w(: @ & = + $), %w(^ / ? # [ ] , ;)
hex = unsafe.map { |char| '%' + char.unpack('H2').first.upcase }
@segment = "#{safe}#{unsafe}".freeze
@@ -1024,42 +1024,6 @@ def order_query_string(qs)
end
end
-class MapperDeprecatedRouteTest < Test::Unit::TestCase
- def setup
- @set = mock("set")
- @mapper = ActionController::Routing::RouteSet::Mapper.new(@set)
- end
-
- def test_should_add_new_and_deprecated_named_routes
- @set.expects(:add_named_route).with("new", "path", {:a => "b"})
- @set.expects(:add_deprecated_named_route).with("new", "old", "path", {:a => "b"})
- @mapper.deprecated_named_route("new", "old", "path", {:a => "b"})
- end
-
- def test_should_not_add_deprecated_named_route_if_both_are_the_same
- @set.expects(:add_named_route).with("new", "path", {:a => "b"})
- @set.expects(:add_deprecated_named_route).with("new", "new", "path", {:a => "b"}).never
- @mapper.deprecated_named_route("new", "new", "path", {:a => "b"})
- end
-end
-
-class RouteSetDeprecatedRouteTest < Test::Unit::TestCase
- def setup
- @set = ActionController::Routing::RouteSet.new
- end
-
- def test_should_add_deprecated_route
- @set.expects(:add_named_route).with("old", "path", {:a => "b"})
- @set.add_deprecated_named_route("new", "old", "path", {:a => "b"})
- end
-
- def test_should_fire_deprecation_warning_on_access
- @set.add_deprecated_named_route("new_outer_inner_path", "outer_new_inner_path", "/outers/:outer_id/inners/new", :controller => :inners)
- ActiveSupport::Deprecation.expects(:warn)
- @set.named_routes["outer_new_inner_path"]
- end
-end
-
end # uses_mocha
class RouteBuilderTest < Test::Unit::TestCase
@@ -1213,22 +1177,6 @@ def test_optional_segments_preceding_required_segments
assert_equal nil, builder.warn_output # should only warn on the :person segment
end
- def test_comma_isnt_a_route_separator
- segments = builder.segments_for_route_path '/books/:id,:action'
- defaults = { :action => 'show' }
- assert_raise(ArgumentError) do
- builder.assign_route_options(segments, defaults, {})
- end
- end
-
- def test_semicolon_isnt_a_route_separator
- segments = builder.segments_for_route_path '/books/:id;:action'
- defaults = { :action => 'show' }
- assert_raise(ArgumentError) do
- builder.assign_route_options(segments, defaults, {})
- end
- end
-
def test_segmentation_of_dot_path
segments = builder.segments_for_route_path '/books/:action.rss'
assert builder.assign_route_options(segments, {}, {}).empty?
Please sign in to comment.
Something went wrong with that request. Please try again.