Merge pull request #19633 from y00rb/sort_router_parameters_duplicated_keys

avoid error when sort mixture keys in symbol and string

actionpack/lib/action_dispatch/journey/formatter.rb

@@ -39,7 +39,7 @@ module ActionDispatch
           return [route.format(parameterized_parts), params]
         end
 
-        message = "No route matches #{Hash[constraints.sort].inspect}"
+        message = "No route matches #{Hash[constraints.sort_by{|k,v| k.to_s}].inspect}"
         message << " missing required keys: #{missing_keys.sort.inspect}" unless missing_keys.empty?
 
         raise ActionController::UrlGenerationError, message

actionpack/lib/action_dispatch/routing/route_set.rb

@@ -226,7 +226,7 @@ module ActionDispatch
               params = parameterize_args(args) { |missing_key|
                 missing_keys << missing_key
               }
-              constraints = Hash[@route.requirements.merge(params).sort]
+              constraints = Hash[@route.requirements.merge(params).sort_by{|k,v| k.to_s}]
               message = "No route matches #{constraints.inspect}"
               message << " missing required keys: #{missing_keys.sort.inspect}"
 

actionpack/test/dispatch/routing_test.rb

@@ -4476,6 +4476,19 @@ class TestUrlGenerationErrors < ActionDispatch::IntegrationTest
     error = assert_raises(ActionController::UrlGenerationError, message){ product_path(id: nil) }
     assert_equal message, error.message
   end
+
+  test "url helpers raise message with mixed parameters when generation fails " do
+    url, missing = { action: 'show', controller: 'products', id: nil, "id"=>"url-tested"}, [:id]
+    message = "No route matches #{url.inspect} missing required keys: #{missing.inspect}"
+
+    # Optimized url helper
+    error = assert_raises(ActionController::UrlGenerationError){ product_path(nil, 'id'=>'url-tested') }
+    assert_equal message, error.message
+
+    # Non-optimized url helper
+    error = assert_raises(ActionController::UrlGenerationError, message){ product_path(id: nil, 'id'=>'url-tested') }
+    assert_equal message, error.message
+  end
 end
 
 class TestDefaultUrlOptions < ActionDispatch::IntegrationTest

actionpack/test/journey/router_test.rb

@@ -401,6 +401,33 @@ module ActionDispatch
         assert_equal({:id => 1, :relative_url_root => nil}, params)
       end
 
+      def test_generate_missing_keys_no_matches_different_format_keys
+        path  = Path::Pattern.from_string '/:controller/:action/:name'
+        @router.routes.add_route @app, path, {}, {}, {}
+        primarty_parameters = {
+          :id                => 1,
+          :controller        => "tasks",
+          :action            => "show",
+          :relative_url_root => nil
+        }
+        redirection_parameters = {
+          'action'=>'show',
+        }
+        missing_key = 'name'
+        missing_parameters ={
+          missing_key => "task_1"
+        }
+        request_parameters = primarty_parameters.merge(redirection_parameters).merge(missing_parameters)
+
+        message = "No route matches #{Hash[request_parameters.sort_by{|k,v|k.to_s}].inspect} missing required keys: #{[missing_key.to_sym].inspect}"
+
+        error = assert_raises(ActionController::UrlGenerationError) do
+          @formatter.generate(
+            nil, request_parameters, request_parameters)
+        end
+        assert_equal message, error.message
+      end
+
       def test_generate_uses_recall_if_needed
         path  = Path::Pattern.from_string '/:controller(/:action(/:id))'
         @router.routes.add_route @app, path, {}, {}, {}