Merge branch 'master' of git://github.com/rails/rails

7 files changed, 64 insertions, 150 deletions

diff --git a/actionpack/lib/action_dispatch/routing.rb b/actionpack/lib/action_dispatch/routing.rb
index 68ed1e3340..9b977800b4 100644
--- a/actionpack/lib/action_dispatch/routing.rb
+++ b/actionpack/lib/action_dispatch/routing.rb
@@ -264,11 +264,8 @@ module ActionDispatch
     autoload :RouteSet, 'action_dispatch/routing/route_set'
 
     SEPARATORS = %w( / . ? )
-
     HTTP_METHODS = [:get, :head, :post, :put, :delete, :options]
 
-    ALLOWED_REQUIREMENTS_FOR_OPTIMISATION = [:controller, :action].to_set
-
     # The root paths which may contain controller files
     mattr_accessor :controller_paths
     self.controller_paths = []
@@ -279,46 +276,28 @@ module ActionDispatch
     end
 
     class << self
-      # Expects an array of controller names as the first argument.
-      # Executes the passed block with only the named controllers named available.
-      # This method is used in internal Rails testing.
-      def with_controllers(names)
-        prior_controllers = @possible_controllers
-        use_controllers! names
-        yield
-      ensure
-        use_controllers! prior_controllers
+      def controller_constraints
+        @controller_constraints ||= Regexp.union(*possible_controllers.collect { |n| Regexp.escape(n) })
       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 }
+      def clear_controller_cache!
+        @controller_constraints = nil
       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 = []
 
-          paths = controller_paths.select { |path| File.directory?(path) && path != "." }
+          # 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
+          end
 
+          # Find controllers in controllers/ directory
+          paths = controller_paths.select { |path| File.directory?(path) && path != "." }
           seen_paths = Hash.new {|h, k| h[k] = true; false}
           normalize_paths(paths).each do |load_path|
             Dir["#{load_path}/**/*_controller.rb"].collect do |path|
@@ -327,55 +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!
-        end
-        @possible_controllers
-      end
-
-      # Replaces the internal list of controllers available to ActionController::Routing with the passed argument.
-      #   ActionController::Routing.use_controllers!([ "posts", "comments", "admin/comments" ])
-      def use_controllers!(controller_names)
-        @possible_controllers = controller_names
-      end
+          possible_controllers.uniq!
 
-      # Returns a controller path for a new +controller+ based on a +previous+ controller path.
-      # Handles 4 scenarios:
-      #
-      # * stay in the previous controller:
-      #     controller_relative_to( nil, "groups/discussion" ) # => "groups/discussion"
-      #
-      # * stay in the previous namespace:
-      #     controller_relative_to( "posts", "groups/discussion" ) # => "groups/posts"
-      #
-      # * forced move to the root namespace:
-      #     controller_relative_to( "/posts", "groups/discussion" ) # => "posts"
-      #
-      # * previous namespace is root:
-      #     controller_relative_to( "posts", "anything_with_no_slashes" ) # =>"posts"
-      #
-      def controller_relative_to(controller, previous)
-        if controller.nil?           then previous
-        elsif controller[0] == ?/    then controller[1..-1]
-        elsif %r{^(.*)/} =~ previous then "#{$1}/#{controller}"
-        else controller
+          possible_controllers
         end
-      end
-    end
 
-    ActiveSupport::Inflector.module_eval do
-      # Ensures that routes are reloaded when Rails inflections are updated.
-      def inflections_with_route_reloading(&block)
-        returning(inflections_without_route_reloading(&block)) {
-          ActionDispatch::Routing::Routes.reload! if block_given?
-        }
-      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
 
-      alias_method_chain :inflections, :route_reloading
+          # start with longest path, first
+          paths = paths.uniq.sort_by { |path| - path.length }
+        end
     end
   end
 end
diff --git a/actionpack/lib/action_dispatch/routing/deprecated_mapper.rb b/actionpack/lib/action_dispatch/routing/deprecated_mapper.rb
index 0564ba9797..dd76391870 100644
--- a/actionpack/lib/action_dispatch/routing/deprecated_mapper.rb
+++ b/actionpack/lib/action_dispatch/routing/deprecated_mapper.rb
@@ -113,8 +113,7 @@ module ActionDispatch
           end
         end
 
-        possible_names = Routing.possible_controllers.collect { |n| Regexp.escape(n) }
-        requirements[:controller] ||= Regexp.union(*possible_names)
+        requirements[:controller] ||= Routing.controller_constraints
 
         if defaults[:controller]
           defaults[:action] ||= 'index'
diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb
index 6e112c9b54..cfe7425a61 100644
--- a/actionpack/lib/action_dispatch/routing/mapper.rb
+++ b/actionpack/lib/action_dispatch/routing/mapper.rb
@@ -282,6 +282,8 @@ module ActionDispatch
         constraints.reject! { |k, v| segment_keys.include?(k.to_s) }
         conditions.merge!(constraints)
 
+        requirements[:controller] ||= Routing.controller_constraints
+
         if via = options[:via]
           via = Array(via).map { |m| m.to_s.upcase }
           conditions[:request_method] = Regexp.union(*via)
diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb
index c15aaceb5b..79e15edeaa 100644
--- a/actionpack/lib/action_dispatch/routing/route_set.rb
+++ b/actionpack/lib/action_dispatch/routing/route_set.rb
@@ -246,7 +246,9 @@ module ActionDispatch
       end
 
       def load!
-        Routing.use_controllers!(nil) # Clear the controller cache so we may discover new ones
+        # Clear the controller cache so we may discover new ones
+        Routing.clear_controller_cache!
+
         load_routes!
       end
 
diff --git a/actionpack/test/abstract_unit.rb b/actionpack/test/abstract_unit.rb
index 775cfc82bf..9d055da4b9 100644
--- a/actionpack/test/abstract_unit.rb
+++ b/actionpack/test/abstract_unit.rb
@@ -191,20 +191,8 @@ class ::ApplicationController < ActionController::Base
 end
 
 module ActionController
-  module Routing
-    def self.possible_controllers
-      @@possible_controllers ||= []
-    end
-  end
-
   class Base
     include ActionController::Testing
-
-    def self.inherited(klass)
-      name = klass.name.underscore.sub(/_controller$/, '')
-      ActionController::Routing.possible_controllers << name unless name.blank?
-      super
-    end
   end
 
   Base.view_paths = FIXTURE_LOAD_PATH
diff --git a/actionpack/test/controller/routing_test.rb b/actionpack/test/controller/routing_test.rb
index 4eaf309c41..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
 
@@ -1762,13 +1738,6 @@ class RouteLoadingTest < Test::Unit::TestCase
     2.times { routes.reload! }
   end
 
-  def test_adding_inflections_forces_reload
-    ActiveSupport::Inflector::Inflections.instance.expects(:uncountable).with('equipment')
-    routes.expects(:reload!)
-
-    ActiveSupport::Inflector.inflections { |inflect| inflect.uncountable('equipment') }
-  end
-
   def test_load_with_configuration
     routes.configuration_files.clear
     routes.add_configuration_file("foobarbaz")
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