Support double-yield inside an around callback

It's questionable whether this is a good thing -- it forces any later/
inner callback to handle multiple invocations, along with the actual
wrapped action. But it worked prior to 871ca21f6a1d65c0ec78cb5a9641411e2210460b,
so we shouldn't break it unintentionally.

2 files changed, 77 insertions, 18 deletions

diff --git a/activesupport/lib/active_support/callbacks.rb b/activesupport/lib/active_support/callbacks.rb
index af8ddb176f..e6c79f2a38 100644
--- a/activesupport/lib/active_support/callbacks.rb
+++ b/activesupport/lib/active_support/callbacks.rb
@@ -109,16 +109,22 @@ module ActiveSupport
         invoke_sequence = Proc.new do
           skipped = nil
           while true
-            current, next_sequence = next_sequence, next_sequence.nested
+            current = next_sequence
             current.invoke_before(env)
             if current.final?
               env.value = !env.halted && (!block_given? || yield)
             elsif current.skip?(env)
               (skipped ||= []) << current
+              next_sequence = next_sequence.nested
               next
             else
-              target, block, method, *arguments = current.expand_call_template(env, invoke_sequence)
-              target.send(method, *arguments, &block)
+              next_sequence = next_sequence.nested
+              begin
+                target, block, method, *arguments = current.expand_call_template(env, invoke_sequence)
+                target.send(method, *arguments, &block)
+              ensure
+                next_sequence = current
+              end
             end
             current.invoke_after(env)
             skipped.pop.invoke_after(env) while skipped && skipped.first
diff --git a/activesupport/test/callbacks_test.rb b/activesupport/test/callbacks_test.rb
index aadc40ab84..22f66978f8 100644
--- a/activesupport/test/callbacks_test.rb
+++ b/activesupport/test/callbacks_test.rb
@@ -224,10 +224,51 @@ module CallbacksTest
     define_callbacks :save
   end
 
-  class AroundPerson < MySuper
+  class MySlate < MySuper
     attr_reader :history
     attr_accessor :save_fails
 
+    def initialize
+      @history = []
+    end
+
+    def save
+      run_callbacks :save do
+        raise "inside save" if save_fails
+        @history << "running"
+      end
+    end
+
+    def no; false; end
+    def yes; true; end
+
+    def method_missing(sym, *)
+      case sym
+      when /^log_(.*)/
+        @history << $1
+        nil
+      when /^wrap_(.*)/
+        @history << "wrap_#$1"
+        yield
+        @history << "unwrap_#$1"
+        nil
+      when /^double_(.*)/
+        @history << "first_#$1"
+        yield
+        @history << "second_#$1"
+        yield
+        @history << "third_#$1"
+      else
+        super
+      end
+    end
+
+    def respond_to_missing?(sym)
+      sym =~ /^(log|wrap)_/ || super
+    end
+  end
+
+  class AroundPerson < MySlate
     set_callback :save, :before, :nope,           if: :no
     set_callback :save, :before, :nope,           unless: :yes
     set_callback :save, :after,  :tweedle
@@ -242,9 +283,6 @@ module CallbacksTest
     set_callback :save, :around, :w0tno,          if: :no
     set_callback :save, :around, :tweedle_deedle
 
-    def no; false; end
-    def yes; true; end
-
     def nope
       @history << "boom"
     end
@@ -283,17 +321,6 @@ module CallbacksTest
       yield
       @history << "tweedle deedle post"
     end
-
-    def initialize
-      @history = []
-    end
-
-    def save
-      run_callbacks :save do
-        raise "inside save" if save_fails
-        @history << "running"
-      end
-    end
   end
 
   class AroundPersonResult < MySuper
@@ -408,6 +435,32 @@ module CallbacksTest
     end
   end
 
+  class DoubleYieldTest < ActiveSupport::TestCase
+    class DoubleYieldModel < MySlate
+      set_callback :save, :around, :wrap_outer
+      set_callback :save, :around, :double_trouble
+      set_callback :save, :around, :wrap_inner
+    end
+
+    def test_double_save
+      double = DoubleYieldModel.new
+      double.save
+      assert_equal [
+        "wrap_outer",
+        "first_trouble",
+        "wrap_inner",
+        "running",
+        "unwrap_inner",
+        "second_trouble",
+        "wrap_inner",
+        "running",
+        "unwrap_inner",
+        "third_trouble",
+        "unwrap_outer",
+      ], double.history
+    end
+  end
+
   class CallStackTest < ActiveSupport::TestCase
     def test_tidy_call_stack
       around = AroundPerson.new