From 5b99c113732307056d46bc984c75015cbdc6fffc Mon Sep 17 00:00:00 2001 From: spackmat Date: Mon, 30 Jan 2023 15:08:08 +0100 Subject: [PATCH 1/3] fix: allow array values for expression parameters Fixes the problem described in https://github.com/lexik/LexikFormFilterBundle/issues/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. --- .../Listener/DoctrineApplyFilterListener.php | 13 +++++--- Filter/Condition/Condition.php | 5 ++-- .../Expression/ExpressionParameterValue.php | 30 +++++++++++++++++++ Tests/Fixtures/Filter/FormType.php | 10 ++++--- .../Filter/ItemCallbackFilterType.php | 20 ++++++++----- 5 files changed, 60 insertions(+), 18 deletions(-) create mode 100644 Filter/Doctrine/Expression/ExpressionParameterValue.php diff --git a/Event/Listener/DoctrineApplyFilterListener.php b/Event/Listener/DoctrineApplyFilterListener.php index f7091c3..78efa12 100644 --- 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 index ff272d8..e7459a1 100644 --- a/Filter/Condition/Condition.php +++ b/Filter/Condition/Condition.php @@ -20,11 +20,12 @@ class Condition implements ConditionInterface private $expression; /** - * @var array + * @var array * * 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 new file mode 100644 index 0000000..07ca13a --- /dev/null +++ b/Filter/Doctrine/Expression/ExpressionParameterValue.php @@ -0,0 +1,30 @@ +value = $value; + $this->type = $type; + } +} diff --git a/Tests/Fixtures/Filter/FormType.php b/Tests/Fixtures/Filter/FormType.php index 97b79e0..8b51793 100644 --- 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 index b2d29f7..f9056c8 100644 --- 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; From 2c434afb4d527756f3c77d415f90f08dbfd6e795 Mon Sep 17 00:00:00 2001 From: spackmat Date: Mon, 30 Jan 2023 15:20:15 +0100 Subject: [PATCH 2/3] fix tests for last commit In 5b99c113732307056d46bc984c75015cbdc6fffc the tests failed do to not updated assertions. --- Tests/Filter/Doctrine/DBALQueryBuilderUpdaterTest.php | 4 ++-- Tests/Filter/Doctrine/ORMQueryBuilderUpdaterTest.php | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Tests/Filter/Doctrine/DBALQueryBuilderUpdaterTest.php b/Tests/Filter/Doctrine/DBALQueryBuilderUpdaterTest.php index 06d6279..78ae86f 100644 --- a/Tests/Filter/Doctrine/DBALQueryBuilderUpdaterTest.php +++ b/Tests/Filter/Doctrine/DBALQueryBuilderUpdaterTest.php @@ -32,7 +32,7 @@ public function testDisabledFieldQuery() public function testApplyFilterOption() { parent::createApplyFilterOptionTest('getSQL', array( - 'SELECT i FROM item i WHERE (i.name <> \'blabla\') AND (i.position <> 2)', + 'SELECT i FROM item i WHERE (i.name <> :name) AND (i.position <> :position)', )); } @@ -82,7 +82,7 @@ public function testDateTimeRange() public function testFilterStandardType() { parent::createFilterStandardTypeTest('getSQL', array( - 'SELECT i FROM item i WHERE (i.name LIKE \'%hey dude%\') AND (i.position = 99)', + 'SELECT i FROM item i WHERE (i.name LIKE \'%hey dude%\') AND (i.position = :position)', )); } diff --git a/Tests/Filter/Doctrine/ORMQueryBuilderUpdaterTest.php b/Tests/Filter/Doctrine/ORMQueryBuilderUpdaterTest.php index e40ca1c..02e9d24 100644 --- a/Tests/Filter/Doctrine/ORMQueryBuilderUpdaterTest.php +++ b/Tests/Filter/Doctrine/ORMQueryBuilderUpdaterTest.php @@ -35,7 +35,7 @@ public function testDisabledFieldQuery() public function testApplyFilterOption() { parent::createApplyFilterOptionTest('getDQL', array( - 'SELECT i FROM Lexik\Bundle\FormFilterBundle\Tests\Fixtures\Entity\Item i WHERE i.name <> \'blabla\' AND i.position <> 2', + 'SELECT i FROM Lexik\Bundle\FormFilterBundle\Tests\Fixtures\Entity\Item i WHERE i.name <> :name AND i.position <> :position', )); } @@ -85,7 +85,7 @@ public function testDateTimeRange() public function testFilterStandardType() { parent::createFilterStandardTypeTest('getDQL', array( - 'SELECT i FROM Lexik\Bundle\FormFilterBundle\Tests\Fixtures\Entity\Item i WHERE i.name LIKE \'%hey dude%\' AND i.position = 99', + 'SELECT i FROM Lexik\Bundle\FormFilterBundle\Tests\Fixtures\Entity\Item i WHERE i.name LIKE \'%hey dude%\' AND i.position = :position', )); } From fec4aa3496bec08589964fca18ca96c663d9658f Mon Sep 17 00:00:00 2001 From: spackmat Date: Wed, 1 Feb 2023 11:36:03 +0100 Subject: [PATCH 3/3] refactor: Use the ExpressionParameterValue internally This changes the AbstractDoctrine- and DoctrineORMSubscriber to use the new `ExpressionParameterValue` --- Event/Subscriber/AbstractDoctrineSubscriber.php | 17 +++++++++-------- Event/Subscriber/DoctrineORMSubscriber.php | 5 +++-- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/Event/Subscriber/AbstractDoctrineSubscriber.php b/Event/Subscriber/AbstractDoctrineSubscriber.php index e2b2cba..bb006bc 100644 --- a/Event/Subscriber/AbstractDoctrineSubscriber.php +++ b/Event/Subscriber/AbstractDoctrineSubscriber.php @@ -2,6 +2,7 @@ namespace Lexik\Bundle\FormFilterBundle\Event\Subscriber; +use Lexik\Bundle\FormFilterBundle\Filter\Doctrine\Expression\ExpressionParameterValue; use Lexik\Bundle\FormFilterBundle\Filter\FilterOperands; use Lexik\Bundle\FormFilterBundle\Filter\Form\Type\BooleanFilterType; use Lexik\Bundle\FormFilterBundle\Event\GetFilterConditionEvent; @@ -30,12 +31,12 @@ public function filterValue(GetFilterConditionEvent $event) if (is_array($values['value']) && sizeof($values['value']) > 0) { $event->setCondition( $expr->in($event->getField(), ':'.$paramName), - array($paramName => array($values['value'], Connection::PARAM_STR_ARRAY)) + array($paramName => new ExpressionParameterValue($values['value'], Connection::PARAM_STR_ARRAY)) ); } elseif (!is_array($values['value'])) { $event->setCondition( $expr->eq($event->getField(), ':'.$paramName), - array($paramName => array($values['value'], Types::STRING)) + array($paramName => new ExpressionParameterValue($values['value'], Types::STRING)) ); } } @@ -56,7 +57,7 @@ public function filterBoolean(GetFilterConditionEvent $event) $event->setCondition( $expr->eq($event->getField(), ':'.$paramName), - array($paramName => array($value, Types::BOOLEAN)) + array($paramName => new ExpressionParameterValue($value, Types::BOOLEAN)) ); } } @@ -74,7 +75,7 @@ public function filterCheckbox(GetFilterConditionEvent $event) $event->setCondition( $expr->eq($event->getField(), ':'.$paramName), - array($paramName => array($values['value'], Types::STRING)) + array($paramName => new ExpressionParameterValue($values['value'], Types::STRING)) ); } } @@ -92,7 +93,7 @@ public function filterDate(GetFilterConditionEvent $event) $event->setCondition( $expr->eq($event->getField(), ':'.$paramName), - array($paramName => array($values['value'], Types::DATE_MUTABLE)) + array($paramName => new ExpressionParameterValue($values['value'], Types::DATE_MUTABLE)) ); } } @@ -124,7 +125,7 @@ public function filterDateTime(GetFilterConditionEvent $event) $event->setCondition( $expr->eq($event->getField(), ':'.$paramName), - array($paramName => array($values['value'], Types::DATETIME_MUTABLE)) + array($paramName => new ExpressionParameterValue($values['value'], Types::DATETIME_MUTABLE)) ); } } @@ -158,7 +159,7 @@ public function filterNumber(GetFilterConditionEvent $event) $event->setCondition( $expr->$op($event->getField(), ':'.$paramName), - array($paramName => array($values['value'], is_int($values['value']) ? Types::INTEGER : Types::FLOAT)) + array($paramName => new ExpressionParameterValue($values['value'], is_int($values['value']) ? Types::INTEGER : Types::FLOAT)) ); } } @@ -211,7 +212,7 @@ public function filterNumberRange(GetFilterConditionEvent $event) $rightParamName = sprintf('p_%s_right', str_replace('.', '_', $event->getField())); $expression->add($expr->$rightCond($event->getField(), ':'.$rightParamName)); - $params[$rightParamName] = array($rightValue, is_int($rightValue) ? Types::INTEGER : Types::FLOAT); + $params[$rightParamName] = new ExpressionParameterValue($rightValue, is_int($rightValue) ? Types::INTEGER : Types::FLOAT); } } diff --git a/Event/Subscriber/DoctrineORMSubscriber.php b/Event/Subscriber/DoctrineORMSubscriber.php index 3db9776..4a70dee 100644 --- a/Event/Subscriber/DoctrineORMSubscriber.php +++ b/Event/Subscriber/DoctrineORMSubscriber.php @@ -9,6 +9,7 @@ use Doctrine\ORM\QueryBuilder; use Doctrine\ORM\Mapping\ClassMetadataInfo; use Lexik\Bundle\FormFilterBundle\Event\GetFilterConditionEvent; +use Lexik\Bundle\FormFilterBundle\Filter\Doctrine\Expression\ExpressionParameterValue; use Symfony\Component\EventDispatcher\EventSubscriberInterface; /** @@ -100,13 +101,13 @@ public function filterEntity(GetFilterConditionEvent $event) if (count($ids) > 0) { $event->setCondition( $expr->in($filterField, ':'.$paramName), - array($paramName => array($ids, Connection::PARAM_INT_ARRAY)) + array($paramName => new ExpressionParameterValue($ids, Connection::PARAM_INT_ARRAY)) ); } } else { $event->setCondition( $expr->eq($filterField, ':'.$paramName), - array($paramName => array( + array($paramName => new ExpressionParameterValue( $this->getEntityIdentifier($values['value'], $queryBuilder->getEntityManager()), Types::INTEGER ))