Move Dispatcher setup to Railties and add instrumentation hook.

10 files changed, 163 insertions, 113 deletions

diff --git a/actionpack/lib/action_controller.rb b/actionpack/lib/action_controller.rb
index 42582c4525..8bc2cc79d2 100644
--- a/actionpack/lib/action_controller.rb
+++ b/actionpack/lib/action_controller.rb
@@ -39,7 +39,7 @@ module ActionController
     autoload :Verification
   end
 
-  autoload :Dispatcher,      'action_controller/dispatch/dispatcher'
+  autoload :Dispatcher,      'action_controller/deprecated/dispatcher'
   autoload :Integration,     'action_controller/deprecated/integration_test'
   autoload :IntegrationTest, 'action_controller/deprecated/integration_test'
   autoload :PerformanceTest, 'action_controller/deprecated/performance_test'
diff --git a/actionpack/lib/action_controller/deprecated/dispatcher.rb b/actionpack/lib/action_controller/deprecated/dispatcher.rb
new file mode 100644
index 0000000000..3da3c8ce7d
--- /dev/null
+++ b/actionpack/lib/action_controller/deprecated/dispatcher.rb
@@ -0,0 +1,31 @@
+module ActionController
+  class Dispatcher
+    cattr_accessor :prepare_each_request
+    self.prepare_each_request = false
+
+    class << self
+      def before_dispatch(*args, &block)
+        ActiveSupport::Deprecation.warn "ActionController::Dispatcher.before_dispatch is deprecated. " <<
+          "Please use ActionDispatch::Callbacks.before instead.", caller
+        ActionDispatch::Callbacks.before(*args, &block)
+      end
+
+      def after_dispatch(*args, &block)
+        ActiveSupport::Deprecation.warn "ActionController::Dispatcher.after_dispatch is deprecated. " <<
+          "Please use ActionDispatch::Callbacks.after instead.", caller
+        ActionDispatch::Callbacks.after(*args, &block)
+      end
+
+      def to_prepare(*args, &block)
+        ActiveSupport::Deprecation.warn "ActionController::Dispatcher.to_prepare is deprecated. " <<
+          "Please use ActionDispatch::Callbacks.to_prepare instead.", caller
+        ActionDispatch::Callbacks.after(*args, &block)
+      end
+
+      def new
+        ActiveSupport::Deprecation.warn "ActionController::Dispatcher.new is deprecated, use Rails.application instead."
+        Rails.application
+      end
+    end
+  end
+end
diff --git a/actionpack/lib/action_controller/dispatch/dispatcher.rb b/actionpack/lib/action_controller/dispatch/dispatcher.rb
deleted file mode 100644
index cf02757cf6..0000000000
--- a/actionpack/lib/action_controller/dispatch/dispatcher.rb
+++ /dev/null
@@ -1,52 +0,0 @@
-require 'active_support/core_ext/module/delegation'
-
-module ActionController
-  # Dispatches requests to the appropriate controller and takes care of
-  # reloading the app after each request when Dependencies.load? is true.
-  class Dispatcher
-    cattr_accessor :prepare_each_request
-    self.prepare_each_request = false
-
-    class << self
-      def define_dispatcher_callbacks(cache_classes)
-        unless cache_classes
-          # Run prepare callbacks before every request in development mode
-          self.prepare_each_request = true
-
-          ActionDispatch::Callbacks.after_dispatch do
-            # Cleanup the application before processing the current request.
-            ActiveRecord::Base.reset_subclasses if defined?(ActiveRecord)
-            ActiveSupport::Dependencies.clear
-            ActiveRecord::Base.clear_reloadable_connections! if defined?(ActiveRecord)
-          end
-
-          ActionView::Helpers::AssetTagHelper.cache_asset_timestamps = false
-        end
-
-        if defined?(ActiveRecord)
-          to_prepare(:activerecord_instantiate_observers) do
-            ActiveRecord::Base.instantiate_observers
-          end
-        end
-
-        if Base.logger && Base.logger.respond_to?(:flush)
-          after_dispatch do
-            Base.logger.flush
-          end
-        end
-
-        to_prepare do
-          I18n.reload!
-        end
-      end
-
-      delegate :to_prepare, :before_dispatch, :around_dispatch, :after_dispatch,
-        :to => ActionDispatch::Callbacks
-
-      def new
-        # DEPRECATE Rails application fallback
-        Rails.application
-      end
-    end
-  end
-end
diff --git a/actionpack/lib/action_controller/metal/instrumentation.rb b/actionpack/lib/action_controller/metal/instrumentation.rb
index e0c75c9e44..4549f6c451 100644
--- a/actionpack/lib/action_controller/metal/instrumentation.rb
+++ b/actionpack/lib/action_controller/metal/instrumentation.rb
@@ -66,7 +66,7 @@ module ActionController
 
     module ClassMethods
       # A hook which allows other frameworks to log what happened during
