Remove deprecated callbacks from ActionDispatch middlewares

Author: rafaelfranca

actionpack/CHANGELOG.md

@@ -1,3 +1,11 @@
+*   Remove deprecated `.to_prepare`, `.to_cleanup`, `.prepare!` and `.cleanup!` from `ActionDispatch::Reloader`.
+
+    *Rafael Mendonça França*
+
+*   Remove deprecated `ActionDispatch::Callbacks.to_prepare` and `ActionDispatch::Callbacks.to_cleanup`.
+
+    *Rafael Mendonça França*
+
 *   Remove deprecated `ActionController::Metal.call`.
 
     *Rafael Mendonça França*

actionpack/lib/action_dispatch/middleware/callbacks.rb

@@ -1,4 +1,3 @@
-
 module ActionDispatch
   # Provides callbacks to be executed before and after dispatching the request.
   class Callbacks
@@ -7,17 +6,6 @@ class Callbacks
     define_callbacks :call
 
     class << self
-      def to_prepare(*args, &block)
-        ActiveSupport::Reloader.to_prepare(*args, &block)
-      end
-
-      def to_cleanup(*args, &block)
-        ActiveSupport::Reloader.to_complete(*args, &block)
-      end
-
-      deprecate to_prepare: "use ActiveSupport::Reloader.to_prepare instead",
-        to_cleanup: "use ActiveSupport::Reloader.to_complete instead"
-
       def before(*args, &block)
         set_callback(:call, :before, *args, &block)
       end

actionpack/lib/action_dispatch/middleware/reloader.rb

@@ -1,54 +1,10 @@
 module ActionDispatch
-  # ActionDispatch::Reloader provides prepare and cleanup callbacks,
-  # intended to assist with code reloading during development.
-  #
-  # Prepare callbacks are run before each request, and cleanup callbacks
-  # after each request. In this respect they are analogs of ActionDispatch::Callback's
-  # before and after callbacks. However, cleanup callbacks are not called until the
-  # request is fully complete -- that is, after #close has been called on
-  # the response body. This is important for streaming responses such as the
-  # following:
-  #
-  #     self.response_body = -> (response, output) do
-  #       # code here which refers to application models
-  #     end
-  #
-  # Cleanup callbacks will not be called until after the response_body lambda
-  # is evaluated, ensuring that it can refer to application models and other
-  # classes before they are unloaded.
+  # ActionDispatch::Reloader wraps the request with callbacks provided by ActiveSupport::Reloader
+  # callbacks, intended to assist with code reloading during development.
   #
   # By default, ActionDispatch::Reloader is included in the middleware stack
   # only in the development environment; specifically, when +config.cache_classes+
-  # is false. Callbacks may be registered even when it is not included in the
-  # middleware stack, but are executed only when <tt>ActionDispatch::Reloader.prepare!</tt>
-  # or <tt>ActionDispatch::Reloader.cleanup!</tt> are called manually.
-  #
+  # is false.
   class Reloader < Executor
-    def self.to_prepare(*args, &block)
-      ActiveSupport::Reloader.to_prepare(*args, &block)
-    end
-
-    def self.to_cleanup(*args, &block)
-      ActiveSupport::Reloader.to_complete(*args, &block)
-    end
-
-    def self.prepare!
-      default_reloader.prepare!
-    end
-
-    def self.cleanup!
-      default_reloader.reload!
-    end
-
-    class << self
-      attr_accessor :default_reloader # :nodoc:
-
-      deprecate to_prepare: "use ActiveSupport::Reloader.to_prepare instead",
-        to_cleanup: "use ActiveSupport::Reloader.to_complete instead",
-        prepare!: "use Rails.application.reloader.prepare! instead",
-        cleanup!: "use Rails.application.reloader.reload! instead of cleanup + prepare"
-    end
-
-    self.default_reloader = ActiveSupport::Reloader
   end
 end

actionpack/lib/action_dispatch/railtie.rb

