Moving event creation to the resource

This allows the Transaction class to reuse the
event creation code when it creates noop and restart
events.

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

6 files changed, 72 insertions, 30 deletions

diff --git a/lib/puppet/property.rb b/lib/puppet/property.rb
index bb58caf5e..9d50dcf6a 100644
--- a/lib/puppet/property.rb
+++ b/lib/puppet/property.rb
@@ -155,10 +155,9 @@ class Puppet::Property < Puppet::Parameter
         end).to_sym
     end
 
-    # Create our event object.
+    # Return a modified form of the resource event.
     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)
+        resource.event :name => event_name, :desired_value => should, :property => name
     end
 
     attr_reader :shadow
diff --git a/lib/puppet/transaction.rb b/lib/puppet/transaction.rb
index a5feecbc2..0a26e33fe 100644
--- a/lib/puppet/transaction.rb
+++ b/lib/puppet/transaction.rb
@@ -409,7 +409,7 @@ class Puppet::Transaction
         end
 
         if restarted
-            queue_event(resource, Puppet::Transaction::Event.new(:restarted, resource))
+            queue_event(resource, resource.event(:name => :restarted, :status => "success"))
 
             @resourcemetrics[:restarted] += 1
         end
@@ -544,7 +544,8 @@ class Puppet::Transaction
     end
 
     def process_callback(resource, callback, events)
-        process_noop_events(resource, callback, events) and return false if events.detect { |e| e.name == :noop }
+        # XXX Should it be any event, or all events?
+        process_noop_events(resource, callback, events) and return false unless events.detect { |e| e.status != "noop" }
         resource.send(callback)
 
         resource.notice "Triggered '#{callback}' from #{events.length} events"
@@ -561,7 +562,7 @@ class Puppet::Transaction
         resource.notice "Would have triggered '#{callback}' from #{events.length} events"
 
         # And then add an event for it.
-        queue_event(resource, Puppet::Transaction::Event.new(:noop, resource))
+        queue_event(resource, resource.event(:status => "noop", :name => :noop_restart))
         true # so the 'and if' works
     end
 end
diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb
index 84f0b93d0..2512c72d3 100644
--- a/lib/puppet/type.rb
+++ b/lib/puppet/type.rb
@@ -484,6 +484,12 @@ class Type
         }
     end
 
+    # Create a transaction event.  Called by Transaction or by
+    # a property.
+    def event(options = {})
+        Puppet::Transaction::Event.new({:resource => ref, :file => file, :line => line, :tags => tags, :version => version}.merge(options))
+    end
+
     # Let the catalog determine whether a given cached value is
     # still valid or has expired.
     def expirer
diff --git a/spec/unit/property.rb b/spec/unit/property.rb
index 6d3871d7b..5bc29a61a 100755
--- a/spec/unit/property.rb
+++ b/spec/unit/property.rb
@@ -108,21 +108,26 @@ describe Puppet::Property do
 
     describe "when creating an event" do
         before do
-            @resource = stub 'resource', :ref => "File[/foo]", :file => "/my/file", :line => 50, :tags => %w{foo bar}, :version => 42
+            @event = Puppet::Transaction::Event.new
+
+            # Use a real resource so we can test the event creation integration
+            @resource = Puppet::Type.type(:mount).new :name => "foo"
             @instance = @class.new(:resource => @resource)
             @instance.stubs(:should).returns "myval"
         end
 
+        it "should use an event from the resource as the base event" do
+            event = Puppet::Transaction::Event.new
+            @resource.expects(:event).returns event
+
+            @instance.event.should equal(event)
+        end
+
         it "should have the default event name" do
             @instance.expects(:event_name).returns :my_event
             @instance.event.name.should == :my_event
         end
 
-        it "should have the resource's reference as the resource" do
-            @resource.stubs(:ref).returns "File[/yay]"
-            @instance.event.resource.should == "File[/yay]"
-        end
-
         it "should have the property's name" do
             @instance.event.property.should == @instance.name
         end
@@ -131,13 +136,6 @@ describe Puppet::Property do
             @instance.stubs(:should).returns "foo"
             @instance.event.desired_value.should == "foo"
         end
-
-        {:file => "/my/file", :line => 50, :tags => %{foo bar}, :version => 50}.each do |attr, value|
-            it "should set the #{attr}" do
-                @instance.stubs(attr).returns value
-                @instance.event.send(attr).should == value
-            end
-        end
     end
 
     describe "when shadowing metaparameters" do
