From a77de09812eec5f342dd159f053038c6f67e6da4 Mon Sep 17 00:00:00 2001
From: Yang Bo <yangbodotnet@gmail.com>
Date: Wed, 8 Apr 2015 22:18:56 +0800
Subject: [PATCH] sort_by instead of sort

it is avoid sort errot within different and mixed keys.
used `sort_by` + `block` to list parameter by keys.
keep minimum changes
---
 .../lib/action_dispatch/journey/formatter.rb  |  2 +-
 .../lib/action_dispatch/routing/route_set.rb  |  2 +-
 actionpack/test/dispatch/routing_test.rb      | 13 +++++++++
 actionpack/test/journey/router_test.rb        | 27 +++++++++++++++++++
 4 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/actionpack/lib/action_dispatch/journey/formatter.rb b/actionpack/lib/action_dispatch/journey/formatter.rb
index 992c1a9efe..c0566c6fc9 100644
--- a/actionpack/lib/action_dispatch/journey/formatter.rb
+++ b/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
diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb
index 0f3734dd74..d0d8ded515 100644
--- a/actionpack/lib/action_dispatch/routing/route_set.rb
+++ b/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}"
 
diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb
index 55fc160ac8..62c99a2edc 100644
--- a/actionpack/test/dispatch/routing_test.rb
+++ b/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
diff --git a/actionpack/test/journey/router_test.rb b/actionpack/test/journey/router_test.rb
index 19c61b5914..a134e343cc 100644
--- a/actionpack/test/journey/router_test.rb
+++ b/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, {}, {}, {}