From 54250a5bfe6992afaaca6357d3b414e6c49651ba Mon Sep 17 00:00:00 2001
From: Andrew White <andyw@pixeltrix.co.uk>
Date: Sat, 3 Jul 2010 08:14:17 +0100
Subject: Refactor recall parameter normalization [#5021 state:resolved]
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: José Valim <jose.valim@gmail.com>
---
 .../lib/action_dispatch/routing/route_set.rb       | 22 +++++-----------
 actionpack/test/template/url_helper_test.rb        | 29 ++++++++++++++++++++--
 2 files changed, 33 insertions(+), 18 deletions(-)

diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb
index afa312889f..177a6db04e 100644
--- a/actionpack/lib/action_dispatch/routing/route_set.rb
+++ b/actionpack/lib/action_dispatch/routing/route_set.rb
@@ -318,7 +318,6 @@ module ActionDispatch
           @extras      = extras
 
           normalize_options!
-          normalize_recall!
           normalize_controller_action_id!
           use_relative_controller!
           controller.sub!(%r{^/}, '') if controller
@@ -335,7 +334,11 @@ module ActionDispatch
 
         def use_recall_for(key)
           if @recall[key] && (!@options.key?(key) || @options[key] == @recall[key])
-            @options[key] = @recall.delete(key)
+            if named_route_exists?
+              @options[key] = @recall.delete(key) if segment_keys.include?(key)
+            else
+              @options[key] = @recall.delete(key)
+            end
           end
         end
 
@@ -359,15 +362,6 @@ module ActionDispatch
           end
         end
 
-        def normalize_recall!
-          # If the target route is not a standard route then remove controller and action
-          # from the options otherwise they will appear in the url parameters
-          if block_or_proc_route_target?
-            recall.delete(:controller) unless segment_keys.include?(:controller)
-            recall.delete(:action) unless segment_keys.include?(:action)
-          end
-        end
-
         # This pulls :controller, :action, and :id out of the recall.
         # The recall key is only used if there is no key in the options
         # or if the key in the options is identical. If any of
@@ -440,12 +434,8 @@ module ActionDispatch
             named_route && set.named_routes[named_route]
           end
 
-          def block_or_proc_route_target?
-            named_route_exists? && !set.named_routes[named_route].app.is_a?(Dispatcher)
-          end
-
           def segment_keys
-            named_route_exists? ? set.named_routes[named_route].segment_keys : []
+            set.named_routes[named_route].segment_keys
           end
       end
 
diff --git a/actionpack/test/template/url_helper_test.rb b/actionpack/test/template/url_helper_test.rb
index 765beebfa3..048f96c9a9 100644
--- a/actionpack/test/template/url_helper_test.rb
+++ b/actionpack/test/template/url_helper_test.rb
@@ -406,6 +406,14 @@ end
 class UrlHelperControllerTest < ActionController::TestCase
   class UrlHelperController < ActionController::Base
     test_routes do |map|
+      match 'url_helper_controller_test/url_helper/show/:id',
+        :to => 'url_helper_controller_test/url_helper#show',
+        :as => :show
+
+      match 'url_helper_controller_test/url_helper/profile/:name',
+        :to => 'url_helper_controller_test/url_helper#show',
+        :as => :profile
+
       match 'url_helper_controller_test/url_helper/show_named_route',
         :to => 'url_helper_controller_test/url_helper#show_named_route',
         :as => :show_named_route
@@ -418,6 +426,14 @@ class UrlHelperControllerTest < ActionController::TestCase
         :as => :normalize_recall_params
     end
 
+    def show
+      if params[:name]
+        render :inline => 'ok'
+      else
+        redirect_to profile_path(params[:id])
+      end
+    end
+
     def show_url_for
       render :inline => "<%= url_for :controller => 'url_helper_controller_test/url_helper', :action => 'show_url_for' %>"
     end
@@ -484,15 +500,24 @@ class UrlHelperControllerTest < ActionController::TestCase
     assert_equal 'http://testtwo.host/url_helper_controller_test/url_helper/show_named_route', @response.body
   end
 
-  def test_recall_params_should_be_normalized_when_using_block_route
+  def test_recall_params_should_be_normalized
     get :normalize_recall_params
     assert_equal '/url_helper_controller_test/url_helper/normalize_recall_params', @response.body
   end
 
-  def test_recall_params_should_not_be_changed_when_using_normal_route
+  def test_recall_params_should_not_be_changed
     get :recall_params_not_changed
     assert_equal '/url_helper_controller_test/url_helper/show_url_for', @response.body
   end
+
+  def test_recall_params_should_normalize_id
+    get :show, :id => '123'
+    assert_equal 302, @response.status
+    assert_equal 'http://test.host/url_helper_controller_test/url_helper/profile/123', @response.location
+
+    get :show, :name => '123'
+    assert_equal 'ok', @response.body
+  end
 end
 
 class TasksController < ActionController::Base
-- 
cgit v1.2.3