diff --git a/spec/unit/transaction.rb b/spec/unit/transaction.rb
index abb362123..d0b3a2dd8 100755
--- a/spec/unit/transaction.rb
+++ b/spec/unit/transaction.rb
@@ -111,7 +111,7 @@ describe Puppet::Transaction do
             @graph = stub 'graph', :matching_edges => [], :resource => @resource
             @transaction.stubs(:relationship_graph).returns @graph
 
-            @event = Puppet::Transaction::Event.new(:foo, @resource)
+            @event = Puppet::Transaction::Event.new(:name => :foo, :resource => @resource)
         end
 
         it "should store each event in its event list" do
@@ -218,8 +218,8 @@ describe Puppet::Transaction do
             @transaction = Puppet::Transaction.new(Puppet::Resource::Catalog.new)
             @transaction.stubs(:queue_event)
 
-            @resource = stub 'resource', :notice => nil
-            @event = Puppet::Transaction::Event.new(:event, @resource)
+            @resource = stub 'resource', :notice => nil, :event => @event
+            @event = Puppet::Transaction::Event.new(:name => :event, :resource => @resource)
         end
 
         it "should call the required callback once for each set of associated events" do
@@ -241,14 +241,13 @@ describe Puppet::Transaction do
             @transaction.resourcemetrics[:restarted].should == 1
         end
 
-        it "should queue a 'restarted' event with the resource as the source" do
+        it "should queue a 'restarted' event generated by the resource" do
             @transaction.expects(:queued_events).with(@resource).yields(:callback1, [@event])
 
             @resource.stubs(:callback1)
 
-            @transaction.expects(:queue_event).with do |resource, event|
-                event.name == :restarted and resource == @resource
-            end
+            @resource.expects(:event).with(:name => :restarted, :status => "success").returns "myevent"
+            @transaction.expects(:queue_event).with(@resource, "myevent")
 
             @transaction.process_events(@resource)
         end
@@ -263,9 +262,25 @@ describe Puppet::Transaction do
             @transaction.process_events(@resource)
         end
 
-        describe "and the events include a noop event" do
+        describe "and the events include a noop event and at least one non-noop event" do
+            before do
+                @event.stubs(:status).returns "noop"
+                @event2 = Puppet::Transaction::Event.new(:name => :event, :resource => @resource)
+                @event2.status = "success"
+                @transaction.expects(:queued_events).with(@resource).yields(:callback1, [@event, @event2])
+            end
+
+            it "should call the callback" do
+                @resource.expects(:callback1)
+
+                @transaction.process_events(@resource)
+            end
+        end
+
+        describe "and the events are all noop events" do
             before do
-                @event.stubs(:name).returns :noop
+                @event.stubs(:status).returns "noop"
+                @resource.stubs(:event).returns(Puppet::Transaction::Event.new)
                 @transaction.expects(:queued_events).with(@resource).yields(:callback1, [@event])
             end
 
@@ -281,8 +296,10 @@ describe Puppet::Transaction do
                 @transaction.process_events(@resource)
             end
 
-            it "should queue a new noop event" do
-                @transaction.expects(:queue_event).with { |resource, event| event.name == :noop and resource == @resource }
+            it "should queue a new noop event generated from the resource" do
+                event = Puppet::Transaction::Event.new
+                @resource.expects(:event).with(:status => "noop", :name => :noop_restart).returns event
+                @transaction.expects(:queue_event).with(@resource, event)
 
                 @transaction.process_events(@resource)
             end
diff --git a/spec/unit/type.rb b/spec/unit/type.rb
index 575b6c093..be456a88b 100755
--- a/spec/unit/type.rb
+++ b/spec/unit/type.rb
@@ -102,6 +102,27 @@ describe Puppet::Type do
         Puppet::Type.type(:mount).new(:name => "foo").type.should == :mount
     end
 
+    describe "when creating an event" do
+        before do
+            @resource = Puppet::Type.type(:mount).new :name => "foo"
+        end
+
+        it "should have the resource's reference as the resource" do
+            @resource.event.resource.should == "Mount[foo]"
+        end
+
+        {:file => "/my/file", :line => 50, :tags => %{foo bar}, :version => 50}.each do |attr, value|
+            it "should set the #{attr}" do
+                @resource.stubs(attr).returns value
+                @resource.event.send(attr).should == value
+            end
+        end
+
+        it "should allow specification of event attributes" do
+            @resource.event(:status => "noop").status.should == "noop"
+        end
+    end
+
     describe "when choosing a default provider" do
         it "should choose the provider with the highest specificity" do
             # Make a fake type