core icon indicating copy to clipboard operation
core copied to clipboard

Setting `KeepLegacyInflector` to false, will remove all filters from Get/GetCollection operations.

Open frankdekker opened this issue 1 year ago • 10 comments

API Platform version(s) affected: v3.2.3

Description Below a (slimmed down) example of our entity and ApiPlatform resource and filter definition:

<?php
declare(strict_types=1);

namespace DR\Review\Entity\Review;

use ApiPlatform\Doctrine\Orm\Filter\SearchFilter;
use ApiPlatform\Metadata\ApiFilter;
use ApiPlatform\Metadata\ApiResource;
use ApiPlatform\Metadata\GetCollection;
use Doctrine\ORM\Mapping as ORM;

#[ApiResource(operations: [new GetCollection()])]
#[ApiFilter(SearchFilter::class,properties: ['id' => 'exact'])]
class CodeReviewActivity
{
    #[ORM\Id]
    #[ORM\GeneratedValue]
    #[ORM\Column]
    private ?int $id = null;
}

When I toggle keep_legacy_inflector setting to false the search filter will disappear from the documentation and api.

How to reproduce
Create the entity above and set keep_legacy_inflector to false. Keep an eye on the namespace. The namespace must with start 2 capital letters.

Possible Solution
I've tracked down the problem to: vendor/api-platform/core/src/Metadata/Util/Inflector.php in the tableize method. See phpunit test below for the difference in output. It seems that the (new UnicodeString($word))->snake()->toString() does not return the exact same output as the original tableize method. Handling multiple sequential uppercase letters differently.

PHPUnit test that reproduces the problem:

<?php
declare(strict_types=1);

use ApiPlatform\Metadata\Util\Inflector;
use PHPUnit\Framework\TestCase;

class InflectorTest extends TestCase
{
    public function testTableize(): void
    {
        $word = "DRReviewEntityReviewCodeReviewActivityApiPlatformDoctrineOrmFilterSearchFilter";

        Inflector::keepLegacyInflector(false);
        $outputA = Inflector::tableize($word);

        Inflector::keepLegacyInflector(true);
        $outputB = Inflector::tableize($word);

        static::assertSame($outputA, $outputB);
        // assert fails:
        // $outputA: dr_review_entity_review_code_review_activity_api_platform_doctrine_orm_filter_search_filter
        // $outputB: d_r_review_entity_review_code_review_activity_api_platform_doctrine_orm_filter_search_filter
    }
}

Additional Context

frankdekker avatar Nov 12 '23 09:11 frankdekker

Haven't investigated further yet, but this option makes some OrderFilters disappear in my current project too.

mrossard avatar Dec 06 '23 10:12 mrossard

Actually same exact problem here, the ApiResources having this problem have double uppercase letters in their names...

mrossard avatar Dec 06 '23 12:12 mrossard

Exactly the same for me (filters are ignored when keepLegacyInflector is set to false and the name contains 2 consecutive uppercases or more).

I'm not convinced by the conclusion that Inflector::tableize should do the same thing if keep_legacy_inflector is set to true or false. there is an if/else The solution depends on what the keepLegacyInflector is supposed to do. Maybe it's normal that the legacy inflector doesn't do the same than the new one. The problem is that the new one is not used at every step when we set keepLegacyInflector to false.

There is 3 Inflectors Doctrine\Inflector\Inflector ApiPlatform\Util\Inflector (redirect all method calls to Doctrine\Inflector\Inflector) ApiPlatform\Metadata\Util\Inflector (has it's own implementation, redirect method calls to Doctrine\Inflector\Inflector only if the keep_legacy_inflector option is set to true)

During serviceLocator initialization, we use ApiPlatform\Util\Inflector ; so the legacy one is used, and our filters are stored with the legacy naming convention image

At the moment we create the context to inject it into the Operation, we use ApiPlatform\Metadata\Util\Inflector to retrieve the instance of our filters, and we obtain a name following the new convention. image

And later we always 'continue' because their is no registered filter with the given name image

Other possible solution : add a use statement to replace ApiPlatform\Util\Inflector by ApiPlatform\Metadata\Util\Inflector image

Other possible solution : change the code of ApiPlatform\Util\Inflector to do the same than ApiPlatform\Metadata\Util\Inflector ; but it will still be strange to have 2 inflectors that do the same thing, I wonder what is the reason why those 2 Inflector classes coexist.

@soyuka what do you think ? Is it good ideas ? If you think I'm on the right way I'd be happy to make a PR.

celinederoland avatar Dec 19 '23 13:12 celinederoland

Hi, ApiPlatform\Util\Inflector is deprecated. We should definitely always go to the new inflector or the one you choose please open a PR.

soyuka avatar Dec 20 '23 09:12 soyuka

Same issue here. Any news?

jotwea avatar Apr 08 '24 12:04 jotwea

Im actually seeing a very similar issue.

  • Set keep_legacy_inflector to false
  • The API resources are not multiple capital letters in a row however
  • Custom filters are not getting triggered. Built in ones are.

jonnyeom avatar Aug 01 '24 16:08 jonnyeom

Im actually seeing a very similar issue.

  • Set keep_legacy_inflector to false
  • The API resources are not multiple capital letters in a row however
  • Custom filters are not getting triggered. Built in ones are.

I was wrong, there was a Multiple Capital letters in a row in the namespace

jonnyeom avatar Aug 01 '24 17:08 jonnyeom