From eeb56ff33c86fc277afde93b8da82f4287f4cd78 Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Tue, 17 Dec 2024 10:40:35 +0100 Subject: [PATCH 1/5] db: Add model for the schema table --- library/Icingadb/Model/Schema.php | 50 +++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) create mode 100644 library/Icingadb/Model/Schema.php diff --git a/library/Icingadb/Model/Schema.php b/library/Icingadb/Model/Schema.php new file mode 100644 index 000000000..4d7618ffc --- /dev/null +++ b/library/Icingadb/Model/Schema.php @@ -0,0 +1,50 @@ +add(new Binary(['id'])); + $behaviors->add(new MillisecondTimestamp(['timestamp'])); + } +} From b10b534ee67ab5337cf5e1679977eb1857acc80d Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Tue, 17 Dec 2024 10:41:11 +0100 Subject: [PATCH 2/5] Introduce singleton `Icinga\Module\Icingadb\Common\Backend` --- library/Icingadb/Common/Backend.php | 168 ++++++++++++++++++++++++++++ 1 file changed, 168 insertions(+) create mode 100644 library/Icingadb/Common/Backend.php diff --git a/library/Icingadb/Common/Backend.php b/library/Icingadb/Common/Backend.php new file mode 100644 index 000000000..42aca17ba --- /dev/null +++ b/library/Icingadb/Common/Backend.php @@ -0,0 +1,168 @@ +get('icingadb', 'resource') + )); + + $config->options = [PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_OBJ]; + if ($config->db === 'mysql') { + $config->options[PDO::MYSQL_ATTR_INIT_COMMAND] = "SET SESSION SQL_MODE='STRICT_TRANS_TABLES" + . ",NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_ENGINE_SUBSTITUTION'"; + } + + self::$db = new Connection($config); + + $adapter = self::$db->getAdapter(); + if ($adapter instanceof Pgsql) { + $quoted = $adapter->quoteIdentifier('user'); + self::$db->getQueryBuilder() + ->on(QueryBuilder::ON_SELECT_ASSEMBLED, function (&$sql) use ($quoted) { + // user is a reserved key word in PostgreSQL, so we need to quote it. + // TODO(lippserd): This is pretty hacky, + // reconsider how to properly implement identifier quoting. + $sql = str_replace(' user ', sprintf(' %s ', $quoted), $sql); + $sql = str_replace(' user.', sprintf(' %s.', $quoted), $sql); + $sql = str_replace('(user.', sprintf('(%s.', $quoted), $sql); + }) + ->on(QueryBuilder::ON_ASSEMBLE_SELECT, function (Select $select) { + // For SELECT DISTINCT, all ORDER BY columns must appear in SELECT list. + if (! $select->getDistinct() || ! $select->hasOrderBy()) { + return; + } + + $candidates = []; + foreach ($select->getOrderBy() as list($columnOrAlias, $_)) { + if ($columnOrAlias instanceof Expression) { + // Expressions can be and include anything, + // also columns that aren't already part of the SELECT list, + // so we're not trying to guess anything here. + // Such expressions must be in the SELECT list if necessary and + // referenced manually with an alias in ORDER BY. + continue; + } + + $candidates[$columnOrAlias] = true; + } + + foreach ($select->getColumns() as $alias => $column) { + if (is_int($alias)) { + if ($column instanceof Expression) { + // This is the complement to the above consideration. + // If it is an unaliased expression, ignore it. + continue; + } + } else { + unset($candidates[$alias]); + } + + if (! $column instanceof Expression) { + unset($candidates[$column]); + } + } + + if (! empty($candidates)) { + $select->columns(array_keys($candidates)); + } + }); + } + } + + return self::$db; + } + + /** + * Get the schema version of the Icinga DB + * + * @return int + */ + public static function getDbSchemaVersion(): int + { + if (self::$schemaVersion === null) { + self::$schemaVersion = Schema::on(self::getDb()) + ->columns('version') + ->first() + ->version ?? 0; + } + + return self::$schemaVersion; + } + + /** + * Set the connection to the Icinga Redis + * + * Usually not required, as the connection is created on demand. Useful for testing. + * + * @param IcingaRedis $redis + * + * @return void + */ + public static function setRedis(IcingaRedis $redis): void + { + self::$redis = $redis; + } + + /** + * Get the connection to the Icinga Redis + * + * @return IcingaRedis + */ + public static function getRedis(): IcingaRedis + { + if (self::$redis === null) { + self::$redis = new IcingaRedis(); + } + + return self::$redis; + } +} From edbafbd14429ae6adba10942693946a4016acc2c Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Tue, 17 Dec 2024 10:42:05 +0100 Subject: [PATCH 3/5] Database: Use the new singleton instead --- library/Icingadb/Common/Database.php | 84 +--------------------------- 1 file changed, 1 insertion(+), 83 deletions(-) diff --git a/library/Icingadb/Common/Database.php b/library/Icingadb/Common/Database.php index 8fa87cc2f..782b55a60 100644 --- a/library/Icingadb/Common/Database.php +++ b/library/Icingadb/Common/Database.php @@ -4,99 +4,17 @@ namespace Icinga\Module\Icingadb\Common; -use Icinga\Application\Config as AppConfig; -use Icinga\Data\ResourceFactory; -use Icinga\Exception\ConfigurationError; -use ipl\Sql\Adapter\Pgsql; -use ipl\Sql\Config as SqlConfig; use ipl\Sql\Connection; -use ipl\Sql\Expression; -use ipl\Sql\QueryBuilder; -use ipl\Sql\Select; -use PDO; trait Database { - /** @var Connection Connection to the Icinga database */ - private $db; - /** * Get the connection to the Icinga database * * @return Connection - * - * @throws ConfigurationError If the related resource configuration does not exist */ public function getDb(): Connection { - if ($this->db === null) { - $config = new SqlConfig(ResourceFactory::getResourceConfig( - AppConfig::module('icingadb')->get('icingadb', 'resource') - )); - - $config->options = [PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_OBJ]; - if ($config->db === 'mysql') { - $config->options[PDO::MYSQL_ATTR_INIT_COMMAND] = "SET SESSION SQL_MODE='STRICT_TRANS_TABLES" - . ",NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_ENGINE_SUBSTITUTION'"; - } - - $this->db = new Connection($config); - - $adapter = $this->db->getAdapter(); - if ($adapter instanceof Pgsql) { - $quoted = $adapter->quoteIdentifier('user'); - $this->db->getQueryBuilder() - ->on(QueryBuilder::ON_SELECT_ASSEMBLED, function (&$sql) use ($quoted) { - // user is a reserved key word in PostgreSQL, so we need to quote it. - // TODO(lippserd): This is pretty hacky, - // reconsider how to properly implement identifier quoting. - $sql = str_replace(' user ', sprintf(' %s ', $quoted), $sql); - $sql = str_replace(' user.', sprintf(' %s.', $quoted), $sql); - $sql = str_replace('(user.', sprintf('(%s.', $quoted), $sql); - }) - ->on(QueryBuilder::ON_ASSEMBLE_SELECT, function (Select $select) { - // For SELECT DISTINCT, all ORDER BY columns must appear in SELECT list. - if (! $select->getDistinct() || ! $select->hasOrderBy()) { - return; - } - - $candidates = []; - foreach ($select->getOrderBy() as list($columnOrAlias, $_)) { - if ($columnOrAlias instanceof Expression) { - // Expressions can be and include anything, - // also columns that aren't already part of the SELECT list, - // so we're not trying to guess anything here. - // Such expressions must be in the SELECT list if necessary and - // referenced manually with an alias in ORDER BY. - continue; - } - - $candidates[$columnOrAlias] = true; - } - - foreach ($select->getColumns() as $alias => $column) { - if (is_int($alias)) { - if ($column instanceof Expression) { - // This is the complement to the above consideration. - // If it is an unaliased expression, ignore it. - continue; - } - } else { - unset($candidates[$alias]); - } - - if (! $column instanceof Expression) { - unset($candidates[$column]); - } - } - - if (! empty($candidates)) { - $select->columns(array_keys($candidates)); - } - }); - } - } - - return $this->db; + return Backend::getDb(); } } From 1a23a6b3cc80e76f7103e276d2a55e506f8160dc Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Tue, 17 Dec 2024 10:42:57 +0100 Subject: [PATCH 4/5] IcingaRedis: Use the new singleton instead --- doc/05-Upgrading.md | 3 ++ library/Icingadb/Common/IcingaRedis.php | 30 +++++++------------ .../Common/ObjectInspectionDetail.php | 2 +- .../Icingadb/Redis/VolatileStateResults.php | 3 +- 4 files changed, 17 insertions(+), 21 deletions(-) diff --git a/doc/05-Upgrading.md b/doc/05-Upgrading.md index 437e74308..2a6efae94 100644 --- a/doc/05-Upgrading.md +++ b/doc/05-Upgrading.md @@ -12,6 +12,9 @@ The following classes have been deprecated and will be removed in a future relea * `\Icinga\Module\Icingadb\Command\Object\ScheduleHostDowntimeCommand` * `\Icinga\Module\Icingadb\Command\Object\ScheduleServiceDowntimeCommand` +The following methods have been deprecated and will be removed in a future release: +* `\Icinga\Module\Icingadb\Common\IcingaRedis::instance()`: Use `\Icinga\Module\Icingadb\Common\Backend::getRedis()` instead. + ## Upgrading to Icinga DB Web v1.1 **Breaking Changes** diff --git a/library/Icingadb/Common/IcingaRedis.php b/library/Icingadb/Common/IcingaRedis.php index a22a0f03b..0926814fd 100644 --- a/library/Icingadb/Common/IcingaRedis.php +++ b/library/Icingadb/Common/IcingaRedis.php @@ -12,9 +12,6 @@ class IcingaRedis { - /** @var static The singleton */ - protected static $instance; - /** @var Redis Connection to the Icinga Redis */ private $redis; @@ -24,15 +21,12 @@ class IcingaRedis /** * Get the singleton * + * @deprecated Use {@see Backend::getRedis()} instead * @return static */ public static function instance(): self { - if (self::$instance === null) { - self::$instance = new static(); - } - - return self::$instance; + return Backend::getRedis(); } /** @@ -40,19 +34,17 @@ public static function instance(): self * * @return bool */ - public static function isUnavailable(): bool + public function isUnavailable(): bool { - $self = self::instance(); - - if (! $self->redisUnavailable && $self->redis === null) { + if (! $this->redisUnavailable && $this->redis === null) { try { - $self->getConnection(); + $this->getConnection(); } catch (Exception $_) { // getConnection already logs the error } } - return $self->redisUnavailable; + return $this->redisUnavailable; } /** @@ -126,7 +118,7 @@ public function getConnection(): Redis */ public static function fetchHostState(array $ids, array $columns): Generator { - return self::fetchState('icinga:host:state', $ids, $columns); + return Backend::getRedis()->fetchState('icinga:host:state', $ids, $columns); } /** @@ -139,7 +131,7 @@ public static function fetchHostState(array $ids, array $columns): Generator */ public static function fetchServiceState(array $ids, array $columns): Generator { - return self::fetchState('icinga:service:state', $ids, $columns); + return Backend::getRedis()->fetchState('icinga:service:state', $ids, $columns); } /** @@ -151,10 +143,10 @@ public static function fetchServiceState(array $ids, array $columns): Generator * * @return Generator */ - protected static function fetchState(string $key, array $ids, array $columns): Generator + protected function fetchState(string $key, array $ids, array $columns): Generator { try { - $results = self::instance()->getConnection()->hmget($key, $ids); + $results = $this->getConnection()->hmget($key, $ids); } catch (Exception $_) { // The error has already been logged elsewhere return; @@ -192,7 +184,7 @@ protected static function fetchState(string $key, array $ids, array $columns): G public static function getLastIcingaHeartbeat(Redis $redis = null) { if ($redis === null) { - $redis = self::instance()->getConnection(); + $redis = Backend::getRedis()->getConnection(); } // Predis doesn't support streams (yet). diff --git a/library/Icingadb/Common/ObjectInspectionDetail.php b/library/Icingadb/Common/ObjectInspectionDetail.php index b30797b4e..db408ae92 100644 --- a/library/Icingadb/Common/ObjectInspectionDetail.php +++ b/library/Icingadb/Common/ObjectInspectionDetail.php @@ -120,7 +120,7 @@ protected function createRedisInfo(): array $title = new HtmlElement('h2', null, Text::create(t('Volatile State Details'))); try { - $json = IcingaRedis::instance()->getConnection() + $json = Backend::getRedis()->getConnection() ->hGet("icinga:{$this->object->getTableName()}:state", bin2hex($this->object->id)); } catch (Exception $e) { return [$title, sprintf('Failed to load redis data: %s', $e->getMessage())]; diff --git a/library/Icingadb/Redis/VolatileStateResults.php b/library/Icingadb/Redis/VolatileStateResults.php index 43446d681..f3459e2b9 100644 --- a/library/Icingadb/Redis/VolatileStateResults.php +++ b/library/Icingadb/Redis/VolatileStateResults.php @@ -6,6 +6,7 @@ use Icinga\Application\Benchmark; use Icinga\Module\Icingadb\Common\Auth; +use Icinga\Module\Icingadb\Common\Backend; use Icinga\Module\Icingadb\Common\IcingaRedis; use Icinga\Module\Icingadb\Model\DependencyNode; use Icinga\Module\Icingadb\Model\Host; @@ -39,7 +40,7 @@ public static function fromQuery(Query $query) { $self = parent::fromQuery($query); $self->resolver = $query->getResolver(); - $self->redisUnavailable = IcingaRedis::isUnavailable(); + $self->redisUnavailable = Backend::getRedis()->isUnavailable(); return $self; } From c57298e7769af7243839d127859aee83ac5a751a Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Tue, 17 Dec 2024 15:28:54 +0100 Subject: [PATCH 5/5] test: Make sure a backend exists --- .../library/Icingadb/Model/Behavior/FlattenedObjectVarsTest.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/php/library/Icingadb/Model/Behavior/FlattenedObjectVarsTest.php b/test/php/library/Icingadb/Model/Behavior/FlattenedObjectVarsTest.php index 7cfd97ab8..e5709ef76 100644 --- a/test/php/library/Icingadb/Model/Behavior/FlattenedObjectVarsTest.php +++ b/test/php/library/Icingadb/Model/Behavior/FlattenedObjectVarsTest.php @@ -4,6 +4,7 @@ namespace Tests\Icinga\Modules\Icingadb\Model\Behavior; +use Icinga\Module\Icingadb\Common\Backend; use Icinga\Module\Icingadb\Model\Host; use ipl\Sql\Connection; use ipl\Sql\Test\SqlAssertions; @@ -101,6 +102,7 @@ class FlattenedObjectVarsTest extends TestCase public function setUp(): void { $this->connection = new TestConnection(); + Backend::setDb($this->connection); $this->setUpSqlAssertions(); }