From d1526a36cbca35c070b0574ea0d2332dcc61ad6e Mon Sep 17 00:00:00 2001 From: Ravi Kumar Kempapura Srinivasa Date: Tue, 12 May 2020 23:20:01 +0200 Subject: [PATCH] Address the comments on IcingaDbState and the Controllers Address the comments from Eric in the pull request on IcingaDbState, Web/Controller, HostController and ServiceController. --- application/controllers/HostController.php | 9 +- application/controllers/ServiceController.php | 11 ++- .../Businessprocess/State/IcingaDbState.php | 12 +-- library/Businessprocess/Web/Controller.php | 91 ------------------- 4 files changed, 18 insertions(+), 105 deletions(-) diff --git a/application/controllers/HostController.php b/application/controllers/HostController.php index 176a317..0115516 100644 --- a/application/controllers/HostController.php +++ b/application/controllers/HostController.php @@ -3,10 +3,8 @@ namespace Icinga\Module\Businessprocess\Controllers; use Icinga\Module\Businessprocess\Common\IcingadbDatabase; -//use Icinga\Module\Businessprocess\Web\Controller; use Icinga\Module\Businessprocess\IcingaDbBackend; use Icinga\Module\Icingadb\Model\Host; -use Icinga\Module\Monitoring\Backend; use Icinga\Module\Monitoring\Controller; use Icinga\Web\Url; @@ -16,10 +14,11 @@ class HostController extends Controller public function showAction() { - $hostName = $this->params->get('host'); - $icingadb = $this->params->get('icingadb'); + $icingadb = $this->params->shift('icingadb'); if ($icingadb) { + $hostName = $this->params->shift('host'); + $host = Host::on($this->getDb()); $host->getSelectBase() ->where(['host.name = ?' => $hostName]); @@ -33,6 +32,8 @@ class HostController extends Controller $this->redirectNow(Url::fromPath('icingadb/host')->setParams($this->params)); } } else { + $hostName = $this->params->get('host'); + $query = $this->backend->select() ->from('hoststatus', array('host_name')) ->where('host_name', $hostName); diff --git a/application/controllers/ServiceController.php b/application/controllers/ServiceController.php index 2313291..5b03aed 100644 --- a/application/controllers/ServiceController.php +++ b/application/controllers/ServiceController.php @@ -5,7 +5,6 @@ namespace Icinga\Module\Businessprocess\Controllers; use Icinga\Module\Businessprocess\Common\IcingadbDatabase; use Icinga\Module\Businessprocess\IcingaDbBackend; use Icinga\Module\Icingadb\Model\Service; -use Icinga\Module\Monitoring\Backend; use Icinga\Module\Monitoring\Controller; use Icinga\Web\Url; @@ -15,11 +14,12 @@ class ServiceController extends Controller public function showAction() { - $hostName = $this->params->get('host'); - $serviceName = $this->params->get('service'); - $icingadb = $this->params->get('icingadb'); + $icingadb = $this->params->shift('icingadb'); if ($icingadb) { + $hostName = $this->params->shift('host'); + $serviceName = $this->params->shift('service'); + $service = Service::on($this->getDb())->with('host'); $service->getSelectBase() ->where(['service_host.name = ?' => $hostName, 'service.name = ?' => $serviceName]); @@ -35,6 +35,9 @@ class ServiceController extends Controller $this->redirectNow(Url::fromPath('icingadb/service')->setParams($this->params)); } } else { + $hostName = $this->params->get('host'); + $serviceName = $this->params->get('service'); + $query = $this->backend->select() ->from('servicestatus', array('service_description')) ->where('host_name', $hostName) diff --git a/library/Businessprocess/State/IcingaDbState.php b/library/Businessprocess/State/IcingaDbState.php index 1a14df4..09af2ce 100644 --- a/library/Businessprocess/State/IcingaDbState.php +++ b/library/Businessprocess/State/IcingaDbState.php @@ -29,6 +29,7 @@ class IcingaDbState extends IcingaDbBackend { $self = new static($config); $self->retrieveStatesFromBackend(); + return $config; } @@ -51,7 +52,7 @@ class IcingaDbState extends IcingaDbBackend $config = $this->config; Benchmark::measure(sprintf( - 'Retrieving states for business process %s', + 'Retrieving states for business process %s using Icinga DB backend', $config->getName() )); @@ -73,14 +74,14 @@ class IcingaDbState extends IcingaDbBackend $stateCol = 'state.soft_state'; } - $hostStatusCols = array( + $hostStatusCols = [ 'hostname' => 'host.name', 'last_state_change' => 'state.last_state_change', 'in_downtime' => 'state.in_downtime', 'ack' => 'state.is_acknowledged', 'state' => $stateCol, 'display_name' =>'host.display_name' - ); + ]; $queryHost = $queryHost->columns($hostStatusCols)->assembleSelect(); @@ -98,7 +99,7 @@ class IcingaDbState extends IcingaDbBackend IcingaDbBackend::applyMonitoringRestriction($queryService); - $serviceStatusCols = array( + $serviceStatusCols = [ 'hostname' => 'host.name', 'service' => 'service.name', 'last_state_change' => 'state.last_state_change', @@ -107,7 +108,7 @@ class IcingaDbState extends IcingaDbBackend 'state' => $stateCol, 'display_name' => 'service.display_name', 'host_display_name' => 'host.display_name' - ); + ]; $queryService = $queryService->columns($serviceStatusCols)->assembleSelect(); @@ -128,7 +129,6 @@ class IcingaDbState extends IcingaDbBackend } } - // TODO: Union, single query? Benchmark::measure('Got states for business process ' . $config->getName()); return $this; diff --git a/library/Businessprocess/Web/Controller.php b/library/Businessprocess/Web/Controller.php index 3079fab..d1104d8 100644 --- a/library/Businessprocess/Web/Controller.php +++ b/library/Businessprocess/Web/Controller.php @@ -3,10 +3,6 @@ namespace Icinga\Module\Businessprocess\Web; use Icinga\Application\Icinga; -use Icinga\Data\Filter\Filter; -use Icinga\Data\Filterable; -use Icinga\Exception\ConfigurationError; -use Icinga\Exception\QueryException; use Icinga\Module\Businessprocess\BpConfig; use Icinga\Module\Businessprocess\Modification\ProcessChanges; use Icinga\Module\Businessprocess\Storage\LegacyStorage; @@ -16,14 +12,10 @@ use Icinga\Module\Businessprocess\Web\Component\Controls; use Icinga\Module\Businessprocess\Web\Component\Content; use Icinga\Module\Businessprocess\Web\Component\Tabs; use Icinga\Module\Businessprocess\Web\Form\FormLoader; -use Icinga\Module\Icingadb\Compat\MonitoringRestrictions; -use Icinga\Module\Icingadb\Compat\UrlMigrator; use Icinga\Web\Controller as ModuleController; use Icinga\Web\Notification; use Icinga\Web\View; use ipl\Html\Html; -use ipl\Orm\Compat\FilterProcessor; -use ipl\Orm\Query; class Controller extends ModuleController { @@ -275,87 +267,4 @@ class Controller extends ModuleController return $this->storage; } - - /** - * Apply a restriction of the authenticated on the given filterable - * - * @param string $name Name of the restriction - * @param Filterable $filterable Filterable to restrict - * - * @return Filterable The filterable having the restriction applied - */ - protected function applyRestriction($name, Filterable $filterable) - { - $filterable->applyFilter($this->getRestriction($name)); - return $filterable; - } - - /** - * Get a restriction of the authenticated - * - * @param string $name Name of the restriction - * - * @return Filter Filter object - * @throws ConfigurationError If the restriction contains invalid filter columns - */ - protected function getRestriction($name) - { - $restriction = Filter::matchAny(); - $restriction->setAllowedFilterColumns(array( - 'host_name', - 'hostgroup_name', - 'instance_name', - 'service_description', - 'servicegroup_name', - function ($c) { - return preg_match('/^_(?:host|service)_/i', $c); - } - )); - foreach ($this->getRestrictions($name) as $filter) { - if ($filter === '*') { - return Filter::matchAll(); - } - try { - $restriction->addFilter(Filter::fromQueryString($filter)); - } catch (QueryException $e) { - throw new ConfigurationError( - $this->translate( - 'Cannot apply restriction %s using the filter %s. You can only use the following columns: %s' - ), - $name, - $filter, - implode(', ', array( - 'instance_name', - 'host_name', - 'hostgroup_name', - 'service_description', - 'servicegroup_name', - '_(host|service)_' - )), - $e - ); - } - } - - if ($restriction->isEmpty()) { - return Filter::matchAll(); - } - - return $restriction; - } - - public function applyMonitoringRestriction(Query $query, $queryTransformer = null) - { - if ($queryTransformer === null || UrlMigrator::hasQueryTransformer($queryTransformer)) { - $restriction = UrlMigrator::transformFilter( - MonitoringRestrictions::getRestriction('monitoring/filter/objects'), - $queryTransformer - ); - if ($restriction) { - FilterProcessor::apply($restriction, $query); - } - } - - return $this; - } }