(6911) Cleanup and renaming of transaction internals

The preceeding changes left some rough edges in the Transactions (a short,
badly named method that was only used in one place and would be clearer in-
line, a return value that was carfully retained and never used, etc.) This
commit clears some of that up.

2 files changed, 15 insertions, 32 deletions

diff --git a/lib/puppet/transaction.rb b/lib/puppet/transaction.rb
index 79fdb044c..c502fc627 100644
--- a/lib/puppet/transaction.rb
+++ b/lib/puppet/transaction.rb
@@ -80,18 +80,14 @@ class Puppet::Transaction
     if skip?(resource)
       resource_status(resource).skipped = true
     else
-      eval_children_and_apply_resource(resource, ancestor)
+      resource_status(resource).scheduled = true
+      apply(resource, ancestor)
     end
 
     # Check to see if there are any events queued for this resource
     event_manager.process_events(resource)
   end
 
-  def eval_children_and_apply_resource(resource, ancestor = nil)
-    resource_status(resource).scheduled = true
-    apply(resource, ancestor)
-  end
-
   # This method does all the actual work of running a transaction.  It
   # collects all of the changes, executes them, and responds to any
   # necessary events.
@@ -105,18 +101,12 @@ class Puppet::Transaction
 
     begin
       relationship_graph.traverse do |resource|
-        next if stop_processing?
         if resource.is_a?(Puppet::Type::Component)
           Puppet.warning "Somehow left a component in the relationship graph"
-          next
-        end
-        ret = nil
-        seconds = thinmark do
-          ret = eval_resource(resource)
+        else
+          seconds = thinmark { eval_resource(resource) }
+          resource.info "Evaluated in %0.2f seconds" % seconds if Puppet[:evaltrace] and @catalog.host_config?
         end
-
-        resource.info "Evaluated in %0.2f seconds" % seconds if Puppet[:evaltrace] and @catalog.host_config?
-        ret
       end
     ensure
       # And then close the transaction log.
@@ -280,13 +270,14 @@ class Puppet::Transaction
   # except via the Transaction#relationship_graph
 
   class Relationship_graph_wrapper
-    attr_reader :real_graph,:transaction,:ready,:generated,:done
+    attr_reader :real_graph,:transaction,:ready,:generated,:done,:unguessable_deterministic_key
     def initialize(real_graph,transaction)
       @real_graph = real_graph
       @transaction = transaction
       @ready = {}
       @generated = {}
       @done = {}
+      @unguessable_deterministic_key = Hash.new { |h,k| h[k] = Digest::SHA1.hexdigest("NaCl, MgSO4 (salts) and then #{k.title}") }
       vertices.each { |v| check_if_now_ready(v) }
     end
     def method_missing(*args,&block)
@@ -301,10 +292,12 @@ class Puppet::Transaction
     end
     def check_if_now_ready(r)
       ready[r] = true if direct_dependencies_of(r).all? { |r2| done[r2] }
-    end  
+    end
+    def next_resource
+      ready.keys.sort_by { |r0| unguessable_deterministic_key[r0] }.first
+    end
     def traverse(&block)
-      unguessable_deterministic_key = Hash.new { |h,k| h[k] = Digest::SHA1.hexdigest("NaCl, MgSO4 (salts) and then #{k.title}") }
-      while r = ready.keys.sort_by { |r0| unguessable_deterministic_key[r0] }.first
+      while (r = next_resource) && !transaction.stop_processing?
         if !generated[r]
           transaction.eval_generate(r)
           generated[r] = true
diff --git a/spec/unit/transaction_spec.rb b/spec/unit/transaction_spec.rb
index 076b626bb..8d40136fb 100755
--- a/spec/unit/transaction_spec.rb
+++ b/spec/unit/transaction_spec.rb
@@ -114,7 +114,6 @@ describe Puppet::Transaction do
   describe "when evaluating a resource" do
     before do
       @transaction = Puppet::Transaction.new(Puppet::Resource::Catalog.new)
-      @transaction.stubs(:eval_children_and_apply_resource)
       @transaction.stubs(:skip?).returns false
 
       @resource = Puppet::Type.type(:file).new :path => @basepath
@@ -126,12 +125,6 @@ describe Puppet::Transaction do
       @transaction.eval_resource(@resource)
     end
 
-    it "should eval and apply children" do
-      @transaction.expects(:eval_children_and_apply_resource).with(@resource, nil)
-
-      @transaction.eval_resource(@resource)
-    end
-
     it "should process events" do
       @transaction.event_manager.expects(:process_events).with(@resource)
 
@@ -389,22 +382,19 @@ describe Puppet::Transaction do
 
     describe 'within an evaluate call' do
       before do
-        @resource = stub 'resource', :ref => 'some_ref'
-        @relationship_graph = stub 'relationship_graph'
+        @resource = Puppet::Type.type(:notify).new :title => "foobar"
         @catalog.add_resource @resource
         @transaction.stubs(:prepare)
-        @transaction.stubs(:relationship_graph).returns @relationship_graph
-        @relationship_graph.stubs(:traverse).yields @resource
       end
 
       it 'should stop processing if :stop_processing? is true' do
-        @transaction.expects(:stop_processing?).returns(true)
+        @transaction.stubs(:stop_processing?).returns(true)
         @transaction.expects(:eval_resource).never
         @transaction.evaluate
       end
 
       it 'should continue processing if :stop_processing? is false' do
-        @transaction.expects(:stop_processing?).returns(false)
+        @transaction.stubs(:stop_processing?).returns(false)
         @transaction.expects(:eval_resource).returns(nil)
         @transaction.evaluate
       end