The whole system now uses Configuration objects instead of

ever converting the Transportable objects into a tree of components
and then converting that into a graph.  This is a significant
step, and drastically simplifies the model of how to use a configuration.
The old code might have looked something like this:

    file = Puppet::Type.create :path => "/whatever", ...
    comp = Puppet::Type.create :name => :whatever
    comp.push file
    transaction = comp.evaluate
    transaction.evaluate

The new code looks like this:

    file = Puppet::Type.create :path => "/whatever", ...
    config = Puppet::Node::Configuration.new
    config.add_resource file
    config.apply

I did not really intend to do this much refactoring, but I
found I could not use a Configuration object to do work
without refactoring a lot of the system.  The primary problem
was that the Client::Master and the Config classes determined
how the transactions behaved; when I moved to using a Configuration,
this distinction was lost, which meant that configurations were
often needing to create other configurations, which resulted in
a whole lot of infinite recursion (e.g., Config objects that create
directories for Puppet use Configuration objects -- yes, I'm
s/Config/Settings/g soon -- and these Configuration objects would
need to create directories).

Not everything is fixed, but it's very close.  I am clearly over
the hump, though, so I wanted to get a commit in.

4 files changed, 265 insertions, 21 deletions

diff --git a/spec/unit/node/configuration.rb b/spec/unit/node/configuration.rb
index 774a1550f..3c30d9b3e 100755
--- a/spec/unit/node/configuration.rb
+++ b/spec/unit/node/configuration.rb
@@ -134,25 +134,158 @@ describe Puppet::Node::Configuration, " when extracting transobjects" do
     end
 end
 
-describe Puppet::Node::Configuration, " functioning as a resource container" do
+describe Puppet::Node::Configuration, " when functioning as a resource container" do
     before do
-        @graph = Puppet::Node::Configuration.new("host")
+        @config = Puppet::Node::Configuration.new("host")
         @one = stub 'resource1', :ref => "Me[you]"
         @two = stub 'resource2', :ref => "Me[him]"
         @dupe = stub 'resource3', :ref => "Me[you]"
     end
 
     it "should make all vertices available by resource reference" do
-        @graph.add_resource(@one)
-        @graph.resource(@one.ref).should equal(@one)
+        @config.add_resource(@one)
+        @config.resource(@one.ref).should equal(@one)
+        @config.vertices.find { |r| r.ref == @one.ref }.should equal(@one)
     end
 
     it "should not allow two resources with the same resource reference" do
-        @graph.add_resource(@one)
-        proc { @graph.add_resource(@dupe) }.should raise_error(ArgumentError)
+        @config.add_resource(@one)
+        proc { @config.add_resource(@dupe) }.should raise_error(ArgumentError)
     end
 
     it "should not store objects that do not respond to :ref" do
-        proc { @graph.add_resource("thing") }.should raise_error(ArgumentError)
+        proc { @config.add_resource("thing") }.should raise_error(ArgumentError)
     end
