Skip to content

Commit

Permalink
Transform custom variable filters as late as possible
Browse files Browse the repository at this point in the history
fixes #865
  • Loading branch information
nilmerg committed Mar 12, 2024
1 parent 274b903 commit f3a21c0
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 10 deletions.
39 changes: 34 additions & 5 deletions library/Icingadb/Model/Behavior/FlattenedObjectVars.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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();
$reflection = new \ReflectionClass($condition);
$reflection->getProperty('metaData')->setAccessible(true);
$reflection->getProperty('metaData')->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');
}
}

Expand Down
17 changes: 17 additions & 0 deletions library/Icingadb/Model/CustomvarFlat.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;

class CustomvarFlat extends Model
Expand Down Expand Up @@ -42,6 +45,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)
Expand Down
5 changes: 0 additions & 5 deletions phpstan-baseline.neon
Original file line number Diff line number Diff line change
Expand Up @@ -3195,11 +3195,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
Expand Down

0 comments on commit f3a21c0

Please sign in to comment.