Merge remote branch 'joshk/redirect_routing'

Conflicts:
	actionpack/CHANGELOG
	actionpack/lib/action_controller/metal/mime_responds.rb
Signed-off-by: NJosé Valim <jose.valim@gmail.com>

actionpack/CHANGELOG

@@ -2,6 +2,8 @@
 
 * url_for and named url helpers now accept :subdomain and :domain as options [Josh Kalderimis]
 
+* The redirect route method now also accepts a hash of options which will only change the parts of the url in question, or an object which responds to call, allowing for redirects to be reused (check the documentation for examples). [Josh Kalderimis]
+
 * Added config.action_controller.include_all_helpers. By default 'helper :all' is done in ActionController::Base, which includes all the helpers by default. Setting include_all_helpers to false will result in including only application_helper and helper corresponding to controller (like foo_helper for foo_controller). [Piotr Sarnacki]
 
 * Added a convenience idiom to generate HTML5 data-* attributes in tag helpers from a :data hash of options:

actionpack/lib/action_dispatch/http/url.rb

@@ -24,6 +24,58 @@ def self.named_host?(host)
         !(host.nil? || /\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}$/.match(host))
       end
 
+      def self.url_for(options = {})
+        unless options[:host].present? || options[:only_path].present?
+          raise ArgumentError, 'Missing host to link to! Please provide the :host parameter, set default_url_options[:host], or set :only_path to true'
+        end
+
+        rewritten_url = ""
+
+        unless options[:only_path]
+          rewritten_url << (options[:protocol] || "http")
+          rewritten_url << "://" unless rewritten_url.match("://")
+          rewritten_url << rewrite_authentication(options)
+          rewritten_url << host_or_subdomain_and_domain(options)
+          rewritten_url << ":#{options.delete(:port)}" if options[:port]
+        end
+
+        path = options.delete(:path) || ''
+
+        params = options[:params] || {}
+        params.reject! {|k,v| !v }
+
+        rewritten_url << (options[:trailing_slash] ? path.sub(/\?|\z/) { "/" + $& } : path)
+        rewritten_url << "?#{params.to_query}" unless params.empty?
+        rewritten_url << "##{Rack::Mount::Utils.escape_uri(options[:anchor].to_param.to_s)}" if options[:anchor]
+        rewritten_url
+      end
+
+      class << self
+        private
+
+        def rewrite_authentication(options)
+          if options[:user] && options[:password]
+            "#{Rack::Utils.escape(options[:user])}:#{Rack::Utils.escape(options[:password])}@"
+          else
+            ""
+          end
+        end
+
+        def host_or_subdomain_and_domain(options)
+          return options[:host] unless options[:subdomain] || options[:domain]
+
+          tld_length = options[:tld_length] || @@tld_length
+
+          host = ""
+          host << (options[:subdomain] || extract_subdomain(options[:host], tld_length))
+          host << "."
+          host << (options[:domain]    || extract_domain(options[:host], tld_length))
+          host
+        end
+
+      end
+
+
 
       # Returns the complete URL used for this request.
       def url

actionpack/lib/action_dispatch/routing/mapper.rb

@@ -2,6 +2,7 @@
 require 'active_support/core_ext/hash/except'
 require 'active_support/core_ext/object/blank'
 require 'active_support/inflector'
+require 'action_dispatch/routing/redirection'
 
 module ActionDispatch
   module Routing
@@ -383,39 +384,6 @@ def delete(*args, &block)
           map_method(:delete, *args, &block)
         end
 
-        # Redirect any path to another path:
-        #
-        #   match "/stories" => redirect("/posts")
-        def redirect(*args)
-          options = args.last.is_a?(Hash) ? args.pop : {}
-
-          path      = args.shift || Proc.new
-          path_proc = path.is_a?(Proc) ? path : proc { |params| (params.empty? || !path.match(/%\{\w*\}/)) ? path : (path % params) }
-          status    = options[:status] || 301
-
-          lambda do |env|
-            req = Request.new(env)
-
-            params = [req.symbolized_path_parameters]
-            params << req if path_proc.arity > 1
-
-            uri = URI.parse(path_proc.call(*params))
-            uri.scheme ||= req.scheme
-            uri.host   ||= req.host
-            uri.port   ||= req.port unless req.standard_port?
-
-            body = %(<html><body>You are being <a href="#{ERB::Util.h(uri.to_s)}">redirected</a>.</body></html>)
-
-            headers = {
-              'Location' => uri.to_s,
-              'Content-Type' => 'text/html',
-              'Content-Length' => body.length.to_s
-            }
-
-            [ status, headers, [body] ]
-          end
-        end
-
         private
           def map_method(method, *args, &block)
             options = args.extract_options!