+
+    it "should remove all resources when asked" do
+        @config.add_resource @one
+        @config.add_resource @two
+        @one.expects :remove
+        @two.expects :remove
+        @config.clear(true)
+    end
+
+    it "should support a mechanism for finishing resources" do
+        @one.expects :finish
+        @two.expects :finish
+        @config.add_resource @one
+        @config.add_resource @two
+
+        @config.finalize
+    end
+    
+    it "should optionally support an initialization block and should finalize after such blocks" do
+        @one.expects :finish
+        @two.expects :finish
+        config = Puppet::Node::Configuration.new("host") do |conf|
+            conf.add_resource @one
+            conf.add_resource @two
+        end
+    end
+end
+
+module ApplyingConfigurations
+    def setup
+        @config = Puppet::Node::Configuration.new("host")
+
+        @config.retrieval_duration = Time.now
+        @transaction = mock 'transaction'
+        Puppet::Transaction.stubs(:new).returns(@transaction)
+        @transaction.stubs(:evaluate)
+        @transaction.stubs(:cleanup)
+        @transaction.stubs(:addtimes)
+    end
+end
+
+describe Puppet::Node::Configuration, " when applying" do
+    include ApplyingConfigurations
+
+    it "should create and evaluate a transaction" do
+        @transaction.expects(:evaluate)
+        @config.apply
+    end
+
+    it "should provide the configuration time to the transaction" do
+        @transaction.expects(:addtimes).with do |arg|
+            arg[:config_retrieval].should be_instance_of(Time)
+            true
+        end
+        @config.apply
+    end
+
+    it "should clean up the transaction" do
+        @transaction.expects :cleanup
+        @config.apply
+    end
+    
+    it "should return the transaction" do
+        @config.apply.should equal(@transaction)
+    end
+
+    it "should yield the transaction if a block is provided" do
+        pending "the code works but is not tested"
+    end
+    
+    it "should default to not being a host configuration" do
+        @config.host_config.should be_nil
+    end
+end
+
+describe Puppet::Node::Configuration, " when applying host configurations" do
+    include ApplyingConfigurations
+
+    # super() doesn't work in the setup method for some reason
+    before do
+        @config.host_config = true
+    end
+
+    it "should send a report if reporting is enabled" do
+        Puppet[:report] = true
+        @transaction.expects :send_report
+        @config.apply
+    end
+
+    it "should send a report if report summaries are enabled" do
+        Puppet[:summarize] = true
+        @transaction.expects :send_report
+        @config.apply
+    end
+
+    it "should initialize the state database before applying a configuration" do
+        Puppet::Util::Storage.expects(:load)
+
+        # Short-circuit the apply, so we know we're loading before the transaction
+        Puppet::Transaction.expects(:new).raises ArgumentError
+        proc { @config.apply }.should raise_error(ArgumentError)
+    end
+
+    it "should sync the state database after applying" do
+        Puppet::Util::Storage.expects(:store)
+        @config.apply
+    end
+
+    after { Puppet.config.clear }
+end
+
+describe Puppet::Node::Configuration, " when applying non-host configurations" do
+    include ApplyingConfigurations
+
+    before do
+        @config.host_config = false
+    end
+    
+    it "should never send reports" do
+        Puppet[:report] = true
+        Puppet[:summarize] = true
+        @transaction.expects(:send_report).never
+        @config.apply
+    end
+
+    it "should never modify the state database" do
+        Puppet::Util::Storage.expects(:load).never
+        Puppet::Util::Storage.expects(:store).never
+        @config.apply
+    end
+
+    after { Puppet.config.clear }
 end
diff --git a/spec/unit/node/node.rb b/spec/unit/node/node.rb
index 9342dc5ce..2f63c253d 100755
--- a/spec/unit/node/node.rb
+++ b/spec/unit/node/node.rb
@@ -11,6 +11,10 @@ describe Puppet::Node, " when initializing" do
         @node.name.should == "testnode"
     end
 
+    it "should not allow nil node names" do
+        proc { Puppet::Node.new(nil) }.should raise_error(ArgumentError)
+    end
+
     it "should default to an empty parameter hash" do
         @node.parameters.should == {}
     end
diff --git a/spec/unit/other/transbucket.rb b/spec/unit/other/transbucket.rb
index c013973ee..8cb9abaa4 100755
--- a/spec/unit/other/transbucket.rb
+++ b/spec/unit/other/transbucket.rb
@@ -48,7 +48,7 @@ describe Puppet::TransBucket do
     end
 end
 
-describe Puppet::TransBucket, " when generating a resource graph" do
+describe Puppet::TransBucket, " when generating a configuration" do
     before do
         @bottom = Puppet::TransBucket.new
         @bottom.type = "fake"
@@ -70,7 +70,7 @@ describe Puppet::TransBucket, " when generating a resource graph" do
         @top.push(@topobj)
         @top.push(@middle)
 
-        @graph = @top.to_graph
+        @config = @top.to_configuration
 
         @users = %w{top middle bottom}
         @fakes = %w{fake[bottom] fake[middle] fake[top]}
