Random code cleanup

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

3 files changed, 50 insertions, 65 deletions

diff --git a/lib/puppet/property.rb b/lib/puppet/property.rb
index 753c0d7c6..e246eb702 100644
--- a/lib/puppet/property.rb
+++ b/lib/puppet/property.rb
@@ -152,8 +152,8 @@ class Puppet::Property < Puppet::Parameter
     end
 
     # Figure out which event to return.
-    def event(name, event = nil)
-        if value_event = self.class.value_option(name, :event)
+    def default_event_name(value, event = nil)
+        if value_event = self.class.value_option(value, :event)
             return value_event
         end
 
@@ -315,7 +315,7 @@ class Puppet::Property < Puppet::Parameter
             devfail "Cannot use obsolete :call value '%s' for property '%s'" % [call, self.class.name]
         end
 
-        return event(name, event)
+        return default_event_name(name, event)
     end
 
     # If there's a shadowing metaparam, instantiate it now.
diff --git a/lib/puppet/transaction.rb b/lib/puppet/transaction.rb
index 6671fb1f0..a5feecbc2 100644
--- a/lib/puppet/transaction.rb
+++ b/lib/puppet/transaction.rb
@@ -409,7 +409,7 @@ class Puppet::Transaction
         end
 
         if restarted
-            queue_events(Puppet::Transaction::Event.new(:restarted, resource))
+            queue_event(resource, Puppet::Transaction::Event.new(:restarted, resource))
 
             @resourcemetrics[:restarted] += 1
         end
@@ -417,12 +417,8 @@ class Puppet::Transaction
 
     # Queue events for other resources to respond to.  All of these events have
     # to be from the same resource.
-    def queue_events(*events)
-        events.flatten!
-
-        @events += events
-
-        resource = events[0].resource
+    def queue_event(resource, event)
+        @events << event
 
         # Collect the targets of any subscriptions to those events.  We pass
         # the parent resource in so it will override the source in the events,
@@ -431,20 +427,20 @@ class Puppet::Transaction
             next unless method = edge.callback
             next unless edge.target.respond_to?(method)
 
-            queue_events_for_resource(resource, edge.target, method, events)
+            queue_event_for_resource(resource, edge.target, method, event)
         end
 
         if resource.self_refresh? and ! resource.deleting?
-            queue_events_for_resource(resource, resource, :refresh, events)
+            queue_event_for_resource(resource, resource, :refresh, event)
         end
     end
 
-    def queue_events_for_resource(source, target, callback, events)
+    def queue_event_for_resource(source, target, callback, event)
         source.info "Scheduling #{callback} of #{target}"
 
         @event_queues[target] ||= {}
         @event_queues[target][callback] ||= []
-        @event_queues[target][callback] += events
+        @event_queues[target][callback] << event
     end
 
     def queued_events(resource)
@@ -461,13 +457,8 @@ class Puppet::Transaction
     # Roll all completed changes back.
     def rollback
         @changes.reverse.collect do |change|
-            # skip changes that were never actually run
-            unless change.changed
-                Puppet.debug "%s was not changed" % change.to_s
-                next
-            end
             begin
-                events = change.backward
+                event = change.backward
             rescue => detail
                 Puppet.err("%s rollback failed: %s" % [change,detail])
                 if Puppet[:trace]
@@ -483,7 +474,7 @@ class Puppet::Transaction
             end
 
             # And queue the events
-            queue_events(events)
+            queue_event(change.resource, event)
 
             # Now check to see if there are any events for this child.
             process_events(change.property.resource)
@@ -542,21 +533,14 @@ class Puppet::Transaction
     def apply_change(resource, change)
         @changes << change
 
-        # use an array, so that changes can return more than one
-        # event if they want
-        events = [change.forward].flatten.reject { |e| e.nil? }
+        event = change.forward
 
-        # Mark that our change happened, so it can be reversed
-        # if we ever get to that point
-        change.changed = true
-        @resourcemetrics[:applied] += 1
-        queue_events(events)
-    rescue => detail
-        puts detail.backtrace if Puppet[:trace]
-        is = change.property.is_to_s(change.is)
-        should = change.property.should_to_s(change.should)
-        change.property.err "change from #{is} to #{should} failed: #{detail}"
-        @failures[resource] += 1
+        if event.status == "success"
+            @resourcemetrics[:applied] += 1
+        else
+            @failures[resource] += 1
+        end
+        queue_event(resource, event)
     end
 
     def process_callback(resource, callback, events)
