Permalink
Browse files

Privatize Routing.possible_controllers and fix brittle url helper

controller test loading.
  • Loading branch information...
1 parent 15ab3a9 commit 59dbae145b939ca733b7fc9ed0205b01c3d6799c @josh josh committed Nov 24, 2009
View
66 actionpack/lib/action_dispatch/routing.rb
@@ -277,45 +277,23 @@ module Helpers
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 @@ def possible_controllers
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
View
24 actionpack/test/controller/routing_test.rb
@@ -51,30 +51,6 @@ def test_route_generation_allows_passing_non_string_values_to_generated_helper
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
View
45 actionpack/test/template/url_helper_test.rb
@@ -391,54 +391,47 @@ def sort_query_string_params(uri)
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,15 +443,15 @@ def default_url_options(options = nil)
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
protected
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

0 comments on commit 59dbae1

Please sign in to comment.