Test and implement loop detection

fixes #9786

library/Businessprocess/BpNode.php

@@ -3,6 +3,7 @@
 namespace Icinga\Module\Businessprocess;
 
 use Icinga\Exception\ConfigurationError;
+use Icinga\Module\Businessprocess\Exception\NestingError;
 
 class BpNode extends Node
 {
@@ -44,6 +45,8 @@ class BpNode extends Node
         0 => 4
     );
 
+    protected static $loopDetection = array();
+
     protected $className = 'process';
 
     public function __construct(BusinessProcess $bp, $object)
@@ -262,11 +265,24 @@ class BpNode extends Node
         return $this;
     }
 
+    /**
+     * @return int
+     */
     public function getState()
     {
         if ($this->state === null) {
-            $this->calculateState();
+            try {
+                $this->reCalculateState();
+            } catch (NestingError $e) {
+                $this->bp->addError(
+                    $this->bp->translate('Nesting error detected: %s'),
+                    $e->getMessage()
+                );
+
+                $this->state = 3;
+            }
         }
+
         return $this->state;
     }
 
@@ -275,13 +291,23 @@ class BpNode extends Node
         return self::$sortStateInversionMap[$state >> self::SHIFT_FLAGS] << self::SHIFT_FLAGS;
     }
 
-    protected function calculateState()
+    /**
+     * @return $this
+     */
+    public function reCalculateState()
     {
         $sort_states = array();
         $lastStateChange = 0;
+        if (!$this->hasChildren()) {
+            $this->state = 0;
+            return $this;
+        }
+
         foreach ($this->getChildren() as $child) {
+            $this->beginLoopDetection();
             $sort_states[] = $child->getSortingState();
             $lastStateChange = max($lastStateChange, $child->getLastStateChange());
+            $this->endLoopDetection();
         }
 
         $this->setLastStateChange($lastStateChange);
@@ -315,6 +341,38 @@ class BpNode extends Node
         }
 
         $this->state = $this->sortStateTostate($sort_state);
+        return $this;
+    }
+
+    protected function beginLoopDetection()
+    {
+        $name = $this->name;
+        if (array_key_exists($name, self::$loopDetection)) {
+            $loop = array_keys(self::$loopDetection);
+            $loop[] = $name;
+            self::$loopDetection = array();
+            throw new NestingError('Loop detected: %s', implode(' -> ', $loop));
+        }
+
+        self::$loopDetection[$name] = true;
+    }
+
+    protected function endLoopDetection()
+    {
+        unset(self::$loopDetection[$this->name]);
+    }
+
+    public function checkForLoops()
+    {
+        foreach ($this->getChildren() as $child) {
+            $this->beginLoopDetection();
+            if ($child instanceof BpNode) {
+                $child->checkForLoops();
+            }
+            $this->endLoopDetection();
+        }
+
+        return $this;
     }
 
     public function setDisplay($display)

library/Businessprocess/BusinessProcess.php

@@ -649,7 +649,7 @@ class BusinessProcess
         return $errors;
     }
 
-    protected function translate($msg)
+    public function translate($msg)
     {
         return mt('businessprocess', $msg);
     }
@@ -670,6 +670,22 @@ class BusinessProcess
         }
     }
 
+    /**
+     * @param string $msg,...
+     *
+     * @return $this
+     */
+    public function addError($msg)
+    {
+        $args = func_get_args();
+        array_shift($args);
+        $this->errors[] = vsprintf($msg, $args);
+        return $this;
+    }
+
+    /**
+     * @deprecated
+     */
     protected function error($msg)
     {
         $args = func_get_args();

library/Businessprocess/Test/BaseTestCase.php

@@ -5,6 +5,8 @@ namespace Icinga\Module\Businessprocess\Test;
 use Icinga\Application\Config;
 use Icinga\Application\ApplicationBootstrap;
 use Icinga\Application\Icinga;
+use Icinga\Module\Businessprocess\BusinessProcess;
+use Icinga\Module\Businessprocess\Storage\LegacyStorage;
 use Icinga\Module\Businessprocess\Web\FakeRequest;
 use PHPUnit_Framework_TestCase;
 
@@ -27,6 +29,25 @@ abstract class BaseTestCase extends PHPUnit_Framework_TestCase
         return Config::module('businessprocess')->getSection('global');
     }
 
