Ticket #7229: put_and_delete_requires_id.diff

test/controller/resources_test.rb

@@ -27 +27 @@
-    assert_resource_methods [:new, :preview, :draft], resource, :new,        :get
-  end
-
+  def test_should_resource_controller_name_equal_resource_name_by_default
+    resource = ActionController::Resources::Resource.new(:messages, {})
+    assert_equal 'messages', resource.controller
+  end
+
+  def test_should_resource_controller_name_equal_controller_option
+    resource = ActionController::Resources::Resource.new(:messages, :controller => 'posts')
+    assert_equal 'posts', resource.controller
+  end
+
+  def test_should_all_singleton_paths_be_the_same
+    [ :path, :nesting_path_prefix, :member_path ].each do |method|
+      resource = ActionController::Resources::SingletonResource.new(:messages, :path_prefix => 'admin')
+      assert_equal 'admin/messages', resource.send(method)
+    end
+  end
+
-  def test_default_restful_routes
-    with_restful_routing :messages do
-      assert_simply_restful_for :messages
@@ -116 +133 @@
-    end
-  end
-
-
-  def test_with_new_action
-    with_restful_routing :messages, :new => { :preview => :post } do
-      preview_options = {:action => 'preview'}
@@ -280 +296 @@
-    end
-  end
-
+  def test_should_not_allow_delete_or_put_on_collection_path
+    controller_name = :messages
+    with_restful_routing controller_name do
+      options = { :controller => controller_name.to_s }
+      collection_path = "/#{controller_name}"
+
+      assert_raises(ActionController::RoutingError) do
+        assert_recognizes(options.merge(:action => 'update'), :path => collection_path, :method => :put)
+      end
+
+      assert_raises(ActionController::RoutingError) do
+        assert_recognizes(options.merge(:action => 'destroy'), :path => collection_path, :method => :delete)
+      end
+    end
+  end
+
-  protected
-    def with_restful_routing(*args)
-      with_routing do |set|
@@ -311 +343 @@
-
-      collection_path = "/#{options[:path_prefix]}#{controller_name}"
-      member_path = "#{collection_path}/1"
+      edit_member_path = "#{member_path};edit"
-      new_path = "#{collection_path}/new"
-
-      with_options(options[:options]) do |controller|
- 
- 
- 
- 
-"#{member_path};edit",
- 
+
+
+
+
+edit_member_path,    
+
-      end
-
-      assert_recognizes(
+        options[:options].merge(:action => 'index'),
+        :path => collection_path, :method => :get)
+
+      assert_recognizes(
+        options[:options].merge(:action => 'new'),
+        :path => new_path, :method => :get)
+
+      assert_recognizes(
-        options[:options].merge(:action => 'create'),
-        :path => collection_path, :method => :post)
-
-      assert_recognizes(
+        options[:options].merge(:action => 'show', :id => '1'),
+        :path => member_path, :method => :get)
+
+      assert_recognizes(
+        options[:options].merge(:action => 'edit', :id => '1'),
+        :path => edit_member_path, :method => :get)
+
+      assert_recognizes(
-        options[:options].merge(:action => 'update', :id => '1'),
-        :path => member_path, :method => :put)
-
@@ -365 +414 @@
-
-    def assert_singleton_routes_for(singleton_name, options = {})
-      (options[:options] ||= {})[:controller] ||= singleton_name.to_s
-
+
-      full_path = "/#{options[:path_prefix]}#{singleton_name}"
+      edit_path = "#{full_path};edit"
+      new_path  = "#{full_path}/new"
+
-      with_options options[:options] do |controller|
-        controller.assert_routing full_path,          :action => 'show'
-        controller.assert_routing "#{full_path}.xml", :action => 'show', :format => 'xml'
-"#{full_path}/new",
-"#{full_path};edit",
+new_path,          
+edit_path,         
-      end
-
+      assert_recognizes(options[:options].merge(:action => 'show'),    :path => full_path, :method => :get)
+      assert_recognizes(options[:options].merge(:action => 'new'),     :path => new_path,  :method => :get)
+      assert_recognizes(options[:options].merge(:action => 'edit'),    :path => edit_path, :method => :get)
-      assert_recognizes(options[:options].merge(:action => 'create'),  :path => full_path, :method => :post)
-      assert_recognizes(options[:options].merge(:action => 'update'),  :path => full_path, :method => :put)
-      assert_recognizes(options[:options].merge(:action => 'destroy'), :path => full_path, :method => :delete)