@@ -78,18 +78,28 @@ describe Puppet::TransBucket, " when generating a resource graph" do
 
     it "should convert all transportable objects to RAL resources" do
         @users.each do |name|
-            @graph.vertices.find { |r| r.class.name == :user and r.title == name }.should be_instance_of(Puppet::Type.type(:user))
+            @config.vertices.find { |r| r.class.name == :user and r.title == name }.should be_instance_of(Puppet::Type.type(:user))
         end
     end
 
     it "should convert all transportable buckets to RAL components" do
         @fakes.each do |name|
-            @graph.vertices.find { |r| r.class.name == :component and r.title == name }.should be_instance_of(Puppet::Type.type(:component))
+            @config.vertices.find { |r| r.class.name == :component and r.title == name }.should be_instance_of(Puppet::Type.type(:component))
         end
     end
 
     it "should add all resources to the graph's resource table" do
-        @graph.resource("fake[top]").should equal(@top)
+        @config.resource("fake[top]").should equal(@top)
+    end
+
+    it "should finalize all resources" do
+        @config.vertices.each do |vertex| vertex.should be_finalized end
+    end
+
+    it "should only call to_type on each resource once" do
+        @topobj.expects(:to_type)
+        @bottomobj.expects(:to_type)
+        @top.to_configuration
     end
 
     after do
diff --git a/spec/unit/util/config.rb b/spec/unit/util/config.rb
index 28ccb04d7..ff9e28d3b 100755
--- a/spec/unit/util/config.rb
+++ b/spec/unit/util/config.rb
@@ -299,7 +299,7 @@ describe Puppet::Util::Config, " when parsing its configuration" do
     end
 
     it "should support specifying file all metadata (owner, group, mode) in the configuration file" do
-        @config.setdefaults :section, :myfile => ["/my/file", "a"]
+        @config.setdefaults :section, :myfile => ["/myfile", "a"]
 
         text = "[main]
         myfile = /other/file {owner = luke, group = luke, mode = 644}
@@ -312,7 +312,7 @@ describe Puppet::Util::Config, " when parsing its configuration" do
     end
 
     it "should support specifying file a single piece of metadata (owner, group, or mode) in the configuration file" do
-        @config.setdefaults :section, :myfile => ["/my/file", "a"]
+        @config.setdefaults :section, :myfile => ["/myfile", "a"]
 
         text = "[main]
         myfile = /other/file {owner = luke}
@@ -388,16 +388,34 @@ describe Puppet::Util::Config, " when reparsing its configuration" do
 end
 
 describe Puppet::Util::Config, " when being used to manage the host machine" do
+    before do
+        @config = Puppet::Util::Config.new
+        @config.setdefaults :main, :maindir => ["/maindir", "a"], :seconddir => ["/seconddir", "a"]
+        @config.setdefaults :other, :otherdir => {:default => "/otherdir", :desc => "a", :owner => "luke", :group => "johnny", :mode => 0755}
+        @config.setdefaults :files, :myfile => {:default => "/myfile", :desc => "a", :mode => 0755}
+    end
+
     it "should provide a method that writes files with the correct modes" do
         pending "Not converted from test/unit yet"
     end
 
     it "should provide a method that creates directories with the correct modes" do
-        pending "Not converted from test/unit yet"
+        Puppet::Util::SUIDManager.expects(:asuser).with("luke", "johnny").yields
+        Dir.expects(:mkdir).with("/otherdir", 0755)
+        @config.mkdir(:otherdir)
     end
 
-    it "should provide a method to declare what directories should exist" do
-        pending "Not converted from test/unit yet"
+    it "should be able to create needed directories in a single section" do
+        Dir.expects(:mkdir).with("/maindir")
+        Dir.expects(:mkdir).with("/seconddir")
+        @config.use(:main)
+    end
+
+    it "should be able to create needed directories in multiple sections" do
+        Dir.expects(:mkdir).with("/maindir")
+        Dir.expects(:mkdir).with("/otherdir", 0755)
+        Dir.expects(:mkdir).with("/seconddir")
+        @config.use(:main, :other)
     end
 
     it "should provide a method to trigger enforcing of file modes on existing files and directories" do