-      # controller process action. This method should return an awway
+      # controller process action. This method should return an array
       # with the messages to be added.
       # :api: plugin
       def log_process_action(controller) #:nodoc:
diff --git a/actionpack/lib/action_controller/railtie.rb b/actionpack/lib/action_controller/railtie.rb
index d4886ecefd..741101a210 100644
--- a/actionpack/lib/action_controller/railtie.rb
+++ b/actionpack/lib/action_controller/railtie.rb
@@ -69,12 +69,12 @@ module ActionController
       app.config.middleware.insert_before(:"ActionDispatch::ParamsParser", middleware)
     end
 
-    # # Prepare dispatcher callbacks and run 'prepare' callbacks
+    # Prepare dispatcher callbacks and run 'prepare' callbacks
     initializer "action_controller.prepare_dispatcher" do |app|
       # TODO: This used to say unless defined?(Dispatcher). Find out why and fix.
+      # Notice that at this point, ActionDispatch::Callbacks were already loaded.
       require 'rails/dispatcher'
-
-      Dispatcher.define_dispatcher_callbacks(app.config.cache_classes)
+      ActionController::Dispatcher.prepare_each_request = true unless app.config.cache_classes
 
       unless app.config.cache_classes
         # Setup dev mode route reloading
@@ -85,7 +85,7 @@ module ActionController
             app.reload_routes!
           end
         end
-        ActionDispatch::Callbacks.before_dispatch { |callbacks| reload_routes.call }
+        ActionDispatch::Callbacks.before { |callbacks| reload_routes.call }
       end
     end
 
diff --git a/actionpack/lib/action_dispatch/middleware/callbacks.rb b/actionpack/lib/action_dispatch/middleware/callbacks.rb
index 49bc20f11f..8098933e01 100644
--- a/actionpack/lib/action_dispatch/middleware/callbacks.rb
+++ b/actionpack/lib/action_dispatch/middleware/callbacks.rb
@@ -1,4 +1,10 @@
 module ActionDispatch
+  # Provide callbacks to be executed before and after the request dispatch.
+  #
+  # It also provides a to_prepare callback, which is performed in all requests
+  # in development by only once in production and notification callback for async
+  # operations.
+  #
   class Callbacks
     include ActiveSupport::Callbacks
 
@@ -29,12 +35,6 @@ module ActionDispatch
       set_callback(:call, :after, *args, &block)
     end
 
-    class << self
-      # DEPRECATED
-      alias_method :before_dispatch, :before
-      alias_method :after_dispatch, :after
-    end
-
     def initialize(app, prepare_each_request = false)
       @app, @prepare_each_request = app, prepare_each_request
       run_callbacks(:prepare)
@@ -43,7 +43,10 @@ module ActionDispatch
     def call(env)
       run_callbacks(:call) do
         run_callbacks(:prepare) if @prepare_each_request
-        @app.call(env)
+
+        ActiveSupport::Notifications.instrument "action_dispatch.callback" do
+          @app.call(env)
+        end
       end
     end
   end
