fixing bug 1372: save method not catching validations on accessors

fix_hooks_in_before_on_non_dirty_resources.patch

From feeb8da75a0f6b2dd6d07c53fa4235fef21f050f Mon Sep 17 00:00:00 2001
From: Kabari Hendrick <kabari@gmail.com>
Date: Sat, 18 Dec 2010 10:41:19 -0600
Subject: [PATCH] fixing bug 1372: save method not catching validations on accessors


Signed-off-by: Kabari Hendrick <kabari@gmail.com>
---
 lib/dm-core/resource.rb                  |    5 ++---
 lib/dm-core/spec/shared/resource_spec.rb |    5 +++--
 spec/public/model/hook_spec.rb           |   30 +++++++++++++++++++++++++-----
 3 files changed, 30 insertions(+), 10 deletions(-)

diff --git a/lib/dm-core/resource.rb b/lib/dm-core/resource.rb
index 52f549a..85dc2f5 100644
--- a/lib/dm-core/resource.rb
+++ b/lib/dm-core/resource.rb
@@ -996,6 +996,7 @@ module DataMapper
       catch :halt do
         before_save_hook
         before_update_hook
+        return saved? unless dirty_self? # don't save record or run after callbacks unless it has changed
         _persist
         after_update_hook
         after_save_hook
@@ -1026,13 +1027,11 @@ module DataMapper
     #
     # @api semipublic
     def save_self(execute_hooks = true)
-      # short-circuit if the resource is not dirty
-      return saved? unless dirty_self?
 
       if execute_hooks
         new? ? create_with_hooks : update_with_hooks
       else
-        _persist
+        _persist if dirty_self?
       end
       clean?
     end
diff --git a/lib/dm-core/spec/shared/resource_spec.rb b/lib/dm-core/spec/shared/resource_spec.rb
index f810aca..de93847 100644
--- a/lib/dm-core/spec/shared/resource_spec.rb
+++ b/lib/dm-core/spec/shared/resource_spec.rb
@@ -726,7 +726,7 @@ share_examples_for 'A public Resource' do
         end
 
         it 'should call save hook expected number of times' do
-          @user.save_hook_call_count.should be_nil
+          @user.save_hook_call_count.should == (method == :save ? 1 : nil)
         end
       end
 
@@ -742,7 +742,7 @@ share_examples_for 'A public Resource' do
         end
 
         it 'should call save hook expected number of times' do
-          @user.save_hook_call_count.should be_nil
+          @user.save_hook_call_count.should == (method == :save ? 1 : nil)
         end
       end
 
@@ -763,6 +763,7 @@ share_examples_for 'A public Resource' do
         end
 
         it 'should call save hook expected number of times' do
+          
           @user.save_hook_call_count.should == (method == :save ? 1 : nil)
         end
       end
diff --git a/spec/public/model/hook_spec.rb b/spec/public/model/hook_spec.rb
index 8949ff5..7eefc01 100644
--- a/spec/public/model/hook_spec.rb
+++ b/spec/public/model/hook_spec.rb
@@ -7,9 +7,16 @@ describe DataMapper::Model::Hook do
 
       property :id, Serial
       property :value, Integer, :required => true, :default => 1
-
+      
+      attr_accessor :hook_count
+      
       def an_instance_method
       end
+      
+      def update_hook_count
+        @hook_count ||= 0
+        @hook_count += 1
+      end
     end
 
     class ::ModelHookSpecsSubclass < ModelHookSpecs; end
@@ -40,7 +47,7 @@ describe DataMapper::Model::Hook do
         before do
           @hooks = hooks = []
           ModelHookSpecs.before(:save) { hooks << :before_save }
-
+          
           @resource.save
         end
 
@@ -70,14 +77,20 @@ describe DataMapper::Model::Hook do
         before do
           @hooks = hooks = []
           ModelHookSpecs.before(:update) { hooks << :before_update }
-
+          
           @resource.save
-          @resource.update(:value => 2)
         end
 
         it 'should execute before update hook' do
+          @resource.update(:value => 2)
           @hooks.should == [ :before_update ]
         end
+        
+        it "should execute hook when no property is changed" do
+          ModelHookSpecs.before(:update, :update_hook_count)
+          @resource.update(:hook_count => 1)
+          @resource.hook_count.should == 2
+        end
       end
     end
 
@@ -183,12 +196,19 @@ describe DataMapper::Model::Hook do
           ModelHookSpecs.after(:update) { hooks << :after_update }
 
           @resource.save
-          @resource.update(:value => 2)
         end
 
         it 'should execute after update hook' do
+          @resource.update(:value => 2)
           @hooks.should == [ :after_update ]
         end
+        
+        it "should execute hook when no property is changed" do
+          ModelHookSpecs.before(:update, :update_hook_count)
+          @resource.update(:hook_count => 1)
+          @resource.hook_count.should == 2
+        end
+        
       end
     end
 
-- 
1.7.3.4