From 226dfc2681c98deaf14e4ae82e973d1d5caedd68 Mon Sep 17 00:00:00 2001 From: Carlhuda Date: Wed, 24 Feb 2010 16:01:03 -0800 Subject: [PATCH] WIP: Remove the global router --- actionpack/lib/action_controller.rb | 2 +- actionpack/lib/action_controller/base.rb | 2 +- .../lib/action_controller/deprecated.rb | 3 +- .../lib/action_controller/metal/head.rb | 4 +- .../action_controller/metal/redirecting.rb | 4 +- .../lib/action_controller/metal/url_for.rb | 165 ----------------- actionpack/lib/action_controller/test_case.rb | 10 +- .../lib/action_controller/url_rewriter.rb | 9 +- actionpack/lib/action_dispatch/routing.rb | 1 + .../lib/action_dispatch/routing/route_set.rb | 30 ++++ .../lib/action_dispatch/routing/routes.rb | 2 +- .../lib/action_dispatch/routing/url_for.rb | 167 ++++++++++++++++++ .../testing/assertions/routing.rb | 27 ++- .../action_dispatch/testing/integration.rb | 10 +- actionpack/lib/action_view/test_case.rb | 3 +- actionpack/test/abstract_unit.rb | 69 +++++--- .../controller/action_pack_assertions_test.rb | 9 +- actionpack/test/controller/base_test.rb | 4 +- .../test/controller/integration_test.rb | 11 +- actionpack/test/controller/rescue_test.rb | 4 +- actionpack/test/controller/resources_test.rb | 13 +- actionpack/test/controller/test_test.rb | 8 +- actionpack/test/controller/url_for_test.rb | 2 +- .../test/controller/url_rewriter_test.rb | 32 ++-- actionpack/test/dispatch/routing_test.rb | 10 +- actionpack/test/template/test_case_test.rb | 2 +- 26 files changed, 341 insertions(+), 262 deletions(-) delete mode 100644 actionpack/lib/action_controller/metal/url_for.rb create mode 100644 actionpack/lib/action_dispatch/routing/url_for.rb diff --git a/actionpack/lib/action_controller.rb b/actionpack/lib/action_controller.rb index 759e52b135..93d8fb612f 100644 --- a/actionpack/lib/action_controller.rb +++ b/actionpack/lib/action_controller.rb @@ -32,7 +32,7 @@ module ActionController autoload :SessionManagement autoload :Streaming autoload :Testing - autoload :UrlFor + # ROUTES TODO: Proxy UrlFor to Rails.application.routes.url_helpers autoload :Verification end diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb index 7f1aa95f6f..0777d9dc16 100644 --- a/actionpack/lib/action_controller/base.rb +++ b/actionpack/lib/action_controller/base.rb @@ -9,7 +9,7 @@ module ActionController include ActionController::Helpers include ActionController::HideActions - include ActionController::UrlFor + # include ActionController::UrlFor include ActionController::Redirecting include ActionController::Rendering include ActionController::Renderers::All diff --git a/actionpack/lib/action_controller/deprecated.rb b/actionpack/lib/action_controller/deprecated.rb index 50c4058845..6088d97abe 100644 --- a/actionpack/lib/action_controller/deprecated.rb +++ b/actionpack/lib/action_controller/deprecated.rb @@ -1,4 +1,5 @@ ActionController::AbstractRequest = ActionController::Request = ActionDispatch::Request ActionController::AbstractResponse = ActionController::Response = ActionDispatch::Response ActionController::Routing = ActionDispatch::Routing -ActionController::UrlWriter = ActionController::UrlFor +# ROUTES TODO: Figure out how to deprecate this. +# ActionController::UrlWriter = ActionController::UrlFor diff --git a/actionpack/lib/action_controller/metal/head.rb b/actionpack/lib/action_controller/metal/head.rb index 37be8b3999..fe6e6186a5 100644 --- a/actionpack/lib/action_controller/metal/head.rb +++ b/actionpack/lib/action_controller/metal/head.rb @@ -1,7 +1,6 @@ module ActionController module Head extend ActiveSupport::Concern - include ActionController::UrlFor # Return a response that has no content (merely headers). The options # argument is interpreted to be a hash of header names and values. @@ -25,6 +24,9 @@ module ActionController end self.status = status + # ROUTES TODO: Figure out how to rescue from a no method error + # This is needed only if you wire up a controller yourself, and + # this not working would be baffling without a better error self.location = url_for(location) if location self.content_type = Mime[formats.first] self.response_body = " " diff --git a/actionpack/lib/action_controller/metal/redirecting.rb b/actionpack/lib/action_controller/metal/redirecting.rb index 25e4e18493..de40ea77b1 100644 --- a/actionpack/lib/action_controller/metal/redirecting.rb +++ b/actionpack/lib/action_controller/metal/redirecting.rb @@ -12,7 +12,6 @@ module ActionController include AbstractController::Logger include ActionController::RackDelegation - include ActionController::UrlFor # Redirects the browser to the target specified in +options+. This parameter can take one of three forms: # @@ -84,6 +83,9 @@ module ActionController raise RedirectBackError unless refer = request.headers["Referer"] refer else + # ROUTES TODO: Figure out how to rescue from a no method error + # This is needed only if you wire up a controller yourself, and + # this not working would be baffling without a better error url_for(options) end.gsub(/[\r\n]/, '') end diff --git a/actionpack/lib/action_controller/metal/url_for.rb b/actionpack/lib/action_controller/metal/url_for.rb deleted file mode 100644 index 0a9ea7fe1a..0000000000 --- a/actionpack/lib/action_controller/metal/url_for.rb +++ /dev/null @@ -1,165 +0,0 @@ -require 'active_support/core_ext/class/attribute' -require 'active_support/core_ext/module/attribute_accessors' - -module ActionController - # In routes.rb one defines URL-to-controller mappings, but the reverse - # is also possible: an URL can be generated from one of your routing definitions. - # URL generation functionality is centralized in this module. - # - # See ActionDispatch::Routing and ActionController::Resources for general - # information about routing and routes.rb. - # - # Tip: If you need to generate URLs from your models or some other place, - # then ActionController::UrlFor is what you're looking for. Read on for - # an introduction. - # - # == URL generation from parameters - # - # As you may know, some functions - such as ActionController::Base#url_for - # and ActionView::Helpers::UrlHelper#link_to, can generate URLs given a set - # of parameters. For example, you've probably had the chance to write code - # like this in one of your views: - # - # <%= link_to('Click here', :controller => 'users', - # :action => 'new', :message => 'Welcome!') %> - # - # #=> Generates a link to: /users/new?message=Welcome%21 - # - # link_to, and all other functions that require URL generation functionality, - # actually use ActionController::UrlFor under the hood. And in particular, - # they use the ActionController::UrlFor#url_for method. One can generate - # the same path as the above example by using the following code: - # - # include UrlFor - # url_for(:controller => 'users', - # :action => 'new', - # :message => 'Welcome!', - # :only_path => true) - # # => "/users/new?message=Welcome%21" - # - # Notice the :only_path => true part. This is because UrlFor has no - # information about the website hostname that your Rails app is serving. So if you - # want to include the hostname as well, then you must also pass the :host - # argument: - # - # include UrlFor - # url_for(:controller => 'users', - # :action => 'new', - # :message => 'Welcome!', - # :host => 'www.example.com') # Changed this. - # # => "http://www.example.com/users/new?message=Welcome%21" - # - # By default, all controllers and views have access to a special version of url_for, - # that already knows what the current hostname is. So if you use url_for in your - # controllers or your views, then you don't need to explicitly pass the :host - # argument. - # - # For convenience reasons, mailers provide a shortcut for ActionController::UrlFor#url_for. - # So within mailers, you only have to type 'url_for' instead of 'ActionController::UrlFor#url_for' - # in full. However, mailers don't have hostname information, and what's why you'll still - # have to specify the :host argument when generating URLs in mailers. - # - # - # == URL generation for named routes - # - # UrlFor also allows one to access methods that have been auto-generated from - # named routes. For example, suppose that you have a 'users' resource in your - # routes.rb: - # - # map.resources :users - # - # This generates, among other things, the method users_path. By default, - # this method is accessible from your controllers, views and mailers. If you need - # to access this auto-generated method from other places (such as a model), then - # you can do that by including ActionController::UrlFor in your class: - # - # class User < ActiveRecord::Base - # include ActionController::UrlFor - # - # def base_uri - # user_path(self) - # end - # end - # - # User.find(1).base_uri # => "/users/1" - # - module UrlFor - extend ActiveSupport::Concern - - included do - ActionDispatch::Routing::Routes.install_helpers(self) - - # Including in a class uses an inheritable hash. Modules get a plain hash. - if respond_to?(:class_attribute) - class_attribute :default_url_options - else - mattr_accessor :default_url_options - end - - self.default_url_options = {} - end - - # Overwrite to implement a number of default options that all url_for-based methods will use. The default options should come in - # the form of a hash, just like the one you would use for url_for directly. Example: - # - # def default_url_options(options) - # { :project => @project.active? ? @project.url_name : "unknown" } - # end - # - # As you can infer from the example, this is mostly useful for situations where you want to centralize dynamic decisions about the - # urls as they stem from the business domain. Please note that any individual url_for call can always override the defaults set - # by this method. - def default_url_options(options = nil) - self.class.default_url_options - end - - def rewrite_options(options) #:nodoc: - if options.delete(:use_defaults) != false && (defaults = default_url_options(options)) - defaults.merge(options) - else - options - end - end - - # Generate a url based on the options provided, default_url_options and the - # routes defined in routes.rb. The following options are supported: - # - # * :only_path - If true, the relative url is returned. Defaults to +false+. - # * :protocol - The protocol to connect to. Defaults to 'http'. - # * :host - Specifies the host the link should be targeted at. - # If :only_path is false, this option must be - # provided either explicitly, or via +default_url_options+. - # * :port - Optionally specify the port to connect to. - # * :anchor - An anchor name to be appended to the path. - # * :skip_relative_url_root - If true, the url is not constructed using the - # +relative_url_root+ set in ActionController::Base.relative_url_root. - # * :trailing_slash - If true, adds a trailing slash, as in "/archive/2009/" - # - # Any other key (:controller, :action, etc.) given to - # +url_for+ is forwarded to the Routes module. - # - # Examples: - # - # url_for :controller => 'tasks', :action => 'testing', :host=>'somehost.org', :port=>'8080' # => 'http://somehost.org:8080/tasks/testing' - # url_for :controller => 'tasks', :action => 'testing', :host=>'somehost.org', :anchor => 'ok', :only_path => true # => '/tasks/testing#ok' - # url_for :controller => 'tasks', :action => 'testing', :trailing_slash=>true # => 'http://somehost.org/tasks/testing/' - # url_for :controller => 'tasks', :action => 'testing', :host=>'somehost.org', :number => '33' # => 'http://somehost.org/tasks/testing?number=33' - def url_for(options = {}) - options ||= {} - case options - when String - options - when Hash - _url_rewriter.rewrite(rewrite_options(options)) - else - polymorphic_url(options) - end - end - - protected - - def _url_rewriter - ActionController::UrlRewriter - end - end -end diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index 99924205dd..5f8bc6f325 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -17,9 +17,9 @@ module ActionController end end - def assign_parameters(controller_path, action, parameters = {}) + def assign_parameters(router, controller_path, action, parameters = {}) parameters = parameters.symbolize_keys.merge(:controller => controller_path, :action => action) - extra_keys = ActionDispatch::Routing::Routes.extra_keys(parameters) + extra_keys = router.extra_keys(parameters) non_path_parameters = get? ? query_parameters : request_parameters parameters.each do |key, value| if value.is_a? Fixnum @@ -220,7 +220,7 @@ module ActionController def process(action, parameters = nil, session = nil, flash = nil, http_method = 'GET') # Sanity check for required instance variables so we can give an # understandable error message. - %w(@controller @request @response).each do |iv_name| + %w(@router @controller @request @response).each do |iv_name| if !(instance_variable_names.include?(iv_name) || instance_variable_names.include?(iv_name.to_sym)) || instance_variable_get(iv_name).nil? raise "#{iv_name} is nil: make sure you set it in your test's setup method." end @@ -236,7 +236,7 @@ module ActionController @request.env['REQUEST_METHOD'] = http_method parameters ||= {} - @request.assign_parameters(@controller.class.name.underscore.sub(/_controller$/, ''), action.to_s, parameters) + @request.assign_parameters(@router, @controller.class.name.underscore.sub(/_controller$/, ''), action.to_s, parameters) @request.session = ActionController::TestSession.new(session) unless session.nil? @request.session["flash"] = @request.flash.update(flash || {}) @@ -340,7 +340,7 @@ module ActionController options.update(:only_path => true, :action => action) url = ActionController::UrlRewriter.new(@request, parameters) - @request.request_uri = url.rewrite(options) + @request.request_uri = url.rewrite(@router, options) end end end diff --git a/actionpack/lib/action_controller/url_rewriter.rb b/actionpack/lib/action_controller/url_rewriter.rb index 933a1fa8f9..272465d4f5 100644 --- a/actionpack/lib/action_controller/url_rewriter.rb +++ b/actionpack/lib/action_controller/url_rewriter.rb @@ -9,11 +9,11 @@ module ActionController @request, @parameters = request, parameters end - def rewrite(options = {}) + def rewrite(router, options = {}) options[:host] ||= @request.host_with_port options[:protocol] ||= @request.protocol - self.class.rewrite(options, @request.symbolized_path_parameters) do |options| + self.class.rewrite(router, options, @request.symbolized_path_parameters) do |options| process_path_options(options) end end @@ -24,7 +24,8 @@ module ActionController alias_method :to_s, :to_str - def self.rewrite(options, path_segments=nil) + # ROUTES TODO: Class method code smell + def self.rewrite(router, options, path_segments=nil) rewritten_url = "" unless options[:only_path] @@ -40,7 +41,7 @@ module ActionController path_options = options.except(*RESERVED_OPTIONS) path_options = yield(path_options) if block_given? - path = Routing::Routes.generate(path_options, path_segments || {}) + path = router.generate(path_options, path_segments || {}) rewritten_url << ActionController::Base.relative_url_root.to_s unless options[:skip_relative_url_root] rewritten_url << (options[:trailing_slash] ? path.sub(/\?|\z/) { "/" + $& } : path) diff --git a/actionpack/lib/action_dispatch/routing.rb b/actionpack/lib/action_dispatch/routing.rb index 325d2f7f04..2cc7fe5344 100644 --- a/actionpack/lib/action_dispatch/routing.rb +++ b/actionpack/lib/action_dispatch/routing.rb @@ -206,6 +206,7 @@ module ActionDispatch autoload :Route, 'action_dispatch/routing/route' autoload :Routes, 'action_dispatch/routing/routes' autoload :RouteSet, 'action_dispatch/routing/route_set' + autoload :UrlFor, 'action_dispatch/routing/url_for' SEPARATORS = %w( / . ? ) HTTP_METHODS = [:get, :head, :post, :put, :delete, :options] diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index 8778fd2932..8b761d393f 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -272,6 +272,36 @@ module ActionDispatch named_routes.install(destinations, regenerate_code) end + # ROUTES TODO: Revisit the name of these methods + def url_helpers + @url_helpers ||= begin + router = self + Module.new do + extend ActiveSupport::Concern + include UrlFor + + define_method(:_router) { router } + end + end + end + + def named_url_helpers + @named_url_helpers ||= begin + router = self + + Module.new do + extend ActiveSupport::Concern + include router.url_helpers + + # ROUTES TODO: install_helpers isn't great... can we make a module with the stuff that + # we can include? + included do + router.install_helpers(self) + end + end + end + end + def empty? routes.empty? end diff --git a/actionpack/lib/action_dispatch/routing/routes.rb b/actionpack/lib/action_dispatch/routing/routes.rb index 4714556d36..34afada9c9 100644 --- a/actionpack/lib/action_dispatch/routing/routes.rb +++ b/actionpack/lib/action_dispatch/routing/routes.rb @@ -1,5 +1,5 @@ # A singleton that stores the current route set -ActionDispatch::Routing::Routes = ActionDispatch::Routing::RouteSet.new +# ActionDispatch::Routing::Routes = ActionDispatch::Routing::RouteSet.new # To preserve compatibility with pre-3.0 Rails action_controller/deprecated.rb # defines ActionDispatch::Routing::Routes as an alias diff --git a/actionpack/lib/action_dispatch/routing/url_for.rb b/actionpack/lib/action_dispatch/routing/url_for.rb new file mode 100644 index 0000000000..64c695fe13 --- /dev/null +++ b/actionpack/lib/action_dispatch/routing/url_for.rb @@ -0,0 +1,167 @@ +module ActionDispatch + module Routing + # In routes.rb one defines URL-to-controller mappings, but the reverse + # is also possible: an URL can be generated from one of your routing definitions. + # URL generation functionality is centralized in this module. + # + # See ActionDispatch::Routing and ActionController::Resources for general + # information about routing and routes.rb. + # + # Tip: If you need to generate URLs from your models or some other place, + # then ActionController::UrlFor is what you're looking for. Read on for + # an introduction. + # + # == URL generation from parameters + # + # As you may know, some functions - such as ActionController::Base#url_for + # and ActionView::Helpers::UrlHelper#link_to, can generate URLs given a set + # of parameters. For example, you've probably had the chance to write code + # like this in one of your views: + # + # <%= link_to('Click here', :controller => 'users', + # :action => 'new', :message => 'Welcome!') %> + # + # #=> Generates a link to: /users/new?message=Welcome%21 + # + # link_to, and all other functions that require URL generation functionality, + # actually use ActionController::UrlFor under the hood. And in particular, + # they use the ActionController::UrlFor#url_for method. One can generate + # the same path as the above example by using the following code: + # + # include UrlFor + # url_for(:controller => 'users', + # :action => 'new', + # :message => 'Welcome!', + # :only_path => true) + # # => "/users/new?message=Welcome%21" + # + # Notice the :only_path => true part. This is because UrlFor has no + # information about the website hostname that your Rails app is serving. So if you + # want to include the hostname as well, then you must also pass the :host + # argument: + # + # include UrlFor + # url_for(:controller => 'users', + # :action => 'new', + # :message => 'Welcome!', + # :host => 'www.example.com') # Changed this. + # # => "http://www.example.com/users/new?message=Welcome%21" + # + # By default, all controllers and views have access to a special version of url_for, + # that already knows what the current hostname is. So if you use url_for in your + # controllers or your views, then you don't need to explicitly pass the :host + # argument. + # + # For convenience reasons, mailers provide a shortcut for ActionController::UrlFor#url_for. + # So within mailers, you only have to type 'url_for' instead of 'ActionController::UrlFor#url_for' + # in full. However, mailers don't have hostname information, and what's why you'll still + # have to specify the :host argument when generating URLs in mailers. + # + # + # == URL generation for named routes + # + # UrlFor also allows one to access methods that have been auto-generated from + # named routes. For example, suppose that you have a 'users' resource in your + # routes.rb: + # + # map.resources :users + # + # This generates, among other things, the method users_path. By default, + # this method is accessible from your controllers, views and mailers. If you need + # to access this auto-generated method from other places (such as a model), then + # you can do that by including ActionController::UrlFor in your class: + # + # class User < ActiveRecord::Base + # include ActionController::UrlFor + # + # def base_uri + # user_path(self) + # end + # end + # + # User.find(1).base_uri # => "/users/1" + # + module UrlFor + extend ActiveSupport::Concern + + included do + # ActionDispatch::Routing::Routes.install_helpers(self) + + # Including in a class uses an inheritable hash. Modules get a plain hash. + if respond_to?(:class_attribute) + class_attribute :default_url_options + else + mattr_accessor :default_url_options + end + + self.default_url_options = {} + end + + # Overwrite to implement a number of default options that all url_for-based methods will use. The default options should come in + # the form of a hash, just like the one you would use for url_for directly. Example: + # + # def default_url_options(options) + # { :project => @project.active? ? @project.url_name : "unknown" } + # end + # + # As you can infer from the example, this is mostly useful for situations where you want to centralize dynamic decisions about the + # urls as they stem from the business domain. Please note that any individual url_for call can always override the defaults set + # by this method. + def default_url_options(options = nil) + # ROUTES TODO: This should probably be an instance method + self.class.default_url_options + end + + def rewrite_options(options) #:nodoc: + if options.delete(:use_defaults) != false && (defaults = default_url_options(options)) + defaults.merge(options) + else + options + end + end + + # Generate a url based on the options provided, default_url_options and the + # routes defined in routes.rb. The following options are supported: + # + # * :only_path - If true, the relative url is returned. Defaults to +false+. + # * :protocol - The protocol to connect to. Defaults to 'http'. + # * :host - Specifies the host the link should be targeted at. + # If :only_path is false, this option must be + # provided either explicitly, or via +default_url_options+. + # * :port - Optionally specify the port to connect to. + # * :anchor - An anchor name to be appended to the path. + # * :skip_relative_url_root - If true, the url is not constructed using the + # +relative_url_root+ set in ActionController::Base.relative_url_root. + # * :trailing_slash - If true, adds a trailing slash, as in "/archive/2009/" + # + # Any other key (:controller, :action, etc.) given to + # +url_for+ is forwarded to the Routes module. + # + # Examples: + # + # url_for :controller => 'tasks', :action => 'testing', :host=>'somehost.org', :port=>'8080' # => 'http://somehost.org:8080/tasks/testing' + # url_for :controller => 'tasks', :action => 'testing', :host=>'somehost.org', :anchor => 'ok', :only_path => true # => '/tasks/testing#ok' + # url_for :controller => 'tasks', :action => 'testing', :trailing_slash=>true # => 'http://somehost.org/tasks/testing/' + # url_for :controller => 'tasks', :action => 'testing', :host=>'somehost.org', :number => '33' # => 'http://somehost.org/tasks/testing?number=33' + def url_for(options = {}) + options ||= {} + case options + when String + options + when Hash + _url_rewriter.rewrite(_router, rewrite_options(options)) + else + polymorphic_url(options) + end + end + + protected + + # ROUTES TODO: Figure out why _url_rewriter is sometimes the class and + # sometimes an instance. + def _url_rewriter + ActionController::UrlRewriter + end + end + end +end \ No newline at end of file diff --git a/actionpack/lib/action_dispatch/testing/assertions/routing.rb b/actionpack/lib/action_dispatch/testing/assertions/routing.rb index ada749bfe2..f281febbc5 100644 --- a/actionpack/lib/action_dispatch/testing/assertions/routing.rb +++ b/actionpack/lib/action_dispatch/testing/assertions/routing.rb @@ -80,7 +80,7 @@ module ActionDispatch expected_path = "/#{expected_path}" unless expected_path[0] == ?/ # Load routes.rb if it hasn't been loaded. - generated_path, extra_keys = ActionDispatch::Routing::Routes.generate_extras(options, defaults) + generated_path, extra_keys = @router.generate_extras(options, defaults) found_extras = options.reject {|k, v| ! extra_keys.include? k} msg = build_message(message, "found extras , not ", found_extras, extras) @@ -142,22 +142,21 @@ module ActionDispatch # end # def with_routing - real_routes = ActionDispatch::Routing::Routes - ActionDispatch::Routing.module_eval { remove_const :Routes } - - temporary_routes = ActionDispatch::Routing::RouteSet.new - ActionDispatch::Routing.module_eval { const_set :Routes, temporary_routes } - - yield temporary_routes + old_routes, @router = @router, ActionDispatch::Routing::RouteSet.new + old_controller, @controller = @controller, @controller.clone if @controller + # ROUTES TODO: Figure out this insanity + silence_warnings { ::ActionController.const_set(:UrlFor, @router.named_url_helpers) } + _router = @router + @controller.metaclass.send(:send, :include, @router.named_url_helpers) if @controller + yield @router ensure - if ActionDispatch::Routing.const_defined? :Routes - ActionDispatch::Routing.module_eval { remove_const :Routes } - end - ActionDispatch::Routing.const_set(:Routes, real_routes) if real_routes + @router = old_routes + @controller = old_controller if @controller + silence_warnings { ::ActionController.const_set(:UrlFor, @router.named_url_helpers) } if @router end def method_missing(selector, *args, &block) - if @controller && ActionDispatch::Routing::Routes.named_routes.helpers.include?(selector) + if @controller && @router.named_routes.helpers.include?(selector) @controller.send(selector, *args, &block) else super @@ -174,7 +173,7 @@ module ActionDispatch request.env["REQUEST_METHOD"] = request_method.to_s.upcase if request_method request.path = path - params = ActionDispatch::Routing::Routes.recognize_path(path, { :method => request.method }) + params = @router.recognize_path(path, { :method => request.method }) request.path_parameters = params.with_indifferent_access request diff --git a/actionpack/lib/action_dispatch/testing/integration.rb b/actionpack/lib/action_dispatch/testing/integration.rb index 14c7ff642b..3b9d8b0318 100644 --- a/actionpack/lib/action_dispatch/testing/integration.rb +++ b/actionpack/lib/action_dispatch/testing/integration.rb @@ -188,11 +188,11 @@ module ActionDispatch unless defined? @named_routes_configured # install the named routes in this session instance. klass = singleton_class - ActionDispatch::Routing::Routes.install_helpers(klass) + # ActionDispatch::Routing::Routes.install_helpers(klass) # the helpers are made protected by default--we make them public for # easier access during testing and troubleshooting. - klass.module_eval { public *ActionDispatch::Routing::Routes.named_routes.helpers } + # klass.module_eval { public *ActionDispatch::Routing::Routes.named_routes.helpers } @named_routes_configured = true end end @@ -224,9 +224,13 @@ module ActionDispatch # Returns the URL for the given options, according to the rules specified # in the application's routes. def url_for(options) + # ROUTES TODO: @app.router is not guaranteed to exist, so a generic Rack + # application will not work here. This means that a generic Rack application + # integration test cannot call url_for, since the application will not have + # #router on it. controller ? controller.url_for(options) : - generic_url_rewriter.rewrite(options) + generic_url_rewriter.rewrite(SharedTestRoutes, options) end private diff --git a/actionpack/lib/action_view/test_case.rb b/actionpack/lib/action_view/test_case.rb index af2ed33f3f..f6f4e18a4a 100644 --- a/actionpack/lib/action_view/test_case.rb +++ b/actionpack/lib/action_view/test_case.rb @@ -53,6 +53,7 @@ module ActionView setup :setup_with_controller def setup_with_controller @controller = TestController.new + @router = SharedTestRoutes @output_buffer = ActiveSupport::SafeBuffer.new @rendered = '' @@ -152,7 +153,7 @@ module ActionView end def method_missing(selector, *args) - if ActionDispatch::Routing::Routes.named_routes.helpers.include?(selector) + if @router.named_routes.helpers.include?(selector) @controller.__send__(selector, *args) else super diff --git a/actionpack/test/abstract_unit.rb b/actionpack/test/abstract_unit.rb index 8be212c8ab..846ac5f0d7 100644 --- a/actionpack/test/abstract_unit.rb +++ b/actionpack/test/abstract_unit.rb @@ -64,29 +64,50 @@ module SetupOnce end end -class ActiveSupport::TestCase - include SetupOnce +SharedTestRoutes = ActionDispatch::Routing::RouteSet.new - # Hold off drawing routes until all the possible controller classes - # have been loaded. - setup_once do - ActionDispatch::Routing::Routes.draw do |map| - match ':controller(/:action(/:id))' +module ActiveSupport + class TestCase + include SetupOnce + # Hold off drawing routes until all the possible controller classes + # have been loaded. + setup_once do + SharedTestRoutes.draw do |map| + match ':controller(/:action(/:id))' + end + + # ROUTES TODO: Don't do this here + # brodel :'( + ActionController::IntegrationTest.app.router.draw do + match ':controller(/:action(/:id))' + end end end end +class RoutedRackApp + attr_reader :router + + def initialize(router, &blk) + @router = router + @stack = ActionDispatch::MiddlewareStack.new(&blk).build(@router) + end + + def call(env) + @stack.call(env) + end +end + class ActionController::IntegrationTest < ActiveSupport::TestCase def self.build_app(routes = nil) - ActionDispatch::Flash - ActionDispatch::MiddlewareStack.new { |middleware| + RoutedRackApp.new(routes || ActionDispatch::Routing::RouteSet.new) do |middleware| middleware.use "ActionDispatch::ShowExceptions" middleware.use "ActionDispatch::Callbacks" middleware.use "ActionDispatch::ParamsParser" middleware.use "ActionDispatch::Cookies" middleware.use "ActionDispatch::Flash" middleware.use "ActionDispatch::Head" - }.build(routes || ActionDispatch::Routing::Routes) + end end self.app = build_app @@ -112,20 +133,15 @@ class ActionController::IntegrationTest < ActiveSupport::TestCase end def with_routing(&block) - real_routes = ActionDispatch::Routing::Routes - ActionDispatch::Routing.module_eval { remove_const :Routes } - temporary_routes = ActionDispatch::Routing::RouteSet.new - self.class.app = self.class.build_app(temporary_routes) - ActionDispatch::Routing.module_eval { const_set :Routes, temporary_routes } + old_app, self.class.app = self.class.app, self.class.build_app(temporary_routes) + old_routes = SharedTestRoutes + silence_warnings { Object.const_set(:SharedTestRoutes, temporary_routes) } yield temporary_routes ensure - if ActionDispatch::Routing.const_defined? :Routes - ActionDispatch::Routing.module_eval { remove_const :Routes } - end - ActionDispatch::Routing.const_set(:Routes, real_routes) if real_routes - self.class.app = self.class.build_app + self.class.app = old_app + silence_warnings { Object.const_set(:SharedTestRoutes, old_routes) } end end @@ -190,6 +206,11 @@ module ActionController class TestCase include ActionDispatch::TestProcess + setup do + # ROUTES TODO: The router object should come from somewhere sane + @router = SharedTestRoutes + end + def assert_template(options = {}, message = nil) validate_request! @@ -232,3 +253,11 @@ module ActionController end end end + +# ROUTES TODO: Cleaner way to do this? +module ActionController + UrlFor = SharedTestRoutes.named_url_helpers + class Base + include UrlFor + end +end \ No newline at end of file diff --git a/actionpack/test/controller/action_pack_assertions_test.rb b/actionpack/test/controller/action_pack_assertions_test.rb index d54be9bdc0..7c83a91f4d 100644 --- a/actionpack/test/controller/action_pack_assertions_test.rb +++ b/actionpack/test/controller/action_pack_assertions_test.rb @@ -253,12 +253,13 @@ class ActionPackAssertionsControllerTest < ActionController::TestCase end def test_assert_redirect_to_nested_named_route + @controller = Admin::InnerModuleController.new + with_routing do |set| set.draw do |map| match 'admin/inner_module', :to => 'admin/inner_module#index', :as => :admin_inner_module match ':controller/:action' end - @controller = Admin::InnerModuleController.new process :redirect_to_index # redirection is <{"action"=>"index", "controller"=>"admin/admin/inner_module"}> assert_redirected_to admin_inner_module_path @@ -266,12 +267,13 @@ class ActionPackAssertionsControllerTest < ActionController::TestCase end def test_assert_redirected_to_top_level_named_route_from_nested_controller + @controller = Admin::InnerModuleController.new + with_routing do |set| set.draw do |map| match '/action_pack_assertions/:id', :to => 'action_pack_assertions#index', :as => :top_level match ':controller/:action' end - @controller = Admin::InnerModuleController.new process :redirect_to_top_level_named_route # assert_redirected_to "http://test.host/action_pack_assertions/foo" would pass because of exact match early return assert_redirected_to "/action_pack_assertions/foo" @@ -279,13 +281,14 @@ class ActionPackAssertionsControllerTest < ActionController::TestCase end def test_assert_redirected_to_top_level_named_route_with_same_controller_name_in_both_namespaces + @controller = Admin::InnerModuleController.new + with_routing do |set| set.draw do |map| # this controller exists in the admin namespace as well which is the only difference from previous test match '/user/:id', :to => 'user#index', :as => :top_level match ':controller/:action' end - @controller = Admin::InnerModuleController.new process :redirect_to_top_level_named_route # assert_redirected_to top_level_url('foo') would pass because of exact match early return assert_redirected_to top_level_path('foo') diff --git a/actionpack/test/controller/base_test.rb b/actionpack/test/controller/base_test.rb index 4fcfbacf4e..25006dda76 100644 --- a/actionpack/test/controller/base_test.rb +++ b/actionpack/test/controller/base_test.rb @@ -219,12 +219,14 @@ class EmptyUrlOptionsTest < ActionController::TestCase end def test_named_routes_with_path_without_doing_a_request_first + @controller = EmptyController.new + with_routing do |set| set.draw do |map| resources :things end - assert_equal '/things', EmptyController.new.send(:things_path) + assert_equal '/things', @controller.send(:things_path) end end end diff --git a/actionpack/test/controller/integration_test.rb b/actionpack/test/controller/integration_test.rb index 683ab5236c..29531d237a 100644 --- a/actionpack/test/controller/integration_test.rb +++ b/actionpack/test/controller/integration_test.rb @@ -86,7 +86,7 @@ class SessionTest < Test::Unit::TestCase def test_url_for_without_controller options = {:action => 'show'} mock_rewriter = mock() - mock_rewriter.expects(:rewrite).with(options).returns('/show') + mock_rewriter.expects(:rewrite).with(SharedTestRoutes, options).returns('/show') @session.stubs(:generic_url_rewriter).returns(mock_rewriter) @session.stubs(:controller).returns(nil) assert_equal '/show', @session.url_for(options) @@ -401,9 +401,14 @@ class IntegrationProcessTest < ActionController::IntegrationTest private def with_test_route_set with_routing do |set| + controller = ::IntegrationProcessTest::IntegrationController.clone + controller.class_eval do + include set.named_url_helpers + end + set.draw do |map| - match ':action', :to => ::IntegrationProcessTest::IntegrationController - get 'get/:action', :to => ::IntegrationProcessTest::IntegrationController + match ':action', :to => controller + get 'get/:action', :to => controller end yield end diff --git a/actionpack/test/controller/rescue_test.rb b/actionpack/test/controller/rescue_test.rb index 118b563a72..dd991898a8 100644 --- a/actionpack/test/controller/rescue_test.rb +++ b/actionpack/test/controller/rescue_test.rb @@ -326,7 +326,7 @@ class RescueTest < ActionController::IntegrationTest end test 'rescue routing exceptions' do - @app = ActionDispatch::Rescue.new(ActionDispatch::Routing::Routes) do + @app = ActionDispatch::Rescue.new(SharedTestRoutes) do rescue_from ActionController::RoutingError, lambda { |env| [200, {"Content-Type" => "text/html"}, ["Gotcha!"]] } end @@ -335,7 +335,7 @@ class RescueTest < ActionController::IntegrationTest end test 'unrescued exception' do - @app = ActionDispatch::Rescue.new(ActionDispatch::Routing::Routes) + @app = ActionDispatch::Rescue.new(SharedTestRoutes) assert_raise(ActionController::RoutingError) { get '/b00m' } end diff --git a/actionpack/test/controller/resources_test.rb b/actionpack/test/controller/resources_test.rb index cce98ac482..6f3c16f588 100644 --- a/actionpack/test/controller/resources_test.rb +++ b/actionpack/test/controller/resources_test.rb @@ -125,7 +125,7 @@ class ResourcesTest < ActionController::TestCase def test_with_custom_conditions with_restful_routing :messages, :conditions => { :subdomain => 'app' } do - assert ActionDispatch::Routing::Routes.recognize_path("/messages", :method => :get, :subdomain => 'app') + assert @router.recognize_path("/messages", :method => :get, :subdomain => 'app') end end @@ -394,7 +394,7 @@ class ResourcesTest < ActionController::TestCase assert_restful_routes_for :messages do |options| assert_recognizes(options.merge(:action => "new"), :path => "/messages/new", :method => :get) assert_raise(ActionController::RoutingError) do - ActionDispatch::Routing::Routes.recognize_path("/messages/new", :method => :post) + @router.recognize_path("/messages/new", :method => :post) end end end @@ -504,7 +504,7 @@ class ResourcesTest < ActionController::TestCase def test_restful_routes_dont_generate_duplicates with_restful_routing :messages do - routes = ActionDispatch::Routing::Routes.routes + routes = @router.routes routes.each do |route| routes.each do |r| next if route === r # skip the comparison instance @@ -1162,8 +1162,8 @@ class ResourcesTest < ActionController::TestCase options[:shallow_options] = options[:options] end - new_action = ActionDispatch::Routing::Routes.resources_path_names[:new] || "new" - edit_action = ActionDispatch::Routing::Routes.resources_path_names[:edit] || "edit" + new_action = @router.resources_path_names[:new] || "new" + edit_action = @router.resources_path_names[:edit] || "edit" if options[:path_names] new_action = options[:path_names][:new] if options[:path_names][:new] @@ -1230,6 +1230,8 @@ class ResourcesTest < ActionController::TestCase end @controller = "#{options[:options][:controller].camelize}Controller".constantize.new + # ROUTES TODO: Figure out a way to not extend the routing helpers here + @controller.metaclass.send(:include, @router.named_url_helpers) @request = ActionController::TestRequest.new @response = ActionController::TestResponse.new get :index, options[:options] @@ -1299,6 +1301,7 @@ class ResourcesTest < ActionController::TestCase def assert_singleton_named_routes_for(singleton_name, options = {}) (options[:options] ||= {})[:controller] ||= singleton_name.to_s.pluralize @controller = "#{options[:options][:controller].camelize}Controller".constantize.new + @controller.metaclass.send(:include, @router.named_url_helpers) @request = ActionController::TestRequest.new @response = ActionController::TestResponse.new get :show, options[:options] diff --git a/actionpack/test/controller/test_test.rb b/actionpack/test/controller/test_test.rb index 3f5d60540f..d716dc2661 100644 --- a/actionpack/test/controller/test_test.rb +++ b/actionpack/test/controller/test_test.rb @@ -476,8 +476,8 @@ XML end def test_with_routing_places_routes_back - assert ActionDispatch::Routing::Routes - routes_id = ActionDispatch::Routing::Routes.object_id + assert @router + routes_id = @router.object_id begin with_routing { raise 'fail' } @@ -485,8 +485,8 @@ XML rescue RuntimeError end - assert ActionDispatch::Routing::Routes - assert_equal routes_id, ActionDispatch::Routing::Routes.object_id + assert @router + assert_equal routes_id, @router.object_id end def test_remote_addr diff --git a/actionpack/test/controller/url_for_test.rb b/actionpack/test/controller/url_for_test.rb index 749fa5861f..43e2f91693 100644 --- a/actionpack/test/controller/url_for_test.rb +++ b/actionpack/test/controller/url_for_test.rb @@ -5,7 +5,7 @@ module AbstractController class UrlForTests < ActionController::TestCase class W - include ActionController::UrlFor + include SharedTestRoutes.url_helpers end def teardown diff --git a/actionpack/test/controller/url_rewriter_test.rb b/actionpack/test/controller/url_rewriter_test.rb index c2b8cd85d8..199b824a7a 100644 --- a/actionpack/test/controller/url_rewriter_test.rb +++ b/actionpack/test/controller/url_rewriter_test.rb @@ -12,52 +12,52 @@ class UrlRewriterTests < ActionController::TestCase def test_port assert_equal('http://test.host:1271/c/a/i', - @rewriter.rewrite(:controller => 'c', :action => 'a', :id => 'i', :port => 1271) + @rewriter.rewrite(@router, :controller => 'c', :action => 'a', :id => 'i', :port => 1271) ) end def test_protocol_with_and_without_separator assert_equal('https://test.host/c/a/i', - @rewriter.rewrite(:protocol => 'https', :controller => 'c', :action => 'a', :id => 'i') + @rewriter.rewrite(@router, :protocol => 'https', :controller => 'c', :action => 'a', :id => 'i') ) assert_equal('https://test.host/c/a/i', - @rewriter.rewrite(:protocol => 'https://', :controller => 'c', :action => 'a', :id => 'i') + @rewriter.rewrite(@router, :protocol => 'https://', :controller => 'c', :action => 'a', :id => 'i') ) end def test_user_name_and_password assert_equal( 'http://david:secret@test.host/c/a/i', - @rewriter.rewrite(:user => "david", :password => "secret", :controller => 'c', :action => 'a', :id => 'i') + @rewriter.rewrite(@router, :user => "david", :password => "secret", :controller => 'c', :action => 'a', :id => 'i') ) end def test_user_name_and_password_with_escape_codes assert_equal( 'http://openid.aol.com%2Fnextangler:one+two%3F@test.host/c/a/i', - @rewriter.rewrite(:user => "openid.aol.com/nextangler", :password => "one two?", :controller => 'c', :action => 'a', :id => 'i') + @rewriter.rewrite(@router, :user => "openid.aol.com/nextangler", :password => "one two?", :controller => 'c', :action => 'a', :id => 'i') ) end def test_anchor assert_equal( 'http://test.host/c/a/i#anchor', - @rewriter.rewrite(:controller => 'c', :action => 'a', :id => 'i', :anchor => 'anchor') + @rewriter.rewrite(@router, :controller => 'c', :action => 'a', :id => 'i', :anchor => 'anchor') ) end def test_anchor_should_call_to_param assert_equal( 'http://test.host/c/a/i#anchor', - @rewriter.rewrite(:controller => 'c', :action => 'a', :id => 'i', :anchor => Struct.new(:to_param).new('anchor')) + @rewriter.rewrite(@router, :controller => 'c', :action => 'a', :id => 'i', :anchor => Struct.new(:to_param).new('anchor')) ) end def test_anchor_should_be_cgi_escaped assert_equal( 'http://test.host/c/a/i#anc%2Fhor', - @rewriter.rewrite(:controller => 'c', :action => 'a', :id => 'i', :anchor => Struct.new(:to_param).new('anc/hor')) + @rewriter.rewrite(@router, :controller => 'c', :action => 'a', :id => 'i', :anchor => Struct.new(:to_param).new('anc/hor')) ) end @@ -66,8 +66,8 @@ class UrlRewriterTests < ActionController::TestCase @params[:action] = 'bye' @params[:id] = '2' - assert_equal '/hi/hi/2', @rewriter.rewrite(:only_path => true, :overwrite_params => {:action => 'hi'}) - u = @rewriter.rewrite(:only_path => false, :overwrite_params => {:action => 'hi'}) + assert_equal '/hi/hi/2', @rewriter.rewrite(@router, :only_path => true, :overwrite_params => {:action => 'hi'}) + u = @rewriter.rewrite(@router, :only_path => false, :overwrite_params => {:action => 'hi'}) assert_match %r(/hi/hi/2$), u end @@ -76,8 +76,8 @@ class UrlRewriterTests < ActionController::TestCase @params[:action] = 'list' @params[:list_page] = 1 - assert_equal '/search/list?list_page=2', @rewriter.rewrite(:only_path => true, :overwrite_params => {"list_page" => 2}) - u = @rewriter.rewrite(:only_path => false, :overwrite_params => {:list_page => 2}) + assert_equal '/search/list?list_page=2', @rewriter.rewrite(@router, :only_path => true, :overwrite_params => {"list_page" => 2}) + u = @rewriter.rewrite(@router, :only_path => false, :overwrite_params => {:list_page => 2}) assert_equal 'http://test.host/search/list?list_page=2', u end @@ -91,12 +91,12 @@ class UrlRewriterTests < ActionController::TestCase def test_trailing_slash options = {:controller => 'foo', :action => 'bar', :id => '3', :only_path => true} - assert_equal '/foo/bar/3', @rewriter.rewrite(options) - assert_equal '/foo/bar/3?query=string', @rewriter.rewrite(options.merge({:query => 'string'})) + assert_equal '/foo/bar/3', @rewriter.rewrite(@router, options) + assert_equal '/foo/bar/3?query=string', @rewriter.rewrite(@router, options.merge({:query => 'string'})) options.update({:trailing_slash => true}) - assert_equal '/foo/bar/3/', @rewriter.rewrite(options) + assert_equal '/foo/bar/3/', @rewriter.rewrite(@router, options) options.update({:query => 'string'}) - assert_equal '/foo/bar/3/?query=string', @rewriter.rewrite(options) + assert_equal '/foo/bar/3/?query=string', @rewriter.rewrite(@router, options) end end diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index e29127f78f..7cfc6ef3d7 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -164,6 +164,8 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest Routes end + include Routes.named_url_helpers + def test_logout with_test_routes do delete '/logout' @@ -728,14 +730,6 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest private def with_test_routes - real_routes, temp_routes = ActionDispatch::Routing::Routes, Routes - - ActionDispatch::Routing.module_eval { remove_const :Routes } - ActionDispatch::Routing.module_eval { const_set :Routes, temp_routes } - yield - ensure - ActionDispatch::Routing.module_eval { remove_const :Routes } - ActionDispatch::Routing.const_set(:Routes, real_routes) end end diff --git a/actionpack/test/template/test_case_test.rb b/actionpack/test/template/test_case_test.rb index be2c6b3108..12807baaa7 100644 --- a/actionpack/test/template/test_case_test.rb +++ b/actionpack/test/template/test_case_test.rb @@ -107,7 +107,7 @@ module ActionView end test "is able to use routes" do - controller.request.assign_parameters('foo', 'index') + controller.request.assign_parameters(@router, 'foo', 'index') assert_equal '/foo', url_for assert_equal '/bar', url_for(:controller => 'bar') end