Fixing #1949 - relationships now use attributes instead of a label

This was important because the use of the label to store attributes
was a holdover from the GRATR library, and if we didn't cease its
use before we switched to RESTful catalogs, then we'd be stuck with
the @label instance variable forever, essentially.

Now we can add and remove variables however we please.

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

4 files changed, 57 insertions, 69 deletions

diff --git a/lib/puppet/relationship.rb b/lib/puppet/relationship.rb
index 05b7dc39e..984dffeeb 100644
--- a/lib/puppet/relationship.rb
+++ b/lib/puppet/relationship.rb
@@ -10,40 +10,26 @@
 # It used to be a subclass of GRATR::Edge, but that class has weird hash
 # overrides that dramatically slow down the graphing.
 class Puppet::Relationship
-    attr_accessor :source, :target, :label
+    attr_accessor :source, :target, :callback
 
-    # Return the callback
-    def callback
-        if label
-            label[:callback]
-        else
-            nil
-        end
-    end
+    attr_reader :event
     
-    # Return our event.
-    def event
-        if label
-            label[:event]
-        else
-            nil
+    def event=(event)
+        if event != :NONE and ! callback
+            raise ArgumentError, "You must pass a callback for non-NONE events"
         end
+        @event = event
     end
-    
-    def initialize(source, target, label = {})
-        if label
-            unless label.is_a?(Hash)
-                raise ArgumentError, "Relationship labels must be a hash"
-            end
-        
-            if label[:event] and label[:event] != :NONE and ! label[:callback]
-                raise ArgumentError, "You must pass a callback for non-NONE events"
+
+    def initialize(source, target, options = {})
+        @source, @target = source, target
+
+        options ||= {}
+        [:callback, :event].each do |option|
+            if value = options[option]
+                send(option.to_s + "=", value)
             end
-        else
-            label = {}
         end
-
-        @source, @target, @label = source, target, label
     end
     
     # Does the passed event match our event?  This is where the meaning
@@ -57,6 +43,13 @@ class Puppet::Relationship
             return false
         end
     end
+
+    def label
+        result = {}
+        result[:callback] = callback if callback
+        result[:event] = event if event
+        result
+    end
     
     def ref
         "%s => %s" % [source, target]
diff --git a/lib/puppet/transaction.rb b/lib/puppet/transaction.rb
index cb17de7b9..884206b4c 100644
--- a/lib/puppet/transaction.rb
+++ b/lib/puppet/transaction.rb
@@ -253,10 +253,8 @@ class Transaction
             # We have to dup the label here, else we modify the original edge label,
             # which affects whether a given event will match on the next run, which is,
             # of course, bad.
-            edge = orig_edge.class.new(orig_edge.source, orig_edge.target)
-            label = orig_edge.label.dup
-            label[:event] = events.collect { |e| e.name }
-            edge.label = label
+            edge = orig_edge.class.new(orig_edge.source, orig_edge.target, orig_edge.label)
+            edge.event = events.collect { |e| e.name }
             set_trigger(edge)
         end
 
diff --git a/spec/unit/relationship.rb b/spec/unit/relationship.rb
index 5f96cdf8c..4d1b75856 100755
--- a/spec/unit/relationship.rb
+++ b/spec/unit/relationship.rb
@@ -19,61 +19,58 @@ describe Puppet::Relationship do
         @edge.should respond_to(:target)
     end
 
-    it "should have a :label attribute" do
-        @edge.should respond_to(:label)
+    it "should have a :callback attribute" do
+        @edge.callback = :foo
+        @edge.callback.should == :foo
     end
 
-    it "should provide a :ref method that describes the edge" do
-        @edge = Puppet::Relationship.new("a", "b")
-        @edge.ref.should == "a => b"
+    it "should have an :event attribute" do
+        @edge.event = :NONE
+        @edge.event.should == :NONE
     end
-end
 
-describe Puppet::Relationship, " when initializing" do
-    before do
-        @edge = Puppet::Relationship.new(:a, :b, :testing => :foo)
+    it "should require a callback if a non-NONE event is specified" do
+        proc { @edge.event = :something }.should raise_error(ArgumentError)
     end
 
