Merge pull request #56 from pabloh/add_support_for_after_rollback_on_transactions

Add support for after rollback on transactions

Author: pabloh

CHANGELOG.md

@@ -1,6 +1,9 @@
 ## [1.1.0] - 2025-05-25
 ### Added
 - Added `:if` and `:unless` options for `:transaction` and `:after_commit` methods at `:sequel_models` plugin
+- Added `:after_rollback` method at `:sequel_models` plugin
+### Fixed
+- Fixed bug where setting a callback inside an `around` block could unexpectedly change the operation's result
 
 ## [1.0.0] - 2025-05-19
 ### Changed

lib/pathway.rb

@@ -101,11 +101,10 @@ module ClassMethods
 
         alias_method :result_at, :result_key=
 
-        def process(&bl)
-          dsl = self::DSL
+        def process(&steps)
           define_method(:call) do |input|
-            dsl.new(State.new(self, input:), self)
-               .run(&bl)
+            _dsl_for(input:)
+               .run(&steps)
                .then(&:result)
           end
         end
@@ -135,6 +134,10 @@ def error(type, message: nil, details: nil)
         def wrap_if_present(value, type: :not_found, message: nil, details: {})
           value.nil? ? error(type, message:, details:) : success(value)
         end
+
+        private
+
+        def _dsl_for(vals) = self.class::DSL.new(State.new(self, vals), self)
       end
 
       def self.apply(klass)
@@ -147,8 +150,8 @@ def initialize(state, operation)
           @result, @operation = wrap(state), operation
         end
 
-        def run(&bl)
-          instance_eval(&bl)
+        def run(&steps)
+          instance_eval(&steps)
           @result
         end
 
@@ -174,22 +177,21 @@ def map(callable,...)
           @result = @result.then { |state| bl.call(state,...) }
         end
 
-        def around(execution_strategy, &dsl_block)
+        def around(execution_strategy, &steps)
           @result.then do |state|
-            dsl_runner = ->(dsl = self) { @result = dsl.run(&dsl_block) }
+            steps_runner = ->(dsl = self) { dsl.run(&steps) }
 
-            _callable(execution_strategy).call(dsl_runner, state)
+            _callable(execution_strategy).call(steps_runner, state)
           end
         end
 
-        def if_true(cond, &dsl_block)
+        def if_true(cond, &steps)
           cond = _callable(cond)
-          around(->(dsl_runner, state) { dsl_runner.call if cond.call(state) }, &dsl_block)
+          around(->(runner, state) { runner.call if cond.call(state) }, &steps)
         end
 
-        def if_false(cond, &dsl_block)
-          cond = _callable(cond)
-          if_true(->(state) { !cond.call(state) }, &dsl_block)
+        def if_false(cond, &steps)
+          if_true(_callable(cond) >> :!.to_proc, &steps)
         end
 
         alias_method :sequence, :around

lib/pathway/plugins/sequel_models.rb

@@ -6,54 +6,44 @@ module Pathway
   module Plugins
     module SequelModels
       module DSLMethods
-        def transaction(step_name = nil, if: nil, unless: nil, &)
-          cond, dsl_bl = _transact_opts(step_name, *%i[if unless].map { binding.local_variable_get(_1) }, &)
-
-          if cond
-            if_true(cond) { transaction(&dsl_bl) }
-          else
-            around(->(runner, _) {
-              db.transaction(savepoint: true) do
-                raise Sequel::Rollback if runner.call.failure?
-              end
-            }, &dsl_bl)
+        def transaction(step_name = nil, if: nil, unless: nil, &steps)
+          _with_db_steps(steps, step_name, *_opts_if_unless(binding)) do |runner|
+            db.transaction(savepoint: true) do
+              raise Sequel::Rollback if runner.call.failure?
+            end
           end
         end
 