+    /***
+     * @return BusinessProcess
+     */
+    protected function makeLoop()
+    {
+        return $this->makeInstance()->loadFromString(
+            'loop',
+            "a = b\nb = c\nc = a\nd = a"
+        );
+    }
+
+    /**
+     * @return LegacyStorage
+     */
+    protected function makeInstance()
+    {
+        return new LegacyStorage($this->emptyConfigSection());
+    }
+
     /**
      * @param null $subDir
      * @return string

test/php/library/Businessprocess/BpNodeTest.php

@@ -0,0 +1,41 @@
+<?php
+
+namespace Tests\Icinga\Module\Businessprocess\Operator;
+
+use Icinga\Module\Businessprocess\BpNode;
+use Icinga\Module\Businessprocess\Test\BaseTestCase;
+
+class BpNodeTest extends BaseTestCase
+{
+    /**
+     * @expectedException \Icinga\Module\Businessprocess\Exception\NestingError
+     */
+    public function testThrowsNestingErrorWhenCheckedForLoops()
+    {
+        /** @var BpNode $bpNode */
+        $bpNode = $this->makeLoop()->getNode('d');
+        $bpNode->checkForLoops();
+    }
+
+    /**
+     * @expectedExceptionMessage d -> a -> b -> c -> a
+     * @expectedException \Icinga\Module\Businessprocess\Exception\NestingError
+     */
+    public function testNestingErrorReportsFullLoop()
+    {
+        /** @var BpNode $bpNode */
+        $bpNode = $this->makeLoop()->getNode('d');
+        $bpNode->checkForLoops();
+    }
+
+    public function testStateForALoopGivesUnknown()
+    {
+        $loop = $this->makeLoop();
+        /** @var BpNode $bpNode */
+        $bpNode = $loop->getNode('d');
+        $this->assertEquals(
+            'UNKNOWN',
+            $bpNode->getStateName()
+        );
+    }
+}

test/php/library/Businessprocess/Storage/LegacyStorageTest.php

@@ -7,6 +7,8 @@ use Icinga\Module\Businessprocess\Storage\LegacyStorage;
 
 class LegacyStorageTest extends BaseTestCase
 {
+    private $processClass = 'Icinga\\Module\\Businessprocess\\BusinessProcess';
+
     public function testCanBeInstantiatedWithAnEmptyConfigSection()
     {
         $baseClass = 'Icinga\\Module\\Businessprocess\\Storage\\LegacyStorage';
@@ -76,18 +78,16 @@ class LegacyStorageTest extends BaseTestCase
 
     public function testAValidProcessCanBeLoaded()
     {
-        $processClass = 'Icinga\\Module\\Businessprocess\\BusinessProcess';
         $this->assertInstanceOf(
-            $processClass,
+            $this->processClass,
             $this->makeInstance()->loadProcess('simple_with-header')
         );
     }
 
     public function testProcessConfigCanBeLoadedFromAString()
     {
-        $processClass = 'Icinga\\Module\\Businessprocess\\BusinessProcess';
         $this->assertInstanceOf(
-            $processClass,
+            $this->processClass,
             $this->makeInstance()->loadFromString('dummy', 'a = Host1;ping & Host2;ping')
         );
     }
@@ -104,11 +104,11 @@ class LegacyStorageTest extends BaseTestCase
         );
     }
 
-    /**
-     * @return LegacyStorage
-     */
-    protected function makeInstance()
+    public function testAConfiguredLoopCanBeParsed()
     {
-        return new LegacyStorage($this->emptyConfigSection());
+        $this->assertInstanceOf(
+            $this->processClass,
+            $this->makeLoop()
+        );
     }
 }