From 5739f9221908317019f98b323e8c4996f52efad0 Mon Sep 17 00:00:00 2001 From: Matthew Wilde Date: Mon, 16 May 2016 16:09:03 -0700 Subject: [PATCH] Implement whiny persistence so that `transition!` will raise by default, mirroring ActiveRecord's behavior. @PragTob's suggestion from https://github.com/aasm/aasm/issues/262. --- lib/aasm/base.rb | 3 ++ lib/aasm/configuration.rb | 5 ++- .../persistence/active_record_persistence.rb | 11 ++++++- lib/aasm/version.rb | 2 +- spec/database.rb | 16 +--------- spec/models/silent_persistor.rb | 31 +++++++++++++++++++ ...active_record_persistence_multiple_spec.rb | 24 ++++++++++++++ .../active_record_persistence_spec.rb | 24 ++++++++++++++ 8 files changed, 98 insertions(+), 18 deletions(-) create mode 100644 spec/models/silent_persistor.rb diff --git a/lib/aasm/base.rb b/lib/aasm/base.rb index 0c200f3..f22be89 100644 --- a/lib/aasm/base.rb +++ b/lib/aasm/base.rb @@ -23,6 +23,9 @@ module AASM # don't store any new state if the model is invalid (in ActiveRecord) configure :skip_validation_on_save, false + # raise if the model is invalid (in ActiveRecord) + configure :whiny_persistence, true + # use requires_new for nested transactions (in ActiveRecord) configure :requires_new_transaction, true diff --git a/lib/aasm/configuration.rb b/lib/aasm/configuration.rb index 9d6ad22..832cd08 100644 --- a/lib/aasm/configuration.rb +++ b/lib/aasm/configuration.rb @@ -9,7 +9,10 @@ module AASM # for all persistence layers: create named scopes for each state attr_accessor :create_scopes - # for ActiveRecord: don't store any new state if the model is invalid + # for ActiveRecord: when the model is invalid, true -> raise, false -> return false + attr_accessor :whiny_persistence + + # for ActiveRecord: store the new state even if the model is invalid and return true attr_accessor :skip_validation_on_save # for ActiveRecord: use requires_new for nested transactions? diff --git a/lib/aasm/persistence/active_record_persistence.rb b/lib/aasm/persistence/active_record_persistence.rb index a7be1cf..bdf0ecc 100644 --- a/lib/aasm/persistence/active_record_persistence.rb +++ b/lib/aasm/persistence/active_record_persistence.rb @@ -76,7 +76,12 @@ module AASM self.save end - success ? true : aasm_rollback(name, old_value) + unless success + aasm_rollback(name, old_value) + raise ActiveRecord::RecordInvalid.new(self) if aasm_whiny_persistence(name) + end + + success end # Writes state to the state column, but does not persist it to the database @@ -130,6 +135,10 @@ module AASM AASM::StateMachineStore.fetch(self.class, true).machine(state_machine_name).config.skip_validation_on_save end + def aasm_whiny_persistence(state_machine_name) + AASM::StateMachineStore.fetch(self.class, true).machine(state_machine_name).config.whiny_persistence + end + def aasm_write_attribute(state, name=:default) write_attribute(self.class.aasm(name).attribute_name, aasm_raw_attribute_value(state, name)) end diff --git a/lib/aasm/version.rb b/lib/aasm/version.rb index 494ce74..323c438 100644 --- a/lib/aasm/version.rb +++ b/lib/aasm/version.rb @@ -1,3 +1,3 @@ module AASM - VERSION = "4.10.1" + VERSION = "5.0.0" end diff --git a/spec/database.rb b/spec/database.rb index 1c542f5..b004487 100644 --- a/spec/database.rb +++ b/spec/database.rb @@ -17,7 +17,7 @@ ActiveRecord::Migration.suppress_messages do t.string "right" end - %w(validators multiple_validators).each do |table_name| + %w(validators multiple_validators workers invalid_persistors multiple_invalid_persistors silent_persistors multiple_silent_persistors).each do |table_name| ActiveRecord::Migration.create_table table_name, :force => true do |t| t.string "name" t.string "status" @@ -32,20 +32,6 @@ ActiveRecord::Migration.suppress_messages do end end - ActiveRecord::Migration.create_table "workers", :force => true do |t| - t.string "name" - t.string "status" - end - - ActiveRecord::Migration.create_table "invalid_persistors", :force => true do |t| - t.string "name" - t.string "status" - end - ActiveRecord::Migration.create_table "multiple_invalid_persistors", :force => true do |t| - t.string "name" - t.string "status" - end - ActiveRecord::Migration.create_table "fathers", :force => true do |t| t.string "aasm_state" t.string "type" diff --git a/spec/models/silent_persistor.rb b/spec/models/silent_persistor.rb new file mode 100644 index 0000000..944660d --- /dev/null +++ b/spec/models/silent_persistor.rb @@ -0,0 +1,31 @@ +require 'active_record' + +class SilentPersistor < ActiveRecord::Base + include AASM + aasm :column => :status, :whiny_persistence => false do + state :sleeping, :initial => true + state :running + event :run do + transitions :to => :running, :from => :sleeping + end + event :sleep do + transitions :to => :sleeping, :from => :running + end + end + validates_presence_of :name +end + +class MultipleSilentPersistor < ActiveRecord::Base + include AASM + aasm :left, :column => :status, :whiny_persistence => false do + state :sleeping, :initial => true + state :running + event :run do + transitions :to => :running, :from => :sleeping + end + event :sleep do + transitions :to => :sleeping, :from => :running + end + end + validates_presence_of :name +end diff --git a/spec/unit/persistence/active_record_persistence_multiple_spec.rb b/spec/unit/persistence/active_record_persistence_multiple_spec.rb index 54ad6b6..ce1a335 100644 --- a/spec/unit/persistence/active_record_persistence_multiple_spec.rb +++ b/spec/unit/persistence/active_record_persistence_multiple_spec.rb @@ -384,6 +384,30 @@ describe 'transitions with persistence' do expect(validator).to be_valid expect(validator).to be_sleeping + validator.name = nil + expect(validator).not_to be_valid + expect { validator.run! }.to raise_error(ActiveRecord::RecordInvalid) + expect(validator).to be_sleeping + + validator.reload + expect(validator).not_to be_running + expect(validator).to be_sleeping + + validator.name = 'another name' + expect(validator).to be_valid + expect(validator.run!).to be_truthy + expect(validator).to be_running + + validator.reload + expect(validator).to be_running + expect(validator).not_to be_sleeping + end + + it 'should not store states for invalid models silently if configured' do + validator = MultipleSilentPersistor.create(:name => 'name') + expect(validator).to be_valid + expect(validator).to be_sleeping + validator.name = nil expect(validator).not_to be_valid expect(validator.run!).to be_falsey diff --git a/spec/unit/persistence/active_record_persistence_spec.rb b/spec/unit/persistence/active_record_persistence_spec.rb index 26e002a..d673231 100644 --- a/spec/unit/persistence/active_record_persistence_spec.rb +++ b/spec/unit/persistence/active_record_persistence_spec.rb @@ -385,6 +385,30 @@ describe 'transitions with persistence' do expect(validator).to be_valid expect(validator).to be_sleeping + validator.name = nil + expect(validator).not_to be_valid + expect { validator.run! }.to raise_error(ActiveRecord::RecordInvalid) + expect(validator).to be_sleeping + + validator.reload + expect(validator).not_to be_running + expect(validator).to be_sleeping + + validator.name = 'another name' + expect(validator).to be_valid + expect(validator.run!).to be_truthy + expect(validator).to be_running + + validator.reload + expect(validator).to be_running + expect(validator).not_to be_sleeping + end + + it 'should not store states for invalid models silently if configured' do + validator = SilentPersistor.create(:name => 'name') + expect(validator).to be_valid + expect(validator).to be_sleeping + validator.name = nil expect(validator).not_to be_valid expect(validator.run!).to be_falsey