(#5838) Make file bucket dipper efficient when saving a file that already exists

Before saving to the file bucket, the file bucket dipper now checks to
make sure that no file with the given checksum is already present.

Paired-with: Jesse Wolfe <jesse@puppetlabs.com>

2 files changed, 29 insertions, 1 deletions

diff --git a/lib/puppet/file_bucket/dipper.rb b/lib/puppet/file_bucket/dipper.rb
index b012a8681..f4bef28a8 100644
--- a/lib/puppet/file_bucket/dipper.rb
+++ b/lib/puppet/file_bucket/dipper.rb
@@ -36,7 +36,12 @@ class Puppet::FileBucket::Dipper
       file_bucket_file = Puppet::FileBucket::File.new(contents, :bucket_path => @local_path)
       dest_path = "#{@rest_path}#{file_bucket_file.name}"
 
-      file_bucket_file.save(dest_path)
+      # Make a HEAD request for the file so that we don't waste time
+      # uploading it if it already exists in the bucket.
+      unless Puppet::FileBucket::File.head("#{@rest_path}#{file_bucket_file.checksum_type}/#{file_bucket_file.checksum_data}")
+        file_bucket_file.save(dest_path)
+      end
+
       return file_bucket_file.checksum_data
     rescue => detail
       puts detail.backtrace if Puppet[:trace]
diff --git a/spec/unit/file_bucket/dipper_spec.rb b/spec/unit/file_bucket/dipper_spec.rb
index 730e10792..3e9e8b145 100755
--- a/spec/unit/file_bucket/dipper_spec.rb
+++ b/spec/unit/file_bucket/dipper_spec.rb
@@ -14,10 +14,20 @@ describe Puppet::FileBucket::Dipper do
     file
   end
 
+  it "should fail in an informative way when there are failures checking for the file on the server" do
+    @dipper = Puppet::FileBucket::Dipper.new(:Path => "/my/bucket")
+
+    file = make_tmp_file('contents')
+    Puppet::FileBucket::File.expects(:head).raises ArgumentError
+
+    lambda { @dipper.backup(file) }.should raise_error(Puppet::Error)
+  end
+
   it "should fail in an informative way when there are failures backing up to the server" do
     @dipper = Puppet::FileBucket::Dipper.new(:Path => "/my/bucket")
 
     file = make_tmp_file('contents')
+    Puppet::FileBucket::File.expects(:head).returns false
     Puppet::FileBucket::File.any_instance.expects(:save).raises ArgumentError
 
     lambda { @dipper.backup(file) }.should raise_error(Puppet::Error)
@@ -29,10 +39,22 @@ describe Puppet::FileBucket::Dipper do
     file = make_tmp_file('my contents')
     checksum = Digest::MD5.hexdigest('my contents')
 
+    Puppet::FileBucket::File.expects(:head).returns false
     Puppet::FileBucket::File.any_instance.expects(:save)
     @dipper.backup(file).should == checksum
   end
 
+  it "should not backup a file that is already in the bucket" do
+    @dipper = Puppet::FileBucket::Dipper.new(:Path => "/my/bucket")
+
+    file = make_tmp_file('my contents')
+    checksum = Digest::MD5.hexdigest('my contents')
+
+    Puppet::FileBucket::File.expects(:head).returns true
+    Puppet::FileBucket::File.any_instance.expects(:save).never
+    @dipper.backup(file).should == checksum
+  end
+
   it "should retrieve files from a local bucket" do
     @dipper = Puppet::FileBucket::Dipper.new(:Path => "/my/bucket")
 
@@ -53,6 +75,7 @@ describe Puppet::FileBucket::Dipper do
 
     real_path = Pathname.new(file).realpath
 
+    Puppet::FileBucket::File.expects(:head).with("https://puppetmaster:31337/production/file_bucket_file/md5/#{checksum}").returns false
     Puppet::FileBucket::File.any_instance.expects(:save).with("https://puppetmaster:31337/production/file_bucket_file/md5/#{checksum}")
 
     @dipper.backup(file).should == checksum