Fixing the next round of bugs, mostly little things but I had to modify transactions so they are willing to delete implicit resources even if they have dependencies, else we would often not be able to purge files at all.

git-svn-id: https://reductivelabs.com/svn/puppet/trunk@1917 980ebf18-57e1-0310-9a29-db15c13687c0

8 files changed, 65 insertions, 26 deletions

diff --git a/lib/puppet/pgraph.rb b/lib/puppet/pgraph.rb
index a8ff952ed..e7247418c 100644
--- a/lib/puppet/pgraph.rb
+++ b/lib/puppet/pgraph.rb
@@ -20,10 +20,15 @@ class Puppet::PGraph < GRATR::Digraph
             @edge_number.clear
         end
     end
+
+    # Which resources a given resource depends upon.
+    def dependents(resource)
+        tree_from_vertex(resource, :dfs).keys
+    end
     
-    # The dependencies for a given resource.
+    # The which resources depend upon the given resource.
     def dependencies(resource)
-        tree_from_vertex(resource, :dfs).keys
+        reversal.tree_from_vertex(resource, :dfs).keys
     end
     
     # Override this method to use our class instead.
diff --git a/lib/puppet/transaction.rb b/lib/puppet/transaction.rb
index 99b03b435..67d016c15 100644
--- a/lib/puppet/transaction.rb
+++ b/lib/puppet/transaction.rb
@@ -44,6 +44,16 @@ class Transaction
             # And then return
             return []
         end
+        
+        # If a resource is going to be deleted but it still has dependencies, then
+        # don't delete it unless it's implicit.
+        if ! resource.implicit? and deps = @relgraph.dependents(resource) and ! deps.empty? and changes.detect { |change|
+            change.state.name == :ensure and change.should == :absent
+        }
+            resource.warning "%s still depend%s on me -- not deleting" %
+                [deps.collect { |r| r.ref }.join(","), if deps.length > 1; ""; else "s"; end] 
+            return []
+        end
 
         unless changes.is_a? Array
             changes = [changes]
@@ -237,7 +247,7 @@ class Transaction
         # enough to check the immediate dependencies, which is why we use
         # a tree from the reversed graph.
         skip = false
-        @relgraph.reversal.tree_from_vertex(resource, :dfs).keys.each do |dep|
+        @relgraph.dependencies(resource).each do |dep|
             if fails = failed?(dep)
                 resource.notice "Dependency %s[%s] has %s failures" %
                     [dep.class.name, dep.name, @failures[dep]]
diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb
index 2c99e3343..e55f9f5ae 100644
--- a/lib/puppet/type.rb
+++ b/lib/puppet/type.rb
@@ -334,7 +334,7 @@ class Type < Puppet::Element
 
     # Return the "type[name]" style reference.
     def ref
-        "%s[%s]" % [self.class.name, self.title]
+        "%s[%s]" % [self.class.name.to_s.capitalize, self.title]
     end
 
     # Retrieve the title of an object.  If no title was set separately,
diff --git a/test/other/events.rb b/test/other/events.rb
index 10099fa46..078e6351f 100755
--- a/test/other/events.rb
+++ b/test/other/events.rb
@@ -146,7 +146,8 @@ class TestEvents < Test::Unit::TestCase
         exec3 = Puppet.type(:exec).create(
             :title => "three",
             :name => "echo three >> %s" % file,
-            :path => "/usr/bin:/bin"
+            :path => "/usr/bin:/bin",
+            :require => exec2
         )
         execs = [exec1, exec2, exec3]
 
diff --git a/test/other/pgraph.rb b/test/other/pgraph.rb
index 1ef853cc4..5ee76bfe9 100644
--- a/test/other/pgraph.rb
+++ b/test/other/pgraph.rb
@@ -58,9 +58,15 @@ class TestPGraph < Test::Unit::TestCase
         graph.add_edge!("a", "c")
         graph.add_edge!("b", "d")
         
-        assert_equal(%w{b c d}.sort, graph.dependencies("a").sort)
-        assert_equal(%w{d}.sort, graph.dependencies("b").sort)
-        assert_equal([].sort, graph.dependencies("c").sort)
+        assert_equal(%w{b c d}.sort, graph.dependents("a").sort)
+        assert_equal(%w{d}.sort, graph.dependents("b").sort)
+        assert_equal([].sort, graph.dependents("c").sort)
+        
+        assert_equal(%w{a b}, graph.dependencies("d").sort)
+        assert_equal(%w{a}, graph.dependencies("b").sort)
+        assert_equal(%w{a}, graph.dependencies("c").sort)
+        assert_equal([], graph.dependencies("a").sort)
+        
     end
     
     # Test that we can take a containment graph and rearrange it by dependencies
