OC-10380: skip checksumming for no-content files

2 files changed, 50 insertions, 5 deletions

diff --git a/lib/chef/provider/file.rb b/lib/chef/provider/file.rb
index e727aa9ec1..12fa97e334 100644
--- a/lib/chef/provider/file.rb
+++ b/lib/chef/provider/file.rb
@@ -1,4 +1,4 @@
-#
+
 # Author:: Adam Jacob (<adam@opscode.com>)
 # Author:: Lamont Granquist (<lamont@opscode.com>)
 # Copyright:: Copyright (c) 2008-2013 Opscode, Inc.
@@ -75,7 +75,8 @@ class Chef
         @current_resource ||= Chef::Resource::File.new(@new_resource.name)
         @current_resource.path(@new_resource.path)
         if ::File.exists?(@current_resource.path) && ::File.file?(::File.realpath(@current_resource.path))
-          if @action != :create_if_missing && @current_resource.respond_to?(:checksum)
+          if should_do_checksum?
+            Chef::Log.debug("#{@new_resource} checksumming file at #{@new_resource.path}.")
             @current_resource.checksum(checksum(@current_resource.path))
           end
           load_resource_attributes_from_file(@current_resource)
@@ -159,6 +160,25 @@ class Chef
 
       private
 
+      def should_do_checksum?
+        if @current_resource.respond_to?(:checksum)
+          case
+          when @new_resource.checksum
+            return true
+          when @action == :create_if_missing
+            return false
+          when @new_resource.respond_to?(:source)
+            return true
+          when @new_resource.content.nil?
+            return false
+          else
+            return true
+          end
+        end
+        # We may be a File::Provider::Directory
+        false
+      end
+
       # Handles resource requirements for action :create when some fs entry
       # already exists at the destination path. For actions other than create,
       # we don't care what kind of thing is at the destination path because:
@@ -240,8 +260,8 @@ class Chef
 
       def content
         @content ||= begin
-           load_current_resource if @current_resource.nil?
-           @content_class.new(@new_resource, @current_resource, @run_context)
+          load_current_resource if @current_resource.nil?
+          @content_class.new(@new_resource, @current_resource, @run_context)
         end
       end
 
@@ -330,7 +350,9 @@ class Chef
         do_backup unless file_created?
         deployment_strategy.deploy(tempfile.path, ::File.realpath(@new_resource.path))
         Chef::Log.info("#{@new_resource} updated file contents #{@new_resource.path}")
-        @new_resource.checksum(checksum(@new_resource.path)) # for reporting
+        if should_do_checksum?
+          @new_resource.checksum(checksum(@new_resource.path)) # for reporting
+        end
       end
 
       def do_contents_changes
@@ -379,6 +401,7 @@ class Chef
       end
 
       def contents_changed?
+        Chef::Log.debug "calculating checksum of #{tempfile.path} to compare with #{@current_resource.checksum}"
         checksum(tempfile.path) != @current_resource.checksum
       end
 
diff --git a/spec/support/shared/unit/provider/file.rb b/spec/support/shared/unit/provider/file.rb
index 11e991d830..7efea1dd9e 100644
--- a/spec/support/shared/unit/provider/file.rb
+++ b/spec/support/shared/unit/provider/file.rb
@@ -93,6 +93,26 @@ shared_examples_for Chef::Provider::File do
 
   context "when loading the current resource" do
 
+    context "when running load_current_resource" do
+      #
+      # the content objects need the current_resource to be loaded (esp remote_file), so calling
+      # for content inside of load_current_resource is totally crossing the streams...
+      #
+      it "should not try to load the content when the file is present" do
+        setup_normal_file
+        provider.should_not_receive(:tempfile)
+        provider.should_not_receive(:content)
+        provider.load_current_resource
+      end
+
+      it "should not try to load the content when the file is missing" do
+        setup_missing_file
+        provider.should_not_receive(:tempfile)
+        provider.should_not_receive(:content)
+        provider.load_current_resource
+      end
+    end
+
     context "when running load_current_resource and the file exists" do
       before do
         setup_normal_file
@@ -114,6 +134,7 @@ shared_examples_for Chef::Provider::File do
       it "the loaded current_resource content should be nil" do
         provider.current_resource.content.should eql(nil)
       end
+
     end
 
     context "when running load_current_resource and the file does not exist" do
@@ -383,6 +404,7 @@ shared_examples_for Chef::Provider::File do
                                   :for_reporting => diff_for_reporting )
             diff.stub!(:diff).with(resource_path, tempfile_path).and_return(true)
             provider.should_receive(:diff).at_least(:once).and_return(diff)
+            provider.should_receive(:should_do_checksum?).at_least(:once).and_return(true)
             provider.should_receive(:checksum).with(tempfile_path).and_return(tempfile_sha256)
             provider.should_receive(:checksum).with(resource_path).and_return(tempfile_sha256)
             provider.deployment_strategy.should_receive(:deploy).with(tempfile_path, normalized_path)