Skip to content

Commit 859c3e5

Browse files
soyukasoyuka
committed
refactor(metadata): emit Operation::$filters deprecation in metadata
Per review, OpenApiFactory must not emit the Operation::$filters deprecation. Revert the factory to its prior state (no F6 trigger, no per-(filter,shortName) dedup) and emit the deprecation once while building metadata in FiltersResourceMetadataCollectionFactory, worded to point at the "parameters" argument. Drop the two now-obsolete OpenApiFactory deprecation tests. ValueCaster no longer casts the literal "null" string, which is not a supported filter input; only empty values cast to the type default (false/0/0.0). Update the ValueCaster, boolean and numeric tests. Rebuild the deprecation baseline for the relocated trigger.
1 parent af5bb9d commit 859c3e5

8 files changed

Lines changed: 76 additions & 238 deletions

File tree

‎phpunit.baseline.xml‎

Lines changed: 43 additions & 34 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 & 156 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,6 @@
6767
use ApiPlatform\State\Pagination\PaginationOptions;
6868
use ApiPlatform\Tests\Fixtures\TestBundle\ApiResource\WithParameter;
6969
use ApiPlatform\Validator\Exception\ValidationException;
70-
use PHPUnit\Framework\Attributes\IgnoreDeprecations;
7170
use PHPUnit\Framework\TestCase;
7271
use Prophecy\Argument;
7372
use Prophecy\PhpUnit\ProphecyTrait;
@@ -84,7 +83,6 @@ class OpenApiFactoryTest extends TestCase
8483
'output_formats' => ['jsonld' => ['application/ld+json']],
8584
];
8685

87-
#[IgnoreDeprecations]
8886
public function testInvoke(): void
8987
{
9088
$baseOperation = (new HttpOperation())->withTypes(['http://schema.example.com/Dummy'])->withInputFormats(self::OPERATION_FORMATS['input_formats'])->withOutputFormats(self::OPERATION_FORMATS['output_formats'])->withClass(Dummy::class)->withOutput([
@@ -1500,158 +1498,4 @@ public function testMetadataParameterInOpenApiOperationParametersThrows(): void
15001498

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

‎src/State/Parameter/ValueCaster.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ public static function toBool(mixed $v): mixed
3030

3131
return match (strtolower($v)) {
3232
'1', 'true' => true,
33-
'0', 'false', '', 'null' => false,
33+
'0', 'false', '' => false,
3434
default => $v,
3535
};
3636
}
@@ -41,7 +41,7 @@ public static function toInt(mixed $v): mixed
4141
return $v;
4242
}
4343

44-
if (\is_string($v) && ('' === $v || 'null' === strtolower($v))) {
44+
if ('' === $v) {
4545
return 0;
4646
}
4747

@@ -56,7 +56,7 @@ public static function toFloat(mixed $v): mixed
5656
return $v;
5757
}
5858

59-
if (\is_string($v) && ('' === $v || 'null' === strtolower($v))) {
59+
if ('' === $v) {
6060
return 0.0;
6161
}
6262

‎src/State/Tests/Parameter/ValueCasterTest.php‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,8 @@ public static function intProvider(): \Generator
3535
{
3636
yield 'integer string' => ['10', 10];
3737
yield 'empty string' => ['', 0];
38-
yield 'null string' => ['null', 0];
3938
yield 'invalid string' => ['string', 'string'];
39+
yield 'null string is not cast' => ['null', 'null'];
4040
yield 'int passthrough' => [10, 10];
4141
}
4242

@@ -50,8 +50,8 @@ public static function floatProvider(): \Generator
5050
{
5151
yield 'float string' => ['1.5', 1.5];
5252
yield 'empty string' => ['', 0.0];
53-
yield 'null string' => ['null', 0.0];
5453
yield 'invalid string' => ['string', 'string'];
54+
yield 'null string is not cast' => ['null', 'null'];
5555
yield 'float passthrough' => [1.5, 1.5];
5656
}
5757

@@ -61,12 +61,13 @@ public static function boolProvider(): \Generator
6161
yield 'numeric 1' => ['1', true];
6262
yield 'false string' => ['false', false];
6363
yield 'numeric 0' => ['0', false];
64-
// Empty and "null" cast to false deterministically across drivers, instead of
64+
// An empty value casts to false deterministically across drivers, instead of
6565
// reaching the database as a raw string that each driver coerces differently.
6666
yield 'empty string' => ['', false];
67-
yield 'null string' => ['null', false];
68-
// A genuinely invalid value is returned untouched so constraint validation rejects it.
67+
// Genuinely invalid values (including "null") are returned untouched so constraint
68+
// validation rejects them; "null" is not a supported boolean input.
6969
yield 'invalid string' => ['string', 'string'];
70+
yield 'null string is not cast' => ['null', 'null'];
7071
yield 'non-string passthrough' => [true, true];
7172
}
7273
}

