diff --git a/library/Businessprocess/BpNode.php b/library/Businessprocess/BpNode.php index 49a1421..374c304 100644 --- a/library/Businessprocess/BpNode.php +++ b/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) diff --git a/library/Businessprocess/BusinessProcess.php b/library/Businessprocess/BusinessProcess.php index af90113..e1b0005 100644 --- a/library/Businessprocess/BusinessProcess.php +++ b/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(); diff --git a/library/Businessprocess/Test/BaseTestCase.php b/library/Businessprocess/Test/BaseTestCase.php index 8673cda..2113494 100644 --- a/library/Businessprocess/Test/BaseTestCase.php +++ b/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 diff --git a/test/php/library/Businessprocess/BpNodeTest.php b/test/php/library/Businessprocess/BpNodeTest.php new file mode 100644 index 0000000..b9ef61c --- /dev/null +++ b/test/php/library/Businessprocess/BpNodeTest.php @@ -0,0 +1,41 @@ +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() + ); + } +} diff --git a/test/php/library/Businessprocess/Storage/LegacyStorageTest.php b/test/php/library/Businessprocess/Storage/LegacyStorageTest.php index 5abcc87..77adacf 100644 --- a/test/php/library/Businessprocess/Storage/LegacyStorageTest.php +++ b/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() + ); } }