From 94361abc4600d4939f8e939c090ca11265b2eaf3 Mon Sep 17 00:00:00 2001 From: Taylor Fausak Date: Thu, 14 Aug 2014 19:37:16 -0500 Subject: [PATCH] Fix #205; remove transactions --- README.md | 4 +- lib/active_interaction.rb | 1 - lib/active_interaction/base.rb | 25 +--- lib/active_interaction/concerns/runnable.rb | 9 +- .../concerns/transactable.rb | 79 ---------- spec/active_interaction/base_spec.rb | 6 - .../concerns/runnable_spec.rb | 32 ----- .../concerns/transactable_spec.rb | 135 ------------------ 8 files changed, 4 insertions(+), 287 deletions(-) delete mode 100644 lib/active_interaction/concerns/transactable.rb delete mode 100644 spec/active_interaction/concerns/transactable_spec.rb diff --git a/README.md b/README.md index 89063c70..124489d7 100644 --- a/README.md +++ b/README.md @@ -94,9 +94,7 @@ end You may have noticed that ActiveInteraction::Base quacks like ActiveRecord::Base. It can use validations from your Rails application and check option validity with `valid?`. Any errors are added to -`errors` which works exactly like an ActiveRecord model. By default, -everything within the `execute` method is run in a transaction if -ActiveRecord is available. +`errors` which works exactly like an ActiveRecord model. ## How do I call an interaction? diff --git a/lib/active_interaction.rb b/lib/active_interaction.rb index b3e63ed2..009b6530 100644 --- a/lib/active_interaction.rb +++ b/lib/active_interaction.rb @@ -8,7 +8,6 @@ require 'active_interaction/concerns/active_modelable' require 'active_interaction/concerns/hashable' require 'active_interaction/concerns/missable' -require 'active_interaction/concerns/transactable' require 'active_interaction/concerns/runnable' require 'active_interaction/grouped_input' diff --git a/lib/active_interaction/base.rb b/lib/active_interaction/base.rb index 8fd8cc78..27f578d1 100644 --- a/lib/active_interaction/base.rb +++ b/lib/active_interaction/base.rb @@ -58,28 +58,6 @@ class << self # # @raise (see ActiveInteraction::Runnable::ClassMethods#run!) - # @!method transaction(enable, options = {}) - # Configure transactions by enabling or disabling them and setting - # their options. - # - # @example Disable transactions - # Class.new(ActiveInteraction::Base) do - # transaction false - # end - # - # @example Use different transaction options - # Class.new(ActiveInteraction::Base) do - # transaction true, isolation: :serializable - # end - # - # @param enable [Boolean] Should transactions be enabled? - # @param options [Hash] Options to pass to - # `ActiveRecord::Base.transaction`. - # - # @return [nil] - # - # @since 1.2.0 - # Get or set the description. # # @example @@ -220,8 +198,7 @@ def column_for_attribute(name) # # Runs the business logic associated with the interaction. This method is # only run when there are no validation errors. The return value is - # placed into {#result}. By default, this method is run in a transaction - # if ActiveRecord is available (see {.transaction}). + # placed into {#result}. # # @raise (see ActiveInteraction::Runnable#execute) diff --git a/lib/active_interaction/concerns/runnable.rb b/lib/active_interaction/concerns/runnable.rb index 8fb66336..ae3fb5ac 100644 --- a/lib/active_interaction/concerns/runnable.rb +++ b/lib/active_interaction/concerns/runnable.rb @@ -6,14 +6,12 @@ module ActiveInteraction # # @note Must be included after `ActiveModel::Validations`. # - # Runs code in transactions and only provides the result if there are no - # validation errors. + # Runs code and only provides the result if there are no validation errors. # # @private module Runnable extend ActiveSupport::Concern include ActiveModel::Validations - include ActiveInteraction::Transactable included do define_callbacks :execute @@ -82,15 +80,12 @@ def compose(other, *args) def run return unless valid? - self.result = transaction do + self.result = begin run_callbacks(:execute) { execute } rescue Interrupt => interrupt merge_errors_onto_base(interrupt.outcome.errors) - - raise ActiveRecord::Rollback if self.class.transaction? end - end end def merge_errors_onto_base(new_errors) diff --git a/lib/active_interaction/concerns/transactable.rb b/lib/active_interaction/concerns/transactable.rb deleted file mode 100644 index 5e5ad65a..00000000 --- a/lib/active_interaction/concerns/transactable.rb +++ /dev/null @@ -1,79 +0,0 @@ -# coding: utf-8 - -begin - require 'active_record' -rescue LoadError - module ActiveRecord # rubocop:disable Documentation - Rollback = Class.new(ActiveInteraction::Error) - - class Base # rubocop:disable Documentation - def self.transaction(*) - yield - rescue Rollback - end - end - end -end - -module ActiveInteraction - # @private - # - # Execute code in a transaction. If ActiveRecord isn't available, don't do - # anything special. - # - # @since 1.2.0 - module Transactable - extend ActiveSupport::Concern - - # @yield [] - def transaction - return unless block_given? - - if self.class.transaction? - ActiveRecord::Base.transaction(self.class.transaction_options) do - yield - end - else - yield - end - end - - module ClassMethods # rubocop:disable Documentation - # @param klass [Class] - def inherited(klass) - klass.transaction(transaction?, transaction_options.dup) - - super - end - - # @param enable [Boolean] - # @param options [Hash] - # - # @return [nil] - def transaction(enable, options = {}) - @_interaction_transaction_enabled = enable - @_interaction_transaction_options = options - - nil - end - - # @return [Boolean] - def transaction? - unless defined?(@_interaction_transaction_enabled) - @_interaction_transaction_enabled = true - end - - @_interaction_transaction_enabled - end - - # @return [Hash] - def transaction_options - unless defined?(@_interaction_transaction_options) - @_interaction_transaction_options = {} - end - - @_interaction_transaction_options - end - end - end -end diff --git a/spec/active_interaction/base_spec.rb b/spec/active_interaction/base_spec.rb index dc8604d3..917cb2d5 100644 --- a/spec/active_interaction/base_spec.rb +++ b/spec/active_interaction/base_spec.rb @@ -283,12 +283,6 @@ def execute it 'sets the result' do expect(result[:thing]).to eql thing end - - it 'calls #transaction' do - expect_any_instance_of(described_class).to receive(:transaction) - .once.with(no_args) - outcome - end end end diff --git a/spec/active_interaction/concerns/runnable_spec.rb b/spec/active_interaction/concerns/runnable_spec.rb index b64bc777..d46d3b73 100644 --- a/spec/active_interaction/concerns/runnable_spec.rb +++ b/spec/active_interaction/concerns/runnable_spec.rb @@ -180,38 +180,6 @@ end end - context 'with an execute where composition fails' do - before do - interaction = Class.new(TestInteraction) do - validate { errors.add(:base) } - end - - klass.send(:define_method, :execute) { compose(interaction) } - end - - it 'rolls back the transaction' do - instance = klass.new - - allow(instance).to receive(:raise) - instance.send(:run) - expect(instance).to have_received(:raise) - .with(ActiveRecord::Rollback) - end - - context 'without a transaction' do - before { klass.transaction(false) } - - it 'does not roll back' do - instance = klass.new - - allow(instance).to receive(:raise) - instance.send(:run) - expect(instance).to_not have_received(:raise) - .with(ActiveRecord::Rollback) - end - end - end - context 'with invalid post-execution state' do before do klass.class_exec do diff --git a/spec/active_interaction/concerns/transactable_spec.rb b/spec/active_interaction/concerns/transactable_spec.rb deleted file mode 100644 index e798b72a..00000000 --- a/spec/active_interaction/concerns/transactable_spec.rb +++ /dev/null @@ -1,135 +0,0 @@ -# coding: utf-8 - -require 'spec_helper' - -describe ActiveRecord::Base do - describe '.transaction' do - it 'raises an error' do - expect { described_class.transaction }.to raise_error LocalJumpError - end - - it 'silently rescues ActiveRecord::Rollback' do - expect do - described_class.transaction do - fail ActiveRecord::Rollback - end - end.to_not raise_error - end - - context 'with a block' do - it 'yields to the block' do - expect { |b| described_class.transaction(&b) }.to yield_with_no_args - end - - it 'accepts an argument' do - expect { described_class.transaction(nil) {} }.to_not raise_error - end - end - end -end - -describe ActiveInteraction::Transactable do - include_context 'concerns', ActiveInteraction::Transactable - - describe '.transaction' do - it 'returns nil' do - expect(klass.transaction(true)).to be_nil - end - - it 'accepts a flag parameter' do - expect { klass.transaction(true) }.to_not raise_error - end - - it 'also accepts an options parameter' do - expect { klass.transaction(true, {}) }.to_not raise_error - end - end - - describe '.transaction?' do - it 'defaults to true' do - expect(klass.transaction?).to be_truthy - end - - it 'returns the stored value' do - klass.transaction(false) - expect(klass.transaction?).to be_falsey - end - - context 'with a subclass' do - before { klass.transaction(false) } - - let(:subclass) { Class.new(klass) } - - it 'inherits from the superclass' do - expect(subclass.transaction?).to be_falsey - end - end - end - - describe '.transaction_options' do - let(:h) { { rand => rand } } - - it 'defaults to an empty hash' do - expect(klass.transaction_options).to eql({}) - end - - it 'returns the stored value' do - klass.transaction(klass.transaction?, h) - expect(klass.transaction_options).to eql h - end - - context 'with a subclass' do - before { klass.transaction(klass.transaction?, h) } - - let(:subclass) { Class.new(klass) } - - it 'inherits from the superclass' do - expect(subclass.transaction_options).to eql h - end - end - end - - describe '#transaction' do - let(:block) { -> { value } } - let(:result) { instance.transaction(&block) } - let(:value) { double } - - before do - allow(ActiveRecord::Base).to receive(:transaction).and_call_original - end - - it 'returns nil' do - expect(instance.transaction).to be_nil - end - - context 'with transactions disabled' do - before do - klass.transaction(false) - end - - it 'returns the value of the block' do - expect(result).to eql value - end - - it 'does not call ActiveRecord::Base.transaction' do - expect(ActiveRecord::Base).to_not have_received(:transaction) - end - end - - context 'with transactions enabled' do - before do - klass.transaction(true) - end - - it 'returns the value of the block' do - expect(result).to eql value - end - - it 'calls ActiveRecord::Base.transaction' do - result - expect(ActiveRecord::Base).to have_received(:transaction) - .once.with(klass.transaction_options, &block) - end - end - end -end