Permalink
Browse files

adds URL to the body generated by the redirect macro in the routes ma…

…pper as per the RFC, extracts common test pattern into a test macro, adds a test to cover the :status option
  • Loading branch information...
1 parent d87c57b commit 4434e407e93001409605e5f02650b591a0cede32 @fxn fxn committed Aug 9, 2010
Showing with 29 additions and 31 deletions.
  1. +4 −1 actionpack/lib/action_dispatch/routing/mapper.rb
  2. +25 −30 actionpack/test/dispatch/routing_test.rb
View
5 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 @@ def redirect(*args, &block)
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 @@ def redirect(*args, &block)
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
View
55 actionpack/test/dispatch/routing_test.rb
@@ -1,3 +1,4 @@
+require 'erb'
require 'abstract_unit'
require 'controller/fake_controllers'
@@ -56,7 +57,7 @@ def self.matches?(request)
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,28 +502,22 @@ def test_login
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
def test_logout_redirect_without_to
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 @@ def test_member_on_resource
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,22 +1192,18 @@ def test_convention_with_explicit_end
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
def test_redirect_with_port
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 @@ def test_only_scope_should_override_parent_except_scope
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

0 comments on commit 4434e40

Please sign in to comment.