From f8d7c44fea37dff3e9a86652699bffeab0fbe111 Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Sun, 1 Nov 2009 10:47:50 -0600 Subject: 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 --- lib/puppet/property.rb | 5 ++--- lib/puppet/transaction.rb | 7 ++++--- lib/puppet/type.rb | 6 ++++++ spec/unit/property.rb | 24 +++++++++++------------- spec/unit/transaction.rb | 39 ++++++++++++++++++++++++++++----------- spec/unit/type.rb | 21 +++++++++++++++++++++ 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 -- cgit