-        def after_commit(step_name = nil, if: nil, unless: nil, &)
-          cond, dsl_bl = _transact_opts(step_name, *%i[if unless].map { binding.local_variable_get(_1) }, &)
-
-          if cond
-            if_true(cond) { after_commit(&dsl_bl) }
-          else
-            around(->(runner, state) {
-              dsl_copy = self.class::DSL.new(State.new(self, state.to_h.dup), self)
+        def after_commit(step_name = nil, if: nil, unless: nil, &steps)
+          _with_db_steps(steps, step_name, *_opts_if_unless(binding)) do |runner, state|
+            dsl_copy = _dsl_for(state)
+            db.after_commit { runner.call(dsl_copy) }
+          end
+        end
 
-              db.after_commit do
-                runner.call(dsl_copy)
-              end
-            }, &dsl_bl)
+        def after_rollback(step_name = nil, if: nil, unless: nil, &steps)
+          _with_db_steps(steps, step_name, *_opts_if_unless(binding)) do |runner, state|
+            dsl_copy = _dsl_for(state)
+            db.after_rollback(savepoint: true) { runner.call(dsl_copy) }
           end
         end
 
         private
 
-        def _transact_opts(step_name, if_cond, unless_cond, &bl)
-          dsl = if !step_name.nil? == block_given?
-                  raise ArgumentError, 'must provide either a step or a block but not both'
-                else
-                  bl || proc { step step_name }
-                end
-
-          cond = if if_cond && unless_cond
-                   raise ArgumentError, 'options :if and :unless are mutually exclusive'
-                 elsif if_cond
-                   if_cond
-                 elsif unless_cond
-                   _callable(unless_cond) >> :!.to_proc
-                 end
-
-          return cond, dsl
+        def _opts_if_unless(bg) = %i[if unless].map { bg.local_variable_get(_1) }
+
+        def _with_db_steps(steps, step_name=nil, if_cond=nil, unless_cond=nil, &db_logic)
+          raise ArgumentError, 'options :if and :unless are mutually exclusive' if if_cond && unless_cond
+          raise ArgumentError, 'must provide either a step or a block but not both' if !!step_name == !!steps
+          steps ||= proc { step step_name }
+
+          if if_cond
+            if_true(if_cond) { _with_db_steps(steps, &db_logic) }
+          elsif unless_cond
+            if_false(unless_cond) { _with_db_steps(steps, &db_logic) }
+          else
+            around(db_logic, &steps)
+          end
         end
       end
 

spec/plugins/base_spec.rb

@@ -198,7 +198,7 @@ def notify(state)
           expect(result.value).to eq(:UPDATED)
         end
 
-        it "is skiped altogether on a failure state" do
+        it "is skipped altogether on a failure state" do
           allow(back_end).to receive(:call).and_return(Result.failure(:not_available))
           expect(cond).to_not receive(:call)
 
@@ -210,6 +210,46 @@ def notify(state)
 
           expect(result.value).to eq(:ZERO)
         end
+
+        context "when running callbacks after the operation has failled" do
+          let(:logger) { double}
+          let(:operation) { OperationWithCallbacks.new(logger: logger) }
+          let(:operation_class) do
+            Class.new(Operation) do
+              context :logger
+
+              process do
+                around(:cleanup_callback_context) do
+                  around(:put_steps_in_callback) do
+                    set  -> _ { :SHOULD_NOT_BE_SET }
+                    step -> _ { logger.log("calling back from callback") }
+                  end
+                  step :failing_step
+                end
+              end
+
+              def failing_step(_) = error(:im_a_failure!)
+
+              def put_steps_in_callback(runner, st)
+                st[:callbacks] << -> { runner.call(_dsl_for(st)) }
+              end
+
+              def cleanup_callback_context(runner, st)
+                st[:callbacks] = []
+                runner.call
+                st[:callbacks].each(&:call)
+              end
+            end
+          end
+
+          before { stub_const("OperationWithCallbacks", operation_class) }
+
+          it "does not alter the operation result when callback runs after failure" do
+            expect(logger).to receive(:log).with("calling back from callback")
+
+            expect(operation).to fail_on(valid_input).with_type(:im_a_failure!)
+          end
+        end
       end
 
       describe "#if_true" do