From 83af168936d5709e713ece6cbf723c5f64abf46b Mon Sep 17 00:00:00 2001
From: Johannes Meyer <johannes.meyer@icinga.com>
Date: Mon, 25 Mar 2024 10:29:47 +0100
Subject: [PATCH] FlattenedObjectVars: Force processing to finish early

fixes #1001
---
 .../Model/Behavior/FlattenedObjectVars.php    | 24 ++++++-------------
 .../Behavior/FlattenedObjectVarsTest.php      | 12 +++++-----
 2 files changed, 13 insertions(+), 23 deletions(-)

diff --git a/library/Icingadb/Model/Behavior/FlattenedObjectVars.php b/library/Icingadb/Model/Behavior/FlattenedObjectVars.php
index 3c891e6d0..8b110e072 100644
--- a/library/Icingadb/Model/Behavior/FlattenedObjectVars.php
+++ b/library/Icingadb/Model/Behavior/FlattenedObjectVars.php
@@ -11,7 +11,6 @@
 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
@@ -42,22 +41,11 @@ public function rewriteCondition(Filter\Condition $condition, $relation = null)
             // 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);
+            // until the subquery is created. Though, since this is about custom variables, and such can contain dots,
+            // the FilterProcessor then continues traversing the parts of the column's path, which then would include
+            // the dot-separated parts of the custom variable name. To prevent this, we have to signal that what we
+            // return a replacement here, that should be used as-is and not processed further.
+            $condition->metaData()->set('forceResolved', true);
 
             // 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
@@ -66,6 +54,8 @@ public function set($name, $value)
             // 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');
+
+            return $condition;
         }
     }
 
diff --git a/test/php/library/Icingadb/Model/Behavior/FlattenedObjectVarsTest.php b/test/php/library/Icingadb/Model/Behavior/FlattenedObjectVarsTest.php
index 66f93bec5..7cfd97ab8 100644
--- a/test/php/library/Icingadb/Model/Behavior/FlattenedObjectVarsTest.php
+++ b/test/php/library/Icingadb/Model/Behavior/FlattenedObjectVarsTest.php
@@ -109,9 +109,9 @@ public function testSingleUnequalCondition()
         $query = Host::on($this->connection)
             ->columns('host.id')
             ->orderBy('host.id')
-            ->filter(Filter::unequal('host.vars.invalid', 'foo'));
+            ->filter(Filter::unequal('host.vars.in.valid', 'foo'));
 
-        $this->assertSql(self::SINGLE_UNEQUAL_RESULT, $query->assembleSelect(), ['invalid', 'foo', 1]);
+        $this->assertSql(self::SINGLE_UNEQUAL_RESULT, $query->assembleSelect(), ['in.valid', 'foo', 1]);
     }
 
     public function testDoubleUnequalCondition()
@@ -119,13 +119,13 @@ 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.in.valid', 'foo'))
             ->filter(Filter::unequal('host.vars.missing', 'bar'));
 
         $this->assertSql(
             self::DOUBLE_UNEQUAL_RESULT,
             $query->assembleSelect(),
-            ['invalid', 'foo', 'missing', 'bar', 1]
+            ['in.valid', 'foo', 'missing', 'bar', 1]
         );
     }
 
@@ -134,13 +134,13 @@ public function testEqualAndUnequalCondition()
         $query = Host::on($this->connection)
             ->columns('host.id')
             ->orderBy('host.id')
-            ->filter(Filter::unequal('host.vars.invalid', 'bar'))
+            ->filter(Filter::unequal('host.vars.in.valid', 'bar'))
             ->filter(Filter::equal('host.vars.env', 'foo'));
 
         $this->assertSql(
             self::EQUAL_UNEQUAL_RESULT,
             $query->assembleSelect(),
-            ['invalid', 'bar', 1, 'env', 'foo', 1]
+            ['in.valid', 'bar', 1, 'env', 'foo', 1]
         );
     }