Skip to content

Commit eb72ec5

Browse files
soyukasoyuka
committed
fix(filter): relocate Operation::$filters deprecation
Move the 4.4 Operation::$filters deprecation out of OpenApiFactory into FiltersResourceMetadataCollectionFactory so it fires once at metadata build for every consumer, not only during OpenAPI generation; rebuild the deprecation baseline accordingly. Move DummyExceptionToStatus to Entity/Legacy/ with #[ApiFilter] instead of the QueryParameter form, which routed missing-required-param validation through the 422 constraint path (#8211) and broke the exceptionToStatus 400/404 contract the test guards. Bump the OpenApiTest foobar[] parameter index and extend the PropertyFilter OpenAPI example with the nested-property form.
1 parent 343a3cf commit eb72ec5

9 files changed

Lines changed: 65 additions & 219 deletions

File tree

‎phpunit.baseline.xml‎

Lines changed: 35 additions & 33 deletions
Large diffs are not rendered by default.

‎src/Metadata/Resource/Factory/FiltersResourceMetadataCollectionFactory.php‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,12 @@ public function create(string $resourceClass): ResourceMetadataCollection
5858

5959
foreach ($resourceMetadataCollection as $i => $resource) {
6060
foreach ($operations = $resource->getOperations() ?? [] as $operationName => $operation) {
61-
$operations->add($operationName, $operation->withFilters(array_unique(array_merge($resource->getFilters() ?? [], $operation->getFilters() ?? [], $filters))));
61+
$operationFilters = array_unique(array_merge($resource->getFilters() ?? [], $operation->getFilters() ?? [], $filters));
62+
if ($operationFilters) {
63+
trigger_deprecation('api-platform/core', '4.4', \sprintf('Declaring filters on the "%s" operation through "Operation::$filters" is deprecated, use the "parameters" argument instead. It will be removed in 6.0.', $operation->getShortName()));
64+
}
65+
66+
$operations->add($operationName, $operation->withFilters($operationFilters));
6267
}
6368

6469
if ($operations) {

‎src/OpenApi/Factory/OpenApiFactory.php‎

Lines changed: 5 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@
2929
use ApiPlatform\Metadata\HttpOperation;
3030
use ApiPlatform\Metadata\Property\Factory\PropertyMetadataFactoryInterface;
3131
use ApiPlatform\Metadata\Property\Factory\PropertyNameCollectionFactoryInterface;
32-
use ApiPlatform\Metadata\QueryParameter;
3332
use ApiPlatform\Metadata\Resource\Factory\ResourceMetadataCollectionFactoryInterface;
3433
use ApiPlatform\Metadata\Resource\Factory\ResourceNameCollectionFactoryInterface;
3534
use ApiPlatform\Metadata\Resource\ResourceMetadataCollection;
@@ -702,38 +701,27 @@ private function getFiltersParameters(CollectionOperationInterface|HttpOperation
702701
$parameters = [];
703702
$resourceFilters = $operation->getFilters();
704703
$entityClass = $this->getStateOptionsClass($operation, $operation->getClass());
705-
706-
if ($resourceFilters) {
707-
trigger_deprecation('api-platform/core', '4.4', \sprintf('Declaring filters on the "%s" operation through "Operation::$filters" is deprecated, use the "%s" attribute instead. It will be removed in 6.0.', $operation->getShortName(), QueryParameter::class));
708-
}
709-
710-
// Collapse identical filter deprecations to one warning per (filter, shortName) per
711-
// generation: a filter exposing N parameters must not emit the same message N times.
712-
$warned = [];
713704
foreach ($resourceFilters ?? [] as $filterId) {
714705
if (!$this->filterLocator->has($filterId)) {
715706
continue;
716707
}
717708

718709
$filter = $this->filterLocator->get($filterId);
719710
foreach ($filter->getDescription($entityClass) as $name => $description) {
720-
$parameters[] = $this->getFilterParameter($name, $description, $operation->getShortName(), $filterId, $warned);
711+
$parameters[] = $this->getFilterParameter($name, $description, $operation->getShortName(), $filterId);
721712
}
722713
}
723714

724715
return $parameters;
725716
}
726717

727718
/**
728-
* @param array<string, mixed> $description
729-
* @param array<string, array<string, bool>> $warned
719+
* @param array<string, mixed> $description
730720
*/
731-
private function getFilterParameter(string $name, array $description, string $shortName, string $filter, array &$warned = []): Parameter
721+
private function getFilterParameter(string $name, array $description, string $shortName, string $filter): Parameter
732722
{
733-
$warnKey = $filter.'::'.$shortName;
734-
if (isset($description['swagger']) && !isset($warned[$warnKey]['swagger'])) {
723+
if (isset($description['swagger'])) {
735724
trigger_deprecation('api-platform/core', '4.0', \sprintf('Using the "swagger" field of the %s::getDescription() (%s) is deprecated.', $filter, $shortName));
736-
$warned[$warnKey]['swagger'] = true;
737725
}
738726

739727
if (!isset($description['openapi']) || $description['openapi'] instanceof Parameter) {
@@ -782,10 +770,7 @@ private function getFilterParameter(string $name, array $description, string $sh
782770
return $parameter->withSchema($schema);
783771
}
784772

785-
if (!isset($warned[$warnKey]['openapi'])) {
786-
trigger_deprecation('api-platform/core', '4.0', \sprintf('Not using "%s" on the "openapi" field of the %s::getDescription() (%s) is deprecated.', Parameter::class, $filter, $shortName));
787-
$warned[$warnKey]['openapi'] = true;
788-
}
773+
trigger_deprecation('api-platform/core', '4.0', \sprintf('Not using "%s" on the "openapi" field of the %s::getDescription() (%s) is deprecated.', Parameter::class, $filter, $shortName));
789774

790775
$schema = $description['schema'] ?? null;
791776

‎src/OpenApi/Tests/Factory/OpenApiFactoryTest.php‎

Lines changed: 0 additions & 154 deletions
Original file line numberDiff line numberDiff line change
@@ -1498,158 +1498,4 @@ public function testMetadataParameterInOpenApiOperationParametersThrows(): void
14981498

14991499
$factory->__invoke();
15001500
}
1501-
1502-
public function testLegacyFilterDeprecationFiresOncePerFilterAndShortName(): void
1503-
{
1504-
$resourceNameCollectionFactory = $this->createMock(ResourceNameCollectionFactoryInterface::class);
1505-
$resourceCollectionMetadataFactory = $this->createMock(ResourceMetadataCollectionFactoryInterface::class);
1506-
$propertyNameCollectionFactory = $this->createMock(PropertyNameCollectionFactoryInterface::class);
1507-
$propertyMetadataFactory = $this->createMock(PropertyMetadataFactoryInterface::class);
1508-
$definitionNameFactory = new DefinitionNameFactory([]);
1509-
1510-
$resourceCollectionMetadata = new ResourceMetadataCollection(Dummy::class, [(new ApiResource(operations: [
1511-
(new GetCollection())
1512-
->withClass(Dummy::class)
1513-
->withShortName('Dummy')
1514-
->withName('api_dummies_get_collection')
1515-
->withUriTemplate('/dummies')
1516-
->withFilters(['legacy_filter']),
1517-
]))->withClass(Dummy::class)]);
1518-
1519-
$resourceCollectionMetadataFactory
1520-
->method('create')
1521-
->willReturnCallback(static fn (string $resourceClass): ResourceMetadataCollection => match ($resourceClass) {
1522-
Dummy::class => $resourceCollectionMetadata,
1523-
default => new ResourceMetadataCollection($resourceClass, []),
1524-
});
1525-
1526-
$resourceNameCollectionFactory->method('create')->willReturn(new ResourceNameCollection([Dummy::class]));
1527-
$propertyNameCollectionFactory->method('create')->willReturn(new PropertyNameCollection([]));
1528-
1529-
// A legacy filter exposing several parameters, each carrying the deprecated "swagger" field.
1530-
$filter = new DummyFilter([
1531-
'name' => ['property' => 'name', 'type' => 'string', 'required' => false, 'swagger' => ['type' => 'string']],
1532-
'foo' => ['property' => 'foo', 'type' => 'string', 'required' => false, 'swagger' => ['type' => 'string']],
1533-
'bar' => ['property' => 'bar', 'type' => 'string', 'required' => false, 'swagger' => ['type' => 'string']],
1534-
]);
1535-
1536-
$filterLocator = $this->createMock(ContainerInterface::class);
1537-
$filterLocator->method('has')->willReturnCallback(static fn (string $id): bool => 'legacy_filter' === $id);
1538-
$filterLocator->method('get')->willReturn($filter);
1539-
1540-
$schemaFactory = new SchemaFactory(
1541-
resourceMetadataFactory: $resourceCollectionMetadataFactory,
1542-
propertyNameCollectionFactory: $propertyNameCollectionFactory,
1543-
propertyMetadataFactory: $propertyMetadataFactory,
1544-
nameConverter: new CamelCaseToSnakeCaseNameConverter(),
1545-
definitionNameFactory: $definitionNameFactory,
1546-
);
1547-
1548-
$factory = new OpenApiFactory(
1549-
$resourceNameCollectionFactory,
1550-
$resourceCollectionMetadataFactory,
1551-
$propertyNameCollectionFactory,
1552-
$propertyMetadataFactory,
1553-
$schemaFactory,
1554-
$filterLocator,
1555-
[],
1556-
new Options('Test API', 'This is a test API.', '1.2.3'),
1557-
new PaginationOptions(),
1558-
null,
1559-
['json' => ['application/problem+json']]
1560-
);
1561-
1562-
$deprecations = [];
1563-
set_error_handler(static function (int $type, string $message) use (&$deprecations): bool {
1564-
$deprecations[] = $message;
1565-
1566-
return true;
1567-
}, \E_USER_DEPRECATED);
1568-
1569-
try {
1570-
$factory->__invoke();
1571-
} finally {
1572-
restore_error_handler();
1573-
}
1574-
1575-
$swaggerDeprecations = array_filter($deprecations, static fn (string $m): bool => str_contains($m, 'Using the "swagger" field'));
1576-
$this->assertCount(1, $swaggerDeprecations, 'The "swagger" deprecation must fire once per filter and short name, not once per filter parameter.');
1577-
}
1578-
1579-
public function testOperationFiltersDeprecation(): void
1580-
{
1581-
$resourceNameCollectionFactory = $this->createMock(ResourceNameCollectionFactoryInterface::class);
1582-
$resourceCollectionMetadataFactory = $this->createMock(ResourceMetadataCollectionFactoryInterface::class);
1583-
$propertyNameCollectionFactory = $this->createMock(PropertyNameCollectionFactoryInterface::class);
1584-
$propertyMetadataFactory = $this->createMock(PropertyMetadataFactoryInterface::class);
1585-
$definitionNameFactory = new DefinitionNameFactory([]);
1586-
1587-
$resourceCollectionMetadata = new ResourceMetadataCollection(Dummy::class, [(new ApiResource(operations: [
1588-
(new GetCollection())
1589-
->withClass(Dummy::class)
1590-
->withShortName('Dummy')
1591-
->withName('api_dummies_get_collection')
1592-
->withUriTemplate('/dummies')
1593-
->withFilters(['legacy_filter']),
1594-
]))->withClass(Dummy::class)]);
1595-
1596-
$resourceCollectionMetadataFactory
1597-
->method('create')
1598-
->willReturnCallback(static fn (string $resourceClass): ResourceMetadataCollection => match ($resourceClass) {
1599-
Dummy::class => $resourceCollectionMetadata,
1600-
default => new ResourceMetadataCollection($resourceClass, []),
1601-
});
1602-
1603-
$resourceNameCollectionFactory->method('create')->willReturn(new ResourceNameCollection([Dummy::class]));
1604-
$propertyNameCollectionFactory->method('create')->willReturn(new PropertyNameCollection([]));
1605-
1606-
// A legacy filter declared through Operation::$filters, with a well-formed description.
1607-
$filter = new DummyFilter([
1608-
'name' => ['property' => 'name', 'type' => 'string', 'required' => false],
1609-
'foo' => ['property' => 'foo', 'type' => 'string', 'required' => false],
1610-
]);
1611-
1612-
$filterLocator = $this->createMock(ContainerInterface::class);
1613-
$filterLocator->method('has')->willReturnCallback(static fn (string $id): bool => 'legacy_filter' === $id);
1614-
$filterLocator->method('get')->willReturn($filter);
1615-
1616-
$schemaFactory = new SchemaFactory(
1617-
resourceMetadataFactory: $resourceCollectionMetadataFactory,
1618-
propertyNameCollectionFactory: $propertyNameCollectionFactory,
1619-
propertyMetadataFactory: $propertyMetadataFactory,
1620-
nameConverter: new CamelCaseToSnakeCaseNameConverter(),
1621-
definitionNameFactory: $definitionNameFactory,
1622-
);
1623-
1624-
$factory = new OpenApiFactory(
1625-
$resourceNameCollectionFactory,
1626-
$resourceCollectionMetadataFactory,
1627-
$propertyNameCollectionFactory,
1628-
$propertyMetadataFactory,
1629-
$schemaFactory,
1630-
$filterLocator,
1631-
[],
1632-
new Options('Test API', 'This is a test API.', '1.2.3'),
1633-
new PaginationOptions(),
1634-
null,
1635-
['json' => ['application/problem+json']]
1636-
);
1637-
1638-
$deprecations = [];
1639-
set_error_handler(static function (int $type, string $message) use (&$deprecations): bool {
1640-
$deprecations[] = $message;
1641-
1642-
return true;
1643-
}, \E_USER_DEPRECATED);
1644-
1645-
try {
1646-
$factory->__invoke();
1647-
} finally {
1648-
restore_error_handler();
1649-
}
1650-
1651-
$filtersDeprecations = array_values(array_filter($deprecations, static fn (string $m): bool => str_contains($m, 'Operation::$filters')));
1652-
$this->assertCount(1, $filtersDeprecations, 'Declaring filters through Operation::$filters must trigger one deprecation per operation.');
1653-
$this->assertStringContainsString('QueryParameter', $filtersDeprecations[0]);
1654-
}
16551501
}

‎src/Serializer/Filter/PropertyFilter.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -280,7 +280,7 @@ public function getSchema(MetadataParameter $parameter): array
280280
public function getOpenApiParameters(MetadataParameter $parameter): Parameter
281281
{
282282
$example = \sprintf(
283-
'%1$s[]={propertyName}&%1$s[]={anotherPropertyName}',
283+
'%1$s[]={propertyName}&%1$s[]={anotherPropertyName}&%1$s[{nestedPropertyParent}][]={nestedProperty}',
284284
$parameter->getKey()
285285
);
286286

‎src/Symfony/Bundle/Command/UpgradeApiFilterCommand.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,8 @@
4242
* Only `#[ApiFilter]`-generated filters (service ids prefixed `annotated_`) are migrated.
4343
* Resources whose filters cannot be expressed as distinct QueryParameters (e.g. an exact and a
4444
* range filter on the same property) are reported and skipped.
45+
*
46+
* This command is a one-shot upgrade helper for the 4.4 → 5.0 filter migration and will be removed in 6.0.
4547
*/
4648
#[AsCommand(name: 'api:upgrade-filter', description: 'Upgrades legacy #[ApiFilter] declarations to QueryParameter')]
4749
final class UpgradeApiFilterCommand extends Command

tests/Fixtures/TestBundle/Entity/DummyExceptionToStatus.php renamed to tests/Fixtures/TestBundle/Entity/Legacy/DummyExceptionToStatus.php

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,24 +11,30 @@
1111

1212
declare(strict_types=1);
1313

14-
namespace ApiPlatform\Tests\Fixtures\TestBundle\Entity;
14+
namespace ApiPlatform\Tests\Fixtures\TestBundle\Entity\Legacy;
1515

16+
use ApiPlatform\Metadata\ApiFilter;
1617
use ApiPlatform\Metadata\ApiResource;
1718
use ApiPlatform\Metadata\Get;
1819
use ApiPlatform\Metadata\GetCollection;
1920
use ApiPlatform\Metadata\Put;
20-
use ApiPlatform\Metadata\QueryParameter;
2121
use ApiPlatform\Tests\Fixtures\TestBundle\Exception\NotFoundException;
2222
use ApiPlatform\Tests\Fixtures\TestBundle\Filter\RequiredFilter;
2323
use ApiPlatform\Tests\Fixtures\TestBundle\State\DummyExceptionToStatusProvider;
2424
use Doctrine\ORM\Mapping as ORM;
2525
use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
2626

27-
#[ApiResource(exceptionToStatus: [NotFoundHttpException::class => 400], provider: DummyExceptionToStatusProvider::class, operations: [
28-
new Get(uriTemplate: '/dummy_exception_to_statuses/{id}', exceptionToStatus: [NotFoundException::class => 404]),
29-
new Put(uriTemplate: '/dummy_exception_to_statuses/{id}'),
30-
new GetCollection(uriTemplate: '/dummy_exception_to_statuses'),
31-
], graphQlOperations: [], parameters: ['required' => new QueryParameter(filter: new RequiredFilter()), 'not-required' => new QueryParameter(filter: new RequiredFilter())])]
27+
#[ApiResource(
28+
exceptionToStatus: [NotFoundHttpException::class => 400],
29+
provider: DummyExceptionToStatusProvider::class,
30+
operations: [
31+
new Get(uriTemplate: '/dummy_exception_to_statuses/{id}', exceptionToStatus: [NotFoundException::class => 404]),
32+
new Put(uriTemplate: '/dummy_exception_to_statuses/{id}'),
33+
new GetCollection(uriTemplate: '/dummy_exception_to_statuses'),
34+
],
35+
graphQlOperations: []
36+
)]
37+
#[ApiFilter(RequiredFilter::class)]
3238
#[ORM\Entity]
3339
class DummyExceptionToStatus
3440
{

‎tests/Functional/ExceptionToStatusTest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
use ApiPlatform\Symfony\Bundle\Test\ApiTestCase;
1717
use ApiPlatform\Tests\Fixtures\TestBundle\ApiResource\ErrorWithOverridenStatus;
1818
use ApiPlatform\Tests\Fixtures\TestBundle\ApiResource\Issue5924\TooManyRequests;
19-
use ApiPlatform\Tests\Fixtures\TestBundle\Entity\DummyExceptionToStatus;
19+
use ApiPlatform\Tests\Fixtures\TestBundle\Entity\Legacy\DummyExceptionToStatus;
2020
use ApiPlatform\Tests\RecreateSchemaTrait;
2121
use ApiPlatform\Tests\SetupClassResourcesTrait;
2222

‎tests/Functional/OpenApiTest.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -424,8 +424,8 @@ public function testRetrieveTheOpenApiDocumentation(): void
424424
$this->assertFalse($json['paths']['/dummies']['get']['parameters'][4]['required']);
425425
$this->assertSame('boolean', $json['paths']['/dummies']['get']['parameters'][4]['schema']['type']);
426426

427-
$this->assertSame('foobar[]', $json['paths']['/dummy_cars']['get']['parameters'][9]['name']);
428-
$this->assertSame('Allows you to reduce the response to contain only the properties you need. If your desired property is nested, you can address it using nested arrays. Example: foobar[]={propertyName}&foobar[]={anotherPropertyName}&foobar[{nestedPropertyParent}][]={nestedProperty}', $json['paths']['/dummy_cars']['get']['parameters'][9]['description']);
427+
$this->assertSame('foobar[]', $json['paths']['/dummy_cars']['get']['parameters'][10]['name']);
428+
$this->assertSame('Allows you to reduce the response to contain only the properties you need. If your desired property is nested, you can address it using nested arrays. Example: foobar[]={propertyName}&foobar[]={anotherPropertyName}&foobar[{nestedPropertyParent}][]={nestedProperty}', $json['paths']['/dummy_cars']['get']['parameters'][10]['description']);
429429

430430
// Webhook
431431
$this->assertSame('Something else here for example', $json['webhooks']['a/{id}']['get']['description']);

0 commit comments

Comments
 (0)