Execution order is now based on dependency relationships, and those relationships correctly propagate up and descend into components. There is also an event test suite now, along with a (currently simple) component test suite.

git-svn-id: https://reductivelabs.com/svn/puppet/library/trunk@565 980ebf18-57e1-0310-9a29-db15c13687c0

7 files changed, 244 insertions, 171 deletions

diff --git a/lib/puppet/event.rb b/lib/puppet/event.rb
index 86893f6d1..d01863de2 100644
--- a/lib/puppet/event.rb
+++ b/lib/puppet/event.rb
@@ -16,18 +16,18 @@ module Puppet
         # objects react to an event
         class Subscription
             include Puppet
-            attr_accessor :source, :event, :target, :method
+            attr_accessor :source, :event, :target
 
             def initialize(hash)
                 @triggered = false
 
-                hash.each { |method,value|
+                hash.each { |param,value|
                     # assign each value appropriately
                     # this is probably wicked-slow
-                    self.send(method.to_s + "=",value)
+                    self.send(param.to_s + "=",value)
                 }
-                Puppet.debug "New Subscription: '%s' => '%s'" %
-                    [@source,@event]
+                #Puppet.debug "New Subscription: '%s' => '%s'" %
+                #    [@source,@event]
             end
 
             # the transaction is passed in so that we can notify it if
@@ -40,6 +40,12 @@ module Puppet
                 # to the "old" object rather than "new"
                 # but we're pretty far from that being a problem
                 event = nil
+
+                if @event == :NONE
+                    # just ignore these subscriptions
+                    return
+                end
+
                 if transaction.triggercount(self) > 0
                     Puppet.debug "%s has already run" % self
                 else
@@ -55,7 +61,8 @@ module Puppet
                     rescue => detail
                         # um, what the heck do i do when an object fails to refresh?
                         # shouldn't that result in the transaction rolling back?
-                        # XXX yeah, it should
+                        # the 'onerror' metaparam will be used to determine
+                        # behaviour in that case
                         Puppet.err "'%s' failed to %s: '%s'" %
                             [@target,@method,detail]
                         raise
@@ -68,12 +75,13 @@ module Puppet
             end
         end
 
-		attr_accessor :event, :object, :transaction
+		attr_accessor :event, :source, :transaction
 
         @@events = []
 
         @@subscriptions = []
 
+        # I think this method is obsolete
         def self.process
             Puppet.debug "Processing events"
             @@events.each { |event|
@@ -92,6 +100,7 @@ module Puppet
             @@events.clear
         end
 
+        # I think this method is obsolete
         def self.subscribe(hash)
             if hash[:event] == '*'
                 hash[:event] = :ALL_EVENTS
@@ -103,13 +112,13 @@ module Puppet
         end
 
 		def initialize(args)
-            unless args.include?(:event) and args.include?(:change)
-				raise "Event.new called incorrectly"
+            unless args.include?(:event) and args.include?(:source)
+				raise Puppet::DevError, "Event.new called incorrectly"
 			end
 
 			@change = args[:change]
 			@event = args[:event]
-			#@object = args[:object]
+			@source = args[:source]
 			@transaction = args[:transaction]
 
             #Puppet.info "%s: %s(%s)" %
@@ -118,93 +127,13 @@ module Puppet
 
             # initially, just stuff all instances into a central bucket
             # to be handled as a batch
-            @@events.push self
+            #@@events.push self
 		end
 
         def to_s
-            return self.event
+            self.event.to_s
         end
 	end
 end
 
 
-#---------------------------------------------------------------
-# here i'm separating out the methods dealing with handling events
-# currently not in use, so...
-
-class Puppet::NotUsed
-    #---------------------------------------------------------------
-    # return action array
-    # these are actions to use for responding to events
-    # no, this probably isn't the best way, because we're providing
-    # access to the actual hash, which is silly
-    def action
-        if not defined? @actions
-            Puppet.debug "defining action hash"
-            @actions = Hash.new
-        end
-        @actions
-    end
-    #---------------------------------------------------------------
-
-    #---------------------------------------------------------------
-    # call an event
-    # this is called on subscribers by the trigger method from the obj
-    # which sent the event
-    # event handling should probably be taking place in a central process,
-    # but....
-    def event(event,obj)
-        Puppet.debug "#{self} got event #{event} from #{obj}"
-        if @actions.key?(event)
-            Puppet.debug "calling it"
-            @actions[event].call(self,obj,event)
-        else
-            p @actions
-        end
-    end
-    #---------------------------------------------------------------
-
-    #---------------------------------------------------------------
-    # subscribe to an event or all events
-    # this entire event system is a hack job and needs to
-    # be replaced with a central event handler
-    def subscribe(args,&block)
-        obj = args[:object]
-        event = args[:event] || '*'.intern
-        if obj.nil? or event.nil?
-            raise "subscribe was called wrongly; #{obj} #{event}"
-        end
-        obj.action[event] = block
-        #events.each { |event|
-            unless @notify.key?(event)
-                @notify[event] = Array.new
-            end
-            unless @notify[event].include?(obj)
-                Puppet.debug "pushing event '%s' for object '%s'" % [event,obj]
-                @notify[event].push(obj)
-            end
-        #	}
-        #else
-        #	@notify['*'.intern].push(obj)
-    end
-    #---------------------------------------------------------------
-
-    #---------------------------------------------------------------
-    # initiate a response to an event
-    def trigger(event)
-        subscribers = Array.new
-        if @notify.include?('*') and @notify['*'].length > 0
-            @notify['*'].each { |obj| subscribers.push(obj) }
-        end
-        if (@notify.include?(event) and (! @notify[event].empty?) )
-            @notify[event].each { |obj| subscribers.push(obj) }
-        end
-        Puppet.debug "triggering #{event}"
-        subscribers.each { |obj|
-            Puppet.debug "calling #{event} on #{obj}"
-            obj.event(event,self)
-        }
-    end
-    #---------------------------------------------------------------
-
-end # Puppet::Type
diff --git a/lib/puppet/statechange.rb b/lib/puppet/statechange.rb
index f5bd5323d..85e6e47a3 100644
--- a/lib/puppet/statechange.rb
+++ b/lib/puppet/statechange.rb
@@ -65,6 +65,7 @@ module Puppet
                     :event => event,
                     :change => self,
                     :transaction => @transaction,
+                    :source => @state.parent,
                     :message => self.to_s
                 )
             rescue => detail
@@ -77,11 +78,11 @@ module Puppet
                 #    pname = pname.id2name
                 #end
                     #:state => @state,
-                    #:object => @state.parent,
                 Puppet.info "Failed: " + self.to_s
                 return Puppet::Event.new(
                     :event => pname + "_failed",
                     :change => self,
+                    :source => @state.parent,
                     :transaction => @transaction,
                     :message => "Failed: " + self.to_s
                 )
diff --git a/lib/puppet/transaction.rb b/lib/puppet/transaction.rb
index dcb9adc36..9e5a0fe30 100644
--- a/lib/puppet/transaction.rb
+++ b/lib/puppet/transaction.rb
@@ -45,9 +45,9 @@ class Transaction
     # then, we need to pass the event to the object's containing component,
     # to see if it or any of its parents have subscriptions on the event
     def evaluate
-        Puppet.debug "executing %s changes or transactions" % @changes.length
+        Puppet.debug "executing %s changes " % @changes.length
 
-        return @changes.collect { |change|
+        events = @changes.collect { |change|
             if change.is_a?(Puppet::StateChange)
                 change.transaction = self
                 events = nil
@@ -58,24 +58,12 @@ class Transaction
                     #@@changed.push change.state.parent
                 rescue => detail
                     Puppet.err("%s failed: %s" % [change,detail])
-                    raise
-                    # at this point, we would normally do error handling
-                    # but i haven't decided what to do for that yet
-                    # so just record that a sync failed for a given object
-                    #@@failures[change.state.parent] += 1
-                    # this still could get hairy; what if file contents changed,
-                    # but a chmod failed?  how would i handle that error? dern
+                    next
+                    # FIXME this should support using onerror to determine behaviour
                 end
 
                 if events.nil?
                     Puppet.debug "No events returned?"
-                else
-                    # first handle the subscriptions on individual objects
-                    events.each { |event|
-                        change.state.parent.subscribers?(event).each { |sub|
-                            sub.trigger(self)
-                        }
-                    }
                 end
                 events
             elsif change.is_a?(Puppet::Transaction)
@@ -85,12 +73,11 @@ class Transaction
             end
         }.flatten.reject { |event|
             event.nil?
-        }.each { |event|
-            # this handles subscriptions on the components, rather than
-            # on individual objects
-            self.component.subscribers?(event).each { |sub|
-                sub.trigger(self)
-            }
+        }
+
+        events.each { |event|
+            object = event.source
+            object.propagate(event)
         }
     end
     #---------------------------------------------------------------
@@ -98,11 +85,13 @@ class Transaction
     #---------------------------------------------------------------
     # this should only be called by a Puppet::Container object now
     # and it should only receive an array
-    def initialize(tree)
-        @tree = tree
+    def initialize(objects)
+        @objects = objects
         @toplevel = false
 
-        @triggered = Hash.new(0)
+        @triggered = Hash.new { |hash, key|
+            hash[key] = Hash.new(0)
+        }
 
         # of course, this won't work on the second run
         unless defined? @@failures
@@ -111,7 +100,7 @@ class Transaction
         end
         # change collection is in-band, and message generation is out-of-band
         # of course, exception raising is also out-of-band
-        @changes = @tree.collect { |child|
+        @changes = @objects.collect { |child|
             # these children are all Puppet::Type instances
             # not all of the children will return a change, and Containers
             # return transactions
@@ -151,16 +140,14 @@ class Transaction
     #---------------------------------------------------------------
 
     #---------------------------------------------------------------
-    def triggercount(sub)
-        Puppet.debug "Triggercount on %s is %s" % [sub,@triggered[sub]]
-        return @triggered[sub]
+    def triggered(object, method)
+        @triggered[object][method] += 1
     end
     #---------------------------------------------------------------
 
     #---------------------------------------------------------------
-    def triggered(sub)
-        @triggered[sub] += 1
-        Puppet.debug "%s was triggered; count is %s" % [sub,@triggered[sub]]
+    def triggered?(object, method)
+        @triggered[object][method]
     end
     #---------------------------------------------------------------
 end
diff --git a/lib/puppet/transportable.rb b/lib/puppet/transportable.rb
index cf4a4629c..1adaeafe1 100644
--- a/lib/puppet/transportable.rb
+++ b/lib/puppet/transportable.rb
@@ -105,7 +105,7 @@ module Puppet
             else
                 Puppet.debug "%s has no parameters" % @name
             end
-            container = Puppet::Component.new(hash)
+            container = Puppet::Type::Component.new(hash)
             nametable = {}
 
             self.each { |child|
diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb
index 043c90e81..d77e9017b 100644
--- a/lib/puppet/type.rb
+++ b/lib/puppet/type.rb
@@ -10,7 +10,7 @@ require 'puppet/metric'
 require 'puppet/type/state'
 
 
-# XXX see the bottom of the file for the rest of the inclusions
+# see the bottom of the file for the rest of the inclusions
 
 #---------------------------------------------------------------
 # This class is the abstract base class for the mechanism for organizing
@@ -65,6 +65,7 @@ class Type < Puppet::Element
         :noop,
         :schedule,
         :check,
+        :subscribe,
         :require
     ]
 
@@ -90,7 +91,10 @@ class Type < Puppet::Element
         but which should not actually be modified.  This is currently used
         internally, but will eventually be used for querying."
     @@metaparamdoc[:require] = "One or more objects that this object depends on.
-        Changes in the required objects result in the dependent objects being
+        This is used purely for guaranteeing that changes to required objects
+        happen before the dependent object."
+    @@metaparamdoc[:subscribe] = "One or more objects that this object depends on.
+        Changes in the subscribed to objects result in the dependent objects being
         refreshed (e.g., a service will get restarted)."
    
     #---------------------------------------------------------------
@@ -133,7 +137,7 @@ class Type < Puppet::Element
         @@typeary.each { |otype|
             if @@typehash.include?(otype.name)
                 if @@typehash[otype.name] != otype
-                    warning("Object type %s is already defined (%s vs %s)" %
+                    Puppet.warning("Object type %s is already defined (%s vs %s)" %
                         [otype.name,@@typehash[otype.name],otype])
                 end
             else
@@ -646,6 +650,12 @@ class Type < Puppet::Element
         @evalcount = 0
 
         @subscriptions = []
+        @dependencies = []
+
+        # callbacks are per object and event
+        @callbacks = Hash.new { |chash, key|
+            chash[key] = {}
+        }
 
         # states and parameters are treated equivalently from the outside:
         # as name-value pairs (using [] and []=)
@@ -705,7 +715,7 @@ class Type < Puppet::Element
                     self[name] = hash[name]
                 rescue => detail
                     raise Puppet::DevError.new( 
-                        "Could not set %s on %s" % [name, self.class]
+                        "Could not set %s on %s: %s" % [name, self.class, detail]
                     )
                 end
                 hash.delete name
@@ -713,6 +723,7 @@ class Type < Puppet::Element
         }
 
         if hash.length > 0
+            Puppet.debug hash.inspect
             raise Puppet::Error.new("Class %s does not accept argument(s) %s" %
                 [self.class.name, hash.keys.join(" ")])
         end
@@ -976,41 +987,88 @@ class Type < Puppet::Element
     #---------------------------------------------------------------
 
     #---------------------------------------------------------------
-    def subscribe(hash)
-        if hash[:event] == '*'
-            hash[:event] = :ALL_EVENTS
+    # Is the parameter in question a meta-parameter?
+    def self.metaparam?(param)
+        @@metaparams.include?(param)
+    end
+    #---------------------------------------------------------------
+
+    #---------------------------------------------------------------
+    # for each object we require, subscribe to all events that it
+    # generates
+    # we might reduce the level of subscription eventually, but for now...
+    def metarequire=(requires)
+        self.handledepends(requires, :NONE, nil)
+    end
+    #---------------------------------------------------------------
+
+    #---------------------------------------------------------------
+    # for each object we require, subscribe to all events that it
+    # generates
+    # we might reduce the level of subscription eventually, but for now...
+    def metasubscribe=(requires)
+        self.handledepends(requires, :ALL_EVENTS, :refresh)
+    end
+    #---------------------------------------------------------------
+
+    #---------------------------------------------------------------
+    def metanoop=(noop)
+        if noop == "true" or noop == true
+            @noop = true
+        elsif noop == "false" or noop == false
+            @noop = false
+        else
+            raise Puppet::Error.new("Invalid noop value '%s'" % noop)
         end
+    end
+    #---------------------------------------------------------------
 
-        hash[:source] = self
-        sub = Puppet::Event::Subscription.new(hash)
+    #---------------------------------------------------------------
+    def metaonerror=(response)
+        Puppet.debug("Would have called metaonerror")
+        @onerror = response
+    end
+    #---------------------------------------------------------------
 
-        # add to the correct area
-        @subscriptions.push sub
+    #---------------------------------------------------------------
+    def metaschedule=(schedule)
+        @schedule = schedule
     end
     #---------------------------------------------------------------
+    #---------------------------------------------------------------
 
     #---------------------------------------------------------------
-    # return all of the subscriptions to a given event
-    def subscribers?(event)
-        @subscriptions.find_all { |sub|
-            sub.event == event.event or
-                sub.event == :ALL_EVENTS
+    #---------------------------------------------------------------
+    # Subscription and relationship methods
+    #---------------------------------------------------------------
+    #---------------------------------------------------------------
+
+    #---------------------------------------------------------------
+    def addcallback(object, event, method)
+        @callbacks[object][event] = method
+    end
+    #---------------------------------------------------------------
+
+    #---------------------------------------------------------------
+    # return all objects subscribed to the current object
+    def eachdependency
+        @dependencies.each { |dep|
+            yield dep
         }
     end
     #---------------------------------------------------------------
 
     #---------------------------------------------------------------
-    # Is the parameter in question a meta-parameter?
-    def self.metaparam?(param)
-        @@metaparams.include?(param)
+    # return all objects subscribed to the current object
+    def eachsubscriber
+        @subscriptions.each { |sub|
+            yield sub.target
+        }
     end
     #---------------------------------------------------------------
 
     #---------------------------------------------------------------
-    # for each object we require, subscribe to all events that it
-    # generates
-    # we might reduce the level of subscription eventually, but for now...
-    def metarequire=(requires)
+    def handledepends(requires, event, method)
         unless requires.is_a?(Array)
             requires = [requires]
         end
@@ -1021,50 +1079,96 @@ class Type < Puppet::Element
             object = nil
             tname = rname[0]
             unless type = Puppet::Type.type(tname)
-                raise "Could not find type %s" % tname
+                raise Puppet::Error, "Could not find type %s" % tname
             end
             name = rname[1]
             unless object = type[name]
-                raise "Could not retrieve object '%s' of type '%s'" %
+                raise Puppet::Error, "Could not retrieve object '%s' of type '%s'" %
                     [name,type]
             end
-            Puppet.debug("%s requires %s" % [self.name,object])
+            Puppet.debug("%s subscribes to %s" % [self.name,object])
 
-            # for now, we only support this one method, 'refresh'
-            object.subscribe(
-                :event => '*',
-                :target => self,
-                :method => :refresh
+            unless @dependencies.include?(object)
+                @dependencies << object
+            end
+
+            # pure requires don't call methods
+            next if method.nil?
+
+            # ok, both sides of the connection store some information
+            # we store the method to call when a given subscription is 
+            # triggered, but the source object decides whether 
+            sub = object.subscribe(
+                :event => event,
+                :target => self
             )
+            if self.respond_to?(method)
+                self.addcallback(object, event, method)
+            end
             #object.addnotify(self)
         }
     end
     #---------------------------------------------------------------
 
     #---------------------------------------------------------------
-    def metanoop=(noop)
-        if noop == "true" or noop == true
-            @noop = true
-        elsif noop == "false" or noop == false
-            @noop = false
-        else
-            raise Puppet::Error.new("Invalid noop value '%s'" % noop)
+    def propagate(event)
+        self.subscribers?(event).each { |object|
+            object.trigger(event, self)
+        }
+
+        if defined? @parent
+            @parent.propagate(event)
         end
     end
     #---------------------------------------------------------------
 
     #---------------------------------------------------------------
-    def metaonerror=(response)
-        Puppet.debug("Would have called metaonerror")
-        @onerror = response
+    def subscribe(hash)
+        hash[:source] = self
+        sub = Puppet::Event::Subscription.new(hash)
+
+        # add to the correct area
+        @subscriptions.push sub
     end
     #---------------------------------------------------------------
 
     #---------------------------------------------------------------
-    def metaschedule=(schedule)
-        @schedule = schedule
+    # return all of the subscriptions to a given event
+    def subscribers?(event)
+        @subscriptions.find_all { |sub|
+            sub.event == event.event or
+                sub.event == :ALL_EVENTS
+        }.collect { |sub|
+            sub.target
+        }
     end
     #---------------------------------------------------------------
+
+    #---------------------------------------------------------------
+    # we've received an event
+    # we only support local events right now, so we can pass actual
+    # objects around, including the transaction object
+    # the assumption here is that container objects will pass received
+    # methods on to contained objects
+    # i.e., we don't trigger our children, our refresh() method calls
+    # refresh() on our children
+    def trigger(event, source)
+        trans = event.transaction
+        if @callbacks.include?(source)
+            [:ALL_EVENTS, event.event].each { |eventname|
+                if method = @callbacks[source][eventname]
+                    if trans.triggered?(self, method) > 0
+                        next
+                    end
+                    if self.respond_to?(method)
+                        self.send(method)
+                    end
+
+                    trans.triggered(self, method)
+                end
+            }
+        end
+    end
     #---------------------------------------------------------------
 
     #---------------------------------------------------------------
diff --git a/lib/puppet/type/component.rb b/lib/puppet/type/component.rb
index 2274c7c10..a996bd591 100644
--- a/lib/puppet/type/component.rb
+++ b/lib/puppet/type/component.rb
@@ -10,6 +10,7 @@ require 'puppet/type'
 require 'puppet/transaction'
 
 module Puppet
+    class Type
 	class Component < Puppet::Type
         include Enumerable
 
@@ -19,9 +20,42 @@ module Puppet
         @states = []
         @parameters = [:name,:type]
 
+        # topo sort functions
+        def self.sort(objects)
+            list = []
+            inlist = {}
+
+            objects.each { |obj|
+                self.recurse(obj, inlist, list)
+            }
+
+            return list
+        end
+
+        def self.recurse(obj, inlist, list)
+            return if list.include?(obj.object_id)
+            obj.eachdependency { |req|
+                self.recurse(req, inlist, list)
+            }
+            
+            list << obj
+            inlist[obj.object_id] = true
+        end
+
         def each
             @children.each { |child| yield child }
         end
+        
+        # this returns a sorted array, not a new component, but that suits me just fine
+        def flatten
+            self.class.sort(@children.collect { |child|
+                if child.is_a?(self.class)
+                    child.flatten
+                else
+                    child
+                end
+            }.flatten)
+        end
 
         def initialize(args)
             @children = []
@@ -31,16 +65,29 @@ module Puppet
                 args[:type] = "component"
             end
             super(args)
-            debug "Made component with name %s and type %s" % [self.name, self[:type]]
+            Puppet.debug "Made component with name %s and type %s" %
+                [self.name, self[:type]]
         end
 
+        # the "old" way of doing things
         # just turn the container into a transaction
-        def evaluate
+        def oldevaluate
             transaction = Puppet::Transaction.new(@children)
             transaction.component = self
             return transaction
         end
 
+        # flatten all children, sort them, and evaluate them in order
+        # this is only called on one component over the whole system
+        # this also won't work with scheduling, but eh
+        def evaluate
+            # but what about dependencies?
+
+            transaction = Puppet::Transaction.new(self.flatten)
+            transaction.component = self
+            return transaction
+        end
+
         def name
             #return self[:name]
             return "%s[%s]" % [self[:type],self[:name]]
@@ -78,4 +125,5 @@ module Puppet
             return "component(%s)" % self.name
         end
 	end
+    end
 end
diff --git a/lib/puppet/type/exec.rb b/lib/puppet/type/exec.rb
index 4aa34c0fd..c5326ca33 100755
--- a/lib/puppet/type/exec.rb
+++ b/lib/puppet/type/exec.rb
@@ -190,6 +190,10 @@ module Puppet
             def refresh
                 self.state(:returns).sync
             end
+
+            def to_s
+                "exec(%s)" % self.name
+            end
         end
     end
 end