fix: allow array values for expression parameters

Fixes the problem described in lexik#364 where array values for parameters of filter expressions are misinterpreted as [value, type] pairs, throwing strange errors within Doctrine. When the value (with an optional type) is wrapped inside a `ExpressionParameterValue` value object, the `DoctrineApplyFilterListener` detects and used that explicit declaration. When the value is an array, it is checked if it contains a valid DBAL-type string on the second element, before it is assumed that it contains an implicit type decleration in that old array form. That branch can be deprecated in favor of the explicit declaration using the new `ExpressionParameterValue` class.
spackmat · Jan 30, 2023 · 5b99c11 · 5b99c11
1 parent 6638296
commit 5b99c11
Show file tree

Hide file tree

Showing 5 changed files with 60 additions and 18 deletions.
diff --git a/Event/Listener/DoctrineApplyFilterListener.php b/Event/Listener/DoctrineApplyFilterListener.php
@@ -2,12 +2,14 @@
 
 namespace Lexik\Bundle\FormFilterBundle\Event\Listener;
 
-use Doctrine\ORM\Query\Expr\Composite;
+use Doctrine\DBAL\Types\Type;
 use Doctrine\DBAL\Query\Expression\CompositeExpression;
+use Doctrine\ORM\Query\Expr\Composite;
 use Lexik\Bundle\FormFilterBundle\Event\ApplyFilterConditionEvent;
 use Lexik\Bundle\FormFilterBundle\Filter\Condition\ConditionInterface;
 use Lexik\Bundle\FormFilterBundle\Filter\Condition\ConditionNodeInterface;
 use Lexik\Bundle\FormFilterBundle\Filter\Doctrine\DoctrineQueryBuilderAdapter;
+use Lexik\Bundle\FormFilterBundle\Filter\Doctrine\Expression\ExpressionParameterValue;
 
 /**
  * Add filter conditions on a Doctrine ORM or DBAL query builder.
@@ -49,9 +51,12 @@ public function onApplyFilterCondition(ApplyFilterConditionEvent $event)
             $qbAdapter->{$this->whereMethod}($expression);
 
             foreach ($this->parameters as $name => $value) {
-                if (is_array($value)) {
-                    list($value, $type) = $value;
-                    $qbAdapter->setParameter($name, $value, $type);
+                if ($value instanceof ExpressionParameterValue) {
+                    $qbAdapter->setParameter($name, $value->value, $value->type);
+                } elseif (is_array($value) && count($value) === 2 && Type::hasType($value[1])) {
+                    // that could be deprecated in favor of the ExpressionParameterValue class above
+                    // as it is kind of a hacky solution for a legacy architectural decision
+                    $qbAdapter->setParameter($name, $value[0], $value[1]);
                 } else {
                     $qbAdapter->setParameter($name, $value);
                 }

diff --git a/Filter/Condition/Condition.php b/Filter/Condition/Condition.php
@@ -20,11 +20,12 @@ class Condition implements ConditionInterface
     private $expression;
 
     /**
-     * @var array
+     * @var array<string, mixed>
      *
      * array(
      *     'param_name_1' => $value,
-     *     'param_nema_2  => array($value, $type),
+     *     'param_name_2  => ExpressionParameterValue($value, $type = null),
+     *     'param_name_3  => array($value, $type), // can be deprecated, as it interferes with array values (link for IN() expressions)
      * )
      */
     private $parameters;

diff --git a/Filter/Doctrine/Expression/ExpressionParameterValue.php b/Filter/Doctrine/Expression/ExpressionParameterValue.php
@@ -0,0 +1,30 @@
+<?php
+declare(strict_types=1);
+namespace Lexik\Bundle\FormFilterBundle\Filter\Doctrine\Expression;
+
+/**
+ * Holds the value and optionally the type of an expression parameter
+ * used to distinguish an array of values from the former array holding a single value and a type string
+ *
+ * @author Gregor Meyer https://github.com/spackmat
+ */
+final class ExpressionParameterValue
+{
+    /**
+     * should be a public readonly mixed promoted property when PHP level is raised to PHP 8.1
+     * @var mixed
+     */
+    public $value;
+
+    /**
+     * should be a public readonly ?string promoted property when PHP level is raised to PHP 8.1
+     * @var ?string $type
+     */
+    public $type;
+
+    public function __construct($value, ?string $type = null)
+    {
+        $this->value = $value;
+        $this->type = $type;
+    }
+}
diff --git a/Tests/Fixtures/Filter/FormType.php b/Tests/Fixtures/Filter/FormType.php
@@ -3,6 +3,7 @@
 namespace Lexik\Bundle\FormFilterBundle\Tests\Fixtures\Filter;
 
 use Doctrine\ODM\MongoDB\Query\Expr;
