From 08cae4533641cf8aeb6b8aacad982a4a100acca8 Mon Sep 17 00:00:00 2001 From: Sukhwinder Dhillon Date: Tue, 8 Aug 2023 15:22:51 +0200 Subject: [PATCH 1/4] Imported Node: Don't break view if source config file is faulty --- library/Businessprocess/BpConfig.php | 47 +++++++++++++++++-- library/Businessprocess/ImportedNode.php | 14 ++++-- .../Renderer/TileRenderer/NodeTile.php | 14 +++--- .../Businessprocess/Renderer/TreeRenderer.php | 4 +- public/css/module.less | 7 +-- 5 files changed, 66 insertions(+), 20 deletions(-) diff --git a/library/Businessprocess/BpConfig.php b/library/Businessprocess/BpConfig.php index e99f81c..3e652e9 100644 --- a/library/Businessprocess/BpConfig.php +++ b/library/Businessprocess/BpConfig.php @@ -130,6 +130,9 @@ class BpConfig /** @var ProcessChanges */ protected $appliedChanges; + /** @var bool Whether the config is faulty */ + protected $isFaulty = false; + public function __construct() { } @@ -577,7 +580,13 @@ class BpConfig public function getImportedConfig($name) { if (! isset($this->importedConfigs[$name])) { - $import = $this->storage()->loadProcess($name); + try { + $import = $this->storage()->loadProcess($name); + } catch (Exception $e) { + $import = (new static()) + ->setName($name) + ->setFaulty(); + } if ($this->usesSoftStates()) { $import->useSoftStates(); @@ -687,9 +696,17 @@ class BpConfig { if ($this->hasBpNode($name)) { return $this->nodes[$name]; - } else { - throw new NotFoundError('Trying to access a missing business process node "%s"', $name); } + + $msg = $this->isFaulty() + ? sprintf( + t('Trying to import node "%s" from faulty config file "%s.conf"'), + $name, + $this->getName() + ) + : sprintf(t('Trying to access a missing business process node "%s"'), $name); + + throw new NotFoundError($msg); } /** @@ -1070,4 +1087,28 @@ class BpConfig return array_pad($parts, 2, null); } + + /** + * Set whether the config is faulty + * + * @param bool $isFaulty + * + * @return $this + */ + public function setFaulty(bool $isFaulty = true): self + { + $this->isFaulty = $isFaulty; + + return $this; + } + + /** + * Get whether the config is faulty + * + * @return bool + */ + public function isFaulty(): bool + { + return $this->isFaulty; + } } diff --git a/library/Businessprocess/ImportedNode.php b/library/Businessprocess/ImportedNode.php index e20b0a7..a0eb6b1 100644 --- a/library/Businessprocess/ImportedNode.php +++ b/library/Businessprocess/ImportedNode.php @@ -90,6 +90,15 @@ class ImportedNode extends BpNode return $this->childNames; } + public function isMissing() + { + if ($this->missing === null && $this->getBpConfig()->isFaulty()) { + $this->missing = true; + } + + return parent::isMissing(); + } + /** * @return BpNode */ @@ -121,10 +130,9 @@ class ImportedNode extends BpNode )); $node->setBpConfig($this->getBpConfig()); $node->setState(2); - $node->setMissing(false) + $node->setMissing() ->setDowntime(false) - ->setAck(false) - ->setAlias($e->getMessage()); + ->setAck(false); return $node; } diff --git a/library/Businessprocess/Renderer/TileRenderer/NodeTile.php b/library/Businessprocess/Renderer/TileRenderer/NodeTile.php index 1ca46c7..652ca0e 100644 --- a/library/Businessprocess/Renderer/TileRenderer/NodeTile.php +++ b/library/Businessprocess/Renderer/TileRenderer/NodeTile.php @@ -12,6 +12,7 @@ use Icinga\Web\Url; use ipl\Html\BaseHtmlElement; use ipl\Html\Html; use ipl\Web\Widget\Icon; +use ipl\Web\Widget\Link; use ipl\Web\Widget\StateBall; class NodeTile extends BaseHtmlElement @@ -102,11 +103,7 @@ class NodeTile extends BaseHtmlElement $this->add($link); } else { - $this->add(Html::tag( - 'span', - ['class' => 'missing-node-msg'], - sprintf('Trying to access a missing business process node "%s"', $node->getNodeName()) - )); + $this->add(new Link($node->getAlias(), $this->getMainNodeUrl($node)->getAbsoluteUrl())); } if ($this->renderer->rendersSubNode() @@ -210,12 +207,15 @@ class NodeTile extends BaseHtmlElement new Icon('sitemap') )); if ($node instanceof ImportedNode) { - if ($node->getBpConfig()->hasNode($node->getName())) { + $bpConfig = $node->getBpConfig(); + if ($bpConfig->isFaulty() || $bpConfig->hasNode($node->getName())) { $this->actions()->add(Html::tag( 'a', [ 'data-base-target' => '_next', - 'href' => $this->renderer->getSourceUrl($node)->getAbsoluteUrl(), + 'href' => $bpConfig->isFaulty() + ? $this->renderer->getBaseUrl()->setParam('config', $bpConfig->getName()) + : $this->renderer->getSourceUrl($node)->getAbsoluteUrl(), 'title' => mt( 'businessprocess', 'Show this process as part of its original configuration' diff --git a/library/Businessprocess/Renderer/TreeRenderer.php b/library/Businessprocess/Renderer/TreeRenderer.php index 308d628..097d148 100644 --- a/library/Businessprocess/Renderer/TreeRenderer.php +++ b/library/Businessprocess/Renderer/TreeRenderer.php @@ -234,7 +234,9 @@ class TreeRenderer extends Renderer } elseif ($differentConfig) { $summary->add($this->actionIcon( 'share', - $this->getSourceUrl($node)->addParams(['mode' => 'tree'])->getAbsoluteUrl(), + $node->getBpConfig()->isFaulty() + ? $this->getBaseUrl()->setParam('config', $node->getBpConfig()->getName()) + : $this->getSourceUrl($node)->addParams(['mode' => 'tree'])->getAbsoluteUrl(), mt('businessprocess', 'Show this process as part of its original configuration') )->addAttributes(['data-base-target' => '_next'])); } diff --git a/public/css/module.less b/public/css/module.less index 65abdfd..500a137 100644 --- a/public/css/module.less +++ b/public/css/module.less @@ -481,8 +481,7 @@ td > a > .state-badges { border: 1px solid @body-bg-color; } - > a, - .missing-node-msg { + > a { display: block; text-decoration: none; font-size: 0.7em; @@ -492,10 +491,6 @@ td > a > .state-badges { word-wrap: break-word; } - .missing-node-msg { - font-size: 0.5em; - } - &:hover { box-shadow: 0 0 .2em @gray; } From f93873623f11bc3b948a24940a1f505977324b6f Mon Sep 17 00:00:00 2001 From: Sukhwinder Dhillon Date: Wed, 9 Aug 2023 09:54:37 +0200 Subject: [PATCH 2/4] Call it `faulty config` intead of `invalid config` --- application/clicommands/CleanupCommand.php | 2 +- library/Businessprocess/Web/Component/Dashboard.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/application/clicommands/CleanupCommand.php b/application/clicommands/CleanupCommand.php index 04fc781..f0041c8 100644 --- a/application/clicommands/CleanupCommand.php +++ b/application/clicommands/CleanupCommand.php @@ -52,7 +52,7 @@ class CleanupCommand extends Command $bp = $this->storage->loadProcess($configName); } catch (Exception $e) { Logger::error( - 'Failed to scan the %s.conf file for missing nodes. Invalid config found.', + 'Failed to scan the %s.conf file for missing nodes. Faulty config found.', $configName ); diff --git a/library/Businessprocess/Web/Component/Dashboard.php b/library/Businessprocess/Web/Component/Dashboard.php index e78c46c..d211772 100644 --- a/library/Businessprocess/Web/Component/Dashboard.php +++ b/library/Businessprocess/Web/Component/Dashboard.php @@ -100,7 +100,7 @@ class Dashboard extends BaseHtmlElement $this->add(new BpDashboardTile( new BpConfig(), $title, - sprintf(t('File %s has invalid config'), $name . '.conf'), + sprintf(t('File %s has faulty config'), $name . '.conf'), 'file-circle-xmark', 'businessprocess/process/show', ['config' => $name] From 25f37d6575dc47b5957417f376150e8acdedf21c Mon Sep 17 00:00:00 2001 From: Sukhwinder Dhillon Date: Wed, 9 Aug 2023 12:29:18 +0200 Subject: [PATCH 3/4] Use unescaped node name for missing children --- application/controllers/ProcessController.php | 2 +- library/Businessprocess/BpConfig.php | 2 +- library/Businessprocess/BpNode.php | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/application/controllers/ProcessController.php b/application/controllers/ProcessController.php index c735af2..0cfced2 100644 --- a/application/controllers/ProcessController.php +++ b/application/controllers/ProcessController.php @@ -448,7 +448,7 @@ class ProcessController extends Controller protected function prepareMissingNodeLinks(HtmlElement $ul): void { - $missing = $this->bp->getMissingChildren(); + $missing = array_keys($this->bp->getMissingChildren()); if (! empty($missing)) { $missingLinkedNodes = null; foreach ($this->bp->getImportedNodes() as $process) { diff --git a/library/Businessprocess/BpConfig.php b/library/Businessprocess/BpConfig.php index 3e652e9..e2fd5c9 100644 --- a/library/Businessprocess/BpConfig.php +++ b/library/Businessprocess/BpConfig.php @@ -701,7 +701,7 @@ class BpConfig $msg = $this->isFaulty() ? sprintf( t('Trying to import node "%s" from faulty config file "%s.conf"'), - $name, + self::unescapeName($name), $this->getName() ) : sprintf(t('Trying to access a missing business process node "%s"'), $name); diff --git a/library/Businessprocess/BpNode.php b/library/Businessprocess/BpNode.php index 67a2602..419f836 100644 --- a/library/Businessprocess/BpNode.php +++ b/library/Businessprocess/BpNode.php @@ -275,11 +275,11 @@ class BpNode extends Node foreach ($this->getChildren() as $child) { if ($child->isMissing()) { - $missing[$child->getName()] = $child; + $missing[$child->getAlias()] = $child; } foreach ($child->getMissingChildren() as $m) { - $missing[$m->getName()] = $m; + $missing[$m->getAlias()] = $m; } } From 2576fc3dc310558888c64f9895007061ab514f72 Mon Sep 17 00:00:00 2001 From: Sukhwinder Dhillon Date: Wed, 9 Aug 2023 12:38:28 +0200 Subject: [PATCH 4/4] BpConfig: Don't add same error message multiple times --- library/Businessprocess/BpConfig.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/library/Businessprocess/BpConfig.php b/library/Businessprocess/BpConfig.php index e2fd5c9..30cdf56 100644 --- a/library/Businessprocess/BpConfig.php +++ b/library/Businessprocess/BpConfig.php @@ -929,7 +929,10 @@ class BpConfig throw new IcingaException($msg); } - $this->errors[] = $msg; + if (! in_array($msg, $this->errors)) { + $this->errors[] = $msg; + } + return $this; }