diff --git a/doc/02-Installation.md.d/From-Source.md b/doc/02-Installation.md.d/From-Source.md index b430d16a0..f6e4c121f 100644 --- a/doc/02-Installation.md.d/From-Source.md +++ b/doc/02-Installation.md.d/From-Source.md @@ -11,7 +11,7 @@ Make sure you use `icingadb` as the module name. The following requirements must * The following PHP modules must be installed: `cURL`, `dom`, `json`, `libxml` * [Icinga DB](https://github.com/Icinga/icingadb) * [Icinga Web 2](https://github.com/Icinga/icingaweb2) (≥2.9) -* [Icinga PHP Library (ipl)](https://github.com/Icinga/icinga-php-library) (≥0.13) +* [Icinga PHP Library (ipl)](https://github.com/Icinga/icinga-php-library) (≥0.13.2) * [Icinga PHP Thirdparty](https://github.com/Icinga/icinga-php-thirdparty) (≥0.12) diff --git a/library/Icingadb/Model/Behavior/FlattenedObjectVars.php b/library/Icingadb/Model/Behavior/FlattenedObjectVars.php index b1c308a35..3c891e6d0 100644 --- a/library/Icingadb/Model/Behavior/FlattenedObjectVars.php +++ b/library/Icingadb/Model/Behavior/FlattenedObjectVars.php @@ -11,6 +11,7 @@ use ipl\Orm\Contract\QueryAwareBehavior; use ipl\Orm\Contract\RewriteColumnBehavior; use ipl\Orm\Query; +use ipl\Stdlib\Data; use ipl\Stdlib\Filter; class FlattenedObjectVars implements RewriteColumnBehavior, QueryAwareBehavior @@ -32,11 +33,39 @@ public function rewriteCondition(Filter\Condition $condition, $relation = null) $column = $condition->metaData()->get('columnName'); if ($column !== null) { $relation = substr($relation, 0, -5) . 'customvar_flat.'; - $nameFilter = Filter::like($relation . 'flatname', $column); - $class = get_class($condition); - $valueFilter = new $class($relation . 'flatvalue', $condition->getValue()); - - return Filter::all($nameFilter, $valueFilter); + $condition->metaData() + ->set('requiresTransformation', true) + ->set('columnPath', $relation . $column) + ->set('relationPath', substr($relation, 0, -1)); + + // The ORM's FilterProcessor only optimizes filter conditions that are in the same level (chain). + // Previously, this behavior transformed a single condition to an ALL chain and hence the semantics + // of the level changed, since the FilterProcessor interpreted the conditions separately from there on. + // To not change the semantics of the condition it is required to delay the transformation of the condition + // until the subquery is created. Though, since the FilterProcessor only applies behaviors once, this + // hack is required. (The entire filter, metadata and optimization is total garbage.) + $oldMetaData = $condition->metaData(); + $metaDataProperty = (new \ReflectionClass($condition))->getProperty('metaData'); + $metaDataProperty->setAccessible(true); + $metaDataProperty->setValue($condition, new class () extends Data { + public function set($name, $value) + { + if ($name === 'behaviorsApplied') { + return $this; + } + + return parent::set($name, $value); + } + }); + $condition->metaData()->merge($oldMetaData); + + // But to make it even worse: If we do that, (not transforming the condition) the FilterProcessor sees + // multiple conditions as targeting different columns, as it doesn't know that the *columns* are in fact + // custom variables. It then attempts to combine the conditions with an AND, which is not possible, since + // they refer to the same columns (flatname and flatvalue) after being transformed. So we have to make + // the condition refer to a different column, which is totally irrelevant, but since it's always the same + // column, the FilterProcessor won't attempt to combine the conditions. The literal icing on the cake. + $condition->setColumn('always_the_same_but_totally_irrelevant'); } } diff --git a/library/Icingadb/Model/CustomvarFlat.php b/library/Icingadb/Model/CustomvarFlat.php index f9f3c5a8f..e7a04598a 100644 --- a/library/Icingadb/Model/CustomvarFlat.php +++ b/library/Icingadb/Model/CustomvarFlat.php @@ -6,9 +6,12 @@ use ipl\Orm\Behavior\Binary; use ipl\Orm\Behaviors; +use ipl\Orm\Contract\RewriteFilterBehavior; use ipl\Orm\Model; use ipl\Orm\Query; use ipl\Orm\Relations; +use ipl\Stdlib\Filter; +use ipl\Stdlib\Filter\Condition; use Traversable; /** @@ -50,6 +53,20 @@ public function createBehaviors(Behaviors $behaviors) 'customvar_id', 'flatname_checksum' ])); + $behaviors->add(new class implements RewriteFilterBehavior { + public function rewriteCondition(Condition $condition, $relation = null) + { + if ($condition->metaData()->has('requiresTransformation')) { + /** @var string $columnName */ + $columnName = $condition->metaData()->get('columnName'); + $nameFilter = Filter::like($relation . 'flatname', $columnName); + $class = get_class($condition); + $valueFilter = new $class($relation . 'flatvalue', $condition->getValue()); + + return Filter::all($nameFilter, $valueFilter); + } + } + }); } public function createRelations(Relations $relations) diff --git a/module.info b/module.info index c86c74409..994da5d54 100644 --- a/module.info +++ b/module.info @@ -1,6 +1,6 @@ Module: icingadb Version: 1.1.1 Requires: - Libraries: icinga-php-library (>=0.13.0), icinga-php-thirdparty (>=0.12.0) + Libraries: icinga-php-library (>=0.13.2), icinga-php-thirdparty (>=0.12.0) Description: Icinga DB Web UI for Icinga DB – Provides a graphical interface to your Icinga monitoring diff --git a/phpstan-baseline-standard.neon b/phpstan-baseline-standard.neon index 644347155..f91819bc7 100644 --- a/phpstan-baseline-standard.neon +++ b/phpstan-baseline-standard.neon @@ -3055,11 +3055,6 @@ parameters: count: 1 path: library/Icingadb/Model/Behavior/FlattenedObjectVars.php - - - message: "#^Parameter \\#2 \\$value of static method ipl\\\\Stdlib\\\\Filter\\:\\:like\\(\\) expects array\\\\|string, mixed given\\.$#" - count: 1 - path: library/Icingadb/Model/Behavior/FlattenedObjectVars.php - - message: "#^Parameter \\#3 \\$subject of function str_replace expects array\\|string, string\\|null given\\.$#" count: 1 diff --git a/test/php/library/Icingadb/Model/Behavior/FlattenedObjectVarsTest.php b/test/php/library/Icingadb/Model/Behavior/FlattenedObjectVarsTest.php new file mode 100644 index 000000000..66f93bec5 --- /dev/null +++ b/test/php/library/Icingadb/Model/Behavior/FlattenedObjectVarsTest.php @@ -0,0 +1,161 @@ += ?)) OR host.id IS NULL) +ORDER BY host.id +SQL; + + private const DOUBLE_UNEQUAL_RESULT = <<<'SQL' +SELECT host.id +FROM host +WHERE (host.id NOT IN ((SELECT sub_customvar_flat_host.id AS sub_customvar_flat_host_id + FROM customvar_flat sub_customvar_flat + INNER JOIN host_customvar sub_customvar_flat_host_customvar + ON sub_customvar_flat_host_customvar.customvar_id = + sub_customvar_flat.customvar_id + INNER JOIN host sub_customvar_flat_host + ON sub_customvar_flat_host.id = sub_customvar_flat_host_customvar.host_id + WHERE (((sub_customvar_flat.flatname = ?) AND (sub_customvar_flat.flatvalue = ?)) OR + ((sub_customvar_flat.flatname = ?) AND (sub_customvar_flat.flatvalue = ?))) + AND (sub_customvar_flat_host.id IS NOT NULL) + GROUP BY sub_customvar_flat_host.id + HAVING COUNT(DISTINCT sub_customvar_flat.id) >= ?)) OR host.id IS NULL) +ORDER BY host.id +SQL; + + private const EQUAL_UNEQUAL_RESULT = <<<'SQL' +SELECT host.id +FROM host +WHERE ((host.id NOT IN ((SELECT sub_customvar_flat_host.id AS sub_customvar_flat_host_id + FROM customvar_flat sub_customvar_flat + INNER JOIN host_customvar sub_customvar_flat_host_customvar + ON sub_customvar_flat_host_customvar.customvar_id = + sub_customvar_flat.customvar_id + INNER JOIN host sub_customvar_flat_host + ON sub_customvar_flat_host.id = sub_customvar_flat_host_customvar.host_id + WHERE ((sub_customvar_flat.flatname = ?) AND (sub_customvar_flat.flatvalue = ?)) + AND (sub_customvar_flat_host.id IS NOT NULL) + GROUP BY sub_customvar_flat_host.id + HAVING COUNT(DISTINCT sub_customvar_flat.id) >= ?)) OR host.id IS NULL)) + AND (host.id IN ((SELECT sub_customvar_flat_host.id AS sub_customvar_flat_host_id + FROM customvar_flat sub_customvar_flat + INNER JOIN host_customvar sub_customvar_flat_host_customvar + ON sub_customvar_flat_host_customvar.customvar_id = + sub_customvar_flat.customvar_id + INNER JOIN host sub_customvar_flat_host + ON sub_customvar_flat_host.id = sub_customvar_flat_host_customvar.host_id + WHERE (sub_customvar_flat.flatname = ?) + AND (sub_customvar_flat.flatvalue = ?) + GROUP BY sub_customvar_flat_host.id + HAVING COUNT(DISTINCT sub_customvar_flat.id) >= ?))) +ORDER BY host.id +SQL; + + private const DOUBLE_EQUAL_RESULT = <<<'SQL' +SELECT host.id +FROM host +WHERE host.id IN ((SELECT sub_customvar_flat_host.id AS sub_customvar_flat_host_id + FROM customvar_flat sub_customvar_flat + INNER JOIN host_customvar sub_customvar_flat_host_customvar + ON sub_customvar_flat_host_customvar.customvar_id = + sub_customvar_flat.customvar_id + INNER JOIN host sub_customvar_flat_host + ON sub_customvar_flat_host.id = sub_customvar_flat_host_customvar.host_id + WHERE ((sub_customvar_flat.flatname = ?) AND (sub_customvar_flat.flatvalue = ?)) + OR ((sub_customvar_flat.flatname = ?) AND (sub_customvar_flat.flatvalue = ?)) + GROUP BY sub_customvar_flat_host.id + HAVING COUNT(DISTINCT sub_customvar_flat.id) >= ?)) +ORDER BY host.id +SQL; + + /** @var Connection */ + private $connection; + + public function setUp(): void + { + $this->connection = new TestConnection(); + $this->setUpSqlAssertions(); + } + + public function testSingleUnequalCondition() + { + $query = Host::on($this->connection) + ->columns('host.id') + ->orderBy('host.id') + ->filter(Filter::unequal('host.vars.invalid', 'foo')); + + $this->assertSql(self::SINGLE_UNEQUAL_RESULT, $query->assembleSelect(), ['invalid', 'foo', 1]); + } + + public function testDoubleUnequalCondition() + { + $query = Host::on($this->connection) + ->columns('host.id') + ->orderBy('host.id') + ->filter(Filter::unequal('host.vars.invalid', 'foo')) + ->filter(Filter::unequal('host.vars.missing', 'bar')); + + $this->assertSql( + self::DOUBLE_UNEQUAL_RESULT, + $query->assembleSelect(), + ['invalid', 'foo', 'missing', 'bar', 1] + ); + } + + public function testEqualAndUnequalCondition() + { + $query = Host::on($this->connection) + ->columns('host.id') + ->orderBy('host.id') + ->filter(Filter::unequal('host.vars.invalid', 'bar')) + ->filter(Filter::equal('host.vars.env', 'foo')); + + $this->assertSql( + self::EQUAL_UNEQUAL_RESULT, + $query->assembleSelect(), + ['invalid', 'bar', 1, 'env', 'foo', 1] + ); + } + + public function testDoubleEqualCondition() + { + $query = Host::on($this->connection) + ->columns('host.id') + ->orderBy('host.id') + ->filter(Filter::equal('host.vars.env', 'foo')) + ->filter(Filter::equal('host.vars.os', 'bar')); + + $this->assertSql( + self::DOUBLE_EQUAL_RESULT, + $query->assembleSelect(), + ['env', 'foo', 'os', 'bar', 2] + ); + } +}