Refactoring part of the file/filebucket integration

The goal of this commit is to fix ordering issues
that could result when the filebuckets are added
to the catalog after the resources that use them.

This condition showed up somewhat arbitrarily.

Signed-off-by: Luke Kanies <luke@madstop.com>

3 files changed, 58 insertions, 10 deletions

diff --git a/spec/integration/transaction.rb b/spec/integration/transaction.rb
index 17aba0df8..c06a43d80 100755
--- a/spec/integration/transaction.rb
+++ b/spec/integration/transaction.rb
@@ -7,10 +7,10 @@ require 'puppet/transaction'
 describe Puppet::Transaction do
     it "should not apply generated resources if the parent resource fails" do
         catalog = Puppet::Resource::Catalog.new
-        resource = Puppet::Type.type(:file).new :path => "/foo/bar"
+        resource = Puppet::Type.type(:file).new :path => "/foo/bar", :backup => false
         catalog.add_resource resource
 
-        child_resource = Puppet::Type.type(:file).new :path => "/foo/bar/baz"
+        child_resource = Puppet::Type.type(:file).new :path => "/foo/bar/baz", :backup => false
 
         resource.expects(:eval_generate).returns([child_resource])
 
diff --git a/spec/integration/type/file.rb b/spec/integration/type/file.rb
index 2877153f0..cced1ed1c 100755
--- a/spec/integration/type/file.rb
+++ b/spec/integration/type/file.rb
@@ -33,7 +33,7 @@ describe Puppet::Type.type(:file) do
         it "should be able to recurse over a nonexistent file" do
             @path = tmpfile("file_integration_tests")
 
-            @file = Puppet::Type::File.new(:name => @path, :mode => 0644, :recurse => true)
+            @file = Puppet::Type::File.new(:name => @path, :mode => 0644, :recurse => true, :backup => false)
 
             @catalog = Puppet::Resource::Catalog.new
             @catalog.add_resource @file
@@ -46,7 +46,7 @@ describe Puppet::Type.type(:file) do
 
             build_path(@path)
 
-            @file = Puppet::Type::File.new(:name => @path, :mode => 0644, :recurse => true)
+            @file = Puppet::Type::File.new(:name => @path, :mode => 0644, :recurse => true, :backup => false)
 
             @catalog = Puppet::Resource::Catalog.new
             @catalog.add_resource @file
@@ -69,7 +69,7 @@ describe Puppet::Type.type(:file) do
 
             dest = tmpfile("file_link_integration_dest")
 
-            @file = Puppet::Type::File.new(:name => dest, :target => source, :recurse => true, :ensure => :link)
+            @file = Puppet::Type::File.new(:name => dest, :target => source, :recurse => true, :ensure => :link, :backup => false)
 
             @catalog = Puppet::Resource::Catalog.new
             @catalog.add_resource @file
@@ -96,7 +96,7 @@ describe Puppet::Type.type(:file) do
 
             dest = tmpfile("file_source_integration_dest")
 
-            @file = Puppet::Type::File.new(:name => dest, :source => source, :recurse => true)
+            @file = Puppet::Type::File.new(:name => dest, :source => source, :recurse => true, :backup => false)
 
             @catalog = Puppet::Resource::Catalog.new
             @catalog.add_resource @file
@@ -131,7 +131,7 @@ describe Puppet::Type.type(:file) do
             File.open(s1, "w") { |f| f.puts "uno" }
             File.open(s2, "w") { |f| f.puts "dos" }
 
-            @file = Puppet::Type::File.new(:name => @dest, :source => @source, :recurse => true)
+            @file = Puppet::Type::File.new(:name => @dest, :source => @source, :recurse => true, :backup => false)
 
             @catalog = Puppet::Resource::Catalog.new
             @catalog.add_resource @file
@@ -197,7 +197,7 @@ describe Puppet::Type.type(:file) do
 
             File.open(source, "w") { |f| f.print "foo" }
 
-            file = Puppet::Type::File.new(:name => dest, :source => source)
+            file = Puppet::Type::File.new(:name => dest, :source => source, :backup => false)
 
             catalog = Puppet::Resource::Catalog.new
             catalog.add_resource file
@@ -269,7 +269,8 @@ describe Puppet::Type.type(:file) do
         file = Puppet::Type.type(:file).new(
             :name => dest,
             :ensure => :absent,
-            :source => source
+            :source => source,
+            :backup => false
         )
 
         catalog = Puppet::Resource::Catalog.new
diff --git a/spec/unit/type/file.rb b/spec/unit/type/file.rb
index cdc4ffd57..58cd4ad23 100755
--- a/spec/unit/type/file.rb
+++ b/spec/unit/type/file.rb
@@ -680,7 +680,7 @@ describe Puppet::Type.type(:file) do
             Puppet::Type::File.new(:name => "/my/file", :backup => ".bak")[:backup].should == ".bak"
         end
 
-        it "should set the file bucket when backup is set to a string matching the name of a filebucket in the catalog" do
+        it "should set the filebucket when backup is set to a string matching the name of a filebucket in the catalog" do
             catalog = Puppet::Resource::Catalog.new
             bucket_resource = Puppet::Type.type(:filebucket).new :name => "foo", :path => "/my/file/bucket"
             catalog.add_resource bucket_resource
@@ -703,5 +703,52 @@ describe Puppet::Type.type(:file) do
 
             file.bucket.should == bucket_resource.bucket
         end
+
+        it "should have a nil filebucket if backup is false" do
+            catalog = Puppet::Resource::Catalog.new
+            bucket_resource = Puppet::Type.type(:filebucket).new :name => "foo", :path => "/my/file/bucket"
+            catalog.add_resource bucket_resource
+
+            file = Puppet::Type::File.new(:name => "/my/file", :backup => false)
+            catalog.add_resource file
+
+            file.bucket.should be_nil
+        end
+
+        it "should have a nil filebucket if backup is set to a string starting with '.'" do
+            catalog = Puppet::Resource::Catalog.new
+            bucket_resource = Puppet::Type.type(:filebucket).new :name => "foo", :path => "/my/file/bucket"
+            catalog.add_resource bucket_resource
+
+            file = Puppet::Type::File.new(:name => "/my/file", :backup => ".foo")
+            catalog.add_resource file
+
+            file.bucket.should be_nil
+        end
+
+        it "should fail if there's no catalog and backup is not false" do
+            file = Puppet::Type::File.new(:name => "/my/file", :backup => "foo")
+
+            lambda { file.bucket }.should raise_error(Puppet::Error)
+        end
+
+        it "should fail if a non-existent catalog is specified" do
+            file = Puppet::Type::File.new(:name => "/my/file", :backup => "foo")
+            catalog = Puppet::Resource::Catalog.new
+            catalog.add_resource file
+
+            lambda { file.bucket }.should raise_error(Puppet::Error)
+        end
+
+        it "should be able to use the default filebucket without a catalog" do
+            file = Puppet::Type::File.new(:name => "/my/file", :backup => "puppet")
+            file.bucket.should be_instance_of(Puppet::Network::Client::Dipper)
+        end
+
+        it "should look up the filebucket during finish()" do
+            file = Puppet::Type::File.new(:name => "/my/file", :backup => ".foo")
+            file.expects(:bucket)
+            file.finish
+        end
     end
 end