@@ -636,7 +604,7 @@ def namespace(path, options = {})
                       :shallow_path => path, :shallow_prefix => path }.merge!(options)
           scope(options) { yield }
         end
-        
+
         # === Parameter Restriction
         # Allows you to constrain the nested routes based on a set of rules.
         # For instance, in order to change the routes to allow for a dot character in the +id+ parameter:
@@ -647,7 +615,7 @@ def namespace(path, options = {})
         #
         # Now routes such as +/posts/1+ will no longer be valid, but +/posts/1.1+ will be.
         # The +id+ parameter must match the constraint passed in for this example.
-        # 
+        #
         # You may use this to also resrict other parameters:
         #
         #   resources :posts do
@@ -1369,6 +1337,7 @@ def match(*args)
 
       include Base
       include HttpHelpers
+      include Redirection
       include Scoping
       include Resources
       include Shorthand

actionpack/lib/action_dispatch/routing/redirection.rb

+require 'action_dispatch/http/request'
+
+module ActionDispatch
+  module Routing
+    module Redirection
+
+      # Redirect any path to another path:
+      #
+      #   match "/stories" => redirect("/posts")
+      #
+      # You can also use interpolation in the supplied redirect argument:
+      #
+      #   match 'docs/:article', :to => redirect('/wiki/%{article}')
+      #
+      # Alternatively you can use one of the other syntaxes:
+      #
+      # The block version of redirect allows for the easy encapsulation of any logic associated with
+      # the redirect in question. Either the params and request are supplied as arguments, or just
+      # params, depending of how many arguments your block accepts. A string is required as a
+      # return value.
+      #
+      #   match 'jokes/:number', :to => redirect do |params, request|
+      #     path = (params[:number].to_i.even? ? "/wheres-the-beef" : "/i-love-lamp")
+      #     "http://#{request.host_with_port}/#{path}"
+      #   end
+      #
+      # The options version of redirect allows you to supply only the parts of the url which need
+      # to change, it also supports interpolation of the path similar to the first example.
+      #
+      #   match 'stores/:name',       :to => redirect(:subdomain => 'stores', :path => '/%{name}')
+      #   match 'stores/:name(*all)', :to => redirect(:subdomain => 'stores', :path => '/%{name}%{all}')
+      #
+      # Finally, an object which responds to call can be supplied to redirect, allowing you to reuse
+      # common redirect routes. The call method must accept two arguments, params and request, and return
+      # a string.
+      #
+      #   match 'accounts/:name' => redirect(SubdomainRedirector.new('api'))
+      #
+      def redirect(*args, &block)
+        options = args.last.is_a?(Hash) ? args.pop : {}
+        status  = options.delete(:status) || 301
+
+        path = args.shift
+
+        path_proc = if path.is_a?(String)
+          proc { |params| (params.empty? || !path.match(/%\{\w*\}/)) ? path : (path % params) }
+        elsif options.any?
+          options_proc(options)
+        elsif path.respond_to?(:call)
+          proc { |params, request| path.call(params, request) }
+        elsif block
+          block
+        else
+          raise ArgumentError, "redirection argument not supported"
+        end
+
+        redirection_proc(status, path_proc)
+      end
+
+      private
+
+        def options_proc(options)
+          proc do |params, request|
+            path = if options[:path].nil?
+              request.path
+            elsif params.empty? || !options[:path].match(/%\{\w*\}/)
+              options.delete(:path)
+            else
+              (options.delete(:path) % params)
+            end
+
+            default_options = {
+              :protocol => request.protocol,
+              :host => request.host,
+              :port => request.optional_port,
+              :path => path,
+              :params => request.query_parameters
+            }
+
+            ActionDispatch::Http::URL.url_for(options.reverse_merge(default_options))
+          end
+        end
+
+        def redirection_proc(status, path_proc)
+          lambda do |env|
+            req = Request.new(env)
+
+            params = [req.symbolized_path_parameters]
+            params << req if path_proc.arity > 1
+
+            uri = URI.parse(path_proc.call(*params))
+            uri.scheme ||= req.scheme
+            uri.host   ||= req.host
+            uri.port   ||= req.port unless req.standard_port?
+
+            body = %(<html><body>You are being <a href="#{ERB::Util.h(uri.to_s)}">redirected</a>.</body></html>)
+
+            headers = {
+              'Location' => uri.to_s,
+              'Content-Type' => 'text/html',
+              'Content-Length' => body.length.to_s
+            }
+
+            [ status, headers, [body] ]
+          end
+        end
+
+    end
+  end
+end
\ No newline at end of file

actionpack/lib/action_dispatch/routing/route_set.rb

@@ -442,12 +442,9 @@ def generate
 
           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 @@ def generate(options, recall = {}, extras = false)
       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 @@ def url_for(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)
