From be85afbffdd68270eda4588e606ca802cfb4130c Mon Sep 17 00:00:00 2001
From: Clemens Tolboom <clemens@build2be.com>
Date: Sat, 1 Jun 2013 17:53:37 +0200
Subject: [PATCH 1/6] Setting up test and fixtures for subgraphs.

---
 tests/Fhaculty/Graph/GraphVizTest.php         | 32 +++++++++++++++++++
 .../fixtures/graph-cluster-complex-tree.dot   | 26 +++++++++++++++
 .../Graph/fixtures/graph-cluster-tree.dot     | 14 ++++++++
 .../Fhaculty/Graph/fixtures/graph-cluster.dot |  9 ++++++
 tests/Fhaculty/Graph/fixtures/out/README.md   |  1 +
 5 files changed, 82 insertions(+)
 create mode 100644 tests/Fhaculty/Graph/fixtures/graph-cluster-complex-tree.dot
 create mode 100644 tests/Fhaculty/Graph/fixtures/graph-cluster-tree.dot
 create mode 100644 tests/Fhaculty/Graph/fixtures/graph-cluster.dot
 create mode 100644 tests/Fhaculty/Graph/fixtures/out/README.md

diff --git a/tests/Fhaculty/Graph/GraphVizTest.php b/tests/Fhaculty/Graph/GraphVizTest.php
index 181f6241..73f630b8 100644
--- a/tests/Fhaculty/Graph/GraphVizTest.php
+++ b/tests/Fhaculty/Graph/GraphVizTest.php
@@ -191,6 +191,38 @@ public function testEdgeLabels()
         $this->assertEquals($expected, $this->getDotScriptForGraph($graph));
     }
 
