Switching the base class for the Relationship class.

It was previously using the GRATR::Edge class, which
had wonky overrides that dramatically slowed down
sorting (its hash mechanism hashed the source and
target so that edges with the same source/target got
the same hash, which we actually don't want any more).

This shouldn't change any functionality, just performance.

I didn't retain all functionality from the Edge class, but
a lot of that functionality was, um, horrible, like Edge[]
being equivalent to Edge.new.

6 files changed, 168 insertions, 85 deletions

diff --git a/lib/puppet/metatype/relationships.rb b/lib/puppet/metatype/relationships.rb
index c7a0a91ee..4fb78ae56 100644
--- a/lib/puppet/metatype/relationships.rb
+++ b/lib/puppet/metatype/relationships.rb
@@ -40,7 +40,7 @@ class Puppet::Type
                     end
                 end
                 
-                reqs << Puppet::Relationship[dep, self]
+                reqs << Puppet::Relationship.new(dep, self)
             }
         }
         
diff --git a/lib/puppet/relationship.rb b/lib/puppet/relationship.rb
index 0b958fa39..c611928f2 100644
--- a/lib/puppet/relationship.rb
+++ b/lib/puppet/relationship.rb
@@ -3,12 +3,15 @@
 #  Created by Luke A. Kanies on 2006-11-24.
 #  Copyright (c) 2006. All rights reserved.
 
-require 'puppet/external/gratr'
-
 # subscriptions are permanent associations determining how different
 # objects react to an event
 
-class Puppet::Relationship < GRATR::Edge
+# This is Puppet's class for modeling edges in its configuration graph.
+# 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
+
     # Return the callback
     def callback
         if label
@@ -30,17 +33,17 @@ class Puppet::Relationship < GRATR::Edge
     def initialize(source, target, label = {})
         if label
             unless label.is_a?(Hash)
-                raise Puppet::DevError, "The label must be a hash"
+                raise ArgumentError, "Relationship labels must be a hash"
             end
         
             if label[:event] and label[:event] != :NONE and ! label[:callback]
-                raise Puppet::DevError, "You must pass a callback for non-NONE events"
+                raise ArgumentError, "You must pass a callback for non-NONE events"
             end
         else
             label = {}
         end
-        
-        super(source, target, label)
+
+        @source, @target, @label = source, target, label
     end
     
     # Does the passed event match our event?  This is where the meaning
diff --git a/spec/unit/other/pgraph.rb b/spec/unit/other/pgraph.rb
index 9446a8371..41835ebc7 100755
--- a/spec/unit/other/pgraph.rb
+++ b/spec/unit/other/pgraph.rb
@@ -50,8 +50,8 @@ describe Puppet::PGraph, " when matching edges" do
 	    @none = Puppet::Event.new(:source => "a", :event => :NONE)
 
 	    @edges = {}
-	    @edges["a/b"] = Puppet::Relationship["a", "b", {:event => :yay, :callback => :refresh}]
-	    @edges["a/c"] = Puppet::Relationship["a", "c", {:event => :yay, :callback => :refresh}]
+	    @edges["a/b"] = Puppet::Relationship.new("a", "b", {:event => :yay, :callback => :refresh})
+	    @edges["a/c"] = Puppet::Relationship.new("a", "c", {:event => :yay, :callback => :refresh})
 	    @graph.add_edge!(@edges["a/b"])
     end
 
@@ -65,7 +65,9 @@ describe Puppet::PGraph, " when matching edges" do
 
     it "should match multiple edges" do
 	    @graph.add_edge!(@edges["a/c"])
-        @graph.matching_edges([@event]).sort.should == [@edges["a/b"], @edges["a/c"]].sort
+        edges = @graph.matching_edges([@event])
+        edges.should be_include(@edges["a/b"])
+        edges.should be_include(@edges["a/c"])
     end
 end
 
