From 656d079ffc739c01cbcad987022b2697efa2508a Mon Sep 17 00:00:00 2001 From: Javier Spagnoletti Date: Sat, 20 Mar 2021 10:34:41 -0300 Subject: [PATCH] Fix filter query when using "OR" conditions --- src/Filter/Filter.php | 79 ++++++++++++++++++++++++++++++++++++- tests/Filter/FilterTest.php | 63 ++++++++++++++++++++++++++++- 2 files changed, 139 insertions(+), 3 deletions(-) diff --git a/src/Filter/Filter.php b/src/Filter/Filter.php index 01df49c64..6fc718910 100644 --- a/src/Filter/Filter.php +++ b/src/Filter/Filter.php @@ -13,6 +13,7 @@ namespace Sonata\DoctrineORMAdminBundle\Filter; +use Doctrine\ORM\Query\Expr\Orx; use Sonata\AdminBundle\Datagrid\ProxyQueryInterface as BaseProxyQueryInterface; use Sonata\AdminBundle\Filter\Filter as BaseFilter; use Sonata\DoctrineORMAdminBundle\Datagrid\ProxyQueryInterface; @@ -24,6 +25,15 @@ abstract class Filter extends BaseFilter */ protected $active = false; + /** + * Holds an array of `orX` expressions used by each admin when the condition + * equals the value on `FilterInterface::CONDITION_OR`, using the admin code + * as index. + * + * @var array + */ + private static $orExpressionsByAdmin = []; + /** * NEXT_MAJOR change $query type for Sonata\DoctrineORMAdminBundle\Datagrid\ProxyQueryInterface. * @@ -96,7 +106,7 @@ protected function applyWhere(BaseProxyQueryInterface $query, $parameter) } if (self::CONDITION_OR === $this->getCondition()) { - $query->getQueryBuilder()->orWhere($parameter); + $this->addOrParameter($query, $parameter); } else { $query->getQueryBuilder()->andWhere($parameter); } @@ -141,4 +151,71 @@ protected function getNewParameterName(BaseProxyQueryInterface $query) // by underscores. return str_replace('.', '_', $this->getName()).'_'.$query->getUniqueParameterId(); } + + /** + * Adds the parameter to the corresponding `Orx` expression used in the `where` clause. + * If it doesn't exist, a new one is created. + * This method groups the filter "OR" conditions based on the "admin_code" option. It this + * option is not set, it uses a marker (":sonata_admin_datagrid_filter_query_marker") in + * the resulting DQL in order to identify the corresponding "WHERE (...)" condition + * group each time it is required. + * It allows to get queries like "WHERE previous_condition = previous_value AND (filter_1 = value OR filter_2 = value OR ...)", + * where the logical "OR" operators added by the filters are grouped inside a condition, + * instead of having unfolded "WHERE ..." clauses like "WHERE previous_condition = previous_value OR filter_1 = value OR filter_2 = value OR ...", + * which will produce undesired results. + * + * TODO: Remove the logic related to the ":sonata_admin_datagrid_filter_query_marker" marker when + * the constraint for "sonata-project/admin-bundle" guarantees that the "admin_code" option is set. + * + * @param mixed $parameter + */ + private function addOrParameter(BaseProxyQueryInterface $query, $parameter): void + { + $adminCode = $this->getOption('admin_code'); + $orExpression = self::$orExpressionsByAdmin[$adminCode] ?? null; + if ($orExpression instanceof Orx) { + $orExpression->add($parameter); + + return; + } + + $qb = $query->getQueryBuilder(); + $where = $qb->getDQLPart('where'); + + // Search for the ":sonata_admin_datagrid_filter_query_marker" marker in order to + // get the `Orx` expression. + if (null === $adminCode && null !== $where) { + foreach ($where->getParts() as $expression) { + if (!$expression instanceof Orx) { + continue; + } + + $expressionParts = $expression->getParts(); + + if (isset($expressionParts[0]) && \is_string($expressionParts[0]) && + 0 === strpos($expressionParts[0], ':sonata_admin_datagrid_filter_query_marker') + ) { + $expression->add($parameter); + + return; + } + } + } + + // Create a new `Orx` expression. + $orExpression = $qb->expr()->orX(); + + if (null === $adminCode) { + // Add the ":sonata_admin_datagrid_filter_query_marker" parameter as marker for the `Orx` expression. + $orExpression->add($qb->expr()->isNull(':sonata_admin_datagrid_filter_query_marker')); + $qb->setParameter('sonata_admin_datagrid_filter_query_marker', 'sonata_admin.datagrid.filter_query.marker'); + } else { + self::$orExpressionsByAdmin[$adminCode] = $orExpression; + } + + $orExpression->add($parameter); + + // Add the `Orx` expression to the `where` clause. + $qb->andWhere($orExpression); + } } diff --git a/tests/Filter/FilterTest.php b/tests/Filter/FilterTest.php index c895ca4ff..75f0e8418 100644 --- a/tests/Filter/FilterTest.php +++ b/tests/Filter/FilterTest.php @@ -13,11 +13,14 @@ namespace Sonata\DoctrineORMAdminBundle\Tests\Filter; -use PHPUnit\Framework\TestCase; +use Doctrine\ORM\EntityManagerInterface; +use Doctrine\ORM\Query\Expr; use Sonata\AdminBundle\Datagrid\ProxyQueryInterface; +use Sonata\DoctrineORMAdminBundle\Datagrid\ProxyQuery; use Sonata\DoctrineORMAdminBundle\Filter\Filter; +use Sonata\DoctrineORMAdminBundle\Filter\StringFilter; -final class FilterTest extends TestCase +final class FilterTest extends FilterTestCase { /** * @var Filter @@ -58,6 +61,62 @@ public function testIsActive(): void $this->assertFalse($this->filter->isActive()); } + /** + * @dataProvider orExpressionProvider + */ + public function testOrExpression(string $expected, array $filterOptions = []): void + { + $expressionBuilder = new Expr(); + $entityManager = $this->createMock(EntityManagerInterface::class); + $entityManager->method('getExpressionBuilder')->willReturn($expressionBuilder); + $queryBuilder = new TestQueryBuilder($entityManager); + + $queryBuilder->select('e') + ->from('MyEntity', 'e') + ->where($queryBuilder->expr()->eq(1, 2)) + ->andWhere( + $queryBuilder->expr()->orX( + $queryBuilder->expr()->eq(3, 4), + $queryBuilder->expr()->eq(5, 6) + ) + ); + + $proxyQuery = new ProxyQuery($queryBuilder); + $this->assertSameQuery([], $proxyQuery); + + $filter1 = new StringFilter(); + $filter1->setCondition(Filter::CONDITION_OR); + $filter1->initialize('field_name', $filterOptions); + + $filter1->filter($proxyQuery, 'alias', 'project', ['value' => 'sonata-project']); + + $filter2 = new StringFilter(); + $filter2->setCondition(Filter::CONDITION_OR); + $filter2->initialize('field_name', $filterOptions); + + $filter2->filter($proxyQuery, 'alias', 'version', ['value' => '3.x']); + + $this->assertSame($expected, $queryBuilder->getDQL()); + } + + public function orExpressionProvider(): iterable + { + yield 'Using "admin_code" option' => [ + 'SELECT e FROM MyEntity e WHERE 1 = 2 AND (3 = 4 OR 5 = 6) AND (alias.project LIKE :field_name_0 OR alias.version LIKE :field_name_1)', + [ + 'allow_empty' => false, + 'admin_code' => 'my_admin', + ], + ]; + + yield 'Missing "admin_code" option, fallback to DQL marker' => [ + 'SELECT e FROM MyEntity e WHERE 1 = 2 AND (3 = 4 OR 5 = 6) AND (:sonata_admin_datagrid_filter_query_marker IS NULL OR alias.project LIKE :field_name_0 OR alias.version LIKE :field_name_1)', + [ + 'allow_empty' => false, + ], + ]; + } + private function createFilter(): Filter { return new class() extends Filter {