diff --git a/actionpack/lib/action_controller.rb b/actionpack/lib/action_controller.rb index 3b97f3849829f..536154fc6b71f 100644 --- a/actionpack/lib/action_controller.rb +++ b/actionpack/lib/action_controller.rb @@ -46,7 +46,6 @@ module ActionController eager_autoload do autoload :RecordIdentifier - autoload :UrlRewriter # TODO: Don't autoload exceptions, setup explicit # requires for files that need them diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index c43e560ac15cd..072d289964ea5 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -335,23 +335,22 @@ def rescue_action_in_public! @request.remote_addr = '208.77.188.166' # example.com end - private - def build_request_uri(action, parameters) - unless @request.env["PATH_INFO"] - options = @controller.__send__(:url_options).merge(parameters) - options.update( - :only_path => true, - :action => action, - :relative_url_root => nil, - :_path_segments => @request.symbolized_path_parameters) - - rewriter = ActionController::UrlRewriter - url, query_string = rewriter.rewrite(@router, options).split("?", 2) - - @request.env["SCRIPT_NAME"] = @controller.config.relative_url_root - @request.env["PATH_INFO"] = url - @request.env["QUERY_STRING"] = query_string || "" + private + def build_request_uri(action, parameters) + unless @request.env["PATH_INFO"] + options = @controller.__send__(:url_options).merge(parameters) + options.update( + :only_path => true, + :action => action, + :relative_url_root => nil, + :_path_segments => @request.symbolized_path_parameters) + + url, query_string = @router.rewrite(options).split("?", 2) + + @request.env["SCRIPT_NAME"] = @controller.config.relative_url_root + @request.env["PATH_INFO"] = url + @request.env["QUERY_STRING"] = query_string || "" + end end end - end end diff --git a/actionpack/lib/action_controller/url_rewriter.rb b/actionpack/lib/action_controller/url_rewriter.rb deleted file mode 100644 index b387d4959363f..0000000000000 --- a/actionpack/lib/action_controller/url_rewriter.rb +++ /dev/null @@ -1,64 +0,0 @@ -require 'active_support/core_ext/hash/except' - -module ActionController - # Rewrites URLs for Base.redirect_to and Base.url_for in the controller. - module UrlRewriter #:nodoc: - RESERVED_OPTIONS = [:anchor, :params, :only_path, :host, :protocol, :port, :trailing_slash, :skip_relative_url_root] - - # ROUTES TODO: Class method code smell - def self.rewrite(router, options) - handle_positional_args(options) - - rewritten_url = "" - - path_segments = options.delete(:_path_segments) - - unless options[:only_path] - rewritten_url << (options[:protocol] || "http") - rewritten_url << "://" unless rewritten_url.match("://") - rewritten_url << rewrite_authentication(options) - - raise "Missing host to link to! Please provide :host parameter or set default_url_options[:host]" unless options[:host] - - rewritten_url << options[:host] - rewritten_url << ":#{options.delete(:port)}" if options.key?(:port) - end - - path_options = options.except(*RESERVED_OPTIONS) - path_options = yield(path_options) if block_given? - path = router.generate(path_options, path_segments || {}) - - # 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] - - rewritten_url - end - - 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))}@" - else - "" - end - end - - end -end diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index e99e39269b407..15c7cd00ba94b 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -410,6 +410,38 @@ def generate(options, recall = {}, extras = false) Generator.new(options, recall, @set, extras).generate end + RESERVED_OPTIONS = [:anchor, :params, :only_path, :host, :protocol, :port, :trailing_slash, :skip_relative_url_root] + + # TODO: Terrible method name + def rewrite(options) + handle_positional_args(options) + + rewritten_url = "" + + path_segments = options.delete(:_path_segments) + + unless options[:only_path] + rewritten_url << (options[:protocol] || "http") + rewritten_url << "://" unless rewritten_url.match("://") + rewritten_url << rewrite_authentication(options) + + raise "Missing host to link to! Please provide :host parameter or set default_url_options[:host]" unless options[:host] + + rewritten_url << options[:host] + rewritten_url << ":#{options.delete(:port)}" if options.key?(:port) + end + + path_options = options.except(*RESERVED_OPTIONS) + path_options = yield(path_options) if block_given? + path = generate(path_options, path_segments || {}) + + # 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] + + rewritten_url + end + def call(env) @set.call(env) end @@ -435,6 +467,30 @@ def recognize_path(path, environment = {}) raise ActionController::RoutingError, "No route matches #{path.inspect}" end + + private + def 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 rewrite_authentication(options) + if options[:user] && options[:password] + "#{Rack::Utils.escape(options.delete(:user))}:#{Rack::Utils.escape(options.delete(:password))}@" + else + "" + end + end end end end diff --git a/actionpack/lib/action_dispatch/routing/url_for.rb b/actionpack/lib/action_dispatch/routing/url_for.rb index 4629e7e002b97..0ba04316d74f3 100644 --- a/actionpack/lib/action_dispatch/routing/url_for.rb +++ b/actionpack/lib/action_dispatch/routing/url_for.rb @@ -131,7 +131,7 @@ def url_for(options = nil) when String options when nil, Hash - ActionController::UrlRewriter.rewrite(_router, url_options.merge(options || {})) + _router.rewrite(url_options.merge(options || {})) else polymorphic_url(options) end diff --git a/actionpack/test/controller/caching_test.rb b/actionpack/test/controller/caching_test.rb index 2be91893d4725..08efd5a48c055 100644 --- a/actionpack/test/controller/caching_test.rb +++ b/actionpack/test/controller/caching_test.rb @@ -64,7 +64,6 @@ def setup @controller.cache_store = :file_store, FILE_STORE_PATH @params = {:controller => 'posts', :action => 'index', :only_path => true, :skip_relative_url_root => true} - @rewriter = ActionController::UrlRewriter FileUtils.rm_rf(File.dirname(FILE_STORE_PATH)) FileUtils.mkdir_p(FILE_STORE_PATH) @@ -82,9 +81,9 @@ def test_page_caching_resources_saves_to_correct_path_with_extension_even_if_def match '/', :to => 'posts#index', :as => :main end @params[:format] = 'rss' - assert_equal '/posts.rss', @rewriter.rewrite(@router, @params) + assert_equal '/posts.rss', @router.rewrite(@params) @params[:format] = nil - assert_equal '/', @rewriter.rewrite(@router, @params) + assert_equal '/', @router.rewrite(@params) end end diff --git a/actionpack/test/controller/url_rewriter_test.rb b/actionpack/test/controller/url_rewriter_test.rb index 3bc8bec3fb8e8..cf775846dfbdd 100644 --- a/actionpack/test/controller/url_rewriter_test.rb +++ b/actionpack/test/controller/url_rewriter_test.rb @@ -1,8 +1,6 @@ require 'abstract_unit' require 'controller/fake_controllers' -ActionController::UrlRewriter - class UrlRewriterTests < ActionController::TestCase class Rewriter def initialize(request) @@ -13,8 +11,7 @@ def initialize(request) end def rewrite(router, options) - options = @options.merge(options) - ActionController::UrlRewriter.rewrite(router, options) + router.rewrite(@options.merge(options)) end end