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)

git-svn-id: http://svn.code.sf.net/p/itop/code/trunk@4644 a333f486-631f-4898-b8df-5754b55c2be0

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);
+				}
 			}
 		}
 	}

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');
+		}
+	}
+}