* 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

@@ -0,0 +1,2 @@
+rdoc
+pkg

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)

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)
+  def aasm_current_state_with_persistence=(state)
-    @aasm_current_state = 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

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}

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.send(:include, AASM::Persistence::ActiveRecordPersistence::WriteState) unless base.method_defined?(:aasm_write_state)
-        base.before_save do |record|
+        base.send(:include, AASM::Persistence::ActiveRecordPersistence::WriteStateWithoutPersistence) unless base.method_defined?(:aasm_write_state_without_persistence)        
-          record.send("#{record.class.aasm_column}=", record.aasm_current_state.to_s)
-        end
       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 <tt>aasm_read_state</tt>
+        #
+        #   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 <tt>state</tt> 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 <tt>state</tt> 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 <tt>aasm_current_state</tt>
+        #
+        # 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

spec/functional/conversation.rb

@@ -45,4 +45,5 @@ class Conversation
   def aasm_write_state(state)
     @persister.write_state(state)
   end
+
 end

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

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