From 36fd9efb5e4bfc9ac3acd4189d4dc457dea8102a Mon Sep 17 00:00:00 2001 From: Carlhuda Date: Thu, 25 Feb 2010 15:05:10 -0800 Subject: Continued effort to deglobalize the router --- actionpack/lib/action_controller.rb | 2 +- actionpack/lib/action_controller/base.rb | 9 +----- actionpack/lib/action_controller/metal/url_for.rb | 21 +++++++++++++ actionpack/lib/action_controller/railtie.rb | 1 + actionpack/lib/action_controller/test_case.rb | 2 +- actionpack/lib/action_controller/url_rewriter.rb | 21 +++++++++++++ .../lib/action_dispatch/routing/route_set.rb | 23 ++++----------- actionpack/lib/action_dispatch/routing/url_for.rb | 31 +++++--------------- actionpack/test/controller/base_test.rb | 1 + actionpack/test/controller/routing_test.rb | 34 +++++----------------- 10 files changed, 66 insertions(+), 79 deletions(-) create mode 100644 actionpack/lib/action_controller/metal/url_for.rb diff --git a/actionpack/lib/action_controller.rb b/actionpack/lib/action_controller.rb index 93d8fb612f..759e52b135 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 - # ROUTES TODO: Proxy UrlFor to Rails.application.routes.url_helpers + autoload :UrlFor autoload :Verification end diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb index 0777d9dc16..f8ddc8da09 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 @@ -82,12 +82,5 @@ module ActionController filter end - protected - - # Overwrite url rewriter to use request. - def _url_rewriter - return ActionController::UrlRewriter unless request - @_url_rewriter ||= ActionController::UrlRewriter.new(request, params) - end end end diff --git a/actionpack/lib/action_controller/metal/url_for.rb b/actionpack/lib/action_controller/metal/url_for.rb new file mode 100644 index 0000000000..013834ddee --- /dev/null +++ b/actionpack/lib/action_controller/metal/url_for.rb @@ -0,0 +1,21 @@ +module ActionController + module UrlFor + extend ActiveSupport::Concern + + include ActionDispatch::Routing::UrlFor + include ActionController::RackDelegation + + def merge_options(options) + super.reverse_merge( + :host => request.host_with_port, + :protocol => request.protocol, + :_path_segments => request.symbolized_path_parameters + ) + end + + def _router + raise "In order to use #url_for, you must include the helpers of a particular " \ + "router. For instance, `include Rails.application.router.named_url_helpers" + end + end +end \ No newline at end of file diff --git a/actionpack/lib/action_controller/railtie.rb b/actionpack/lib/action_controller/railtie.rb index 015a8212c4..5a16d82e49 100644 --- a/actionpack/lib/action_controller/railtie.rb +++ b/actionpack/lib/action_controller/railtie.rb @@ -1,6 +1,7 @@ require "action_controller" require "rails" require "action_view/railtie" +require "active_support/core_ext/class/subclasses" module ActionController class Railtie < Rails::Railtie diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index 5f8bc6f325..0fa2a8b90c 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -336,7 +336,7 @@ module ActionController private def build_request_uri(action, parameters) unless @request.env['REQUEST_URI'] - options = @controller.__send__(:rewrite_options, parameters) + options = @controller.__send__(:merge_options, parameters) options.update(:only_path => true, :action => action) url = ActionController::UrlRewriter.new(@request, parameters) diff --git a/actionpack/lib/action_controller/url_rewriter.rb b/actionpack/lib/action_controller/url_rewriter.rb index 272465d4f5..807b21cd0e 100644 --- a/actionpack/lib/action_controller/url_rewriter.rb +++ b/actionpack/lib/action_controller/url_rewriter.rb @@ -26,8 +26,14 @@ module ActionController # ROUTES TODO: Class method code smell def self.rewrite(router, options, path_segments=nil) + handle_positional_args(options) + rewritten_url = "" + # ROUTES TODO: Fix the tests + segments = options.delete(:_path_segments) + path_segments = path_segments ? path_segments.merge(segments || {}) : segments + unless options[:only_path] rewritten_url << (options[:protocol] || "http") rewritten_url << "://" unless rewritten_url.match("://") @@ -52,6 +58,21 @@ module ActionController protected + def self.handle_positional_args(options) + return unless args = options.delete(:_positional_args) + + keys = options.delete(:_positional_keys) + keys -= options.keys if args.size < keys.size - 1 # take format into account + + args = args.zip(keys).inject({}) do |h, (v, k)| + h[k] = v + h + end + + # Tell url_for to skip default_url_options + options.merge!(args) + end + def self.rewrite_authentication(options) if options[:user] && options[:password] "#{Rack::Utils.escape(options.delete(:user))}:#{Rack::Utils.escape(options.delete(:password))}@" diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index 8b761d393f..7bfe4fa2bf 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -194,24 +194,11 @@ module ActionDispatch # end @module.module_eval <<-END_EVAL, __FILE__, __LINE__ + 1 def #{selector}(*args) - if args.empty? || Hash === args.first - options = #{hash_access_method}(args.first || {}) - else - options = #{hash_access_method}(args.extract_options!) - default = default_url_options(options) if self.respond_to?(:default_url_options, true) - options = (default ||= {}).merge(options) - - keys = #{route.segment_keys.inspect} - keys -= options.keys if args.size < keys.size - 1 # take format into account - - args = args.zip(keys).inject({}) do |h, (v, k)| - h[k] = v - h - end - - # Tell url_for to skip default_url_options - options[:use_defaults] = false - options.merge!(args) + options = #{hash_access_method}(args.extract_options!) + + if args.any? + options[:_positional_args] = args + options[:_positional_keys] = #{route.segment_keys.inspect} end url_for(options) diff --git a/actionpack/lib/action_dispatch/routing/url_for.rb b/actionpack/lib/action_dispatch/routing/url_for.rb index 64c695fe13..1d6da817ed 100644 --- a/actionpack/lib/action_dispatch/routing/url_for.rb +++ b/actionpack/lib/action_dispatch/routing/url_for.rb @@ -85,8 +85,6 @@ module ActionDispatch 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 @@ -97,23 +95,16 @@ module ActionDispatch 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)) + def url_options + self.class.default_url_options.merge + end + + def merge_options(options) #:nodoc: + if options.delete(:use_defaults) != false && respond_to?(:default_url_options) && (defaults = default_url_options(options)) defaults.merge(options) else options @@ -149,19 +140,11 @@ module ActionDispatch when String options when Hash - _url_rewriter.rewrite(_router, rewrite_options(options)) + ActionController::UrlRewriter.rewrite(_router, merge_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/test/controller/base_test.rb b/actionpack/test/controller/base_test.rb index 25006dda76..436ee212c8 100644 --- a/actionpack/test/controller/base_test.rb +++ b/actionpack/test/controller/base_test.rb @@ -220,6 +220,7 @@ class EmptyUrlOptionsTest < ActionController::TestCase def test_named_routes_with_path_without_doing_a_request_first @controller = EmptyController.new + @controller.request = @request with_routing do |set| set.draw do |map| diff --git a/actionpack/test/controller/routing_test.rb b/actionpack/test/controller/routing_test.rb index f390bbdc89..f2fccfbcfc 100644 --- a/actionpack/test/controller/routing_test.rb +++ b/actionpack/test/controller/routing_test.rb @@ -52,29 +52,11 @@ class UriReservedCharactersRoutingTest < Test::Unit::TestCase end class MockController - attr_accessor :routes - - def initialize(routes) - self.routes = routes - end - def url_for(options) - only_path = options.delete(:only_path) - - port = options.delete(:port) || 80 - port_string = port == 80 ? '' : ":#{port}" - - protocol = options.delete(:protocol) || "http" - host = options.delete(:host) || "test.host" - anchor = "##{options.delete(:anchor)}" if options.key?(:anchor) + options[:protocol] ||= "http" + options[:host] ||= "test.host" - path = routes.generate(options) - - only_path ? "#{path}#{anchor}" : "#{protocol}://#{host}#{port_string}#{path}#{anchor}" - end - - def request - @request ||= ActionController::TestRequest.new + super(options) end end @@ -268,9 +250,9 @@ class LegacyRouteSetTests < Test::Unit::TestCase end def setup_for_named_route - klass = Class.new(MockController) - rs.install_helpers(klass) - klass.new(rs) + inst = MockController.clone.new + inst.class.send(:include, rs.named_url_helpers) + inst end def test_named_route_without_hash @@ -759,9 +741,7 @@ class RouteSetTest < ActiveSupport::TestCase map.users '/admin/users', :controller => 'admin/users', :action => 'index' end - klass = Class.new(MockController) - set.install_helpers(klass) - klass.new(set) + MockController.clone.new.tap { |inst| inst.class.send(:include, set.named_url_helpers)} end def test_named_route_hash_access_method -- cgit v1.2.3