From db90e1c30beca4cad81451474b9dc893d4f4e5a7 Mon Sep 17 00:00:00 2001 From: soyuka Date: Sat, 13 Mar 2021 10:14:46 +0100 Subject: [PATCH] Revert "Handle binary UUID in SearchFilter (#3774)" This reverts commit ff248aea8dfb355fbbddd748b35f4f12964e4342. --- .github/workflows/ci.yml | 2 +- CHANGELOG.md | 1 - behat.yml.dist | 2 +- features/doctrine/search_filter.feature | 46 ---------- .../Doctrine/Orm/Filter/SearchFilter.php | 62 ++++++------- tests/Behat/DoctrineContext.php | 30 ------- .../Doctrine/Orm/Filter/SearchFilterTest.php | 23 ++--- .../Entity/RamseyUuidBinaryDummy.php | 89 ------------------- tests/Fixtures/app/config/config_common.yml | 3 +- tests/Fixtures/app/config/config_mysql.yml | 1 - tests/Fixtures/app/config/config_postgres.yml | 1 - tests/Fixtures/app/config/config_sqlite.yml | 1 - 12 files changed, 43 insertions(+), 218 deletions(-) delete mode 100644 tests/Fixtures/TestBundle/Entity/RamseyUuidBinaryDummy.php diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e009e8aa5ec..45443d91d00 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -384,7 +384,7 @@ jobs: - name: Clear test app cache run: tests/Fixtures/app/console cache:clear --ansi - name: Run Behat tests - run: vendor/bin/behat --out=std --format=progress --profile=default --no-interaction --tags '~@!lowest' + run: vendor/bin/behat --out=std --format=progress --profile=default --no-interaction postgresql: name: Behat (PHP ${{ matrix.php }}) (PostgreSQL) diff --git a/CHANGELOG.md b/CHANGELOG.md index b0b03f98dbe..4a6ff23beba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,7 +13,6 @@ * Security: Use a `NullToken` when using the new authenticator manager in the resource access checker (#4067) * Mercure: Do not use data in options when deleting (#4056) * Doctrine: Support for foreign identifiers (#4042) -* Doctrine: Support for binary UUID in search filter (#3774) * Doctrine: Do not add join or lookup for search filter with empty value (#3703) * Doctrine: Reduce code duplication in search filter (#3541) * JSON Schema: Allow generating documentation when property and method start from "is" (property `isActive` and method `isActive`) (#4064) diff --git a/behat.yml.dist b/behat.yml.dist index 6625645655c..c3af2501185 100644 --- a/behat.yml.dist +++ b/behat.yml.dist @@ -50,7 +50,7 @@ postgres: - 'Behat\MinkExtension\Context\MinkContext' - 'behatch:context:rest' filters: - tags: '~@sqlite&&~@mongodb&&~@elasticsearch&&~@!postgres' + tags: '~@sqlite&&~@mongodb&&~@elasticsearch' mongodb: suites: diff --git a/features/doctrine/search_filter.feature b/features/doctrine/search_filter.feature index ad4b30c0a10..c396159bf77 100644 --- a/features/doctrine/search_filter.feature +++ b/features/doctrine/search_filter.feature @@ -539,52 +539,6 @@ Feature: Search filter on collections } """ - @!postgres - @!mongodb - @!lowest - Scenario: Search collection by binary UUID (Ramsey) - Given there is a ramsey identified resource with binary uuid "c19900a9-d2b2-45bf-b040-05c72d321282" - And there is a ramsey identified resource with binary uuid "a96cb2ed-e3dc-4449-9842-830e770cdecc" - When I send a "GET" request to "/ramsey_uuid_binary_dummies?id=c19900a9-d2b2-45bf-b040-05c72d321282" - Then the response status code should be 200 - And the response should be in JSON - And the header "Content-Type" should be equal to "application/ld+json; charset=utf-8" - And the JSON node "hydra:totalItems" should be equal to "1" - - @!postgres - @!mongodb - @!lowest - Scenario: Search collection by binary UUID (Ramsey) (multiple values) - Given there is a ramsey identified resource with binary uuid "f71a6469-1bfc-4945-bad1-d6092f09a8c3" - When I send a "GET" request to "/ramsey_uuid_binary_dummies?id[]=c19900a9-d2b2-45bf-b040-05c72d321282&id[]=f71a6469-1bfc-4945-bad1-d6092f09a8c3" - Then the response status code should be 200 - And the response should be in JSON - And the header "Content-Type" should be equal to "application/ld+json; charset=utf-8" - And the JSON node "hydra:totalItems" should be equal to "2" - - @!postgres - @!mongodb - @!lowest - Scenario: Search collection by related binary UUID (Ramsey) - Given there is a ramsey identified resource with binary uuid "56fa36c3-2b5e-4813-9e3a-b0bbe2ab5553" having a related resource with binary uuid "02227dc6-a371-4b8b-a34c-bbbf921b8ebd" - And there is a ramsey identified resource with binary uuid "4d796212-4b26-4e19-b092-a32d990b1e7e" having a related resource with binary uuid "31f64c33-6061-4fc1-b0e8-f4711b607c7d" - When I send a "GET" request to "/ramsey_uuid_binary_dummies?relateds=02227dc6-a371-4b8b-a34c-bbbf921b8ebd" - Then the response status code should be 200 - And the response should be in JSON - And the header "Content-Type" should be equal to "application/ld+json; charset=utf-8" - And the JSON node "hydra:totalItems" should be equal to "1" - - @!postgres - @!mongodb - @!lowest - Scenario: Search collection by related binary UUID (Ramsey) (multiple values) - Given there is a ramsey identified resource with binary uuid "3248c908-a89d-483a-b75f-25888730d391" having a related resource with binary uuid "d7b2e909-37b0-411e-814c-74e044afbccb" - When I send a "GET" request to "/ramsey_uuid_binary_dummies?relateds[]=02227dc6-a371-4b8b-a34c-bbbf921b8ebd&relateds[]=d7b2e909-37b0-411e-814c-74e044afbccb" - Then the response status code should be 200 - And the response should be in JSON - And the header "Content-Type" should be equal to "application/ld+json; charset=utf-8" - And the JSON node "hydra:totalItems" should be equal to "2" - Scenario: Search for entities within an impossible range When I send a "GET" request to "/dummies?name=MuYm" Then the response status code should be 200 diff --git a/src/Bridge/Doctrine/Orm/Filter/SearchFilter.php b/src/Bridge/Doctrine/Orm/Filter/SearchFilter.php index 858406d421e..a33147652e6 100644 --- a/src/Bridge/Doctrine/Orm/Filter/SearchFilter.php +++ b/src/Bridge/Doctrine/Orm/Filter/SearchFilter.php @@ -21,10 +21,8 @@ use ApiPlatform\Core\Bridge\Doctrine\Orm\Util\QueryNameGeneratorInterface; use ApiPlatform\Core\Exception\InvalidArgumentException; use Doctrine\DBAL\Types\Type as DBALType; -use Doctrine\ORM\Query\Parameter; use Doctrine\ORM\QueryBuilder; use Doctrine\Persistence\ManagerRegistry; -use Doctrine\Persistence\Mapping\ClassMetadata; use Psr\Log\LoggerInterface; use Symfony\Component\HttpFoundation\RequestStack; use Symfony\Component\PropertyAccess\PropertyAccess; @@ -91,15 +89,6 @@ protected function filterProperty(string $property, $value, QueryBuilder $queryB [$alias, $field, $associations] = $this->addJoinsForNestedProperty($property, $alias, $queryBuilder, $queryNameGenerator, $resourceClass); } - $caseSensitive = true; - $strategy = $this->properties[$property] ?? self::STRATEGY_EXACT; - - // prefixing the strategy with i makes it case insensitive - if (0 === strpos($strategy, 'i')) { - $strategy = substr($strategy, 1); - $caseSensitive = false; - } - $metadata = $this->getNestedMetadata($resourceClass, $associations); if ($metadata->hasField($field)) { @@ -115,7 +104,16 @@ protected function filterProperty(string $property, $value, QueryBuilder $queryB return; } - $this->addWhereByStrategy($strategy, $queryBuilder, $queryNameGenerator, $alias, $field, $values, $caseSensitive, $metadata); + $caseSensitive = true; + $strategy = $this->properties[$property] ?? self::STRATEGY_EXACT; + + // prefixing the strategy with i makes it case insensitive + if (0 === strpos($strategy, 'i')) { + $strategy = substr($strategy, 1); + $caseSensitive = false; + } + + $this->addWhereByStrategy($strategy, $queryBuilder, $queryNameGenerator, $alias, $field, $values, $caseSensitive); return; } @@ -145,12 +143,21 @@ protected function filterProperty(string $property, $value, QueryBuilder $queryB $associationAlias = $alias; $associationField = $field; + $valueParameter = $queryNameGenerator->generateParameterName($associationField); if ($metadata->isCollectionValuedAssociation($associationField)) { $associationAlias = QueryBuilderHelper::addJoinOnce($queryBuilder, $queryNameGenerator, $alias, $associationField); $associationField = $associationFieldIdentifier; } - $this->addWhereByStrategy($strategy, $queryBuilder, $queryNameGenerator, $associationAlias, $associationField, $values, $caseSensitive, $metadata); + if (1 === \count($values)) { + $queryBuilder + ->andWhere($queryBuilder->expr()->eq($associationAlias.'.'.$associationField, ':'.$valueParameter)) + ->setParameter($valueParameter, $values[0]); + } else { + $queryBuilder + ->andWhere($queryBuilder->expr()->in($associationAlias.'.'.$associationField, ':'.$valueParameter)) + ->setParameter($valueParameter, $values); + } } /** @@ -158,15 +165,8 @@ 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, $values, bool $caseSensitive/*, ClassMetadata $metadata*/) + protected function addWhereByStrategy(string $strategy, QueryBuilder $queryBuilder, QueryNameGeneratorInterface $queryNameGenerator, string $alias, string $field, $values, bool $caseSensitive) { - if (\func_num_args() > 7 && ($metadata = func_get_arg(7)) instanceof ClassMetadata) { - $type = $metadata->getTypeOfField($field); - } else { - @trigger_error(sprintf('Method %s() will have a 8th argument `$metadata` in version API Platform 3.0.', __FUNCTION__), \E_USER_DEPRECATED); - $type = null; - } - if (!\is_array($values)) { $values = [$values]; } @@ -175,26 +175,18 @@ protected function addWhereByStrategy(string $strategy, QueryBuilder $queryBuild $valueParameter = ':'.$queryNameGenerator->generateParameterName($field); $aliasedField = sprintf('%s.%s', $alias, $field); - if (self::STRATEGY_EXACT === $strategy) { - if (1 === \count($values)) { + if (null == $strategy || self::STRATEGY_EXACT == $strategy) { + if (1 == \count($values)) { $queryBuilder ->andWhere($queryBuilder->expr()->eq($wrapCase($aliasedField), $wrapCase($valueParameter))) - ->setParameter($valueParameter, $values[0], $type); + ->setParameter($valueParameter, $values[0]); return; } - $parameters = $queryBuilder->getParameters(); - $inQuery = []; - foreach ($values as $value) { - $inQuery[] = $valueParameter; - $parameters->add(new Parameter($valueParameter, $caseSensitive ? $value : strtolower($value), $type)); - $valueParameter = ':'.$queryNameGenerator->generateParameterName($field); - } - $queryBuilder - ->andWhere($wrapCase($aliasedField).' IN ('.implode(', ', $inQuery).')') - ->setParameters($parameters); + ->andWhere($queryBuilder->expr()->in($wrapCase($aliasedField), $valueParameter)) + ->setParameter($valueParameter, $caseSensitive ? $values : array_map('strtolower', $values)); return; } @@ -236,7 +228,7 @@ protected function addWhereByStrategy(string $strategy, QueryBuilder $queryBuild } $queryBuilder->andWhere($queryBuilder->expr()->orX(...$ors)); - array_walk($parameters, [$queryBuilder, 'setParameter'], $type); + array_walk($parameters, [$queryBuilder, 'setParameter']); } /** diff --git a/tests/Behat/DoctrineContext.php b/tests/Behat/DoctrineContext.php index 5d1f3fd54ab..318ee695ec7 100644 --- a/tests/Behat/DoctrineContext.php +++ b/tests/Behat/DoctrineContext.php @@ -141,7 +141,6 @@ use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\Pet; use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\Product; use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\Question; -use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\RamseyUuidBinaryDummy; use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\RamseyUuidDummy; use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\RelatedDummy; use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\RelatedOwnedDummy; @@ -1344,35 +1343,6 @@ public function thereIsARamseyIdentifiedResource(string $uuid) $this->manager->flush(); } - /** - * @Given there is a ramsey identified resource with binary uuid :uuid - */ - public function thereIsARamseyIdentifiedResourceWithBinaryUuid(string $uuid) - { - $dummy = new RamseyUuidBinaryDummy(); - $dummy->setId($uuid); - - $this->manager->persist($dummy); - $this->manager->flush(); - } - - /** - * @Given there is a ramsey identified resource with binary uuid :uuid having a related resource with binary uuid :uuid_related - */ - public function thereIsARamseyIdentifiedResourceWithBinaryUuidHavingARelatedResourceWithBinaryUuid(string $uuid, string $uuidRelated) - { - $related = new RamseyUuidBinaryDummy(); - $related->setId($uuidRelated); - - $dummy = new RamseyUuidBinaryDummy(); - $dummy->setId($uuid); - $dummy->addRelated($related); - - $this->manager->persist($related); - $this->manager->persist($dummy); - $this->manager->flush(); - } - /** * @Given there is a dummy object with a fourth level relation */ diff --git a/tests/Bridge/Doctrine/Orm/Filter/SearchFilterTest.php b/tests/Bridge/Doctrine/Orm/Filter/SearchFilterTest.php index 3f74e66625b..e53463292cf 100644 --- a/tests/Bridge/Doctrine/Orm/Filter/SearchFilterTest.php +++ b/tests/Bridge/Doctrine/Orm/Filter/SearchFilterTest.php @@ -390,18 +390,22 @@ public function provideApplyTestData(): array $filterFactory, ], 'exact (multiple values)' => [ - sprintf('SELECT %s FROM %s %1$s WHERE %1$s.name IN (:name_p1, :name_p2)', $this->alias, Dummy::class), + sprintf('SELECT %s FROM %s %1$s WHERE %1$s.name IN(:name_p1)', $this->alias, Dummy::class), [ - 'name_p1' => 'CaSE', - 'name_p2' => 'SENSitive', + 'name_p1' => [ + 'CaSE', + 'SENSitive', + ], ], $filterFactory, ], 'exact (multiple values; case insensitive)' => [ - sprintf('SELECT %s FROM %s %1$s WHERE LOWER(%1$s.name) IN (:name_p1, :name_p2)', $this->alias, Dummy::class), + sprintf('SELECT %s FROM %s %1$s WHERE LOWER(%1$s.name) IN(:name_p1)', $this->alias, Dummy::class), [ - 'name_p1' => 'case', - 'name_p2' => 'insensitive', + 'name_p1' => [ + 'case', + 'insensitive', + ], ], $filterFactory, ], @@ -543,11 +547,10 @@ public function provideApplyTestData(): array $filterFactory, ], 'mixed IRI and entity ID values for relations' => [ - sprintf('SELECT %s FROM %s %1$s INNER JOIN %1$s.relatedDummies relatedDummies_a1 WHERE %1$s.relatedDummy IN (:relatedDummy_p1, :relatedDummy_p2) AND relatedDummies_a1.id = :id_p4', $this->alias, Dummy::class), + sprintf('SELECT %s FROM %s %1$s INNER JOIN %1$s.relatedDummies relatedDummies_a1 WHERE %1$s.relatedDummy IN(:relatedDummy_p1) AND relatedDummies_a1.id = :relatedDummies_p2', $this->alias, Dummy::class), [ - 'relatedDummy_p1' => 1, - 'relatedDummy_p2' => 2, - 'id_p4' => 1, + 'relatedDummy_p1' => [1, 2], + 'relatedDummies_p2' => 1, ], $filterFactory, ], diff --git a/tests/Fixtures/TestBundle/Entity/RamseyUuidBinaryDummy.php b/tests/Fixtures/TestBundle/Entity/RamseyUuidBinaryDummy.php deleted file mode 100644 index 5cede9f62b3..00000000000 --- a/tests/Fixtures/TestBundle/Entity/RamseyUuidBinaryDummy.php +++ /dev/null @@ -1,89 +0,0 @@ - - * - * 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\TestBundle\Entity; - -use ApiPlatform\Core\Annotation\ApiFilter; -use ApiPlatform\Core\Annotation\ApiResource; -use ApiPlatform\Core\Bridge\Doctrine\Orm\Filter\SearchFilter; -use Doctrine\Common\Collections\ArrayCollection; -use Doctrine\Common\Collections\Collection; -use Doctrine\ORM\Mapping as ORM; -use Ramsey\Uuid\Uuid; -use Ramsey\Uuid\UuidInterface; - -/** - * @ORM\Entity - * @ApiResource - * @ApiFilter(SearchFilter::class, properties={"id"="exact", "relateds"="exact"}) - */ -class RamseyUuidBinaryDummy -{ - /** - * @var UuidInterface - * - * @ORM\Id - * @ORM\Column(type="uuid_binary", unique=true) - */ - private $id; - - /** - * @var Collection - * - * @ORM\OneToMany(targetEntity="RamseyUuidBinaryDummy", mappedBy="relatedParent") - */ - private $relateds; - - /** - * @var ?RamseyUuidBinaryDummy - * - * @ORM\ManyToOne(targetEntity="RamseyUuidBinaryDummy", inversedBy="relateds") - */ - private $relatedParent; - - public function __construct() - { - $this->relateds = new ArrayCollection(); - } - - public function getId(): UuidInterface - { - return $this->id; - } - - public function setId(string $uuid): void - { - $this->id = Uuid::fromString($uuid); - } - - public function getRelateds(): Collection - { - return $this->relateds; - } - - public function addRelated(self $dummy): void - { - $this->relateds->add($dummy); - $dummy->setRelatedParent($this); - } - - public function getRelatedParent(): ?self - { - return $this->relatedParent; - } - - public function setRelatedParent(self $dummy): void - { - $this->relatedParent = $dummy; - } -} diff --git a/tests/Fixtures/app/config/config_common.yml b/tests/Fixtures/app/config/config_common.yml index 69db4cfb002..6b989cfa8c8 100644 --- a/tests/Fixtures/app/config/config_common.yml +++ b/tests/Fixtures/app/config/config_common.yml @@ -8,8 +8,7 @@ doctrine: path: '%kernel.cache_dir%/db.sqlite' charset: 'UTF8' types: - uuid: Ramsey\Uuid\Doctrine\UuidType - uuid_binary: Ramsey\Uuid\Doctrine\UuidBinaryType + uuid: Ramsey\Uuid\Doctrine\UuidType orm: auto_generate_proxy_classes: '%kernel.debug%' diff --git a/tests/Fixtures/app/config/config_mysql.yml b/tests/Fixtures/app/config/config_mysql.yml index e69bf6b474a..d83e9650176 100644 --- a/tests/Fixtures/app/config/config_mysql.yml +++ b/tests/Fixtures/app/config/config_mysql.yml @@ -13,4 +13,3 @@ doctrine: server_version: '%env(MYSQL_VERSION)%' types: uuid: Ramsey\Uuid\Doctrine\UuidType - uuid_binary: Ramsey\Uuid\Doctrine\UuidBinaryType diff --git a/tests/Fixtures/app/config/config_postgres.yml b/tests/Fixtures/app/config/config_postgres.yml index 54928efaa0f..20c21168dd2 100644 --- a/tests/Fixtures/app/config/config_postgres.yml +++ b/tests/Fixtures/app/config/config_postgres.yml @@ -13,4 +13,3 @@ doctrine: server_version: '%env(POSTGRES_VERSION)%' types: uuid: Ramsey\Uuid\Doctrine\UuidType - uuid_binary: Ramsey\Uuid\Doctrine\UuidBinaryType diff --git a/tests/Fixtures/app/config/config_sqlite.yml b/tests/Fixtures/app/config/config_sqlite.yml index 55707b7c681..1407de645d4 100644 --- a/tests/Fixtures/app/config/config_sqlite.yml +++ b/tests/Fixtures/app/config/config_sqlite.yml @@ -10,4 +10,3 @@ doctrine: url: '%env(resolve:DATABASE_URL)%' types: uuid: Ramsey\Uuid\Doctrine\UuidType - uuid_binary: Ramsey\Uuid\Doctrine\UuidBinaryType