(#5838) Support paths as part of file bucket requests.

In versions of Puppet 2.6.0-2.6.4, file bucket requests are of the
form md5/<checksum>/<path>.  The path functionality has been removed,
however we still need to support requests coming from older clients.

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

2 files changed, 79 insertions, 85 deletions

diff --git a/lib/puppet/indirector/file_bucket_file/file.rb b/lib/puppet/indirector/file_bucket_file/file.rb
index 38e0be6e9..8bea2d767 100644
--- a/lib/puppet/indirector/file_bucket_file/file.rb
+++ b/lib/puppet/indirector/file_bucket_file/file.rb
@@ -73,7 +73,7 @@ module Puppet::FileBucketFile
     end
 
     def request_to_checksum( request )
-      checksum_type, checksum = request.key.split(/\//, 2)
+      checksum_type, checksum, path = request.key.split(/\//, 3) # Note: we ignore path if present.
       raise "Unsupported checksum type #{checksum_type.inspect}" if checksum_type != 'md5'
       raise "Invalid checksum #{checksum.inspect}" if checksum !~ /^[0-9a-f]{32}$/
       checksum
diff --git a/spec/unit/indirector/file_bucket_file/file_spec.rb b/spec/unit/indirector/file_bucket_file/file_spec.rb
index cb614f36e..9187f4da0 100755
--- a/spec/unit/indirector/file_bucket_file/file_spec.rb
+++ b/spec/unit/indirector/file_bucket_file/file_spec.rb
@@ -69,95 +69,89 @@ HERE
 
 
   [true, false].each do |override_bucket_path|
-    describe "when retrieving files and bucket path #{if override_bucket_path then 'is' else 'is not' end} overridden" do
-      before :each do
-        Puppet.settings.stubs(:use)
-        @store = Puppet::FileBucketFile::File.new
-
-        @digest = "70924d6fa4b2d745185fa4660703a5c0"
-
-        @bucket_dir = tmpdir("bucket")
-
-        if override_bucket_path
-          Puppet[:bucketdir] = "/bogus/path" # should not be used
-        else
-          Puppet[:bucketdir] = @bucket_dir
-        end
-
-        @dir = "#{@bucket_dir}/7/0/9/2/4/d/6/f/70924d6fa4b2d745185fa4660703a5c0"
-        @contents_path = "#{@dir}/contents"
-
-        request_options = {}
-        if override_bucket_path
-          request_options[:bucket_path] = @bucket_dir
+    describe "when bucket path #{if override_bucket_path then 'is' else 'is not' end} overridden" do
+      [true, false].each do |supply_path|
+        describe "when #{supply_path ? 'supplying' : 'not supplying'} a path" do
+          before :each do
+            Puppet.settings.stubs(:use)
+            @store = Puppet::FileBucketFile::File.new
+            @contents = "my content"
+
+            @digest = "f2bfa7fc155c4f42cb91404198dda01f"
+            @digest.should == Digest::MD5.hexdigest(@contents)
+
+            @bucket_dir = tmpdir("bucket")
+
+            if override_bucket_path
+              Puppet[:bucketdir] = "/bogus/path" # should not be used
+            else
+              Puppet[:bucketdir] = @bucket_dir
+            end
+
+            @dir = "#{@bucket_dir}/f/2/b/f/a/7/f/c/f2bfa7fc155c4f42cb91404198dda01f"
+            @contents_path = "#{@dir}/contents"
+          end
+
+          describe "when retrieving files" do
+            before :each do
+
+              request_options = {}
+              if override_bucket_path
+                request_options[:bucket_path] = @bucket_dir
+              end
+
+              key = "md5/#{@digest}"
+              if supply_path
+                key += "//path/to/file"
+              end
+
+              @request = Puppet::Indirector::Request.new(:indirection_name, :find, key, request_options)
+            end
+
+            def make_bucketed_file
+              FileUtils.mkdir_p(@dir)
+              File.open(@contents_path, 'w') { |f| f.write @contents }
+            end
+
+            it "should return an instance of Puppet::FileBucket::File created with the content if the file exists" do
+              make_bucketed_file
+
+              bucketfile = @store.find(@request)
+              bucketfile.should be_a(Puppet::FileBucket::File)
+              bucketfile.contents.should == @contents
+              @store.head(@request).should == true
+            end
+
+            it "should return nil if no file is found" do
+              @store.find(@request).should be_nil
+              @store.head(@request).should == false
+            end
+          end
+
+          describe "when saving files" do
+            it "should save the contents to the calculated path" do
+              options = {}
+              if override_bucket_path
+                options[:bucket_path] = @bucket_dir
+              end
+
+              key = "md5/#{@digest}"
+              if supply_path
+                key += "//path/to/file"
+              end
+
+              file_instance = Puppet::FileBucket::File.new(@contents, options)
+              request = Puppet::Indirector::Request.new(:indirection_name, :save, key, file_instance)
+
+              @store.save(request)
+              File.read("#{@dir}/contents").should == @contents
+            end
+          end
         end
-
-        @request = Puppet::Indirector::Request.new(:indirection_name, :find, "md5/#{@digest}", request_options)
-      end
-
-      def make_bucketed_file
-        FileUtils.mkdir_p(@dir)
-        File.open(@contents_path, 'w') { |f| f.write @contents }
-      end
-
-      it "should return an instance of Puppet::FileBucket::File created with the content if the file exists" do
-        @contents = "my content"
-        make_bucketed_file
-
-        bucketfile = @store.find(@request)
-        bucketfile.should be_a(Puppet::FileBucket::File)
-        bucketfile.contents.should == @contents
-      end
-
-      it "should return nil if no file is found" do
-        @store.find(@request).should be_nil
       end
     end
   end
 
-  describe "when saving files" do
-    before do
-      # this is the default from spec_helper, but it keeps getting reset at odd times
-      Puppet[:bucketdir] = "/dev/null/bucket"
-
-      @digest = "4a8ec4fa5f01b4ab1a0ab8cbccb709f0"
-      @checksum = "{md5}4a8ec4fa5f01b4ab1a0ab8cbccb709f0"
-      @dir = '/dev/null/bucket/4/a/8/e/c/4/f/a/4a8ec4fa5f01b4ab1a0ab8cbccb709f0'
-
-      @contents = "file contents"
-
-      @bucket = stub "bucket file"
-      @bucket.stubs(:bucket_path)
-      @bucket.stubs(:checksum_data).returns(@digest)
-      @bucket.stubs(:path).returns(nil)
-      @bucket.stubs(:checksum).returns(nil)
-      @bucket.stubs(:contents).returns("file contents")
-    end
-
-    it "should save the contents to the calculated path" do
-      ::File.stubs(:directory?).with(@dir).returns(true)
-      ::File.expects(:exist?).with("#{@dir}/contents").returns false
-
-      mockfile = mock "file"
-      mockfile.expects(:print).with(@contents)
-      ::File.expects(:open).with("#{@dir}/contents", ::File::WRONLY|::File::CREAT, 0440).yields(mockfile)
-
-      Puppet::FileBucketFile::File.new.send(:save_to_disk, @bucket)
-    end
-
-    it "should make any directories necessary for storage" do
-      FileUtils.expects(:mkdir_p).with do |arg|
-        ::File.umask == 0007 and arg == @dir
-      end
-      ::File.expects(:directory?).with(@dir).returns(false)
-      ::File.expects(:open).with("#{@dir}/contents", ::File::WRONLY|::File::CREAT, 0440)
-      ::File.expects(:exist?).with("#{@dir}/contents").returns false
-
-      Puppet::FileBucketFile::File.new.send(:save_to_disk, @bucket)
-    end
-  end
-
-
   describe "when verifying identical files" do
     before do
       # this is the default from spec_helper, but it keeps getting reset at odd times