Merge branch 'ticket/next/4590' into next

Conflicts:
	lib/puppet/util/monkey_patches.rb
            -- two unrelated additions had been made, kept them both.

4 files changed, 257 insertions, 24 deletions

diff --git a/spec/unit/resource/catalog_spec.rb b/spec/unit/resource/catalog_spec.rb
index 2b6beb5e9..fbfe29ff3 100755
--- a/spec/unit/resource/catalog_spec.rb
+++ b/spec/unit/resource/catalog_spec.rb
@@ -374,7 +374,7 @@ describe Puppet::Resource::Catalog, "when compiling" do
       @original.add_edge(@r1,@r2)
       @original.filter do |r|
         r == @r1
-      end.edge(@r1,@r2).should be_empty
+      end.edge?(@r1,@r2).should_not be
     end
   end
 
@@ -933,8 +933,8 @@ describe Puppet::Resource::Catalog, "when converting to pson" do
     @catalog.add_edge(one, two)
     @catalog.add_edge(two, three)
 
-    @catalog.edge(one, two  ).expects(:to_pson_data_hash).returns "one_two_pson"
-    @catalog.edge(two, three).expects(:to_pson_data_hash).returns "two_three_pson"
+    @catalog.edges_between(one, two  )[0].expects(:to_pson_data_hash).returns "one_two_pson"
+    @catalog.edges_between(two, three)[0].expects(:to_pson_data_hash).returns "two_three_pson"
 
     PSON.parse(@catalog.to_pson,:create_additions => false)['data']['edges'].sort.should == %w{one_two_pson two_three_pson}.sort
   end
diff --git a/spec/unit/simple_graph_spec.rb b/spec/unit/simple_graph_spec.rb
index 2ca8888c5..fa0bcb06a 100755
--- a/spec/unit/simple_graph_spec.rb
+++ b/spec/unit/simple_graph_spec.rb
@@ -31,12 +31,6 @@ describe Puppet::SimpleGraph do
     proc { @graph.to_dot_graph }.should_not raise_error
   end
 
-  it "should always put its edges first when printing yaml" do
-    @graph = Puppet::SimpleGraph.new
-    @graph.add_edge(:one, :two)
-    @graph.to_yaml_properties[0].should == "@edges"
-  end
-
   describe "when managing vertices" do
     before do
       @graph = Puppet::SimpleGraph.new
@@ -117,16 +111,31 @@ describe Puppet::SimpleGraph do
       @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, :callback => :awesome)
-      @graph.add_edge(edge)
-      @graph.edge_label(:one, :two).should == {:callback => :awesome}
-    end
+    describe "when retrieving edges between two nodes" do
+      it "should handle the case of nodes not in the graph" do
+        @graph.edges_between(:one, :two).should == []
+      end
 
-    it "should provide a method for retrieving an edge" do
-      edge = Puppet::Relationship.new(:one, :two)
-      @graph.add_edge(edge)
-      @graph.edge(:one, :two).should equal(edge)
+      it "should handle the case of nodes with no edges between them" do
+        @graph.add_vertex(:one)
+        @graph.add_vertex(:two)
+        @graph.edges_between(:one, :two).should == []
+      end
+
+      it "should handle the case of nodes connected by a single edge" do
+        edge = Puppet::Relationship.new(:one, :two)
+        @graph.add_edge(edge)
+        @graph.edges_between(:one, :two).length.should == 1
+        @graph.edges_between(:one, :two)[0].should equal(edge)
+      end
+
+      it "should handle the case of nodes connected by multiple edges" do
+        edge1 = Puppet::Relationship.new(:one, :two, :callback => :foo)
+        edge2 = Puppet::Relationship.new(:one, :two, :callback => :bar)
+        @graph.add_edge(edge1)
+        @graph.add_edge(edge2)
+        Set.new(@graph.edges_between(:one, :two)).should == Set.new([edge1, edge2])
+      end
     end
 
     it "should add the edge source as a vertex if it is not already" do
@@ -253,7 +262,7 @@ describe Puppet::SimpleGraph do
 
     it "should retain labels on edges" do
       @graph.add_edge(:one, :two, :callback => :awesome)
