controllers, libs: less noise from exceptions

This commit is contained in:
Thomas Gelf 2018-08-08 10:07:39 +02:00
parent 440186fa0e
commit 14a3039f65
7 changed files with 103 additions and 88 deletions

View File

@ -22,12 +22,13 @@ use Icinga\Module\Director\Web\Widget\ActivityLogInfo;
use Icinga\Module\Director\Web\Widget\DeployedConfigInfoHeader; use Icinga\Module\Director\Web\Widget\DeployedConfigInfoHeader;
use Icinga\Module\Director\Web\Widget\ShowConfigFile; use Icinga\Module\Director\Web\Widget\ShowConfigFile;
use Icinga\Web\Notification; use Icinga\Web\Notification;
use Icinga\Web\Url;
use Exception; use Exception;
use RuntimeException;
use dipl\Html\Html; use dipl\Html\Html;
use dipl\Html\HtmlString; use dipl\Html\HtmlString;
use dipl\Html\Icon; use dipl\Html\Icon;
use dipl\Html\Link; use dipl\Html\Link;
use dipl\Web\Url;
class ConfigController extends ActionController class ConfigController extends ActionController
{ {
@ -38,10 +39,7 @@ class ConfigController extends ActionController
} }
/** /**
* @throws IcingaException * @throws \Icinga\Security\SecurityException
* @throws \Icinga\Exception\ConfigurationError
* @throws \Icinga\Exception\Http\HttpNotFoundException
* @throws \Icinga\Exception\ProgrammingError
*/ */
public function deploymentsAction() public function deploymentsAction()
{ {
@ -84,9 +82,9 @@ class ConfigController extends ActionController
} }
/** /**
* @throws IcingaException
* @throws NotFoundError * @throws NotFoundError
* @throws \Icinga\Exception\ConfigurationError * @throws \Icinga\Module\Director\Exception\DuplicateKeyException
* @throws \Icinga\Security\SecurityException
*/ */
public function deployAction() public function deployAction()
{ {
@ -96,10 +94,10 @@ class ConfigController extends ActionController
} }
if (! $request->isPost()) { if (! $request->isPost()) {
throw new IcingaException( throw new RuntimeException(sprintf(
'Unsupported method: %s', 'Unsupported method: %s',
$request->getMethod() $request->getMethod()
); ));
} }
$this->assertPermission('director/deploy'); $this->assertPermission('director/deploy');
@ -126,10 +124,7 @@ class ConfigController extends ActionController
} }
/** /**
* @throws IcingaException * @throws \Icinga\Security\SecurityException
* @throws \Icinga\Exception\ConfigurationError
* @throws \Icinga\Exception\Http\HttpNotFoundException
* @throws \Icinga\Exception\ProgrammingError
*/ */
public function activitiesAction() public function activitiesAction()
{ {
@ -180,7 +175,6 @@ class ConfigController extends ActionController
/** /**
* @throws IcingaException * @throws IcingaException
* @throws \Icinga\Exception\ConfigurationError
* @throws \Icinga\Exception\Http\HttpNotFoundException * @throws \Icinga\Exception\Http\HttpNotFoundException
* @throws \Icinga\Exception\ProgrammingError * @throws \Icinga\Exception\ProgrammingError
*/ */
@ -209,8 +203,7 @@ class ConfigController extends ActionController
} }
/** /**
* @throws IcingaException * @throws \Icinga\Security\SecurityException
* @throws \Icinga\Exception\ConfigurationError
*/ */
public function settingsAction() public function settingsAction()
{ {
@ -231,11 +224,8 @@ class ConfigController extends ActionController
/** /**
* Show all files for a given config * Show all files for a given config
* *
* @throws IcingaException
* @throws \Icinga\Exception\ConfigurationError
* @throws \Icinga\Exception\Http\HttpNotFoundException
* @throws \Icinga\Exception\MissingParameterException * @throws \Icinga\Exception\MissingParameterException
* @throws \Icinga\Exception\ProgrammingError * @throws \Icinga\Security\SecurityException
*/ */
public function filesAction() public function filesAction()
{ {
@ -280,11 +270,8 @@ class ConfigController extends ActionController
/** /**
* Show a single file * Show a single file
* *
* @throws IcingaException
* @throws \Icinga\Exception\ConfigurationError
* @throws \Icinga\Exception\Http\HttpNotFoundException
* @throws \Icinga\Exception\MissingParameterException * @throws \Icinga\Exception\MissingParameterException
* @throws \Icinga\Exception\ProgrammingError * @throws \Icinga\Security\SecurityException
*/ */
public function fileAction() public function fileAction()
{ {
@ -318,8 +305,7 @@ class ConfigController extends ActionController
/** /**
* TODO: Check if this can be removed * TODO: Check if this can be removed
* *
* @throws \Icinga\Exception\ConfigurationError * @throws \Icinga\Security\SecurityException
* @throws \Icinga\Exception\ProgrammingError
*/ */
public function storeAction() public function storeAction()
{ {
@ -334,9 +320,7 @@ class ConfigController extends ActionController
} }
/** /**
* @throws IcingaException * @throws \Icinga\Security\SecurityException
* @throws \Icinga\Exception\ConfigurationError
* @throws \Icinga\Exception\ProgrammingError
*/ */
public function diffAction() public function diffAction()
{ {
@ -387,7 +371,6 @@ class ConfigController extends ActionController
/** /**
* @throws IcingaException * @throws IcingaException
* @throws \Icinga\Exception\ConfigurationError
* @throws \Icinga\Exception\MissingParameterException * @throws \Icinga\Exception\MissingParameterException
*/ */
public function filediffAction() public function filediffAction()
@ -417,7 +400,6 @@ class ConfigController extends ActionController
/** /**
* @param $checksum * @param $checksum
* @throws \Icinga\Exception\ProgrammingError
*/ */
protected function deploymentSucceeded($checksum) protected function deploymentSucceeded($checksum)
{ {
@ -436,7 +418,6 @@ class ConfigController extends ActionController
/** /**
* @param $checksum * @param $checksum
* @param null $error * @param null $error
* @throws \Icinga\Exception\ProgrammingError
*/ */
protected function deploymentFailed($checksum, $error = null) protected function deploymentFailed($checksum, $error = null)
{ {
@ -456,7 +437,6 @@ class ConfigController extends ActionController
/** /**
* @return \dipl\Web\Widget\Tabs * @return \dipl\Web\Widget\Tabs
* @throws \Icinga\Exception\ProgrammingError
*/ */
protected function configTabs() protected function configTabs()
{ {

View File

@ -3,6 +3,8 @@
namespace Icinga\Module\Director\Controllers; namespace Icinga\Module\Director\Controllers;
use dipl\Html\Html; use dipl\Html\Html;
use dipl\Html\Link;
use dipl\Web\Url;
use Exception; use Exception;
use Icinga\Module\Director\CustomVariable\CustomVariableDictionary; use Icinga\Module\Director\CustomVariable\CustomVariableDictionary;
use Icinga\Module\Director\Db\AppliedServiceSetLoader; use Icinga\Module\Director\Db\AppliedServiceSetLoader;
@ -21,8 +23,6 @@ use Icinga\Module\Director\Web\Table\IcingaHostAppliedServicesTable;
use Icinga\Module\Director\Web\Table\IcingaHostServiceTable; use Icinga\Module\Director\Web\Table\IcingaHostServiceTable;
use Icinga\Module\Director\Web\Table\IcingaServiceSetServiceTable; use Icinga\Module\Director\Web\Table\IcingaServiceSetServiceTable;
use Icinga\Module\Director\Web\Widget\HostServiceRedirector; use Icinga\Module\Director\Web\Widget\HostServiceRedirector;
use Icinga\Web\Url;
use dipl\Html\Link;
class HostController extends ObjectController class HostController extends ObjectController
{ {
@ -33,7 +33,6 @@ class HostController extends ObjectController
/** /**
* @return HostgroupRestriction * @return HostgroupRestriction
* @throws \Icinga\Exception\ConfigurationError
*/ */
protected function getHostgroupRestriction() protected function getHostgroupRestriction()
{ {
@ -46,10 +45,6 @@ class HostController extends ObjectController
$this->addOptionalMonitoringLink(); $this->addOptionalMonitoringLink();
} }
/**
* @throws \Icinga\Exception\ConfigurationError
* @throws \Icinga\Exception\Http\HttpNotFoundException
*/
public function serviceAction() public function serviceAction()
{ {
$host = $this->getHostObject(); $host = $this->getHostObject();
@ -63,10 +58,6 @@ class HostController extends ObjectController
); );
} }
/**
* @throws \Icinga\Exception\ConfigurationError
* @throws \Icinga\Exception\Http\HttpNotFoundException
*/
public function servicesetAction() public function servicesetAction()
{ {
$host = $this->getHostObject(); $host = $this->getHostObject();
@ -80,9 +71,6 @@ class HostController extends ObjectController
); );
} }
/**
* @throws \Icinga\Exception\Http\HttpNotFoundException
*/
protected function addServicesHeader() protected function addServicesHeader()
{ {
$host = $this->getHostObject(); $host = $this->getHostObject();
@ -115,8 +103,6 @@ class HostController extends ObjectController
} }
/** /**
* @throws \Icinga\Exception\ConfigurationError
* @throws \Icinga\Exception\Http\HttpNotFoundException
* @throws \Icinga\Exception\NotFoundError * @throws \Icinga\Exception\NotFoundError
*/ */
public function invalidserviceAction() public function invalidserviceAction()
@ -132,8 +118,6 @@ class HostController extends ObjectController
} }
/** /**
* @throws \Icinga\Exception\ConfigurationError
* @throws \Icinga\Exception\Http\HttpNotFoundException
* @throws \Icinga\Exception\NotFoundError * @throws \Icinga\Exception\NotFoundError
*/ */
public function servicesAction() public function servicesAction()
@ -203,7 +187,6 @@ class HostController extends ObjectController
/** /**
* @param IcingaHost $host * @param IcingaHost $host
* @param IcingaHost|null $affectedHost * @param IcingaHost|null $affectedHost
* @throws \Icinga\Exception\ConfigurationError
*/ */
protected function addHostServiceSetTables(IcingaHost $host, IcingaHost $affectedHost = null) protected function addHostServiceSetTables(IcingaHost $host, IcingaHost $affectedHost = null)
{ {
@ -240,8 +223,6 @@ class HostController extends ObjectController
} }
/** /**
* @throws \Icinga\Exception\ConfigurationError
* @throws \Icinga\Exception\Http\HttpNotFoundException
* @throws \Icinga\Exception\NotFoundError * @throws \Icinga\Exception\NotFoundError
*/ */
public function appliedserviceAction() public function appliedserviceAction()
@ -278,8 +259,6 @@ class HostController extends ObjectController
} }
/** /**
* @throws \Icinga\Exception\ConfigurationError
* @throws \Icinga\Exception\Http\HttpNotFoundException
* @throws \Icinga\Exception\NotFoundError * @throws \Icinga\Exception\NotFoundError
*/ */
public function inheritedserviceAction() public function inheritedserviceAction()
@ -319,9 +298,7 @@ class HostController extends ObjectController
} }
/** /**
* @throws \Icinga\Exception\ConfigurationError
* @throws \Icinga\Exception\NotFoundError * @throws \Icinga\Exception\NotFoundError
* @throws \Icinga\Exception\ProgrammingError
*/ */
public function removesetAction() public function removesetAction()
{ {
@ -348,8 +325,6 @@ class HostController extends ObjectController
} }
/** /**
* @throws \Icinga\Exception\ConfigurationError
* @throws \Icinga\Exception\Http\HttpNotFoundException
* @throws \Icinga\Exception\NotFoundError * @throws \Icinga\Exception\NotFoundError
*/ */
public function servicesetserviceAction() public function servicesetserviceAction()
@ -390,9 +365,6 @@ class HostController extends ObjectController
$this->commonForServices(); $this->commonForServices();
} }
/**
* @throws \Icinga\Exception\Http\HttpNotFoundException
*/
protected function commonForServices() protected function commonForServices()
{ {
$host = $this->object; $host = $this->object;
@ -406,9 +378,7 @@ class HostController extends ObjectController
} }
/** /**
* @throws \Icinga\Exception\Http\HttpNotFoundException
* @throws \Icinga\Exception\NotFoundError * @throws \Icinga\Exception\NotFoundError
* @throws \Icinga\Exception\ProgrammingError
*/ */
public function agentAction() public function agentAction()
{ {

View File

@ -10,9 +10,7 @@ use RuntimeException;
class Migrations class Migrations
{ {
/** /** @var \Zend_Db_Adapter_Abstract */
* @var \Zend_Db_Adapter_Abstract
*/
protected $db; protected $db;
/** /**

View File

@ -2,9 +2,9 @@
namespace Icinga\Module\Director\Web\Controller\Extension; namespace Icinga\Module\Director\Web\Controller\Extension;
use Icinga\Exception\ConfigurationError;
use Icinga\Module\Director\Db; use Icinga\Module\Director\Db;
use Icinga\Module\Director\Web\Controller\ActionController; use Icinga\Module\Director\Web\Controller\ActionController;
use RuntimeException;
trait DirectorDb trait DirectorDb
{ {
@ -111,7 +111,6 @@ trait DirectorDb
} }
/** /**
* @throws ConfigurationError
* *
* @return Db * @return Db
*/ */
@ -123,12 +122,12 @@ trait DirectorDb
$this->db = Db::fromResourceName($resourceName); $this->db = Db::fromResourceName($resourceName);
} elseif ($this instanceof ActionController) { } elseif ($this instanceof ActionController) {
if ($this->getRequest()->isApiRequest()) { if ($this->getRequest()->isApiRequest()) {
throw new ConfigurationError('Icinga Director is not correctly configured'); throw new RuntimeException('Icinga Director is not correctly configured');
} else { } else {
$this->redirectNow('director'); $this->redirectNow('director');
} }
} else { } else {
throw new ConfigurationError('Icinga Director is not correctly configured'); throw new RuntimeException('Icinga Director is not correctly configured');
} }
} }

View File

@ -3,6 +3,7 @@
namespace Icinga\Module\Director\Web; namespace Icinga\Module\Director\Web;
use Exception; use Exception;
use Icinga\Exception\ProgrammingError;
use Icinga\Module\Director\Core\CoreApi; use Icinga\Module\Director\Core\CoreApi;
use Icinga\Module\Director\Forms\IcingaForgetApiKeyForm; use Icinga\Module\Director\Forms\IcingaForgetApiKeyForm;
use Icinga\Module\Director\Forms\IcingaGenerateApiKeyForm; use Icinga\Module\Director\Forms\IcingaGenerateApiKeyForm;
@ -35,7 +36,6 @@ class SelfService
/** /**
* @param ControlsAndContent $controller * @param ControlsAndContent $controller
* @throws \Icinga\Exception\ProgrammingError
*/ */
public function renderTo(ControlsAndContent $controller) public function renderTo(ControlsAndContent $controller)
{ {
@ -74,7 +74,6 @@ class SelfService
/** /**
* @param ControlsAndContent $cc * @param ControlsAndContent $cc
* @throws \Icinga\Exception\ProgrammingError
*/ */
protected function showSelfServiceTemplateInstructions(ControlsAndContent $cc) protected function showSelfServiceTemplateInstructions(ControlsAndContent $cc)
{ {
@ -100,7 +99,7 @@ class SelfService
] ]
)); ));
if (Icinga::app()->getModuleManager()->hasLoaded('doc')) { if ($this->hasDocsModuleLoaded()) {
$actions->add(Link::create( $actions->add(Link::create(
$this->translate('Documentation'), $this->translate('Documentation'),
'doc/module/director/chapter/Self-Service-API', 'doc/module/director/chapter/Self-Service-API',
@ -144,7 +143,6 @@ class SelfService
/** /**
* @param ControlsAndContent $cc * @param ControlsAndContent $cc
* @throws \Icinga\Exception\ProgrammingError
*/ */
protected function showNewAgentInstructions(ControlsAndContent $cc) protected function showNewAgentInstructions(ControlsAndContent $cc)
{ {
@ -153,7 +151,7 @@ class SelfService
$key = $host->getSingleResolvedProperty('api_key'); $key = $host->getSingleResolvedProperty('api_key');
$cc->addTitle($this->translate('Configure this Agent via Self Service API')); $cc->addTitle($this->translate('Configure this Agent via Self Service API'));
if (Icinga::app()->getModuleManager()->hasLoaded('doc')) { if ($this->hasDocsModuleLoaded()) {
$actions = $cc->actions(); $actions = $cc->actions();
$actions->add(Link::create( $actions->add(Link::create(
$this->translate('Documentation'), $this->translate('Documentation'),
@ -276,4 +274,16 @@ class SelfService
echo $script; echo $script;
exit; exit;
} }
/**
* @return bool
*/
protected function hasDocsModuleLoaded()
{
try {
return Icinga::app()->getModuleManager()->hasLoaded('doc');
} catch (ProgrammingError $e) {
return false;
}
}
} }

View File

@ -8,11 +8,12 @@ use Icinga\Data\Filter\FilterChain;
use Icinga\Data\Filter\FilterExpression; use Icinga\Data\Filter\FilterExpression;
use Icinga\Data\Filter\FilterNot; use Icinga\Data\Filter\FilterNot;
use Icinga\Data\Filter\FilterOr; use Icinga\Data\Filter\FilterOr;
use Icinga\Exception\ProgrammingError; use InvalidArgumentException;
use Icinga\Exception\QueryException; use RuntimeException;
use Zend_Db_Adapter_Abstract as DbAdapter; use Zend_Db_Adapter_Abstract as DbAdapter;
use Zend_Db_Expr as DbExpr; use Zend_Db_Expr as DbExpr;
use Zend_Db_Select as DbSelect; use Zend_Db_Select as DbSelect;
use Zend_Db_Select_Exception as DbSelectException;
class FilterRenderer class FilterRenderer
{ {
@ -66,7 +67,14 @@ class FilterRenderer
protected function extractColumnMap(DbSelect $query) protected function extractColumnMap(DbSelect $query)
{ {
$map = []; $map = [];
foreach ($query->getPart(DbSelect::COLUMNS) as $col) { try {
$columns = $query->getPart(DbSelect::COLUMNS);
} catch (DbSelectException $e) {
// Will not happen.
throw new RuntimeException($e->getMessage());
}
foreach ($columns as $col) {
if ($col[1] instanceof DbExpr) { if ($col[1] instanceof DbExpr) {
$map[$col[2]] = (string) $col[1]; $map[$col[2]] = (string) $col[1];
} else { } else {
@ -97,21 +105,27 @@ class FilterRenderer
$op = ' AND '; $op = ' AND ';
$prefix = 'NOT '; $prefix = 'NOT ';
} else { } else {
throw new ProgrammingError( throw new InvalidArgumentException(
'Cannot render a %s filter chain for Zf Db', 'Cannot render a %s filter chain for Zf Db',
get_class($filter) get_class($filter)
); );
} }
$parts = array(); $parts = [];
if (! $filter->isEmpty()) { if ($filter->isEmpty()) {
// Hint: we might want to fail here
return '';
} else {
foreach ($filter->filters() as $f) { foreach ($filter->filters() as $f) {
$part = $this->renderFilter($f, $level + 1); $part = $this->renderFilter($f, $level + 1);
if ($part !== '') { if ($part !== '') {
$parts[] = $part; $parts[] = $part;
} }
} }
if (! empty($parts)) { if (empty($parts)) {
// will not happen, as we are not empty
return '';
} else {
if ($level > 0) { if ($level > 0) {
return "$prefix (" . implode($op, $parts) . ')'; return "$prefix (" . implode($op, $parts) . ')';
} else { } else {
@ -124,6 +138,9 @@ class FilterRenderer
protected function renderFilterExpression(FilterExpression $filter) protected function renderFilterExpression(FilterExpression $filter)
{ {
$col = $this->lookupColumnAlias($filter->getColumn()); $col = $this->lookupColumnAlias($filter->getColumn());
if (! ctype_digit($col)) {
$col = $this->db->quoteIdentifier($col);
}
$sign = $filter->getSign(); $sign = $filter->getSign();
$expression = $filter->getExpression(); $expression = $filter->getExpression();
@ -195,7 +212,7 @@ class FilterRenderer
); );
} }
throw new ProgrammingError( throw new InvalidArgumentException(
'Array expressions can only be rendered for = and !=, got %s', 'Array expressions can only be rendered for = and !=, got %s',
$sign $sign
); );
@ -210,8 +227,13 @@ class FilterRenderer
{ {
if ($filter instanceof FilterChain) { if ($filter instanceof FilterChain) {
return $this->renderFilterChain($filter, $level); return $this->renderFilterChain($filter, $level);
} else { } elseif ($filter instanceof FilterExpression) {
return $this->renderFilterExpression($filter); return $this->renderFilterExpression($filter);
} else {
throw new RuntimeException(sprintf(
'Filter of type FilterChain or FilterExpression expected, got %s',
get_class($filter)
));
} }
} }
@ -219,7 +241,7 @@ class FilterRenderer
{ {
// bindParam? bindValue? // bindParam? bindValue?
if (is_array($value)) { if (is_array($value)) {
$ret = array(); $ret = [];
foreach ($value as $val) { foreach ($value as $val) {
$ret[] = $this->escape($val); $ret[] = $this->escape($val);
} }
@ -243,7 +265,9 @@ class FilterRenderer
return sprintf('(%1$s NOT IN (%2$s) OR %1$s IS NULL)', $col, $this->escape($expression)); return sprintf('(%1$s NOT IN (%2$s) OR %1$s IS NULL)', $col, $this->escape($expression));
} }
throw new QueryException('Unable to render array expressions with operators other than equal or not equal'); throw new InvalidArgumentException(
'Unable to render array expressions with operators other than equal or not equal'
);
} elseif ($sign === '=' && strpos($expression, '*') !== false) { } elseif ($sign === '=' && strpos($expression, '*') !== false) {
if ($expression === '*') { if ($expression === '*') {
return new DbExpr('TRUE'); return new DbExpr('TRUE');

View File

@ -2,9 +2,43 @@
namespace dipl\Web\Widget; namespace dipl\Web\Widget;
use Exception;
use Icinga\Web\Widget\Tabs as WebTabs; use Icinga\Web\Widget\Tabs as WebTabs;
use InvalidArgumentException;
use dipl\Html\ValidHtml; use dipl\Html\ValidHtml;
class Tabs extends WebTabs implements ValidHtml class Tabs extends WebTabs implements ValidHtml
{ {
/**
* @param string $name
* @return $this
*/
public function activate($name)
{
try {
parent::activate($name);
} catch (Exception $e) {
throw new InvalidArgumentException(
"Can't activate '$name', there is no such tab"
);
}
return $this;
}
/**
* @param string $name
* @param array|\Icinga\Web\Widget\Tab $tab
* @return $this
*/
public function add($name, $tab)
{
try {
parent::add($name, $tab);
} catch (Exception $e) {
throw new InvalidArgumentException($e->getMessage());
}
return $this;
}
} }