From 49ebbc4cdb1673ded9f8990b745735c36c518611 Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Tue, 30 Jun 2020 16:07:47 +0200 Subject: [PATCH] Apply state overrides on demand instead of directly Internally non-process children are only instantiated once. This means when applying state overrides directly they're used everywhere and do not differ between the containing process. State overrides are now applied explicitly and on demand, decoupling them from children. --- application/clicommands/ProcessCommand.php | 14 ++-- application/forms/AddNodeForm.php | 17 ++--- application/forms/EditNodeForm.php | 24 ++++--- library/Businessprocess/BpNode.php | 64 +++++++++++++++++-- .../Modification/NodeAddChildrenAction.php | 18 +----- library/Businessprocess/Node.php | 38 ++--------- library/Businessprocess/Renderer/Renderer.php | 6 +- .../Renderer/TileRenderer/NodeTile.php | 8 ++- .../Businessprocess/Renderer/TreeRenderer.php | 33 +++++----- .../Storage/LegacyConfigParser.php | 2 +- .../Storage/LegacyConfigRenderer.php | 6 +- .../Web/Form/Element/StateOverrides.php | 9 ++- 12 files changed, 133 insertions(+), 106 deletions(-) diff --git a/application/clicommands/ProcessCommand.php b/application/clicommands/ProcessCommand.php index af3cc29..0d00a37 100644 --- a/application/clicommands/ProcessCommand.php +++ b/application/clicommands/ProcessCommand.php @@ -167,21 +167,22 @@ class ProcessCommand extends Command } } - protected function renderProblemTree($tree, $useColors = false, $depth = 0) + protected function renderProblemTree($tree, $useColors = false, $depth = 0, BpNode $parent = null) { $output = ''; foreach ($tree as $name => $subtree) { /** @var Node $node */ $node = $subtree['node']; + $state = $parent !== null ? $parent->getChildState($node) : $node->getState(); if ($node instanceof HostNode) { - $colors = $this->hostColors[$node->getState()]; + $colors = $this->hostColors[$state]; } else { - $colors = $this->serviceColors[$node->getState()]; + $colors = $this->serviceColors[$state]; } - $state = sprintf('[%s]', $node->getStateName()); + $state = sprintf('[%s]', $node->getStateName($state)); if ($useColors) { $state = $this->screen->colorize($state, $colors[0], $colors[1]); } @@ -193,7 +194,10 @@ class ProcessCommand extends Command $state, $node->getAlias() ); - $output .= $this->renderProblemTree($subtree['children'], $useColors, $depth + 1); + + if ($node instanceof BpNode) { + $output .= $this->renderProblemTree($subtree['children'], $useColors, $depth + 1, $node); + } } return $output; diff --git a/application/forms/AddNodeForm.php b/application/forms/AddNodeForm.php index a01aee2..11f24f3 100644 --- a/application/forms/AddNodeForm.php +++ b/application/forms/AddNodeForm.php @@ -502,16 +502,17 @@ class AddNodeForm extends QuickForm $changes = ProcessChanges::construct($this->bp, $this->session); switch ($this->getValue('node_type')) { case 'service': - $properties = $this->getValues(); - unset($properties['children']); + $stateOverrides = $this->getValue('stateOverrides'); + if (! empty($stateOverrides)) { + $services = []; + foreach ($this->getValue('children') as $service) { + $services[$service] = $stateOverrides; + } - $services = []; - foreach ($this->getValue('children') as $service) { - $services[$service] = $properties; + $changes->modifyNode($this->parent, [ + 'stateOverrides' => array_merge($this->parent->getStateOverrides(), $services) + ]); } - - $changes->addChildrenToNode($services, $this->parent); - break; case 'host': case 'process': if ($this->hasParentNode()) { diff --git a/application/forms/EditNodeForm.php b/application/forms/EditNodeForm.php index 2d60d88..98596e5 100644 --- a/application/forms/EditNodeForm.php +++ b/application/forms/EditNodeForm.php @@ -156,7 +156,6 @@ class EditNodeForm extends QuickForm if ($this->hasParentNode()) { $this->addElement('hidden', 'node_type', [ 'disabled' => true, - 'ignore' => true, 'decorators' => ['ViewHelper'], 'value' => $monitoredNodeType ]); @@ -192,7 +191,7 @@ class EditNodeForm extends QuickForm if ($this->getSentValue('hosts') === null) { $this->addServicesElement($this->host); $this->addServiceOverrideCheckbox(); - if (! empty($this->node->getStateOverrides()) || $this->getSentValue('service_override') === '1') { + if ($this->getElement('service_override')->isChecked() || $this->getSentValue('service_override') === '1') { $this->addServiceOverrideElement(); } } elseif ($host = $this->getSentValue('hosts')) { @@ -236,7 +235,7 @@ class EditNodeForm extends QuickForm $this->addElement('checkbox', 'service_override', [ 'ignore' => true, 'class' => 'autosubmit', - 'value' => ! empty($this->node->getStateOverrides()), + 'value' => ! empty($this->parent->getStateOverrides($this->node->getName())), 'label' => $this->translate('Override Service State'), 'description' => $this->translate('Enable service state overrides') ]); @@ -247,7 +246,7 @@ class EditNodeForm extends QuickForm $this->addElement('stateOverrides', 'stateOverrides', [ 'required' => true, 'states' => $this->enumServiceStateList(), - 'value' => $this->node->getStateOverrides(), + 'value' => $this->parent->getStateOverrides($this->node->getName()), 'label' => $this->translate('State Overrides') ]); } @@ -449,11 +448,18 @@ class EditNodeForm extends QuickForm switch ($this->getValue('node_type')) { case 'service': - $properties = $this->getValues(); - unset($properties['children']); - $services = [$this->getValue('children') => $properties]; - $changes->addChildrenToNode($services, $this->parent); - break; + $stateOverrides = $this->getValue('stateOverrides') ?: []; + if (! empty($stateOverrides)) { + $stateOverrides = array_merge( + $this->parent->getStateOverrides(), + [$this->getValue('children') => $stateOverrides] + ); + } else { + $stateOverrides = $this->parent->getStateOverrides(); + unset($stateOverrides[$this->getValue('children')]); + } + + $changes->modifyNode($this->parent, ['stateOverrides' => $stateOverrides]); case 'host': case 'process': $changes->addChildrenToNode($this->getValue('children'), $this->parent); diff --git a/library/Businessprocess/BpNode.php b/library/Businessprocess/BpNode.php index 9aeaab8..58a7c11 100644 --- a/library/Businessprocess/BpNode.php +++ b/library/Businessprocess/BpNode.php @@ -26,6 +26,7 @@ class BpNode extends Node protected $missing = null; protected $empty = null; protected $missingChildren; + protected $stateOverrides = []; protected static $emptyStateSummary = array( 'OK' => 0, @@ -71,7 +72,7 @@ class BpNode extends Node } elseif ($child->isMissing()) { $this->counters['MISSING']++; } else { - $state = $child->getStateName(); + $state = $child->getStateName($this->getChildState($child)); $this->counters[$state]++; } } @@ -127,9 +128,13 @@ class BpNode extends Node $problems = array(); foreach ($this->getChildren() as $child) { - if ($child->isProblem() - || ($child instanceof BpNode && $child->hasProblems()) - ) { + if (isset($this->stateOverrides[$child->getName()])) { + $problem = $this->getChildState($child) > 0; + } else { + $problem = $child->isProblem() || ($child instanceof BpNode && $child->hasProblems()); + } + + if ($problem) { $problems[] = $child; } } @@ -187,7 +192,8 @@ class BpNode extends Node if ($nodeState !== 0) { foreach ($this->getChildren() as $child) { - $childState = $rootCause ? $child->getSortingState() : $child->getState(); + $childState = $this->getChildState($child); + $childState = $rootCause ? $child->getSortingState($childState) : $childState; if (($rootCause ? $this->getSortingState() : $nodeState) === $childState) { $name = $child->getName(); $tree[$name] = [ @@ -327,6 +333,31 @@ class BpNode extends Node return $this->info_command; } + public function setStateOverrides(array $overrides, $name = null) + { + if ($name === null) { + $this->stateOverrides = $overrides; + } else { + $this->stateOverrides[$name] = $overrides; + } + + return $this; + } + + public function getStateOverrides($name = null) + { + $overrides = null; + if ($name !== null) { + if (isset($this->stateOverrides[$name])) { + $overrides = $this->stateOverrides[$name]; + } + } else { + $overrides = $this->stateOverrides; + } + + return $overrides; + } + public function getAlias() { return $this->alias ? preg_replace('~_~', ' ', $this->alias) : $this->name; @@ -354,6 +385,27 @@ class BpNode extends Node return $this->state; } + /** + * Get the given child's state, possibly adjusted by override rules + * + * @param Node|string $child + * @return int + */ + public function getChildState($child) + { + if (! $child instanceof Node) { + $child = $this->getChildByName($child); + } + + $childName = $child->getName(); + $childState = $child->getState(); + if (! isset($this->stateOverrides[$childName][$childState])) { + return $childState; + } + + return $this->stateOverrides[$childName][$childState]; + } + public function getHtmlId() { return 'businessprocess-' . preg_replace('/[\r\n\t\s]/', '_', $this->getName()); @@ -391,7 +443,7 @@ class BpNode extends Node $child->setMissing(); } - $sort_states[] = $child->getSortingState(); + $sort_states[] = $child->getSortingState($this->getChildState($child)); $lastStateChange = max($lastStateChange, $child->getLastStateChange()); $bp->endLoopDetection($this->name); } diff --git a/library/Businessprocess/Modification/NodeAddChildrenAction.php b/library/Businessprocess/Modification/NodeAddChildrenAction.php index 8f9c5e0..5d5ab29 100644 --- a/library/Businessprocess/Modification/NodeAddChildrenAction.php +++ b/library/Businessprocess/Modification/NodeAddChildrenAction.php @@ -31,27 +31,15 @@ class NodeAddChildrenAction extends NodeAction { $node = $config->getBpNode($this->getNodeName()); - foreach ($this->children as $name => $properties) { - if (is_int($name)) { - // TODO: This may be removed once host nodes also have properties - $name = $properties; - } - + foreach ($this->children as $name) { if (! $config->hasNode($name) || $config->getNode($name)->getBpConfig()->getName() !== $config->getName()) { if (strpos($name, ';') !== false) { list($host, $service) = preg_split('/;/', $name, 2); if ($service === 'Hoststatus') { - $child = $config->createHost($host); + $config->createHost($host); } else { - $child = $config->createService($host, $service); - } - - if (is_array($properties) && !empty($properties)) { - foreach ($properties as $key => $value) { - $func = 'set' . ucfirst($key); - $child->$func($value); - } + $config->createService($host, $service); } } elseif ($name[0] === '@' && strpos($name, ':') !== false) { list($configName, $nodeName) = preg_split('~:\s*~', substr($name, 1), 2); diff --git a/library/Businessprocess/Node.php b/library/Businessprocess/Node.php index bec3346..f22339b 100644 --- a/library/Businessprocess/Node.php +++ b/library/Businessprocess/Node.php @@ -46,8 +46,6 @@ abstract class Node self::NODE_EMPTY => 0 ); - protected $stateOverrides = []; - /** @var string Alias of the node */ protected $alias; @@ -199,18 +197,6 @@ abstract class Node return $this; } - public function setStateOverrides(array $overrides) - { - $this->stateOverrides = $overrides; - - return $this; - } - - public function getStateOverrides() - { - return $this->stateOverrides; - } - /** * Forget my state * @@ -260,30 +246,14 @@ abstract class Node ); } - if (isset($this->stateOverrides[$this->state])) { - return $this->stateOverrides[$this->state]; - } else { - return $this->state; - } - } - - public function getRealState() - { - if ($this->state === null) { - throw new ProgrammingError( - sprintf( - 'Node %s is unable to retrieve it\'s state', - $this->name - ) - ); - } - return $this->state; } - public function getSortingState() + public function getSortingState($state = null) { - $state = $this->getState(); + if ($state === null) { + $state = $this->getState(); + } if (self::$ackIsOk && $this->isAcknowledged()) { $state = self::ICINGA_OK; diff --git a/library/Businessprocess/Renderer/Renderer.php b/library/Businessprocess/Renderer/Renderer.php index 4c9a8de..35e3225 100644 --- a/library/Businessprocess/Renderer/Renderer.php +++ b/library/Businessprocess/Renderer/Renderer.php @@ -180,9 +180,9 @@ abstract class Renderer extends HtmlDocument if ($node->isEmpty() && ! $node instanceof MonitoredNode) { $classes = array('empty'); } else { - $classes = array( - strtolower($node->getStateName()) - ); + $classes = [strtolower($node->getStateName( + $this->parent !== null ? $this->parent->getChildState($node) : null + ))]; } if ($node->hasMissingChildren()) { $classes[] = 'missing-children'; diff --git a/library/Businessprocess/Renderer/TileRenderer/NodeTile.php b/library/Businessprocess/Renderer/TileRenderer/NodeTile.php index 6e7d8f0..ef26693 100644 --- a/library/Businessprocess/Renderer/TileRenderer/NodeTile.php +++ b/library/Businessprocess/Renderer/TileRenderer/NodeTile.php @@ -113,12 +113,14 @@ class NodeTile extends BaseHtmlElement )); } - if ($node instanceof ServiceNode && $node->getRealState() !== $node->getState()) { - $this->add((new StateBall(strtolower($node->getStateName($node->getRealState()))))->addAttributes([ + if ($this->renderer->rendersSubNode() + && $this->renderer->getParentNode()->getChildState($node) !== $node->getState() + ) { + $this->add((new StateBall(strtolower($node->getStateName())))->addAttributes([ 'class' => 'overridden-state', 'title' => sprintf( '%s', - $node->getStateName($node->getRealState()) + $node->getStateName() ) ])); } diff --git a/library/Businessprocess/Renderer/TreeRenderer.php b/library/Businessprocess/Renderer/TreeRenderer.php index 056cded..efe8d77 100644 --- a/library/Businessprocess/Renderer/TreeRenderer.php +++ b/library/Businessprocess/Renderer/TreeRenderer.php @@ -79,7 +79,7 @@ class TreeRenderer extends Renderer if ($node instanceof BpNode) { $html[] = $this->renderNode($bp, $node); } else { - $html[] = $this->renderChild($bp, $node); + $html[] = $this->renderChild($bp, $this->parent, $node); } } @@ -106,9 +106,10 @@ class TreeRenderer extends Renderer /** * @param Node $node * @param array $path + * @param BpNode $parent * @return BaseHtmlElement[] */ - public function getNodeIcons(Node $node, array $path = null) + public function getNodeIcons(Node $node, array $path = null, BpNode $parent = null) { $icons = []; if (empty($path) && $node instanceof BpNode) { @@ -116,7 +117,7 @@ class TreeRenderer extends Renderer } else { $icons[] = $node->getIcon(); } - $state = strtolower($node->getStateName()); + $state = strtolower($node->getStateName($parent !== null ? $parent->getChildState($node) : null)); if ($node->isHandled()) { $state = $state . '-handled'; } @@ -136,20 +137,20 @@ class TreeRenderer extends Renderer return $icons; } - public function getOverriddenState(Node $node) + public function getOverriddenState($fakeState, Node $node) { $overriddenState = Html::tag('div', ['class' => 'overridden-state']); - $overriddenState->add((new StateBall(strtolower($node->getStateName($node->getRealState()))))->addAttributes([ - 'title' => sprintf( - '%s', - $node->getStateName($node->getRealState()) - ) - ])); - $overriddenState->add(Html::tag('i', ['class' => 'icon icon-right-small'])); $overriddenState->add((new StateBall(strtolower($node->getStateName())))->addAttributes([ 'title' => sprintf( '%s', $node->getStateName() + ) + ])); + $overriddenState->add(Html::tag('i', ['class' => 'icon icon-right-small'])); + $overriddenState->add((new StateBall(strtolower($node->getStateName($fakeState))))->addAttributes([ + 'title' => sprintf( + '%s', + $node->getStateName($fakeState) ), 'class' => 'last' ])); @@ -243,14 +244,14 @@ class TreeRenderer extends Renderer if ($child instanceof BpNode) { $ul->add($this->renderNode($bp, $child, $path)); } else { - $ul->add($this->renderChild($bp, $child, $path)); + $ul->add($this->renderChild($bp, $node, $child, $path)); } } return $li; } - protected function renderChild($bp, Node $node, $path = null) + protected function renderChild($bp, BpNode $parent, Node $node, $path = null) { $li = Html::tag('li', [ 'class' => 'movable', @@ -258,14 +259,14 @@ class TreeRenderer extends Renderer 'data-node-name' => $node->getName() ]); - $li->add($this->getNodeIcons($node, $path)); + $li->add($this->getNodeIcons($node, $path, $parent)); $link = $node->getLink(); $link->getAttributes()->set('data-base-target', '_next'); $li->add($link); - if ($node->getRealState() !== $node->getState()) { - $li->add($this->getOverriddenState($node)); + if (($overriddenState = $parent->getChildState($node)) !== $node->getState()) { + $li->add($this->getOverriddenState($overriddenState, $node)); } if (! $this->isLocked() && $node->getBpConfig()->getName() === $this->getBusinessProcess()->getName()) { diff --git a/library/Businessprocess/Storage/LegacyConfigParser.php b/library/Businessprocess/Storage/LegacyConfigParser.php index 1afc256..4142731 100644 --- a/library/Businessprocess/Storage/LegacyConfigParser.php +++ b/library/Businessprocess/Storage/LegacyConfigParser.php @@ -247,7 +247,7 @@ class LegacyConfigParser $stateOverrides[(int) $from] = (int) $to; } - $node->getChildByName($childName)->setStateOverrides($stateOverrides); + $node->setStateOverrides($stateOverrides, $childName); } } diff --git a/library/Businessprocess/Storage/LegacyConfigRenderer.php b/library/Businessprocess/Storage/LegacyConfigRenderer.php index eba26bc..ebe9589 100644 --- a/library/Businessprocess/Storage/LegacyConfigRenderer.php +++ b/library/Businessprocess/Storage/LegacyConfigRenderer.php @@ -218,14 +218,14 @@ class LegacyConfigRenderer public static function renderStateOverrides(BpNode $node) { $stateOverrides = ''; - foreach ($node->getChildren() as $child) { + foreach ($node->getStateOverrides() as $childName => $overrideRules) { $overrides = []; - foreach ($child->getStateOverrides() as $from => $to) { + foreach ($overrideRules as $from => $to) { $overrides[] = sprintf('%d-%d', $from, $to); } if (! empty($overrides)) { - $stateOverrides .= '!' . $child->getName() . '|' . join(',', $overrides); + $stateOverrides .= '!' . $childName . '|' . join(',', $overrides); } } diff --git a/library/Businessprocess/Web/Form/Element/StateOverrides.php b/library/Businessprocess/Web/Form/Element/StateOverrides.php index 49cb233..c2216c0 100644 --- a/library/Businessprocess/Web/Form/Element/StateOverrides.php +++ b/library/Businessprocess/Web/Form/Element/StateOverrides.php @@ -41,9 +41,12 @@ class StateOverrides extends FormElement public function setValue($value) { $cleanedValue = []; - foreach ($value as $from => $to) { - if ((int) $from !== (int) $to) { - $cleanedValue[$from] = $to; + + if (! empty($value)) { + foreach ($value as $from => $to) { + if ((int) $from !== (int) $to) { + $cleanedValue[$from] = $to; + } } }