-      edge = @graph.reversal.edge(:two, :one)
+      edge = @graph.reversal.edges_between(:two, :one)[0]
       edge.label.should == {:callback => :awesome}
     end
   end
@@ -522,14 +531,14 @@ describe Puppet::SimpleGraph do
     it "should not add labels to edges that have none" do
       @depgraph.add_edge(@two, @three)
       splice
-      @depgraph.edge_label("c", "i").should == {}
+      @depgraph.edges_between("c", "i")[0].label.should == {}
     end
 
     it "should copy labels over edges that have none" do
       @depgraph.add_edge("c", @three, {:callback => :refresh})
       splice
       # And make sure the label got copied.
-      @depgraph.edge_label("c", "i").should == {:callback => :refresh}
+      @depgraph.edges_between("c", "i")[0].label.should == {:callback => :refresh}
     end
 
     it "should not replace a label with a nil label" do
@@ -537,7 +546,7 @@ describe Puppet::SimpleGraph do
       @depgraph.add_edge(@middle, @three)
       @depgraph.add_edge("c", @three, {:callback => :refresh})
       splice
-      @depgraph.edge_label("c", "i").should == {:callback => :refresh}
+      @depgraph.edges_between("c", "i")[0].label.should == {:callback => :refresh}
     end
 
     it "should copy labels to all created edges" do
@@ -547,8 +556,207 @@ describe Puppet::SimpleGraph do
       @three.each do |child|
         edge = Puppet::Relationship.new("c", child)
         @depgraph.should be_edge(edge.source, edge.target)
