Remove deprecated named routes [pixeltrix]

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@7415 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

2 files changed, 56 insertions, 102 deletions

diff --git a/actionpack/test/controller/resources_test.rb b/actionpack/test/controller/resources_test.rb
index c3e0836205..57b956e777 100644
--- a/actionpack/test/controller/resources_test.rb
+++ b/actionpack/test/controller/resources_test.rb
@@ -149,7 +149,6 @@ class ResourcesTest < Test::Unit::TestCase
   
       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 @@ class ResourcesTest < Test::Unit::TestCase
   
       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 @@ class ResourcesTest < Test::Unit::TestCase
       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 @@ class ResourcesTest < Test::Unit::TestCase
       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 @@ class ResourcesTest < Test::Unit::TestCase
     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 @@ class ResourcesTest < Test::Unit::TestCase
     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 @@ class ResourcesTest < Test::Unit::TestCase
       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 @@ class ResourcesTest < Test::Unit::TestCase
       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 @@ class ResourcesTest < Test::Unit::TestCase
       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|
diff --git a/actionpack/test/controller/routing_test.rb b/actionpack/test/controller/routing_test.rb
index 858ec6f3d1..5ca63396b4 100644
--- a/actionpack/test/controller/routing_test.rb
+++ b/actionpack/test/controller/routing_test.rb
@@ -22,7 +22,7 @@ class UriReservedCharactersRoutingTest < Test::Unit::TestCase
       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 @@ class RouteTest < Test::Unit::TestCase
     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 @@ class RouteBuilderTest < Test::Unit::TestCase
     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?