Do not apply behaviors twice (#107)

This change prevents now the subject behaviors from being applied for
the same filter condition over again when it's wrapped into a subquery.
This PR removes also the (now obsolete if condition) from the binary
behaviour.

fixes #48
closes Icinga/icingaweb2-module-x509#174

phpstan-baseline.neon

@@ -15,11 +15,6 @@ parameters:
 			count: 1
 			path: src/Behavior/Binary.php
 
-		-
-			message: "#^Parameter \\#1 \\$string of function substr expects string, mixed given\\.$#"
-			count: 1
-			path: src/Behavior/Binary.php
-
 		-
 			message: "#^Parameter \\#2 \\.\\.\\.\\$values of function sprintf expects bool\\|float\\|int\\|string\\|null, mixed given\\.$#"
 			count: 1

src/Behavior/Binary.php

@@ -59,18 +59,6 @@ public function toDb($value, $key, $_)
             return $value;
         }
 
-        /**
-         * TODO(lippserd): If the filter is moved to a subquery, the value has already been processed.
-         * This is because our filter processor is unfortunately doing the transformation twice at the moment:
-         *
-         * {@link https://github.com/Icinga/ipl-orm/issues/48}
-         *
-         * {@see \ipl\Orm\Compat\FilterProcessor::requireAndResolveFilterColumns()}
-         */
-        if (substr($value, 0, 2) === '\\x') {
-            return $value;
-        }
-
         return sprintf('\\x%s', bin2hex($value));
     }
 

src/Compat/FilterProcessor.php

@@ -107,6 +107,7 @@ protected function requireAndResolveFilterColumns(Filter\Rule $filter, Query $qu
             $relations = new AppendIterator();
             $relations->append(new ArrayIterator([$baseTable => null]));
             $relations->append($resolver->resolveRelations($relationPath));
+            $behaviorsApplied = $filter->metaData()->get('behaviorsApplied', false);
             foreach ($relations as $path => $relation) {
                 $columnName = substr($column, strlen($path) + 1);
 
@@ -121,25 +122,40 @@ protected function requireAndResolveFilterColumns(Filter\Rule $filter, Query $qu
                 // This is only used within the Binary behavior in rewriteCondition().
                 $filter->metaData()->set('originalValue', $filter->getValue());
 
-                try {
-                    // Prepare filter as if it were final to allow full control for rewrite filter behaviors
-                    $filter->setValue($subjectBehaviors->persistProperty($filter->getValue(), $columnName));
-                } catch (ValueConversionException $_) {
-                    // The search bar may submit values with wildcards or whatever the user has entered.
-                    // In this case, we can simply ignore this error instead of rendering a stack trace.
+                if (! $behaviorsApplied) {
+                    try {
+                        // Prepare filter as if it were final to allow full control for rewrite filter behaviors
+                        $filter->setValue($subjectBehaviors->persistProperty($filter->getValue(), $columnName));
+                    } catch (ValueConversionException $_) {
+                        // The search bar may submit values with wildcards or whatever the user has entered.
+                        // In this case, we can simply ignore this error instead of rendering a stack trace.
+                    }
                 }
 
                 $filter->setColumn($resolver->getAlias($subject) . '.' . $columnName);
                 $filter->metaData()->set('columnName', $columnName);
                 $filter->metaData()->set('relationPath', $path);
 
-                $rewrittenFilter = $subjectBehaviors->rewriteCondition($filter, $path . '.');
-                if ($rewrittenFilter !== null) {
-                    return $this->requireAndResolveFilterColumns($rewrittenFilter, $query, $forceOptimization)
-                        ?: $rewrittenFilter;
+                if (! $behaviorsApplied) {
+                    $rewrittenFilter = $subjectBehaviors->rewriteCondition($filter, $path . '.');
+                    if ($rewrittenFilter !== null) {
+                        return $this->requireAndResolveFilterColumns($rewrittenFilter, $query, $forceOptimization)
+                            ?: $rewrittenFilter;
+                    }
                 }
             }
 
+            /**
+             * We have applied all the subject behaviors for this filter condition, so set this metadata to prevent
+             * the behaviors from being applied for the same filter condition over again later in case of a subquery.
+             * The behaviors are processed again due to $subQueryFilter being evaluated by this processor as part of
+             * the subquery. The reason for this is the application of aliases used in said subquery. Since this is
+             * part of the filter column qualification, and the behaviors are not, this should be separately done.
+             * There's a similar comment in {@see Query::createSubQuery()} which should be considered when working
+             * on improving this.
+             */
+            $filter->metaData()->set('behaviorsApplied', true);
+
             if (! $resolver->hasSelectableColumn($subject, $columnName)) {
                 throw new InvalidColumnException($columnName, $subject);
             }