+    public function testSubgraph()
+    {
+        $graph = new Graph();
+        $graph->createVertex('n1')->setGroup(0);
+        $graph->createVertex('n2');
+        $graph->createVertex('n21')->setGroup(1);
+        $graph->createVertex('n22')->setGroup(1);
+
+        $expected = file_get_contents(__DIR__ . '/fixtures/graph-cluster.dot');
+
+        $this->assertEquals($expected, $this->getDotScriptForGraph($graph));
+
+    }
+
+    public function testSubgraphTree()
+    {
+        $graph = new Graph();
+        $graph->createVertex('n1')->setGroup(0);
+        $graph->createVertex('n2');
+        $graph->createVertex('n21')->setGroup(1);
+        $graph->createVertex('n211')->setGroup(1);
+        $graph->createVertex('n212')->setGroup(1);
+        $graph->createVertex('n22')->setGroup(1);
+
+        $in_file = __DIR__ . '/fixtures/graph-cluster-tree.dot';
+        $out_file = __DIR__ . '/fixtures/out/graph-cluster-tree.dot';
+        $expected = file_get_contents($in_file);
+        file_put_contents($out_file, $this->getDotScriptForGraph($graph));
+
+        $this->assertEquals($expected, $this->getDotScriptForGraph($graph));
+
+    }
 
     private function getDotScriptForGraph(Graph $graph)
     {
diff --git a/tests/Fhaculty/Graph/fixtures/graph-cluster-complex-tree.dot b/tests/Fhaculty/Graph/fixtures/graph-cluster-complex-tree.dot
new file mode 100644
index 00000000..5f3a6f2f
--- /dev/null
+++ b/tests/Fhaculty/Graph/fixtures/graph-cluster-complex-tree.dot
@@ -0,0 +1,26 @@
+// Graph has a name (tooltip in SVG output)
+graph Faculty {
+  // cluster_ is a special prefix making it a box with a label
+  subgraph cluster_graph {
+    label = "<<package>>\nGraph"
+
+    graph_php [
+      label = "Graph.php"
+    ]
+
+    // Note the absence of cluster_ making their groupness hard to tell
+    subgraph algorithm {
+      label = "<<package>>\nAlgorithm"
+        subgraph cluster_maxflow {
+          label = "<<package>>\nMaxFlow"
+          EdmondsKarp
+      }
+      AlgorithmX -- AlgorithmY
+    }
+    subgraph cluster_edge {
+      label = "<<package>>\nEdge"
+      P
+      Q
+    }
+  }
+}
diff --git a/tests/Fhaculty/Graph/fixtures/graph-cluster-tree.dot b/tests/Fhaculty/Graph/fixtures/graph-cluster-tree.dot
new file mode 100644
index 00000000..afc20910
--- /dev/null
+++ b/tests/Fhaculty/Graph/fixtures/graph-cluster-tree.dot
@@ -0,0 +1,14 @@
+graph G {
+  "n1"
+  "n2"
+  subgraph cluster_1 {
+    label = 1
+    "n21"
+    subgraph cluster_2 {
+      label = 2
+      "n211"
+      "n212"
+    }
+    "n22"
+  }
+}
diff --git a/tests/Fhaculty/Graph/fixtures/graph-cluster.dot b/tests/Fhaculty/Graph/fixtures/graph-cluster.dot
new file mode 100644
index 00000000..f294c167
--- /dev/null
+++ b/tests/Fhaculty/Graph/fixtures/graph-cluster.dot
@@ -0,0 +1,9 @@
+graph G {
+  "n1"
+  "n2"
+  subgraph cluster_1 {
+    label = 1
+    "n21"
+    "n22"
+  }
+}
diff --git a/tests/Fhaculty/Graph/fixtures/out/README.md b/tests/Fhaculty/Graph/fixtures/out/README.md
new file mode 100644
index 00000000..b986f962
--- /dev/null
+++ b/tests/Fhaculty/Graph/fixtures/out/README.md
@@ -0,0 +1 @@
+This directory is used to generate output from some fixtures for visual inspection.

From 19ce5483f319404c873db57605acf041742e3f37 Mon Sep 17 00:00:00 2001
From: Clemens Tolboom <clemens@build2be.com>
Date: Sat, 1 Jun 2013 18:43:08 +0200
Subject: [PATCH 2/6] Added GroupTest.

---
 tests/Fhaculty/Graph/Algorithm/GroupTest.php | 31 ++++++++++++++++++++
 1 file changed, 31 insertions(+)
 create mode 100644 tests/Fhaculty/Graph/Algorithm/GroupTest.php

diff --git a/tests/Fhaculty/Graph/Algorithm/GroupTest.php b/tests/Fhaculty/Graph/Algorithm/GroupTest.php
new file mode 100644
index 00000000..d3f169f8
--- /dev/null
+++ b/tests/Fhaculty/Graph/Algorithm/GroupTest.php
@@ -0,0 +1,31 @@
+<?php
+
+use Fhaculty\Graph\Algorithm\Groups as Groups;
+use Fhaculty\Graph\Graph;
+
+class GroupTest extends TestCase
+{
+    public function testGroupsTwo()
+    {
+        $graph = new Graph();
+        $graph->createVertex('no group');
+        $graph->createVertex('group 0')->setGroup(0);
+
+        $alg = new Groups($graph);
+        $this->assertEquals(2, $alg->getNumberOfGroups(), 'Empty group must differ from 0');
+
+    }
+
+    public function testGroupsOneAndTwo()
+    {
+
+        $graph = new Graph();
+        $graph->createVertex('group 1')->setGroup(1);
+        $graph->createVertex('group 2.1')->setGroup(2);
+        $graph->createVertex('group 2.2')->setGroup(2);
+
+        $alg = new Groups($graph);
+        $this->assertEquals(2, $alg->getNumberOfGroups());
+
+    }
+}

From 190eeba1569e5333c98da9f282ac2c0013453a18 Mon Sep 17 00:00:00 2001
From: Clemens Tolboom <clemens@build2be.com>
Date: Tue, 4 Jun 2013 15:39:00 +0200
Subject: [PATCH 3/6] Add NULL for no group.

---
 lib/Fhaculty/Graph/Algorithm/Groups.php | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/Fhaculty/Graph/Algorithm/Groups.php b/lib/Fhaculty/Graph/Algorithm/Groups.php
index 3a087f74..835c4060 100644
--- a/lib/Fhaculty/Graph/Algorithm/Groups.php
+++ b/lib/Fhaculty/Graph/Algorithm/Groups.php
@@ -75,7 +75,9 @@ public function getGroups()
     {
         $groups = array();
         foreach ($this->graph->getVertices() as $vertex) {
-            $groups[$vertex->getGroup()] = true;
+            $group = $vertex->getGroup();
+            $group = (is_null($group)? "_NULL_" : $group);
+            $groups[$group] = true;
         }
 
         return array_keys($groups);

From 7a0d0fdb1628de769c1f003a2f0738edc95baf3c Mon Sep 17 00:00:00 2001
From: Clemens Tolboom <clemens@build2be.com>
Date: Tue, 4 Jun 2013 15:40:22 +0200
Subject: [PATCH 4/6] Allow for string as Group ID.

---
 lib/Fhaculty/Graph/Vertex.php | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/lib/Fhaculty/Graph/Vertex.php b/lib/Fhaculty/Graph/Vertex.php
index c85e3434..0804f46c 100644
--- a/lib/Fhaculty/Graph/Vertex.php
+++ b/lib/Fhaculty/Graph/Vertex.php
@@ -218,10 +218,10 @@ public static function getAll($vertices, $by = self::ORDER_FIFO, $desc = false)
     /**
      * group number
      *
-     * @var int
+     * @var int|string
      * @see Vertex::setGroup()
      */
-    private $group = 0;
+    private $group = NULL;
 
     /**
      * Creates a Vertex (MUST NOT BE CALLED MANUALLY!)
@@ -301,15 +301,11 @@ public function getFlow()
     /**
      * set group number of this vertex
      *
-     * @param  int                      $group
+     * @param  int | string             $group
      * @return Vertex                   $this (chainable)
-     * @throws InvalidArgumentException if group is not numeric
      */
     public function setGroup($group)
     {
-        if (!is_int($group)) {
-            throw new InvalidArgumentException('Invalid group number');
-        }
         $this->group = $group;
 
         return $this;
@@ -318,7 +314,7 @@ public function setGroup($group)
     /**
      * get group number
      *
-     * @return int
+     * @return int| string
      */
     public function getGroup()
     {

From 1ca74f007f26cf0f72f9e52fd2195a1b6940175e Mon Sep 17 00:00:00 2001
From: Clemens Tolboom <clemens@build2be.com>
Date: Tue, 4 Jun 2013 15:41:49 +0200
Subject: [PATCH 5/6] Add string groupIDs to tests.

---
 tests/Fhaculty/Graph/GraphVizTest.php         | 20 ++++----
 .../Graph/fixtures/graph-cluster-tree.dot     | 25 ++++++---
 .../Fhaculty/Graph/fixtures/graph-cluster.dot | 20 ++++++--
 .../Graph/fixtures/out/graph-cluster-tree.dot | 25 +++++++++
 .../fixtures/out/in/graph-cluster-tree.svg    | 51 +++++++++++++++++++
 5 files changed, 119 insertions(+), 22 deletions(-)
 create mode 100644 tests/Fhaculty/Graph/fixtures/out/graph-cluster-tree.dot
 create mode 100644 tests/Fhaculty/Graph/fixtures/out/in/graph-cluster-tree.svg

diff --git a/tests/Fhaculty/Graph/GraphVizTest.php b/tests/Fhaculty/Graph/GraphVizTest.php
index 73f630b8..f5c2fed4 100644
--- a/tests/Fhaculty/Graph/GraphVizTest.php
+++ b/tests/Fhaculty/Graph/GraphVizTest.php
@@ -194,10 +194,10 @@ public function testEdgeLabels()
     public function testSubgraph()
     {
         $graph = new Graph();
-        $graph->createVertex('n1')->setGroup(0);
-        $graph->createVertex('n2');
-        $graph->createVertex('n21')->setGroup(1);
-        $graph->createVertex('n22')->setGroup(1);
+        $graph->createVertex('n1')->setGroup("A");
+        $graph->createVertex('n2')->setGroup("B");
+        $graph->createVertex('n21')->setGroup("B:A");
+        $graph->createVertex('n22')->setGroup("B:A");
 
         $expected = file_get_contents(__DIR__ . '/fixtures/graph-cluster.dot');
 
@@ -208,12 +208,12 @@ public function testSubgraph()
     public function testSubgraphTree()
     {
         $graph = new Graph();
-        $graph->createVertex('n1')->setGroup(0);
-        $graph->createVertex('n2');
-        $graph->createVertex('n21')->setGroup(1);
-        $graph->createVertex('n211')->setGroup(1);
-        $graph->createVertex('n212')->setGroup(1);
-        $graph->createVertex('n22')->setGroup(1);
+        $graph->createVertex('n1')->setGroup('A');
+        $graph->createVertex('n21')->setGroup("B");
+        $graph->createVertex('n211')->setGroup('B:A');
+        $graph->createVertex('n212')->setGroup('B:A');
+        $graph->createVertex('n2111')->setGroup('B:A:A');
+        $graph->createVertex('n3');
 
         $in_file = __DIR__ . '/fixtures/graph-cluster-tree.dot';
         $out_file = __DIR__ . '/fixtures/out/graph-cluster-tree.dot';
diff --git a/tests/Fhaculty/Graph/fixtures/graph-cluster-tree.dot b/tests/Fhaculty/Graph/fixtures/graph-cluster-tree.dot
index afc20910..19abd1b2 100644
--- a/tests/Fhaculty/Graph/fixtures/graph-cluster-tree.dot
+++ b/tests/Fhaculty/Graph/fixtures/graph-cluster-tree.dot
@@ -1,14 +1,25 @@
 graph G {
-  "n1"
-  "n2"
-  subgraph cluster_1 {
-    label = 1
+  subgraph cluster_A {
+    label = "A"
+    "n1"
+  }
+  subgraph cluster_B {
+    label = "B"
     "n21"
-    subgraph cluster_2 {
-      label = 2
+    subgraph cluster_B_A {
+      label = "B:A"
       "n211"
       "n212"
+      subgraph cluster_B_A_A {
+        label = "B:A:A"
+        "n2111"
+      }
     }
-    "n22"
   }
+  "n1"
+  "n21"
+  "n211"
+  "n212"
+  "n2111"
+  "n3"
 }
diff --git a/tests/Fhaculty/Graph/fixtures/graph-cluster.dot b/tests/Fhaculty/Graph/fixtures/graph-cluster.dot
index f294c167..613d1bc2 100644
--- a/tests/Fhaculty/Graph/fixtures/graph-cluster.dot
+++ b/tests/Fhaculty/Graph/fixtures/graph-cluster.dot
@@ -1,9 +1,19 @@
 graph G {
+  subgraph cluster_A {
+    label = "A"
+    "n1"
+  }
+  subgraph cluster_B {
+    label = "B"
+    "n2"
+    subgraph cluster_B_A {
+      label = "B:A"
+      "n21"
+      "n22"
+    }
+  }
   "n1"
   "n2"
-  subgraph cluster_1 {
-    label = 1
-    "n21"
-    "n22"
-  }
+  "n21"
+  "n22"
 }
diff --git a/tests/Fhaculty/Graph/fixtures/out/graph-cluster-tree.dot b/tests/Fhaculty/Graph/fixtures/out/graph-cluster-tree.dot
new file mode 100644
index 00000000..19abd1b2
--- /dev/null
+++ b/tests/Fhaculty/Graph/fixtures/out/graph-cluster-tree.dot
@@ -0,0 +1,25 @@
+graph G {
+  subgraph cluster_A {
+    label = "A"
+    "n1"
+  }
+  subgraph cluster_B {
+    label = "B"
+    "n21"
+    subgraph cluster_B_A {
+      label = "B:A"
+      "n211"
+      "n212"
+      subgraph cluster_B_A_A {
+        label = "B:A:A"
+        "n2111"
+      }
+    }
+  }
+  "n1"
+  "n21"
+  "n211"
+  "n212"
+  "n2111"
+  "n3"
+}
diff --git a/tests/Fhaculty/Graph/fixtures/out/in/graph-cluster-tree.svg b/tests/Fhaculty/Graph/fixtures/out/in/graph-cluster-tree.svg
new file mode 100644
index 00000000..25f070b4
--- /dev/null
+++ b/tests/Fhaculty/Graph/fixtures/out/in/graph-cluster-tree.svg
@@ -0,0 +1,51 @@
+<?xml version="1.0" encoding="UTF-8" standalone="no"?>
+<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN"
+ "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd">
+<!-- Generated by graphviz version 2.26.3 (20100126.1600)
+ -->
+<!-- Title: G Pages: 1 -->
+<svg width="450pt" height="138pt"
+ viewBox="0.00 0.00 450.00 138.00" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
+<g id="graph1" class="graph" transform="scale(1 1) rotate(0) translate(4 134)">
+<title>G</title>
+<polygon fill="white" stroke="white" points="-4,5 -4,-134 447,-134 447,5 -4,5"/>
+<g id="graph2" class="cluster"><title>cluster_1</title>
+<polygon fill="none" stroke="black" points="136,-8 136,-122 434,-122 434,-8 136,-8"/>
+<text text-anchor="middle" x="285" y="-105.4" font-family="Times,serif" font-size="14.00">1</text>
+</g>
+<g id="graph3" class="cluster"><title>cluster_2</title>
+<polygon fill="none" stroke="black" points="280,-16 280,-91 426,-91 426,-16 280,-16"/>
+<text text-anchor="middle" x="353" y="-74.4" font-family="Times,serif" font-size="14.00">2</text>
+</g>
+<!-- n1 -->
+<g id="node1" class="node"><title>n1</title>
+<ellipse fill="none" stroke="black" cx="27" cy="-42" rx="27" ry="18"/>
+<text text-anchor="middle" x="27" y="-36.9" font-family="Times,serif" font-size="14.00">n1</text>
+</g>
+<!-- n2 -->
+<g id="node2" class="node"><title>n2</title>
+<ellipse fill="none" stroke="black" cx="99" cy="-42" rx="27" ry="18"/>
+<text text-anchor="middle" x="99" y="-36.9" font-family="Times,serif" font-size="14.00">n2</text>
+</g>
+<!-- n21 -->
+<g id="node4" class="node"><title>n21</title>
+<ellipse fill="none" stroke="black" cx="243" cy="-42" rx="27" ry="18"/>
+<text text-anchor="middle" x="243" y="-36.9" font-family="Times,serif" font-size="14.00">n21</text>
+</g>
+<!-- n211 -->
+<g id="node6" class="node"><title>n211</title>
+<ellipse fill="none" stroke="black" cx="390" cy="-42" rx="28.0958" ry="18"/>
+<text text-anchor="middle" x="390" y="-36.9" font-family="Times,serif" font-size="14.00">n211</text>
+</g>
+<!-- n212 -->
+<g id="node7" class="node"><title>n212</title>
+<ellipse fill="none" stroke="black" cx="316" cy="-42" rx="28.0958" ry="18"/>
+<text text-anchor="middle" x="316" y="-36.9" font-family="Times,serif" font-size="14.00">n212</text>
+</g>
+<!-- n22 -->
+<g id="node8" class="node"><title>n22</title>
+<ellipse fill="none" stroke="black" cx="171" cy="-42" rx="27" ry="18"/>
+<text text-anchor="middle" x="171" y="-36.9" font-family="Times,serif" font-size="14.00">n22</text>
+</g>
+</g>
+</svg>

From 986643ff0611e65a8f936dccf999e4a7946e9ccb Mon Sep 17 00:00:00 2001
From: Clemens Tolboom <clemens@build2be.com>
Date: Tue, 4 Jun 2013 15:42:41 +0200
Subject: [PATCH 6/6] Make Graphviz cluster recursive.

---
 lib/Fhaculty/Graph/GraphViz.php | 68 +++++++++++++++++++++------------
 1 file changed, 44 insertions(+), 24 deletions(-)

diff --git a/lib/Fhaculty/Graph/GraphViz.php b/lib/Fhaculty/Graph/GraphViz.php
index cedb8a25..0993075c 100644
--- a/lib/Fhaculty/Graph/GraphViz.php
+++ b/lib/Fhaculty/Graph/GraphViz.php
@@ -299,33 +299,35 @@ public function createScript()
             $gid = 0;
             $indent = str_repeat($this->formatIndent, 2);
             // put each group of vertices in a separate subgraph cluster
-            foreach ($alg->getGroups() as $group) {
-                $script .= $this->formatIndent . 'subgraph cluster_' . $gid++ . ' {' . self::EOL .
-                           $indent . 'label = ' . $this->escape($group) . self::EOL;
-                foreach($alg->getVerticesGroup($group) as $vid => $vertex) {
-                    $layout = $this->getLayoutVertex($vertex);
-
-                    $script .= $indent . $this->escapeId($vid);
-                    if($layout){
-                        $script .= ' ' . $this->escapeAttributes($layout);
-                    }
-                    $script .= self::EOL;
+            $groups = $alg->getGroups();
+            $tree = array();
+            foreach ($groups as $group) {
+              $fragments = explode(":", $group);
+              $pointer = &$tree;
+              while (count($fragments)) {
+                $key = array_shift($fragments);
+                if (!isset($pointer[$key])) {
+                  $pointer[$key] = array();
                 }
-                $script .= '  }' . self::EOL;
+                $pointer = &$pointer[$key];
+              }
             }
-        } else {
-            // explicitly add all isolated vertices (vertices with no edges) and vertices with special layout set
-            // other vertices wil be added automatically due to below edge definitions
-            foreach ($this->graph->getVertices() as $vid => $vertex){
-                $layout = $this->getLayoutVertex($vertex);
-
-                if($vertex->isIsolated() || $layout){
-                    $script .= $this->formatIndent . $this->escapeId($vid);
-                    if($layout){
-                        $script .= ' ' . $this->escapeAttributes($layout);
-                    }
-                    $script .= self::EOL;
+            foreach( $tree as $group => $subtree) {
+                $this->printCluster($group, $subtree, $alg, $script);
+            }
+        }
+
+        // explicitly add all isolated vertices (vertices with no edges) and vertices with special layout set
+        // other vertices wil be added automatically due to below edge definitions
+        foreach ($this->graph->getVertices() as $vid => $vertex){
+            $layout = $this->getLayoutVertex($vertex);
+
+            if($vertex->isIsolated() || $layout){
+                $script .= $this->formatIndent . $this->escapeId($vid);
+                if($layout){
+                    $script .= ' ' . $this->escapeAttributes($layout);
                 }
+                $script .= self::EOL;
             }
         }
 
@@ -356,6 +358,24 @@ public function createScript()
         return $script;
     }
 
+    protected function printCluster($group, $tree, $alg, &$script, $depth = 1) {
+        if ($group == "_NULL_") {
+            return;
+        }
+        $script .= str_repeat($this->formatIndent, $depth) . 'subgraph cluster_' . str_replace(":", "_", $group) . ' {' . self::EOL;
+        $script .= str_repeat($this->formatIndent, $depth + 1) . 'label = ' . $this->escape($group) . self::EOL;
+        foreach($alg->getVerticesGroup($group) as $vid => $vertex) {
+            $script .= str_repeat($this->formatIndent, $depth + 1) . $this->escape($vid);
+            $script .= self::EOL;
+        }
+        if (!empty($tree)) {
+            foreach($tree as $sub_group => $sub_tree) {
+              $this->printCluster($group . ':' . $sub_group, $sub_tree, $alg, $script, $depth + 1);
+            }
+        }
+        $script .= str_repeat($this->formatIndent, $depth) . '}' . self::EOL;
+    }
+
     /**
      * escape given id string and wrap in quotes if needed
      *