Ticket #8558: fix_resource_route_names.edge.diff

actionpack/test/controller/routing_test.rb

@@ -22 +22 @@
-      map.connect ':controller/:action/:variable'
-    end
-
- , ;), %w(^ / ? # [ ]
+), %w(^ / ? # [ ] , ;
-    hex = unsafe.map { |char| '%' + char.unpack('H2').first.upcase }
-
-    @segment = "#{safe}#{unsafe}".freeze
@@ -1024 +1024 @@
-    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 +1177 @@
-    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?

actionpack/test/controller/resources_test.rb

@@ -149 +149 @@
-  
-      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 +167 @@
-  
-      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 +230 @@
-      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 +244 @@
-      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 +309 @@
-    end
-  end
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
+
+
+
+
+
+
+
+
+
+
+
-
-  def test_should_create_singleton_resource_routes
-    with_singleton_resources :account do
@@ -471 +459 @@
-    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 +656 @@
-      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 +713 @@
-      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 +724 @@
-      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|

actionpack/lib/action_controller/base.rb

@@ -308 +308 @@
-    # 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
@@ -603 +607 @@
-      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

actionpack/lib/action_controller/resources.rb

@@ -94 +94 @@
-        "#{name_prefix}#{singular}_"
-      end
-
+      def action_separator
+        @action_separator ||= Base.resource_action_separator
+      end
-
-      protected
-        def arrange_actions
@@ -431 +434 @@
-        resource.collection_methods.each do |method, actions|
-          actions.each do |action|
-            action_options = action_options_for(action, resource, method)
-
-
-
-
+
+
-          end
-        end
-      end
@@ -460 +461 @@
-          actions.each do |action|
-            action_options = action_options_for(action, resource, method)
-            if action == :new
-
-
-
+
+
-            else
-
-
-
+
+
-            end
-          end
-        end
@@ -476 +475 @@
-        resource.member_methods.each do |method, actions|
-          actions.each do |action|
-            action_options = action_options_for(action, resource, method)
-
-
-
+
+
-          end
-        end
-

actionpack/lib/action_controller/routing.rb

@@ -247 +247 @@
-  #  end
-  #
-  module Routing
-.
+; . ,
-
-    HTTP_METHODS = [:get, :head, :post, :put, :delete]
-
@@ -548 +548 @@
-    end
-
-    class Segment #:nodoc:
-,;
+
-      UNSAFE_PCHAR = Regexp.new("[^#{URI::REGEXP::PATTERN::UNRESERVED}#{RESERVED_PCHAR}]", false, 'N').freeze
-
-      attr_accessor :is_optional
@@ -561 +561 @@
-      def extraction_code
-        nil
-      end
-
+
-      # Continue generating string for the prior segments.
-      def continue_string_structure(prior_segments)
-        if prior_segments.empty?
@@ -1020 +1020 @@
-          @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 +1052 @@
-      class NamedRouteCollection #:nodoc:
-        include Enumerable
-
-, :deprecated_named_routes
+
-
-        def initialize
-          clear!
@@ -1066 +1061 @@
-        def clear!
-          @routes = {}
-          @helpers = []
-          @deprecated_named_routes = {}
-          
-          @module ||= Module.new
-          @module.instance_methods.each do |selector|
@@ -1080 +1074 @@
-        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
-
@@ -1239 +1227 @@
-        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