Transform custom variable filters as late as possible

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)
 
 <!-- {% include "02-Installation.md" %} -->

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');
         }
     }
 

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)

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

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\\>\\|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

test/php/library/Icingadb/Model/Behavior/FlattenedObjectVarsTest.php

@@ -0,0 +1,161 @@
+<?php
+
+/* Icinga DB Web | (c) 2024 Icinga GmbH | GPLv2 */
+
+namespace Tests\Icinga\Modules\Icingadb\Model\Behavior;
+
+use Icinga\Module\Icingadb\Model\Host;
+use ipl\Sql\Connection;
+use ipl\Sql\Test\SqlAssertions;
+use ipl\Sql\Test\TestConnection;
+use ipl\Stdlib\Filter;
+use PHPUnit\Framework\TestCase;
+
+class FlattenedObjectVarsTest extends TestCase
+{
+    use SqlAssertions;
+
+    private const SINGLE_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)
+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]
+        );
+    }
+}