‎tests/Functional/Parameters/BooleanFilterTest.php‎

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -77,13 +77,12 @@ public static function booleanFilterScenariosProvider(): \Generator
7777
}
7878

7979
/**
80-
* With the canonical ExactFilter, an empty or unparseable boolean value casts to false
81-
* (deterministically across drivers), so it matches the inactive rows — unlike the
82-
* deprecated BooleanFilter which skipped filtering (returning all rows, covered by the
83-
* legacy regression suite).
80+
* With the canonical ExactFilter, an empty boolean value casts to false (deterministically
81+
* across drivers), so it matches the inactive rows — unlike the deprecated BooleanFilter
82+
* which skipped filtering (returning all rows, covered by the legacy regression suite).
8483
*/
85-
#[DataProvider('booleanFilterNullAndEmptyScenariosProvider')]
86-
public function testBooleanFilterWithNullAndEmptyValues(string $url): void
84+
#[DataProvider('booleanFilterEmptyScenariosProvider')]
85+
public function testBooleanFilterWithEmptyValues(string $url): void
8786
{
8887
$response = self::createClient()->request('GET', $url);
8988
$this->assertResponseIsSuccessful();
@@ -94,15 +93,13 @@ public function testBooleanFilterWithNullAndEmptyValues(string $url): void
9493
$this->assertCount(1, $filteredItems, \sprintf('Expected one inactive item for URL %s', $url));
9594

9695
foreach ($filteredItems as $item) {
97-
$this->assertFalse($item['active'], 'Expected an empty/null boolean value to cast to false');
96+
$this->assertFalse($item['active'], 'Expected an empty boolean value to cast to false');
9897
}
9998
}
10099

101-
public static function booleanFilterNullAndEmptyScenariosProvider(): \Generator
100+
public static function booleanFilterEmptyScenariosProvider(): \Generator
102101
{
103-
yield 'active_null_value' => ['/filtered_boolean_parameters?active=null'];
104102
yield 'active_empty_value' => ['/filtered_boolean_parameters?active='];
105-
yield 'enabled_alias_null_value' => ['/filtered_boolean_parameters?enabled=null'];
106103
yield 'enabled_alias_empty_value' => ['/filtered_boolean_parameters?enabled='];
107104
}
108105

‎tests/Functional/Parameters/NumericFilterTest.php‎

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -67,12 +67,12 @@ public static function rangeFilterScenariosProvider(): \Generator
6767
}
6868

6969
/**
70-
* With the canonical ExactFilter, a null/empty value is matched literally and yields no
71-
* result — unlike the deprecated NumericFilter which skipped filtering (returning all rows,
72-
* covered by the legacy regression suite).
70+
* With the canonical ExactFilter, an empty value casts to 0/0.0 (deterministically across
71+
* drivers) and matches no row in this fixture — unlike the deprecated NumericFilter which
72+
* skipped filtering (returning all rows, covered by the legacy regression suite).
7373
*/
74-
#[DataProvider('nullAndEmptyScenariosProvider')]
75-
public function testRangeFilterWithNullAndEmptyValues(string $url): void
74+
#[DataProvider('emptyScenariosProvider')]
75+
public function testRangeFilterWithEmptyValues(string $url): void
7676
{
7777
$response = self::createClient()->request('GET', $url);
7878
$this->assertResponseIsSuccessful();
@@ -83,13 +83,10 @@ public function testRangeFilterWithNullAndEmptyValues(string $url): void
8383
$this->assertCount(0, $filteredItems, \sprintf('Expected no item for URL %s', $url));
8484
}
8585

86-
public static function nullAndEmptyScenariosProvider(): \Generator
86+
public static function emptyScenariosProvider(): \Generator
8787
{
88-
yield 'quantity_int_null_value' => ['/filtered_numeric_parameters?quantity=null'];
8988
yield 'quantity_int_empty_value' => ['/filtered_numeric_parameters?quantity='];
90-
yield 'ratio_float_null_value' => ['/filtered_numeric_parameters?ratio=null'];
9189
yield 'ratio_float_empty_value' => ['/filtered_numeric_parameters?ratio='];
92-
yield 'amount_alias_int_null_value' => ['/filtered_numeric_parameters?amount=null'];
9390
yield 'amount_alias_int_empty_value' => ['/filtered_numeric_parameters?amount='];
9491
}
9592

0 commit comments

Comments
 (0)