Skip to content

Commit cf37b7d

Browse files
committed
feat(symfony): upgrade-filter handles relations, dates, case, arguments
Close five gaps in the api:upgrade-filter codemod so it migrates the real-world filter shapes faithfully or skips what it cannot: - Search-on-relation -> IriFilter (native type resolves to a resource) - DateFilter null-management -> filterContext: DateFilter::INCLUDE_NULL_* - i-variant search strategies -> caseSensitive flag (iexact->ExactFilter) - custom filter arguments preserved; filters keyed by service id so two instances of one class survive; nested keys emit an explicit property - detect & skip resources the target filters cannot express: an in-place filters: service filter sharing a key (F5/F6 overlap), or a property renamed by a name converter (the overlay filters do not denormalize)
1 parent 990d34a commit cf37b7d

13 files changed

Lines changed: 739 additions & 46 deletions

src/Symfony/Bundle/Command/Upgrade/UpgradeApiFilterCollisionException.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
*
2121
* @internal
2222
*/
23-
final class UpgradeApiFilterCollisionException extends \RuntimeException
23+
final class UpgradeApiFilterCollisionException extends UpgradeApiFilterSkipException
2424
{
2525
public function __construct(public readonly string $parameterKey)
2626
{

src/Symfony/Bundle/Command/Upgrade/UpgradeApiFilterMapper.php

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,24 +43,37 @@ public function map(string $filterClass, ?string $strategy = null, ?string $prop
4343
'NumericFilter' => new UpgradeApiFilterMapping($canonical('ExactFilter'), castToNativeType: true, nativeType: $propertyNativeType ?? 'int'),
4444
'BackedEnumFilter' => new UpgradeApiFilterMapping($canonical('ExactFilter'), castToNativeType: true, nativeType: $propertyNativeType),
4545
'OrderFilter' => new UpgradeApiFilterMapping($canonical('SortFilter')),
46-
'SearchFilter' => new UpgradeApiFilterMapping($canonical($this->searchReplacement($strategy, $isRelation))),
46+
'SearchFilter' => $this->searchReplacement($canonical, $strategy, $isRelation),
4747
default => new UpgradeApiFilterMapping($filterClass),
4848
};
4949
}
5050

51-
private function searchReplacement(?string $strategy, bool $isRelation): string
51+
/**
52+
* @param callable(string): string $canonical
53+
*/
54+
private function searchReplacement(callable $canonical, ?string $strategy, bool $isRelation): UpgradeApiFilterMapping
5255
{
5356
if ($isRelation) {
54-
return 'IriFilter';
57+
return new UpgradeApiFilterMapping($canonical('IriFilter'));
5558
}
5659

57-
return match ($strategy) {
60+
// A leading "i" makes the legacy strategy case-insensitive; the new search filters are
61+
// case-insensitive by default, so a case-sensitive (non-"i") strategy opts back in.
62+
$caseInsensitive = null !== $strategy && str_starts_with($strategy, 'i');
63+
$base = $caseInsensitive ? substr($strategy, 1) : $strategy;
64+
65+
$shortName = match ($base) {
5866
'exact' => 'ExactFilter',
5967
'start' => 'StartSearchFilter',
6068
'end' => 'EndSearchFilter',
6169
'word_start' => 'WordStartSearchFilter',
6270
default => 'PartialSearchFilter',
6371
};
72+
73+
// ExactFilter has no case-sensitivity option.
74+
$caseSensitive = 'ExactFilter' !== $shortName && !$caseInsensitive;
75+
76+
return new UpgradeApiFilterMapping($canonical($shortName), caseSensitive: $caseSensitive);
6477
}
6578

6679
private function driverNamespace(string $filterClass): ?string

src/Symfony/Bundle/Command/Upgrade/UpgradeApiFilterMapping.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ public function __construct(
2424
public string $filterClass,
2525
public bool $castToNativeType = false,
2626
public ?string $nativeType = null,
27+
public bool $caseSensitive = false,
2728
) {
2829
}
2930
}
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the API Platform project.
5+
*
6+
* (c) Kévin Dunglas <dunglas@gmail.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
declare(strict_types=1);
13+
14+
namespace ApiPlatform\Symfony\Bundle\Command\Upgrade;
15+
16+
/**
17+
* Thrown when a filtered property is renamed by a configured name converter. The new overlay filters
18+
* do not denormalize the property (the parameter factory normalizes it), so such a resource cannot be
19+
* auto-migrated faithfully and is reported and skipped.
20+
*
21+
* @internal
22+
*/
23+
final class UpgradeApiFilterNameConversionException extends UpgradeApiFilterSkipException
24+
{
25+
public function __construct(public readonly string $property)
26+
{
27+
parent::__construct(\sprintf('Cannot auto-migrate: property "%s" is renamed by a name converter, which the target filters do not support. Migrate this resource manually.', $property));
28+
}
29+
}

src/Symfony/Bundle/Command/Upgrade/UpgradeApiFilterParameter.php

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,21 @@
2626
* @param string|null $property explicit property when it differs from $key
2727
* @param string|null $nativeType scalar native type hint (bool|int|float|string), null to omit
2828
* @param bool $castToNativeType whether the QueryParameter should coerce the raw value
29+
* @param string|null $filterContext filter-specific config carried by the QueryParameter (e.g. the
30+
* DateFilter null-management mode), null to omit
31+
* @param bool $caseSensitive emit `caseSensitive: true` on the search filter (case-sensitive
32+
* strategy); the new search filters are case-insensitive by default
33+
* @param array<string, mixed> $arguments constructor arguments to pass to the (kept) filter, named
2934
*/
3035
public function __construct(
3136
public string $key,
3237
public string $filterClass,
3338
public ?string $property = null,
3439
public ?string $nativeType = null,
3540
public bool $castToNativeType = false,
41+
public ?string $filterContext = null,
42+
public bool $caseSensitive = false,
43+
public array $arguments = [],
3644
) {
3745
}
3846
}

