SearchFilter: fix regression in 2.6.3 (#4134)

* Revert "Handle binary UUID in SearchFilter (#3774)" This reverts commit ff248ae. Co-authored-by: Alan Poulain <contact@alanpoulain.eu>
api-platform · Mar 16, 2021 · e74bf8e · e74bf8e
1 parent f193281
commit e74bf8e
Show file tree

Hide file tree

Showing 14 changed files with 37 additions and 209 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
@@ -5,6 +5,7 @@
 * Doctrine: Fix purging HTTP cache for unreadable relations (#3441)
 * GraphQL: Manage `pagination_use_output_walkers` and `pagination_fetch_join_collection` for operations (#3311)
 * Swagger UI: Remove Google fonts (#4112)
+* Doctrine: Revert #3774 support for binary UUID in search filter (#4134) 
 
 ## 2.6.3
 
@@ -13,9 +14,7 @@
 * 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)
 * OpenAPI: Fix missing 422 responses in the documentation (#4086)
 * OpenAPI: Fix error when schema is empty (#4051)

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
@@ -1039,3 +993,11 @@ Feature: Search filter on collections
       }
     }
     """
+
+  @createSchema
+  Scenario: Search by date (#4128)
+    Given there are 3 dummydate objects with dummyDate
+    When I send a "GET" request to "/dummy_dates?dummyDate=2015-04-01"
+    Then the response status code should be 200
+    And the response should be in JSON
+    And the JSON node "hydra:totalItems" should be equal to 1
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;
@@ -115,7 +113,7 @@ protected function filterProperty(string $property, $value, QueryBuilder $queryB
                 return;
             }
 
-            $this->addWhereByStrategy($strategy, $queryBuilder, $queryNameGenerator, $alias, $field, $values, $caseSensitive, $metadata);
+            $this->addWhereByStrategy($strategy, $queryBuilder, $queryNameGenerator, $alias, $field, $values, $caseSensitive);
 
             return;
         }
@@ -150,23 +148,16 @@ protected function filterProperty(string $property, $value, QueryBuilder $queryB
             $associationField = $associationFieldIdentifier;
         }
 
-        $this->addWhereByStrategy($strategy, $queryBuilder, $queryNameGenerator, $associationAlias, $associationField, $values, $caseSensitive, $metadata);
+        $this->addWhereByStrategy($strategy, $queryBuilder, $queryNameGenerator, $associationAlias, $associationField, $values, $caseSensitive);
     }
 
     /**
      * 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 +166,18 @@ protected function addWhereByStrategy(string $strategy, QueryBuilder $queryBuild
         $valueParameter = ':'.$queryNameGenerator->generateParameterName($field);
         $aliasedField = sprintf('%s.%s', $alias, $field);
 
-        if (self::STRATEGY_EXACT === $strategy) {
+        if (!$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 +219,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 = :id_p2', $this->alias, Dummy::class),
                     [
-                        'relatedDummy_p1' => 1,
-                        'relatedDummy_p2' => 2,
-                        'id_p4' => 1,
+                        'relatedDummy_p1' => [1, 2],
+                        'id_p2' => 1,
                     ],
                     $filterFactory,
                 ],

diff --git a/tests/Fixtures/TestBundle/Document/DummyDate.php b/tests/Fixtures/TestBundle/Document/DummyDate.php
@@ -16,6 +16,7 @@
 use ApiPlatform\Core\Annotation\ApiFilter;
 use ApiPlatform\Core\Annotation\ApiResource;
 use ApiPlatform\Core\Bridge\Doctrine\MongoDbOdm\Filter\DateFilter;
+use ApiPlatform\Core\Bridge\Doctrine\MongoDbOdm\Filter\SearchFilter;
 use Doctrine\ODM\MongoDB\Mapping\Annotations as ODM;
 
 /**
@@ -27,6 +28,7 @@
  * @ApiResource(attributes={
  *     "filters"={"my_dummy_date.mongodb.date"}
  * })
+ * @ApiFilter(SearchFilter::class, properties={"dummyDate"})
  * @ApiFilter(DateFilter::class, properties={
  *     "dateIncludeNullAfter"=DateFilter::INCLUDE_NULL_AFTER,
  *     "dateIncludeNullBefore"=DateFilter::INCLUDE_NULL_BEFORE,

diff --git a/tests/Fixtures/TestBundle/Entity/DummyDate.php b/tests/Fixtures/TestBundle/Entity/DummyDate.php
@@ -16,6 +16,7 @@
 use ApiPlatform\Core\Annotation\ApiFilter;
 use ApiPlatform\Core\Annotation\ApiResource;
 use ApiPlatform\Core\Bridge\Doctrine\Orm\Filter\DateFilter;
+use ApiPlatform\Core\Bridge\Doctrine\Orm\Filter\SearchFilter;
 use Doctrine\ORM\Mapping as ORM;
 
 /**
@@ -26,6 +27,7 @@
  * @ApiResource(attributes={
  *     "filters"={"my_dummy_date.date"}
  * })
+ * @ApiFilter(SearchFilter::class, properties={"dummyDate"})
  * @ApiFilter(DateFilter::class, properties={
  *     "dateIncludeNullAfter"=DateFilter::INCLUDE_NULL_AFTER,
  *     "dateIncludeNullBefore"=DateFilter::INCLUDE_NULL_BEFORE,

diff --git a/tests/Fixtures/TestBundle/Entity/RamseyUuidBinaryDummy.php b/tests/Fixtures/TestBundle/Entity/RamseyUuidBinaryDummy.php