From 0367317dd62ecd177d57d469a4d57974b75e425b Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Sun, 22 May 2005 07:43:05 +0000 Subject: Deprecated redirect_to_path and redirect_to_url in favor of letting redirect_to do the right thing when passed either a path or url. Introduced r as a unified method for render (still under construction) git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@1349 5ecf4fe2-1ee6-0310-87b1-e25e094e27de --- actionpack/CHANGELOG | 2 + actionpack/lib/action_controller/base.rb | 103 +++++++-- actionpack/lib/action_controller/layout.rb | 34 ++- actionpack/lib/action_controller/request.rb | 37 ++-- actionpack/lib/action_controller/response.rb | 2 +- .../test/controller/action_pack_assertions_test.rb | 4 +- actionpack/test/controller/new_render_test.rb | 237 +++++++++++++++++++++ actionpack/test/controller/request_test.rb | 22 +- .../test/template/form_options_helper_test.rb | 5 +- 9 files changed, 396 insertions(+), 50 deletions(-) create mode 100644 actionpack/test/controller/new_render_test.rb (limited to 'actionpack') diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index ec06c28ed1..5fb999faeb 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -1,5 +1,7 @@ *SVN* +* Deprecated redirect_to_path and redirect_to_url in favor of letting redirect_to do the right thing when passed either a path or url. + * Fixed use of an integer as return code for renders, so render_text "hello world", 404 now works #1327 * Fixed assert_redirect_to to work with redirect_to_path #869 [Nicholas Seckar] diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb index 5eba5c7167..c449d33c22 100755 --- a/actionpack/lib/action_controller/base.rb +++ b/actionpack/lib/action_controller/base.rb @@ -439,6 +439,52 @@ module ActionController #:nodoc: end protected + # A unified replacement for the individual renders (work-in-progress). + def r(options = {}, &block) + raise DoubleRenderError, "Can only render or redirect once per action" if performed? + add_variables_to_assigns + options[:status] = (options[:status] || DEFAULT_RENDER_STATUS_CODE).to_s + + if options[:text] + @response.headers["Status"] = options[:status] + @response.body = block_given? ? block : options[:text] + @performed_render = true + return options[:text] + + elsif options[:file] + assert_existance_of_template_file(options[:file]) if options[:use_full_path] + logger.info("Rendering #{options[:file]} (#{options[:status]})") unless logger.nil? + r(options.merge({ :text => @template.render_file(options[:file], options[:use_full_path])})) + + elsif options[:template] + r(options.merge({ :file => options[:template], :use_full_path => true })) + + elsif options[:inline] + r(options.merge({ :text => @template.render_template(options[:type] || :rhtml, options[:inline]) })) + + elsif options[:action] + r(options.merge({ :template => default_template_name(options[:action]) })) + + elsif options[:partial] && options[:collection] + r(options.merge({ + :text => ( + @template.render_partial_collection( + options[:partial], options[:collection], options[:spacer_template], options[:local_assigns] + ) || '' + ) + })) + + elsif options[:partial] + r(options.merge({ :text => @template.render_partial(options[:partial], options[:object], options[:local_assigns]) })) + + elsif options[:nothing] + r(options.merge({ :text => "" })) + + else + r(options.merge({ :template => default_template_name })) + end + end + # Renders the template specified by template_name, which defaults to the name of the current controller and action. # So calling +render+ in WeblogController#show will attempt to render "#{template_root}/weblog/show.rhtml" or # "#{template_root}/weblog/show.rxml" (in that order). The template_root is set on the ActionController::Base class and is @@ -480,7 +526,7 @@ module ActionController #:nodoc: def render_text(text = nil, status = nil, &block) #:doc: raise DoubleRenderError, "Can only render or redirect once per action" if performed? add_variables_to_assigns - @response.headers["Status"] = status.to_s || DEFAULT_RENDER_STATUS_CODE + @response.headers["Status"] = (status || DEFAULT_RENDER_STATUS_CODE).to_s @response.body = block_given? ? block : text @performed_render = true end @@ -645,32 +691,51 @@ module ActionController #:nodoc: def default_url_options(options) #:doc: end - # Redirects the browser to an URL that has been rewritten according to the hash of +options+ using a "302 Moved" HTTP header. - # See url_for for a description of the valid options. + # Redirects the browser to the target specified in +options+. This parameter can take one of three forms: + # + # * Hash: The URL will be generated by calling url_for with the +options+. + # * String starting with protocol:// (like http://): Is passed straight through as the target for redirection. + # * String not containing a protocol: The current current protocol and host is prepended to the string. + # + # Examples: + # redirect_to :action => "show", :id => 5 + # redirect_to "http://www.rubyonrails.org" + # redirect_to "/images/screenshot.jpg" + # + # The redirection happens as a "302 Moved" header. def redirect_to(options = {}, *parameters_for_method_reference) #:doc: - if parameters_for_method_reference.empty? - @response.redirected_to = options - redirect_to_url(url_for(options)) - else - @response.redirected_to, @response.redirected_to_method_params = options, parameters_for_method_reference - redirect_to_url(url_for(options, *parameters_for_method_reference)) + case options + when %r{^\w+://.*} + raise DoubleRenderError, "Can only render or redirect once per action" if performed? + logger.info("Redirected to #{url}") unless logger.nil? + @response.redirect(options) + @performed_redirect = true + + when String + redirect_to(request.protocol + request.host_with_port + options) + + else + if parameters_for_method_reference.empty? + response.redirected_to = options + redirect_to(url_for(options)) + else + response.redirected_to, response.redirected_to_method_params = options, parameters_for_method_reference + redirect_to(url_for(options, *parameters_for_method_reference)) + end end end - # Redirects the browser to the specified path within the current host (specified with a leading /). Used to sidestep - # the URL rewriting and go directly to a known path. Example: redirect_to_path "/images/screenshot.jpg". + # Deprecated in favor of calling redirect_to directly with the path. def redirect_to_path(path) #:doc: - redirect_to_url(@request.protocol + @request.host_with_port + path) + redirect_to(path) end - # Redirects the browser to the specified url. Used to redirect outside of the current application. Example: - # redirect_to_url "http://www.rubyonrails.org". If the resource has moved permanently, it's possible to pass true as the - # second parameter and the browser will get "301 Moved Permanently" instead of "302 Found". + # Deprecated in favor of calling redirect_to directly with the url. If the resource has moved permanently, it's possible to pass + # true as the second parameter and the browser will get "301 Moved Permanently" instead of "302 Found". This can also be done through + # just setting the headers["Status"] to "301 Moved Permanently" before using the redirect_to. def redirect_to_url(url, permanently = false) #:doc: - raise DoubleRenderError, "Can only render or redirect once per action" if performed? - logger.info("Redirected to #{url}") unless logger.nil? - @response.redirect(url, permanently) - @performed_redirect = true + headers["Status"] = "301 Moved Permanently" if permanently + redirect_to(url) end # Resets the session by clearing out all the objects stored within and initializing a new session object. diff --git a/actionpack/lib/action_controller/layout.rb b/actionpack/lib/action_controller/layout.rb index aca3dbd797..37dba36186 100644 --- a/actionpack/lib/action_controller/layout.rb +++ b/actionpack/lib/action_controller/layout.rb @@ -5,6 +5,10 @@ module ActionController #:nodoc: base.class_eval do alias_method :render_without_layout, :render alias_method :render, :render_with_layout + + alias_method :r_without_layout, :r + alias_method :r, :r_with_layout + class << self alias_method :inherited_without_layout, :inherited end @@ -212,8 +216,33 @@ module ActionController #:nodoc: end end + def r_with_layout(options = {}) + if (layout = active_layout_for_r(options)) && options[:text] + add_variables_to_assigns + logger.info("Rendering #{template_name} within #{layout}") unless logger.nil? + + @content_for_layout = r_without_layout(options) + add_variables_to_assigns + + erase_render_results + r_without_layout(options.merge({ :text => @template.render_file(layout, true)})) + else + r_without_layout(options) + end + end + private - + def active_layout_for_r(options = {}) + case options[:layout] + when FalseClass + nil + when NilClass + active_layout if action_has_layout? + else + active_layout(options[:layout]) + end + end + def action_has_layout? conditions = self.class.layout_conditions case @@ -225,6 +254,5 @@ module ActionController #:nodoc: true end end - end -end +end \ No newline at end of file diff --git a/actionpack/lib/action_controller/request.rb b/actionpack/lib/action_controller/request.rb index af81f0775a..2cb4377273 100755 --- a/actionpack/lib/action_controller/request.rb +++ b/actionpack/lib/action_controller/request.rb @@ -1,6 +1,8 @@ module ActionController # These methods are available in both the production and test Request objects. class AbstractRequest + cattr_accessor :relative_url_root + # Returns both GET and POST parameters in a single hash. def parameters @parameters ||= request_parameters.merge(query_parameters).merge(path_parameters).with_indifferent_access @@ -57,6 +59,16 @@ module ActionController def yaml_post? post_format == :yaml && post? end + + + # Returns true if the request's "X-Requested-With" header contains + # "XMLHttpRequest". (The Prototype Javascript library sends this header with + # every Ajax request.) + def xml_http_request? + env['HTTP_X_REQUESTED_WITH'] =~ /XMLHttpRequest/i + end + alias xhr? :xml_http_request? + # Determine originating IP address. REMOTE_ADDR is the standard @@ -120,20 +132,15 @@ module ActionController protocol == 'https://' end - # returns the interpreted path to requested resource after - # all the installation directory of this application was taken into account + # Returns the interpreted path to requested resource after all the installation directory of this application was taken into account def path - uri = request_uri - path = uri ? uri.split('?').first : '' - - # cut off the part of the url which leads to the installation directory of this app - path[relative_url_root.length..-1] + path = (uri = request_uri) ? uri.split('?').first : '' + path[relative_url_root.length..-1] # cut off the part of the url which leads to the installation directory of this app end - # returns the path minus the web server relative - # installation directory - def relative_url_root - @@relative_url_root ||= File.dirname(env["SCRIPT_NAME"].to_s).gsub /(^\.$|^\/$)/, '' + # Returns the path minus the web server relative installation directory + def relative_url_root(force_reload = false) + @@relative_url_root ||= File.dirname(env["SCRIPT_NAME"].to_s).gsub(/(^\.$|^\/$)/, '') end def port @@ -158,14 +165,6 @@ module ActionController @path_parameters ||= {} end - # Returns true if the request's "X-Requested-With" header contains - # "XMLHttpRequest". (The Prototype Javascript library sends this header with - # every Ajax request.) - def xml_http_request? - env['HTTP_X_REQUESTED_WITH'] =~ /XMLHttpRequest/i - end - alias xhr? :xml_http_request? - #-- # Must be implemented in the concrete request #++ diff --git a/actionpack/lib/action_controller/response.rb b/actionpack/lib/action_controller/response.rb index 2e6c5fcc37..227aa27cc5 100755 --- a/actionpack/lib/action_controller/response.rb +++ b/actionpack/lib/action_controller/response.rb @@ -8,7 +8,7 @@ module ActionController end def redirect(to_url, permanently = false) - @headers["Status"] = permanently ? "301 Moved Permanently" : "302 Found" + @headers["Status"] ||= "302 Found" @headers["location"] = to_url @body = "You are being redirected." diff --git a/actionpack/test/controller/action_pack_assertions_test.rb b/actionpack/test/controller/action_pack_assertions_test.rb index 3fc8a2c99a..ccde516f31 100644 --- a/actionpack/test/controller/action_pack_assertions_test.rb +++ b/actionpack/test/controller/action_pack_assertions_test.rb @@ -13,7 +13,7 @@ class ActionPackAssertionsController < ActionController::Base def hello_xml_world() render "test/hello_xml_world"; end # a redirect to an internal location - def redirect_internal() redirect_to "nothing"; end + def redirect_internal() redirect_to "/nothing"; end def redirect_to_action() redirect_to :action => "flash_me", :id => 1, :params => { "panda" => "fun" }; end @@ -270,7 +270,7 @@ class ActionPackAssertionsControllerTest < Test::Unit::TestCase # check the redirection location def test_redirection_location process :redirect_internal - assert_equal 'nothing', @response.redirect_url + assert_equal 'http://test.host/nothing', @response.redirect_url process :redirect_external assert_equal 'http://www.rubyonrails.org', @response.redirect_url diff --git a/actionpack/test/controller/new_render_test.rb b/actionpack/test/controller/new_render_test.rb new file mode 100644 index 0000000000..96037093fc --- /dev/null +++ b/actionpack/test/controller/new_render_test.rb @@ -0,0 +1,237 @@ +require File.dirname(__FILE__) + '/../abstract_unit' + +Customer = Struct.new("Customer", :name) + +module Fun + class GamesController < ActionController::Base + def hello_world + end + end +end + + +class TestController < ActionController::Base + layout :determine_layout + + def hello_world + end + + def render_hello_world + r :template => "test/hello_world" + end + + def render_hello_world_from_variable + @person = "david" + r :text => "hello #{@person}" + end + + def render_action_hello_world + r :action => "hello_world" + end + + def render_text_hello_world + r :text => "hello world" + end + + def render_custom_code + r :text => "hello world", :status => "404 Moved" + end + + def render_xml_hello + @name = "David" + r :template => "test/hello" + end + + def greeting + # let's just rely on the template + end + + def layout_test + r :action => "hello_world" + end + + def layout_test_with_different_layout + r :action => "hello_world", :layout => "standard" + end + + def rendering_without_layout + r :action => "hello_world", :layout => false + end + + def builder_layout_test + r :action => "hello" + end + + def partials_list + @customers = [ Customer.new("david"), Customer.new("mary") ] + r :action => "list" + end + + def partial_only + render_partial + end + + def hello_in_a_string + @customers = [ Customer.new("david"), Customer.new("mary") ] + r :text => "How's there? #{render_to_string("test/list")}" + end + + def accessing_params_in_template + r :inline => "Hello: <%= params[:name] %>" + end + + def rescue_action(e) raise end + + private + def determine_layout + case action_name + when "layout_test", "rendering_without_layout" + "layouts/standard" + when "builder_layout_test" + "layouts/builder" + end + end +end + +TestController.template_root = File.dirname(__FILE__) + "/../fixtures/" +Fun::GamesController.template_root = File.dirname(__FILE__) + "/../fixtures/" + +class TestLayoutController < ActionController::Base + layout "layouts/standard" + + def hello_world + end + + def hello_world_outside_layout + end + + def rescue_action(e) + raise unless ActionController::MissingTemplate === e + end +end + +class RenderTest < Test::Unit::TestCase + def setup + @request = ActionController::TestRequest.new + @response = ActionController::TestResponse.new + + @request.host = "www.nextangle.com" + end + + def test_simple_show + @request.action = "hello_world" + response = process_request + assert_equal "200 OK", response.headers["Status"] + assert_equal "test/hello_world", response.template.first_render + end + + def test_do_with_render + @request.action = "render_hello_world" + assert_equal "test/hello_world", process_request.template.first_render + end + + def test_do_with_render_from_variable + @request.action = "render_hello_world_from_variable" + assert_equal "hello david", process_request.body + end + + def test_do_with_render_action + @request.action = "render_action_hello_world" + assert_equal "test/hello_world", process_request.template.first_render + end + + def test_do_with_render_text + @request.action = "render_text_hello_world" + assert_equal "hello world", process_request.body + end + + def test_do_with_render_custom_code + @request.action = "render_custom_code" + assert_equal "404 Moved", process_request.headers["Status"] + end + + def test_attempt_to_access_object_method + @request.action = "clone" + assert_raises(ActionController::UnknownAction, "No action responded to [clone]") { process_request } + end + + def test_private_methods + @request.action = "determine_layout" + assert_raises(ActionController::UnknownAction, "No action responded to [determine_layout]") { process_request } + end + + def test_access_to_request_in_view + ActionController::Base.view_controller_internals = false + + @request.action = "hello_world" + response = process_request + assert_nil response.template.assigns["request"] + + ActionController::Base.view_controller_internals = true + + @request.action = "hello_world" + response = process_request + assert_kind_of ActionController::AbstractRequest, response.template.assigns["request"] + end + + def test_render_xml + @request.action = "render_xml_hello" + assert_equal "\n

