api-platform / core

The server component of API Platform: hypermedia and GraphQL APIs in minutes
https://api-platform.com
MIT License
2.39k stars 848 forks source link

Filter Extension broken in 3.2.2 #5934

Open bendavies opened 8 months ago

bendavies commented 8 months ago

API Platform version(s) affected: 3.2.2

Description
ApiFilter annotations have stopped working in 3.2.2.

How to reproduce
Given a. resource:

<?php

declare(strict_types=1);

namespace BindHQ\Api\ApiResource;

use ApiPlatform\Doctrine\Orm\Filter\SearchFilter;
use ApiPlatform\Doctrine\Orm\State\Options;
use ApiPlatform\Metadata\ApiFilter;
use ApiPlatform\Metadata\ApiProperty;
use ApiPlatform\Metadata\ApiResource;
use ApiPlatform\Metadata\Get;
use ApiPlatform\Metadata\GetCollection;
use BindHQ\Api\State\EntityClassDtoStateProvider;
use BindHQ\Bundle\EntitiesBundle\Entity\MarketingCompany;

#[ApiResource(
    shortName: 'MarketingCompany',
    operations: [
        new Get(),
        new GetCollection(),
    ],
    provider: EntityClassDtoStateProvider::class,
    stateOptions: new Options(entityClass: MarketingCompany::class),
)]
#[ApiFilter(SearchFilter::class, properties: [
    'name' => 'ipartial',
])]
class MarketingCompanyApi
{
    #[ApiProperty(readable: false, writable: false, identifier: true)]
    public ?int $id = null;
    public ?string $name;
}

Performing a request like:

GET /api/marketing-companies?name=foo

Will get to here: https://github.com/api-platform/core/blob/ad2cbe073b1cf0d5589c2ef1e48603e4426286ad/src/Doctrine/Orm/Extension/FilterExtension.php#L54

the $filterId and the service name in ContainerInterface $filterLocator are different: annotated_bind_hq_api_api_resource_marketing_company_api_api_platform_doctrine_orm_filter_search_filter vs annotated_bind_h_q_api_api_resource_marketing_company_api_api_platform_doctrine_orm_filter_search_filter

the difference is: annotated_bind_hq_ vs annotated_bind_h_q_

Possible Solution
I'm guessing this is down to https://github.com/api-platform/core/pull/5637

Additional Context

soyuka commented 8 months ago

Thanks for the report. Does keep_legacy_inflector: true solves the issue for now?

bendavies commented 8 months ago

Thanks for the report. Does keep_legacy_inflector: true solves the issue for now?

yes, it does. 👍

jotwea commented 6 months ago

I am currently facing the same issue. Setting keep_legacy_inflector: true worked for me for the moment.

gseric commented 2 months ago

I think the issue is in a approach used for the new Inflector implementation. It's a singleton instead of proper service, so it's not configured properly at the run time.

AP bundle extension configures it here: https://github.com/api-platform/core/blob/6d15e228611875a3c1a41a14ecf26a38bd67639a/src/Symfony/Bundle/DependencyInjection/ApiPlatformExtension.php#L986 by calling this static method: https://github.com/api-platform/core/blob/6d15e228611875a3c1a41a14ecf26a38bd67639a/src/Metadata/Util/Inflector.php#L35

However, that applies only at the container build time. During app run time, this static method is not called. Therefore, Inflector will always use its default config (Doctrine inflector) at the run time.

So, in short, if keep_legacy_inflector=false:

  1. service container filters IDs are generated using Symfony inflector (BindHQ\Api\... -> annotated_bind_hq_api_...)
  2. run time filters IDs are generated using Doctrine inflector (BindHQ\Api\... -> annotated_bind_h_q_api_...)
soyuka commented 2 months ago

feel free to provide a patch! Thanks

soyuka commented 1 month ago

After investigating this I think that we should definitely provide an inflector service instead of using a static inflector. It'll allow to override the inflector as well. I'm even thinking that we could revert https://github.com/api-platform/core/commit/6babb3d6b707290fdf314c0e96acd525d6f96670 and provide a better way upgrade path before 4.x.