From 291a098c111ff419506094e14c0186389b0020ca Mon Sep 17 00:00:00 2001 From: Matthew Draper Date: Tue, 5 Apr 2016 06:11:28 +0930 Subject: Directly support stateful executor hooks Also, make sure to call the +complete+ hooks if +run+ fails. --- .../lib/active_support/execution_wrapper.rb | 45 ++++++++- activesupport/lib/active_support/reloader.rb | 8 +- activesupport/test/executor_test.rb | 107 +++++++++++++++++++++ 3 files changed, 156 insertions(+), 4 deletions(-) (limited to 'activesupport') diff --git a/activesupport/lib/active_support/execution_wrapper.rb b/activesupport/lib/active_support/execution_wrapper.rb index 2bd1c01d35..00c5745a25 100644 --- a/activesupport/lib/active_support/execution_wrapper.rb +++ b/activesupport/lib/active_support/execution_wrapper.rb @@ -19,6 +19,32 @@ module ActiveSupport set_callback(:complete, *args, &block) end + # Register an object to be invoked during both the +run+ and + # +complete+ steps. + # + # +hook.complete+ will be passed the value returned from +hook.run+, + # and will only be invoked if +run+ has previously been called. + # (Mostly, this means it won't be invoked if an exception occurs in + # a preceding +to_run+ block; all ordinary +to_complete+ blocks are + # invoked in that situation.) + def self.register_hook(hook, outer: false) + if outer + run_args = [prepend: true] + complete_args = [:after] + else + run_args = complete_args = [] + end + + to_run(*run_args) do + hook_state[hook] = hook.run + end + to_complete(*complete_args) do + if hook_state.key?(hook) + hook.complete hook_state[hook] + end + end + end + # Run this execution. # # Returns an instance, whose +complete!+ method *must* be invoked @@ -29,7 +55,15 @@ module ActiveSupport if active? Null else - new.tap(&:run!) + new.tap do |instance| + success = nil + begin + instance.run! + success = true + ensure + instance.complete! unless success + end + end end end @@ -37,11 +71,11 @@ module ActiveSupport def self.wrap return yield if active? - state = run! + instance = run! begin yield ensure - state.complete! + instance.complete! end end @@ -74,5 +108,10 @@ module ActiveSupport ensure self.class.active.delete Thread.current end + + private + def hook_state + @_hook_state ||= {} + end end end diff --git a/activesupport/lib/active_support/reloader.rb b/activesupport/lib/active_support/reloader.rb index 5d1f0e1e66..5623bdd349 100644 --- a/activesupport/lib/active_support/reloader.rb +++ b/activesupport/lib/active_support/reloader.rb @@ -43,7 +43,13 @@ module ActiveSupport # Initiate a manual reload def self.reload! executor.wrap do - new.tap(&:run!).complete! + new.tap do |instance| + begin + instance.run! + ensure + instance.complete! + end + end end prepare! end diff --git a/activesupport/test/executor_test.rb b/activesupport/test/executor_test.rb index 6db6db4fa8..d9b389461a 100644 --- a/activesupport/test/executor_test.rb +++ b/activesupport/test/executor_test.rb @@ -1,6 +1,9 @@ require 'abstract_unit' class ExecutorTest < ActiveSupport::TestCase + class DummyError < RuntimeError + end + def test_wrap_invokes_callbacks called = [] executor.to_run { called << :run } @@ -35,6 +38,20 @@ class ExecutorTest < ActiveSupport::TestCase assert_equal [:run, :body, :complete], called end + def test_exceptions_unwind + called = [] + executor.to_run { called << :run_1 } + executor.to_run { raise DummyError } + executor.to_run { called << :run_2 } + executor.to_complete { called << :complete } + + assert_raises(DummyError) do + executor.wrap { called << :body } + end + + assert_equal [:run_1, :complete], called + end + def test_avoids_double_wrapping called = [] executor.to_run { called << :run } @@ -51,6 +68,96 @@ class ExecutorTest < ActiveSupport::TestCase assert_equal [:run, :early, :body, :late, :complete], called end + def test_hooks_carry_state + supplied_state = :none + + hook = Class.new do + define_method(:run) do + :some_state + end + + define_method(:complete) do |state| + supplied_state = state + end + end.new + + executor.register_hook(hook) + + executor.wrap { } + + assert_equal :some_state, supplied_state + end + + def test_nil_state_is_sufficient + supplied_state = :none + + hook = Class.new do + define_method(:run) do + nil + end + + define_method(:complete) do |state| + supplied_state = state + end + end.new + + executor.register_hook(hook) + + executor.wrap { } + + assert_equal nil, supplied_state + end + + def test_exception_skips_uninvoked_hook + supplied_state = :none + + hook = Class.new do + define_method(:run) do + :some_state + end + + define_method(:complete) do |state| + supplied_state = state + end + end.new + + executor.to_run do + raise DummyError + end + executor.register_hook(hook) + + assert_raises(DummyError) do + executor.wrap { } + end + + assert_equal :none, supplied_state + end + + def test_exception_unwinds_invoked_hook + supplied_state = :none + + hook = Class.new do + define_method(:run) do + :some_state + end + + define_method(:complete) do |state| + supplied_state = state + end + end.new + + executor.register_hook(hook) + executor.to_run do + raise DummyError + end + + assert_raises(DummyError) do + executor.wrap { } + end + + assert_equal :some_state, supplied_state + end + def test_separate_classes_can_wrap other_executor = Class.new(ActiveSupport::Executor) -- cgit v1.2.3