Implemented redirects and partial rendering in new base.

216309c1 · Yehuda Katz + Carl Lerche · 72ca7c59 · 216309c1 · 216309c1 · 216309c1
20 changed file
--- a/actionpack/lib/action_controller/abstract/base.rb
+++ b/actionpack/lib/action_controller/abstract/base.rb
 module AbstractController
+  class Error < StandardError; end
+  
+  class DoubleRenderError < Error
+    DEFAULT_MESSAGE = "Render and/or redirect were called multiple times in this action. Please note that you may only call render OR redirect, and at most once per action. Also note that neither redirect nor render terminate execution of the action, so if you want to exit an action after redirecting, you need to do something like \"redirect_to(...) and return\"."
+
+    def initialize(message = nil)
+      super(message || DEFAULT_MESSAGE)
+    end
+  end  
+  
  class Base
    
    attr_internal :response_body

--- a/actionpack/lib/action_controller/abstract/layouts.rb
+++ b/actionpack/lib/action_controller/abstract/layouts.rb
@@ -51,7 +51,7 @@ def _layout
    end
    
    def _render_template(template, options)
-      _action_view._render_template_with_layout(template, options[:_layout], options)
+      _action_view._render_template_from_controller(template, options[:_layout], options, options[:_partial])
    end
        
  private

--- a/actionpack/lib/action_controller/abstract/renderer.rb
+++ b/actionpack/lib/action_controller/abstract/renderer.rb
 require "action_controller/abstract/logger"

 module AbstractController
-  class AbstractControllerError < StandardError; end
-  class DoubleRenderError < AbstractControllerError
-    DEFAULT_MESSAGE = "Render and/or redirect were called multiple times in this action. Please note that you may only call render OR redirect, and at most once per action. Also note that neither redirect nor render terminate execution of the action, so if you want to exit an action after redirecting, you need to do something like \"redirect_to(...) and return\"."
-
-    def initialize(message = nil)
-      super(message || DEFAULT_MESSAGE)
-    end
-  end  
-  
  module Renderer
    extend ActiveSupport::DependencyModule

@@ -27,12 +18,12 @@ def _action_view
      @_action_view ||= ActionView::Base.new(self.class.view_paths, {}, self)      
    end
        
-    def render(options = {})
+    def render(*args)
      if response_body
        raise AbstractController::DoubleRenderError, "OMG"
      end
      
-      self.response_body = render_to_body(options)
+      self.response_body = render_to_body(*args)
    end
    
    # Raw rendering of a template to a Rack-compatible body.
@@ -43,10 +34,16 @@ def render(options = {})
    # :api: plugin
    def render_to_body(options = {})
      name = options[:_template_name] || action_name
-      
-      options[:_template] ||= view_paths.find_by_parts(name.to_s, {:formats => formats}, options[:_prefix])
-      
-      _render_template(options[:_template], options)
+
+      # TODO: Refactor so we can just use the normal template logic for this
+      if options[:_partial_object]
+        _action_view._render_partial_from_controller(options)
+      else 
+        options[:_template] ||= view_paths.find_by_parts(name.to_s, {:formats => formats}, 
+        options[:_prefix], options[:_partial])
+
+        _render_template(options[:_template], options)
+      end
    end

    # Raw rendering of a template to a string.
@@ -60,7 +57,7 @@ def render_to_string(options = {})
    end

    def _render_template(template, options)
-      _action_view._render_template_with_layout(template)
+      _action_view._render_template_from_controller(template, nil, options, options[:_partial])
    end
    
    def view_paths() _view_paths end

--- a/actionpack/lib/action_controller/base/redirect.rb
+++ b/actionpack/lib/action_controller/base/redirect.rb
@@ -70,7 +70,9 @@ def redirect_to(options = {}, response_status = {}) #:doc:
    def redirect_to_full_url(url, status)
      raise DoubleRenderError if performed?
      logger.info("Redirected to #{url}") if logger && logger.info?
-      response.redirect(url, interpret_status(status))
+      response.status = interpret_status(status)
+      response.location = url.gsub(/[\r\n]/, '')
+      response.body = "<html><body>You are being <a href=\"#{CGI.escapeHTML(url)}\">redirected</a>.</body></html>"      
      @performed_redirect = true
    end
    