+use Lexik\Bundle\FormFilterBundle\Filter\Doctrine\Expression\ExpressionParameterValue;
 use Lexik\Bundle\FormFilterBundle\Filter\Query\QueryInterface;
 use Symfony\Component\Form\AbstractType;
 use Symfony\Component\Form\FormBuilderInterface;
@@ -24,11 +25,12 @@ public function buildForm(FormBuilderInterface $builder, array $options)
                 if (!empty($values['value'])) {
                     if ($filterQuery->getExpr() instanceof Expr) {
                         $expr = $filterQuery->getExpr()->field($field)->equals($values['value']);
-                    } else {
-                        $expr = $filterQuery->getExpr()->eq($field, $values['value']);
+                        return $filterQuery->createCondition($expr);
                     }
-
-                    return $filterQuery->createCondition($expr);
+                    return $filterQuery->createCondition(
+                        $filterQuery->getExpr()->eq($field, ':position'),
+                        ['position' => new ExpressionParameterValue($values['value'])]
+                    );
                 }
 
                 return null;

diff --git a/Tests/Fixtures/Filter/ItemCallbackFilterType.php b/Tests/Fixtures/Filter/ItemCallbackFilterType.php
@@ -3,6 +3,7 @@
 namespace Lexik\Bundle\FormFilterBundle\Tests\Fixtures\Filter;
 
 use Doctrine\ODM\MongoDB\Query\Expr;
+use Lexik\Bundle\FormFilterBundle\Filter\Doctrine\Expression\ExpressionParameterValue;
 use Lexik\Bundle\FormFilterBundle\Filter\Form\Type\NumberFilterType;
 use Lexik\Bundle\FormFilterBundle\Filter\Form\Type\TextFilterType;
 use Lexik\Bundle\FormFilterBundle\Filter\Query\QueryInterface;
@@ -26,11 +27,12 @@ public function buildForm(FormBuilderInterface $builder, array $options)
                 if (!empty($values['value'])) {
                     if ($filterQuery->getExpr() instanceof Expr) {
                         $expr = $filterQuery->getExpr()->field($field)->notEqual($values['value']);
-                    } else {
-                        $expr = $filterQuery->getExpr()->neq($field, $values['value']);
+                        return $filterQuery->createCondition($expr);
                     }
-
-                    return $filterQuery->createCondition($expr);
+                    return $filterQuery->createCondition(
+                        $filterQuery->getExpr()->neq($field, ':position'),
+                        ['position' => new ExpressionParameterValue($values['value'])]
+                    );
                 }
 
                 return null;
@@ -51,11 +53,13 @@ public function fieldNameCallback(QueryInterface $filterQuery, $field, $values)
         if (!empty($values['value'])) {
             if ($filterQuery->getExpr() instanceof Expr) {
                 $expr = $filterQuery->getExpr()->field($field)->notEqual($values['value']);
-            } else {
-                $expr = $filterQuery->getExpr()->neq($field, sprintf('\'%s\'', $values['value']));
+                return $filterQuery->createCondition($expr);
             }
-
-            return $filterQuery->createCondition($expr);
+            $paramName = substr($field, strrpos($field, '.') + (false === strrpos($field, '.') ? 0 : 1));
+            return $filterQuery->createCondition(
+                $filterQuery->getExpr()->neq($field, ':' . $paramName),
+                [$paramName => new ExpressionParameterValue($values['value'])]
+            );
         }
 
         return null;