diff options
| -rw-r--r-- | actionpack/lib/action_dispatch/routing/mapper.rb | 113 | ||||
| -rw-r--r-- | actionpack/test/dispatch/routing_test.rb | 44 |
2 files changed, 84 insertions, 73 deletions
diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 234e34a24f..f3cbba243c 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -44,7 +44,8 @@ module ActionDispatch IGNORE_OPTIONS = [:to, :as, :via, :on, :constraints, :defaults, :only, :except, :anchor, :shallow, :shallow_path, :shallow_prefix] def initialize(set, scope, path, options) - @set, @scope, @options = set, scope, options + @set, @scope = set, scope + @options = (@scope[:options] || {}).merge(options) @path = normalize_path(path) normalize_options! end @@ -57,18 +58,11 @@ module ActionDispatch def normalize_options! path_without_format = @path.sub(/\(\.:format\)$/, '') - @options = (@scope[:options] || {}).merge(@options) - - if @scope[:as] && !@options[:as].blank? - @options[:as] = "#{@scope[:as]}_#{@options[:as]}" - elsif @scope[:as] && @options[:as] == "" - @options[:as] = @scope[:as].to_s - end if using_match_shorthand?(path_without_format, @options) to_shorthand = @options[:to].blank? @options[:to] ||= path_without_format[1..-1].sub(%r{/([^/]*)$}, '#\1') - @options[:as] ||= path_without_format[1..-1].gsub("/", "_") + @options[:as] ||= Mapper.normalize_name(path_without_format) end @options.merge!(default_controller_and_action(to_shorthand)) @@ -80,8 +74,8 @@ module ActionDispatch end def normalize_path(path) - raise ArgumentError, "path is required" if @scope[:path].blank? && path.blank? - path = Mapper.normalize_path("#{@scope[:path]}/#{path}") + raise ArgumentError, "path is required" if path.blank? + path = Mapper.normalize_path(path) if path.match(':controller') raise ArgumentError, ":controller segment is not allowed within a namespace block" if @scope[:module] @@ -93,7 +87,11 @@ module ActionDispatch @options.reverse_merge!(:controller => /.+?/) end - path + if path.include?(":format") + path + else + "#{path}(.:format)" + end end def app @@ -216,6 +214,10 @@ module ActionDispatch path end + def self.normalize_name(name) + normalize_path(name)[1..-1].gsub("/", "_") + end + module Base def initialize(set) #:nodoc: @set = set @@ -324,13 +326,7 @@ module ActionDispatch ActiveSupport::Deprecation.warn ":name_prefix was deprecated in the new router syntax. Use :as instead.", caller end - case args.first - when String - options[:path] = args.first - when Symbol - options[:controller] = args.first - end - + options[:path] = args.first if args.first.is_a?(String) recover = {} options[:constraints] ||= {} @@ -362,8 +358,9 @@ module ActionDispatch @scope[:blocks] = recover[:block] end - def controller(controller) - scope(controller.to_sym) { yield } + def controller(controller, options={}) + options[:controller] = controller + scope(options) { yield } end def namespace(path, options = {}) @@ -444,9 +441,9 @@ module ActionDispatch module Resources # CANONICAL_ACTIONS holds all actions that does not need a prefix or # a path appended since they fit properly in their scope level. - VALID_ON_OPTIONS = [:new, :collection, :member] - CANONICAL_ACTIONS = [:index, :create, :new, :show, :update, :destroy] - RESOURCE_OPTIONS = [:as, :controller, :path, :only, :except] + VALID_ON_OPTIONS = [:new, :collection, :member] + RESOURCE_OPTIONS = [:as, :controller, :path, :only, :except] + CANONICAL_ACTIONS = %w(index create new show update destroy) class Resource #:nodoc: DEFAULT_ACTIONS = [:index, :create, :new, :show, :update, :destroy, :edit] @@ -700,41 +697,27 @@ module ActionDispatch return member { match(*args) } end - path = options.delete(:path) + if resource_scope? + raise ArgumentError, "can't define route directly in resource(s) scope" + end + action = args.first + path = path_for_action(action, options.delete(:path)) - if action.is_a?(Symbol) || (resource_method_scope? && action.to_s =~ /^[A-Za-z_]\w*$/) - path = path_for_action(action, path) - options[:action] ||= action + if action.to_s =~ /^[\w\/]+$/ + options[:action] ||= action unless action.to_s.include?("/") options[:as] = name_for_action(action, options[:as]) - - with_exclusive_scope do - return super(path, options) - end - elsif resource_method_scope? - path = path_for_custom_action - options[:as] = name_for_action(options[:as]) if options[:as] - args.push(options) - - with_exclusive_scope do - scope(path) do - return super - end - end - end - - if resource_scope? - raise ArgumentError, "can't define route directly in resource(s) scope" + else + options[:as] = name_for_action(options[:as]) end - args.push(options) - super + super(path, options) end def root(options={}) if @scope[:scope_level] == :resources - with_scope_level(:nested) do - scope(parent_resource.path, :as => parent_resource.collection_name) do + with_scope_level(:root) do + scope(parent_resource.path) do super(options) end end @@ -871,7 +854,7 @@ module ActionDispatch end def canonical_action?(action, flag) - flag && CANONICAL_ACTIONS.include?(action) + flag && resource_method_scope? && CANONICAL_ACTIONS.include?(action.to_s) end def shallow_scoping? @@ -882,18 +865,10 @@ module ActionDispatch prefix = shallow_scoping? ? "#{@scope[:shallow_path]}/#{parent_resource.path}/:id" : @scope[:path] - if canonical_action?(action, path.blank?) - "#{prefix}(.:format)" - else - "#{prefix}/#{action_path(action, path)}(.:format)" - end - end - - def path_for_custom_action - if shallow_scoping? - "#{@scope[:shallow_path]}/#{parent_resource.path}/:id" + path = if canonical_action?(action, path.blank?) + prefix.to_s else - @scope[:path] + "#{prefix}/#{action_path(action, path)}" end end @@ -913,6 +888,7 @@ module ActionDispatch def name_for_action(action, as=nil) prefix = prefix_name_for_action(action, as) + prefix = Mapper.normalize_name(prefix) if prefix name_prefix = @scope[:as] if parent_resource @@ -922,15 +898,18 @@ module ActionDispatch name = case @scope[:scope_level] when :collection - [name_prefix, collection_name] + [prefix, name_prefix, collection_name] when :new - [:new, name_prefix, member_name] + [prefix, :new, name_prefix, member_name] + when :member + [prefix, shallow_scoping? ? @scope[:shallow_prefix] : name_prefix, member_name] + when :root + [name_prefix, collection_name, prefix] else - [shallow_scoping? ? @scope[:shallow_prefix] : name_prefix, member_name] + [name_prefix, member_name, prefix] end - name.unshift(prefix) - name.select(&:present?).join("_") + name.select(&:present?).join("_").presence end end diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index 292cfc2a02..92692fb131 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -40,6 +40,13 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest get :new, :path => "build" post :create, :path => "create", :as => "" put :update + get :remove, :action => :destroy, :as => :remove + end + + scope "pagemark", :controller => "pagemarks", :as => :pagemark do + get "new", :path => "build" + post "create", :as => "" + put "update" get "remove", :action => :destroy, :as => :remove end @@ -203,11 +210,12 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end resources :customers do - get "recent" => "customers#recent", :on => :collection - get "profile" => "customers#profile", :on => :member + get :recent, :on => :collection + get "profile", :on => :member + get "secret/profile" => "customers#secret", :on => :member post "preview" => "customers#preview", :as => :another_preview, :on => :new resource :avatar do - get "thumbnail(.:format)" => "avatars#thumbnail", :as => :thumbnail, :on => :member + get "thumbnail" => "avatars#thumbnail", :as => :thumbnail, :on => :member end resources :invoices do get "outstanding" => "invoices#outstanding", :as => :outstanding, :on => :collection @@ -648,15 +656,15 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest with_test_routes do get '/bookmark/build' assert_equal 'bookmarks#new', @response.body - assert_equal '/bookmark/build', new_bookmark_path + assert_equal '/bookmark/build', bookmark_new_path post '/bookmark/create' assert_equal 'bookmarks#create', @response.body assert_equal '/bookmark/create', bookmark_path - put '/bookmark' + put '/bookmark/update' assert_equal 'bookmarks#update', @response.body - assert_equal '/bookmark', update_bookmark_path + assert_equal '/bookmark/update', bookmark_update_path get '/bookmark/remove' assert_equal 'bookmarks#destroy', @response.body @@ -664,6 +672,26 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end end + def test_pagemarks + with_test_routes do + get '/pagemark/build' + assert_equal 'pagemarks#new', @response.body + assert_equal '/pagemark/build', pagemark_new_path + + post '/pagemark/create' + assert_equal 'pagemarks#create', @response.body + assert_equal '/pagemark/create', pagemark_path + + put '/pagemark/update' + assert_equal 'pagemarks#update', @response.body + assert_equal '/pagemark/update', pagemark_update_path + + get '/pagemark/remove' + assert_equal 'pagemarks#destroy', @response.body + assert_equal '/pagemark/remove', pagemark_remove_path + end + end + def test_admin with_test_routes do get '/admin', {}, {'REMOTE_ADDR' => '192.168.1.100'} @@ -1564,6 +1592,7 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest with_test_routes do assert_equal '/customers/recent', recent_customers_path assert_equal '/customers/1/profile', profile_customer_path(:id => '1') + assert_equal '/customers/1/secret/profile', secret_profile_customer_path(:id => '1') assert_equal '/customers/new/preview', another_preview_new_customer_path assert_equal '/customers/1/avatar/thumbnail.jpg', thumbnail_customer_avatar_path(:customer_id => '1', :format => :jpg) assert_equal '/customers/1/invoices/outstanding', outstanding_customer_invoices_path(:customer_id => '1') @@ -1577,6 +1606,9 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest get '/customers/1/invoices/overdue' assert_equal 'invoices#overdue', @response.body + + get '/customers/1/secret/profile' + assert_equal 'customers#secret', @response.body end end |