From e688d045113e0a03db0d78396bc7edb6f8c4abf5 Mon Sep 17 00:00:00 2001
From: Romain Quetiez <romain.quetiez@combodo.com>
Date: Fri, 31 Mar 2017 14:48:30 +0000
Subject: [PATCH] N.788 Impact analysis: graph not refreshed when trying to
 filter out some classes. The node removal algorithm has been improved in two
 places. 1) Do not create loops (i.e. an edge having both ends on the same
 node) when removing a node. 2) Correctly cleanup nodes having a loop (in case
 there is a loop in the original graph (defensive programming)

SVN:trunk[4644]
---
 core/simplegraph.class.inc.php | 24 +++++++++++-----
 test/testlist.inc.php          | 50 ++++++++++++++++++++++++++++++++++
 2 files changed, 67 insertions(+), 7 deletions(-)

diff --git a/core/simplegraph.class.inc.php b/core/simplegraph.class.inc.php
index f817cd73a..905cc4b7e 100644
--- a/core/simplegraph.class.inc.php
+++ b/core/simplegraph.class.inc.php
@@ -1,5 +1,5 @@
 <?php
-// Copyright (C) 2015 Combodo SARL
+// Copyright (C) 2015-2017 Combodo SARL
 //
 //   This file is part of iTop.
 //
@@ -18,7 +18,7 @@
 /**
  * Data structures (i.e. PHP classes) to manage "graphs"
  *
- * @copyright   Copyright (C) 2015 Combodo SARL
+ * @copyright   Copyright (C) 2015-2017 Combodo SARL
  * @license     http://opensource.org/licenses/AGPL-3.0
  * 
  * Example:
@@ -346,14 +346,15 @@ class SimpleGraph
 		}
 		unset($this->aNodes[$oNode->GetId()]);
 	}
-	
+
 	/**
 	 * Removes the given node but preserves the connectivity of the graph
 	 * all "source" nodes are connected to all "sink" nodes
 	 * @param GraphNode $oNode
+	 * @param bool $bAllowLoopingEdge
 	 * @throws SimpleGraphException
 	 */
-	public function FilterNode(GraphNode $oNode)
+	public function FilterNode(GraphNode $oNode, $bAllowLoopingEdge = false)
 	{
 		if (!array_key_exists($oNode->GetId(), $this->aNodes)) throw new SimpleGraphException('Cannot filter the node (id='.$oNode->GetId().') from the graph. The node was not found in the graph.');
 		
@@ -362,13 +363,19 @@ class SimpleGraph
 		foreach($oNode->GetOutgoingEdges() as $oEdge)
 		{
 			$sSinkId =  $oEdge->GetSinkNode()->GetId();
-			$aSinkNodes[$sSinkId] = $oEdge->GetSinkNode();
+			if ($sSinkId != $oNode->GetId())
+			{
+				$aSinkNodes[$sSinkId] = $oEdge->GetSinkNode();
+			}
 			$this->_RemoveEdge($oEdge);
 		}
 		foreach($oNode->GetIncomingEdges() as $oEdge)
 		{
 			$sSourceId =  $oEdge->GetSourceNode()->GetId();
-			$aSourceNodes[$sSourceId] = $oEdge->GetSourceNode();
+			if ($sSourceId != $oNode->GetId())
+			{
+				$aSourceNodes[$sSourceId] = $oEdge->GetSourceNode();
+			}
 			$this->_RemoveEdge($oEdge);
 		}
 		unset($this->aNodes[$oNode->GetId()]);
@@ -377,7 +384,10 @@ class SimpleGraph
 		{
 			foreach($aSinkNodes as $sSinkId => $oSinkNode)
 			{
-				$oEdge = new RelationEdge($this, $oSourceNode, $oSinkNode);
+				if ($bAllowLoopingEdge || ($oSourceNode->GetId() != $oSinkNode->GetId()))
+				{
+					$oEdge = new RelationEdge($this, $oSourceNode, $oSinkNode);
+				}
 			}
 		}
 	}
diff --git a/test/testlist.inc.php b/test/testlist.inc.php
index d0d698ccf..59e09020e 100644
--- a/test/testlist.inc.php
+++ b/test/testlist.inc.php
@@ -5545,3 +5545,53 @@ class TestBug609 extends TestBizModel
 		}
 	}
 }
+
+class TestBug788 extends TestBizModel
+{
+	static public function GetName()
+	{
+		return 'Graph - delete nodes';
+	}
+
+	static public function GetDescription()
+	{
+		return '(N.788) Graph not refreshed when unchecking some classes';
+	}
+
+	protected function DoExecute()
+	{
+		$oGraph = new SimpleGraph();
+		$a = new GraphNode($oGraph, 'A');
+		$b = new GraphNode($oGraph, 'B');
+		$c = new GraphNode($oGraph, 'C');
+		new GraphEdge($oGraph, 'A--B', $a, $b);
+		new GraphEdge($oGraph, 'B--C', $b, $c);
+		new GraphEdge($oGraph, 'C--B', $c, $b);
+
+		echo "<h5>Graphe initial</h5>";
+		echo $oGraph->DumpAsHtmlImage();
+		echo $oGraph->DumpAsHTMLText();
+
+		echo "<h5>Removing C</h5>";
+		$oGraph->FilterNode($c);
+		unset($c);
+		echo $oGraph->DumpAsHtmlImage();
+		echo $oGraph->DumpAsHTMLText();
+
+		if ((count($oGraph->_GetNodes()) != 2) || (count($oGraph->_GetEdges()) != 1))
+		{
+			throw new Exception('The graph should be made of 2 nodes and 1 edge');
+		}
+
+		echo "<h5>Removing B</h5>";
+		$oGraph->FilterNode($b);
+		unset($b);
+		echo $oGraph->DumpAsHtmlImage();
+		echo $oGraph->DumpAsHTMLText();
+
+		if ((count($oGraph->_GetNodes()) != 1) || (count($oGraph->_GetEdges()) != 0))
+		{
+			throw new Exception('The graph should contain only the node A');
+		}
+	}
+}