-          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 @@ def recognize_path(path, environment = {})
 
       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 @@ def handle_positional_args(options)
           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

actionpack/test/dispatch/request_test.rb

 require 'abstract_unit'
 
 class RequestTest < ActiveSupport::TestCase
+
+  def url_for(options = {})
+    options.reverse_merge!(:host => 'www.example.com')
+    ActionDispatch::Http::URL.url_for(options)
+  end
+
+  test "url_for class method" do
+    e = assert_raise(ArgumentError) { url_for(:host => nil) }
+    assert_match(/Please provide the :host parameter/, e.message)
+
+    assert_equal '/books', url_for(:only_path => true, :path => '/books')
+
+    assert_equal 'http://www.example.com',  url_for
+    assert_equal 'http://api.example.com',  url_for(:subdomain => 'api')
+    assert_equal 'http://www.ror.com',      url_for(:domain => 'ror.com')
+    assert_equal 'http://api.ror.co.uk',    url_for(:host => 'www.ror.co.uk', :subdomain => 'api', :tld_length => 2)
+    assert_equal 'http://www.example.com:8080',   url_for(:port => 8080)
+    assert_equal 'https://www.example.com',       url_for(:protocol => 'https')
+    assert_equal 'http://www.example.com/docs',   url_for(:path => '/docs')
+    assert_equal 'http://www.example.com#signup', url_for(:anchor => 'signup')
+    assert_equal 'http://www.example.com/',       url_for(:trailing_slash => true)
+    assert_equal 'http://dhh:supersecret@www.example.com', url_for(:user => 'dhh', :password => 'supersecret')
+    assert_equal 'http://www.example.com?search=books',    url_for(:params => { :search => 'books' })
+  end
+
   test "remote ip" do
     request = stub_request 'REMOTE_ADDR' => '1.2.3.4'
     assert_equal '1.2.3.4', request.remote_ip

actionpack/test/dispatch/routing_test.rb

@@ -13,6 +13,12 @@ def self.matches?(request)
     end
   end
 
+  class YoutubeFavoritesRedirector
+    def self.call(params, request)
+      "http://www.youtube.com/watch?v=#{params[:youtube_id]}"
+    end
+  end
+
   stub_controllers do |routes|
     Routes = routes
     Routes.draw do
@@ -54,6 +60,16 @@ def self.matches?(request)
       match 'account/login', :to => redirect("/login")
       match 'secure', :to => redirect("/secure/login")
 
+      match 'mobile', :to => redirect(:subdomain => 'mobile')
+      match 'documentation', :to => redirect(:domain => 'example-documentation.com', :path => '')
+      match 'new_documentation', :to => redirect(:path => '/documentation/new')
+      match 'super_new_documentation', :to => redirect(:host => 'super-docs.com')
+
+      match 'stores/:name',        :to => redirect(:subdomain => 'stores', :path => '/%{name}')
+      match 'stores/:name(*rest)', :to => redirect(:subdomain => 'stores', :path => '/%{name}%{rest}')
+
+      match 'youtube_favorites/:youtube_id/:name', :to => redirect(YoutubeFavoritesRedirector)
+
       constraints(lambda { |req| true }) do
         match 'account/overview'
       end
@@ -667,6 +683,55 @@ def test_redirect_proc_with_request
     end
   end
 
+  def test_redirect_hash_with_subdomain
+    with_test_routes do
+      get '/mobile'
+      verify_redirect 'http://mobile.example.com/mobile'
+    end
+  end
+
+  def test_redirect_hash_with_domain_and_path
+    with_test_routes do
+      get '/documentation'
+      verify_redirect 'http://www.example-documentation.com'
+    end
+  end
+
+  def test_redirect_hash_with_path
+    with_test_routes do
+      get '/new_documentation'
+      verify_redirect 'http://www.example.com/documentation/new'
+    end
+  end
+
+  def test_redirect_hash_with_host
+    with_test_routes do
+      get '/super_new_documentation?section=top'
+      verify_redirect 'http://super-docs.com/super_new_documentation?section=top'
+    end
+  end
+
+  def test_redirect_hash_path_substitution
+    with_test_routes do
+      get '/stores/iernest'
+      verify_redirect 'http://stores.example.com/iernest'
+    end
+  end
+
+  def test_redirect_hash_path_substitution_with_catch_all
+    with_test_routes do
+      get '/stores/iernest/products'
+      verify_redirect 'http://stores.example.com/iernest/products'
+    end
+  end
+
+  def test_redirect_class
+    with_test_routes do
+      get '/youtube_favorites/oHg5SJYRHA0/rick-rolld'
+      verify_redirect 'http://www.youtube.com/watch?v=oHg5SJYRHA0'
+    end
+  end
+
   def test_openid
     with_test_routes do
       get '/openid/login'