Unique the segment keys array for non-optimized url helpers

In Rails 3.2 you only needed pass an argument for dynamic segment once so
unique the segment keys array to match the number of args. Since the number
of args is less than required parts the non-optimized code path is selected.
This means to benefit from optimized url generation the arg needs to be
specified as many times as it appears in the path.

Fixes #12808

3 files changed, 31 insertions, 1 deletions

diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md
index dc627a4da4..bf608e162b 100644
--- a/actionpack/CHANGELOG.md
+++ b/actionpack/CHANGELOG.md
@@ -1,3 +1,15 @@
+*   Unique the segment keys array for non-optimized url helpers
+
+    In Rails 3.2 you only needed pass an argument for dynamic segment once so
+    unique the segment keys array to match the number of args. Since the number
+    of args is less than required parts the non-optimized code path is selected.
+    This means to benefit from optimized url generation the arg needs to be
+    specified as many times as it appears in the path.
+
+    Fixes #12808
+
+    *Andrew White*
+
 *   Show full route constraints in error message
 
     When an optimized helper fails to generate, show the full route constraints
diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb
index 10c8d748df..a03fb4cee7 100644
--- a/actionpack/lib/action_dispatch/routing/route_set.rb
+++ b/actionpack/lib/action_dispatch/routing/route_set.rb
@@ -220,7 +220,7 @@ module ActionDispatch
 
           def initialize(route, options)
             @options      = options
-            @segment_keys = route.segment_keys
+            @segment_keys = route.segment_keys.uniq
             @route        = route
           end
 
diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb
index d41b94511a..840c157642 100644
--- a/actionpack/test/dispatch/routing_test.rb
+++ b/actionpack/test/dispatch/routing_test.rb
@@ -2864,6 +2864,24 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest
     assert !@request.params[:action].frozen?
   end
 
+  def test_multiple_positional_args_with_the_same_name
+    draw do
+      get '/downloads/:id/:id.tar' => 'downloads#show', as: :download, format: false
+    end
+
+    expected_params = {
+      controller: 'downloads',
+      action:     'show',
+      id:         '1'
+    }
+
+    get '/downloads/1/1.tar'
+    assert_equal 'downloads#show', @response.body
+    assert_equal expected_params, @request.symbolized_path_parameters
+    assert_equal '/downloads/1/1.tar', download_path('1')
+    assert_equal '/downloads/1/1.tar', download_path('1', '1')
+  end
+
 private
 
   def draw(&block)