@@ -408,13 +426,85 @@ describe Puppet::Util::Config, " when being used to manage the host machine" do
         pending "Not converted from test/unit yet"
     end
 
-    it "should provide an option to create needed users and groups" do
+    it "should provide a method to convert the file mode enforcement into transportable resources" do
+        # Make it think we're root so it tries to manage user and group.
+        Puppet.features.stubs(:root?).returns(true)
+        File.stubs(:exist?).with("/myfile").returns(true)
+        trans = nil
+        trans = @config.to_transportable
+        resources = []
+        trans.delve { |obj| resources << obj if obj.is_a? Puppet::TransObject }
+        %w{/maindir /seconddir /otherdir /myfile}.each do |path|
+            obj = resources.find { |r| r.type == "file" and r.name == path }
+            if path.include?("dir")
+                obj[:ensure].should == :directory
+            else
+                # Do not create the file, just manage mode
+                obj[:ensure].should be_nil
+            end
+            obj.should be_instance_of(Puppet::TransObject)
+            case path
+            when "/otherdir":
+                obj[:owner].should == "luke"
+                obj[:group].should == "johnny"
+                obj[:mode].should == 0755
+            when "/myfile":
+                obj[:mode].should == 0755
+            end
+        end
+    end
+
+    it "should not try to manage user or group when not running as root" do
+        Puppet.features.stubs(:root?).returns(false)
+        trans = nil
+        trans = @config.to_transportable(:other)
+        trans.delve do |obj|
+            next unless obj.is_a?(Puppet::TransObject)
+            obj[:owner].should be_nil
+            obj[:group].should be_nil
+        end
+    end
+
+    it "should add needed users and groups to the manifest when asked" do
+        # This is how we enable user/group management
+        @config.setdefaults :main, :mkusers => [true, "w"]
+        Puppet.features.stubs(:root?).returns(false)
+        trans = nil
+        trans = @config.to_transportable(:other)
+        resources = []
+        trans.delve { |obj| resources << obj if obj.is_a? Puppet::TransObject and obj.type != "file" }
+
+        user = resources.find { |r| r.type == "user" }
+        user.should be_instance_of(Puppet::TransObject)
+        user.name.should == "luke"
+        user[:ensure].should == :present
+
+        # This should maybe be a separate test, but...
+        group = resources.find { |r| r.type == "group" }
+        group.should be_instance_of(Puppet::TransObject)
+        group.name.should == "johnny"
+        group[:ensure].should == :present
+    end
+
+    it "should ignore tags and schedules when creating files and directories"
+
+    it "should apply all resources in debug mode to reduce logging"
+
+    it "should not try to manage absent files" do
+        # Make it think we're root so it tries to manage user and group.
+        Puppet.features.stubs(:root?).returns(true)
+        trans = nil
+        trans = @config.to_transportable
+        file = nil
+        trans.delve { |obj| file = obj if obj.name == "/myfile" }
+        file.should be_nil
+    end
+
+    it "should be able to turn the current configuration into a parseable manifest" do
         pending "Not converted from test/unit yet"
     end
 
-    it "should provide a method to print out the current configuration" do
-        pending "Not converted from test/unit yet"
-    end
+    it "should convert octal numbers correctly when producing a manifest"
 
     it "should be able to provide all of its parameters in a format compatible with GetOpt::Long" do
         pending "Not converted from test/unit yet"
@@ -423,4 +513,11 @@ describe Puppet::Util::Config, " when being used to manage the host machine" do
     it "should not attempt to manage files within /dev" do
         pending "Not converted from test/unit yet"
     end
+
+    it "should not modify the stored state database when managing resources" do
+        Puppet::Util::Storage.expects(:store).never
+        Puppet::Util::Storage.expects(:load).never
+        Dir.expects(:mkdir).with("/maindir")
+        @config.use(:main)
+    end
 end