Cleaning up Event creation

The Property class is now completely responsible
for creating the event, and it adds all of the metadata
that a log message would normally have.  This provides
a cleaner definition of responsibility, and will allow
further cleaning up in later commits.

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

3 files changed, 65 insertions, 57 deletions

diff --git a/lib/puppet/property.rb b/lib/puppet/property.rb
index e246eb702..23b9d780a 100644
--- a/lib/puppet/property.rb
+++ b/lib/puppet/property.rb
@@ -152,31 +152,25 @@ class Puppet::Property < Puppet::Parameter
     end
 
     # Figure out which event to return.
-    def default_event_name(value, event = nil)
-        if value_event = self.class.value_option(value, :event)
-            return value_event
-        end
+    def event_name
+        value = self.should
 
-        if event and event.is_a?(Symbol)
-            if event == :nochange
-                return nil
-            else
-                return event
-            end
-        end
+        event_name = self.class.value_option(value, :event) and return event_name
 
-        if self.class.name == :ensure
-            event = case self.should
-            when :present; (@resource.class.name.to_s + "_created").intern
-            when :absent; (@resource.class.name.to_s + "_removed").intern
-            else
-                (@resource.class.name.to_s + "_changed").intern
-            end
+        name == :ensure or return (name.to_s + "_changed").to_sym
+
+        return (resource.type.to_s + case value
+        when :present; "_created"
+        when :absent; "_removed"
         else
-            event = (@resource.class.name.to_s + "_changed").intern
-        end
+            "_changed"
+        end).to_sym
+    end
 
-        return event
+    # Create our event object.
+    def event
+        Puppet::Transaction::Event.new(:name => event_name, :resource => resource.ref, :desired_value => should,
+            :file => file, :line => line, :tags => tags, :property => name, :version => version)
     end
 
     attr_reader :shadow
@@ -314,8 +308,6 @@ class Puppet::Property < Puppet::Parameter
             # do so in the block.
             devfail "Cannot use obsolete :call value '%s' for property '%s'" % [call, self.class.name]
         end
-
-        return default_event_name(name, event)
     end
 
     # If there's a shadowing metaparam, instantiate it now.
diff --git a/lib/puppet/transaction/change.rb b/lib/puppet/transaction/change.rb
index 5e1aff106..49569d6e3 100644
--- a/lib/puppet/transaction/change.rb
+++ b/lib/puppet/transaction/change.rb
@@ -16,17 +16,11 @@ class Puppet::Transaction::Change
     end
 
     # Create our event object.
-    def event(event_name)
-        event_name ||= property.default_event_name(should)
-
-        # default to a simple event type
-        unless event_name.is_a?(Symbol)
-            @property.warning "Property '#{property.class}' returned invalid event '#{event_name}'; resetting to default"
-
-            event_name = property.default_event_name(should)
-        end
-
-        Puppet::Transaction::Event.new(event_name, resource.ref, property.name, is, should)
+    def event
+        result = property.event
+        result.previous_value = is
+        result.desired_value = should
+        result
     end
 
     def initialize(property, currentvalue)
@@ -41,32 +35,23 @@ class Puppet::Transaction::Change
     # Perform the actual change.  This method can go either forward or
     # backward, and produces an event.
     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)
-        end
-
-        # The transaction catches any exceptions here.
-        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
+        return noop_event if noop?
+
+        property.sync
+
+        result = event()
+        result.log = property.notice property.change_to_s(is, should)
+        result.status = "success"
+        result
     rescue => detail
         puts detail.backtrace if Puppet[:trace]
-        event = event(nil)
-        event.status = "failure"
+        result = event()
+        result.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
+        result.log = property.err "change from #{is} to #{should} failed: #{detail}"
+        result
     end
 
     def forward
@@ -90,4 +75,13 @@ class Puppet::Transaction::Change
     def to_s
         return "change %s" % @property.change_to_s(@is, @should)
     end
+
+    private
+
+    def noop_event
+        result = event
+        result.log = property.log "is #{property.is_to_s(is)}, should be #{property.should_to_s(should)} (noop)"
+        result.status = "noop"
+        return result
+    end
 end
diff --git a/lib/puppet/transaction/event.rb b/lib/puppet/transaction/event.rb
index 5d422f93d..abd5c8041 100644
--- a/lib/puppet/transaction/event.rb
+++ b/lib/puppet/transaction/event.rb
@@ -1,7 +1,29 @@
-require 'puppet'
+require 'puppet/transaction'
+require 'puppet/util/tagging'
 
 # A simple struct for storing what happens on the system.
-Puppet::Transaction::Event = Struct.new(:name, :resource, :property, :result, :log, :previous_value, :desired_value) do
+class Puppet::Transaction::Event 
+    include Puppet::Util::Tagging
+
+    ATTRIBUTES = [:name, :resource, :property, :previous_value, :desired_value, :status, :log, :node, :version, :file, :line]
+    attr_accessor *ATTRIBUTES
+    attr_writer :tags
+    attr_accessor :time
+
+    EVENT_STATUSES = %w{noop success failure}
+
+    def initialize(*args)
+        options = args.last.is_a?(Hash) ? args.pop : ATTRIBUTES.inject({}) { |hash, attr| hash[attr] = args.pop; hash }
+        options.each { |attr, value| send(attr.to_s + "=", value) unless value.nil? }
+        
+        @time = Time.now
+    end
+
+    def status=(value)
+        raise ArgumentError, "Event status can only be #{EVENT_STATUSES.join(', ')}" unless EVENT_STATUSES.include?(value)
+        @status = value
+    end
+
     def to_s
         log
     end