-        @depgraph.edge_label(edge.source, edge.target).should == {:callback => :refresh}
+        @depgraph.edges_between(edge.source, edge.target)[0].label.should == {:callback => :refresh}
+      end
+    end
+  end
+
+  it "should serialize to YAML using the old format by default" do
+    Puppet::SimpleGraph.use_new_yaml_format.should == false
+  end
+
+  describe "(yaml tests)" do
+    def empty_graph(graph)
+    end
+
+    def one_vertex_graph(graph)
+      graph.add_vertex(:a)
+    end
+
+    def graph_without_edges(graph)
+      [:a, :b, :c].each { |x| graph.add_vertex(x) }
+    end
+
+    def one_edge_graph(graph)
+      graph.add_edge(:a, :b)
+    end
+
+    def many_edge_graph(graph)
+      graph.add_edge(:a, :b)
+      graph.add_edge(:a, :c)
+      graph.add_edge(:b, :d)
+      graph.add_edge(:c, :d)
+    end
+
+    def labeled_edge_graph(graph)
+      graph.add_edge(:a, :b, :callback => :foo, :event => :bar)
+    end
+
+    def overlapping_edge_graph(graph)
+      graph.add_edge(:a, :b, :callback => :foo, :event => :bar)
+      graph.add_edge(:a, :b, :callback => :biz, :event => :baz)
+    end
+
+    def self.all_test_graphs
+      [:empty_graph, :one_vertex_graph, :graph_without_edges, :one_edge_graph, :many_edge_graph, :labeled_edge_graph,
+       :overlapping_edge_graph]
+    end
+
+    def object_ids(enumerable)
+      # Return a sorted list of the object id's of the elements of an
+      # enumerable.
+      enumerable.collect { |x| x.object_id }.sort
+    end
+
+    def graph_to_yaml(graph, which_format)
+      previous_use_new_yaml_format = Puppet::SimpleGraph.use_new_yaml_format
+      Puppet::SimpleGraph.use_new_yaml_format = (which_format == :new)
+      ZAML.dump(graph)
+    ensure
+      Puppet::SimpleGraph.use_new_yaml_format = previous_use_new_yaml_format
+    end
+
+    # Test serialization of graph to YAML.
+    [:old, :new].each do |which_format|
+      all_test_graphs.each do |graph_to_test|
+        it "should be able to serialize #{graph_to_test} to YAML (#{which_format} format)" do
+          graph = Puppet::SimpleGraph.new
+          send(graph_to_test, graph)
+          yaml_form = graph_to_yaml(graph, which_format)
+
+          # Hack the YAML so that objects in the Puppet namespace get
+          # changed to YAML::DomainType objects.  This lets us inspect
+          # the serialized objects easily without invoking any
+          # yaml_initialize hooks.
+          yaml_form.gsub!('!ruby/object:Puppet::', '!hack/object:Puppet::')
+          serialized_object = YAML.load(yaml_form)
+
+          # Check that the object contains instance variables @edges and
+          # @vertices only.  @reversal is also permitted, but we don't
+          # check it, because it is going to be phased out.
+          serialized_object.type_id.should == 'object:Puppet::SimpleGraph'
+          serialized_object.value.keys.reject { |x| x == 'reversal' }.sort.should == ['edges', 'vertices']
+
+          # Check edges by forming a set of tuples (source, target,
+          # callback, event) based on the graph and the YAML and make sure
+          # they match.
+          edges = serialized_object.value['edges']
+          edges.should be_a(Array)
+          expected_edge_tuples = graph.edges.collect { |edge| [edge.source, edge.target, edge.callback, edge.event] }
+          actual_edge_tuples = edges.collect do |edge|
+            edge.type_id.should == 'object:Puppet::Relationship'
+            %w{source target}.each { |x| edge.value.keys.should include(x) }
+            edge.value.keys.each { |x| ['source', 'target', 'callback', 'event'].should include(x) }
+            %w{source target callback event}.collect { |x| edge.value[x] }
+          end
+          Set.new(actual_edge_tuples).should == Set.new(expected_edge_tuples)
+          actual_edge_tuples.length.should == expected_edge_tuples.length
+
+          # Check vertices one by one.
+          vertices = serialized_object.value['vertices']
+          if which_format == :old
+            vertices.should be_a(Hash)
+            Set.new(vertices.keys).should == Set.new(graph.vertices)
+            vertices.each do |key, value|
+              value.type_id.should == 'object:Puppet::SimpleGraph::VertexWrapper'
+              value.value.keys.sort.should == %w{adjacencies vertex}
+              value.value['vertex'].should equal(key)
+              adjacencies = value.value['adjacencies']
+              adjacencies.should be_a(Hash)
+              Set.new(adjacencies.keys).should == Set.new([:in, :out])
+              [:in, :out].each do |direction|
+                adjacencies[direction].should be_a(Hash)
+                expected_adjacent_vertices = Set.new(graph.adjacent(key, :direction => direction, :type => :vertices))
+                Set.new(adjacencies[direction].keys).should == expected_adjacent_vertices
+                adjacencies[direction].each do |adj_key, adj_value|
+                  # Since we already checked edges, just check consistency
+                  # with edges.
+                  desired_source = direction == :in ? adj_key : key
+                  desired_target = direction == :in ? key : adj_key
+                  expected_edges = edges.select do |edge|
+                    edge.value['source'] == desired_source && edge.value['target'] == desired_target
+                  end
+                  adj_value.should be_a(Set)
+                  if object_ids(adj_value) != object_ids(expected_edges)
+                    raise "For vertex #{key.inspect}, direction #{direction.inspect}: expected adjacencies #{expected_edges.inspect} but got #{adj_value.inspect}"
+                  end
+                end
+              end
+            end
+          else
+            vertices.should be_a(Array)
+            Set.new(vertices).should == Set.new(graph.vertices)
+            vertices.length.should == graph.vertices.length
+          end
+        end
+      end
+
+      # Test deserialization of graph from YAML.  This presumes the
+      # correctness of serialization to YAML, which has already been
+      # tested.
+      all_test_graphs.each do |graph_to_test|
+        it "should be able to deserialize #{graph_to_test} from YAML (#{which_format} format)" do
+          reference_graph = Puppet::SimpleGraph.new
+          send(graph_to_test, reference_graph)
+          yaml_form = graph_to_yaml(reference_graph, which_format)
+          recovered_graph = YAML.load(yaml_form)
+
+          # Test that the recovered vertices match the vertices in the
+          # reference graph.
+          expected_vertices = reference_graph.vertices.to_a
+          recovered_vertices = recovered_graph.vertices.to_a
+          Set.new(recovered_vertices).should == Set.new(expected_vertices)
+          recovered_vertices.length.should == expected_vertices.length
+
+          # Test that the recovered edges match the edges in the
+          # reference graph.
+          expected_edge_tuples = reference_graph.edges.collect do |edge|
+            [edge.source, edge.target, edge.callback, edge.event]
+          end
+          recovered_edge_tuples = recovered_graph.edges.collect do |edge|
+            [edge.source, edge.target, edge.callback, edge.event]
+          end
+          Set.new(recovered_edge_tuples).should == Set.new(expected_edge_tuples)
+          recovered_edge_tuples.length.should == expected_edge_tuples.length
+
+          # We ought to test that the recovered graph is self-consistent
+          # too.  But we're not going to bother with that yet because
+          # the internal representation of the graph is about to change.
+        end
+      end
+
+      it "should be able to serialize a graph where the vertices contain backreferences to the graph (#{which_format} format)" do
+        reference_graph = Puppet::SimpleGraph.new
+        vertex = Object.new
+        vertex.instance_eval { @graph = reference_graph }
+        reference_graph.add_edge(vertex, :other_vertex)
+        yaml_form = graph_to_yaml(reference_graph, which_format)
+        recovered_graph = YAML.load(yaml_form)
+
+        recovered_graph.vertices.length.should == 2
+        recovered_vertex = recovered_graph.vertices.reject { |x| x.is_a?(Symbol) }[0]
+        recovered_vertex.instance_eval { @graph }.should equal(recovered_graph)
+        recovered_graph.edges.length.should == 1
+        recovered_edge = recovered_graph.edges[0]
+        recovered_edge.source.should equal(recovered_vertex)
+        recovered_edge.target.should == :other_vertex
+      end
+    end
+
+    it "should serialize properly when used as a base class" do
+      class Puppet::TestDerivedClass < Puppet::SimpleGraph
+        attr_accessor :foo
       end
