Changeset 7713

Timestamp:

10/01/07 06:44:07 (1 year ago)

Author:

nzkoz

Message:

Change the resource seperator from ; to / change the generated routes to use the new-style named routes. e.g. new_group_user_path(@group) instead of group_new_user_path(@group). [pixeltrix] Closes #8558

Files:

• branches/1-2-stable/actionpack/CHANGELOG (modified) (1 diff)
• branches/1-2-stable/actionpack/lib/action_controller/base.rb (modified) (1 diff)
• branches/1-2-stable/actionpack/lib/action_controller/resources.rb (modified) (17 diffs)
• branches/1-2-stable/actionpack/lib/action_controller/routing.rb (modified) (4 diffs)
• branches/1-2-stable/actionpack/test/controller/resources_test.rb (modified) (22 diffs)

branches/1-2-stable/actionpack/CHANGELOG

@@ -1 +1 @@
-*SVN*
+
+* Change the resource seperator from ; to / change the generated routes to use the new-style named routes.  e.g.  new_group_user_path(@group) instead of group_new_user_path(@group). [pixeltrix]
+
+
-
-* Integration tests: introduce methods for other HTTP methods.  #6353 [caboose]

branches/1-2-stable/actionpack/lib/action_controller/base.rb

@@ -293 +293 @@
-    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>.

branches/1-2-stable/actionpack/lib/action_controller/resources.rb

@@ -3 +3 @@
-    class Resource #:nodoc:
-      attr_reader :collection_methods, :member_methods, :new_methods
-
+ew_n
-      attr_reader :plural, :singular
-      attr_reader :options
@@ -10 +10 @@
-        @plural   = entities
-        @singular = options[:singular] || plural.to_s.singularize
-
+
-        @options = options
-
@@ -17 +17 @@
-        set_prefixes
-      end
-
+
-      def controller
-        @controller ||= (options[:controller] || plural).to_s
-      end
-
+
-      def path
-        @path ||= "#{path_prefix}/#{plural}"
-      end
-
+
-      def new_path
-        @new_path ||= "#{path}/new"
-      end
-
+
-      def member_path
-        @member_path ||= "#{path}/:id"
-      end
-
+
-      def nesting_path_prefix
-        @nesting_path_prefix ||= "#{path}/:#{singular}_id"
-      end
-
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
-      protected
-        def arrange_actions
@@ -44 +64 @@
-          @new_methods        = arrange_actions_by_methods(options.delete(:new))
-        end
-
+
-        def add_default_actions
-          add_default_action(member_methods, :get, :edit)
@@ -53 +73 @@
-          @path_prefix = options.delete(:path_prefix)
-          @name_prefix = options.delete(:name_prefix)
+          @new_name_prefix = options.delete(:new_name_prefix)
-        end
-
@@ -61 +82 @@
-          end
-        end
-
+
-        def add_default_action(collection, method, action)
-          (collection[method] ||= []).unshift(action)
@@ -179 +200 @@
-    #   The comment resources work the same, but must now include a value for :article_id.
-    #   
-
-
-
-
-
+article_
+article_
+
+article_
+article_
-    #
-    # * <tt>:name_prefix</tt> -- define a prefix for all generated routes, usually ending in an underscore.
@@ -193 +214 @@
-    # * <tt>:collection</tt> -- add named routes for other actions that operate on the collection.
-    #   Takes a hash of <tt>#{action} => #{method}</tt>, where method is <tt>:get</tt>/<tt>:post</tt>/<tt>:put</tt>/<tt>:delete</tt>
-;
+/
-    # * <tt>:member</tt> -- same as :collection, but for actions that operate on a specific member.
-    # * <tt>:new</tt> -- same as :collection, but for actions that operate on the new resource action.
@@ -205 +226 @@
-    #  
-    #   map.resources :messages, :collection => { :rss => :get }
-;
+/
-    #   #     also adds a named route called "rss_messages"
-    # 
-    #   map.resources :messages, :member => { :mark => :post }
-;
+/
-    #   #     also adds a named route called "mark_message"
-    # 
-    #   map.resources :messages, :new => { :preview => :post }
-;
+/
-    #   #     also adds a named route called "preview_new_message"
-    # 
-    #   map.resources :messages, :new => { :new => :any, :preview => :post }
-;
+/
-    #   #     also adds a named route called "preview_new_message"
-    #   # --> /messages/new can be invoked via any request method
@@ -236 +257 @@
-    # 
-    # See map.resources for general conventions.  These are the main differences:
-
-
-
+
+
+
+
-    #   - When nesting singleton resources, only the singular name is used as the path prefix (example: 'account/messages/1')
-    #
@@ -301 +323 @@
-
-          if block_given?
-
+:new_name_prefix => resource.nesting_name_prefix, 
-          end
-        end
@@ -316 +338 @@
-
-          if block_given?
-
+:new_name_prefix => resource.nesting_name_prefix, 
-          end
-        end
@@ -325 +347 @@
-          actions.each do |action|
-            action_options = action_options_for(action, resource, method)
-
-
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
-          end
-        end
@@ -335 +370 @@
-        map.named_route("#{resource.name_prefix}#{resource.plural}", resource.path, index_action_options)
-        map.named_route("formatted_#{resource.name_prefix}#{resource.plural}", "#{resource.path}.:format", index_action_options)
+
+        if resource.deprecate_name_prefix?
+          map.deprecated_named_route("#{resource.name_prefix}#{resource.plural}", "#{resource.plural}")
+          map.deprecated_named_route("formatted_#{resource.name_prefix}#{resource.plural}", "formatted_#{resource.plural}")
+        end
-
-        create_action_options = action_options_for("create", resource)
@@ -352 +392 @@
-            action_options = action_options_for(action, resource, method)
-            if action == :new
-
-
+
+
+
+
+
+
+
+
+
+
+
+
+
+
-            else
-
-
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
-            end
-          end
@@ -366 +432 @@
-          actions.each do |action|
-            action_options = action_options_for(action, resource, method)
-
-
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
-          end
-        end
@@ -374 +454 @@
-        map.named_route("#{resource.name_prefix}#{resource.singular}", resource.member_path, show_action_options)
-        map.named_route("formatted_#{resource.name_prefix}#{resource.singular}", "#{resource.member_path}.:format", show_action_options)
+
+        if resource.deprecate_name_prefix?
+          map.deprecated_named_route("#{resource.name_prefix}#{resource.singular}", "#{resource.singular}")
+          map.deprecated_named_route("formatted_#{resource.name_prefix}#{resource.singular}", "formatted_#{resource.singular}")
+        end
-
-        update_action_options = action_options_for("update", resource)

