Revert "Handle binary UUID in SearchFilter (api-platform#3774)"

This reverts commit ff248ae.
soyuka · Mar 16, 2021 · b29548d · b29548d
1 parent f193281
commit b29548d
Show file tree

Hide file tree

Showing 12 changed files with 43 additions and 218 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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,28 +143,30 @@ 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);
+        }
     }
 
     /**
      * Adds where clause according to the strategy.
      *
      * @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
@@ -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
@@ -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
diff --git 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
@@ -13,4 +13,3 @@ doctrine:
         server_version: '%env(MYSQL_VERSION)%'
         types:
             uuid: Ramsey\Uuid\Doctrine\UuidType
-            uuid_binary: Ramsey\Uuid\Doctrine\UuidBinaryType