+      derived = Puppet::TestDerivedClass.new
+      derived.add_edge(:a, :b)
+      derived.foo = 1234
+      recovered_derived = YAML.load(YAML.dump(derived))
+      recovered_derived.class.should equal(Puppet::TestDerivedClass)
+      recovered_derived.edges.length.should == 1
+      recovered_derived.edges[0].source.should == :a
+      recovered_derived.edges[0].target.should == :b
+      recovered_derived.vertices.length.should == 2
+      recovered_derived.foo.should == 1234
     end
   end
 end
diff --git a/spec/unit/util/monkey_patches_spec.rb b/spec/unit/util/monkey_patches_spec.rb
index b0f61c808..049ed1044 100644
--- a/spec/unit/util/monkey_patches_spec.rb
+++ b/spec/unit/util/monkey_patches_spec.rb
@@ -5,3 +5,29 @@ Dir.chdir(File.dirname(__FILE__)) { (s = lambda { |f| File.exist?(f) ? require(f
 require 'puppet/util/monkey_patches'
 
 
+
+describe "yaml deserialization" do
+  it "should call yaml_initialize when deserializing objects that have that method defined" do
+    class Puppet::TestYamlInitializeClass
+      attr_reader :foo
+
+      def yaml_initialize(tag, var)
+        var.should == {'foo' => 100}
+        instance_variables.should == []
+        @foo = 200
+      end
+    end
+
+    obj = YAML.load("--- !ruby/object:Puppet::TestYamlInitializeClass\n  foo: 100")
+    obj.foo.should == 200
+  end
+
+  it "should not call yaml_initialize if not defined" do
+    class Puppet::TestYamlNonInitializeClass
+      attr_reader :foo
+    end
+
+    obj = YAML.load("--- !ruby/object:Puppet::TestYamlNonInitializeClass\n  foo: 100")
+    obj.foo.should == 100
+  end
+end
diff --git a/spec/unit/util/zaml_spec.rb b/spec/unit/util/zaml_spec.rb
index b223f89d4..358c6aa11 100755
--- a/spec/unit/util/zaml_spec.rb
+++ b/spec/unit/util/zaml_spec.rb
@@ -61,4 +61,3 @@ describe "Pure ruby yaml implementation" do
     x2[2].should equal(x2)
   end
 end
-