Refactor Dispatcher callbacks to remove unnecessary Dependencies checks in production environment.

5 files changed, 73 insertions, 89 deletions

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<html><body><h1>400 Bad Request</h1></body></html>", @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
 
diff --git a/railties/lib/console_app.rb b/railties/lib/console_app.rb
index c7673642ec..88e7962b43 100644
--- a/railties/lib/console_app.rb
+++ b/railties/lib/console_app.rb
@@ -24,7 +24,7 @@ end
 def reload!
   puts "Reloading..."
   dispatcher = ActionController::Dispatcher.new($stdout)
-  dispatcher.cleanup_application(true)
-  dispatcher.prepare_application(true)
+  dispatcher.cleanup_application
+  dispatcher.reload_application
   true
 end
diff --git a/railties/lib/initializer.rb b/railties/lib/initializer.rb
index 18af73fc89..b5bf9266f5 100644
--- a/railties/lib/initializer.rb
+++ b/railties/lib/initializer.rb
@@ -135,6 +135,9 @@ module Rails
 
       load_application_initializers
 
+      # Prepare dispatcher callbacks and run 'prepare' callbacks
+      prepare_dispatcher
+
       # the framework is now fully initialized
       after_initialize
 
@@ -442,6 +445,12 @@ module Rails
       end
     end
 
+    def prepare_dispatcher
+      require 'dispatcher' unless defined?(::Dispatcher)
+      Dispatcher.define_dispatcher_callbacks(configuration.cache_classes)
+      Dispatcher.new(RAILS_DEFAULT_LOGGER).send :run_callbacks, :prepare_dispatch
+    end
+
   end
 
   # The Configuration class holds all the parameters for the Initializer and
diff --git a/railties/test/console_app_test.rb b/railties/test/console_app_test.rb
index 7615d95498..6cfc907b80 100644
--- a/railties/test/console_app_test.rb
+++ b/railties/test/console_app_test.rb
@@ -13,17 +13,20 @@ require 'console_app'
 Test::Unit.run = false
 
 class ConsoleAppTest < Test::Unit::TestCase
-  def test_reload_should_fire_preparation_callbacks    
-    a = b = c = nil
+  uses_mocha 'console reload test' do
+    def test_reload_should_fire_preparation_callbacks
+      a = b = c = nil
 
-    Dispatcher.to_prepare { a = b = c = 1 }
-    Dispatcher.to_prepare { b = c = 2 }
-    Dispatcher.to_prepare { c = 3 } 
+      Dispatcher.to_prepare { a = b = c = 1 }
+      Dispatcher.to_prepare { b = c = 2 }
+      Dispatcher.to_prepare { c = 3 }
+      ActionController::Routing::Routes.expects(:reload)
 
-    reload!
+      reload!
 
-    assert_equal 1, a
-    assert_equal 2, b
-    assert_equal 3, c
+      assert_equal 1, a
+      assert_equal 2, b
+      assert_equal 3, c
+    end
   end
 end