branches/1-2-stable/actionpack/lib/action_controller/routing.rb

@@ -990 +990 @@
-          @set.add_named_route(name, path, options)
-        end
+        
+        def deprecated_named_route(name, deprecated_name, options = {})
+          @set.add_deprecated_named_route(name, deprecated_name)
+        end
-
-        # Added deprecation notice for anyone who already added a named route called "root".
@@ -1020 +1024 @@
-          @routes = {}
-          @helpers = []
-
+
-          @module ||= Module.new
-          @module.instance_methods.each do |selector|
@@ -1055 +1059 @@
-        def install(destinations = [ActionController::Base, ActionView::Base])
-          Array(destinations).each { |dest| dest.send :include, @module }
+        end
+        
+        def define_deprecated_named_route_methods(name, deprecated_name)
+
+          [:url, :path].each do |kind|
+            @module.send :module_eval, <<-end_eval # We use module_eval to avoid leaks
+
+              def #{url_helper_name(deprecated_name, kind)}(*args)
+
+                ActiveSupport::Deprecation.warn(
+                  'The named route "#{url_helper_name(deprecated_name, kind)}" uses a format that has been deprecated. ' +
+                  'You should use "#{url_helper_name(name, kind)}" instead.', caller
+                )
+
+                send :#{url_helper_name(name, kind)}, *args
+
+              end
+
+              def #{hash_access_name(deprecated_name, kind)}(*args)
+
+                ActiveSupport::Deprecation.warn(
+                  'The named route "#{hash_access_name(deprecated_name, kind)}" uses a format that has been deprecated. ' +
+                  'You should use "#{hash_access_name(name, kind)}" instead.', caller
+                )
+
+                send :#{hash_access_name(name, kind)}, *args
+
+              end
+
+            end_eval
+          end
+
-        end
-
@@ -1178 +1214 @@
-        named_routes[name] = add_route(path, options)
-      end
+      
+      def add_deprecated_named_route(name, deprecated_name)
+        named_routes.define_deprecated_named_route_methods(name, deprecated_name)
+      end
-  
-      def options_as_params(options)

branches/1-2-stable/actionpack/test/controller/resources_test.rb

