From 73d9ae39ab6c96fc2ec6d9dba96faa85bda4fee1 Mon Sep 17 00:00:00 2001 From: Oleg Andreyev Date: Fri, 27 Dec 2019 23:12:23 +0200 Subject: [PATCH 1/2] Passing custom doctrine type to addWhereByStrategy !squash remove type from $fieldType missed doctrine type of bottom of filterProperty --- .../Doctrine/Orm/Filter/SearchFilter.php | 29 +++++++++---------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/src/Bridge/Doctrine/Orm/Filter/SearchFilter.php b/src/Bridge/Doctrine/Orm/Filter/SearchFilter.php index da0b8f3b505..6b539999645 100644 --- a/src/Bridge/Doctrine/Orm/Filter/SearchFilter.php +++ b/src/Bridge/Doctrine/Orm/Filter/SearchFilter.php @@ -92,12 +92,11 @@ protected function filterProperty(string $property, $value, QueryBuilder $queryB $caseSensitive = true; $metadata = $this->getNestedMetadata($resourceClass, $associations); - if ($metadata->hasField($field)) { - if ('id' === $field) { - $values = array_map([$this, 'getIdFromValue'], $values); - } + $doctrineTypeField = $this->getDoctrineFieldType($property, $resourceClass); + $values = array_map([$this, 'getIdFromValue'], $values); - if (!$this->hasValidValues($values, $this->getDoctrineFieldType($property, $resourceClass))) { + if ($metadata->hasField($field)) { + if (!$this->hasValidValues($values, $doctrineTypeField)) { $this->logger->notice('Invalid filter ignored', [ 'exception' => new InvalidArgumentException(sprintf('Values for field "%s" are not valid according to the doctrine type.', $field)), ]); @@ -114,7 +113,7 @@ protected function filterProperty(string $property, $value, QueryBuilder $queryB } if (1 === \count($values)) { - $this->addWhereByStrategy($strategy, $queryBuilder, $queryNameGenerator, $alias, $field, $values[0], $caseSensitive); + $this->addWhereByStrategy($strategy, $queryBuilder, $queryNameGenerator, $alias, $field, $doctrineTypeField, $values[0], $caseSensitive); return; } @@ -140,9 +139,7 @@ protected function filterProperty(string $property, $value, QueryBuilder $queryB return; } - $values = array_map([$this, 'getIdFromValue'], $values); $associationFieldIdentifier = 'id'; - $doctrineTypeField = $this->getDoctrineFieldType($property, $resourceClass); if (null !== $this->identifiersExtractor) { $associationResourceClass = $metadata->getAssociationTargetClass($field); @@ -171,11 +168,11 @@ protected function filterProperty(string $property, $value, QueryBuilder $queryB if (1 === \count($values)) { $queryBuilder ->andWhere(sprintf('%s.%s = :%s', $associationAlias, $associationField, $valueParameter)) - ->setParameter($valueParameter, $values[0]); + ->setParameter($valueParameter, $values[0], $doctrineTypeField); } else { $queryBuilder ->andWhere(sprintf('%s.%s IN (:%s)', $associationAlias, $associationField, $valueParameter)) - ->setParameter($valueParameter, $values); + ->setParameter($valueParameter, $values, $doctrineTypeField); } } @@ -184,7 +181,7 @@ protected function filterProperty(string $property, $value, QueryBuilder $queryB * * @throws InvalidArgumentException If strategy does not exist */ - protected function addWhereByStrategy(string $strategy, QueryBuilder $queryBuilder, QueryNameGeneratorInterface $queryNameGenerator, string $alias, string $field, $value, bool $caseSensitive) + protected function addWhereByStrategy(string $strategy, QueryBuilder $queryBuilder, QueryNameGeneratorInterface $queryNameGenerator, string $alias, string $field, $fieldType, $value, bool $caseSensitive) { $wrapCase = $this->createWrapCase($caseSensitive); $valueParameter = $queryNameGenerator->generateParameterName($field); @@ -194,27 +191,27 @@ protected function addWhereByStrategy(string $strategy, QueryBuilder $queryBuild case self::STRATEGY_EXACT: $queryBuilder ->andWhere(sprintf($wrapCase('%s.%s').' = '.$wrapCase(':%s'), $alias, $field, $valueParameter)) - ->setParameter($valueParameter, $value); + ->setParameter($valueParameter, $value, $fieldType); break; case self::STRATEGY_PARTIAL: $queryBuilder ->andWhere(sprintf($wrapCase('%s.%s').' LIKE '.$wrapCase('CONCAT(\'%%\', :%s, \'%%\')'), $alias, $field, $valueParameter)) - ->setParameter($valueParameter, $value); + ->setParameter($valueParameter, $value, $fieldType); break; case self::STRATEGY_START: $queryBuilder ->andWhere(sprintf($wrapCase('%s.%s').' LIKE '.$wrapCase('CONCAT(:%s, \'%%\')'), $alias, $field, $valueParameter)) - ->setParameter($valueParameter, $value); + ->setParameter($valueParameter, $value, $fieldType); break; case self::STRATEGY_END: $queryBuilder ->andWhere(sprintf($wrapCase('%s.%s').' LIKE '.$wrapCase('CONCAT(\'%%\', :%s)'), $alias, $field, $valueParameter)) - ->setParameter($valueParameter, $value); + ->setParameter($valueParameter, $value, $fieldType); break; case self::STRATEGY_WORD_START: $queryBuilder ->andWhere(sprintf($wrapCase('%1$s.%2$s').' LIKE '.$wrapCase('CONCAT(:%3$s, \'%%\')').' OR '.$wrapCase('%1$s.%2$s').' LIKE '.$wrapCase('CONCAT(\'%% \', :%3$s, \'%%\')'), $alias, $field, $valueParameter)) - ->setParameter($valueParameter, $value); + ->setParameter($valueParameter, $value, $fieldType); break; default: throw new InvalidArgumentException(sprintf('strategy %s does not exist.', $strategy)); From 2550831af7297614bddf31e50031681c8cee3b12 Mon Sep 17 00:00:00 2001 From: soyuka Date: Fri, 24 Apr 2020 11:46:17 +0200 Subject: [PATCH 2/2] Deprecate extending searchfilter Adds correct deprecations to introduce back #3331 --- .../Doctrine/Orm/Filter/SearchFilter.php | 12 +++++++-- .../Doctrine/Orm/Filter/SearchFilterTest.php | 13 +++++++++ tests/Fixtures/ExtendingSearchFilter.php | 27 +++++++++++++++++++ 3 files changed, 50 insertions(+), 2 deletions(-) create mode 100644 tests/Fixtures/ExtendingSearchFilter.php diff --git a/src/Bridge/Doctrine/Orm/Filter/SearchFilter.php b/src/Bridge/Doctrine/Orm/Filter/SearchFilter.php index 6b539999645..9157107f97d 100644 --- a/src/Bridge/Doctrine/Orm/Filter/SearchFilter.php +++ b/src/Bridge/Doctrine/Orm/Filter/SearchFilter.php @@ -33,6 +33,7 @@ * Filter the collection by given properties. * * @author Kévin Dunglas + * @final */ class SearchFilter extends AbstractContextAwareFilter implements SearchFilterInterface { @@ -113,7 +114,7 @@ protected function filterProperty(string $property, $value, QueryBuilder $queryB } if (1 === \count($values)) { - $this->addWhereByStrategy($strategy, $queryBuilder, $queryNameGenerator, $alias, $field, $doctrineTypeField, $values[0], $caseSensitive); + $this->addWhereByStrategy($strategy, $queryBuilder, $queryNameGenerator, $alias, $field, $values[0], $caseSensitive, $doctrineTypeField); return; } @@ -181,8 +182,15 @@ protected function filterProperty(string $property, $value, QueryBuilder $queryB * * @throws InvalidArgumentException If strategy does not exist */ - protected function addWhereByStrategy(string $strategy, QueryBuilder $queryBuilder, QueryNameGeneratorInterface $queryNameGenerator, string $alias, string $field, $fieldType, $value, bool $caseSensitive) + protected function addWhereByStrategy(string $strategy, QueryBuilder $queryBuilder, QueryNameGeneratorInterface $queryNameGenerator, string $alias, string $field, $value, bool $caseSensitive/*, string $fieldType = null*/) { + $fieldType = null; + if (8 === \func_num_args()) { + $fieldType = func_get_arg(7); + } else { + @trigger_error(sprintf('Method "%s()" will have a 8th `string $fieldType` argument in version 3.0. Not defining it is deprecated since 2.6.', __METHOD__), E_USER_DEPRECATED); + } + $wrapCase = $this->createWrapCase($caseSensitive); $valueParameter = $queryNameGenerator->generateParameterName($field); diff --git a/tests/Bridge/Doctrine/Orm/Filter/SearchFilterTest.php b/tests/Bridge/Doctrine/Orm/Filter/SearchFilterTest.php index 5b5e6f418e5..0fe0dc146e7 100644 --- a/tests/Bridge/Doctrine/Orm/Filter/SearchFilterTest.php +++ b/tests/Bridge/Doctrine/Orm/Filter/SearchFilterTest.php @@ -20,6 +20,7 @@ use ApiPlatform\Core\Exception\InvalidArgumentException; use ApiPlatform\Core\Test\DoctrineOrmFilterTestCase; use ApiPlatform\Core\Tests\Bridge\Doctrine\Common\Filter\SearchFilterTestTrait; +use ApiPlatform\Core\Tests\Fixtures\ExtendingSearchFilter; use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\Dummy; use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\RelatedDummy; use ApiPlatform\Core\Tests\Fixtures\TestBundle\Serializer\NameConverter\CustomConverter; @@ -363,6 +364,18 @@ private function doTestApplyWithAnotherAlias(bool $request) $expectedDql = sprintf('SELECT %s FROM %s %1$s WHERE %1$s.name = :name_p1', 'somealias', Dummy::class); $this->assertEquals($expectedDql, $queryBuilder->getQuery()->getDQL()); + $this->assertTrue($queryBuilder->getParameter('name_p1')->typeWasSpecified()); + } + + /** + * @group legacy + * @expectedDeprecation The "ApiPlatform\Core\Bridge\Doctrine\Orm\Filter\SearchFilter" class is considered final. It may change without further notice as of its next major version. You should not extend it from "ApiPlatform\Core\Tests\Fixtures\ExtendingSearchFilter". + * + * @expectedDeprecation Method "ApiPlatform\Core\Bridge\Doctrine\Orm\Filter\SearchFilter::addWhereByStrategy()" will have a 8th `string $fieldType` argument in version 3.0. Not defining it is deprecated since 2.6. + */ + public function testDeprecateAddWhereByStrategyWithoutType() + { + new ExtendingSearchFilter($this->repository->createQueryBuilder($this->alias)); } public function provideApplyTestData(): array diff --git a/tests/Fixtures/ExtendingSearchFilter.php b/tests/Fixtures/ExtendingSearchFilter.php new file mode 100644 index 00000000000..337518feb9e --- /dev/null +++ b/tests/Fixtures/ExtendingSearchFilter.php @@ -0,0 +1,27 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace ApiPlatform\Core\Tests\Fixtures; + +use ApiPlatform\Core\Bridge\Doctrine\Common\Filter\SearchFilterInterface; +use ApiPlatform\Core\Bridge\Doctrine\Orm\Filter\SearchFilter; +use ApiPlatform\Core\Bridge\Doctrine\Orm\Util\QueryNameGenerator; +use Doctrine\ORM\QueryBuilder; + +class ExtendingSearchFilter extends SearchFilter implements SearchFilterInterface +{ + public function __construct(QueryBuilder $queryBuilder) + { + parent::addWhereByStrategy(self::STRATEGY_EXACT, $queryBuilder, new QueryNameGenerator(), 'o', 'name', 'test', false); + } +}