From 050c3964d8fe8d68c03fff593e3c09b5eae77a46 Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Sun, 12 Feb 2006 05:51:02 +0000 Subject: [PATCH] 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 --- actionpack/lib/action_controller.rb | 8 +- actionpack/lib/action_controller/base.rb | 75 ++++++++++--------- .../lib/action_controller/benchmarking.rb | 4 +- actionpack/lib/action_controller/caching.rb | 4 +- .../lib/action_controller/components.rb | 65 ++++++++++++---- actionpack/lib/action_controller/filters.rb | 25 ++++++- actionpack/lib/action_controller/flash.rb | 72 +++++++++++------- .../action_controller/session_management.rb | 35 +++++---- actionpack/test/controller/filters_test.rb | 4 +- 9 files changed, 185 insertions(+), 107 deletions(-) diff --git a/actionpack/lib/action_controller.rb b/actionpack/lib/action_controller.rb index d9762973d801..80772f758150 100755 --- a/actionpack/lib/action_controller.rb +++ b/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 diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb index 50fded00c555..d8138be3dbe3 100755 --- a/actionpack/lib/action_controller/base.rb +++ b/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 diff --git a/actionpack/lib/action_controller/benchmarking.rb b/actionpack/lib/action_controller/benchmarking.rb index eff4d97faec5..a30212c8bd4d 100644 --- a/actionpack/lib/action_controller/benchmarking.rb +++ b/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 diff --git a/actionpack/lib/action_controller/caching.rb b/actionpack/lib/action_controller/caching.rb index dda54459ed06..e6849596db7d 100644 --- a/actionpack/lib/action_controller/caching.rb +++ b/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 diff --git a/actionpack/lib/action_controller/components.rb b/actionpack/lib/action_controller/components.rb index 89a2c03aeae2..28480e47b6a5 100644 --- a/actionpack/lib/action_controller/components.rb +++ b/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 diff --git a/actionpack/lib/action_controller/filters.rb b/actionpack/lib/action_controller/filters.rb index 820eca1754b7..008486751d0d 100644 --- a/actionpack/lib/action_controller/filters.rb +++ b/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 diff --git a/actionpack/lib/action_controller/flash.rb b/actionpack/lib/action_controller/flash.rb index 6a41aa97a6dd..383263fe920d 100644 --- a/actionpack/lib/action_controller/flash.rb +++ b/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 flash["notice"] to read a notice you put there or - # flash["notice"] = "hello" 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 flash.keep 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 flash["notice"] to read a notice you put there or + # flash["notice"] = "hello" 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 flash.keep instead + def keep_flash #:doc: + warn 'keep_flash is deprecated; use flash.keep instead.' + flash.keep + end + end end -end +end \ No newline at end of file diff --git a/actionpack/lib/action_controller/session_management.rb b/actionpack/lib/action_controller/session_management.rb index 950f48abf488..408ef2790e56 100644 --- a/actionpack/lib/action_controller/session_management.rb +++ b/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 diff --git a/actionpack/test/controller/filters_test.rb b/actionpack/test/controller/filters_test.rb index 3f4f3b07bef3..59bff7653675 100644 --- a/actionpack/test/controller/filters_test.rb +++ b/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