Moved guard clauses for state transition into State #commit methods

* Update state objects to handle setting the FKs directly.
* Make sure the child key changes if the parent key changes.

[#1284 state:resolved]

dm-core.gemspec

@@ -9,7 +9,7 @@ Gem::Specification.new do |s|
 
   s.required_rubygems_version = Gem::Requirement.new("> 1.3.1") if s.respond_to? :required_rubygems_version=
   s.authors = ["Dan Kubb"]
-  s.date = %q{2010-05-19}
+  s.date = %q{2010-05-21}
   s.description = %q{Faster, Better, Simpler.}
   s.email = %q{dan.kubb@gmail.com}
   s.extra_rdoc_files = [

lib/dm-core/resource.rb

@@ -1009,7 +1009,7 @@ def create_with_hooks
     #
     # @api private
     def _update
-      self.persisted_state = persisted_state.commit if valid_attributes?
+      self.persisted_state = persisted_state.commit
       clean?
     end
 
@@ -1168,14 +1168,6 @@ def set_default_value(subject)
       persisted_state.__send__(:set_default_value, subject)
     end
 
-    # @api private
-    def valid_attributes?
-      original_attributes.each_key do |property|
-        return false if property.kind_of?(Property) && !property.valid?(property.get!(self))
-      end
-      true
-    end
-
     # Execute all the queued up hooks for a given type and name
     #
     # @param [Symbol] type

lib/dm-core/resource/state.rb

@@ -64,6 +64,17 @@ def reset_original_attributes
         original_attributes.clear
       end
 
+      def set_child_keys
+        relationships.each do |relationship|
+          set_child_key(relationship)
+        end
+      end
+
+      def set_child_key(relationship)
+        return unless relationship.loaded?(resource) && relationship.respond_to?(:resource_for)
+        set(relationship, get(relationship))
+      end
+
     end # class State
   end # module Resource
 end # module DataMapper

lib/dm-core/resource/state/dirty.rb

@@ -17,11 +17,14 @@ def delete
 
         def commit
           remove_from_identity_map
+          set_child_keys
+          return self unless valid_attributes?
           update_resource
           reset_original_attributes
           reset_resource_key
-          add_to_identity_map
           Clean.new(resource)
+        ensure
+          add_to_identity_map
         end
 
         def rollback
@@ -76,8 +79,11 @@ def reset_resource_relationships
           end
         end
 
-        def reset_original_attributes
-          original_attributes.clear
+        def valid_attributes?
+          original_attributes.each_key do |property|
+            return false if property.kind_of?(Property) && !property.valid?(property.get!(resource))
+          end
+          true
         end
 
       end # class Dirty

lib/dm-core/resource/state/transient.rb

@@ -19,7 +19,9 @@ def delete
         end
 
         def commit
+          set_child_keys
           set_default_values
+          return self unless valid_attributes?
           create_resource
           set_repository
           reset_original_attributes
@@ -64,6 +66,13 @@ def set_repository
           resource.instance_variable_set(:@_repository, repository)
         end
 
+        def valid_attributes?
+          properties.all? do |property|
+            value = get(property)
+            property.serial? && value.nil? ? true : property.valid?(value)
+          end
+        end
+
       end # class Transient
     end # class State
   end # module Resource

spec/semipublic/resource/state/dirty_spec.rb

@@ -43,6 +43,12 @@ class ::Author
         should eql(DataMapper::Resource::State::Clean.new(@resource))
       end
 
+      it 'should set the child key if the parent key changes' do
+        original_id = @parent.id
+        @parent.update(:id => 42).should be_true
+        method(:subject).should change(@resource, :parent_id).from(original_id.to_s).to('42')
+      end
+
       it 'should update the resource' do
         subject
         @model.get!(*@resource.key).should == @resource

spec/semipublic/resource/state/transient_spec.rb

@@ -43,6 +43,12 @@ class ::Author
         method(:subject).should change(@resource, :id).from(nil)
       end
 
+      it 'should set the child key if the parent key changes' do
+        original_id = @parent.id
+        @parent.update(:id => 42).should be_true
+        method(:subject).should change(@resource, :parent_id).from(original_id).to(42)
+      end
+
       it 'should set default values' do
         method(:subject).should change { @model.properties[:active].get!(@resource) }.from(nil).to(true)
       end
@@ -57,9 +63,16 @@ class ::Author
       end
 
       it 'should reset original attributes' do
+        original_attributes = {
+          @model.properties[:name]      => nil,
+          @model.properties[:coding]    => nil,
+          @model.properties[:parent_id] => nil,
+          @model.relationships[:parent] => nil,
+        }
+
         expect do
           @resource.persisted_state = subject
-        end.should change { @resource.original_attributes.dup }.from(@model.properties[:name] => nil, @model.properties[:coding] => nil).to({})
+        end.should change { @resource.original_attributes.dup }.from(original_attributes).to({})
       end
 
       it 'should add the resource to the identity map' do
@@ -88,50 +101,52 @@ class ::Author
   describe '#get' do
     subject { @state.get(@key) }
 
-    describe 'with a set value' do
-      before do
-        @key = @model.properties[:coding]
-        @key.should be_loaded(@resource)
-      end
+    supported_by :all do
+      describe 'with a set value' do
+        before do
+          @key = @model.properties[:coding]
+          @key.should be_loaded(@resource)
+        end
 
-      it 'should return value' do
-        should be(false)
-      end
+        it 'should return value' do
+          should be(false)
+        end
 
-      it 'should be idempotent' do
-        should equal(subject)
+        it 'should be idempotent' do
+          should equal(subject)
+        end
       end
-    end
 
-    describe 'with an unset value and no default value' do
-      before do
-        @key = @model.properties[:age]
-        @key.should_not be_loaded(@resource)
-        @key.should_not be_default
-      end
+      describe 'with an unset value and no default value' do
+        before do
+          @key = @model.properties[:age]
+          @key.should_not be_loaded(@resource)
+          @key.should_not be_default
+        end
 
-      it 'should return nil' do
-        should be_nil
-      end
+        it 'should return nil' do
+          should be_nil
+        end
 
-      it 'should be idempotent' do
-        should equal(subject)
+        it 'should be idempotent' do
+          should equal(subject)
+        end
       end
-    end
 
-    describe 'with an unset value and a default value' do
-      before do
-        @key = @model.properties[:description]
-        @key.should_not be_loaded(@resource)
-        @key.should be_default
-      end
+      describe 'with an unset value and a default value' do
+        before do
+          @key = @model.properties[:description]
+          @key.should_not be_loaded(@resource)
+          @key.should be_default
+        end
 
-      it 'should return the name' do
-        should == 'Dan Kubb'
-      end
+        it 'should return the name' do
+          should == 'Dan Kubb'
+        end
 
-      it 'should be idempotent' do
-        should equal(subject)
+        it 'should be idempotent' do
+          should equal(subject)
+        end
       end
     end
   end