@@ -11 +11 @@
-class CommentsController < ResourcesController; end
-
-
-
+
+
+
-
-class ResourcesTest < Test::Unit::TestCase
@@ -64 +65 @@
-  end
-
-
+p
-    with_restful_routing :messages, :comments, :path_prefix => '/thread/:thread_id' do
-      assert_simply_restful_for :messages, :path_prefix => 'thread/5/', :options => { :thread_id => '5' }
@@ -70 +71 @@
-    end
-  end
-
+
-  def test_with_name_prefix
-    with_restful_routing :messages, :name_prefix => 'post_' do
@@ -79 +80 @@
-  def test_with_collection_action
-    rss_options = {:action => 'rss'}
-;
+/
-    actions = { 'a' => :put, 'b' => :post, 'c' => :delete }
-
@@ -87 +88 @@
-
-        actions.each do |action, method|
-;
+/
-        end
-      end
@@ -94 +95 @@
-        assert_named_route rss_path, :rss_messages_path, rss_options
-        actions.keys.each do |action|
-;
+/
-        end
-      end
@@ -104 +105 @@
-      with_restful_routing :messages, :member => { :mark => method } do
-        mark_options = {:action => 'mark', :id => '1'}
-;
+/
-        assert_restful_routes_for :messages do |options|
-          assert_recognizes(options.merge(mark_options), :path => mark_path, :method => method)
@@ -121 +122 @@
-        %w(mark unmark).each do |action|
-          action_options = {:action => action, :id => '1'}
-;
+/
-          assert_restful_routes_for :messages do |options|
-            assert_recognizes(options.merge(action_options), :path => action_path, :method => method)
@@ -137 +138 @@
-    with_restful_routing :messages, :new => { :preview => :post } do
-      preview_options = {:action => 'preview'}
-;
+/
-      assert_restful_routes_for :messages do |options|
-        assert_recognizes(options.merge(preview_options), :path => preview_path, :method => :post)
@@ -179 +180 @@
-      assert_simply_restful_for :messages,
-        :path_prefix => 'threads/1/',
+        :name_prefix => 'thread_',
-        :options => { :thread_id => '1' }
-      assert_simply_restful_for :comments,
-        :path_prefix => 'threads/1/messages/2/',
+        :name_prefix => 'thread_message_',
-        :options => { :thread_id => '1', :message_id => '2' }
-    end
@@ -218 +221 @@
-        end
-      end
-
+
-      assert_singleton_restful_for :admin
-
+, :name_prefix => 'admin_'
-    end
-  end
@@ -228 +231 @@
-      with_singleton_resources :account, :member => { :reset => method } do
-        reset_options = {:action => 'reset'}
-;
+/
-        assert_singleton_routes_for :account do |options|
-          assert_recognizes(options.merge(reset_options), :path => reset_path, :method => method)
@@ -245 +248 @@
-        %w(reset disable).each do |action|
-          action_options = {:action => action}
-;
+/
-          assert_singleton_routes_for :account do |options|
-            assert_recognizes(options.merge(action_options), :path => action_path, :method => method)
@@ -265 +268 @@
-        end
-      end
-
+
-      assert_singleton_restful_for :account
-
+, :name_prefix => 'account_'
-    end
-  end
@@ -280 +283 @@
-
-      assert_singleton_restful_for :account, :path_prefix => '7/', :options => { :site_id => '7' }
-
-
-
-
+name_prefix => 'account_', :
+
+
+
-  def test_should_nest_singleton_resource_in_resources
-    with_routing do |set|
@@ -291 +294 @@
-        end
-      end
-
+
-      assert_simply_restful_for :threads
-
+name_prefix => 'thread_', :
-    end
-  end
@@ -313 +316 @@
-  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_should_add_deprecated_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_deprecated do
+        assert_named_route "/threads/1/messages/search", "thread_search_messages_path", {}
+        assert_named_route "/threads/1/messages/new", "thread_new_message_path", {}
+        assert_named_route "/threads/1/messages/new/preview", "thread_preview_new_message_path", {}
+      end
+    end
+  end
+
+  def test_should_add_deprecated_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_deprecated do
+        assert_named_route "/admin/account/login", "admin_login_account_path", {}
+        assert_named_route "/admin/account/new", "admin_new_account_path", {}
+        assert_named_route "/admin/account/new/preview", "admin_preview_new_account_path", {}
+      end
+    end
+  end
+
+  def test_should_add_deprecated_named_routes_for_nested_resources
+    with_routing do |set|
+      set.draw do |map|
+        map.resources :threads do |map|
+          map.resources :messages do |map|
+            map.resources :comments
+          end
+        end
+      end
+
+      assert_simply_restful_for :threads
+      assert_simply_restful_for :messages,
+        :path_prefix => 'threads/1/',
+        :name_prefix => 'thread_',
+        :options => { :thread_id => '1' }
+      assert_simply_restful_for :comments,
+        :path_prefix => 'threads/1/messages/2/',
+        :name_prefix => 'thread_message_',
+        :options => { :thread_id => '1', :message_id => '2' }
+
+      assert_deprecated do
+        assert_named_route "/threads/1/messages", "messages_path", {}
+        assert_named_route "/threads/1/messages/1", "message_path", {:thread_id => '1', :id => '1'}
+        assert_named_route "/threads/1/messages/new", "new_message_path", {:thread_id => '1'}
+        assert_named_route "/threads/1/messages/1/edit", "edit_message_path", {:thread_id => '1', :id => '1'}
+      end
+    end
+  end
+
+  def test_should_add_deprecated_named_routes_for_nested_singleton_resources
+    with_routing do |set|
+      set.draw do |map|
+        map.resource :admin do |admin|
+          admin.resource :account
+        end
+      end
+
+      assert_singleton_restful_for :admin
+      assert_singleton_restful_for :account, :path_prefix => 'admin/', :name_prefix => 'admin_'
+
+      assert_deprecated do
+        assert_named_route "/admin/account", "account_path", {}
+        assert_named_route "/admin/account/new", "new_account_path", {}
+        assert_named_route "/admin/account/edit", "edit_account_path", {}
+      end
+    end
+  end
+
+  def test_should_add_deprecated_named_routes_for_nested_resources_in_singleton_resource
+    with_routing do |set|
+      set.draw do |map|
+        map.resource :account do |account|
+          account.resources :messages
+        end
+      end
+
+      assert_singleton_restful_for :account
+      assert_simply_restful_for :messages, :path_prefix => 'account/', :name_prefix => 'account_'
+
+      assert_deprecated do
+        assert_named_route "/account/messages", "messages_path", {}
+        assert_named_route "/account/messages/1", "message_path", {:id => '1'}
+        assert_named_route "/account/messages/new", "new_message_path", {}
+        assert_named_route "/account/messages/1/edit", "edit_message_path", {:id => '1'}
+      end
+    end
+  end
+
+  def test_should_add_deprecated_named_routes_for_nested_singleton_resource_in_resources
+    with_routing do |set|
+      set.draw do |map|
+        map.resources :threads do |thread|
+          thread.resource :admin
+        end
+      end
+
+      assert_simply_restful_for :threads
+      assert_singleton_restful_for :admin, :path_prefix => 'threads/5/', :name_prefix => 'thread_', :options => { :thread_id => '5' }
+
+      assert_deprecated do
+        assert_named_route "/threads/5/admin", "admin_path", {}
+        assert_named_route "/threads/5/admin/new", "new_admin_path", {}
+        assert_named_route "/threads/5/admin/edit", "edit_admin_path", {}
+      end
+    end
+  end
+
+  def test_should_add_deprecated_formatted_routes
+    with_routing do |set|
+      set.draw do |map|
+        map.resources :products, :collection => { :specials => :get }, :member => { :thumbnail => :get }
+        map.resource :account, :member => { :icon => :get }
+      end
+      assert_restful_routes_for :products do |options|
+        assert_recognizes options.merge({ :action => 'specials', :format => 'xml' }), :path => '/products.xml;specials', :method => :get
+        assert_recognizes options.merge({ :action => 'thumbnail', :format => 'jpg', :id => '1' }), :path => '/products/1.jpg;thumbnail', :method => :get
+      end
+      assert_singleton_restful_for :account do |options|
+        assert_recognizes options.merge({ :action => 'icon', :format => 'jpg' }), :path => '/account.jpg;icon', :method => :get
+      end
+    end
+  end
+
-  protected
-    def with_restful_routing(*args)
@@ -320 +498 @@
-      end
-    end
-
+
-    def with_singleton_resources(*args)
-      with_routing do |set|
@@ -345 +523 @@
-      member_path                = "#{collection_path}/1"
-      new_path                   = "#{collection_path}/new"
-;
-.xml;edit
+/
+/edit.xml
-
-      with_options(options[:options]) do |controller|
@@ -396 +574 @@
-
-      assert_named_route "#{full_prefix}",            "#{name_prefix}#{controller_name}_path",              options[:options]
-#{name_prefix}new_
+new_#{name_prefix}
-      assert_named_route "#{full_prefix}/1",          "#{name_prefix}#{singular_name}_path",                options[:options].merge(:id => '1')
-;edit",     "#{name_prefix}edit_
+/edit",     "edit_#{name_prefix}
-      assert_named_route "#{full_prefix}.xml",        "formatted_#{name_prefix}#{controller_name}_path",    options[:options].merge(            :format => 'xml')
-#{name_prefix}new_
+new_#{name_prefix}
-      assert_named_route "#{full_prefix}/1.xml",      "formatted_#{name_prefix}#{singular_name}_path",      options[:options].merge(:id => '1', :format => 'xml')
-.xml;edit", "formatted_#{name_prefix}edit_
+/edit.xml", "formatted_edit_#{name_prefix}
-      yield options[:options] if block_given?
-    end
@@ -411 +589 @@
-      full_path           = "/#{options[:path_prefix]}#{singleton_name}"
-      new_path            = "#{full_path}/new"
-;
-.xml;edit
+/
+/edit.xml
-
-      with_options options[:options] do |controller|
@@ -449 +627 @@
-
-      full_path = "/#{options[:path_prefix]}#{singleton_name}"
-
-
-
-
-
-
-
+
+
+
+
+
+
+
+
-    end
-