diff --git a/actionpack/lib/action_view/helpers/form_helper.rb b/actionpack/lib/action_view/helpers/form_helper.rb
index cd805061be..20cee46d02 100644
--- a/actionpack/lib/action_view/helpers/form_helper.rb
+++ b/actionpack/lib/action_view/helpers/form_helper.rb
@@ -1080,7 +1080,7 @@ module ActionView
       # Add the submit button for the given form. When no value is given, it checks
       # if the object is a new resource or not to create the proper label:
       #
-      #   <% form_for @post do %>
+      #   <% form_for @post do |f| %>
       #     <%= f.submit %>
       #   <% end %>
       # 
diff --git a/actionpack/lib/action_view/railtie.rb b/actionpack/lib/action_view/railtie.rb
index e368aab825..968dc7b25e 100644
--- a/actionpack/lib/action_view/railtie.rb
+++ b/actionpack/lib/action_view/railtie.rb
@@ -7,5 +7,11 @@ module ActionView
 
     require "action_view/railties/subscriber"
     subscriber ActionView::Railties::Subscriber.new
+
+    initializer "action_view.cache_asset_timestamps" do |app|
+      unless app.config.cache_classes
+        ActionView::Helpers::AssetTagHelper.cache_asset_timestamps = false
+      end
+    end
   end
 end
\ No newline at end of file
diff --git a/actionpack/test/controller/dispatcher_test.rb b/actionpack/test/controller/dispatcher_test.rb
index 64f1ad7610..7e19bce3b7 100644
--- a/actionpack/test/controller/dispatcher_test.rb
+++ b/actionpack/test/controller/dispatcher_test.rb
@@ -1,73 +1,59 @@
 require 'abstract_unit'
 
-class DispatcherTest < Test::Unit::TestCase
-  Dispatcher = ActionController::Dispatcher
-
-  class Foo
-    cattr_accessor :a, :b
-  end
+# Ensure deprecated dispatcher works
+class DeprecatedDispatcherTest < ActiveSupport::TestCase
+  class DummyApp
+    def call(env)
+      [200, {}, 'response']
+    end
+  end 
 
   def setup
-    ENV['REQUEST_METHOD'] = 'GET'
-
-    # Clear callbacks as they are redefined by Dispatcher#define_dispatcher_callbacks
     ActionDispatch::Callbacks.reset_callbacks(:prepare)
     ActionDispatch::Callbacks.reset_callbacks(:call)
-
-    ActionController::Routing::Routes.stubs(:call).returns([200, {}, 'response'])
-    Dispatcher.stubs(:require_dependency)
   end
 
-  def teardown
-    ENV.delete 'REQUEST_METHOD'
-  end
+  def test_assert_deprecated_to_prepare
+    a = nil
+
+    assert_deprecated do
+      ActionController::Dispatcher.to_prepare { a = 1 }
+    end
 
-  def test_clears_dependencies_after_dispatch_if_in_loading_mode
-    ActiveSupport::Dependencies.expects(:clear).once
-    dispatch(false)
+    assert_nil a
+    dispatch
+    assert_equal 1, a
   end
 
-  def test_prepare_callbacks
-    a = b = c = nil
-    ActionDispatch::Callbacks.to_prepare { |*args| a = b = c = 1 }
-    ActionDispatch::Callbacks.to_prepare { |*args| b = c = 2 }
-    ActionDispatch::Callbacks.to_prepare { |*args| c = 3 }
+  def test_assert_deprecated_before_dispatch
+    a = nil
 
-    # Ensure to_prepare callbacks are not run when defined
-    assert_nil a || b || c
+    assert_deprecated do
+      ActionController::Dispatcher.before_dispatch { a = 1 }
+    end
 
-    # Run callbacks
+    assert_nil a
     dispatch
-
     assert_equal 1, a
-    assert_equal 2, b
-    assert_equal 3, c
-
-    # Make sure they are only run once
-    a = b = c = nil
-    dispatch
-    assert_nil a || b || c
   end
 
-  def test_to_prepare_with_identifier_replaces
-    ActionDispatch::Callbacks.to_prepare(:unique_id) { |*args| Foo.a, Foo.b = 1, 1 }
-    ActionDispatch::Callbacks.to_prepare(:unique_id) { |*args| Foo.a = 2 }
+  def test_assert_deprecated_after_dispatch
+    a = nil
+
+    assert_deprecated do
+      ActionController::Dispatcher.after_dispatch { a = 1 }
+    end
 
