From 08561b22920aa5eaa76addd8b0da8feb189e0d18 Mon Sep 17 00:00:00 2001
From: Paul Berry <paul@puppetlabs.com>
Date: Tue, 11 Jan 2011 14:56:14 -0800
Subject: (#5838) Refactored Puppet::Network::Rights#fail_on_deny

Changed into a method that returns the exception to raised rather than
raising it.

Paired-with: Jesse Wolfe <jesse@puppetlabs.com>
---
 spec/unit/network/rest_authconfig_spec.rb |  2 +-
 spec/unit/network/rights_spec.rb          | 46 +++++++++++++++----------------
 2 files changed, 24 insertions(+), 24 deletions(-)

(limited to 'spec')

diff --git a/spec/unit/network/rest_authconfig_spec.rb b/spec/unit/network/rest_authconfig_spec.rb
index 06436e723..9892c2b25 100755
--- a/spec/unit/network/rest_authconfig_spec.rb
+++ b/spec/unit/network/rest_authconfig_spec.rb
@@ -47,7 +47,7 @@ describe Puppet::Network::RestAuthConfig do
   end
 
   it "should ask for authorization to the ACL subsystem" do
-    @acl.expects(:fail_on_deny).with("/path/to/resource", :node => "me", :ip => "127.0.0.1", :method => :save, :environment => :env, :authenticated => true)
+    @acl.expects(:is_forbidden_and_why?).with("/path/to/resource", :node => "me", :ip => "127.0.0.1", :method => :save, :environment => :env, :authenticated => true).returns(nil)
 
     @authconfig.allowed?(@request)
   end
diff --git a/spec/unit/network/rights_spec.rb b/spec/unit/network/rights_spec.rb
index 969fc189e..ca3f22464 100755
--- a/spec/unit/network/rights_spec.rb
+++ b/spec/unit/network/rights_spec.rb
@@ -155,19 +155,19 @@ describe Puppet::Network::Rights do
       Puppet::Network::Rights::Right.stubs(:new).returns(@pathacl)
     end
 
-    it "should delegate to fail_on_deny" do
-      @right.expects(:fail_on_deny).with("namespace", :node => "host.domain.com", :ip => "127.0.0.1")
+    it "should delegate to is_forbidden_and_why?" do
+      @right.expects(:is_forbidden_and_why?).with("namespace", :node => "host.domain.com", :ip => "127.0.0.1").returns(nil)
 
       @right.allowed?("namespace", "host.domain.com", "127.0.0.1")
     end
 
-    it "should return true if fail_on_deny doesn't fail" do
-      @right.stubs(:fail_on_deny)
+    it "should return true if is_forbidden_and_why? returns nil" do
+      @right.stubs(:is_forbidden_and_why?).returns(nil)
       @right.allowed?("namespace", :args).should be_true
     end
 
-    it "should return false if fail_on_deny raises an AuthorizationError" do
-      @right.stubs(:fail_on_deny).raises(Puppet::Network::AuthorizationError.new("forbidden"))
+    it "should return false if is_forbidden_and_why? returns an AuthorizationError" do
+      @right.stubs(:is_forbidden_and_why?).returns(Puppet::Network::AuthorizationError.new("forbidden"))
       @right.allowed?("namespace", :args1, :args2).should be_false
     end
 
@@ -179,7 +179,7 @@ describe Puppet::Network::Rights do
       acl.expects(:match?).returns(true)
       acl.expects(:allowed?).with { |node,ip,h| node == "node" and ip == "ip" }.returns(true)
 
-      @right.fail_on_deny("namespace", { :node => "node", :ip => "ip" } )
+      @right.is_forbidden_and_why?("namespace", { :node => "node", :ip => "ip" } ).should == nil
     end
 
     it "should then check for path rights if no namespace match" do
@@ -195,7 +195,7 @@ describe Puppet::Network::Rights do
       acl.expects(:allowed?).never
       @pathacl.expects(:allowed?).returns(true)
 
-      @right.fail_on_deny("/path/to/there", {})
+      @right.is_forbidden_and_why?("/path/to/there", {}).should == nil
     end
 
     it "should pass the match? return to allowed?" do
@@ -204,12 +204,12 @@ describe Puppet::Network::Rights do
       @pathacl.expects(:match?).returns(:match)
       @pathacl.expects(:allowed?).with { |node,ip,h| h[:match] == :match }.returns(true)
 
-      @right.fail_on_deny("/path/to/there", {})
+      @right.is_forbidden_and_why?("/path/to/there", {}).should == nil
     end
 
     describe "with namespace acls" do
-      it "should raise an error if this namespace right doesn't exist" do
-        lambda{ @right.fail_on_deny("namespace") }.should raise_error
+      it "should return an ArgumentError if this namespace right doesn't exist" do
+        lambda { @right.is_forbidden_and_why?("namespace") }.should raise_error(ArgumentError)
       end
     end
 
@@ -235,7 +235,7 @@ describe Puppet::Network::Rights do
         @long_acl.expects(:allowed?).returns(true)
         @short_acl.expects(:allowed?).never
 
-        @right.fail_on_deny("/path/to/there/and/there", {})
+        @right.is_forbidden_and_why?("/path/to/there/and/there", {}).should == nil
       end
 
       it "should select the first match that doesn't return :dunno" do
@@ -248,7 +248,7 @@ describe Puppet::Network::Rights do
         @long_acl.expects(:allowed?).returns(:dunno)
         @short_acl.expects(:allowed?).returns(true)
 
-        @right.fail_on_deny("/path/to/there/and/there", {})
+        @right.is_forbidden_and_why?("/path/to/there/and/there", {}).should == nil
       end
 
       it "should not select an ACL that doesn't match" do
@@ -261,7 +261,7 @@ describe Puppet::Network::Rights do
         @long_acl.expects(:allowed?).never
         @short_acl.expects(:allowed?).returns(true)
 
-        @right.fail_on_deny("/path/to/there/and/there", {})
+        @right.is_forbidden_and_why?("/path/to/there/and/there", {}).should == nil
       end
 
       it "should not raise an AuthorizationError if allowed" do
@@ -270,7 +270,7 @@ describe Puppet::Network::Rights do
         @long_acl.stubs(:match?).returns(true)
         @long_acl.stubs(:allowed?).returns(true)
 
-        lambda { @right.fail_on_deny("/path/to/there/and/there", {}) }.should_not raise_error(Puppet::Network::AuthorizationError)
+        @right.is_forbidden_and_why?("/path/to/there/and/there", {}).should == nil
       end
 
       it "should raise an AuthorizationError if the match is denied" do
@@ -279,11 +279,11 @@ describe Puppet::Network::Rights do
         @long_acl.stubs(:match?).returns(true)
         @long_acl.stubs(:allowed?).returns(false)
 
-        lambda{ @right.fail_on_deny("/path/to/there", {}) }.should raise_error(Puppet::Network::AuthorizationError)
+        @right.is_forbidden_and_why?("/path/to/there", {}).should be_instance_of(Puppet::Network::AuthorizationError)
       end
 
       it "should raise an AuthorizationError if no path match" do
-        lambda { @right.fail_on_deny("/nomatch", {}) }.should raise_error(Puppet::Network::AuthorizationError)
+        @right.is_forbidden_and_why?("/nomatch", {}).should be_instance_of(Puppet::Network::AuthorizationError)
       end
     end
 
@@ -309,7 +309,7 @@ describe Puppet::Network::Rights do
         @regex_acl1.expects(:allowed?).returns(true)
         @regex_acl2.expects(:allowed?).never
 
-        @right.fail_on_deny("/files/repository/myfile/other", {})
+        @right.is_forbidden_and_why?("/files/repository/myfile/other", {}).should == nil
       end
 
       it "should select the first match that doesn't return :dunno" do
@@ -322,7 +322,7 @@ describe Puppet::Network::Rights do
         @regex_acl1.expects(:allowed?).returns(:dunno)
         @regex_acl2.expects(:allowed?).returns(true)
 
-        @right.fail_on_deny("/files/repository/myfile/other", {})
+        @right.is_forbidden_and_why?("/files/repository/myfile/other", {}).should == nil
       end
 
       it "should not select an ACL that doesn't match" do
@@ -335,7 +335,7 @@ describe Puppet::Network::Rights do
         @regex_acl1.expects(:allowed?).never
         @regex_acl2.expects(:allowed?).returns(true)
 
-        @right.fail_on_deny("/files/repository/myfile/other", {})
+        @right.is_forbidden_and_why?("/files/repository/myfile/other", {}).should == nil
       end
 
       it "should not raise an AuthorizationError if allowed" do
@@ -344,15 +344,15 @@ describe Puppet::Network::Rights do
         @regex_acl1.stubs(:match?).returns(true)
         @regex_acl1.stubs(:allowed?).returns(true)
 
-        lambda { @right.fail_on_deny("/files/repository/myfile/other", {}) }.should_not raise_error(Puppet::Network::AuthorizationError)
+        @right.is_forbidden_and_why?("/files/repository/myfile/other", {}).should == nil
       end
 
       it "should raise an error if no regex acl match" do
-        lambda{ @right.fail_on_deny("/path", {}) }.should raise_error(Puppet::Network::AuthorizationError)
+        @right.is_forbidden_and_why?("/path", {}).should be_instance_of(Puppet::Network::AuthorizationError)
       end
 
       it "should raise an AuthorizedError on deny" do
-        lambda { @right.fail_on_deny("/path", {}) }.should raise_error(Puppet::Network::AuthorizationError)
+        @right.is_forbidden_and_why?("/path", {}).should be_instance_of(Puppet::Network::AuthorizationError)
       end
 
     end
-- 
cgit 


From c514c641d0c0090be29252dcc773385248d3fe93 Mon Sep 17 00:00:00 2001
From: Paul Berry <paul@puppetlabs.com>
Date: Mon, 10 Jan 2011 17:05:38 -0800
Subject: (#5838) Added support for HEAD requests to the indirector.

Added the ability for the indirector to handle REST HEAD requests.
These are done using a new indirector method, head(), which should
return true if find() would return a result and false if find() would
return nil.

Access control for the head method is the union of that for the find
and save methods.  That is, if either find or save is allowed, then
head is allowed.  This is necessary so that users will not have to
change their authconfig to take advantage of the new feature.

Paired-with: Jesse Wolfe <jesse@puppetlabs.com>
---
 spec/unit/indirector/file_server_spec.rb  | 13 +++---
 spec/unit/indirector/indirection_spec.rb  | 69 +++++++++++++++++++++++++++++++
 spec/unit/indirector/rest_spec.rb         | 36 ++++++++++++++++
 spec/unit/indirector_spec.rb              |  5 +++
 spec/unit/network/http/api/v1_spec.rb     |  4 ++
 spec/unit/network/http/handler_spec.rb    | 33 +++++++++++++++
 spec/unit/network/rest_authconfig_spec.rb |  2 +-
 spec/unit/network/rights_spec.rb          | 20 +++++++++
 8 files changed, 174 insertions(+), 8 deletions(-)

(limited to 'spec')

diff --git a/spec/unit/indirector/file_server_spec.rb b/spec/unit/indirector/file_server_spec.rb
index 686f79a24..eafcf2bb7 100755
--- a/spec/unit/indirector/file_server_spec.rb
+++ b/spec/unit/indirector/file_server_spec.rb
@@ -229,6 +229,12 @@ describe Puppet::Indirector::FileServer do
   describe "when checking authorization" do
     before do
       @request.method = :find
+
+      @mount = stub 'mount'
+      @configuration.stubs(:split_path).with(@request).returns([@mount, "rel/path"])
+      @request.stubs(:node).returns("mynode")
+      @request.stubs(:ip).returns("myip")
+      @mount.stubs(:allowed?).with("mynode", "myip").returns "something"
     end
 
     it "should return false when destroying" do
@@ -254,13 +260,6 @@ describe Puppet::Indirector::FileServer do
     end
 
     it "should return the results of asking the mount whether the node and IP are authorized" do
-      @mount = stub 'mount'
-      @configuration.expects(:split_path).with(@request).returns([@mount, "rel/path"])
-
-      @request.stubs(:node).returns("mynode")
-      @request.stubs(:ip).returns("myip")
-      @mount.expects(:allowed?).with("mynode", "myip").returns "something"
-
       @file_server.authorized?(@request).should == "something"
     end
   end
diff --git a/spec/unit/indirector/indirection_spec.rb b/spec/unit/indirector/indirection_spec.rb
index 1e774fb2e..a910cb6f2 100755
--- a/spec/unit/indirector/indirection_spec.rb
+++ b/spec/unit/indirector/indirection_spec.rb
@@ -383,6 +383,75 @@ describe Puppet::Indirector::Indirection do
       end
     end
 
+    describe "and doing a head operation" do
+      before { @method = :head }
+
+      it_should_behave_like "Indirection Delegator"
+      it_should_behave_like "Delegation Authorizer"
+
+      it "should return true if the head method returned true" do
+        @terminus.expects(:head).returns(true)
+        @indirection.head("me").should == true
+      end
+
+      it "should return false if the head method returned false" do
+        @terminus.expects(:head).returns(false)
+        @indirection.head("me").should == false
+      end
+
+      describe "when caching is enabled" do
+        before do
+          @indirection.cache_class = :cache_terminus
+          @cache_class.stubs(:new).returns(@cache)
+
+          @instance.stubs(:expired?).returns false
+        end
+
+        it "should first look in the cache for an instance" do
+          @terminus.stubs(:find).never
+          @terminus.stubs(:head).never
+          @cache.expects(:find).returns @instance
+
+          @indirection.head("/my/key").should == true
+        end
+
+        it "should not save to the cache" do
+          @cache.expects(:find).returns nil
+          @cache.expects(:save).never
+          @terminus.expects(:head).returns true
+          @indirection.head("/my/key").should == true
+        end
+
+        it "should not fail if the cache fails" do
+          @terminus.stubs(:head).returns true
+
+          @cache.expects(:find).raises ArgumentError
+          lambda { @indirection.head("/my/key") }.should_not raise_error
+        end
+
+        it "should look in the main terminus if the cache fails" do
+          @terminus.expects(:head).returns true
+          @cache.expects(:find).raises ArgumentError
+          @indirection.head("/my/key").should == true
+        end
+
+        it "should send a debug log if it is using the cached object" do
+          Puppet.expects(:debug)
+          @cache.stubs(:find).returns @instance
+
+          @indirection.head("/my/key")
+        end
+
+        it "should not accept the cached object if it is expired" do
+          @instance.stubs(:expired?).returns true
+
+          @cache.stubs(:find).returns @instance
+          @terminus.stubs(:head).returns false
+          @indirection.head("/my/key").should == false
+        end
+      end
+    end
+
     describe "and storing a model instance" do
       before { @method = :save }
 
diff --git a/spec/unit/indirector/rest_spec.rb b/spec/unit/indirector/rest_spec.rb
index 5f0fe363a..7bc1cc513 100755
--- a/spec/unit/indirector/rest_spec.rb
+++ b/spec/unit/indirector/rest_spec.rb
@@ -282,6 +282,42 @@ describe Puppet::Indirector::REST do
     end
   end
 
+  describe "when doing a head" do
+    before :each do
+      @connection = stub('mock http connection', :head => @response)
+      @searcher.stubs(:network).returns(@connection)
+
+      # Use a key with spaces, so we can test escaping
+      @request = Puppet::Indirector::Request.new(:foo, :head, "foo bar")
+    end
+
+    it "should call the HEAD http method on a network connection" do
+      @searcher.expects(:network).returns @connection
+      @connection.expects(:head).returns @response
+      @searcher.head(@request)
+    end
+
+    it "should return true if there was a successful http response" do
+      @connection.expects(:head).returns @response
+      @response.stubs(:code).returns "200"
+
+      @searcher.head(@request).should == true
+    end
+
+    it "should return false if there was a successful http response" do
+      @connection.expects(:head).returns @response
+      @response.stubs(:code).returns "404"
+
+      @searcher.head(@request).should == false
+    end
+
+    it "should use the URI generated by the Handler module" do
+      @searcher.expects(:indirection2uri).with(@request).returns "/my/uri"
+      @connection.expects(:head).with { |path, args| path == "/my/uri" }.returns(@response)
+      @searcher.head(@request)
+    end
+  end
+
   describe "when doing a search" do
     before :each do
       @connection = stub('mock http connection', :get => @response)
diff --git a/spec/unit/indirector_spec.rb b/spec/unit/indirector_spec.rb
index acbfc1726..6f44764da 100755
--- a/spec/unit/indirector_spec.rb
+++ b/spec/unit/indirector_spec.rb
@@ -118,6 +118,11 @@ describe Puppet::Indirector, "when redirecting a model" do
     it_should_behave_like "Delegated Indirection Method"
   end
 
+  describe "when performing a head request" do
+    before { @method = :head }
+    it_should_behave_like "Delegated Indirection Method"
+  end
+
   # This is an instance method, so it behaves a bit differently.
   describe "when saving instances via the model" do
     before do
diff --git a/spec/unit/network/http/api/v1_spec.rb b/spec/unit/network/http/api/v1_spec.rb
index c593242c0..23a291cf3 100644
--- a/spec/unit/network/http/api/v1_spec.rb
+++ b/spec/unit/network/http/api/v1_spec.rb
@@ -68,6 +68,10 @@ describe Puppet::Network::HTTP::API::V1 do
       @tester.uri2indirection("GET", "/env/foo/bar", {}).method.should == :find
     end
 
+    it "should choose 'head' as the indirection method if the http method is a HEAD and the indirection name is singular" do
+      @tester.uri2indirection("HEAD", "/env/foo/bar", {}).method.should == :head
+    end
+
     it "should choose 'search' as the indirection method if the http method is a GET and the indirection name is plural" do
       @tester.uri2indirection("GET", "/env/foos/bar", {}).method.should == :search
     end
diff --git a/spec/unit/network/http/handler_spec.rb b/spec/unit/network/http/handler_spec.rb
index cdbce41f7..8464ae68e 100755
--- a/spec/unit/network/http/handler_spec.rb
+++ b/spec/unit/network/http/handler_spec.rb
@@ -256,6 +256,39 @@ describe Puppet::Network::HTTP::Handler do
       end
     end
 
+    describe "when performing head operation" do
+      before do
+        @irequest = stub 'indirection_request', :method => :head, :indirection_name => "my_handler", :to_hash => {}, :key => "my_result", :model => @model_class
+
+        @model_class.stubs(:head).returns true
+      end
+
+      it "should use the indirection request to find the model class" do
+        @irequest.expects(:model).returns @model_class
+
+        @handler.do_head(@irequest, @request, @response)
+      end
+
+      it "should use the escaped request key" do
+        @model_class.expects(:head).with do |key, args|
+          key == "my_result"
+        end.returns true
+        @handler.do_head(@irequest, @request, @response)
+      end
+
+      it "should not generate a response when a model head call succeeds" do
+        @handler.expects(:set_response).never
+        @handler.do_head(@irequest, @request, @response)
+      end
+
+      it "should return a 404 when the model head call returns false" do
+        @model_class.stubs(:name).returns "my name"
+        @handler.expects(:set_response).with { |response, body, status| status == 404 }
+        @model_class.stubs(:head).returns(false)
+        @handler.do_head(@irequest, @request, @response)
+      end
+    end
+
     describe "when searching for model instances" do
       before do
         @irequest = stub 'indirection_request', :method => :find, :indirection_name => "my_handler", :to_hash => {}, :key => "key", :model => @model_class
diff --git a/spec/unit/network/rest_authconfig_spec.rb b/spec/unit/network/rest_authconfig_spec.rb
index 9892c2b25..d629f8670 100755
--- a/spec/unit/network/rest_authconfig_spec.rb
+++ b/spec/unit/network/rest_authconfig_spec.rb
@@ -47,7 +47,7 @@ describe Puppet::Network::RestAuthConfig do
   end
 
   it "should ask for authorization to the ACL subsystem" do
-    @acl.expects(:is_forbidden_and_why?).with("/path/to/resource", :node => "me", :ip => "127.0.0.1", :method => :save, :environment => :env, :authenticated => true).returns(nil)
+    @acl.expects(:is_request_forbidden_and_why?).with(@request).returns(nil)
 
     @authconfig.allowed?(@request)
   end
diff --git a/spec/unit/network/rights_spec.rb b/spec/unit/network/rights_spec.rb
index ca3f22464..3b9e48374 100755
--- a/spec/unit/network/rights_spec.rb
+++ b/spec/unit/network/rights_spec.rb
@@ -9,6 +9,26 @@ describe Puppet::Network::Rights do
     @right = Puppet::Network::Rights.new
   end
 
+  describe "when validating a :head request" do
+    [:find, :save].each do |allowed_method|
+      it "should allow the request if only #{allowed_method} is allowed" do
+        rights = Puppet::Network::Rights.new
+        rights.newright("/")
+        rights.allow("/", "*")
+        rights.restrict_method("/", allowed_method)
+        rights.restrict_authenticated("/", :any)
+        request = Puppet::Indirector::Request.new(:indirection_name, :head, "key")
+        rights.is_request_forbidden_and_why?(request).should == nil
+      end
+    end
+
+    it "should disallow the request if neither :find nor :save is allowed" do
+      rights = Puppet::Network::Rights.new
+      request = Puppet::Indirector::Request.new(:indirection_name, :head, "key")
+      rights.is_request_forbidden_and_why?(request).should be_instance_of(Puppet::Network::AuthorizationError)
+    end
+  end
+
   [:allow, :deny, :restrict_method, :restrict_environment, :restrict_authenticated].each do |m|
     it "should have a #{m} method" do
       @right.should respond_to(m)
-- 
cgit 


From 9cfd3d58699b0d0c3ab53cb37226cade84d7ec64 Mon Sep 17 00:00:00 2001
From: Paul Berry <paul@puppetlabs.com>
Date: Tue, 11 Jan 2011 13:36:05 -0800
Subject: (#5838) Reworked file dipper spec to perform less stubbing.

This is in preparation for adding more functionality to the file
dipper.

Paired-with: Jesse Wolfe <jesse@puppetlabs.com>
---
 spec/unit/file_bucket/dipper_spec.rb | 114 ++++++++++-------------------------
 1 file changed, 32 insertions(+), 82 deletions(-)

(limited to 'spec')

diff --git a/spec/unit/file_bucket/dipper_spec.rb b/spec/unit/file_bucket/dipper_spec.rb
index 799e899e7..86ad01f83 100755
--- a/spec/unit/file_bucket/dipper_spec.rb
+++ b/spec/unit/file_bucket/dipper_spec.rb
@@ -2,121 +2,71 @@
 
 require File.dirname(__FILE__) + '/../../spec_helper'
 
+require 'pathname'
+
 require 'puppet/file_bucket/dipper'
 describe Puppet::FileBucket::Dipper do
-  before do
-    ['/my/file'].each do |x|
-      Puppet::FileBucket::Dipper.any_instance.stubs(:absolutize_path).with(x).returns(x)
-    end
+  include PuppetSpec::Files
+
+  def make_tmp_file(contents)
+    file = tmpfile("file_bucket_file")
+    File.open(file, 'w') { |f| f.write(contents) }
+    file
   end
 
   it "should fail in an informative way when there are failures backing up to the server" do
-    File.stubs(:exist?).returns true
-    File.stubs(:read).returns "content"
-
     @dipper = Puppet::FileBucket::Dipper.new(:Path => "/my/bucket")
 
-    filemock = stub "bucketfile"
-    Puppet::FileBucket::File.stubs(:new).returns(filemock)
-    filemock.expects(:name).returns "name"
-    filemock.expects(:save).raises ArgumentError
+    file = make_tmp_file('contents')
+    Puppet::FileBucket::File.any_instance.expects(:save).raises ArgumentError
 
-    lambda { @dipper.backup("/my/file") }.should raise_error(Puppet::Error)
+    lambda { @dipper.backup(file) }.should raise_error(Puppet::Error)
   end
 
   it "should backup files to a local bucket" do
-    @dipper = Puppet::FileBucket::Dipper.new(
-      :Path => "/my/bucket"
-    )
-
-    File.stubs(:exist?).returns true
-    File.stubs(:read).with("/my/file").returns "my contents"
-
-    bucketfile = stub "bucketfile"
-    bucketfile.stubs(:name).returns('md5/DIGEST123')
-    bucketfile.stubs(:checksum_data).returns("DIGEST123")
-    bucketfile.expects(:save).with('md5/DIGEST123')
-
+    @dipper = Puppet::FileBucket::Dipper.new(:Path => "/my/bucket")
 
-          Puppet::FileBucket::File.stubs(:new).with(
-                
-      "my contents",
-      :bucket_path => '/my/bucket',
-        
-      :path => '/my/file'
-    ).returns(bucketfile)
+    file = make_tmp_file('my contents')
+    checksum = Digest::MD5.hexdigest('my contents')
 
-    @dipper.backup("/my/file").should == "DIGEST123"
+    Puppet::FileBucket::File.any_instance.expects(:save)
+    @dipper.backup(file).should == checksum
   end
 
   it "should retrieve files from a local bucket" do
-    @dipper = Puppet::FileBucket::Dipper.new(
-      :Path => "/my/bucket"
-    )
-
-    File.stubs(:exist?).returns true
-    File.stubs(:read).with("/my/file").returns "my contents"
+    @dipper = Puppet::FileBucket::Dipper.new(:Path => "/my/bucket")
 
-    bucketfile = stub "bucketfile"
-    bucketfile.stubs(:to_s).returns "Content"
+    checksum = Digest::MD5.hexdigest('my contents')
 
     Puppet::FileBucket::File.expects(:find).with{|x,opts|
-      x == 'md5/DIGEST123'
-    }.returns(bucketfile)
+      x == "md5/#{checksum}"
+    }.returns(Puppet::FileBucket::File.new('my contents'))
 
-    @dipper.getfile("DIGEST123").should == "Content"
+    @dipper.getfile(checksum).should == 'my contents'
   end
 
   it "should backup files to a remote server" do
+    @dipper = Puppet::FileBucket::Dipper.new(:Server => "puppetmaster", :Port => "31337")
 
-          @dipper = Puppet::FileBucket::Dipper.new(
-                
-      :Server => "puppetmaster",
-        
-      :Port   => "31337"
-    )
-
-    File.stubs(:exist?).returns true
-    File.stubs(:read).with("/my/file").returns "my contents"
+    file = make_tmp_file('my contents')
+    checksum = Digest::MD5.hexdigest('my contents')
 
-    bucketfile = stub "bucketfile"
-    bucketfile.stubs(:name).returns('md5/DIGEST123')
-    bucketfile.stubs(:checksum_data).returns("DIGEST123")
-    bucketfile.expects(:save).with('https://puppetmaster:31337/production/file_bucket_file/md5/DIGEST123')
+    real_path = Pathname.new(file).realpath
 
+    Puppet::FileBucket::File.any_instance.expects(:save).with("https://puppetmaster:31337/production/file_bucket_file/md5/#{checksum}/#{real_path}")
 
-          Puppet::FileBucket::File.stubs(:new).with(
-                
-      "my contents",
-      :bucket_path => nil,
-        
-      :path => '/my/file'
-    ).returns(bucketfile)
-
-    @dipper.backup("/my/file").should == "DIGEST123"
+    @dipper.backup(file).should == checksum
   end
 
   it "should retrieve files from a remote server" do
+    @dipper = Puppet::FileBucket::Dipper.new(:Server => "puppetmaster", :Port => "31337")
 
-          @dipper = Puppet::FileBucket::Dipper.new(
-                
-      :Server => "puppetmaster",
-        
-      :Port   => "31337"
-    )
-
-    File.stubs(:exist?).returns true
-    File.stubs(:read).with("/my/file").returns "my contents"
-
-    bucketfile = stub "bucketfile"
-    bucketfile.stubs(:to_s).returns "Content"
+    checksum = Digest::MD5.hexdigest('my contents')
 
     Puppet::FileBucket::File.expects(:find).with{|x,opts|
-      x == 'https://puppetmaster:31337/production/file_bucket_file/md5/DIGEST123'
-    }.returns(bucketfile)
+      x == "https://puppetmaster:31337/production/file_bucket_file/md5/#{checksum}"
+    }.returns(Puppet::FileBucket::File.new('my contents'))
 
-    @dipper.getfile("DIGEST123").should == "Content"
+    @dipper.getfile(checksum).should == "my contents"
   end
-
-
 end
-- 
cgit 


From 89f56920f26544f7c5aa97785567b193034db151 Mon Sep 17 00:00:00 2001
From: Paul Berry <paul@puppetlabs.com>
Date: Tue, 11 Jan 2011 15:24:10 -0800
Subject: (#5838) Implemented the "head" method for FileBucketFile::File
 terminus.

In order to do this it was necessary to refactor FileBucketFile to
untangle responsibilities for computing paths, reading files, etc.  In
the process, removed speculative generalizations and unused
functionality.

Paired-with: Jesse Wolfe <jesse@puppetlabs.com>
---
 spec/unit/file_bucket/dipper_spec.rb               |   2 +-
 spec/unit/file_bucket/file_spec.rb                 | 108 ++----------
 spec/unit/indirector/file_bucket_file/file_spec.rb | 185 ++-------------------
 3 files changed, 30 insertions(+), 265 deletions(-)

(limited to 'spec')

diff --git a/spec/unit/file_bucket/dipper_spec.rb b/spec/unit/file_bucket/dipper_spec.rb
index 86ad01f83..730e10792 100755
--- a/spec/unit/file_bucket/dipper_spec.rb
+++ b/spec/unit/file_bucket/dipper_spec.rb
@@ -53,7 +53,7 @@ describe Puppet::FileBucket::Dipper do
 
     real_path = Pathname.new(file).realpath
 
-    Puppet::FileBucket::File.any_instance.expects(:save).with("https://puppetmaster:31337/production/file_bucket_file/md5/#{checksum}/#{real_path}")
+    Puppet::FileBucket::File.any_instance.expects(:save).with("https://puppetmaster:31337/production/file_bucket_file/md5/#{checksum}")
 
     @dipper.backup(file).should == checksum
   end
diff --git a/spec/unit/file_bucket/file_spec.rb b/spec/unit/file_bucket/file_spec.rb
index 3ad70c203..82063c2e3 100644
--- a/spec/unit/file_bucket/file_spec.rb
+++ b/spec/unit/file_bucket/file_spec.rb
@@ -7,13 +7,16 @@ require 'digest/md5'
 require 'digest/sha1'
 
 describe Puppet::FileBucket::File do
+  include PuppetSpec::Files
+
   before do
     # this is the default from spec_helper, but it keeps getting reset at odd times
-    Puppet[:bucketdir] = "/dev/null/bucket"
+    @bucketdir = tmpdir('bucket')
+    Puppet[:bucketdir] = @bucketdir
 
     @digest = "4a8ec4fa5f01b4ab1a0ab8cbccb709f0"
     @checksum = "{md5}4a8ec4fa5f01b4ab1a0ab8cbccb709f0"
-    @dir = '/dev/null/bucket/4/a/8/e/c/4/f/a/4a8ec4fa5f01b4ab1a0ab8cbccb709f0'
+    @dir = File.join(@bucketdir, '4/a/8/e/c/4/f/a/4a8ec4fa5f01b4ab1a0ab8cbccb709f0')
 
     @contents = "file contents"
   end
@@ -22,17 +25,6 @@ describe Puppet::FileBucket::File do
     Puppet::FileBucket::File.new(@contents).to_s.should == @contents
   end
 
-  it "should calculate the checksum type from the passed in checksum" do
-    Puppet::FileBucket::File.new(@contents, :checksum => @checksum).checksum_type.should == "md5"
-  end
-
-  it "should allow contents to be specified in a block" do
-    bucket = Puppet::FileBucket::File.new(nil) do |fb|
-      fb.contents = "content"
-    end
-    bucket.contents.should == "content"
-  end
-
   it "should raise an error if changing content" do
     x = Puppet::FileBucket::File.new("first")
     proc { x.contents = "new" }.should raise_error
@@ -42,14 +34,6 @@ describe Puppet::FileBucket::File do
     proc { Puppet::FileBucket::File.new(5) }.should raise_error(ArgumentError)
   end
 
-  it "should raise an error if setting contents to a non-string" do
-    proc do
-      Puppet::FileBucket::File.new(nil) do |x|
-        x.contents = 5
-      end
-    end.should raise_error(ArgumentError)
-  end
-
   it "should set the contents appropriately" do
     Puppet::FileBucket::File.new(@contents).contents.should == @contents
   end
@@ -62,33 +46,6 @@ describe Puppet::FileBucket::File do
     Puppet::FileBucket::File.new(@contents).checksum.should == @checksum
   end
 
-  it "should remove the old checksum value if the algorithm is changed" do
-    sum = Puppet::FileBucket::File.new(@contents)
-    sum.checksum.should_not be_nil
-
-    newsum = Digest::SHA1.hexdigest(@contents).to_s
-    sum.checksum_type = :sha1
-    sum.checksum.should == "{sha1}#{newsum}"
-  end
-
-  it "should support specifying the checksum_type during initialization" do
-    sum = Puppet::FileBucket::File.new(@contents, :checksum_type => :sha1)
-    sum.checksum_type.should == :sha1
-  end
-
-  it "should fail when an unsupported checksum_type is used" do
-    proc { Puppet::FileBucket::File.new(@contents, :checksum_type => :nope) }.should raise_error(ArgumentError)
-  end
-
-  it "should fail if given an checksum at initialization that does not match the contents" do
-    proc { Puppet::FileBucket::File.new(@contents, :checksum => "{md5}00000000000000000000000000000000") }.should raise_error(RuntimeError)
-  end
-
-  it "should fail if assigned a checksum that does not match the contents" do
-    bucket = Puppet::FileBucket::File.new(@contents)
-    proc { bucket.checksum = "{md5}00000000000000000000000000000000" }.should raise_error(RuntimeError)
-  end
-
   describe "when using back-ends" do
     it "should redirect using Puppet::Indirector" do
       Puppet::Indirector::Indirection.instance(:file_bucket_file).model.should equal(Puppet::FileBucket::File)
@@ -129,26 +86,6 @@ describe Puppet::FileBucket::File do
 
       Puppet::FileBucket::File.new(@contents).save
     end
-
-    it "should append the path to the paths file" do
-      remote_path = '/path/on/the/remote/box'
-
-      ::File.expects(:directory?).with(@dir).returns(true)
-      ::File.expects(:open).with("#{@dir}/contents", ::File::WRONLY|::File::CREAT, 0440)
-      ::File.expects(:exist?).with("#{@dir}/contents").returns false
-
-      mockfile = mock "file"
-      mockfile.expects(:puts).with('/path/on/the/remote/box')
-      ::File.expects(:exist?).with("#{@dir}/paths").returns false
-      ::File.expects(:open).with("#{@dir}/paths", ::File::WRONLY|::File::CREAT|::File::APPEND).yields mockfile
-      Puppet::FileBucket::File.new(@contents, :path => remote_path).save
-
-    end
-  end
-
-  it "should accept a path" do
-    remote_path = '/path/on/the/remote/box'
-    Puppet::FileBucket::File.new(@contents, :path => remote_path).path.should == remote_path
   end
 
   it "should return a url-ish name" do
@@ -160,18 +97,6 @@ describe Puppet::FileBucket::File do
     lambda { bucket.name = "sha1/4a8ec4fa5f01b4ab1a0ab8cbccb709f0/new/path" }.should raise_error
   end
 
-  it "should accept a url-ish name" do
-    bucket = Puppet::FileBucket::File.new(@contents)
-    lambda { bucket.name = "sha1/034fa2ed8e211e4d20f20e792d777f4a30af1a93/new/path" }.should_not raise_error
-    bucket.checksum_type.should == "sha1"
-    bucket.checksum_data.should == '034fa2ed8e211e4d20f20e792d777f4a30af1a93'
-    bucket.path.should == "new/path"
-  end
-
-  it "should return a url-ish name with a path" do
-    Puppet::FileBucket::File.new(@contents, :path => 'my/path').name.should == "md5/4a8ec4fa5f01b4ab1a0ab8cbccb709f0/my/path"
-  end
-
   it "should convert the contents to PSON" do
     Puppet::FileBucket::File.new(@contents).to_pson.should == '{"contents":"file contents"}'
   end
@@ -191,36 +116,31 @@ describe Puppet::FileBucket::File do
 
   end
 
+  def make_bucketed_file
+    FileUtils.mkdir_p(@dir)
+    File.open("#{@dir}/contents", 'w') { |f| f.write @contents }
+  end
+
   describe "using the indirector's find method" do
     it "should return nil if a file doesn't exist" do
-      ::File.expects(:exist?).with("#{@dir}/contents").returns false
-
-      bucketfile = Puppet::FileBucket::File.find("{md5}#{@digest}")
+      bucketfile = Puppet::FileBucket::File.find("md5/#{@digest}")
       bucketfile.should == nil
     end
 
     it "should find a filebucket if the file exists" do
-      ::File.expects(:exist?).with("#{@dir}/contents").returns true
-      ::File.expects(:exist?).with("#{@dir}/paths").returns false
-      ::File.expects(:read).with("#{@dir}/contents").returns @contents
-
-      bucketfile = Puppet::FileBucket::File.find("{md5}#{@digest}")
+      make_bucketed_file
+      bucketfile = Puppet::FileBucket::File.find("md5/#{@digest}")
       bucketfile.should_not == nil
     end
 
     describe "using RESTish digest notation" do
       it "should return nil if a file doesn't exist" do
-        ::File.expects(:exist?).with("#{@dir}/contents").returns false
-
         bucketfile = Puppet::FileBucket::File.find("md5/#{@digest}")
         bucketfile.should == nil
       end
 
       it "should find a filebucket if the file exists" do
-        ::File.expects(:exist?).with("#{@dir}/contents").returns true
-        ::File.expects(:exist?).with("#{@dir}/paths").returns false
-        ::File.expects(:read).with("#{@dir}/contents").returns @contents
-
+        make_bucketed_file
         bucketfile = Puppet::FileBucket::File.find("md5/#{@digest}")
         bucketfile.should_not == nil
       end
diff --git a/spec/unit/indirector/file_bucket_file/file_spec.rb b/spec/unit/indirector/file_bucket_file/file_spec.rb
index 7bf02b5b3..6057fd2e6 100755
--- a/spec/unit/indirector/file_bucket_file/file_spec.rb
+++ b/spec/unit/indirector/file_bucket_file/file_spec.rb
@@ -5,6 +5,8 @@ require ::File.dirname(__FILE__) + '/../../../spec_helper'
 require 'puppet/indirector/file_bucket_file/file'
 
 describe Puppet::FileBucketFile::File do
+  include PuppetSpec::Files
+
   it "should be a subclass of the Code terminus class" do
     Puppet::FileBucketFile::File.superclass.should equal(Puppet::Indirector::Code)
   end
@@ -66,141 +68,40 @@ HERE
   end
 
 
-  describe "the find_by_checksum method" 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"
-    end
-
-    it "should return nil if a file doesn't exist" do
-      ::File.expects(:exist?).with("#{@dir}/contents").returns false
-
-      bucketfile = Puppet::FileBucketFile::File.new.send(:find_by_checksum, "{md5}#{@digest}", {})
-      bucketfile.should == nil
-    end
-
-    it "should find a filebucket if the file exists" do
-      ::File.expects(:exist?).with("#{@dir}/contents").returns true
-      ::File.expects(:exist?).with("#{@dir}/paths").returns false
-      ::File.expects(:read).with("#{@dir}/contents").returns @contents
-
-      bucketfile = Puppet::FileBucketFile::File.new.send(:find_by_checksum, "{md5}#{@digest}", {})
-      bucketfile.should_not == nil
-    end
-
-    it "should load the paths" do
-      paths = ["path1", "path2"]
-      ::File.expects(:exist?).with("#{@dir}/contents").returns true
-      ::File.expects(:exist?).with("#{@dir}/paths").returns true
-      ::File.expects(:read).with("#{@dir}/contents").returns @contents
-
-      mockfile = mock "file"
-      mockfile.expects(:readlines).returns( paths )
-      ::File.expects(:open).with("#{@dir}/paths").yields mockfile
-
-      Puppet::FileBucketFile::File.new.send(:find_by_checksum, "{md5}#{@digest}", {}).paths.should == paths
-    end
-
-  end
-
   describe "when retrieving files" do
     before :each do
       Puppet.settings.stubs(:use)
       @store = Puppet::FileBucketFile::File.new
 
       @digest = "70924d6fa4b2d745185fa4660703a5c0"
-      @sum = stub 'sum', :name => @digest
 
-      @dir = "/what/ever"
+      @bucket_dir = tmpdir("bucket")
 
-      Puppet.stubs(:[]).with(:bucketdir).returns(@dir)
+      Puppet.stubs(:[]).with(:bucketdir).returns(@bucket_dir)
 
-      @contents_path = '/what/ever/7/0/9/2/4/d/6/f/70924d6fa4b2d745185fa4660703a5c0/contents'
-      @paths_path    = '/what/ever/7/0/9/2/4/d/6/f/70924d6fa4b2d745185fa4660703a5c0/paths'
+      @dir = "#{@bucket_dir}/7/0/9/2/4/d/6/f/70924d6fa4b2d745185fa4660703a5c0"
+      @contents_path = "#{@dir}/contents"
 
-      @request = stub 'request', :key => "md5/#{@digest}/remote/path", :options => {}
+      @request = Puppet::Indirector::Request.new(:indirection_name, :find, "md5/#{@digest}")
     end
 
-    it "should call find_by_checksum" do
-      @store.expects(:find_by_checksum).with{|x,opts| x == "{md5}#{@digest}"}.returns(false)
-      @store.find(@request)
-    end
-
-    it "should look for the calculated path" do
-      ::File.expects(:exist?).with(@contents_path).returns(false)
-      @store.find(@request)
+    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
-      content = "my content"
-      bucketfile = stub 'bucketfile'
-      bucketfile.stubs(:bucket_path)
-      bucketfile.stubs(:bucket_path=)
-      bucketfile.stubs(:checksum_data).returns(@digest)
-      bucketfile.stubs(:checksum).returns(@checksum)
-
-      bucketfile.expects(:contents=).with(content)
-      Puppet::FileBucket::File.expects(:new).with(nil, {:checksum => "{md5}#{@digest}"}).yields(bucketfile).returns(bucketfile)
+      @contents = "my content"
+      make_bucketed_file
 
-      ::File.expects(:exist?).with(@contents_path).returns(true)
-      ::File.expects(:exist?).with(@paths_path).returns(false)
-      ::File.expects(:read).with(@contents_path).returns(content)
-
-      @store.find(@request).should equal(bucketfile)
+      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
-      ::File.expects(:exist?).with(@contents_path).returns(false)
       @store.find(@request).should be_nil
     end
-
-    it "should fail intelligently if a found file cannot be read" do
-      ::File.expects(:exist?).with(@contents_path).returns(true)
-      ::File.expects(:read).with(@contents_path).raises(RuntimeError)
-      proc { @store.find(@request) }.should raise_error(Puppet::Error)
-    end
-
-  end
-
-  describe "when determining file paths" do
-    before do
-      Puppet[:bucketdir] = '/dev/null/bucketdir'
-      @digest = 'DEADBEEFC0FFEE'
-      @bucket = stub_everything "bucket"
-      @bucket.expects(:checksum_data).returns(@digest)
-    end
-
-    it "should use the value of the :bucketdir setting as the root directory" do
-      path = Puppet::FileBucketFile::File.new.send(:contents_path_for, @bucket)
-      path.should =~ %r{^/dev/null/bucketdir}
-    end
-
-    it "should choose a path 8 directories deep with each directory name being the respective character in the filebucket" do
-      path = Puppet::FileBucketFile::File.new.send(:contents_path_for, @bucket)
-      dirs = @digest[0..7].split("").join(File::SEPARATOR)
-      path.should be_include(dirs)
-    end
-
-    it "should use the full filebucket as the final directory name" do
-      path = Puppet::FileBucketFile::File.new.send(:contents_path_for, @bucket)
-      ::File.basename(::File.dirname(path)).should == @digest
-    end
-
-    it "should use 'contents' as the actual file name" do
-      path = Puppet::FileBucketFile::File.new.send(:contents_path_for, @bucket)
-      ::File.basename(path).should == "contents"
-    end
-
-    it "should use the bucketdir, the 8 sum character directories, the full filebucket, and 'contents' as the full file name" do
-      path = Puppet::FileBucketFile::File.new.send(:contents_path_for, @bucket)
-      path.should == ['/dev/null/bucketdir', @digest[0..7].split(""), @digest, "contents"].flatten.join(::File::SEPARATOR)
-    end
   end
 
   describe "when saving files" do
@@ -276,60 +177,4 @@ HERE
     end
 
   end
-
-
-  describe "when writing to the paths file" do
-    before do
-      Puppet[:bucketdir] = '/dev/null/bucketdir'
-      @digest = '70924d6fa4b2d745185fa4660703a5c0'
-      @bucket = stub_everything "bucket"
-
-      @paths_path    = '/dev/null/bucketdir/7/0/9/2/4/d/6/f/70924d6fa4b2d745185fa4660703a5c0/paths'
-
-      @paths = []
-      @bucket.stubs(:paths).returns(@paths)
-      @bucket.stubs(:checksum_data).returns(@digest)
-    end
-
-    it "should create a file if it doesn't exist" do
-      @bucket.expects(:path).returns('path/to/save').at_least_once
-      File.expects(:exist?).with(@paths_path).returns(false)
-      file = stub "file"
-      file.expects(:puts).with('path/to/save')
-      File.expects(:open).with(@paths_path, ::File::WRONLY|::File::CREAT|::File::APPEND).yields(file)
-
-      Puppet::FileBucketFile::File.new.send(:save_path_to_paths_file, @bucket)
-    end
-
-    it "should append to a file if it exists" do
-      @bucket.expects(:path).returns('path/to/save').at_least_once
-      File.expects(:exist?).with(@paths_path).returns(true)
-      old_file = stub "file"
-      old_file.stubs(:readlines).returns []
-      File.expects(:open).with(@paths_path).yields(old_file)
-
-      file = stub "file"
-      file.expects(:puts).with('path/to/save')
-      File.expects(:open).with(@paths_path, ::File::WRONLY|::File::CREAT|::File::APPEND).yields(file)
-
-      Puppet::FileBucketFile::File.new.send(:save_path_to_paths_file, @bucket)
-    end
-
-    it "should not alter a file if it already contains the path" do
-      @bucket.expects(:path).returns('path/to/save').at_least_once
-      File.expects(:exist?).with(@paths_path).returns(true)
-      old_file = stub "file"
-      old_file.stubs(:readlines).returns ["path/to/save\n"]
-      File.expects(:open).with(@paths_path).yields(old_file)
-
-      Puppet::FileBucketFile::File.new.send(:save_path_to_paths_file, @bucket)
-    end
-
-    it "should do nothing if there is no path" do
-      @bucket.expects(:path).returns(nil).at_least_once
-
-      Puppet::FileBucketFile::File.new.send(:save_path_to_paths_file, @bucket)
-    end
-  end
-
 end
-- 
cgit 


From 94d71799f1ee196186bc3a8a5a1b06ef2ae0806e Mon Sep 17 00:00:00 2001
From: Paul Berry <paul@puppetlabs.com>
Date: Tue, 11 Jan 2011 13:45:55 -0800
Subject: (#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>
---
 spec/unit/file_bucket/dipper_spec.rb | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

(limited to 'spec')

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
-- 
cgit 


From 002f9f1905b089ebca628a6b743c0659e30ff9bc Mon Sep 17 00:00:00 2001
From: Paul Berry <paul@puppetlabs.com>
Date: Wed, 12 Jan 2011 15:18:21 -0800
Subject: (#5838) Improve the quality of file bucket specs.

Paired-with: Jesse Wolfe <jesse@puppetlabs.com>
---
 spec/unit/file_bucket/dipper_spec.rb               | 43 +++++++++++-----
 spec/unit/indirector/file_bucket_file/file_spec.rb | 59 +++++++++++++---------
 2 files changed, 66 insertions(+), 36 deletions(-)

(limited to 'spec')

diff --git a/spec/unit/file_bucket/dipper_spec.rb b/spec/unit/file_bucket/dipper_spec.rb
index 3e9e8b145..db40c6296 100755
--- a/spec/unit/file_bucket/dipper_spec.rb
+++ b/spec/unit/file_bucket/dipper_spec.rb
@@ -5,6 +5,8 @@ require File.dirname(__FILE__) + '/../../spec_helper'
 require 'pathname'
 
 require 'puppet/file_bucket/dipper'
+require 'puppet/indirector/file_bucket_file/rest'
+
 describe Puppet::FileBucket::Dipper do
   include PuppetSpec::Files
 
@@ -34,14 +36,17 @@ describe Puppet::FileBucket::Dipper do
   end
 
   it "should backup files to a local bucket" do
-    @dipper = Puppet::FileBucket::Dipper.new(:Path => "/my/bucket")
+    Puppet[:bucketdir] = "/non/existent/directory"
+    file_bucket = tmpdir("bucket")
+
+    @dipper = Puppet::FileBucket::Dipper.new(:Path => file_bucket)
 
     file = make_tmp_file('my contents')
-    checksum = Digest::MD5.hexdigest('my contents')
+    checksum = "2975f560750e71c478b8e3b39a956adb"
+    Digest::MD5.hexdigest('my contents').should == checksum
 
-    Puppet::FileBucket::File.expects(:head).returns false
-    Puppet::FileBucket::File.any_instance.expects(:save)
     @dipper.backup(file).should == checksum
+    File.exists?("#{file_bucket}/2/9/7/5/f/5/6/0/2975f560750e71c478b8e3b39a956adb/contents").should == true
   end
 
   it "should not backup a file that is already in the bucket" do
@@ -60,11 +65,13 @@ describe Puppet::FileBucket::Dipper do
 
     checksum = Digest::MD5.hexdigest('my contents')
 
-    Puppet::FileBucket::File.expects(:find).with{|x,opts|
-      x == "md5/#{checksum}"
-    }.returns(Puppet::FileBucket::File.new('my contents'))
+    request = nil
+
+    Puppet::FileBucketFile::File.any_instance.expects(:find).with{ |r| request = r }.once.returns(Puppet::FileBucket::File.new('my contents'))
 
     @dipper.getfile(checksum).should == 'my contents'
+
+    request.key.should == "md5/#{checksum}"
   end
 
   it "should backup files to a remote server" do
@@ -75,10 +82,18 @@ 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}")
+    request1 = nil
+    request2 = nil
+
+    Puppet::FileBucketFile::Rest.any_instance.expects(:head).with { |r| request1 = r }.once.returns(nil)
+    Puppet::FileBucketFile::Rest.any_instance.expects(:save).with { |r| request2 = r }.once
 
     @dipper.backup(file).should == checksum
+    [request1, request2].each do |r|
+      r.server.should == 'puppetmaster'
+      r.port.should == 31337
+      r.key.should == "md5/#{checksum}"
+    end
   end
 
   it "should retrieve files from a remote server" do
@@ -86,10 +101,14 @@ describe Puppet::FileBucket::Dipper do
 
     checksum = Digest::MD5.hexdigest('my contents')
 
-    Puppet::FileBucket::File.expects(:find).with{|x,opts|
-      x == "https://puppetmaster:31337/production/file_bucket_file/md5/#{checksum}"
-    }.returns(Puppet::FileBucket::File.new('my contents'))
+    request = nil
+
+    Puppet::FileBucketFile::Rest.any_instance.expects(:find).with { |r| request = r }.returns(Puppet::FileBucket::File.new('my contents'))
 
     @dipper.getfile(checksum).should == "my contents"
+
+    request.server.should == 'puppetmaster'
+    request.port.should == 31337
+    request.key.should == "md5/#{checksum}"
   end
 end
diff --git a/spec/unit/indirector/file_bucket_file/file_spec.rb b/spec/unit/indirector/file_bucket_file/file_spec.rb
index 6057fd2e6..cb614f36e 100755
--- a/spec/unit/indirector/file_bucket_file/file_spec.rb
+++ b/spec/unit/indirector/file_bucket_file/file_spec.rb
@@ -68,39 +68,50 @@ HERE
   end
 
 
-  describe "when retrieving files" do
-    before :each do
-      Puppet.settings.stubs(:use)
-      @store = Puppet::FileBucketFile::File.new
+  [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"
+        @digest = "70924d6fa4b2d745185fa4660703a5c0"
 
-      @bucket_dir = tmpdir("bucket")
+        @bucket_dir = tmpdir("bucket")
 
-      Puppet.stubs(:[]).with(:bucketdir).returns(@bucket_dir)
+        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"
+        @dir = "#{@bucket_dir}/7/0/9/2/4/d/6/f/70924d6fa4b2d745185fa4660703a5c0"
+        @contents_path = "#{@dir}/contents"
 
-      @request = Puppet::Indirector::Request.new(:indirection_name, :find, "md5/#{@digest}")
-    end
+        request_options = {}
+        if override_bucket_path
+          request_options[:bucket_path] = @bucket_dir
+        end
 
-    def make_bucketed_file
-      FileUtils.mkdir_p(@dir)
-      File.open(@contents_path, 'w') { |f| f.write @contents }
-    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
+      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
+        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
+      it "should return nil if no file is found" do
+        @store.find(@request).should be_nil
+      end
     end
   end
 
-- 
cgit 


From abc62560f78fa227d6ffd3263a095665609a15b5 Mon Sep 17 00:00:00 2001
From: Paul Berry <paul@puppetlabs.com>
Date: Wed, 12 Jan 2011 15:41:39 -0800
Subject: (#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>
---
 spec/unit/indirector/file_bucket_file/file_spec.rb | 162 ++++++++++-----------
 1 file changed, 78 insertions(+), 84 deletions(-)

(limited to 'spec')

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
-- 
cgit