Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Stopped the massive bleeding of concerns into ActionController::Base.…

… Base no longer knows about flash, filters, or components. This may well have introduced some instability, please do test with apps, especially the ones using components. [DHH]

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@3580 5ecf4fe2-1ee6-0310-87b1-e25e094e27de
  • Loading branch information...
commit 050c3964d8fe8d68c03fff593e3c09b5eae77a46 1 parent 838ec41
@dhh dhh authored
View
8 actionpack/lib/action_controller.rb
@@ -38,9 +38,9 @@
require 'action_controller/deprecated_redirects'
require 'action_controller/rescue'
require 'action_controller/benchmarking'
+require 'action_controller/flash'
require 'action_controller/filters'
require 'action_controller/layout'
-require 'action_controller/flash'
require 'action_controller/dependencies'
require 'action_controller/pagination'
require 'action_controller/scaffolding'
@@ -48,10 +48,10 @@
require 'action_controller/cookies'
require 'action_controller/cgi_process'
require 'action_controller/caching'
-require 'action_controller/components'
require 'action_controller/verification'
require 'action_controller/streaming'
require 'action_controller/session_management'
+require 'action_controller/components'
require 'action_controller/macros/auto_complete'
require 'action_controller/macros/in_place_editing'
@@ -59,9 +59,9 @@
ActionController::Base.template_class = ActionView::Base
ActionController::Base.class_eval do
+ include ActionController::Flash
include ActionController::Filters
include ActionController::Layout
- include ActionController::Flash
include ActionController::Benchmarking
include ActionController::Rescue
include ActionController::Dependencies
@@ -70,10 +70,10 @@
include ActionController::Helpers
include ActionController::Cookies
include ActionController::Caching
- include ActionController::Components
include ActionController::Verification
include ActionController::Streaming
include ActionController::SessionManagement
+ include ActionController::Components
include ActionController::Macros::AutoComplete
include ActionController::Macros::InPlaceEditing
end
View
75 actionpack/lib/action_controller/base.rb
@@ -305,8 +305,8 @@ class Base
class << self
# Factory for the standard create, process loop where the controller is discarded after processing.
- def process(request, response, parent_controller = nil) #:nodoc:
- new(parent_controller).process(request, response)
+ def process(request, response) #:nodoc:
+ new.process(request, response)
end
# Converts the class name from something like "OneModule::TwoModule::NeatController" to "NeatController".
@@ -347,42 +347,20 @@ def hide_action(*names)
end
public
- # If this controller was instantiated to process a component request,
- # +parent_controller+ points to the instantiator of this controller.
- attr_reader :parent_controller
-
- # Create a new controller instance.
- def initialize(parent_controller=nil) #:nodoc:
- @parent_controller = parent_controller
- end
-
# Extracts the action_name from the request parameters and performs that action.
def process(request, response, method = :perform_action, *arguments) #:nodoc:
initialize_template_class(response)
assign_shortcuts(request, response)
-
- my_flash = flash # calling flash creates @flash
- if my_parent = @parent_controller
- # only discard flash if this controller isn't a component request controller
- my_flash.discard
- end
-
initialize_current_url
- @action_name = params['action'] || 'index'
- @variables_added = nil
- @before_filter_chain_aborted = false
-
- log_processing if logger
+ action_name(:refresh)
+ forget_variables_added_to_assigns
+
+ log_processing
send(method, *arguments)
- @response
+
+ return response
ensure
- unless my_parent
- unless @before_filter_chain_aborted
- my_flash.sweep
- clear_persistent_model_associations
- end
- close_session
- end
+ process_cleanup
end
# Returns a URL that has been rewritten according to the options hash and the defined Routes.
@@ -472,6 +450,15 @@ def controller_name
self.class.controller_name
end
+ # Returns the name of the current action
+ def action_name(refresh = false)
+ if @action_name.nil? || refresh
+ @action_name = (params['action'] || 'index')
+ end
+
+ @action_name
+ end
+
def session_enabled?
request.session_options[:disabled] != false
end
@@ -668,9 +655,11 @@ def render(options = nil, deprecated_status = nil, &block) #:doc:
# of sending it as the response body to the browser.
def render_to_string(options = nil, &block) #:doc:
result = render(options, &block)
+
erase_render_results
- @variables_added = nil
- @template.instance_variable_set("@assigns_added", nil)
+ forget_variables_added_to_assigns
+ reset_variables_added_to_assigns
+
result
end
@@ -886,9 +875,11 @@ def initialize_current_url
end
def log_processing
- logger.info "\n\nProcessing #{controller_class_name}\##{action_name} (for #{request_origin}) [#{request.method.to_s.upcase}]"
- logger.info " Session ID: #{@session.session_id}" if @session and @session.respond_to?(:session_id)
- logger.info " Parameters: #{@params.inspect}"
+ if logger
+ logger.info "\n\nProcessing #{controller_class_name}\##{action_name} (for #{request_origin}) [#{request.method.to_s.upcase}]"
+ logger.info " Session ID: #{@session.session_id}" if @session and @session.respond_to?(:session_id)
+ logger.info " Parameters: #{@params.inspect}"
+ end
end
def perform_action
@@ -921,6 +912,14 @@ def add_variables_to_assigns
@variables_added = true
end
end
+
+ def forget_variables_added_to_assigns
+ @variables_added = nil
+ end
+
+ def reset_variables_added_to_assigns
+ @template.instance_variable_set("@assigns_added", nil)
+ end
def add_instance_variables_to_assigns
@@protected_variables_cache ||= protected_instance_variables.inject({}) { |h, k| h[k] = true; h }
@@ -993,5 +992,9 @@ def strip_out_controller!(path)
def template_path_includes_controller?(path)
path.to_s['/'] && self.class.controller_path.split('/')[-1] == path.split('/')[0]
end
+
+ def process_cleanup
+ close_session
+ end
end
end
View
4 actionpack/lib/action_controller/benchmarking.rb
@@ -4,9 +4,9 @@ module ActionController #:nodoc:
# The benchmarking module times the performance of actions and reports to the logger. If the Active Record
# package has been included, a separate timing section for database calls will be added as well.
module Benchmarking #:nodoc:
- def self.append_features(base)
- super
+ def self.included(base)
base.extend(ClassMethods)
+
base.class_eval do
alias_method :perform_action_without_benchmark, :perform_action
alias_method :perform_action, :perform_action_with_benchmark
View
4 actionpack/lib/action_controller/caching.rb
@@ -8,9 +8,9 @@ module ActionController #:nodoc:
#
# Note: To turn off all caching and sweeping, set Base.perform_caching = false.
module Caching
- def self.append_features(base) #:nodoc:
- super
+ def self.included(base) #:nodoc:
base.send(:include, Pages, Actions, Fragments, Sweeping)
+
base.class_eval do
@@perform_caching = true
cattr_accessor :perform_caching
View
65 actionpack/lib/action_controller/components.rb
@@ -36,17 +36,36 @@ module ActionController #:nodoc:
# So to repeat: Components are a special-purpose approach that can often be replaced with better use of partials and filters.
module Components
def self.included(base) #:nodoc:
+ base.send :include, InstanceMethods
base.extend(ClassMethods)
- base.send(:include, InstanceMethods)
base.helper do
def render_component(options)
@controller.send(:render_component_as_string, options)
end
end
+
+ base.class_eval do
+ alias_method :process_cleanup_without_components, :process_cleanup
+ alias_method :process_cleanup, :process_cleanup_with_components
+
+ alias_method :set_session_options_without_components, :set_session_options
+ alias_method :set_session_options, :set_session_options_with_components
+ end
+
+ # If this controller was instantiated to process a component request,
+ # +parent_controller+ points to the instantiator of this controller.
+ base.send :attr_accessor, :parent_controller
end
module ClassMethods
+ # Track parent controller to identify component requests
+ def process_with_components(request, response, parent_controller = nil) #:nodoc:
+ controller = new
+ controller.parent_controller = parent_controller
+ controller.process(request, response)
+ end
+
# Set the template root to be one directory behind the root dir of the controller. Examples:
# /code/weblog/components/admin/users_controller.rb with Admin::UsersController
# will use /code/weblog/components as template root
@@ -64,6 +83,12 @@ def uses_component_template_root
end
module InstanceMethods
+ # Extracts the action_name from the request parameters and performs that action.
+ def process_with_components(request, response, method = :perform_action, *arguments) #:nodoc:
+ flash.discard if component_request?
+ process_without_components(request, response, method, *arguments)
+ end
+
protected
# Renders the component specified as the response for the current method
def render_component(options) #:doc:
@@ -90,8 +115,8 @@ def component_response(options, reuse_response)
klass = component_class(options)
request = request_for_component(klass.controller_name, options)
response = reuse_response ? @response : @response.dup
-
- klass.process(request, response, self)
+
+ klass.process_with_components(request, response, self)
end
# determine the controller class for the component request
@@ -118,18 +143,30 @@ def request_for_component(controller_name, options)
)
request
- end
+ end
- def component_logging(options)
- if logger
- logger.info "Start rendering component (#{options.inspect}): "
- result = yield
- logger.info "\n\nEnd of component rendering"
- result
- else
- yield
- end
- end
+ def component_logging(options)
+ if logger
+ logger.info "Start rendering component (#{options.inspect}): "
+ result = yield
+ logger.info "\n\nEnd of component rendering"
+ result
+ else
+ yield
+ end
+ end
+
+ def component_request?
+ !@parent_controller.nil?
+ end
+
+ def set_session_options_with_components(request)
+ set_session_options_without_components(request) unless component_request?
+ end
+
+ def process_cleanup_with_components
+ process_cleanup_without_components unless component_request?
+ end
end
end
end
View
25 actionpack/lib/action_controller/filters.rb
@@ -335,23 +335,35 @@ def condition_hash(filters, *actions)
end
module InstanceMethods # :nodoc:
- def self.append_features(base)
- super
- base.class_eval {
+ def self.included(base)
+ base.class_eval do
alias_method :perform_action_without_filters, :perform_action
alias_method :perform_action, :perform_action_with_filters
- }
+
+ alias_method :process_without_filters, :process
+ alias_method :process, :process_with_filters
+
+ alias_method :process_cleanup_without_filters, :process_cleanup
+ alias_method :process_cleanup, :process_cleanup_with_filters
+ end
end
def perform_action_with_filters
before_action_result = before_action
+
unless before_action_result == false || performed?
perform_action_without_filters
after_action
end
+
@before_filter_chain_aborted = (before_action_result == false)
end
+ def process_with_filters(request, response, method = :perform_action, *arguments) #:nodoc:
+ @before_filter_chain_aborted = false
+ process_without_filters(request, response, method, *arguments)
+ end
+
# Calls all the defined before-filter filters, which are added by using "before_filter :method".
# If any of the filters return false, no more filters will be executed and the action is aborted.
def before_action #:doc:
@@ -368,6 +380,7 @@ def after_action #:doc:
def call_filters(filters)
filters.each do |filter|
next if action_exempted?(filter)
+
filter_result = case
when filter.is_a?(Symbol)
self.send(filter)
@@ -405,6 +418,10 @@ def action_exempted?(filter)
ea.include?(action_name)
end
end
+
+ def process_cleanup_with_filters
+ process_cleanup_without_filters unless @before_filter_chain_aborted
+ end
end
end
end
View
72 actionpack/lib/action_controller/flash.rb
@@ -24,7 +24,16 @@ module ActionController #:nodoc:
#
# See docs on the FlashHash class for more details about the flash.
module Flash
+ def self.included(base)
+ base.send :include, InstanceMethods
+ base.class_eval do
+ alias_method :process_cleanup_without_flash, :process_cleanup
+ alias_method :process_cleanup, :process_cleanup_with_flash
+ end
+ end
+
+
class FlashNow #:nodoc:
def initialize flash
@flash = flash
@@ -52,14 +61,14 @@ def []=(k, v) #:nodoc:
super
end
- def update h #:nodoc:
- h.keys.each{|k| discard k }
+ def update(h) #:nodoc:
+ h.keys.each{ |k| discard(k) }
super
end
- alias merge! update
+ alias :merge! :update
- def replace h #:nodoc:
+ def replace(h) #:nodoc:
@used = {}
super
end
@@ -124,31 +133,36 @@ def use(k=nil, v=true)
end
end
-
- protected
- # Access the contents of the flash. Use <tt>flash["notice"]</tt> to read a notice you put there or
- # <tt>flash["notice"] = "hello"</tt> to put a new one.
- # Note that if sessions are disabled only flash.now will work.
- def flash #:doc:
- @flash ||=
- if @parent_controller
- @parent_controller.flash
- elsif @session.is_a?(Hash)
- # @session is a Hash, if sessions are disabled
- # we don't put the flash in the session in this case
- FlashHash.new
- else
- # otherwise, @session is a CGI::Session or a TestSession
- # so make sure it gets retrieved from/saved to session storage after request processing
- @session["flash"] ||= FlashHash.new
- end
- end
-
- # deprecated. use <tt>flash.keep</tt> instead
- def keep_flash #:doc:
- warn 'keep_flash is deprecated; use flash.keep instead.'
- flash.keep
+ module InstanceMethods
+ def process_cleanup_with_flash
+ process_cleanup_without_flash
+ flash.sweep
end
+
+ protected
+ # Access the contents of the flash. Use <tt>flash["notice"]</tt> to read a notice you put there or
+ # <tt>flash["notice"] = "hello"</tt> to put a new one.
+ # Note that if sessions are disabled only flash.now will work.
+ def flash #:doc:
+ @flash ||=
+ if @parent_controller
+ @parent_controller.flash
+ elsif @session.is_a?(Hash)
+ # @session is a Hash, if sessions are disabled
+ # we don't put the flash in the session in this case
+ FlashHash.new
+ else
+ # otherwise, @session is a CGI::Session or a TestSession
+ # so make sure it gets retrieved from/saved to session storage after request processing
+ @session["flash"] ||= FlashHash.new
+ end
+ end
+ # deprecated. use <tt>flash.keep</tt> instead
+ def keep_flash #:doc:
+ warn 'keep_flash is deprecated; use flash.keep instead.'
+ flash.keep
+ end
+ end
end
-end
+end
View
35 actionpack/lib/action_controller/session_management.rb
@@ -6,11 +6,14 @@
module ActionController #:nodoc:
module SessionManagement #:nodoc:
- def self.append_features(base)
- super
+ def self.included(base)
base.extend(ClassMethods)
- base.send(:alias_method, :process_without_session_management_support, :process)
- base.send(:alias_method, :process, :process_with_session_management_support)
+
+ base.send :alias_method, :process_without_session_management_support, :process
+ base.send :alias_method, :process, :process_with_session_management_support
+
+ base.send :alias_method, :process_cleanup_without_session_management_support, :process_cleanup
+ base.send :alias_method, :process_cleanup, :process_cleanup_with_session_management_support
end
module ClassMethods
@@ -110,26 +113,30 @@ def session_options_for(request, action) #:nodoc:
end
def process_with_session_management_support(request, response, method = :perform_action, *arguments) #:nodoc:
- unless @parent_controller
- # only determine session options if this isn't a controller created for component request processing
- action = request.parameters["action"] || "index"
- request.session_options = self.class.session_options_for(request, action)
- end
+ set_session_options(request)
process_without_session_management_support(request, response, method, *arguments)
end
private
+ def set_session_options(request)
+ request.session_options = self.class.session_options_for(request, request.parameters["action"] || "index")
+ end
+
+ def process_cleanup_with_session_management_support
+ process_cleanup_without_session_management_support
+ clear_persistent_model_associations
+ end
+
# Clear cached associations in session data so they don't overflow
# the database field. Only applies to ActiveRecordStore since there
# is not a standard way to iterate over session data.
def clear_persistent_model_associations #:doc:
- if defined?(@session) and @session.instance_variables.include?('@data')
+ if defined?(@session) && @session.instance_variables.include?('@data')
session_data = @session.instance_variable_get('@data')
- if session_data and session_data.respond_to?(:each_value)
+
+ if session_data && session_data.respond_to?(:each_value)
session_data.each_value do |obj|
- if obj.respond_to?(:clear_association_cache)
- obj.clear_association_cache
- end
+ obj.clear_association_cache if obj.respond_to?(:clear_association_cache)
end
end
end
View
4 actionpack/test/controller/filters_test.rb
@@ -189,8 +189,8 @@ class AroundFilterController < PrependingController
class MixedFilterController < PrependingController
cattr_accessor :execution_log
- def initialize(parent_controller=nil)
- super(parent_controller)
+
+ def initialize
@@execution_log = ""
end
Please sign in to comment.
Something went wrong with that request. Please try again.