Skip to content

feat(symfony): api:upgrade-filter codemod + filter fixture migration#8344

Merged
soyuka merged 5 commits into
api-platform:mainfrom
soyuka:bc-filter
Jun 25, 2026
Merged

feat(symfony): api:upgrade-filter codemod + filter fixture migration#8344
soyuka merged 5 commits into
api-platform:mainfrom
soyuka:bc-filter

Conversation

@soyuka

@soyuka soyuka commented Jun 23, 2026

Copy link
Copy Markdown
Member

What

Migrates the functional-test fixtures off the legacy #[ApiFilter] / multi-strategy filters onto the canonical QueryParameter set (4.4 deliverable §7), and ships the api:upgrade-filter codemod that performs the migration (deliverable §9, beyond the planned stub).

The migration is dogfooded: the codemod runs over our own fixtures, the suite stays green on both paths, and the converted fixtures are committed.

Fixture migration (Option A)

Per family, fixtures move to the canonical filters; a focused Legacy/ regression suite keeps the deprecated paths alive:

  • Boolean/Numeric/BackedEnum → ExactFilter (+ nativeType, castToNativeType)
  • Order → SortFilter (order[:property])
  • Search per-strategy → Exact/Partial/Start/End/WordStart (+ caseSensitive), relation → IriFilter
  • Date/Range/Exists → kept (survive in 5.0), exercised via Legacy/ #[ApiFilter] anchors

Codemod: api:upgrade-filter

A php-parser, format-preserving command that rewrites class- and property-level #[ApiFilter] into QueryParameter entries on #[ApiResource] (cs-fixer post-pass). --dry-run (default) prints a diff; --force writes.

Resolution highlights:

  • Search-on-relation → IriFilter via the property's native type resolving to a resource (gated by the resource-class resolver, so value objects like \DateTime stay search filters).
  • DateFilter null-management → filterContext: DateFilter::INCLUDE_NULL_* (read from the filter's properties map, not the description).
  • i-variant strategies → caseSensitive (the new search filters are case-insensitive by default); iexact → ExactFilter.
  • Custom-filter constructor arguments preserved; filters are keyed by service id so two instances of the same class (e.g. two GroupFilters) both survive; nested keys (colors.prop) emit an explicit property.
  • Detect & skip what the target filters cannot express, with a clear warning:
    • an in-place filters: service filter sharing a query key (would shadow it)
    • a property renamed by a name converter (the new overlay filters do not denormalize; the parameter factory normalizes — so a name-converted property would target the wrong field). The only working form changes the public query name, so these are reported for manual migration instead.

Sweep result: 10 resources migrated, 6 skipped (5 name-conversion + 1 service/#[ApiFilter] key overlap), kept as-is.

Tests / CI

  • Codemod unit tests (mapper / resolver / visitor / command).
  • CI upgrade-filter job: --force then the functional suite (guards both the migrated and Legacy/ paths).
  • Full Functional/{Doctrine,Parameters,JsonApi,GraphQl} green.

Not in this PR (remaining 4.4 filter work)

  • Runtime deprecation triggers (§5.4): F5 #[ApiFilter] (AttributeFilterPass), F6 Operation::$filters (OpenApiFactory + ParameterResourceMetadataCollectionFactory). The 6 skipped resources still carry #[ApiFilter] outside Legacy/ and must move there (or get #[IgnoreDeprecations]) before F5 lands, to keep the no deprecations job green.
  • Drop @internal + document BackwardCompatibleFilterDescriptionTrait (§5.6).
  • Docs: rewrite docs/core/filters.md, add docs/upgrade/filters.md, CHANGELOG (§5.8).
  • GraphQl parity (§10): ComparisonFilter operator form + nested filter args are not generated in the schema (pre-existing on main, e.g. colors(prop:)).

Comment thread .github/workflows/ci.yml Outdated
php-version: ${{ matrix.php }}
extensions: intl, bcmath, curl, openssl, mbstring
ini-values: memory_limit=-1
tools: composer:2.9.8

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why version constraint on this?

Comment thread src/OpenApi/Factory/OpenApiFactory.php Outdated
$entityClass = $this->getStateOptionsClass($operation, $operation->getClass());

if ($resourceFilters) {
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));

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

problem here: use parameters instead ? no point mentioning attribute here. i think we should only deprecate during metadata revert changes in this class.

Comment thread src/OpenApi/Factory/OpenApiFactory.php Outdated

// Collapse identical filter deprecations to one warning per (filter, shortName) per
// generation: a filter exposing N parameters must not emit the same message N times.
$warned = [];

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove this these are old deprecations we'll remove these in 5.0 anyways

$filtersDeprecations = array_values(array_filter($deprecations, static fn (string $m): bool => str_contains($m, 'Operation::$filters')));
$this->assertCount(1, $filtersDeprecations, 'Declaring filters through Operation::$filters must trigger one deprecation per operation.');
$this->assertStringContainsString('QueryParameter', $filtersDeprecations[0]);
}

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove this test

* Resources whose filters cannot be expressed as distinct QueryParameters (e.g. an exact and a
* range filter on the same property) are reported and skipped.
*/
#[AsCommand(name: 'api:upgrade-filter', description: 'Upgrades legacy #[ApiFilter] declarations to QueryParameter')]

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should note that this command will be removed in 6.0

Comment thread .github/workflows/ci.yml Outdated
Comment thread src/State/Parameter/ValueCaster.php Outdated
return $v;
}

if (\is_string($v) && ('' === $v || 'null' === strtolower($v))) {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lets not cast null a default value should make this work, 'null' is not supported by new filter

Comment thread src/OpenApi/Factory/OpenApiFactory.php
@soyuka soyuka force-pushed the bc-filter branch 3 times, most recently from 1fb41fb to 7518b39 Compare June 25, 2026 09:48
@soyuka soyuka marked this pull request as ready for review June 25, 2026 10:11
soyuka added 4 commits June 25, 2026 14:35
Add an `api:upgrade-filter` command that rewrites legacy `#[ApiFilter]`
attributes into parameter-based filters via a php-parser visitor. Includes
the mapper/resolver/visitor pipeline, collision/skip/name-conversion
handling, DI wiring, service config and unit tests. CI runs the codemod
with --force before the functional suite.
Run api:upgrade-filter over the test fixtures to move them onto parameter
filters, and relocate the un-migrated #[ApiFilter] fixtures (Boolean,
Numeric, Enum, Order, Search, Date/Range/Exists, ExceptionToStatus) into a
Legacy/ suite that keeps covering the deprecated path.
Emit a deprecation for the #[ApiFilter] attribute and for Operation::$filters
consumption, surfaced through the metadata factory and OpenAPI factory with
de-duplication. Baseline the resulting deprecations.
ValueCaster now casts empty/null boolean params to false and empty/null
numeric params consistently, and rejects empty values cast to a scalar
native type with a 400 via BadRequestException (wired through the Laravel
error renderer).
Co-authored-by: Antoine Bluchet <soyuka@users.noreply.github.com>
@soyuka soyuka merged commit 0fb1dc8 into api-platform:main Jun 25, 2026
83 of 95 checks passed
@soyuka soyuka deleted the bc-filter branch June 25, 2026 13:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant