From 8c432285e7b0b2ecb08f5769a02779f9c199bb85 Mon Sep 17 00:00:00 2001 From: Thomas Gelf Date: Tue, 29 May 2018 21:29:38 +0200 Subject: [PATCH] Various: stick with default exceptions... ...IDE hints are annoying --- .../CustomVariable/CustomVariable.php | 18 ++-- library/Director/Data/Db/DbObject.php | 95 ++++++++++--------- library/Director/Db/Cache/PrefetchCache.php | 6 +- .../Director/IcingaConfig/IcingaConfig.php | 24 ++--- library/Director/Objects/IcingaObject.php | 48 ++++++---- .../Objects/IcingaObjectLegacyAssignments.php | 8 +- library/Director/Objects/IcingaServiceSet.php | 10 +- .../Director/Objects/IcingaHostTest.php | 2 +- .../Director/Objects/IcingaServiceSetTest.php | 6 +- .../Director/Objects/IcingaServiceTest.php | 2 +- 10 files changed, 115 insertions(+), 104 deletions(-) diff --git a/library/Director/CustomVariable/CustomVariable.php b/library/Director/CustomVariable/CustomVariable.php index 24c910b3..e440afd4 100644 --- a/library/Director/CustomVariable/CustomVariable.php +++ b/library/Director/CustomVariable/CustomVariable.php @@ -2,11 +2,12 @@ namespace Icinga\Module\Director\CustomVariable; -use Icinga\Exception\ProgrammingError; +use Exception; use Icinga\Module\Director\Db\Cache\PrefetchCache; use Icinga\Module\Director\IcingaConfig\IcingaConfigHelper as c; use Icinga\Module\Director\IcingaConfig\IcingaConfigRenderer; -use Exception; +use InvalidArgumentException; +use LogicException; abstract class CustomVariable implements IcingaConfigRenderer { @@ -97,16 +98,15 @@ abstract class CustomVariable implements IcingaConfigRenderer /** * @param bool $renderExpressions - * @throws ProgrammingError * @return string */ public function toConfigString($renderExpressions = false) { // TODO: this should be an abstract method once we deprecate PHP < 5.3.9 - throw new ProgrammingError( + throw new LogicException(sprintf( '%s has no toConfigString() implementation', get_class($this) - ); + )); } public function flatten(array & $flat, $prefix) @@ -236,7 +236,7 @@ abstract class CustomVariable implements IcingaConfigRenderer // TODO: check for specific class/stdClass/interface? return new CustomVariableDictionary($key, $value); } else { - throw new ProgrammingError('WTF (%s): %s', $key, var_export($value, 1)); + throw new LogicException(sprintf('WTF (%s): %s', $key, var_export($value, 1))); } } @@ -250,14 +250,14 @@ abstract class CustomVariable implements IcingaConfigRenderer $var = self::create($row->varname, json_decode($row->varvalue)); break; case 'expression': - throw new ProgrammingError( + throw new InvalidArgumentException( 'Icinga code expressions are not yet supported' ); default: - throw new ProgrammingError( + throw new InvalidArgumentException(sprintf( '%s is not a supported custom variable format', $row->format - ); + )); } if (property_exists($row, 'checksum')) { $var->setChecksum($row->checksum); diff --git a/library/Director/Data/Db/DbObject.php b/library/Director/Data/Db/DbObject.php index 0b934999..2e3b8fa5 100644 --- a/library/Director/Data/Db/DbObject.php +++ b/library/Director/Data/Db/DbObject.php @@ -2,13 +2,15 @@ namespace Icinga\Module\Director\Data\Db; -use Icinga\Exception\IcingaException as IE; use Icinga\Exception\NotFoundError; use Icinga\Module\Director\Db; use Icinga\Module\Director\Exception\DuplicateKeyException; use Icinga\Module\Director\Util; -use Exception; +use InvalidArgumentException; +use LogicException; +use RuntimeException; use Zend_Db_Adapter_Abstract; +use Zend_Db_Exception; /** * Base class for ... @@ -88,7 +90,7 @@ abstract class DbObject || $this->keyName === null || $this->defaultProperties === null ) { - throw new IE("Someone extending this class didn't RTFM"); + throw new LogicException("Someone extending this class didn't RTFM"); } $this->properties = $this->defaultProperties; @@ -227,8 +229,6 @@ abstract class DbObject * * @param string $property Property * - * @throws IE - * * @return mixed */ public function get($property) @@ -255,7 +255,10 @@ abstract class DbObject protected function assertPropertyExists($key) { if (! array_key_exists($key, $this->properties)) { - throw new IE('Trying to get invalid property "%s"', $key); + throw new InvalidArgumentException(sprintf( + 'Trying to get invalid property "%s"', + $key + )); } return $this; @@ -285,8 +288,6 @@ abstract class DbObject * @param string $key * @param mixed $value * - * @throws IE - * * @return self */ public function set($key, $value) @@ -298,7 +299,11 @@ abstract class DbObject $func = 'validate' . ucfirst($key); if (method_exists($this, $func) && $this->$func($value) !== true) { - throw new IE('Got invalid value "%s" for "%s"', $value, $key); + throw new InvalidArgumentException(sprintf( + 'Got invalid value "%s" for "%s"', + $value, + $key + )); } $func = 'munge' . ucfirst($key); if (method_exists($this, $func)) { @@ -315,7 +320,10 @@ abstract class DbObject } if (! $this->hasProperty($key)) { - throw new IE('Trying to set invalid key %s', $key); + throw new InvalidArgumentException(sprintf( + 'Trying to set invalid key %s', + $key + )); } if ((is_numeric($value) || is_string($value)) @@ -325,7 +333,7 @@ abstract class DbObject } if ($key === $this->getAutoincKeyName() && $this->hasBeenLoadedFromDb()) { - throw new IE('Changing autoincremental key is not allowed'); + throw new InvalidArgumentException('Changing autoincremental key is not allowed'); } return $this->reallySet($key, $value); @@ -383,13 +391,12 @@ abstract class DbObject * Magic unsetter * * @param string $key - * @throws IE * @return void */ public function __unset($key) { if (! array_key_exists($key, $this->properties)) { - throw new IE('Trying to unset invalid key'); + throw new InvalidArgumentException('Trying to unset invalid key'); } $this->properties[$key] = $this->defaultProperties[$key]; } @@ -398,13 +405,15 @@ abstract class DbObject * Runs set() for every key/value pair of the given Array * * @param array $props Array of properties - * @throws IE * @return self */ public function setProperties($props) { if (! is_array($props)) { - throw new IE('Array required, got %s', gettype($props)); + throw new InvalidArgumentException(sprintf( + 'Array required, got %s', + gettype($props) + )); } foreach ($props as $key => $value) { $this->set($key, $value); @@ -638,11 +647,11 @@ abstract class DbObject { foreach ($properties as $key => $val) { if (! array_key_exists($key, $this->properties)) { - throw new IE( + throw new LogicException(sprintf( 'Trying to set invalid %s key "%s". DB schema change?', $this->table, $key - ); + )); } if ($val === null) { $this->properties[$key] = null; @@ -695,6 +704,7 @@ abstract class DbObject * Ändert den entsprechenden Datensatz in der Datenbank * * @return int Anzahl der geänderten Zeilen + * @throws \Zend_Db_Adapter_Exception */ protected function updateDb() { @@ -716,6 +726,7 @@ abstract class DbObject * Fügt der Datenbank-Tabelle einen entsprechenden Datensatz hinzu * * @return int Anzahl der betroffenen Zeilen + * @throws \Zend_Db_Adapter_Exception */ protected function insertIntoDb() { @@ -740,7 +751,7 @@ abstract class DbObject * * @param DbConnection $db * @return bool Whether storing succeeded - * @throws IE + * @throws DuplicateKeyException */ public function store(DbConnection $db = null) { @@ -749,7 +760,11 @@ abstract class DbObject } if ($this->validate() !== true) { - throw new IE('%s[%s] validation failed', $this->table, $this->getLogId()); + throw new InvalidArgumentException(sprintf( + '%s[%s] validation failed', + $this->table, + $this->getLogId() + )); } if ($this->hasBeenLoadedFromDb() && ! $this->hasBeenModified()) { @@ -767,11 +782,11 @@ abstract class DbObject $result = true; $this->onUpdate(); } else { - throw new IE( + throw new RuntimeException(sprintf( 'FAILED storing %s "%s"', $table, $this->getLogId() - ); + )); } } else { if ($id && $this->existsInDb()) { @@ -797,25 +812,21 @@ abstract class DbObject $this->onInsert(); $result = true; } else { - throw new IE( + throw new RuntimeException(sprintf( 'FAILED to store new %s "%s"', $table, $this->getLogId() - ); + )); } } - } catch (Exception $e) { - if ($e instanceof IE) { - throw $e; - } - - throw new IE( + } catch (Zend_Db_Exception $e) { + throw new RuntimeException(sprintf( 'Storing %s[%s] failed: %s {%s}', $this->table, $this->getLogId(), $e->getMessage(), var_export($this->getProperties(), 1) // TODO: Remove properties - ); + )); } $this->modifiedProperties = array(); @@ -823,6 +834,7 @@ abstract class DbObject $this->loadedProperties = $this->properties; $this->onStore(); $this->loadedFromDb = true; + return $result; } @@ -842,17 +854,17 @@ abstract class DbObject /** * @param string $key * @return self - * @throws IE + * @throws InvalidArgumentException */ protected function setKey($key) { $keyname = $this->getKeyName(); if (is_array($keyname)) { if (! is_array($key)) { - throw new IE( + throw new InvalidArgumentException(sprintf( '%s has a multicolumn key, array required', $this->table - ); + )); } foreach ($keyname as $k) { if (! array_key_exists($k, $key)) { @@ -943,27 +955,27 @@ abstract class DbObject $table = $this->table; if (! $this->hasBeenLoadedFromDb()) { - throw new IE( + throw new LogicException(sprintf( 'Cannot delete %s "%s", it has not been loaded from Db', $table, $this->getLogId() - ); + )); } if (! $this->existsInDb()) { - throw new IE( + throw new InvalidArgumentException(sprintf( 'Cannot delete %s "%s", it does not exist', $table, $this->getLogId() - ); + )); } $this->beforeDelete(); if (! $this->deleteFromDb()) { - throw new IE( + throw new RuntimeException(sprintf( 'Deleting %s (%s) FAILED', $table, $this->getLogId() - ); + )); } // $this->log(sprintf('%s "%s" has been DELETED', $table, this->getLogId())); $this->onDelete(); @@ -988,7 +1000,6 @@ abstract class DbObject * @param DbConnection|null $connection * * @return static - * @throws IE */ public static function create($properties = array(), DbConnection $connection = null) { @@ -1075,7 +1086,6 @@ abstract class DbObject * @param $id * @param DbConnection $connection * @return static - * @throws IE * @throws NotFoundError */ public static function loadWithAutoIncId($id, DbConnection $connection) @@ -1105,7 +1115,6 @@ abstract class DbObject * @param $id * @param DbConnection $connection * @return static - * @throws IE * @throws NotFoundError */ public static function load($id, DbConnection $connection) @@ -1127,7 +1136,6 @@ abstract class DbObject * @param string|null $keyColumn * * @return static[] - * @throws IE */ public static function loadAll(DbConnection $connection, $query = null, $keyColumn = null) { @@ -1161,7 +1169,6 @@ abstract class DbObject * @param bool $force * * @return static[] - * @throws IE */ public static function prefetchAll(DbConnection $connection, $force = false) { diff --git a/library/Director/Db/Cache/PrefetchCache.php b/library/Director/Db/Cache/PrefetchCache.php index 447098dd..905441dd 100644 --- a/library/Director/Db/Cache/PrefetchCache.php +++ b/library/Director/Db/Cache/PrefetchCache.php @@ -2,12 +2,12 @@ namespace Icinga\Module\Director\Db\Cache; -use Icinga\Exception\ProgrammingError; use Icinga\Module\Director\CustomVariable\CustomVariable; use Icinga\Module\Director\Db; use Icinga\Module\Director\Objects\IcingaObject; use Icinga\Module\Director\Resolver\HostServiceBlacklist; use Icinga\Module\Director\Resolver\TemplateTree; +use LogicException; /** * Central prefetch cache @@ -45,14 +45,14 @@ class PrefetchCache } /** - * @throws ProgrammingError + * @throws LogicException * * @return self */ public static function instance() { if (static::$instance === null) { - throw new ProgrammingError('Prefetch cache has not been loaded'); + throw new LogicException('Prefetch cache has not been loaded'); } return static::$instance; diff --git a/library/Director/IcingaConfig/IcingaConfig.php b/library/Director/IcingaConfig/IcingaConfig.php index c792bcdb..cd7a088f 100644 --- a/library/Director/IcingaConfig/IcingaConfig.php +++ b/library/Director/IcingaConfig/IcingaConfig.php @@ -5,10 +5,7 @@ namespace Icinga\Module\Director\IcingaConfig; use Icinga\Application\Benchmark; use Icinga\Application\Hook; use Icinga\Application\Icinga; -use Icinga\Exception\ConfigurationError; -use Icinga\Exception\IcingaException; use Icinga\Exception\NotFoundError; -use Icinga\Exception\ProgrammingError; use Icinga\Module\Director\Application\MemoryLimit; use Icinga\Module\Director\Db\Cache\PrefetchCache; use Icinga\Module\Director\Db; @@ -17,6 +14,9 @@ use Icinga\Module\Director\Objects\IcingaObject; use Icinga\Module\Director\Util; use Icinga\Module\Director\Objects\IcingaHost; use Icinga\Module\Director\Objects\IcingaZone; +use InvalidArgumentException; +use LogicException; +use RuntimeException; class IcingaConfig { @@ -28,9 +28,7 @@ class IcingaConfig protected $lastActivityChecksum; - /** - * @var \Zend_Db_Adapter_Abstract - */ + /** @var \Zend_Db_Adapter_Abstract */ protected $db; protected $connection; @@ -83,17 +81,17 @@ class IcingaConfig if ($this->isLegacy()) { return $this->deploymentModeV1; } else { - throw new ProgrammingError('There is no deployment mode for Icinga 2 config format!'); + throw new LogicException('There is no deployment mode for Icinga 2 config format!'); } } public function setConfigFormat($format) { if (! in_array($format, array('v1', 'v2'))) { - throw new ConfigurationError( + throw new InvalidArgumentException(sprintf( 'Only Icinga v1 and v2 config format is supported, got "%s"', $format - ); + )); } $this->configFormat = $format; @@ -438,8 +436,6 @@ class IcingaConfig } /** - * @throws IcingaException - * * @return self */ protected function generateFromDb() @@ -453,7 +449,7 @@ class IcingaConfig ini_set('zend.enable_gc', 0); if (! $this->connection->isPgsql() && $this->db->quote("1\0") !== '\'1\\0\'') { - throw new IcingaException( + throw new RuntimeException( 'Refusing to render the configuration, your DB layer corrupts binary data.' . ' You might be affected by Zend Framework bug #655' ); @@ -754,10 +750,10 @@ apply Service for (title => params in host.vars["%s"]) { foreach (Hook::all('Director\\ShipConfigFiles') as $hook) { foreach ($hook->fetchFiles() as $filename => $file) { if (array_key_exists($filename, $this->files)) { - throw new ProgrammingError( + throw new LogicException(sprintf( 'Cannot ship one file twice: %s', $filename - ); + )); } if ($file instanceof IcingaConfigFile) { $this->files[$filename] = $file; diff --git a/library/Director/Objects/IcingaObject.php b/library/Director/Objects/IcingaObject.php index c019af6d..6045e219 100644 --- a/library/Director/Objects/IcingaObject.php +++ b/library/Director/Objects/IcingaObject.php @@ -19,6 +19,8 @@ use Icinga\Module\Director\IcingaConfig\IcingaConfigRenderer; use Icinga\Module\Director\IcingaConfig\IcingaConfigHelper as c; use Icinga\Module\Director\IcingaConfig\IcingaLegacyConfigHelper as c1; use Icinga\Module\Director\Repository\IcingaTemplateRepository; +use InvalidArgumentException; +use LogicException; abstract class IcingaObject extends DbObject implements IcingaConfigRenderer { @@ -448,7 +450,7 @@ abstract class IcingaObject extends DbObject implements IcingaConfigRenderer * * @param Filter|string $filter * - * @throws ProgrammingError + * @throws LogicException * * @return self */ @@ -461,11 +463,11 @@ abstract class IcingaObject extends DbObject implements IcingaConfigRenderer $type = get_class($this); } - throw new ProgrammingError( + throw new LogicException(sprintf( 'I can only assign for applied objects or objects with native' . ' support for assigments, got %s', $type - ); + )); } // @codingStandardsIgnoreEnd @@ -739,10 +741,10 @@ abstract class IcingaObject extends DbObject implements IcingaConfigRenderer } elseif ($value === '' || $value === null) { return null; } else { - throw new ProgrammingError( + throw new InvalidArgumentException(sprintf( 'Got invalid boolean: %s', var_export($value, 1) - ); + )); } } @@ -1231,10 +1233,10 @@ abstract class IcingaObject extends DbObject implements IcingaConfigRenderer protected function assertCustomVarsSupport() { if (! $this->supportsCustomVars()) { - throw new ProgrammingError( + throw new LogicException(sprintf( 'Objects of type "%s" have no custom vars', $this->getType() - ); + )); } return $this; @@ -1243,10 +1245,10 @@ abstract class IcingaObject extends DbObject implements IcingaConfigRenderer protected function assertGroupsSupport() { if (! $this->supportsGroups()) { - throw new ProgrammingError( + throw new LogicException(sprintf( 'Objects of type "%s" have no groups', $this->getType() - ); + )); } return $this; @@ -1255,10 +1257,10 @@ abstract class IcingaObject extends DbObject implements IcingaConfigRenderer protected function assertRangesSupport() { if (! $this->supportsRanges()) { - throw new ProgrammingError( + throw new LogicException(sprintf( 'Objects of type "%s" have no ranges', $this->getType() - ); + )); } return $this; @@ -1267,10 +1269,10 @@ abstract class IcingaObject extends DbObject implements IcingaConfigRenderer protected function assertImportsSupport() { if (! $this->supportsImports()) { - throw new ProgrammingError( + throw new LogicException(sprintf( 'Objects of type "%s" have no imports', $this->getType() - ); + )); } return $this; @@ -1305,7 +1307,6 @@ abstract class IcingaObject extends DbObject implements IcingaConfigRenderer /** * @return bool - * @throws ProgrammingError */ public function hasInitializedVars() { @@ -1580,7 +1581,10 @@ abstract class IcingaObject extends DbObject implements IcingaConfigRenderer } elseif ($deploymentMode === 'masterless') { // no additional config } else { - throw new ProgrammingError('Unsupported deployment mode: %s' .$deploymentMode); + throw new LogicException(sprintf( + 'Unsupported deployment mode: %s', + $deploymentMode + )); } $config->configFile( @@ -2336,10 +2340,10 @@ abstract class IcingaObject extends DbObject implements IcingaConfigRenderer return $this->get('object_name'); } else { // TODO: replace with an exception once finished - throw new ProgrammingError( + throw new LogicException(sprintf( 'Trying to access "object_name" for an instance of "%s"', get_class($this) - ); + )); } } @@ -2443,7 +2447,6 @@ abstract class IcingaObject extends DbObject implements IcingaConfigRenderer * @param $type * @param Db $db * @return IcingaObject[] - * @throws ProgrammingError */ public static function loadAllExternalObjectsByType($type, Db $db) { @@ -2452,10 +2455,10 @@ abstract class IcingaObject extends DbObject implements IcingaConfigRenderer $dummy = $class::create(); if (is_array($dummy->getKeyName())) { - throw new ProgrammingError( + throw new LogicException(sprintf( 'There is no support for loading external objects of type "%s"', $type - ); + )); } else { $query = $db->getDbAdapter() ->select() @@ -2593,7 +2596,10 @@ abstract class IcingaObject extends DbObject implements IcingaConfigRenderer $k = $relKey; } else { - throw new ProgrammingError('No such relation: %s', $relKey); + throw new LogicException(sprintf( + 'No such relation: %s', + $relKey + )); } } } diff --git a/library/Director/Objects/IcingaObjectLegacyAssignments.php b/library/Director/Objects/IcingaObjectLegacyAssignments.php index f30b06bb..6ab75c87 100644 --- a/library/Director/Objects/IcingaObjectLegacyAssignments.php +++ b/library/Director/Objects/IcingaObjectLegacyAssignments.php @@ -2,7 +2,7 @@ namespace Icinga\Module\Director\Objects; -use Icinga\Exception\ProgrammingError; +use LogicException; /** * This class is required for historical reasons @@ -15,10 +15,10 @@ class IcingaObjectLegacyAssignments public static function applyToObject(IcingaObject $object, $values) { if (! $object->supportsAssignments()) { - throw new ProgrammingError( + throw new LogicException(sprintf( 'I can only assign for applied objects, got %s', $object->object_type - ); + )); } if ($values === null) { @@ -70,7 +70,7 @@ class IcingaObjectLegacyAssignments protected static function throwCompatError() { - throw new ProgrammingError( + throw new LogicException( 'You ran into an unexpected compatibility issue. Please report' . ' this with details helping us to reproduce this to the' . ' Icinga project' diff --git a/library/Director/Objects/IcingaServiceSet.php b/library/Director/Objects/IcingaServiceSet.php index ad842de1..680a6a87 100644 --- a/library/Director/Objects/IcingaServiceSet.php +++ b/library/Director/Objects/IcingaServiceSet.php @@ -3,10 +3,9 @@ namespace Icinga\Module\Director\Objects; use Icinga\Data\Filter\Filter; -use Icinga\Exception\IcingaException; -use Icinga\Exception\ProgrammingError; use Icinga\Module\Director\Exception\DuplicateKeyException; use Icinga\Module\Director\IcingaConfig\IcingaConfig; +use InvalidArgumentException; class IcingaServiceSet extends IcingaObject { @@ -55,7 +54,10 @@ class IcingaServiceSet extends IcingaObject $this->set('object_name', $keyComponents[0]); $this->set('object_type', 'template'); } else { - throw new IcingaException('Can not parse key: %s', $key); + throw new InvalidArgumentException(sprintf( + 'Can not parse key: %s', + $key + )); } } else { return parent::setKey($key); @@ -287,7 +289,7 @@ class IcingaServiceSet extends IcingaObject $name = $this->getObjectName(); if ($this->isObject() && $this->get('host_id') === null) { - throw new ProgrammingError( + throw new InvalidArgumentException( 'A Service Set cannot be an object with no related host' ); } diff --git a/test/php/library/Director/Objects/IcingaHostTest.php b/test/php/library/Director/Objects/IcingaHostTest.php index a7e2ab88..ef3cbb80 100644 --- a/test/php/library/Director/Objects/IcingaHostTest.php +++ b/test/php/library/Director/Objects/IcingaHostTest.php @@ -425,7 +425,7 @@ class IcingaHostTest extends BaseTestCase $a->store(); try { $b->store(); - } catch (IcingaException $e) { + } catch (\RuntimeException $e) { $msg = $e->getMessage(); $matchMysql = strpos( $msg, diff --git a/test/php/library/Director/Objects/IcingaServiceSetTest.php b/test/php/library/Director/Objects/IcingaServiceSetTest.php index d4b60de0..ad7c1357 100644 --- a/test/php/library/Director/Objects/IcingaServiceSetTest.php +++ b/test/php/library/Director/Objects/IcingaServiceSetTest.php @@ -107,7 +107,7 @@ class IcingaServiceSetTest extends IcingaObjectTestCase } /** - * @expectedException \Icinga\Exception\IcingaException + * @expectedException \RuntimeException */ public function testCreatingSetWithoutType() { @@ -118,9 +118,9 @@ class IcingaServiceSetTest extends IcingaObjectTestCase } /** - * @expectedException \Icinga\Exception\ProgrammingError + * @expectedException \InvalidArgumentException */ - public function testCreatingHostSetWithoutHost() + public function testCreatingServiceSetWithoutHost() { $set = IcingaServiceSet::create(array( 'object_name' => '___TEST__set_BAD2', diff --git a/test/php/library/Director/Objects/IcingaServiceTest.php b/test/php/library/Director/Objects/IcingaServiceTest.php index 7813487b..9154fd28 100644 --- a/test/php/library/Director/Objects/IcingaServiceTest.php +++ b/test/php/library/Director/Objects/IcingaServiceTest.php @@ -51,7 +51,7 @@ class IcingaServiceTest extends BaseTestCase } /** - * @expectedException Icinga\Exception\ProgrammingError + * @expectedException \LogicException */ public function testRefusesAssignRulesWhenNotBeingAnApply() {