Privatize Routing.possible_controllers and fix brittle url helper

controller test loading.

3 files changed, 52 insertions, 83 deletions

diff --git a/actionpack/lib/action_dispatch/routing.rb b/actionpack/lib/action_dispatch/routing.rb
index b43b77fecb..9b977800b4 100644
--- a/actionpack/lib/action_dispatch/routing.rb
+++ b/actionpack/lib/action_dispatch/routing.rb
@@ -277,45 +277,23 @@ module ActionDispatch
 
     class << self
       def controller_constraints
-        Regexp.union(*possible_controllers.collect { |n| Regexp.escape(n) })
+        @controller_constraints ||= Regexp.union(*possible_controllers.collect { |n| Regexp.escape(n) })
       end
 
       def clear_controller_cache!
-        @possible_controllers = nil
+        @controller_constraints = nil
       end
 
-      # Returns an array of paths, cleaned of double-slashes and relative path references.
-      # * "\\\" and "//"  become "\\" or "/".
-      # * "/foo/bar/../config" becomes "/foo/config".
-      # The returned array is sorted by length, descending.
-      def normalize_paths(paths)
-        # do the hokey-pokey of path normalization...
-        paths = paths.collect do |path|
-          path = path.
-            gsub("//", "/").           # replace double / chars with a single
-            gsub("\\\\", "\\").        # replace double \ chars with a single
-            gsub(%r{(.)[\\/]$}, '\1')  # drop final / or \ if path ends with it
-
-          # eliminate .. paths where possible
-          re = %r{[^/\\]+[/\\]\.\.[/\\]}
-          path.gsub!(re, "") while path.match(re)
-          path
-        end
-
-        # start with longest path, first
-        paths = paths.uniq.sort_by { |path| - path.length }
-      end
-
-      # Returns the array of controller names currently available to ActionController::Routing.
-      def possible_controllers
-        unless @possible_controllers
-          @possible_controllers = []
+      private
+        # Returns the array of controller names currently available to ActionController::Routing.
+        def possible_controllers
+          possible_controllers = []
 
           # Find any controller classes already in memory
           ActionController::Base.subclasses.each do |klass|
             controller_name = klass.underscore
             controller_name.gsub!(/_controller\Z/, '')
-            @possible_controllers << controller_name
+            possible_controllers << controller_name
           end
 
           # Find controllers in controllers/ directory
@@ -328,15 +306,37 @@ module ActionDispatch
               controller_name = path[(load_path.length + 1)..-1]
 
               controller_name.gsub!(/_controller\.rb\Z/, '')
-              @possible_controllers << controller_name
+              possible_controllers << controller_name
             end
           end
 
           # remove duplicates
-          @possible_controllers.uniq!
+          possible_controllers.uniq!
+
+          possible_controllers
+        end
+
+        # Returns an array of paths, cleaned of double-slashes and relative path references.
+        # * "\\\" and "//"  become "\\" or "/".
+        # * "/foo/bar/../config" becomes "/foo/config".
+        # The returned array is sorted by length, descending.
+        def normalize_paths(paths)
+          # do the hokey-pokey of path normalization...
+          paths = paths.collect do |path|
+            path = path.
+              gsub("//", "/").           # replace double / chars with a single
+              gsub("\\\\", "\\").        # replace double \ chars with a single
+              gsub(%r{(.)[\\/]$}, '\1')  # drop final / or \ if path ends with it
+
+            # eliminate .. paths where possible
+            re = %r{[^/\\]+[/\\]\.\.[/\\]}
+            path.gsub!(re, "") while path.match(re)
+            path
+          end
+
+          # start with longest path, first
+          paths = paths.uniq.sort_by { |path| - path.length }
         end
-        @possible_controllers
-      end
     end
   end
 end
diff --git a/actionpack/test/controller/routing_test.rb b/actionpack/test/controller/routing_test.rb
index a2fdbfbbbe..b5effeda40 100644
--- a/actionpack/test/controller/routing_test.rb
+++ b/actionpack/test/controller/routing_test.rb
@@ -51,30 +51,6 @@ class UriReservedCharactersRoutingTest < Test::Unit::TestCase
   end
 end
 
