diff options
| -rw-r--r-- | lib/puppet/metatype/metaparams.rb | 16 | ||||
| -rw-r--r-- | lib/puppet/pgraph.rb | 31 | ||||
| -rw-r--r-- | lib/puppet/relationship.rb | 4 | ||||
| -rwxr-xr-x | test/other/pgraph.rb | 76 | ||||
| -rwxr-xr-x | test/other/relationship.rb | 28 |
5 files changed, 134 insertions, 21 deletions
diff --git a/lib/puppet/metatype/metaparams.rb b/lib/puppet/metatype/metaparams.rb index c9c3a694d..93fffc04c 100644 --- a/lib/puppet/metatype/metaparams.rb +++ b/lib/puppet/metatype/metaparams.rb @@ -288,15 +288,15 @@ class Puppet::Type target = object end - # 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 - subargs = { - :event => self.class.events - } - if method = self.class.callback - subargs[:callback] = method + subargs = { + :event => self.class.events, + :callback => method + } + else + # If there's no callback, there's no point in even adding + # a label. + subargs = nil end rel = Puppet::Relationship.new(source, target, subargs) end diff --git a/lib/puppet/pgraph.rb b/lib/puppet/pgraph.rb index de89e0a71..64f411a40 100644 --- a/lib/puppet/pgraph.rb +++ b/lib/puppet/pgraph.rb @@ -33,6 +33,23 @@ class Puppet::PGraph < GRATR::Digraph end end + # Make sure whichever edge has a label keeps the label + def copy_label(source, target, label) + # 'require' relationships will not have a label, + # and all 'subscribe' relationships have the same + # label, at least for now. + + # Labels default to {}, so we can't just test for nil. + newlabel = label || {} + oldlabel = edge_label(source, target) || {} + if ! newlabel.empty? and oldlabel.empty? + edge_label_set(source, target, label) + # We should probably check to see if the labels both exist + # and don't match, but we'd just throw an error which the user + # couldn't do anyting about. + end + end + # Which resources a given resource depends upon. def dependents(resource) tree_from_vertex2(resource).keys @@ -128,6 +145,7 @@ class Puppet::PGraph < GRATR::Digraph s = child t = neighbor end + if s.is_a?(type) raise "Source %s is still a container" % s end @@ -135,9 +153,16 @@ class Puppet::PGraph < GRATR::Digraph raise "Target %s is still a container" % t end - # It *appears* that we're having a problem - # with multigraphs. - next if edge?(s, t) + # We don't want to add multiple copies of the + # same edge, but we *do* want to make sure we + # keep labels around. + # XXX This will *not* work when we support multiple + # types of labels, and only works now because + # you can only do simple subscriptions. + if edge?(s, t) + copy_label(s, t, edge.label) + next + end add_edge!(s, t, edge.label) if cyclic? raise ArgumentError, diff --git a/lib/puppet/relationship.rb b/lib/puppet/relationship.rb index 879beeab7..15f67b384 100644 --- a/lib/puppet/relationship.rb +++ b/lib/puppet/relationship.rb @@ -46,9 +46,9 @@ class Puppet::Relationship < GRATR::Edge # Does the passed event match our event? This is where the meaning # of :NONE comes from. def match?(event) - if event == :NONE or self.event == :NONE + if self.event.nil? or event == :NONE or self.event == :NONE return false - elsif self.event == :ALL_EVENTS or event == :ALL_EVENTS or event == self.event + elsif self.event == :ALL_EVENTS or event == self.event return true else return false diff --git a/test/other/pgraph.rb b/test/other/pgraph.rb index c8a04cb8d..2763142ea 100755 --- a/test/other/pgraph.rb +++ b/test/other/pgraph.rb @@ -91,11 +91,8 @@ class TestPGraph < Test::Unit::TestCase # We have to specify a relationship to our empty container, else it # never makes it into the dep graph in the first place. #{one => two, three => [middle, two, "c"], "f" => "c", "h" => middle, "c" => empty}.each do |source, targets| - {one => two, two => three, middle => three, "f" => "c", "h" => middle, "c" => [three, empty]}.each do |source, targets| - targets = [targets] unless targets.is_a?(Array) - targets.each do |target| - deps.add_edge!(source, target, :callback => :refresh) - end + {one => two, "f" => "c", "h" => middle, "c" => empty}.each do |source, target| + deps.add_edge!(source, target, :callback => :refresh) end #num = 6 @@ -103,7 +100,7 @@ class TestPGraph < Test::Unit::TestCase #contgraph.reversal.to_jpg(File.expand_path("~/tmp/graphs"), "reversal#{num}") #deps.to_jpg(File.expand_path("~/tmp/graphs"), "relationships#{num}") - deps.splice!(contgraph, Container) + assert_nothing_raised { deps.splice!(contgraph, Container) } #deps.to_jpg(File.expand_path("~/tmp/graphs"), "after_relationships#{num}") assert(! deps.cyclic?, "Created a cyclic graph") @@ -128,9 +125,74 @@ class TestPGraph < Test::Unit::TestCase deps.edges.each do |edge| assert_equal({:callback => :refresh}, edge.label, - "Label was not copied on splice") + "Label was not copied for %s => %s" % [edge.source, edge.target]) + end + + # Now add some relationships to three, but only add labels to one of + # the relationships. + Puppet.err :yay + + # Add a simple, label-less relationship + deps.add_edge!(two, three) + assert_nothing_raised { deps.splice!(contgraph, Container) } + + # And make sure it stuck, with no labels. + assert_equal({}, deps.edge_label("c", "i"), + "label was created for c => i") + + # Now add some edges with labels, in a way that should overwrite + deps.add_edge!("c", three, {:callback => :refresh}) + assert_nothing_raised { deps.splice!(contgraph, Container) } + + # And make sure the label got copied. + assert_equal({:callback => :refresh}, deps.edge_label("c", "i"), + "label was not copied for c => i") + + # Lastly, add some new label-less edges and make sure the label stays. + deps.add_edge!(middle, three) + assert_nothing_raised { deps.splice!(contgraph, Container) } + assert_equal({:callback => :refresh}, deps.edge_label("c", "i"), + "label was lost for c => i") + + # Now make sure the 'three' edges all have the label we've used. + # Note that this will not work when we support more than one type of + # subscription. + three.each do |child| + edge = deps.edge_class.new("c", child) + assert(deps.edge?(edge), "no c => %s edge" % child) + assert_equal({:callback => :refresh}, deps[edge], + "label was not retained for c => %s" % child) end end + + def test_copy_label + graph = Puppet::PGraph.new + + # First make an edge with no label + graph.add_edge!(:a, :b) + assert_nil(graph.edge_label(:a, :b), "Created a label") + + # Now try to copy an empty label in. + graph.copy_label(:a, :b, {}) + + # It should just do nothing, since we copied an empty label. + assert_nil(graph.edge_label(:a, :b), "Created a label") + + # Now copy in a real label. + graph.copy_label(:a, :b, {:callback => :yay}) + assert_equal({:callback => :yay}, + graph.edge_label(:a, :b), "Did not copy label") + + # Now copy in a nil label + graph.copy_label(:a, :b, nil) + assert_equal({:callback => :yay}, + graph.edge_label(:a, :b), "lost label") + + # And an empty one. + graph.copy_label(:a, :b, {}) + assert_equal({:callback => :yay}, + graph.edge_label(:a, :b), "lost label") + end end # $Id$ diff --git a/test/other/relationship.rb b/test/other/relationship.rb index c6f8eb763..0a4d725ae 100755 --- a/test/other/relationship.rb +++ b/test/other/relationship.rb @@ -43,6 +43,32 @@ class TestRelationship < Test::Unit::TestCase assert_nil(i.event, "event was not nil") assert_nil(i.callback, "callback was not nil") end + + def test_match + edge = Puppet::Relationship.new(:a, :b) + + assert(! edge.match?(:NONE), "nil event matched :NONE") + assert(! edge.match?(:ALL_EVENT), "nil event matched :ALL_EVENTS") + assert(! edge.match?(:random), "nil event matched random") + + # Now set the edge to none + edge.label = {:event => :NONE} + assert(! edge.match?(:NONE), ":NONE event matched :NONE") + assert(! edge.match?(:ALL_EVENT), ":NONE event matched :ALL_EVENTS") + assert(! edge.match?(:random), ":NONE event matched random") + + # Now set it to :ALL + edge.label = {:event => :ALL_EVENTS} + assert(! edge.match?(:NONE), ":ALL_EVENTS event matched :NONE") + assert(edge.match?(:ALL_EVENTS), ":ALL_EVENTS did not match :ALL_EVENTS") + assert(edge.match?(:random), ":ALL_EVENTS did not match random") + + # And finally, :random + edge.label = {:event => :random} + assert(! edge.match?(:NONE), ":random event matched :NONE") + assert(! edge.match?(:ALL_EVENTS), ":random matched :ALL_EVENTS") + assert(edge.match?(:random), ":random did not match random") + end end -# $Id$
\ No newline at end of file +# $Id$ |