diff --git a/spec/unit/relationship.rb b/spec/unit/relationship.rb
new file mode 100755
index 000000000..5d90a9349
--- /dev/null
+++ b/spec/unit/relationship.rb
@@ -0,0 +1,149 @@
+#!/usr/bin/env ruby
+#
+#  Created by Luke Kanies on 2007-11-1.
+#  Copyright (c) 2006. All rights reserved.
+
+require File.dirname(__FILE__) + '/../spec_helper'
+require 'puppet/relationship'
+
+describe Puppet::Relationship do
+    before do
+        @edge = Puppet::Relationship.new(:a, :b)
+    end
+
+    it "should have a :source attribute" do
+        @edge.should respond_to(:source)
+    end
+
+    it "should have a :target attribute" do
+        @edge.should respond_to(:target)
+    end
+
+    it "should have a :label attribute" do
+        @edge.should respond_to(:label)
+    end
+
+    it "should provide a :ref method that describes the edge" do
+        @edge = Puppet::Relationship.new(stub("a", :ref => "a"), stub("b", :ref => "b"))
+        @edge.ref.should == "a => b"
+    end
+end
+
+describe Puppet::Relationship, " when initializing" do
+    before do
+        @edge = Puppet::Relationship.new(:a, :b, :testing => :foo)
+    end
+
+    it "should use the first argument as the source" do
+        @edge.source.should == :a
+    end
+
+    it "should use the second argument as the target" do
+        @edge.target.should == :b
+    end
+
+    it "should use the third argument as the label" do
+        @edge.label.should == {:testing => :foo}
+    end
+
+    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)
+    end
+
+    it "should require the label to be a hash" do
+        proc { Puppet::Relationship.new(:a, :b, :event) }.should raise_error(ArgumentError)
+    end
+end
+
+describe Puppet::Relationship, " when interpreting the label" do
+    it "should default to an event of nil" 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
+    end
+
+    it "should default to a nil callback" do
+        @edge = Puppet::Relationship.new(:a, :b)
+        @edge.callback.should be_nil
+    end
+
+    it "should expose a provided callback via the :callback method" do
+        @edge = Puppet::Relationship.new(:a, :b, :callback => :testing)
+        @edge.callback.should == :testing
+    end
+end
+
+describe Puppet::Relationship, " when matching edges with no specified event" do
+    before do
+        @edge = Puppet::Relationship.new(:a, :b)
+    end
+
+    it "should not match :NONE" do
+        @edge.should_not be_match(:NONE)
+    end
+
+    it "should not match :ALL_EVENTS" do
+        @edge.should_not be_match(:NONE)
+    end
+
+    it "should not match any other events" do
+        @edge.should_not be_match(:whatever)
+    end
+end
+
+describe Puppet::Relationship, " when matching edges with :NONE as the event" do
+    before do
+        @edge = Puppet::Relationship.new(:a, :b, :event => :NONE)
+    end
+    it "should not match :NONE" do
+        @edge.should_not be_match(:NONE)
+    end
+
+    it "should not match :ALL_EVENTS" do
+        @edge.should_not be_match(:ALL_EVENTS)
+    end
+
+    it "should not match other events" do
+        @edge.should_not be_match(:yayness)
+    end
+end
+
+describe Puppet::Relationship, " when matching edges with :ALL as the event" do
+    before do
+        @edge = Puppet::Relationship.new(:a, :b, :event => :ALL_EVENTS, :callback => :whatever)
+    end
+
+    it "should not match :NONE" do
+        @edge.should_not be_match(:NONE)
+    end
+
+    it "should match :ALL_EVENTS" do
+        @edge.should be_match(:ALLEVENTS)
+    end
+
+    it "should match all other events" do
+        @edge.should be_match(:foo)
+    end
+end
+
+describe Puppet::Relationship, " when matching edges with a non-standard event" do
+    before do
+        @edge = Puppet::Relationship.new(:a, :b, :event => :random, :callback => :whatever)
+    end
+
+    it "should not match :NONE" do
+        @edge.should_not be_match(:NONE)
+    end
+
+    it "should not match :ALL_EVENTS" do
+        @edge.should_not be_match(:ALL_EVENTS)
+    end
+
+    it "should match events with the same name" do
+        @edge.should be_match(:random)
+    end
+end
diff --git a/test/other/relationship.rb b/test/other/relationship.rb
deleted file mode 100755
index d890a031d..000000000
--- a/test/other/relationship.rb
+++ /dev/null
@@ -1,73 +0,0 @@
-#!/usr/bin/env ruby
-#
-#  Created by Luke A. Kanies on 2006-11-24.
-#  Copyright (c) 2006. All rights reserved.
-
-require File.dirname(__FILE__) + '/../lib/puppettest'
-
-require 'puppettest'
-require 'puppet/relationship'
-
-class TestRelationship < Test::Unit::TestCase
-    include PuppetTest
-    
-    def test_initialize
-        rel = Puppet::Relationship
-        
-        [["source", "target", "label"],
-         ["source", "target", {:event => :nothing}]
-        ].each do |ary|
-            # Make sure the label is required
-            assert_raise(Puppet::DevError) do
-                rel.new(*ary)
-            end
-        end
-    end
-    
-    def test_attributes
-        rel = Puppet::Relationship
-        
-        i = nil
-        assert_nothing_raised do
-            i = rel.new "source", "target", :event => :yay, :callback => :boo
-        end
-        
-        assert_equal(:yay, i.event, "event was not retrieved")
-        assert_equal(:boo, i.callback, "callback was not retrieved")
-        
-        # Now try it with nil values
-        assert_nothing_raised("failed to create with no event or callback") {
-            i = rel.new "source", "target"
-        }
-        
-        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
-
diff --git a/test/other/relationships.rb b/test/other/relationships.rb
index 6771b02ca..bfe7e88d7 100755
--- a/test/other/relationships.rb
+++ b/test/other/relationships.rb
@@ -170,7 +170,9 @@ class TestRelationships < Test::Unit::TestCase
         assert_nothing_raised do
             reqs = exec.autorequire
         end
-        assert_equal([Puppet::Relationship[file, exec]], reqs)
+        assert_instance_of(Puppet::Relationship, reqs[0], "Did not return a relationship edge")
+        assert_equal(file, reqs[0].source, "Did not set the autorequire source correctly")
+        assert_equal(exec, reqs[0].target, "Did not set the autorequire target correctly")
         
         # Now make sure that these relationships are added to the 
         # relationship graph