@@ -39,8 +39,6 @@ class Railtie < Rails::Railtie # :nodoc:
       config.action_dispatch.always_write_cookie = Rails.env.development? if config.action_dispatch.always_write_cookie.nil?
       ActionDispatch::Cookies::CookieJar.always_write_cookie = config.action_dispatch.always_write_cookie
 
-      ActionDispatch::Reloader.default_reloader = app.reloader
-
       ActionDispatch.test_app = app
     end
   end

actionpack/test/dispatch/callbacks_test.rb

@@ -35,24 +35,6 @@ def test_before_and_after_callbacks
     assert_equal 6, Foo.b
   end
 
-  def test_to_prepare_and_cleanup_delegation
-    prepared = cleaned = false
-    assert_deprecated do
-      ActionDispatch::Callbacks.to_prepare { prepared = true }
-      ActionDispatch::Callbacks.to_prepare { cleaned = true }
-    end
-
-    assert_deprecated do
-      ActionDispatch::Reloader.prepare!
-    end
-    assert prepared
-
-    assert_deprecated do
-      ActionDispatch::Reloader.cleanup!
-    end
-    assert cleaned
-  end
-
   private
 
     def dispatch(&block)

actionpack/test/dispatch/reloader_test.rb

@@ -1,32 +1,11 @@
 require "abstract_unit"
 
 class ReloaderTest < ActiveSupport::TestCase
-  Reloader = ActionDispatch::Reloader
-
   teardown do
     ActiveSupport::Reloader.reset_callbacks :prepare
     ActiveSupport::Reloader.reset_callbacks :complete
   end
 
-  def test_prepare_callbacks
-    a = b = c = nil
-    assert_deprecated do
-      Reloader.to_prepare { |*args| a = b = c = 1 }
-      Reloader.to_prepare { |*args| b = c = 2 }
-      Reloader.to_prepare { |*args| c = 3 }
-    end
-
-    # Ensure to_prepare callbacks are not run when defined
-    assert_nil a || b || c
-
-    # Run callbacks
-    call_and_return_body
-
-    assert_equal 1, a
-    assert_equal 2, b
-    assert_equal 3, c
-  end
-
   class MyBody < Array
     def initialize(&block)
       @on_close = block
@@ -45,6 +24,23 @@ def close
     end
   end
 
+  def test_prepare_callbacks
+    a = b = c = nil
+    reloader.to_prepare { |*args| a = b = c = 1 }
+    reloader.to_prepare { |*args| b = c = 2 }
+    reloader.to_prepare { |*args| c = 3 }
+
+    # Ensure to_prepare callbacks are not run when defined
+    assert_nil a || b || c
+
+    # Run callbacks
+    call_and_return_body
+
+    assert_equal 1, a
+    assert_equal 2, b
+    assert_equal 3, c
+  end
+
   def test_returned_body_object_always_responds_to_close
     body = call_and_return_body
     assert_respond_to body, :close
@@ -62,15 +58,12 @@ def test_returned_body_object_always_responds_to_close_even_if_called_twice
 
   def test_condition_specifies_when_to_reload
     i, j = 0, 0, 0, 0
-    assert_deprecated do
-      Reloader.to_prepare { |*args| i += 1 }
-      Reloader.to_cleanup { |*args| j += 1 }
-    end
 
-    x = Class.new(ActiveSupport::Reloader)
-    x.check = lambda { i < 3 }
+    reloader = reloader(lambda { i < 3 })
+    reloader.to_prepare { |*args| i += 1 }
+    reloader.to_complete { |*args| j += 1 }
 
-    app = Reloader.new(lambda { |env| [200, {}, []] }, x)
+    app = middleware(lambda { |env| [200, {}, []] }, reloader)
     5.times do
       resp = app.call({})
       resp[2].close
@@ -115,24 +108,20 @@ def test_returned_body_object_responds_to_all_methods_supported_by_underlying_ob
     assert_respond_to body, :bar
   end
 
