From 8c8942ed4f2da52aa42ccd46560acb0b5fd37cb8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Thu, 14 Jan 2010 19:53:07 +0100 Subject: Move Dispatcher setup to Railties and add instrumentation hook. --- actionpack/lib/action_controller.rb | 2 +- .../lib/action_controller/deprecated/dispatcher.rb | 31 +++++++++ .../lib/action_controller/dispatch/dispatcher.rb | 52 -------------- .../lib/action_controller/metal/instrumentation.rb | 2 +- actionpack/lib/action_controller/railtie.rb | 8 +-- .../lib/action_dispatch/middleware/callbacks.rb | 17 +++-- actionpack/lib/action_view/helpers/form_helper.rb | 2 +- actionpack/lib/action_view/railtie.rb | 6 ++ actionpack/test/controller/dispatcher_test.rb | 80 +++++++++------------- actionpack/test/dispatch/callbacks_test.rb | 76 ++++++++++++++++++++ 10 files changed, 163 insertions(+), 113 deletions(-) create mode 100644 actionpack/lib/action_controller/deprecated/dispatcher.rb delete mode 100644 actionpack/lib/action_controller/dispatch/dispatcher.rb create mode 100644 actionpack/test/dispatch/callbacks_test.rb (limited to 'actionpack') 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 -- cgit v1.2.3