Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SearchFilter: fix regression in 2.6.3 #4134

Merged
merged 5 commits into from
Mar 16, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
3 changes: 1 addition & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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)
soyuka marked this conversation as resolved.
Show resolved Hide resolved
* 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)
Expand Down
2 changes: 1 addition & 1 deletion behat.yml.dist
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ postgres:
- 'Behat\MinkExtension\Context\MinkContext'
- 'behatch:context:rest'
filters:
tags: '~@sqlite&&~@mongodb&&~@elasticsearch&&~@!postgres'
tags: '~@sqlite&&~@mongodb&&~@elasticsearch'

mongodb:
suites:
Expand Down
54 changes: 8 additions & 46 deletions features/doctrine/search_filter.feature
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
33 changes: 8 additions & 25 deletions src/Bridge/Doctrine/Orm/Filter/SearchFilter.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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];
}
Expand All @@ -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;
}
Expand Down Expand Up @@ -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']);
}

/**
Expand Down
30 changes: 0 additions & 30 deletions tests/Behat/DoctrineContext.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
*/
Expand Down
23 changes: 13 additions & 10 deletions tests/Bridge/Doctrine/Orm/Filter/SearchFilterTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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,
],
Expand Down Expand Up @@ -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,
],
Expand Down
2 changes: 2 additions & 0 deletions tests/Fixtures/TestBundle/Document/DummyDate.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand All @@ -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,
Expand Down
2 changes: 2 additions & 0 deletions tests/Fixtures/TestBundle/Entity/DummyDate.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand All @@ -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,
Expand Down
89 changes: 0 additions & 89 deletions tests/Fixtures/TestBundle/Entity/RamseyUuidBinaryDummy.php

This file was deleted.

Loading