fixing component flattening and sorting; it was not working for cases where objects inside of components had dependencies from other components

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

2 files changed, 120 insertions, 25 deletions

diff --git a/lib/puppet/type/component.rb b/lib/puppet/type/component.rb
index d6c53fe18..345afb75b 100644
--- a/lib/puppet/type/component.rb
+++ b/lib/puppet/type/component.rb
@@ -23,23 +23,33 @@ module Puppet
         # topo sort functions
         def self.sort(objects)
             list = []
-            inlist = {}
+            tmplist = {}
 
             objects.each { |obj|
-                self.recurse(obj, inlist, list)
+                self.recurse(obj, tmplist, list)
             }
 
-            return list
+            return list.flatten
         end
 
+        # FIXME this method assumes that dependencies themselves
+        # are never components
         def self.recurse(obj, inlist, list)
-            return if inlist.include?(obj.object_id)
+            if inlist.include?(obj.object_id)
+                return
+            end
+            inlist[obj.object_id] = true
             obj.eachdependency { |req|
                 self.recurse(req, inlist, list)
             }
-            
-            list << obj
-            inlist[obj.object_id] = true
+
+            if obj.is_a?(Puppet::Type::Component)
+                obj.each { |child|
+                    self.recurse(child, inlist, list)
+                }
+            else
+                list << obj
+            end
         end
 
         def each
@@ -48,13 +58,7 @@ module Puppet
         
         # this returns a sorted array, not a new component, but that suits me just fine
         def flatten
-            self.class.sort(@children.collect { |child|
-                if child.is_a?(self.class)
-                    child.flatten
-                else
-                    child
-                end
-            }.flatten)
+            self.class.sort(@children).flatten
         end
 
         def initialize(args)
@@ -69,14 +73,6 @@ module Puppet
             #    [self.name, self[:type]]
         end
 
-        # the "old" way of doing things
-        # just turn the container into a transaction
-        def oldevaluate
-            transaction = Puppet::Transaction.new(@children)
-            transaction.component = self
-            return transaction
-        end
-
         # flatten all children, sort them, and evaluate them in order
         # this is only called on one component over the whole system
         # this also won't work with scheduling, but eh
diff --git a/test/types/tc_component.rb b/test/types/tc_component.rb
index 0582ee6f8..5fcee859c 100755
--- a/test/types/tc_component.rb
+++ b/test/types/tc_component.rb
@@ -29,7 +29,7 @@ class TestComponent < TestPuppet
         loop do
             looped += 1
             if looped > 1000
-                $stderr.print "Reached limit of looping"
+                raise "Reached limit of looping"
                 break
             end
             num = rand(limit)
@@ -57,7 +57,7 @@ class TestComponent < TestPuppet
     end
 
     def mkcomp
-        comp = Puppet::Type::Component.new(:name => "component_" + randnum(1000).to_s)
+        Puppet::Type::Component.new(:name => "component_" + randnum(1000).to_s)
     end
 
     def mkrandcomp(numfiles, numdivs)
@@ -66,6 +66,7 @@ class TestComponent < TestPuppet
         found = 0
 
         divs = {}
+
         numdivs.times { |i|
             num = i + 2
             divs[num] = nil
@@ -164,7 +165,7 @@ class TestComponent < TestPuppet
         }
 
         comp = Puppet::Type::Component.new(:name => "RefreshTest")
-        [file,cmd].each { |obj|
+        [cmd, file].each { |obj|
             comp.push obj
         }
         objects = nil
@@ -175,5 +176,103 @@ class TestComponent < TestPuppet
         [cmd, file].each { |obj|
             assert_equal(1, objects.find_all { |o| o.name == obj.name }.length)
         }
+
+        assert(objects[0] == file, "File was not first object")
+        assert(objects[1] == cmd, "Exec was not second object")
+    end
+
+    def test_deepflatten
+        tmpfile = tempfile()
+        @@tmpfiles.push tmpfile
+        trans = nil
+        cmd = nil
+        File.open(tmpfile, File::WRONLY|File::CREAT|File::TRUNC) { |of|
+            of.puts rand(100)
+        }
+        file = Puppet::Type::PFile.new(
+            :path => tmpfile,
+            :checksum => "md5"
+        )
+        assert_nothing_raised {
+            cmd = Puppet::Type::Exec.new(
+                :command => "pwd",
+                :path => "/usr/bin:/bin:/usr/sbin:/sbin",
+                :refreshonly => true
+            )
+        }
+
+        fcomp = newcomp("fflatten", file)
+        ecomp = newcomp("eflatten", cmd)
+
+        # this subscription can screw up the sorting
+        ecomp[:subscribe] = [[fcomp.class.name,fcomp.name]]
+
+        comp = newcomp("bflatten", ecomp, fcomp)
+        objects = nil
+        assert_nothing_raised {
+            objects = comp.flatten
+        }
+
+        assert_equal(objects.length, 2, "Did not get two sorted objects")
+        objects.each { |o|
+            assert(o.is_a?(Puppet::Type), "Object %s is not a Type" % o.class)
+        }
+
+        assert(objects[0] == file, "File was not first object")
+        assert(objects[1] == cmd, "Exec was not second object")
+    end
+
+    def test_deepflatten2
+        tmpfile = tempfile()
+        @@tmpfiles.push tmpfile
+        trans = nil
+        cmd = nil
+        File.open(tmpfile, File::WRONLY|File::CREAT|File::TRUNC) { |of|
+            of.puts rand(100)
+        }
+        file = Puppet::Type::PFile.new(
+            :path => tmpfile,
+            :checksum => "md5"
+        )
+        assert_nothing_raised {
+            cmd = Puppet::Type::Exec.new(
+                :command => "pwd",
+                :path => "/usr/bin:/bin:/usr/sbin:/sbin",
+                :refreshonly => true
+            )
+        }
+
+        ocmd = nil
+        assert_nothing_raised {
+            ocmd = Puppet::Type::Exec.new(
+                :command => "echo true",
+                :path => "/usr/bin:/bin:/usr/sbin:/sbin",
+                :refreshonly => true
+            )
+        }
+
+        fcomp = newcomp("fflatten", file)
+        ecomp = newcomp("eflatten", cmd)
+        ocomp = newcomp("oflatten", ocmd)
+
+        # this subscription can screw up the sorting
+        cmd[:subscribe] = [[fcomp.class.name,fcomp.name]]
+        ocmd[:subscribe] = [[cmd.class.name,cmd.name]]
+
+        comp = newcomp("bflatten", ocomp, ecomp, fcomp)
+        objects = nil
+        assert_nothing_raised {
+            objects = comp.flatten
+        }
+
+        assert_equal(objects.length, 3, "Did not get three sorted objects")
+
+        objects.each { |o|
+            assert(o.is_a?(Puppet::Type), "Object %s is not a Type" % o.class)
+        }
+
+        assert(objects[0] == file, "File was not first object")
+        assert(objects[1] == cmd, "Exec was not second object")
+        assert(objects[2] == ocmd, "Other exec was not second object")
     end
 end