diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 5e95d8ed39..0cb02c5b80 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -62,7 +62,6 @@ module ActionDispatch 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] ||= Mapper.normalize_name(path_without_format) end @options.merge!(default_controller_and_action(to_shorthand)) @@ -924,9 +923,14 @@ module ActionDispatch if action.to_s =~ /^[\w\/]+$/ options[:action] ||= action unless action.to_s.include?("/") - options[:as] = name_for_action(action, options[:as]) else - options[:as] = name_for_action(options[:as]) + action = nil + end + + if options.key?(:as) && !options[:as] + options.delete(:as) + else + options[:as] = name_for_action(options[:as], action) end super(path, options) @@ -1092,18 +1096,16 @@ module ActionDispatch path || @scope[:path_names][name.to_sym] || name.to_s end - def prefix_name_for_action(action, as) - if as.present? + def prefix_name_for_action(as, action) + if as as.to_s - elsif as - nil elsif !canonical_action?(action, @scope[:scope_level]) action.to_s end end - def name_for_action(action, as=nil) - prefix = prefix_name_for_action(action, as) + def name_for_action(as, action) + prefix = prefix_name_for_action(as, action) prefix = Mapper.normalize_name(prefix) if prefix name_prefix = @scope[:as] @@ -1127,7 +1129,8 @@ module ActionDispatch [name_prefix, member_name, prefix] end - name.select(&:present?).join("_").presence + candidate = name.select(&:present?).join("_").presence + candidate unless as.nil? && @set.routes.map(&:name).include?(candidate) end end diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index 53b13501b2..5c188a60c7 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -442,6 +442,15 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest get :preview, :on => :member end + scope :as => "routes" do + get "/c/:id", :as => :collision, :to => "collision#show" + get "/collision", :to => "collision#show" + get "/no_collision", :to => "collision#show", :as => nil + + get "/fc/:id", :as => :forced_collision, :to => "forced_collision#show" + get "/forced_collision", :as => :forced_collision, :to => "forced_collision#show" + end + match '/purchases/:token/:filename', :to => 'purchases#fetch', :token => /[[:alnum:]]{10}/, @@ -2120,6 +2129,15 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest assert_raises(ActionController::RoutingError){ list_todo_path(:list_id => '2', :id => '1') } end + def test_named_routes_collision_is_avoided_unless_explicitly_given_as + assert_equal "/c/1", routes_collision_path(1) + assert_equal "/forced_collision", routes_forced_collision_path + end + + def test_explicitly_avoiding_the_named_route + assert !respond_to?(:routes_no_collision_path) + end + def test_controller_name_with_leading_slash_raise_error assert_raise(ArgumentError) do self.class.stub_controllers do |routes|