From 986aec5dbbdfb578945e706cbe6a54c4f06640e5 Mon Sep 17 00:00:00 2001 From: Pratik Naik Date: Thu, 17 Apr 2008 23:49:03 +0100 Subject: Refactor Dispatcher callbacks to remove unnecessary Dependencies checks in production environment. --- actionpack/lib/action_controller/dispatcher.rb | 73 ++++++++++++-------------- actionpack/test/controller/dispatcher_test.rb | 55 ++++++------------- 2 files changed, 50 insertions(+), 78 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/dispatcher.rb b/actionpack/lib/action_controller/dispatcher.rb index 99e1f74c0a..6e1e7a261f 100644 --- a/actionpack/lib/action_controller/dispatcher.rb +++ b/actionpack/lib/action_controller/dispatcher.rb @@ -5,6 +5,30 @@ module ActionController @@guard = Mutex.new class << self + def define_dispatcher_callbacks(cache_classes) + unless cache_classes + # Development mode callbacks + before_dispatch :reload_application + after_dispatch :cleanup_application + end + + # Common callbacks + to_prepare :load_application_controller do + begin + require_dependency 'application' unless defined?(::ApplicationController) + rescue LoadError => error + raise unless error.message =~ /application\.rb/ + end + end + + if defined?(ActiveRecord) + before_dispatch { ActiveRecord::Base.verify_active_connections! } + to_prepare(:activerecord_instantiate_observers) { ActiveRecord::Base.instantiate_observers } + end + + after_dispatch :flush_logger if defined?(RAILS_DEFAULT_LOGGER) && RAILS_DEFAULT_LOGGER.respond_to?(:flush) + end + # Backward-compatible class method takes CGI-specific args. Deprecated # in favor of Dispatcher.new(output, request, response).dispatch. def dispatch(cgi = nil, session_options = CgiRequest::DEFAULT_SESSION_OPTIONS, output = $stdout) @@ -69,23 +93,9 @@ module ActionController cattr_accessor :error_file_path self.error_file_path = Rails.public_path if defined?(Rails.public_path) - cattr_accessor :unprepared - self.unprepared = true - include ActiveSupport::Callbacks define_callbacks :prepare_dispatch, :before_dispatch, :after_dispatch - before_dispatch :reload_application - before_dispatch :prepare_application - after_dispatch :flush_logger - after_dispatch :cleanup_application - - if defined? ActiveRecord - to_prepare :activerecord_instantiate_observers do - ActiveRecord::Base.instantiate_observers - end - end - def initialize(output, request = nil, response = nil) @output, @request, @response = output, request, response end @@ -114,40 +124,23 @@ module ActionController end def reload_application - if Dependencies.load? - Routing::Routes.reload - self.unprepared = true - end - end + # Run prepare callbacks before every request in development mode + run_callbacks :prepare_dispatch - def prepare_application(force = false) - begin - require_dependency 'application' unless defined?(::ApplicationController) - rescue LoadError => error - raise unless error.message =~ /application\.rb/ - end - - ActiveRecord::Base.verify_active_connections! if defined?(ActiveRecord) - - if unprepared || force - run_callbacks :prepare_dispatch - ActionView::TemplateFinder.reload! unless ActionView::Base.cache_template_loading - self.unprepared = false - end + Routing::Routes.reload + ActionView::TemplateFinder.reload! unless ActionView::Base.cache_template_loading end # Cleanup the application by clearing out loaded classes so they can # be reloaded on the next request without restarting the server. - def cleanup_application(force = false) - if Dependencies.load? || force - ActiveRecord::Base.reset_subclasses if defined?(ActiveRecord) - Dependencies.clear - ActiveRecord::Base.clear_reloadable_connections! if defined?(ActiveRecord) - end + def cleanup_application + ActiveRecord::Base.reset_subclasses if defined?(ActiveRecord) + Dependencies.clear + ActiveRecord::Base.clear_reloadable_connections! if defined?(ActiveRecord) end def flush_logger - RAILS_DEFAULT_LOGGER.flush if defined?(RAILS_DEFAULT_LOGGER) && RAILS_DEFAULT_LOGGER.respond_to?(:flush) + RAILS_DEFAULT_LOGGER.flush end protected diff --git a/actionpack/test/controller/dispatcher_test.rb b/actionpack/test/controller/dispatcher_test.rb index c4f49f1e16..eea0813ed5 100644 --- a/actionpack/test/controller/dispatcher_test.rb +++ b/actionpack/test/controller/dispatcher_test.rb @@ -11,7 +11,13 @@ class DispatcherTest < Test::Unit::TestCase @output = StringIO.new ENV['REQUEST_METHOD'] = 'GET' + # Clear callbacks as they are redefined by Dispatcher#define_dispatcher_callbacks Dispatcher.instance_variable_set("@prepare_dispatch_callbacks", ActiveSupport::Callbacks::CallbackChain.new) + Dispatcher.instance_variable_set("@before_dispatch_callbacks", ActiveSupport::Callbacks::CallbackChain.new) + Dispatcher.instance_variable_set("@after_dispatch_callbacks", ActiveSupport::Callbacks::CallbackChain.new) + + Dispatcher.stubs(:require_dependency) + @dispatcher = Dispatcher.new(@output) end @@ -20,17 +26,13 @@ class DispatcherTest < Test::Unit::TestCase end def test_clears_dependencies_after_dispatch_if_in_loading_mode - Dependencies.stubs(:load?).returns(true) - ActionController::Routing::Routes.expects(:reload).once Dependencies.expects(:clear).once - dispatch + dispatch(@output, false) end def test_leaves_dependencies_after_dispatch_if_not_in_loading_mode - Dependencies.stubs(:load?).returns(false) - ActionController::Routing::Routes.expects(:reload).never Dependencies.expects(:clear).never @@ -51,40 +53,25 @@ class DispatcherTest < Test::Unit::TestCase assert_equal "Status: 400 Bad Request\r\nContent-Type: text/html\r\n\r\n

