Refactor Generator class to not rely on in-place editing the controller

2 files changed, 16 insertions, 11 deletions

diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb
index 966f5bf157..8168bd4fcc 100644
--- a/actionpack/lib/action_dispatch/routing/route_set.rb
+++ b/actionpack/lib/action_dispatch/routing/route_set.rb
@@ -442,12 +442,12 @@ module ActionDispatch
           normalize_options!
           normalize_controller_action_id!
           use_relative_controller!
-          controller.sub!(%r{^/}, '') if controller
+          normalize_controller!
           handle_nil_action!
         end
 
         def controller
-          @controller ||= @options[:controller]
+          @options[:controller]
         end
 
         def current_controller
@@ -476,11 +476,11 @@ module ActionDispatch
 
           if options[:controller]
             options[:action]     ||= 'index'
-            options[:controller]   = options[:controller].to_s.dup
+            options[:controller]   = options[:controller].to_s
           end
 
           if options[:action]
-            options[:action] = options[:action].to_s.dup
+            options[:action] = options[:action].to_s
           end
         end
 
@@ -504,10 +504,15 @@ module ActionDispatch
             old_parts = current_controller.split('/')
             size = controller.count("/") + 1
             parts = old_parts[0...-size] << controller
-            @controller = @options[:controller] = parts.join("/")
+            @options[:controller] = parts.join("/")
           end
         end
 
+        # Remove leading slashes from controllers
+        def normalize_controller!
+          @options[:controller] = controller.sub(%r{^/}, '') if controller
+        end
+
         # This handles the case of :action => nil being explicitly passed.
         # It is identical to :action => "index"
         def handle_nil_action!
diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb
index 91ca058a5d..46d16598f7 100644
--- a/actionpack/test/dispatch/routing_test.rb
+++ b/actionpack/test/dispatch/routing_test.rb
@@ -887,13 +887,13 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest
     assert_equal original_options, options
   end
 
-  # checks that url_for doesn't change controller and action
-  def test_url_for_with_no_side_effects_on_strings
-    # freeze controller and action to be sure they are not changed
-    # we'll get RuntimeError if somebody tries to modify them
-    options = {:controller => '/projects'.freeze, :action => 'status'.freeze}
+  def test_url_for_does_not_modify_controller
+    controller = '/projects'
+    options = {:controller => controller, :action => 'status', :only_path => true}
+    url = url_for(options)
 
-    url_for options
+    assert_equal '/projects/status', url
+    assert_equal '/projects', controller
   end
 
   # tests the arguments modification free version of define_hash_access