From 9a17416d8bda0df0a6961e547c3cf1d677e66b5e Mon Sep 17 00:00:00 2001 From: Carl Lerche Date: Thu, 4 Mar 2010 21:59:42 -0800 Subject: [PATCH] Tweak out url_for uses :script_name and add some tests --- .../lib/action_controller/metal/url_for.rb | 6 +- actionpack/lib/action_controller/railtie.rb | 6 -- .../lib/action_controller/url_rewriter.rb | 4 +- .../lib/action_dispatch/routing/route_set.rb | 12 ++-- .../lib/action_dispatch/routing/url_for.rb | 9 +-- .../test/dispatch/url_generation_test.rb | 58 +++++++++++++++++++ 6 files changed, 71 insertions(+), 24 deletions(-) create mode 100644 actionpack/test/dispatch/url_generation_test.rb diff --git a/actionpack/lib/action_controller/metal/url_for.rb b/actionpack/lib/action_controller/metal/url_for.rb index 89ab0b47533c2..f6b6cb2ff4736 100644 --- a/actionpack/lib/action_controller/metal/url_for.rb +++ b/actionpack/lib/action_controller/metal/url_for.rb @@ -9,12 +9,8 @@ def url_options super.reverse_merge( :host => request.host_with_port, :protocol => request.protocol, - # ROUTES TODO: relative_url_root should be middleware - # and the generator should take SCRIPT_NAME into - # consideration - :script_name => request.env["SCRIPT_NAME"], :_path_segments => request.symbolized_path_parameters - ) + ).merge(:script_name => request.script_name) end def _router diff --git a/actionpack/lib/action_controller/railtie.rb b/actionpack/lib/action_controller/railtie.rb index e638be525178e..e9edf80451a88 100644 --- a/actionpack/lib/action_controller/railtie.rb +++ b/actionpack/lib/action_controller/railtie.rb @@ -51,12 +51,6 @@ class Railtie < Rails::Railtie ac.stylesheets_dir = paths.public.stylesheets.to_a.first ac.secret = app.config.cookie_secret - if ac.relative_url_root - ActiveSupport::Deprecation.warn "config.action_controller.relative_url_root " \ - "is no longer effective. Please set it in the router as " \ - "routes.draw(:script_name => #{ac.relative_url_root.inspect})" - end - ActionController::Base.config.replace(ac) end diff --git a/actionpack/lib/action_controller/url_rewriter.rb b/actionpack/lib/action_controller/url_rewriter.rb index 8821892b3ae8a..981865517f353 100644 --- a/actionpack/lib/action_controller/url_rewriter.rb +++ b/actionpack/lib/action_controller/url_rewriter.rb @@ -32,7 +32,6 @@ def self.rewrite(router, options, path_segments=nil) # ROUTES TODO: Fix the tests segments = options.delete(:_path_segments) - relative_url_root = options.delete(:script_name).to_s path_segments = path_segments ? path_segments.merge(segments || {}) : segments unless options[:only_path] @@ -50,8 +49,7 @@ def self.rewrite(router, options, path_segments=nil) path_options = yield(path_options) if block_given? path = router.generate(path_options, path_segments || {}) - # ROUTES TODO: This can be called directly, so relative_url_root should probably be set in the router - rewritten_url << relative_url_root + # ROUTES TODO: This can be called directly, so script_name should probably be set in the router rewritten_url << (options[:trailing_slash] ? path.sub(/\?|\z/) { "/" + $& } : path) rewritten_url << "##{Rack::Utils.escape(options[:anchor].to_param.to_s)}" if options[:anchor] diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index 26189329c2841..a53d8067dd86f 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -209,29 +209,27 @@ def #{selector}(*args) end end - attr_accessor :routes, :named_routes + attr_accessor :routes, :named_routes, :mount_point attr_accessor :disable_clear_and_finalize, :resources_path_names - attr_accessor :script_name def self.default_resources_path_names { :new => 'new', :edit => 'edit' } end - def initialize + def initialize(options = {}) self.routes = [] self.named_routes = NamedRouteCollection.new self.resources_path_names = self.class.default_resources_path_names.dup self.controller_namespaces = Set.new + self.mount_point = options[:mount_point] @disable_clear_and_finalize = false clear! end - def draw(options = {}, &block) + def draw(&block) clear! unless @disable_clear_and_finalize - @script_name = options[:script_name] - mapper = Mapper.new(self) if block.arity == 1 mapper.instance_exec(DeprecatedMapper.new(self), &block) @@ -341,6 +339,7 @@ def generate_extras(options, recall={}) def generate(options, recall = {}, method = :generate) options, recall = options.dup, recall.dup + script_name = options.delete(:script_name) || mount_point named_route = options.delete(:use_route) options = options_as_params(options) @@ -406,6 +405,7 @@ def generate(options, recall = {}, method = :generate) [path, params.keys] elsif path path << "?#{params.to_query}" if params.any? + path = "#{script_name}#{path}" path else raise ActionController::RoutingError, "No route matches #{options.inspect}" diff --git a/actionpack/lib/action_dispatch/routing/url_for.rb b/actionpack/lib/action_dispatch/routing/url_for.rb index afbb2ecf16a55..be5edcc12029c 100644 --- a/actionpack/lib/action_dispatch/routing/url_for.rb +++ b/actionpack/lib/action_dispatch/routing/url_for.rb @@ -102,14 +102,15 @@ module UrlFor def url_options @url_options ||= begin - opts = self.class.default_url_options - opts.merge(:script_name => _router.script_name) if respond_to?(:_router) - opts + # self.class does not respond to default_url_options when the helpers are extended + # onto a singleton + self.class.respond_to?(:default_url_options) ? self.class.default_url_options : {} end end def url_options=(options) - @url_options = options + defaults = self.class.respond_to?(:default_url_options) ? self.class.default_url_options : {} + @url_options = defaults.merge(options) end # def merge_options(options) #:nodoc: diff --git a/actionpack/test/dispatch/url_generation_test.rb b/actionpack/test/dispatch/url_generation_test.rb new file mode 100644 index 0000000000000..b55e001fa710d --- /dev/null +++ b/actionpack/test/dispatch/url_generation_test.rb @@ -0,0 +1,58 @@ +require 'abstract_unit' + +module TestUrlGeneration + class WithoutMountpoint < ActiveSupport::TestCase + setup do + app = lambda { |env| [200, {}, "Hello"] } + @router = ActionDispatch::Routing::RouteSet.new + @router.draw do + match "/foo", :to => app, :as => :foo + end + + singleton_class.send(:include, @router.url_helpers) + end + + test "generating URLS normally" do + assert_equal "/foo", foo_path + end + + test "accepting a :script_name option" do + assert_equal "/bar/foo", foo_path(:script_name => "/bar") + end + end + + class WithMountPoint < ActionDispatch::IntegrationTest + Router = ActionDispatch::Routing::RouteSet.new(:mount_point => "/baz") + Router.draw { match "/foo", :to => "my_route_generating#index", :as => :foo } + + class ::MyRouteGeneratingController < ActionController::Base + include Router.url_helpers + def index + render :text => foo_path + end + end + + include Router.url_helpers + + def _router + Router + end + + def app + Router + end + + test "using the default :script_name option" do + assert_equal "/baz/foo", foo_path + end + + test "overriding the default :script_name option" do + assert_equal "/bar/foo", foo_path(:script_name => "/bar") + end + + test "the request's SCRIPT_NAME takes precedence over the router's" do + get "/foo", {}, 'SCRIPT_NAME' => "/new" + assert_equal "/new/foo", response.body + end + end +end \ No newline at end of file