@@ -110,17 +116,11 @@ class TestPGraph < Test::Unit::TestCase
         assert(nons.empty?,
             "still contain non-strings %s" % nons.inspect)
         
-        deps.to_jpg("deps-after")
-        
         deps.edges.each do |edge|
             assert_equal({:callback => :refresh}, edge.label,
                 "Label was not copied on splice")
         end
     end
-    
-    # Make sure empty containers are also removed
-    def test_empty_splice
-    end
 end
 
 # $Id$
diff --git a/test/other/transactions.rb b/test/other/transactions.rb
index 4190ada3a..256c815f7 100755
--- a/test/other/transactions.rb
+++ b/test/other/transactions.rb
@@ -760,11 +760,26 @@ class TestTransactions < Test::Unit::TestCase
         
         trans.tags = %w{mytag yaytag}
         assert(trans.tagged?(res), "tags should have matched")
-        
     end
     
-    # Make sure events propagate down the relationship graph appropriately.
-    def test_trigger
+    # We don't want to purge resources that have relationships with other resources,
+    # so we want our transactions to check for that.
+    def test_required_resources_not_deleted
+        @file = Puppet::Type.type(:file)
+        path1 = tempfile()
+        path2 = tempfile()
+        
+        # Create our first file
+        File.open(path1, "w") { |f| f.puts "yay" }
+        
+        # Create a couple of related resources
+        file1 = @file.create :title => "dependee", :path => path1, :ensure => :absent
+        file2 = @file.create :title => "depender", :path => path2, :content => "some stuff", :require => file1
+                
+        # Now make sure we don't actually delete the first file
+        assert_apply(file1, file2)
+        
+        assert(FileTest.exists?(path1), "required file was deleted")
     end
 end
 
diff --git a/test/types/file.rb b/test/types/file.rb
index c57129cd5..917b2e755 100755
--- a/test/types/file.rb
+++ b/test/types/file.rb
@@ -1559,30 +1559,38 @@ class TestFile < Test::Unit::TestCase
         sourcefile = File.join(sourcedir, "sourcefile")
         dsourcefile = File.join(destdir, "sourcefile")
         localfile = File.join(destdir, "localfile")
-        randfile = File.join(destdir, "random")
+        purgee = File.join(destdir, "to_be_purged")
         File.open(sourcefile, "w") { |f| f.puts "funtest" }
         # this file should get removed
-        File.open(randfile, "w") { |f| f.puts "footest" }
+        File.open(purgee, "w") { |f| f.puts "footest" }
 
-        lfobj = Puppet::Type.newfile(:path => localfile, :content => "rahtest")
+        lfobj = Puppet::Type.newfile(:title => "localfile", :path => localfile, :content => "rahtest")
         
 
-        destobj = Puppet::Type.newfile(:path => destdir,
+        destobj = Puppet::Type.newfile(:title => "destdir", :path => destdir,
                                     :source => sourcedir,
                                     :recurse => true)
 
-        assert_apply(lfobj, destobj)
+        puts "a"
+        comp = newcomp(lfobj, destobj)
+        # trans = comp.evaluate
+        assert_apply(comp)
+        # assert_nothing_raised { trans.evaluate }
+        puts "b"
+        # graph = trans.relgraph
+        # graph.to_jpg("/Users/luke/Desktop/pics", "purging")
 
         assert(FileTest.exists?(dsourcefile), "File did not get copied")
         assert(FileTest.exists?(localfile), "File did not get created")
-        assert(FileTest.exists?(randfile), "File got prematurely purged")
+        assert(FileTest.exists?(purgee), "File got prematurely purged")
 
         assert_nothing_raised { destobj[:purge] = true }
-        assert_apply(lfobj, destobj)
+        assert_apply(comp)
+        system("find %s" % destdir)
 
         assert(FileTest.exists?(dsourcefile), "File got purged")
         assert(FileTest.exists?(localfile), "File got purged")
-        assert(! FileTest.exists?(randfile), "File did not get purged")
+        assert(! FileTest.exists?(purgee), "File did not get purged")
     end
 
     # Testing #274.  Make sure target can be used without 'ensure'.
diff --git a/test/types/type.rb b/test/types/type.rb
index ade0275b8..aa37f2a18 100755
--- a/test/types/type.rb
+++ b/test/types/type.rb
@@ -753,7 +753,7 @@ end
     def test_ref
         path = tempfile()
         file = Puppet::Type.newfile(:path => path)
-        assert_equal("file[#{path}]", file.ref)
+        assert_equal("File[#{path}]", file.ref)
 
         exec = Puppet::Type.newexec(:title => "yay", :command => "/bin/echo yay")
         assert_equal("exec[yay]", exec.ref)