From cb6bd4f534c5908cbf78f43077298def3f061fbc Mon Sep 17 00:00:00 2001 From: Jeff Dean Date: Tue, 29 Apr 2008 01:27:56 -0400 Subject: [PATCH] * Specs and bug fixes for the ActiveRecordPersistence, keeping persistence columns in sync Allowing for nil values in states for active record New non-(!) methods that allow for firing events without persisting [Jeff Dean] --- .gitignore | 2 + CHANGELOG | 4 + lib/aasm.rb | 31 +++- lib/persistence.rb | 3 + lib/persistence/active_record_persistence.rb | 141 ++++++++++++++- spec/functional/conversation.rb | 1 + spec/unit/aasm_spec.rb | 29 +++- spec/unit/active_record_persistence_spec.rb | 170 +++++++++++++++++++ 8 files changed, 372 insertions(+), 9 deletions(-) create mode 100644 .gitignore create mode 100644 spec/unit/active_record_persistence_spec.rb diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..efa6628 --- /dev/null +++ b/.gitignore @@ -0,0 +1,2 @@ +rdoc +pkg \ No newline at end of file diff --git a/CHANGELOG b/CHANGELOG index 7c84192..540fcb8 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,3 +1,7 @@ +* Specs and bug fixes for the ActiveRecordPersistence, keeping persistence columns in sync + Allowing for nil values in states for active record + New non-(!) methods that allow for firing events without persisting [Jeff Dean] + * Added aasm_states_for_select that will return a select friendly collection of states. * Add some event callbacks, #aasm_event_fired(from, to), and #aasm_event_failed(event) diff --git a/lib/aasm.rb b/lib/aasm.rb index 85aae4b..5fa6e4f 100644 --- a/lib/aasm.rb +++ b/lib/aasm.rb @@ -39,6 +39,24 @@ module AASM end define_method("#{name.to_s}!") do + new_state = self.class.aasm_events[name].fire(self) + unless new_state.nil? + if self.respond_to?(:aasm_event_fired) + self.aasm_event_fired(self.aasm_current_state, new_state) + end + + self.aasm_current_state_with_persistence = new_state + true + else + if self.respond_to?(:aasm_event_failed) + self.aasm_event_failed(name) + end + + false + end + end + + define_method("#{name.to_s}") do new_state = self.class.aasm_events[name].fire(self) unless new_state.nil? if self.respond_to?(:aasm_event_fired) @@ -55,6 +73,7 @@ module AASM false end end + end def aasm_states @@ -92,10 +111,18 @@ module AASM end private - def aasm_current_state=(state) - @aasm_current_state = state + def aasm_current_state_with_persistence=(state) if self.respond_to?(:aasm_write_state) || self.private_methods.include?('aasm_write_state') aasm_write_state(state) end + self.aasm_current_state = state end + + def aasm_current_state=(state) + if self.respond_to?(:aasm_write_state_without_persistence) || self.private_methods.include?('aasm_write_state_without_persistence') + aasm_write_state_without_persistence(state) + end + @aasm_current_state = state + end + end diff --git a/lib/persistence.rb b/lib/persistence.rb index b75d808..f896964 100644 --- a/lib/persistence.rb +++ b/lib/persistence.rb @@ -1,5 +1,8 @@ module AASM module Persistence + + # Checks to see this class or any of it's superclasses inherit from + # ActiveRecord::Base and if so includes ActiveRecordPersistence def self.set_persistence(base) # Use a fancier auto-loading thingy, perhaps. When there are more persistence engines. hierarchy = base.ancestors.map {|klass| klass.to_s} diff --git a/lib/persistence/active_record_persistence.rb b/lib/persistence/active_record_persistence.rb index 1fcbf09..dbe5ef1 100644 --- a/lib/persistence/active_record_persistence.rb +++ b/lib/persistence/active_record_persistence.rb @@ -1,35 +1,164 @@ module AASM module Persistence module ActiveRecordPersistence + # This method: + # + # * extends the model with ClassMethods + # * includes InstanceMethods + # + # Unless the corresponding methods are already defined, it includes + # * ReadState + # * WriteState + # * WriteStateWithoutPersistence + # + # As a result, it doesn't matter when you define your methods - the following 2 are equivalent + # + # class Foo < ActiveRecord::Base + # def aasm_write_state(state) + # "bar" + # end + # include AASM + # end + # + # class Foo < ActiveRecord::Base + # include AASM + # def aasm_write_state(state) + # "bar" + # end + # end + # def self.included(base) base.extend AASM::Persistence::ActiveRecordPersistence::ClassMethods - base.send(:include, AASM::Persistence::ActiveRecordPersistence::WriteState) unless base.method_defined?(:aasm_write_state) + base.send(:include, AASM::Persistence::ActiveRecordPersistence::InstanceMethods) base.send(:include, AASM::Persistence::ActiveRecordPersistence::ReadState) unless base.method_defined?(:aasm_read_state) - - base.before_save do |record| - record.send("#{record.class.aasm_column}=", record.aasm_current_state.to_s) - end + base.send(:include, AASM::Persistence::ActiveRecordPersistence::WriteState) unless base.method_defined?(:aasm_write_state) + base.send(:include, AASM::Persistence::ActiveRecordPersistence::WriteStateWithoutPersistence) unless base.method_defined?(:aasm_write_state_without_persistence) end module ClassMethods + # Maps to the aasm_column in the database. Deafults to "aasm_state". You can write: + # + # create_table :foos do |t| + # t.string :name + # t.string :aasm_state + # end + # + # class Foo < ActiveRecord::Base + # include AASM + # end + # + # OR: + # + # create_table :foos do |t| + # t.string :name + # t.string :status + # end + # + # class Foo < ActiveRecord::Base + # include AASM + # aasm_column :status + # end + # + # This method is both a getter and a setter def aasm_column(column_name=nil) if column_name @aasm_column = column_name.to_sym else @aasm_column ||= :aasm_state end + @aasm_column + end + + end + + module InstanceMethods + + # Returns the current aasm_state of the object. Respects reload and + # any changes made to the aasm_state field directly + # + # Internally just calls aasm_read_state + # + # foo = Foo.find(1) + # foo.aasm_current_state # => :pending + # foo.aasm_state = "opened" + # foo.aasm_current_state # => :opened + # foo.close # => calls aasm_write_state_without_persistence + # foo.aasm_current_state # => :closed + # foo.reload + # foo.aasm_current_state # => :pending + # + def aasm_current_state + @current_state = aasm_read_state + end + + end + + module WriteStateWithoutPersistence + # Writes state to the state column, but does not persist it to the database + # + # foo = Foo.find(1) + # foo.aasm_current_state # => :opened + # foo.close + # foo.aasm_current_state # => :closed + # Foo.find(1).aasm_current_state # => :opened + # foo.save + # foo.aasm_current_state # => :closed + # Foo.find(1).aasm_current_state # => :closed + # + # NOTE: intended to be called from an event + def aasm_write_state_without_persistence(state) + write_attribute(self.class.aasm_column, state.to_s) end end module WriteState + # Writes state to the state column and persists it to the database + # using update_attribute (which bypasses validation) + # + # foo = Foo.find(1) + # foo.aasm_current_state # => :opened + # foo.close! + # foo.aasm_current_state # => :closed + # Foo.find(1).aasm_current_state # => :closed + # + # NOTE: intended to be called from an event def aasm_write_state(state) update_attribute(self.class.aasm_column, state.to_s) end end module ReadState + + # Returns the value of the aasm_column - called from aasm_current_state + # + # If it's a new record, and the aasm state column is blank it returns the initial state: + # + # class Foo < ActiveRecord::Base + # include AASM + # aasm_column :status + # aasm_state :opened + # aasm_state :closed + # end + # + # foo = Foo.new + # foo.current_state # => :opened + # foo.close + # foo.current_state # => :closed + # + # foo = Foo.find(1) + # foo.current_state # => :opened + # foo.aasm_state = nil + # foo.current_state # => nil + # + # NOTE: intended to be called from an event + # + # This allows for nil aasm states - be sure to add validation to your model def aasm_read_state - new_record? ? self.class.aasm_initial_state : send(self.class.aasm_column).to_sym + if new_record? + send(self.class.aasm_column).blank? ? self.class.aasm_initial_state : send(self.class.aasm_column).to_sym + else + send(self.class.aasm_column).nil? ? nil : send(self.class.aasm_column).to_sym + end end end end diff --git a/spec/functional/conversation.rb b/spec/functional/conversation.rb index 2ca72d7..da6e2bb 100644 --- a/spec/functional/conversation.rb +++ b/spec/functional/conversation.rb @@ -45,4 +45,5 @@ class Conversation def aasm_write_state(state) @persister.write_state(state) end + end diff --git a/spec/unit/aasm_spec.rb b/spec/unit/aasm_spec.rb index 9a0582e..baf3021 100644 --- a/spec/unit/aasm_spec.rb +++ b/spec/unit/aasm_spec.rb @@ -97,7 +97,7 @@ describe AASM, '- initial states' do end end -describe AASM, '- event firing' do +describe AASM, '- event firing with persistence' do it 'should fire the Event' do foo = Foo.new @@ -124,6 +124,33 @@ describe AASM, '- event firing' do end end +describe AASM, '- event firing without persistence' do + it 'should fire the Event' do + foo = Foo.new + + Foo.aasm_events[:close].should_receive(:fire).with(foo) + foo.close + end + + it 'should update the current state' do + foo = Foo.new + foo.close + + foo.aasm_current_state.should == :closed + end + + it 'should attempt to persist if aasm_write_state is defined' do + foo = Foo.new + + def foo.aasm_write_state + end + + foo.should_receive(:aasm_write_state_without_persistence) + + foo.close + end +end + describe AASM, '- persistence' do it 'should read the state if it has not been set and aasm_read_state is defined' do foo = Foo.new diff --git a/spec/unit/active_record_persistence_spec.rb b/spec/unit/active_record_persistence_spec.rb new file mode 100644 index 0000000..d3c61aa --- /dev/null +++ b/spec/unit/active_record_persistence_spec.rb @@ -0,0 +1,170 @@ +require File.join(File.dirname(__FILE__), '..', '..', 'lib', 'aasm') + +begin + require 'active_record' + + # A dummy class for mocking the activerecord connection class + class Connection + end + + class Foo < ActiveRecord::Base + include AASM + + # Fake this column for testing purposes + attr_accessor :aasm_state + + aasm_state :open + aasm_state :closed + + aasm_event :view do + transitions :to => :read, :from => [:needs_attention] + end + end + + class Fi < ActiveRecord::Base + def aasm_read_state + "fi" + end + include AASM + end + + class Fo < ActiveRecord::Base + def aasm_write_state(state) + "fo" + end + include AASM + end + + class Fum < ActiveRecord::Base + def aasm_write_state_without_persistence(state) + "fum" + end + include AASM + end + + describe "aasm model", :shared => true do + it "should include AASM::Persistence::ActiveRecordPersistence" do + @klass.included_modules.should be_include(AASM::Persistence::ActiveRecordPersistence) + end + it "should include AASM::Persistence::ActiveRecordPersistence::InstanceMethods" do + @klass.included_modules.should be_include(AASM::Persistence::ActiveRecordPersistence::InstanceMethods) + end + end + + describe Foo, "class methods" do + before(:each) do + @klass = Foo + end + it_should_behave_like "aasm model" + it "should include AASM::Persistence::ActiveRecordPersistence::ReadState" do + @klass.included_modules.should be_include(AASM::Persistence::ActiveRecordPersistence::ReadState) + end + it "should include AASM::Persistence::ActiveRecordPersistence::WriteState" do + @klass.included_modules.should be_include(AASM::Persistence::ActiveRecordPersistence::WriteState) + end + it "should include AASM::Persistence::ActiveRecordPersistence::WriteStateWithoutPersistence" do + @klass.included_modules.should be_include(AASM::Persistence::ActiveRecordPersistence::WriteStateWithoutPersistence) + end + end + + describe Fi, "class methods" do + before(:each) do + @klass = Fi + end + it_should_behave_like "aasm model" + it "should not include AASM::Persistence::ActiveRecordPersistence::ReadState" do + @klass.included_modules.should_not be_include(AASM::Persistence::ActiveRecordPersistence::ReadState) + end + it "should include AASM::Persistence::ActiveRecordPersistence::WriteState" do + @klass.included_modules.should be_include(AASM::Persistence::ActiveRecordPersistence::WriteState) + end + it "should include AASM::Persistence::ActiveRecordPersistence::WriteStateWithoutPersistence" do + @klass.included_modules.should be_include(AASM::Persistence::ActiveRecordPersistence::WriteStateWithoutPersistence) + end + end + + describe Fo, "class methods" do + before(:each) do + @klass = Fo + end + it_should_behave_like "aasm model" + it "should include AASM::Persistence::ActiveRecordPersistence::ReadState" do + @klass.included_modules.should be_include(AASM::Persistence::ActiveRecordPersistence::ReadState) + end + it "should not include AASM::Persistence::ActiveRecordPersistence::WriteState" do + @klass.included_modules.should_not be_include(AASM::Persistence::ActiveRecordPersistence::WriteState) + end + it "should include AASM::Persistence::ActiveRecordPersistence::WriteStateWithoutPersistence" do + @klass.included_modules.should be_include(AASM::Persistence::ActiveRecordPersistence::WriteStateWithoutPersistence) + end + end + + describe Fum, "class methods" do + before(:each) do + @klass = Fum + end + it_should_behave_like "aasm model" + it "should include AASM::Persistence::ActiveRecordPersistence::ReadState" do + @klass.included_modules.should be_include(AASM::Persistence::ActiveRecordPersistence::ReadState) + end + it "should include AASM::Persistence::ActiveRecordPersistence::WriteState" do + @klass.included_modules.should be_include(AASM::Persistence::ActiveRecordPersistence::WriteState) + end + it "should not include AASM::Persistence::ActiveRecordPersistence::WriteStateWithoutPersistence" do + @klass.included_modules.should_not be_include(AASM::Persistence::ActiveRecordPersistence::WriteStateWithoutPersistence) + end + end + + describe Foo, "instance methods" do + before(:each) do + connection = mock(Connection, :columns => []) + Foo.stub!(:connection).and_return(connection) + end + + it "should respond to aasm read state when not previously defined" do + Foo.new.should respond_to(:aasm_read_state) + end + + it "should respond to aasm write state when not previously defined" do + Foo.new.should respond_to(:aasm_write_state) + end + + it "should respond to aasm write state without persistence when not previously defined" do + Foo.new.should respond_to(:aasm_write_state_without_persistence) + end + + it "should return the initial state when new and the aasm field is nil" do + Foo.new.aasm_current_state.should == :open + end + + it "should return the aasm column when new and the aasm field is not nil" do + foo = Foo.new + foo.aasm_state = "closed" + foo.aasm_current_state.should == :closed + end + + it "should return the aasm column when not new and the aasm_column is not nil" do + foo = Foo.new + foo.stub!(:new_record?).and_return(false) + foo.aasm_state = "state" + foo.aasm_current_state.should == :state + end + + it "should allow a nil state" do + foo = Foo.new + foo.stub!(:new_record?).and_return(false) + foo.aasm_state = nil + foo.aasm_current_state.should be_nil + end + + end + + # TODO: figure out how to test ActiveRecord reload! without a database + +rescue LoadError => e + if e.message == "no such file to load -- active_record" + puts "You must install active record to run this spec. Install with sudo gem install activerecord" + else + raise + end +end