-  def test_cleanup_callbacks_are_called_when_body_is_closed
-    cleaned = false
-    assert_deprecated do
-      Reloader.to_cleanup { cleaned = true }
-    end
+  def test_complete_callbacks_are_called_when_body_is_closed
+    completed = false
+    reloader.to_complete { completed = true }
 
     body = call_and_return_body
-    assert !cleaned
+    assert !completed
 
     body.close
-    assert cleaned
+    assert completed
   end
 
   def test_prepare_callbacks_arent_called_when_body_is_closed
     prepared = false
-    assert_deprecated do
-      Reloader.to_prepare { prepared = true }
-    end
+    reloader.to_prepare { prepared = true }
 
     body = call_and_return_body
     prepared = false
@@ -141,45 +130,9 @@ def test_prepare_callbacks_arent_called_when_body_is_closed
     assert !prepared
   end
 
-  def test_manual_reloading
-    prepared = cleaned = false
-    assert_deprecated do
-      Reloader.to_prepare { prepared = true }
-      Reloader.to_cleanup { cleaned  = true }
-    end
-
-    assert_deprecated do
-      Reloader.prepare!
-    end
-    assert prepared
-    assert !cleaned
-
-    prepared = cleaned = false
-    assert_deprecated do
-      Reloader.cleanup!
-    end
-    assert prepared
-    assert cleaned
-  end
-
-  def test_prepend_prepare_callback
-    i = 10
-    assert_deprecated do
-      Reloader.to_prepare { i += 1 }
-      Reloader.to_prepare(prepend: true) { i = 0 }
-    end
-
-    assert_deprecated do
-      Reloader.prepare!
-    end
-    assert_equal 1, i
-  end
-
-  def test_cleanup_callbacks_are_called_on_exceptions
-    cleaned = false
-    assert_deprecated do
-      Reloader.to_cleanup { cleaned = true }
-    end
+  def test_complete_callbacks_are_called_on_exceptions
+    completed = false
+    reloader.to_complete { completed = true }
 
     begin
       call_and_return_body do
@@ -188,16 +141,26 @@ def test_cleanup_callbacks_are_called_on_exceptions
     rescue
     end
 
-    assert cleaned
+    assert completed
   end
 
   private
     def call_and_return_body(&block)
-      x = Class.new(ActiveSupport::Reloader)
-      x.check = lambda { true }
+      app = middleware(block || proc { [200, {}, "response"] })
+      _, _, body = app.call("rack.input" => StringIO.new(""))
+      body
+    end
+
+    def middleware(inner_app, reloader = reloader())
+      ActionDispatch::Reloader.new(inner_app, reloader)
+    end
+
+    def reloader(check = lambda { true })
+      @reloader ||= begin
+                      reloader = Class.new(ActiveSupport::Reloader)
+                      reloader.check = check
+                      reloader
+                    end
 
-      @response ||= "response"
-      @reloader ||= Reloader.new(block || proc { [200, {}, @response] }, x)
-      @reloader.call("rack.input" => StringIO.new(""))[2]
     end
 end

activesupport/test/reloader_test.rb

@@ -2,12 +2,15 @@
 
 class ReloaderTest < ActiveSupport::TestCase
   def test_prepare_callback
-    prepared = false
+    prepared = completed = false
     reloader.to_prepare { prepared = true }
+    reloader.to_complete { completed = true }
 
     assert !prepared
+    assert !completed
     reloader.prepare!
     assert prepared
+    assert !completed
 
     prepared = false
     reloader.wrap do
@@ -17,6 +20,15 @@ def test_prepare_callback
     assert !prepared
   end
 
+  def test_prepend_prepare_callback
+    i = 10
+    reloader.to_prepare { i += 1 }
+    reloader.to_prepare(prepend: true) { i = 0 }
+
+    reloader.prepare!
+    assert_equal 1, i
+  end
+
   def test_only_run_when_check_passes
     r = new_reloader { true }
     invoked = false