core icon indicating copy to clipboard operation
core copied to clipboard

Expose denormalizePropertyName/normalizePropertyName in a Trait

Open VincentLanglet opened this issue 2 months ago • 2 comments

Description
While writing custom filters, I have the need for the denormalizePropertyName/normalizePropertyName method. But currently to have them I have to extends AbstractFilter.

I feel like it would be more useful to have them directly in a trait in doctrine/common, and this trait could be used by both ApiPlatform\Doctrine\Orm\Filter\AbstractFilter ApiPlatform\Doctrine\Odm\Filter\AbstractFilter

I thought about adding them to PropertyHelperTrait but it requires adding a property nameConverter to this trait which means

  • protected ?NameConverterInterface $nameConverter = null, needs to be changed in AbstractFilter to
public function __construct(
        ...
        ?NameConverterInterface $nameConverter = null,
    ) {
        $this->nameConverter = $nameConverter;
    }

and therefore doctrine/orm & odm package 4.2.9 won't work with 4.2.10 version of doctrine/common ; not sure if it's ok to make such change and add such requirement/conflict between packages

  • If someone use the PropertyHelperTrait AND has a property nameConverter it will break

The other option would be to introduce a new Trait, but I'm not sure what would be a good name for it, based on the fact the first trait is pretty generic with PropertyHelperTrait... any suggestion @soyuka ?

VincentLanglet avatar Dec 08 '25 11:12 VincentLanglet

and therefore doctrine/orm & odm package 4.2.9 won't work with 4.2.10 version of doctrine/common ; not sure if it's ok to make such change and add such requirement/conflict between packages

It's okay we just bump the minimal requested version before releasing. But I'm not sure why you'd need such a thing, on what filters do you rely? Are you using the QueryParameter? Just use the name converter?

soyuka avatar Dec 08 '25 16:12 soyuka

But I'm not sure why you'd need such a thing, on what filters do you rely? Are you using the QueryParameter? Just use the name converter?

Because new filters does not support nested parameters natively anymore (cf https://github.com/api-platform/core/issues/7554 and some other issues/discussions), I had to re-implement it myself (sometimes multiple times for different filters) while still trying to make the property generics in order to avoid having too much filters.

While name converter works fine on fooBar -> foo_bar or foo_bar -> fooBar I assume (but I may need to check again) that it's not the case with dots and does not allows the transformation

  • fooBar.fooBar -> foo_bar.foo_bar
  • foo_bar.foo_bar -> fooBar.fooBar

That why the implode/explode/array_map manipulation is still needed in that case https://github.com/api-platform/core/blob/4a72073573af0740ffb24c7ed1e8a13e36546654/src/Doctrine/Orm/Filter/AbstractFilter.php#L112-L128 and I thought it could be useful to avoid duplicating this "property-dedicated-logic"...

VincentLanglet avatar Dec 08 '25 21:12 VincentLanglet