Hello David

\n

This is grand!

\n\n", process_request.body + end + + def test_render_xml_with_default + @request.action = "greeting" + assert_equal "

This is grand!

\n", process_request.body + end + + def test_layout_rendering + @request.action = "layout_test" + assert_equal "Hello world!", process_request.body + end + + def test_layout_test_with_different_layout + @request.action = "layout_test_with_different_layout" + assert_equal "Hello world!", process_request.body + end + + def test_rendering_without_layout + @request.action = "rendering_without_layout" + assert_equal "Hello world!", process_request.body + end + + def test_render_xml_with_layouts + @request.action = "builder_layout_test" + assert_equal "\n\n

Hello

\n

This is grand!

\n\n
\n", process_request.body + end + + def test_partials_list + @request.action = "partials_list" + assert_equal "Hello: davidHello: mary", process_request.body + end + + def test_partial_only + @request.action = "partial_only" + assert_equal "only partial", process_request.body + end + + def test_render_to_string + @request.action = "hello_in_a_string" + assert_equal "How's there? Hello: davidHello: mary", process_request.body + end + + def test_nested_rendering + @request.action = "hello_world" + assert_equal "Living in a nested world", Fun::GamesController.process(@request, @response).body + end + + def test_accessing_params_in_template + @request.action = "accessing_params_in_template" + @request.query_parameters[:name] = "David" + assert_equal "Hello: David", process_request.body + end + + private + def process_request + TestController.process(@request, @response) + end +end \ No newline at end of file diff --git a/actionpack/test/controller/request_test.rb b/actionpack/test/controller/request_test.rb index 76082c2dbd..00fd41b8dd 100644 --- a/actionpack/test/controller/request_test.rb +++ b/actionpack/test/controller/request_test.rb @@ -65,7 +65,7 @@ class RequestTest < Test::Unit::TestCase assert_equal ":8080", @request.port_string end - def test_relative_url_root + def test_relative_url_root @request.env['SCRIPT_NAME'] = nil assert_equal "", @request.relative_url_root @@ -75,92 +75,108 @@ class RequestTest < Test::Unit::TestCase @request.env['SCRIPT_NAME'] = "/myapp.rb" assert_equal "", @request.relative_url_root + @request.relative_url_root = nil @request.env['SCRIPT_NAME'] = "/hieraki/dispatch.cgi" assert_equal "/hieraki", @request.relative_url_root + @request.relative_url_root = nil @request.env['SCRIPT_NAME'] = "/collaboration/hieraki/dispatch.cgi" assert_equal "/collaboration/hieraki", @request.relative_url_root end def test_request_uri + @request.relative_url_root = nil @request.set_REQUEST_URI "http://www.rubyonrails.org/path/of/some/uri?mapped=1" assert_equal "/path/of/some/uri?mapped=1", @request.request_uri assert_equal "/path/of/some/uri", @request.path + @request.relative_url_root = nil @request.set_REQUEST_URI "http://www.rubyonrails.org/path/of/some/uri" assert_equal "/path/of/some/uri", @request.request_uri assert_equal "/path/of/some/uri", @request.path + @request.relative_url_root = nil @request.set_REQUEST_URI "/path/of/some/uri" assert_equal "/path/of/some/uri", @request.request_uri assert_equal "/path/of/some/uri", @request.path + @request.relative_url_root = nil @request.set_REQUEST_URI "/" assert_equal "/", @request.request_uri assert_equal "/", @request.path + @request.relative_url_root = nil @request.set_REQUEST_URI "/?m=b" assert_equal "/?m=b", @request.request_uri assert_equal "/", @request.path + @request.relative_url_root = nil @request.set_REQUEST_URI "/" @request.env['SCRIPT_NAME'] = "/dispatch.cgi" assert_equal "/", @request.request_uri assert_equal "/", @request.path + @request.relative_url_root = nil @request.set_REQUEST_URI "/hieraki/" @request.env['SCRIPT_NAME'] = "/hieraki/dispatch.cgi" assert_equal "/hieraki/", @request.request_uri assert_equal "/", @request.path + @request.relative_url_root = nil @request.set_REQUEST_URI "/collaboration/hieraki/books/edit/2" @request.env['SCRIPT_NAME'] = "/collaboration/hieraki/dispatch.cgi" assert_equal "/collaboration/hieraki/books/edit/2", @request.request_uri assert_equal "/books/edit/2", @request.path # The following tests are for when REQUEST_URI is not supplied (as in IIS) + @request.relative_url_root = nil @request.set_REQUEST_URI nil @request.env['PATH_INFO'] = "/path/of/some/uri?mapped=1" @request.env['SCRIPT_NAME'] = nil #"/path/dispatch.rb" assert_equal "/path/of/some/uri?mapped=1", @request.request_uri assert_equal "/path/of/some/uri", @request.path + @request.relative_url_root = nil @request.env['PATH_INFO'] = "/path/of/some/uri?mapped=1" @request.env['SCRIPT_NAME'] = "/path/dispatch.rb" assert_equal "/path/of/some/uri?mapped=1", @request.request_uri assert_equal "/of/some/uri", @request.path + @request.relative_url_root = nil @request.env['PATH_INFO'] = "/path/of/some/uri" @request.env['SCRIPT_NAME'] = nil assert_equal "/path/of/some/uri", @request.request_uri assert_equal "/path/of/some/uri", @request.path + @request.relative_url_root = nil @request.env['PATH_INFO'] = "/" assert_equal "/", @request.request_uri assert_equal "/", @request.path + @request.relative_url_root = nil @request.env['PATH_INFO'] = "/?m=b" assert_equal "/?m=b", @request.request_uri assert_equal "/", @request.path + @request.relative_url_root = nil @request.env['PATH_INFO'] = "/" @request.env['SCRIPT_NAME'] = "/dispatch.cgi" assert_equal "/", @request.request_uri assert_equal "/", @request.path + @request.relative_url_root = nil @request.env['PATH_INFO'] = "/hieraki/" @request.env['SCRIPT_NAME'] = "/hieraki/dispatch.cgi" assert_equal "/hieraki/", @request.request_uri assert_equal "/", @request.path # This test ensures that Rails uses REQUEST_URI over PATH_INFO + @request.relative_url_root = nil @request.env['REQUEST_URI'] = "/some/path" @request.env['PATH_INFO'] = "/another/path" @request.env['SCRIPT_NAME'] = "/dispatch.cgi" assert_equal "/some/path", @request.request_uri assert_equal "/some/path", @request.path - - end diff --git a/actionpack/test/template/form_options_helper_test.rb b/actionpack/test/template/form_options_helper_test.rb index cedf17c61b..453afa33e2 100644 --- a/actionpack/test/template/form_options_helper_test.rb +++ b/actionpack/test/template/form_options_helper_test.rb @@ -1,5 +1,4 @@ -require 'test/unit' -require File.dirname(__FILE__) + '/../../lib/action_view/helpers/form_options_helper' +require File.dirname(__FILE__) + '/../abstract_unit' class MockTimeZone attr_reader :name @@ -260,7 +259,7 @@ class FormOptionsHelperTest < Test::Unit::TestCase assert_equal( "", country_select("post", "origin") ) -- cgit v1.2.3