1 files changed, 25 insertions, 31 deletions

diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb
index b8abdabca5..a03fb4cee7 100644
--- a/actionpack/lib/action_dispatch/routing/route_set.rb
+++ b/actionpack/lib/action_dispatch/routing/route_set.rb
@@ -163,9 +163,10 @@ module ActionDispatch
 
             def initialize(route, options)
               super
-              @path_parts   = @route.required_parts
-              @arg_size     = @path_parts.size
-              @string_route = @route.optimized_path
+              @klass          = Journey::Router::Utils
+              @required_parts = @route.required_parts
+              @arg_size       = @required_parts.size
+              @optimized_path = @route.optimized_path
             end
 
             def call(t, args)
@@ -182,43 +183,36 @@ module ActionDispatch
             private
 
             def optimized_helper(args)
-              path = @string_route.dup
-              klass = Journey::Router::Utils
+              params = Hash[parameterize_args(args)]
+              missing_keys = missing_keys(params)
 
-              @path_parts.zip(args) do |part, arg|
-                parameterized_arg = arg.to_param
+              unless missing_keys.empty?
+                raise_generation_error(params, missing_keys)
+              end
 
-                if parameterized_arg.nil? || parameterized_arg.empty?
-                  raise_generation_error(args)
-                end
+              @optimized_path.map{ |segment| replace_segment(params, segment) }.join
+            end
 
-                # Replace each route parameter
-                # e.g. :id for regular parameter or *path for globbing
-                # with ruby string interpolation code
-                path.gsub!(/(\*|:)#{part}/, klass.escape_fragment(parameterized_arg))
-              end
-              path
+            def replace_segment(params, segment)
+              Symbol === segment ? @klass.escape_fragment(params[segment]) : segment
             end
 
             def optimize_routes_generation?(t)
               t.send(:optimize_routes_generation?)
             end
 
-            def raise_generation_error(args)
-              parts, missing_keys = [], []
-
-              @path_parts.zip(args) do |part, arg|
-                parameterized_arg = arg.to_param
-
-                if parameterized_arg.nil? || parameterized_arg.empty?
-                  missing_keys << part
-                end
+            def parameterize_args(args)
+              @required_parts.zip(args.map(&:to_param))
+            end
 
-                parts << [part, arg]
-              end
+            def missing_keys(args)
+              args.select{ |part, arg| arg.nil? || arg.empty? }.keys
+            end
 
-              message = "No route matches #{Hash[parts].inspect}"
-              message << " missing required keys: #{missing_keys.inspect}"
+            def raise_generation_error(args, missing_keys)
+              constraints = Hash[@route.requirements.merge(args).sort]
+              message = "No route matches #{constraints.inspect}"
+              message << " missing required keys: #{missing_keys.sort.inspect}"
 
               raise ActionController::UrlGenerationError, message
             end
@@ -226,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
 
@@ -361,7 +355,7 @@ module ActionDispatch
         include UrlFor
       end
 
-      # Contains all the mounted helpers accross different
+      # Contains all the mounted helpers across different
       # engines and the `main_app` helper for the application.
       # You can include this in your classes if you want to
       # access routes for other engines.