lib/action_controller/resources.rb

@@ -322 +322 @@
-
-      def map_collection_actions(map, resource)
-        resource.collection_methods.each do |method, actions|
-requirement
+condition
-
-          actions.each do |action|
-            map.named_route(
-              "#{resource.name_prefix}#{action}_#{resource.plural}", 
-              "#{resource.path};#{action}", 
-route_options.merge(:action => action.to_
+action_options(action.to_s, resource).merge(route_option
-            )
-
-            map.named_route(
-              "formatted_#{resource.name_prefix}#{action}_#{resource.plural}",
-              "#{resource.path}.:format;#{action}",
-route_options.merge(:action => action.to_
+action_options(action.to_s, resource).merge(route_option
-            )
-          end
-        end
-      end
-
-      def map_default_collection_actions(map, resource)
-:action => "index", :conditions => { :method => :get }
-:action => "index", :conditions => { :method => :get }
+action_options("index", resource)
+action_options("index", resource)
-
-:action => "create", :conditions => { :method => :post }
-:action => "create", :conditions => { :method => :post }
+action_options("create", resource)
+action_options("create", resource)
-      end
-
-      def map_default_singleton_actions(map, resource)
-:action => "create", :conditions => { :method => :post }
-:action => "create", :conditions => { :method => :post }
+action_options("create", resource)
+action_options("create", resource)
-      end
-
-      def map_new_actions(map, resource)
-        resource.new_methods.each do |method, actions|
-requirement
+condition
-          actions.each do |action|
-            if action == :new
-route_options.merge(:action => "new"
-route_options.merge(:action => "new"
+action_options("new", resource).merge(route_options
+action_options("new", resource).merge(route_options
-            else
-route_options.merge(:action => action.to_
-route_options.merge(:action => action.to_
+action_options(action.to_s, resource).merge(route_option
+action_options(action.to_s, resource).merge(route_option
-            end
-          end
-        end
@@ -370 +370 @@
-      
-      def map_member_actions(map, resource)
-        resource.member_methods.each do |method, actions|
-requirement
+condition
-
-          actions.each do |action|
-route_options.merge(:action => action.to_
-route_options.merge(:action => action.to_
+action_options(action.to_s, resource).merge(route_option
+action_options(action.to_s, resource).merge(route_option
-          end
-        end
-
-:action => "show", :conditions => { :method => :get }
-:action => "show", :conditions => { :method => :get }
+action_options("show", resource)
+action_options("show", resource)
-
-:action => "update", :conditions => { :method => :put }
-:action => "update", :conditions => { :method => :put }
+action_options("update", resource)
+action_options("update", resource)
-
-:action => "destroy", :conditions => { :method => :delete }
-:action => "destroy", :conditions => { :method => :delete }
+action_options("destroy", resource)
+action_options("destroy", resource)
-      end
-
-requirement
-method == :any ? {} : { :conditions =>
+condition
+{ :conditions => method == :any ? {} :
-      end
+
+      def action_options(action, resource)
+        default_options = { :action => action }
+        require_id = resource.kind_of?(SingletonResource) ? {} : { :requirements => { :id => Regexp.new("[^#{Routing::SEPARATORS.join}]+") } }
+        case action
+          when "index", "new" : default_options.merge(conditions_for(:get))
+          when "create"       : default_options.merge(conditions_for(:post))
+          when "show", "edit" : default_options.merge(conditions_for(:get)).merge(require_id)
+          when "update"       : default_options.merge(conditions_for(:put)).merge(require_id)
+          when "destroy"      : default_options.merge(conditions_for(:delete)).merge(require_id)
+          else                  default_options
+        end
+      end
-  end
-end
-