diff --git a/CHANGELOG.md b/CHANGELOG.md index c2b8632..e8ccac4 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/lib/pathway.rb b/lib/pathway.rb index 7ff4def..56b31a4 100644 --- a/lib/pathway.rb +++ b/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 diff --git a/lib/pathway/plugins/sequel_models.rb b/lib/pathway/plugins/sequel_models.rb index 90b6a98..b0979cd 100644 --- a/lib/pathway/plugins/sequel_models.rb +++ b/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 diff --git a/spec/plugins/base_spec.rb b/spec/plugins/base_spec.rb index 7d14063..d139e0b 100644 --- a/spec/plugins/base_spec.rb +++ b/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 diff --git a/spec/plugins/sequel_models_spec.rb b/spec/plugins/sequel_models_spec.rb index fc349ae..f57c1ce 100644 --- a/spec/plugins/sequel_models_spec.rb +++ b/spec/plugins/sequel_models_spec.rb @@ -197,7 +197,8 @@ class InvalidUseOfCondOperation < MyOperation let(:operation) { InvalidUseOfCondOperation.new } it 'raises an error' do - expect { operation.call(params) }.to raise_error.with_message('options :if and :unless are mutually exclusive') + expect { operation.call(params) }.to raise_error. + with_message('options :if and :unless are mutually exclusive') end end @@ -213,7 +214,8 @@ class AmbivalentTransactOperation < MyOperation let(:operation) { AmbivalentTransactOperation.new } it 'raises an error' do - expect { operation.call(params) }.to raise_error.with_message('must provide either a step or a block but not both') + expect { operation.call(params) }.to raise_error + .with_message('must provide either a step or a block but not both') end end @@ -227,7 +229,8 @@ class EmptyTransacOperation < MyOperation let(:operation) { EmptyTransacOperation.new } it 'raises an error' do - expect { operation.call(params) }.to raise_error.with_message('must provide either a step or a block but not both') + expect { operation.call(params) }.to raise_error. + with_message('must provide either a step or a block but not both') end end end @@ -254,10 +257,10 @@ class EmptyTransacOperation < MyOperation expect(operation).to fail_on(params) end - context 'when the execution state is changed bellow the after_commit callback' do + context 'and the execution state is changed bellow the after_commit callback' do let(:operation) { ChainedOperation.new(mailer: mailer) } - it 'ignores any state changes that took place subsequent to the after_commit block' do + it 'ignores any state changes that took place following the after_commit block' do allow(MyModel).to receive(:first).with(params).and_return(model) expect(mailer).to receive(:send_emails).with(model) @@ -456,7 +459,8 @@ class InvalidConditionalAfterCommitOperation < MyOperation let(:operation) { InvalidConditionalAfterCommitOperation.new } it 'raises an error' do - expect { operation.call(params) }.to raise_error.with_message('options :if and :unless are mutually exclusive') + expect { operation.call(params) }.to raise_error + .with_message('options :if and :unless are mutually exclusive') end end end @@ -475,7 +479,8 @@ class AmbivalentAfterCommitOperation < MyOperation let(:operation) { AmbivalentAfterCommitOperation.new } it 'raises an error' do - expect { operation.call(params) }.to raise_error.with_message('must provide either a step or a block but not both') + expect { operation.call(params) }.to raise_error + .with_message('must provide either a step or a block but not both') end end @@ -491,7 +496,272 @@ class InvalidAfterCommitOperation < MyOperation let(:operation) { InvalidAfterCommitOperation.new } it 'raises an error' do - expect { operation.call(params) }.to raise_error.with_message('must provide either a step or a block but not both') + expect { operation.call(params) }.to raise_error. + with_message('must provide either a step or a block but not both') + end + end + end + + describe '#after_rollback' do + class LoggerOperation < MyOperation + context :logger + + process do + transaction do + after_rollback do + step :log_error + end + + step :fetch_model + end + end + + def log_error(_) + @logger.log("Ohhh noes!!!!") + end + end + + let(:logger) { double } + + context 'when providing a block' do + class RollbackWithBlockOperation < LoggerOperation + process do + transaction do + after_rollback do + step :log_error + end + + step :fetch_model + end + end + end + + let(:operation) { RollbackWithBlockOperation.new(logger: logger) } + before { expect(DB).to receive(:transaction).and_call_original } + + it 'calls after_rollback block when transaction fails' do + expect(MyModel).to receive(:first).with(params).and_return(nil) + expect(logger).to receive(:log) + + expect(operation).to fail_on(params) + end + + it 'does not call after_rollback block when transaction succeeds' do + expect(MyModel).to receive(:first).with(params).and_return(model) + expect(logger).to_not receive(:log) + + expect(operation).to succeed_on(params) + end + end + + context 'when providing a step' do + class RollbackStepOperation < LoggerOperation + process do + transaction do + after_rollback :log_error + step :fetch_model + end + end + end + + let(:operation) { RollbackStepOperation.new(logger: logger) } + before { expect(DB).to receive(:transaction).and_call_original } + + it 'calls after_rollback step when transaction fails' do + expect(MyModel).to receive(:first).with(params).and_return(nil) + expect(logger).to receive(:log) + + expect(operation).to fail_on(params) + end + + it 'does not call after_rollback step when transaction succeeds' do + expect(MyModel).to receive(:first).with(params).and_return(model) + expect(logger).to_not receive(:log) + + expect(operation).to succeed_on(params) + end + end + + context 'with conditional execution' do + context 'using :if with a block' do + class IfConditionalAfterRollbackOperation < LoggerOperation + context :should_run + + process do + transaction do + after_rollback(if: :should_run?) do + step :log_error + end + step :fetch_model + end + end + + private + def should_run?(state) = state[:should_run] + end + + let(:operation) { IfConditionalAfterRollbackOperation.new(logger: logger, should_run: should_run) } + let(:params) { { email: 'asd@fgh.net' } } + + before { allow(MyModel).to receive(:first).with(params).and_return(nil) } + + context 'when the condition is true' do + let(:should_run) { true } + + it 'executes the after_rollback block' do + expect(logger).to receive(:log) + + expect(operation).to fail_on(params) + end + end + + context 'when the condition is false' do + let(:should_run) { false } + + it 'skips the after_rollback block' do + expect(DB).to_not receive(:after_rollback) + expect(logger).to_not receive(:log) + + expect(operation).to fail_on(params) + end + end + end + + context 'using :unless with a block' do + class UnlessConditionalAfterRollbackOperation < LoggerOperation + context :should_skip + + process do + transaction do + after_rollback(unless: :should_skip?) do + step :log_error + end + step :fetch_model + end + end + + private + def should_skip?(state) = state[:should_skip] + end + + let(:operation) { UnlessConditionalAfterRollbackOperation.new(logger: logger, should_skip: should_skip) } + let(:params) { { email: 'asd@fgh.net' } } + + before { allow(MyModel).to receive(:first).with(params).and_return(nil) } + + context 'when the condition is false' do + let(:should_skip) { false } + + it 'executes the after_rollback block' do + expect(logger).to receive(:log) + + expect(operation).to fail_on(params) + end + end + + context 'when the condition is true' do + let(:should_skip) { true } + + it 'skips the after_rollback block' do + expect(DB).to_not receive(:after_rollback) + expect(logger).to_not receive(:log) + + expect(operation).to fail_on(params) + end + end + end + + context 'using :if with step name' do + class IfStepConditionalAfterRollbackOperation < LoggerOperation + context :should_run + + process do + transaction do + after_rollback :log_error, if: :should_run? + step :fetch_model + end + end + + private + def should_run?(state) = state[:should_run] + end + + before { allow(MyModel).to receive(:first).with(email: 'asd@fgh.net').and_return(nil) } + let(:operation) { IfStepConditionalAfterRollbackOperation.new(logger: logger, should_run: should_run) } + + context 'when the condition is true' do + let(:should_run) { true } + + it 'executes the after_rollback step' do + expect(logger).to receive(:log) + + expect(operation).to fail_on(params) + end + end + + context 'when the condition is false' do + let(:should_run) { false } + + it 'skips the after_rollback step' do + expect(DB).to_not receive(:after_rollback) + expect(logger).to_not receive(:log) + + expect(operation).to fail_on(params) + end + end + end + + context 'when both :if and :unless are provided' do + class InvalidConditionalAfterRollbackOperation < LoggerOperation + process do + transaction do + after_rollback :log_error, if: :is_good?, unless: :is_bad? + end + end + end + + let(:operation) { InvalidConditionalAfterRollbackOperation.new(logger: logger) } + + it 'raises an error' do + expect { operation.call(params) }.to raise_error + .with_message('options :if and :unless are mutually exclusive') + end + end + end + + context 'when providing a block and a step' do + class AmbivalentAfterRollbackOperation < MyOperation + process do + transaction do + after_rollback :perform_db_action do + step :perform_other_db_action + end + end + end + end + + let(:operation) { AmbivalentAfterRollbackOperation.new } + + it 'raises an error' do + expect { operation.call(params) }.to raise_error + .with_message('must provide either a step or a block but not both') + end + end + + context 'when not providing a block nor a step' do + class InvalidAfterRollbackOperation < MyOperation + process do + transaction do + after_rollback + end + end + end + + let(:operation) { InvalidAfterRollbackOperation.new } + + it 'raises an error' do + expect { operation.call(params) }.to raise_error + .with_message('must provide either a step or a block but not both') end end end