adds URL to the body generated by the redirect macro in the routes mapper as per the RFC, extracts common test pattern into a test macro, adds a test to cover the :status option

author: Xavier Noria <fxn@hashref.com> 2010-08-09 13:31:42 +0200
committer: Xavier Noria <fxn@hashref.com> 2010-08-09 13:31:42 +0200
commit: 4434e407e93001409605e5f02650b591a0cede32 (patch)
tree: 11ff28e8255364f1fc2b9efce386d61afadd84f0 /actionpack
parent: d87c57bf3e5718c6995ecc73cf8d7396e3ba4b19 (diff)
download: rails-4434e407e93001409605e5f02650b591a0cede32.tar.gz
rails-4434e407e93001409605e5f02650b591a0cede32.tar.bz2
rails-4434e407e93001409605e5f02650b591a0cede32.zip
2 files changed, 29 insertions, 31 deletions
diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb
index 526c97ff8e..c118c72440 100644
--- a/actionpack/lib/action_dispatch/routing/mapper.rb
+++ b/actionpack/lib/action_dispatch/routing/mapper.rb
@@ -1,3 +1,4 @@
+require 'erb'
 require 'active_support/core_ext/hash/except'
 require 'active_support/core_ext/object/blank'
 
@@ -277,7 +278,6 @@ module ActionDispatch
           path      = args.shift || block
           path_proc = path.is_a?(Proc) ? path : proc { |params| path % params }
           status    = options[:status] || 301
-          body      = 'Moved Permanently'
 
           lambda do |env|
             req = Request.new(env)
@@ -290,11 +290,14 @@ module ActionDispatch
             uri.host   ||= req.host
             uri.port   ||= req.port unless req.port == 80
 
+            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
diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb
index 4808663aa9..3f090b7254 100644
--- a/actionpack/test/dispatch/routing_test.rb
+++ b/actionpack/test/dispatch/routing_test.rb
@@ -1,3 +1,4 @@
+require 'erb'
 require 'abstract_unit'
 require 'controller/fake_controllers'
 
@@ -56,7 +57,7 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest
       match 'account/proc/:name', :to => redirect {|params| "/#{params[:name].pluralize}" }
       match 'account/proc_req' => redirect {|params, req| "/#{req.method}" }
 
-      match 'account/google' => redirect('http://www.google.com/')
+      match 'account/google' => redirect('http://www.google.com/', :status => 302)
 
       match 'openid/login', :via => [:get, :post], :to => "openid#login"
 
@@ -501,9 +502,7 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest
   def test_login_redirect
     with_test_routes do
       get '/account/login'
-      assert_equal 301, @response.status
-      assert_equal 'http://www.example.com/login', @response.headers['Location']
-      assert_equal 'Moved Permanently', @response.body
+      verify_redirect 'http://www.example.com/login'
     end
   end
 
@@ -511,18 +510,14 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest
     with_test_routes do
       assert_equal '/account/logout', logout_redirect_path
       get '/account/logout'
-      assert_equal 301, @response.status
-      assert_equal 'http://www.example.com/logout', @response.headers['Location']
-      assert_equal 'Moved Permanently', @response.body
+      verify_redirect 'http://www.example.com/logout'
     end
   end
 
   def test_namespace_redirect
     with_test_routes do
       get '/private'
-      assert_equal 301, @response.status
-      assert_equal 'http://www.example.com/private/index', @response.headers['Location']
-      assert_equal 'Moved Permanently', @response.body
+      verify_redirect 'http://www.example.com/private/index'
     end
   end
 
@@ -586,27 +581,21 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest
   def test_redirect_modulo
     with_test_routes do
       get '/account/modulo/name'
-      assert_equal 301, @response.status
-      assert_equal 'http://www.example.com/names', @response.headers['Location']
-      assert_equal 'Moved Permanently', @response.body
+      verify_redirect 'http://www.example.com/names'
     end
   end
 
   def test_redirect_proc
     with_test_routes do
       get '/account/proc/person'
-      assert_equal 301, @response.status
-      assert_equal 'http://www.example.com/people', @response.headers['Location']
-      assert_equal 'Moved Permanently', @response.body
+      verify_redirect 'http://www.example.com/people'
     end
   end
 
   def test_redirect_proc_with_request
     with_test_routes do
       get '/account/proc_req'
-      assert_equal 301, @response.status
-      assert_equal 'http://www.example.com/GET', @response.headers['Location']
-      assert_equal 'Moved Permanently', @response.body
+      verify_redirect 'http://www.example.com/GET'
     end
   end
 
@@ -1203,12 +1192,10 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest
     end
   end
 
-  def test_redirect_with_complete_url
+  def test_redirect_with_complete_url_and_status
     with_test_routes do
       get '/account/google'
-      assert_equal 301, @response.status
-      assert_equal 'http://www.google.com/', @response.headers['Location']
-      assert_equal 'Moved Permanently', @response.body
+      verify_redirect 'http://www.google.com/', 302
     end
   end
 
@@ -1216,9 +1203,7 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest
     previous_host, self.host = self.host, 'www.example.com:3000'
     with_test_routes do
       get '/account/login'
-      assert_equal 301, @response.status
-      assert_equal 'http://www.example.com:3000/login', @response.headers['Location']
-      assert_equal 'Moved Permanently', @response.body
+      verify_redirect 'http://www.example.com:3000/login'
     end
   ensure
     self.host = previous_host
@@ -1899,8 +1884,18 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest
     end
   end
 
-  private
-    def with_test_routes
-      yield
-    end
+private
+  def with_test_routes
+    yield
+  end
+
+  def verify_redirect(url, status=301)
+    assert_equal status, @response.status
+    assert_equal url, @response.headers['Location']
+    assert_equal expected_redirect_body(url), @response.body
+  end
+
+  def expected_redirect_body(url)
+    %(<html><body>You are being <a href="#{ERB::Util.h(url)}">redirected</a>.</body></html>)
+  end
 end
author	Xavier Noria <fxn@hashref.com>	2010-08-09 13:31:42 +0200
committer	Xavier Noria <fxn@hashref.com>	2010-08-09 13:31:42 +0200
commit	4434e407e93001409605e5f02650b591a0cede32 (patch)
tree	11ff28e8255364f1fc2b9efce386d61afadd84f0 /actionpack
parent	d87c57bf3e5718c6995ecc73cf8d7396e3ba4b19 (diff)
download	rails-4434e407e93001409605e5f02650b591a0cede32.tar.gz rails-4434e407e93001409605e5f02650b591a0cede32.tar.bz2 rails-4434e407e93001409605e5f02650b591a0cede32.zip