--- a/actionpack/lib/action_controller/new_base.rb
+++ b/actionpack/lib/action_controller/new_base.rb
@@ -5,6 +5,7 @@ module ActionController
  autoload :Http,                 "action_controller/new_base/http"
  autoload :Layouts,              "action_controller/new_base/layouts"
  autoload :Rails2Compatibility,  "action_controller/new_base/compatibility"
+  autoload :Redirector,           "action_controller/new_base/redirector"
  autoload :Renderer,             "action_controller/new_base/renderer"
  autoload :Testing,              "action_controller/new_base/testing"
  autoload :UrlFor,               "action_controller/new_base/url_for"

--- a/actionpack/lib/action_controller/new_base/base.rb
+++ b/actionpack/lib/action_controller/new_base/base.rb
@@ -8,6 +8,7 @@ class Base < Http
    
    include ActionController::HideActions
    include ActionController::UrlFor
+    include ActionController::Redirector
    include ActionController::Renderer
    include ActionController::Layouts
    include ActionController::ConditionalGet
@@ -34,19 +35,83 @@ def self.app_loaded!
      end
    end
    
-    def render(action = action_name, options = {})
+    def render_to_body(action = nil, options = {})
      if action.is_a?(Hash)
        options, action = action, nil 
-      else
-        options.merge! :action => action
+      elsif action.is_a?(String) || action.is_a?(Symbol)
+        key = case action = action.to_s
+        when %r{^/} then :file
+        when %r{/}  then :template
+        else             :action
+        end        
+        options.merge! key => action
+      elsif action
+        options.merge! :partial => action
+      end
+      
+      if options.key?(:action) && options[:action].to_s.index("/")
+        options[:template] = options.delete(:action)
      end
      
-      super(options)
+      # options = {:template => options.to_s} if options.is_a?(String) || options.is_a?(Symbol)
+      super(options) || " "
    end
    
-    def render_to_body(options = {})
-      options = {:template => options} if options.is_a?(String)
-      super
+    # Redirects the browser to the target specified in +options+. This parameter can take one of three forms:
+    #
+    # * <tt>Hash</tt> - The URL will be generated by calling url_for with the +options+.
+    # * <tt>Record</tt> - The URL will be generated by calling url_for with the +options+, which will reference a named URL for that record.
+    # * <tt>String</tt> starting with <tt>protocol://</tt> (like <tt>http://</tt>) - Is passed straight through as the target for redirection.
+    # * <tt>String</tt> not containing a protocol - The current protocol and host is prepended to the string.
+    # * <tt>:back</tt> - Back to the page that issued the request. Useful for forms that are triggered from multiple places.
+    #   Short-hand for <tt>redirect_to(request.env["HTTP_REFERER"])</tt>
+    #
+    # Examples:
+    #   redirect_to :action => "show", :id => 5
+    #   redirect_to post
+    #   redirect_to "http://www.rubyonrails.org"
+    #   redirect_to "/images/screenshot.jpg"
+    #   redirect_to articles_url
+    #   redirect_to :back
+    #
+    # The redirection happens as a "302 Moved" header unless otherwise specified.
+    #
+    # Examples:
+    #   redirect_to post_url(@post), :status=>:found
+    #   redirect_to :action=>'atom', :status=>:moved_permanently
+    #   redirect_to post_url(@post), :status=>301
+    #   redirect_to :action=>'atom', :status=>302
+    #
+    # When using <tt>redirect_to :back</tt>, if there is no referrer,
+    # RedirectBackError will be raised. You may specify some fallback
+    # behavior for this case by rescuing RedirectBackError.    
+    def redirect_to(options = {}, response_status = {}) #:doc:
+      raise ActionControllerError.new("Cannot redirect to nil!") if options.nil?
+
+      status = if options.is_a?(Hash) && options.key?(:status)
+        interpret_status(options.delete(:status))
+      elsif response_status.key?(:status)
+        interpret_status(response_status[:status])
+      else
+        302
+      end
+
+      url = case options
+      # The scheme name consist of a letter followed by any combination of
+      # letters, digits, and the plus ("+"), period ("."), or hyphen ("-")
+      # characters; and is terminated by a colon (":").
+      when %r{^\w[\w\d+.-]*:.*}
+        options
+      when String
+        request.protocol + request.host_with_port + options
+      when :back
+        raise RedirectBackError unless refer = request.headers["Referer"]
+        refer
+      else
+        url_for(options)
+      end
+      
+      super(url, status)
    end
    
    def process_action

