From f63d35fba55867111427bfeb54c31b11e02d5d3f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jos=C3=A9=20Valim?= <jose.valim@gmail.com>
Date: Wed, 29 Sep 2010 14:24:32 +0200
Subject: [PATCH] Ensure that named routes do not overwrite previously defined
 routes.

---
 .../lib/action_dispatch/routing/mapper.rb     | 23 +++++++++++--------
 actionpack/test/dispatch/routing_test.rb      | 18 +++++++++++++++
 2 files changed, 31 insertions(+), 10 deletions(-)

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 @@ def normalize_options!
             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 @@ def match(*args)
 
           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 @@ def action_path(name, path = nil)
             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 @@ def name_for_action(action, as=nil)
               [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 @@ def self.matches?(request)
         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 @@ def test_nested_resource_constraints
     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|
-- 
GitLab