-    it "should use the first argument as the source" do
-        @edge.source.should == :a
+    it "should have a :label attribute" do
+        @edge.should respond_to(:label)
     end
 
-    it "should use the second argument as the target" do
-        @edge.target.should == :b
+    it "should provide a :ref method that describes the edge" do
+        @edge = Puppet::Relationship.new("a", "b")
+        @edge.ref.should == "a => b"
     end
 
-    it "should use the third argument as the label" do
-        @edge.label.should == {:testing => :foo}
-    end
+    it "should be able to produce a label as a hash with its event and callback" do
+        @edge.callback = :foo
+        @edge.event = :bar
 
-    it "should require a callback if a non-NONE event is specified" do
-        proc { Puppet::Relationship.new(:a, :b, :event => :something) }.should raise_error(ArgumentError)
+        @edge.label.should == {:callback => :foo, :event => :bar}
     end
 
-    it "should require the label to be a hash" do
-        proc { Puppet::Relationship.new(:a, :b, :event) }.should raise_error(ArgumentError)
+    it "should work if nil options are provided" do
+        lambda { Puppet::Relationship.new("a", "b", nil) }.should_not raise_error
     end
 end
 
-describe Puppet::Relationship, " when interpreting the label" do
-    it "should default to an event of nil" do
+describe Puppet::Relationship, " when initializing" do
+    before do
         @edge = Puppet::Relationship.new(:a, :b)
-        @edge.event.should be_nil
     end
 
-    it "should expose a provided event via the :event method" do
-        @edge = Puppet::Relationship.new(:a, :b, :event => :something, :callback => :whatever)
-        @edge.event.should == :something
+    it "should use the first argument as the source" do
+        @edge.source.should == :a
     end
 
-    it "should default to a nil callback" do
-        @edge = Puppet::Relationship.new(:a, :b)
-        @edge.callback.should be_nil
+    it "should use the second argument as the target" do
+        @edge.target.should == :b
     end
 
-    it "should expose a provided callback via the :callback method" do
-        @edge = Puppet::Relationship.new(:a, :b, :callback => :testing)
-        @edge.callback.should == :testing
+    it "should set the rest of the arguments as the event and callback" do
+        @edge = Puppet::Relationship.new(:a, :b, :callback => :foo, :event => :bar)
+        @edge.callback.should == :foo
+        @edge.event.should == :bar
     end
 end
 
diff --git a/spec/unit/simple_graph.rb b/spec/unit/simple_graph.rb
index a73e75a42..2c061ae1a 100755
--- a/spec/unit/simple_graph.rb
+++ b/spec/unit/simple_graph.rb
@@ -107,14 +107,14 @@ describe Puppet::SimpleGraph do
         end
 
         it "should provide a method to add an edge by specifying the two vertices and a label" do
-            @graph.add_edge(:one, :two, :stuff => :awesome)
+            @graph.add_edge(:one, :two, :callback => :awesome)
             @graph.edge?(:one, :two).should be_true
         end
 
         it "should provide a method for retrieving an edge label" do
-            edge = Puppet::Relationship.new(:one, :two, :stuff => :awesome)
+            edge = Puppet::Relationship.new(:one, :two, :callback => :awesome)
             @graph.add_edge(edge)
-            @graph.edge_label(:one, :two).should == {:stuff => :awesome}
+            @graph.edge_label(:one, :two).should == {:callback => :awesome}
         end
 
         it "should provide a method for retrieving an edge" do
@@ -236,9 +236,9 @@ describe Puppet::SimpleGraph do
         end
         
         it "should retain labels on edges" do
-            @graph.add_edge(:one, :two, :stuff => :awesome)
+            @graph.add_edge(:one, :two, :callback => :awesome)
             edge = @graph.reversal.edge(:two, :one)
-            edge.label.should == {:stuff => :awesome}
+            edge.label.should == {:callback => :awesome}
         end
     end