src/Symfony/Bundle/Command/Upgrade/UpgradeApiFilterResolver.php

Lines changed: 131 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -13,52 +13,104 @@
1313

1414
namespace ApiPlatform\Symfony\Bundle\Command\Upgrade;
1515

16+
use ApiPlatform\Doctrine\Common\Filter\DateFilterInterface;
17+
use ApiPlatform\Metadata\Exception\PropertyNotFoundException;
1618
use ApiPlatform\Metadata\FilterInterface;
19+
use ApiPlatform\Metadata\Property\Factory\PropertyMetadataFactoryInterface;
20+
use ApiPlatform\Metadata\ResourceClassResolverInterface;
21+
use ApiPlatform\Metadata\Util\TypeHelper;
22+
use Symfony\Component\Serializer\NameConverter\NameConverterInterface;
1723

1824
/**
1925
* Turns the legacy filters declared on a resource into the canonical {@see UpgradeApiFilterParameter}
2026
* list that the visitor injects as QueryParameters.
2127
*
2228
* Properties and strategies are read from each filter's runtime `getDescription()` (the only place
2329
* that knows what a class-level auto-detecting filter actually targets), then mapped to the canonical
24-
* filter by {@see UpgradeApiFilterMapper}.
30+
* filter by {@see UpgradeApiFilterMapper}. A SearchFilter targeting an association cannot be told apart
31+
* from one targeting a scalar field through the description alone, so the property's native type is
32+
* resolved to decide whether it maps to an IriFilter.
2533
*
2634
* @internal
2735
*/
2836
final class UpgradeApiFilterResolver
2937
{
30-
public function __construct(private readonly UpgradeApiFilterMapper $mapper)
31-
{
38+
/** DateFilter null-management modes carried verbatim into the QueryParameter `filterContext`. */
39+
private const DATE_NULL_MANAGEMENT = [
40+
DateFilterInterface::EXCLUDE_NULL,
41+
DateFilterInterface::INCLUDE_NULL_BEFORE,
42+
DateFilterInterface::INCLUDE_NULL_AFTER,
43+
DateFilterInterface::INCLUDE_NULL_BEFORE_AND_AFTER,
44+
];
45+
46+
public function __construct(
47+
private readonly UpgradeApiFilterMapper $mapper,
48+
private readonly PropertyMetadataFactoryInterface $propertyMetadataFactory,
49+
private readonly ResourceClassResolverInterface $resourceClassResolver,
50+
) {
3251
}
3352

3453
/**
35-
* @param array<class-string, FilterInterface> $filtersByClass
54+
* @param list<array{filter: FilterInterface, filterClass: class-string, arguments: array<string, mixed>}> $filters
55+
* one entry per `#[ApiFilter]` declaration (keyed by service id upstream so that two
56+
* instances of the same filter class are kept distinct)
57+
* @param list<FilterInterface> $reservedFilters in-place service filters (the resource `filters:` array) whose query keys must
58+
* not be re-migrated: an #[ApiFilter] mapping onto one of these keys would shadow it
3659
*
3760
* @return list<UpgradeApiFilterParameter>
3861
*
39-
* @throws UpgradeApiFilterCollisionException when two filters map to the same parameter key
62+
* @throws UpgradeApiFilterCollisionException when two filters map to the same parameter key, or an
63+
* #[ApiFilter] key collides with an in-place service filter
4064
*/
41-
public function resolve(string $resourceClass, array $filtersByClass): array
65+
public function resolve(string $resourceClass, array $filters, array $reservedFilters = []): array
4266
{
4367
$params = [];
4468
$seenKeys = [];
4569

46-
foreach ($filtersByClass as $filterClass => $filter) {
47-
foreach ($this->group($filter->getDescription($resourceClass)) as $key => $info) {
70+
foreach ($reservedFilters as $reservedFilter) {
71+
foreach (array_keys($this->group($reservedFilter->getDescription($resourceClass))) as $reservedKey) {
72+
$seenKeys[$reservedKey] = true;
73+
}
74+
}
75+
76+
foreach ($filters as ['filter' => $filter, 'filterClass' => $filterClass, 'arguments' => $arguments]) {
77+
$description = $filter->getDescription($resourceClass);
78+
// The new overlay filters do not denormalize property names, so a resource whose filtered
79+
// properties are renamed by a name converter cannot be migrated faithfully — skip it.
80+
$this->assertNoNameConversion($filter, $description);
81+
82+
// The mode of a DateFilter (include/exclude null) lives in the constructor `properties` map
83+
// as the value, never in getDescription(); read it straight from the filter instance.
84+
$rawProperties = is_callable([$filter, 'getProperties']) ? ($filter->getProperties() ?? []) : [];
85+
86+
foreach ($this->group($description) as $key => $info) {
4887
if (isset($seenKeys[$key])) {
4988
throw new UpgradeApiFilterCollisionException($key);
5089
}
5190
$seenKeys[$key] = true;
5291

53-
$mapping = $this->mapper->map($filterClass, $info['strategy'], $info['type'], $info['isRelation']);
54-
$property = (null !== $info['property'] && $info['property'] !== $key) ? $info['property'] : null;
92+
$isRelation = null !== $info['property'] && $this->isRelation($resourceClass, $info['property']);
93+
$mapping = $this->mapper->map($filterClass, $info['strategy'], $info['type'], $isRelation);
94+
// The new filter system infers the property from a plain key, but cannot for a nested
95+
// (dotted) key, so it must be stated explicitly even when it equals the key.
96+
$property = $this->explicitProperty($info['property'], $key);
97+
98+
$mode = null !== $info['property'] ? ($rawProperties[$info['property']] ?? null) : null;
99+
$filterContext = \is_string($mode) && \in_array($mode, self::DATE_NULL_MANAGEMENT, true) ? $mode : null;
100+
101+
// Constructor arguments only carry over when the filter is kept as-is (custom or a
102+
// surviving filter); a remapped filter has a different constructor.
103+
$filterArguments = $mapping->filterClass === $filterClass ? $arguments : [];
55104

56105
$params[] = new UpgradeApiFilterParameter(
57106
key: $key,
58107
filterClass: $mapping->filterClass,
59108
property: $property,
60109
nativeType: $mapping->nativeType,
61110
castToNativeType: $mapping->castToNativeType,
111+
filterContext: $filterContext,
112+
caseSensitive: $mapping->caseSensitive,
113+
arguments: $filterArguments,
62114
);
63115
}
64116
}
@@ -73,15 +125,22 @@ public function resolve(string $resourceClass, array $filtersByClass): array
73125
*
74126
* @param array<string, array<string, mixed>> $description
75127
*
76-
* @return array<string, array{property: ?string, strategy: ?string, type: ?string, isRelation: bool}>
128+
* @return array<string, array{property: ?string, strategy: ?string, type: ?string}>
77129
*/
78130
private function group(array $description): array
79131
{
80132
$grouped = [];
81133

82134
foreach ($description as $descKey => $meta) {
83135
if (str_starts_with($descKey, 'order[')) {
84-
$grouped['order[:property]'] = ['property' => null, 'strategy' => null, 'type' => null, 'isRelation' => false];
136+
$grouped['order[:property]'] = ['property' => null, 'strategy' => null, 'type' => null];
137+
continue;
138+
}
139+
140+
// ExistsFilter uses the `exists[property]` query syntax; collapse it to the catch-all template
141+
// (the bracketed property, name-converted or not, is resolved by the filter at query time).
142+
if (str_starts_with($descKey, 'exists[')) {
143+
$grouped['exists[:property]'] = ['property' => null, 'strategy' => null, 'type' => null];
85144
continue;
86145
}
87146

@@ -91,10 +150,69 @@ private function group(array $description): array
91150
'property' => $meta['property'] ?? $key,
92151
'strategy' => $meta['strategy'] ?? null,
93152
'type' => $meta['type'] ?? null,
94-
'isRelation' => (bool) ($meta['is_collection'] ?? false) || isset($meta['association']),
95153
];
96154
}
97155

98156
return $grouped;
99157
}
158+
159+
/**
160+
* The new overlay filters read the property as-is (no name-converter denormalization the legacy
161+
* filters did), while the parameter factory normalizes it — so a filtered property renamed by a
162+
* configured name converter would target the wrong field. Detect it and skip the resource.
163+
*
164+
* @param array<string, array<string, mixed>> $description
165+
*
166+
* @throws UpgradeApiFilterNameConversionException
167+
*/
168+
private function assertNoNameConversion(FilterInterface $filter, array $description): void
169+
{
170+
$nameConverter = is_callable([$filter, 'getNameConverter']) ? $filter->getNameConverter() : null;
171+
if (!$nameConverter instanceof NameConverterInterface) {
172+
return;
173+
}
174+
175+
foreach ($description as $meta) {
176+
$property = $meta['property'] ?? null;
177+
if (null === $property) {
178+
continue;
179+
}
180+
181+
$real = implode('.', array_map($nameConverter->denormalize(...), explode('.', (string) $property)));
182+
if ($real !== $property) {
183+
throw new UpgradeApiFilterNameConversionException($property);
184+
}
185+
}
186+
}
187+
188+
private function explicitProperty(?string $property, string $key): ?string
189+
{
190+
if (null === $property) {
191+
return null;
192+
}
193+
194+
return ($property !== $key || str_contains($property, '.')) ? $property : null;
195+
}
196+
197+
/**
198+
* A SearchFilter property is a relation when its native type resolves to an API resource class (an
199+
* object, or a collection of objects). Gating on the resource resolver keeps value objects such as
200+
* \DateTime — which also resolve to a class — out of the IriFilter mapping.
201+
*/
202+
private function isRelation(string $resourceClass, string $property): bool
203+
{
204+
try {
205+
$type = $this->propertyMetadataFactory->create($resourceClass, $property)->getNativeType();
206+
} catch (PropertyNotFoundException) {
207+
return false;
208+
}
209+
210+
if (null === $type) {
211+
return false;
212+
}
213+
214+
$className = TypeHelper::getClassName(TypeHelper::getCollectionValueType($type) ?? $type);
215+
216+
return null !== $className && $this->resourceClassResolver->isResourceClass($className);
217+
}
100218
}
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the API Platform project.
5+
*
6+
* (c) Kévin Dunglas <dunglas@gmail.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
declare(strict_types=1);
13+
14+
namespace ApiPlatform\Symfony\Bundle\Command\Upgrade;
15+
16+
/**
17+
* Base class for the reasons a resource cannot be auto-migrated and is reported and skipped by the command.
18+
*
19+
* @internal
20+
*/
21+
abstract class UpgradeApiFilterSkipException extends \RuntimeException
22+
{
23+
}

0 commit comments

Comments
 (0)