--- a/actionpack/lib/action_controller/new_base/compatibility.rb
+++ b/actionpack/lib/action_controller/new_base/compatibility.rb
@@ -8,27 +8,40 @@ module Rails2Compatibility
      ::ActionController::DoubleRenderError = ::AbstractController::DoubleRenderError
      
      cattr_accessor :session_options
-      self.send(:class_variable_set, "@@session_options", {})
+      self.session_options = {}
      
      cattr_accessor :allow_concurrency
-      self.send(:class_variable_set, "@@allow_concurrency", false)
+      self.allow_concurrency = false
      
      cattr_accessor :param_parsers
-      self.send(:class_variable_set, "@@param_parsers", { Mime::MULTIPART_FORM   => :multipart_form,
-                          Mime::URL_ENCODED_FORM => :url_encoded_form,
-                          Mime::XML              => :xml_simple,
-                          Mime::JSON             => :json })
+      self.param_parsers = { Mime::MULTIPART_FORM   => :multipart_form,
+                             Mime::URL_ENCODED_FORM => :url_encoded_form,
+                             Mime::XML              => :xml_simple,
+                             Mime::JSON             => :json }
                          
      cattr_accessor :relative_url_root
-      self.send(:class_variable_set, "@@relative_url_root", ENV['RAILS_RELATIVE_URL_ROOT'])
+      self.relative_url_root = ENV['RAILS_RELATIVE_URL_ROOT']
      
      cattr_accessor :default_charset
-      self.send(:class_variable_set, "@@default_charset", "utf-8")
+      self.default_charset = "utf-8"
      
-      cattr_reader :protected_instance_variables
-      self.send(:class_variable_set, "@@protected_instance_variables", %w(@assigns @performed_redirect @performed_render @variables_added @request_origin @url @parent_controller
+      # cattr_reader :protected_instance_variables
+      cattr_accessor :protected_instance_variables
+      self.protected_instance_variables = %w(@assigns @performed_redirect @performed_render @variables_added @request_origin @url @parent_controller
                                          @action_name @before_filter_chain_aborted @action_cache_path @_headers @_params
-                                          @_flash @_response))
+                                          @_flash @_response)
+                                          
+      # Indicates whether or not optimise the generated named
+      # route helper methods
+      cattr_accessor :optimise_named_routes
+      self.optimise_named_routes = true
+      
+      cattr_accessor :resources_path_names
+      self.resources_path_names = { :new => 'new', :edit => 'edit' }
+      
+      # Controls the resource action separator
+      cattr_accessor :resource_action_separator
+      self.resource_action_separator = "/"
    end
    
    module ClassMethods
@@ -38,7 +51,12 @@ def rescue_action(env)
        raise env["action_dispatch.rescue.exception"]
      end
    end