400 Bad Request

", @output.string end - def test_reload_application_sets_unprepared_if_loading_dependencies - Dependencies.stubs(:load?).returns(false) - ActionController::Routing::Routes.expects(:reload).never - @dispatcher.unprepared = false - @dispatcher.send!(:reload_application) - assert !@dispatcher.unprepared - - Dependencies.stubs(:load?).returns(true) - ActionController::Routing::Routes.expects(:reload).once - @dispatcher.send!(:reload_application) - assert @dispatcher.unprepared - end - - def test_prepare_application_runs_callbacks_if_unprepared + def test_prepare_callbacks a = b = c = nil Dispatcher.to_prepare { |*args| a = b = c = 1 } Dispatcher.to_prepare { |*args| b = c = 2 } Dispatcher.to_prepare { |*args| c = 3 } - # Skip the callbacks when already prepared. - @dispatcher.unprepared = false - @dispatcher.send! :prepare_application + # Ensure to_prepare callbacks are not run when defined assert_nil a || b || c - # Perform the callbacks when unprepared. - @dispatcher.unprepared = true - @dispatcher.send! :prepare_application + # Run callbacks + @dispatcher.send :run_callbacks, :prepare_dispatch + assert_equal 1, a assert_equal 2, b assert_equal 3, c - # But when not :load, make sure they are only run once + # Make sure they are only run once a = b = c = nil - @dispatcher.send! :prepare_application + @dispatcher.send :dispatch assert_nil a || b || c end @@ -93,28 +80,20 @@ class DispatcherTest < Test::Unit::TestCase Dispatcher.to_prepare(:unique_id) { |*args| a = b = 1 } Dispatcher.to_prepare(:unique_id) { |*args| a = 2 } - @dispatcher.unprepared = true - @dispatcher.send! :prepare_application + @dispatcher.send :run_callbacks, :prepare_dispatch assert_equal 2, a assert_equal nil, b end - def test_to_prepare_only_runs_once_if_not_loading_dependencies - Dependencies.stubs(:load?).returns(false) - called = 0 - Dispatcher.to_prepare(:unprepared_test) { |*args| called += 1 } - 2.times { dispatch } - assert_equal 1, called - end - private - def dispatch(output = @output) + def dispatch(output = @output, cache_classes = true) controller = mock controller.stubs(:process).returns(controller) controller.stubs(:out).with(output).returns('response') ActionController::Routing::Routes.stubs(:recognize).returns(controller) + Dispatcher.define_dispatcher_callbacks(cache_classes) Dispatcher.dispatch(nil, {}, output) end -- cgit v1.2.3