The redirect routing method now allows for a hash of options which only changes the relevant parts of the url, or an object which responds to call can be supplied so common redirect rules can be easily reused. This commit includes a change where url generation from parts has been moved to AD::Http::URL as a class method.

1 files changed, 18 insertions, 44 deletions

diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb
index ebced9cabe..03bfe178e5 100644
--- a/actionpack/lib/action_dispatch/routing/route_set.rb
+++ b/actionpack/lib/action_dispatch/routing/route_set.rb
@@ -442,12 +442,9 @@ module ActionDispatch
 
           raise_routing_error unless path
 
-          params.reject! {|k,v| !v }
-
           return [path, params.keys] if @extras
 
-          path << "?#{params.to_query}" unless params.empty?
-          path
+          [path, params]
         rescue Rack::Mount::RoutingError
           raise_routing_error
         end
@@ -486,7 +483,7 @@ module ActionDispatch
       end
 
       RESERVED_OPTIONS = [:host, :protocol, :port, :subdomain, :domain, :tld_length,
-                          :trailing_slash, :script_name, :anchor, :params, :only_path ]
+                          :trailing_slash, :anchor, :params, :only_path, :script_name]
 
       def _generate_prefix(options = {})
         nil
@@ -498,29 +495,24 @@ module ActionDispatch
 
         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)
-          rewritten_url << host_from_options(options)
-          rewritten_url << ":#{options.delete(:port)}" if options[:port]
-        end
+        user, password = extract_authentication(options)
+        path_segments  = options.delete(:_path_segments)
+        script_name    = options.delete(:script_name)
 
-        script_name = options.delete(:script_name)
         path = (script_name.blank? ? _generate_prefix(options) : script_name.chomp('/')).to_s
 
         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 routes
-        rewritten_url << (options[:trailing_slash] ? path.sub(/\?|\z/) { "/" + $& } : path)
-        rewritten_url << "##{Rack::Mount::Utils.escape_uri(options[:anchor].to_param.to_s)}" if options[:anchor]
+        path_addition, params = generate(path_options, path_segments || {})
+        path << path_addition
 
-        rewritten_url
+        ActionDispatch::Http::URL.url_for(options.merge({
+          :path => path,
+          :params => params,
+          :user => user,
+          :password => password
+        }))
       end
 
       def call(env)
@@ -561,23 +553,12 @@ module ActionDispatch
 
       private
 
-        def host_from_options(options)
-          computed_host = subdomain_and_domain(options) || options[:host]
-          unless computed_host
-            raise ArgumentError, "Missing host to link to! Please provide :host parameter or set default_url_options[:host]"
+        def extract_authentication(options)
+          if options[:user] && options[:password]
+            [options.delete(:user), options.delete(:password)]
+          else
+            nil
           end
-          computed_host
-        end
-
-        def subdomain_and_domain(options)
-          return nil unless options[:subdomain] || options[:domain]
-          tld_length = options[:tld_length] || ActionDispatch::Http::URL.tld_length
-
-          host = ""
-          host << (options[:subdomain] || ActionDispatch::Http::URL.extract_subdomain(options[:host], tld_length))
-          host << "."
-          host << (options[:domain] || ActionDispatch::Http::URL.extract_domain(options[:host], tld_length))
-          host
         end
 
         def handle_positional_args(options)
@@ -590,13 +571,6 @@ module ActionDispatch
           options.merge!(Hash[args.zip(keys).map { |v, k| [k, v] }])
         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