+    assert_nil a
     dispatch
-    assert_equal 2, Foo.a
-    assert_equal nil, Foo.b
+    assert_equal 1, a
   end
 
   private
-    def dispatch(cache_classes = true)
-      ActionController::Dispatcher.prepare_each_request = false
-      Dispatcher.define_dispatcher_callbacks(cache_classes)
 
-      @dispatcher ||= ActionDispatch::Callbacks.new(ActionController::Routing::Routes)
-      @dispatcher.call({'rack.input' => StringIO.new(''), 'action_dispatch.show_exceptions' => false})
+    def dispatch(cache_classes = true)
+      @dispatcher ||= ActionDispatch::Callbacks.new(DummyApp.new, !cache_classes)
+      @dispatcher.call({'rack.input' => StringIO.new('')})
     end
 
-    def assert_subclasses(howmany, klass, message = klass.subclasses.inspect)
-      assert_equal howmany, klass.subclasses.size, message
-    end
 end
diff --git a/actionpack/test/dispatch/callbacks_test.rb b/actionpack/test/dispatch/callbacks_test.rb
new file mode 100644
index 0000000000..8a2d56cfcc
--- /dev/null
+++ b/actionpack/test/dispatch/callbacks_test.rb
@@ -0,0 +1,76 @@
+require 'abstract_unit'
+
+class DispatcherTest < Test::Unit::TestCase
+  class Foo
+    cattr_accessor :a, :b
+  end
+
+  class DummyApp
+    def call(env)
+      [200, {}, 'response']
+    end
+  end 
+
+  def setup
+    Foo.a, Foo.b = 0, 0
+    ActionDispatch::Callbacks.reset_callbacks(:prepare)
+    ActionDispatch::Callbacks.reset_callbacks(:call)
+  end
+
+  def test_prepare_callbacks
+    a = b = c = nil
+    ActionDispatch::Callbacks.to_prepare { |*args| a = b = c = 1 }
+    ActionDispatch::Callbacks.to_prepare { |*args| b = c = 2 }
+    ActionDispatch::Callbacks.to_prepare { |*args| c = 3 }
+
+    # Ensure to_prepare callbacks are not run when defined
+    assert_nil a || b || c
+
+    # Run callbacks
+    dispatch
+
+    assert_equal 1, a
+    assert_equal 2, b
+    assert_equal 3, c
+
+    # Make sure they are only run once
+    a = b = c = nil
+    dispatch
+    assert_nil a || b || c
+  end
+
+  def test_to_prepare_with_identifier_replaces
+    ActionDispatch::Callbacks.to_prepare(:unique_id) { |*args| Foo.a, Foo.b = 1, 1 }
+    ActionDispatch::Callbacks.to_prepare(:unique_id) { |*args| Foo.a = 2 }
+
+    dispatch
+    assert_equal 2, Foo.a
+    assert_equal 0, Foo.b
+  end
+
+  def test_before_and_after_callbacks
+    ActionDispatch::Callbacks.before { |*args| Foo.a += 1; Foo.b += 1 }
+    ActionDispatch::Callbacks.after  { |*args| Foo.a += 1; Foo.b += 1 }
+
+    dispatch
+    assert_equal 2, Foo.a
+    assert_equal 2, Foo.b
+
+    dispatch
+    assert_equal 4, Foo.a
+    assert_equal 4, Foo.b
+  end
+
+  def test_should_send_an_instrumentation_callback_for_async_processing
+    ActiveSupport::Notifications.expects(:instrument).with("action_dispatch.callback")
+    dispatch
+  end
+
+  private
+
+    def dispatch(cache_classes = true)
+      @dispatcher ||= ActionDispatch::Callbacks.new(DummyApp.new, !cache_classes)
+      @dispatcher.call({'rack.input' => StringIO.new('')})
+    end
+
+end