-  
+    
+    def initialize(*)
+      super
+      @template = _action_view
+    end
+    
    def render_to_body(options)
      if options.is_a?(Hash) && options.key?(:template)
        options[:template].sub!(/^\//, '')

--- a/actionpack/lib/action_controller/new_base/http.rb
+++ b/actionpack/lib/action_controller/new_base/http.rb
@@ -38,6 +38,12 @@ def self.call(env)
      controller.call(env).to_rack
    end
    
+    delegate :headers, :to => "@_response"
+    
+    def params
+      @_params ||= @_request.parameters
+    end
+    
    # :api: private
    def call(name, env)
      @_request = ActionDispatch::Request.new(env)

--- a/actionpack/lib/action_controller/new_base/layouts.rb
+++ b/actionpack/lib/action_controller/new_base/layouts.rb
@@ -15,7 +15,7 @@ def render_to_body(options)
      # render :text => ..., :layout => ...
      # or
      # render :anything_else
-      if (!options.key?(:text) && !options.key?(:inline)) || options.key?(:layout)
+      if (!options.key?(:text) && !options.key?(:inline) && !options.key?(:partial)) || options.key?(:layout)
        options[:_layout] = options.key?(:layout) ? _layout_for_option(options[:layout]) : _default_layout
      end
      

--- a/actionpack/lib/action_controller/new_base/redirector.rb
+++ b/actionpack/lib/action_controller/new_base/redirector.rb
+module ActionController
+  class RedirectBackError < AbstractController::Error #:nodoc:
+    DEFAULT_MESSAGE = 'No HTTP_REFERER was set in the request to this action, so redirect_to :back could not be called successfully. If this is a test, make sure to specify request.env["HTTP_REFERER"].'
+
+    def initialize(message = nil)
+      super(message || DEFAULT_MESSAGE)
+    end
+  end
+  
+  module Redirector
+    def redirect_to(url, status) #:doc:
+      raise AbstractController::DoubleRenderError if response_body
+      logger.info("Redirected to #{url}") if logger && logger.info?
+      response.status = status
+      response.location = url.gsub(/[\r\n]/, '')
+      self.response_body = "<html><body>You are being <a href=\"#{CGI.escapeHTML(url)}\">redirected</a>.</body></html>"
+    end
+  end
+end
\ No newline at end of file
--- a/actionpack/lib/action_controller/new_base/renderer.rb
+++ b/actionpack/lib/action_controller/new_base/renderer.rb
@@ -9,13 +9,9 @@ def initialize(*)
      super
    end
    
-    def render(options = {})
+    def render_to_body(options)
      _process_options(options)
      
-      super(options)
-    end
-
-    def render_to_body(options)
      if options.key?(:text)
        options[:_template] = ActionView::TextTemplate.new(_text(options))
        template = nil
@@ -25,12 +21,18 @@ def render_to_body(options)
        options[:_template] = template
      elsif options.key?(:template)
        options[:_template_name] = options[:template]
+      elsif options.key?(:file)
+        options[:_template_name] = options[:file]
+      elsif options.key?(:partial)
+        _render_partial(options[:partial], options)
      else
        options[:_template_name] = (options[:action] || action_name).to_s
        options[:_prefix] = _prefix 
      end
      
      ret = super(options)
+      
+      options[:_template] ||= _action_view._partial
      response.content_type ||= options[:_template].mime_type
      ret
    end
@@ -49,6 +51,21 @@ def _text(options)
      else text.to_s
      end
    end
+    
+    def _render_partial(partial, options)
+      case partial
+      when true
+        options[:_prefix] = _prefix
+      when String
+        options[:_prefix] = _prefix unless partial.index('/')
+        options[:_template_name] = partial
+      else
+        options[:_partial_object] = true
+        return
+      end
+      
+      options[:_partial] = options[:object] || true
+    end
  
    def _process_options(options)
      status, content_type = options.values_at(:status, :content_type)

--- a/actionpack/lib/action_controller/testing/process.rb
+++ b/actionpack/lib/action_controller/testing/process.rb
@@ -161,11 +161,13 @@ def xml_http_request(request_method, action, parameters = nil, session = nil, fl
    alias xhr :xml_http_request

    def assigns(key = nil)
-      if key.nil?
-        @controller.template.assigns
-      else
-        @controller.template.assigns[key.to_s]
+      assigns = {}
+      @controller.instance_variable_names.each do |ivar|
+        next if ActionController::Base.protected_instance_variables.include?(ivar)
+        assigns[ivar[1..-1]] = @controller.instance_variable_get(ivar)
      end
+      
+      key.nil? ? assigns : assigns[key.to_s]
    end

    def session

--- a/actionpack/lib/action_controller/testing/process2.rb
+++ b/actionpack/lib/action_controller/testing/process2.rb
@@ -50,6 +50,7 @@ def process(action, parameters = nil, session = nil, flash = nil, http_method =
      @request.session = ActionController::TestSession.new(session) unless session.nil?
      @request.session["flash"] = ActionController::Flash::FlashHash.new.update(flash) if flash
      build_request_uri(action, parameters)
+      @controller.request = @request
      @controller.params.merge!(parameters)
      # Base.class_eval { include ProcessWithTest } unless Base < ProcessWithTest
      @controller.process_with_test(@request, @response)

--- a/actionpack/lib/action_dispatch/http/response.rb
+++ b/actionpack/lib/action_dispatch/http/response.rb
@@ -156,12 +156,6 @@ def etag=(etag)
      end
    end

-    def redirect(url, status)
-      self.status = status
-      self.location = url.gsub(/[\r\n]/, '')
-      self.body = "<html><body>You are being <a href=\"#{CGI.escapeHTML(url)}\">redirected</a>.</body></html>"
-    end
-
    def sending_file?
      headers["Content-Transfer-Encoding"] == "binary"
    end

--- a/actionpack/lib/action_view/render/partials.rb
+++ b/actionpack/lib/action_view/render/partials.rb
@@ -171,11 +171,21 @@ module ActionView
  #   <% end %>
  module Partials
    extend ActiveSupport::Memoizable
+    extend ActiveSupport::DependencyModule
+    
+    included do
+      attr_accessor :_partial      
+    end
+
+    def _render_partial_from_controller(*args)
+      @assigns_added = false
+      _render_partial(*args)
+    end

    def _render_partial(options = {}) #:nodoc:
      options[:locals] ||= {}

-      case path = partial = options[:partial]        
+      case path = partial = options[:partial]
      when *_array_like_objects
        return _render_partial_collection(partial, options)
      else
@@ -222,16 +232,7 @@ def _render_partial_with_block(layout, block, options)
      ensure
        @_proc_for_layout = nil
      end
-  
-      def _render_partial_with_layout(layout, options)
-        if layout
-          prefix = controller && !layout.include?("/") ? controller.controller_path : nil
-          layout = find_by_parts(layout, {:formats => formats}, prefix, true)
-        end
-        content = _render_partial(options)
-        return _render_content_with_layout(content, layout, options[:locals])
-      end
-    
+      
      def _deprecated_ivar_assign(template)
        if respond_to?(:controller)
          ivar = :"@#{template.variable_name}"
@@ -287,13 +288,17 @@ def _render_partial_object(template, options, object = nil)
          locals = (options[:locals] ||= {})
          object ||= locals[:object] || locals[template.variable_name]
          
-          _set_locals(object, locals, template, options)          
+          _set_locals(object, locals, template, options)
+          
+          self._partial = template
+          
          _render_template(template, locals)
        end
      end

      def _set_locals(object, locals, template, options)
        object ||= _deprecated_ivar_assign(template)
+        
        locals[:object] = locals[template.variable_name] = object
        locals[options[:as]] = object if options[:as]
      end
@@ -316,6 +321,9 @@ def _render_partial_collection(collection, options = {}, passed_template = nil)
          locals[template.counter_name] = index
          
          index += 1
+          
+          self._partial = template
+          
          _render_template(template, locals)
        end.join(spacer)
      end

--- a/actionpack/lib/action_view/render/rendering.rb
+++ b/actionpack/lib/action_view/render/rendering.rb
@@ -95,6 +95,11 @@ def _render_text(text, layout, options)
      layout ? _render_content_with_layout(text, layout, options[:locals]) : text
    end

+    def _render_template_from_controller(*args)
+      @assigns_added = nil
+      _render_template_with_layout(*args)
+    end
+
    def _render_template_with_layout(template, layout = nil, options = {}, partial = false)
      if controller && logger
        logger.info("Rendering #{template.identifier}" + 

--- a/actionpack/test/abstract_unit.rb
+++ b/actionpack/test/abstract_unit.rb
+if ENV["new_base"]
+  require "abstract_unit2"
+else
 $:.unshift(File.dirname(__FILE__) + '/../lib')
 $:.unshift(File.dirname(__FILE__) + '/../../activesupport/lib')
 $:.unshift(File.dirname(__FILE__) + '/fixtures/helpers')
@@ -38,3 +41,4 @@

 FIXTURE_LOAD_PATH = File.join(File.dirname(__FILE__), 'fixtures')
 ActionController::Base.view_paths = FIXTURE_LOAD_PATH
+end
\ No newline at end of file
--- a/actionpack/test/abstract_unit2.rb
+++ b/actionpack/test/abstract_unit2.rb
@@ -75,7 +75,7 @@ class UnknownHttpMethod < ActionControllerError #:nodoc:
  end
  
  class Base
-    use ActionController::Testing
+    include ActionController::Testing
  end
  
  Base.view_paths = FIXTURE_LOAD_PATH
@@ -89,43 +89,42 @@ class TestCase
    end
    
    def assert_template(options = {}, message = nil)
-      validate_response!
-
-      clean_backtrace do
-        case options
-         when NilClass, String
-          hax = @controller._action_view.instance_variable_get(:@_rendered)
-          rendered = (hax[:template] || []).map { |t| t.identifier }
-          msg = build_message(message,
-                  "expecting <?> but rendering with <?>",
-                  options, rendered.join(', '))
-          assert_block(msg) do
-            if options.nil?
-              hax[:template].blank?
-            else
-              rendered.any? { |t| t.match(options) }
-            end
+      validate_request!
+
+      hax = @controller._action_view.instance_variable_get(:@_rendered)
+
+      case options
+       when NilClass, String
+        rendered = (hax[:template] || []).map { |t| t.identifier }
+        msg = build_message(message,
+                "expecting <?> but rendering with <?>",
+                options, rendered.join(', '))
+        assert_block(msg) do
+          if options.nil?
+            hax[:template].blank?
+          else
+            rendered.any? { |t| t.match(options) }
          end
-        when Hash
-          if expected_partial = options[:partial]
-            partials = hax[:partials]
-            if expected_count = options[:count]
-              found = partials.detect { |p, _| p.identifier.match(expected_partial) }
-              actual_count = found.nil? ? 0 : found.second
-              msg = build_message(message,
-                      "expecting ? to be rendered ? time(s) but rendered ? time(s)",
-                       expected_partial, expected_count, actual_count)
-              assert(actual_count == expected_count.to_i, msg)
-            else
-              msg = build_message(message,
-                      "expecting partial <?> but action rendered <?>",
-                      options[:partial], partials.keys)
-              assert(partials.keys.any? { |p| p.identifier.match(expected_partial) }, msg)
-            end
+        end
+      when Hash
+        if expected_partial = options[:partial]
+          partials = hax[:partials]
+          if expected_count = options[:count]
+            found = partials.detect { |p, _| p.identifier.match(expected_partial) }
+            actual_count = found.nil? ? 0 : found.second
+            msg = build_message(message,
+                    "expecting ? to be rendered ? time(s) but rendered ? time(s)",
+                     expected_partial, expected_count, actual_count)
+            assert(actual_count == expected_count.to_i, msg)
          else
-            assert hax[:partials].empty?,
-              "Expected no partials to be rendered"
+            msg = build_message(message,
+                    "expecting partial <?> but action rendered <?>",
+                    options[:partial], partials.keys)
+            assert(partials.keys.any? { |p| p.identifier.match(expected_partial) }, msg)
          end
+        else
+          assert hax[:partials].empty?,
+            "Expected no partials to be rendered"
        end
      end
    end    

--- a/actionpack/test/controller/render_test.rb
+++ b/actionpack/test/controller/render_test.rb
-require ENV['new_base'] ? 'abstract_unit2' : 'abstract_unit'
+require 'abstract_unit'
 require 'controller/fake_models'
 require 'pathname'

@@ -1647,7 +1647,7 @@ def test_render_200_should_set_etag
  def test_render_against_etag_request_should_304_when_match
    @request.if_none_match = etag_for("hello david")
    get :render_hello_world_from_variable
-    assert_equal 304, @response.status
+    assert_equal 304, @response.status.to_i
    assert @response.body.empty?
  end


--- a/actionpack/test/new_base/redirect_test.rb
+++ b/actionpack/test/new_base/redirect_test.rb
+require File.join(File.expand_path(File.dirname(__FILE__)), "test_helper")
\ No newline at end of file