@@ -577,7 +561,7 @@ class Puppet::Transaction
         resource.notice "Would have triggered '#{callback}' from #{events.length} events"
 
         # And then add an event for it.
-        queue_events(Puppet::Transaction::Event.new(:noop, resource))
+        queue_event(resource, Puppet::Transaction::Event.new(:noop, resource))
         true # so the 'and if' works
     end
 end
diff --git a/lib/puppet/transaction/change.rb b/lib/puppet/transaction/change.rb
index b710ee08d..5e1aff106 100644
--- a/lib/puppet/transaction/change.rb
+++ b/lib/puppet/transaction/change.rb
@@ -4,7 +4,7 @@ require 'puppet/transaction/event'
 # Handle all of the work around performing an actual change,
 # including calling 'sync' on the properties and producing events.
 class Puppet::Transaction::Change
-    attr_accessor :is, :should, :path, :property, :changed, :proxy
+    attr_accessor :is, :should, :property, :proxy
 
     # Switch the goals of the property, thus running the change in reverse.
     def backward
@@ -15,26 +15,22 @@ class Puppet::Transaction::Change
         return self.go
     end
 
-    def changed?
-        self.changed
-    end
-
     # Create our event object.
-    def event(name)
+    def event(event_name)
+        event_name ||= property.default_event_name(should)
+
         # default to a simple event type
-        unless name.is_a?(Symbol)
-            @property.warning("Property '%s' returned invalid event '%s'; resetting to default" %
-                [@property.class, name])
+        unless event_name.is_a?(Symbol)
+            @property.warning "Property '#{property.class}' returned invalid event '#{event_name}'; resetting to default"
 
-            name = @property.event(should)
+            event_name = property.default_event_name(should)
         end
 
-        Puppet::Transaction::Event.new(name, self.resource)
+        Puppet::Transaction::Event.new(event_name, resource.ref, property.name, is, should)
     end
 
     def initialize(property, currentvalue)
         @property = property
-        @path = [property.path,"change"].flatten
         @is = currentvalue
 
         @should = property.should
@@ -47,25 +43,30 @@ class Puppet::Transaction::Change
     def go
         if self.noop?
             @property.log "is %s, should be %s (noop)" % [property.is_to_s(@is), property.should_to_s(@should)]
-            return [event(:noop)]
+            return event(:noop)
         end
 
         # The transaction catches any exceptions here.
-        events = @property.sync
-        if events.nil?
-            events = [(@property.name.to_s + "_changed").to_sym]
-        elsif events.is_a?(Array)
-            if events.empty?
-                events = [(@property.name.to_s + "_changed").to_sym]
-            end
-        else
-            events = [events]
-        end
-
-        return events.collect { |name|
-            @report = @property.log(@property.change_to_s(@is, @should))
-            event(name)
-        }
+        event_name = @property.sync
+
+        # Use the first event only, if multiple are provided.
+        # This might result in the event_name being nil,
+        # which is fine.
+        event_name = event_name.shift if event_name.is_a?(Array)
+
+        event = event(event_name)
+        event.log = @property.notice @property.change_to_s(@is, @should)
+        event.status = "success"
+        event
+    rescue => detail
+        puts detail.backtrace if Puppet[:trace]
+        event = event(nil)
+        event.status = "failure"
+
+        is = property.is_to_s(is)
+        should = property.should_to_s(should)
+        event.log = property.err "change from #{is} to #{should} failed: #{detail}"
+        event
     end
 
     def forward
@@ -79,7 +80,7 @@ class Puppet::Transaction::Change
 
     # The resource that generated this change.  This is used for handling events,
     # and the proxy resource is used for generated resources, since we can't
-    # send an event to a resource we don't have a direct relationship.  If we
+    # send an event to a resource we don't have a direct relationship with.  If we
     # have a proxy resource, then the events will be considered to be from
     # that resource, rather than us, so the graph resolution will still work.
     def resource