-class RoutingTest < Test::Unit::TestCase
-  def test_normalize_unix_paths
-    load_paths = %w(. config/../app/controllers config/../app//helpers script/../config/../vendor/rails/actionpack/lib vendor/rails/railties/builtin/rails_info app/models lib script/../config/../foo/bar/../../app/models .foo/../.bar foo.bar/../config)
-    paths = ActionController::Routing.normalize_paths(load_paths)
-    assert_equal %w(vendor/rails/railties/builtin/rails_info vendor/rails/actionpack/lib app/controllers app/helpers app/models config .bar lib .), paths
-  end
-
-  def test_normalize_windows_paths
-    load_paths = %w(. config\\..\\app\\controllers config\\..\\app\\\\helpers script\\..\\config\\..\\vendor\\rails\\actionpack\\lib vendor\\rails\\railties\\builtin\\rails_info app\\models lib script\\..\\config\\..\\foo\\bar\\..\\..\\app\\models .foo\\..\\.bar foo.bar\\..\\config)
-    paths = ActionController::Routing.normalize_paths(load_paths)
-    assert_equal %w(vendor\\rails\\railties\\builtin\\rails_info vendor\\rails\\actionpack\\lib app\\controllers app\\helpers app\\models config .bar lib .), paths
-  end
-
-  def test_routing_helper_module
-    assert_kind_of Module, ActionController::Routing::Helpers
-
-    h = ActionController::Routing::Helpers
-    c = Class.new
-    assert ! c.ancestors.include?(h)
-    ActionController::Routing::Routes.install_helpers c
-    assert c.ancestors.include?(h)
-  end
-end
-
 class MockController
   attr_accessor :routes
 
diff --git a/actionpack/test/template/url_helper_test.rb b/actionpack/test/template/url_helper_test.rb
index bf0b4ad3a7..5c463a4f60 100644
--- a/actionpack/test/template/url_helper_test.rb
+++ b/actionpack/test/template/url_helper_test.rb
@@ -391,54 +391,47 @@ class UrlHelperTest < ActionView::TestCase
     end
 end
 
-class UrlHelperController < ActionController::Base
-  def self.controller_path; 'url_helper_with_controller' end
+class UrlHelperControllerTest < ActionController::TestCase
+  class UrlHelperController < ActionController::Base
+    def show_url_for
+      render :inline => "<%= url_for :controller => 'url_helper_controller_test/url_helper', :action => 'show_url_for' %>"
+    end
 
-  def show_url_for
-    render :inline => "<%= url_for :controller => 'url_helper_with_controller', :action => 'show_url_for' %>"
-  end
+    def show_named_route
+      render :inline => "<%= show_named_route_#{params[:kind]} %>"
+    end
 
-  def show_named_route
-    render :inline => "<%= show_named_route_#{params[:kind]} %>"
-  end
+    def nil_url_for
+      render :inline => '<%= url_for(nil) %>'
+    end
 
-  def nil_url_for
-    render :inline => '<%= url_for(nil) %>'
+    def rescue_action(e) raise e end
   end
 
-  def rescue_action(e) raise e end
-end
-
-class UrlHelperWithControllerTest < ActionController::TestCase
-  def setup
-    super
-    @request    = ActionController::TestRequest.new
-    @response   = ActionController::TestResponse.new
-    @controller = UrlHelperController.new
-  end
+  tests UrlHelperController
 
   def test_url_for_shows_only_path
     get :show_url_for
-    assert_equal '/url_helper_with_controller/show_url_for', @response.body
+    assert_equal '/url_helper_controller_test/url_helper/show_url_for', @response.body
   end
 
   def test_named_route_url_shows_host_and_path
     with_url_helper_routing do
       get :show_named_route, :kind => 'url'
-      assert_equal 'http://test.host/url_helper_with_controller/show_named_route', @response.body
+      assert_equal 'http://test.host/url_helper_controller_test/url_helper/show_named_route', @response.body
     end
   end
 
   def test_named_route_path_shows_only_path
     with_url_helper_routing do
       get :show_named_route, :kind => 'path'
-      assert_equal '/url_helper_with_controller/show_named_route', @response.body
+      assert_equal '/url_helper_controller_test/url_helper/show_named_route', @response.body
     end
   end
 
   def test_url_for_nil_returns_current_path
     get :nil_url_for
-    assert_equal '/url_helper/nil_url_for', @response.body
+    assert_equal '/url_helper_controller_test/url_helper/nil_url_for', @response.body
   end
 
   def test_named_route_should_show_host_and_path_using_controller_default_url_options
@@ -450,7 +443,7 @@ class UrlHelperWithControllerTest < ActionController::TestCase
 
     with_url_helper_routing do
       get :show_named_route, :kind => 'url'
-      assert_equal 'http://testtwo.host/url_helper_with_controller/show_named_route', @response.body
+      assert_equal 'http://testtwo.host/url_helper_controller_test/url_helper/show_named_route', @response.body
     end
   end
 
@@ -458,7 +451,7 @@ class UrlHelperWithControllerTest < ActionController::TestCase
     def with_url_helper_routing
       with_routing do |set|
         set.draw do |map|
-          map.show_named_route 'url_helper_with_controller/show_named_route', :controller => 'url_helper', :action => 'show_named_route'
+          map.show_named_route 'url_helper_controller_test/url_helper/show_named_route', :controller => 'url_helper_controller_test/url_helper', :action => 'show_named_route'
         end
         yield
       end