From 1b7669379b79e7e85be9d6d523f2777b5da0c803 Mon Sep 17 00:00:00 2001
From: Dan Kubb <dan.kubb@gmail.com>
Date: Mon, 1 Feb 2010 23:33:36 -0800
Subject: [PATCH] Skip saving if the dirty test returns false

* This will speed up any cases where the resource is being saved and there
  are before(:save) or after(:save) filters.
* Removed now redundant code from _create and _update

[#1155 state:resolved]
---
 lib/dm-core/resource.rb                    | 10 +++----
 spec/public/shared/resource_shared_spec.rb | 33 +++++++++++++++++++++-
 2 files changed, 36 insertions(+), 7 deletions(-)

diff --git a/lib/dm-core/resource.rb b/lib/dm-core/resource.rb
index 7b93fd8d..edc5cb88 100644
--- a/lib/dm-core/resource.rb
+++ b/lib/dm-core/resource.rb
@@ -849,9 +849,6 @@ def child_collections
     #
     # @api private
     def _create
-      # Can't create a resource that is not dirty and doesn't have serial keys
-      return false if new? && clean?
-
       # set defaults for new resource
       properties.each do |property|
         unless property.serial? || property.loaded?(self)
@@ -884,9 +881,7 @@ def _create
     def _update
       original_attributes = self.original_attributes
 
-      if original_attributes.empty?
-        true
-      elsif original_attributes.any? { |property, _value| !property.valid?(property.get!(self)) }
+      if original_attributes.any? { |property, _value| !property.valid?(property.get!(self)) }
         false
       else
         # remove from the identity map
@@ -919,6 +914,9 @@ def _save(safe)
     #
     # @api semipublic
     def save_self(safe = true)
+      # short-circuit if the resource is not dirty
+      return saved? unless dirty_self?
+
       new_resource = new?
       if safe
         new_resource ? create_hook : update_hook
diff --git a/spec/public/shared/resource_shared_spec.rb b/spec/public/shared/resource_shared_spec.rb
index 5c0792d5..bc9691b2 100644
--- a/spec/public/shared/resource_shared_spec.rb
+++ b/spec/public/shared/resource_shared_spec.rb
@@ -704,6 +704,17 @@
     it { @user.should respond_to(method) }
 
     describe "##{method}" do
+      before :all do
+        @user_model.class_eval do
+          attr_accessor :save_hook_call_count
+
+          before :save do
+            @save_hook_call_count ||= 0
+            @save_hook_call_count += 1
+          end
+        end
+      end
+
       describe 'on a new, not dirty resource' do
         before :all do
           @user = @user_model.new
@@ -713,11 +724,23 @@
         it 'should return false' do
           @return.should be_false
         end
+
+        it 'should call save hook expected number of times' do
+          @user.save_hook_call_count.should be_nil
+        end
       end
 
       describe 'on a not new, not dirty resource' do
+        before :all do
+          @return = @user.__send__(method)
+        end
+
         it 'should return true even when resource is not dirty' do
-          @user.__send__(method).should be_true
+          @return.should be_true
+        end
+
+        it 'should call save hook expected number of times' do
+          @user.save_hook_call_count.should be_nil
         end
       end
 
@@ -736,6 +759,10 @@
         it 'should actually store the changes to persistent storage' do
           @user.attributes.should == @user.reload.attributes
         end
+
+        it 'should call save hook expected number of times' do
+          @user.save_hook_call_count.should == (method == :save ? 1 : nil)
+        end
       end
 
       describe 'on a dirty invalid resource' do
@@ -748,6 +775,10 @@
         it 'should not save an invalid resource' do
           @user.__send__(method).should be_false
         end
+
+        it 'should call save hook expected number of times' do
+          @user.save_hook_call_count.should == (method == :save ? 1 : nil)
+        end
       end
 
       describe 'with new resources in a has relationship' do