Provide a isCaseSensitive option to the ExactFilter/PartialSearchFilter

[VincentLanglet]

Description
Looking at the PartialSearchFilter

core/src/Doctrine/Orm/Filter/PartialSearchFilter.php

Line 26 in bd3ed98

final class PartialSearchFilter implements FilterInterface, OpenApiParameterFilterInterface

versus the previous SearchFilter

core/src/Doctrine/Orm/Filter/SearchFilter.php

Line 136 in bd3ed98

final class SearchFilter extends AbstractFilter implements SearchFilterInterface

it seems like the previous SearchFilter had more functionality and is more consistant that the new ExactFilter/PartialSearchFilter

• the exact strategy is handled by

  core/src/Doctrine/Orm/Filter/ExactFilter.php

  Line 26 in bd3ed98

  final class ExactFilter implements FilterInterface, OpenApiParameterFilterInterface

  but does not provide a case-insensitive strategy
• the partial strategy is handled by by the PartialSearchFilter but only in the case-insensitive strategy
• there is no StartFilter, no EndFilter, no WordFilter.

Is there a reason for such implementation ? @vinceAmstoutz @soyuka

For instance I would be interested by a isCaseSensitive option to the ExactFilter and the PartialSearchFilter.
Are you ok with this ?

[soyuka]

Casing is different accross platforms, on some it even has performance issues. Yes it is wanted to have less functionalities, maintaining the current SearchFilter is a nightmare and it has many bugs that are hard to solve if we don't want to break existing behaviors. Keep your filter simples, if you need case sensitive + partial search (which is weird imo) just create your own filter. A strict equality with casing is in my opinion correct.
Please bring business use cases and we'll see for an option. Indeed we could add start/end/word filter but these are so easy to create that I'm not sure of the added value here...

[VincentLanglet]

Casing is different accross platforms, on some it even has performance issues. Yes it is wanted to have less functionalities, maintaining the current SearchFilter is a nightmare and it has many bugs that are hard to solve if we don't want to break existing behaviors.

Sure that's understandable, I don't want to push you having a too complex PartialSearchFilter, just looking how to avoid duplicating the whole logic just to have a case sensitive PartialSearch (which is my main usecase)

Keep your filter simples, if you need case sensitive + partial search (which is weird imo) just create your own filter. A strict equality with casing is in my opinion correct. Please bring business use cases and we'll see for an option.

The use case is to provide a "Search and replace" feature, which is something which exists for instance on PHPStorm.

• You may want to replace every foo into bar in sentences but also replacing Foo into bar
• or you may want to replace every foo into bar in sentences but un-touch Foo, which is not weird, since you'll maybe replace it into Bar.

I plan to provide this feature with

• One PartialSearchFilter on the GetCollection (but with a caseSensitive option)
• One route to perform the replacement in database

Currently the query is made this way

$queryBuilder->setParameter($parameterName, '%'.strtolower($values).'%');

$queryBuilder->{$context['whereClause'] ?? 'andWhere'}($queryBuilder->expr()->like(
     'LOWER('.$field.')',
     ':'.$parameterName
));

while I would have look for something like we do in our project

if ($isCaseSensitive) {
            $qb->andWhere('tt.content LIKE :search')
                ->setParameter('search', '%'.$escapedSearch.'%');
        } else {
            $qb->andWhere('LOWER(tt.content) LIKE LOWER(:search)')
                ->setParameter('search', '%'.$escapedSearch.'%');
        }

or like it's done on SonataDoctrineORMAdminBundle side
https://github.com/sonata-project/SonataDoctrineORMAdminBundle/blob/214739047182fc85ab97ade350fdc461cb1d52cf/src/Filter/StringFilter.php#L72-L100

I understand you may not want to implement this on ApiPlatform side, but currently I don't even find a way to implement my own filter by extending/composition with the existing PartialSearchFilter, so it would require myself to copy paste it juste to add this extra condition.

[soyuka]

its definitely recommended to create your own filter if you want this logic, just copy paste the partial filter and add your changes. I'd recommend to create 2 filters a CaseSensitiveFilter and use the PartialSearchFilter, it'd be weird to do all this in the same filter.

[VincentLanglet]

just copy paste the partial filter and add your changes.

That's the plan. But if multiple people have the same needs, which is (I think) possible ; it will create lot of duplicated filter while api platform could provide some.

I'd recommend to create 2 filters a CaseSensitiveFilter and use the PartialSearchFilter, it'd be weird to do all this in the same filter.

I need to try but I plan to do

new GetCollection(
            parameters: [
                'search' => new MyCustomPartialSearchfilter(caseSensitive: true, property: 'name'),
                'isearch' => new MyCustomPartialSearchfilter(caseSensitive: false, property: 'name'),
            ],
        ),

if I understood correctly the new filters. (Would love some migration path)

[VincentLanglet]

Still, it was interesting to reimplement my PartialSearchFilter since I think I discovered a bug @soyuka #7653

[vinceAmstoutz]

its definitely recommended to create your own filter if you want this logic, just copy paste the partial filter and add your changes. I'd recommend to create 2 filters a CaseSensitiveFilter and use the PartialSearchFilter, it'd be weird to do all this in the same filter.

Yes I agree.

💡Pro tip: using the bin/console make:filter command instead of copy/paste filter files, but both options work ofc

[vinceAmstoutz]

Casing is different accross platforms, on some it even has performance issues. Yes it is wanted to have less functionalities, maintaining the current SearchFilter is a nightmare and it has many bugs that are hard to solve if we don't want to break existing behaviors.

Sure that's understandable, I don't want to push you having a too complex PartialSearchFilter, just looking how to avoid duplicating the whole logic just to have a case sensitive PartialSearch (which is my main usecase)

Keep your filter simples, if you need case sensitive + partial search (which is weird imo) just create your own filter. A strict equality with casing is in my opinion correct. Please bring business use cases and we'll see for an option.

The use case is to provide a "Search and replace" feature, which is something which exists for instance on PHPStorm.

• You may want to replace every foo into bar in sentences but also replacing Foo into bar
• or you may want to replace every foo into bar in sentences but un-touch Foo, which is not weird, since you'll maybe replace it into Bar.

I plan to provide this feature with

• One PartialSearchFilter on the GetCollection (but with a caseSensitive option)
• One route to perform the replacement in database

Currently the query is made this way

$queryBuilder->setParameter($parameterName, '%'.strtolower($values).'%');

$queryBuilder->{$context['whereClause'] ?? 'andWhere'}($queryBuilder->expr()->like(
     'LOWER('.$field.')',
     ':'.$parameterName
));

while I would have look for something like we do in our project

if ($isCaseSensitive) {
            $qb->andWhere('tt.content LIKE :search')
                ->setParameter('search', '%'.$escapedSearch.'%');
        } else {
            $qb->andWhere('LOWER(tt.content) LIKE LOWER(:search)')
                ->setParameter('search', '%'.$escapedSearch.'%');
        }

or like it's done on SonataDoctrineORMAdminBundle side
https://github.com/sonata-project/SonataDoctrineORMAdminBundle/blob/214739047182fc85ab97ade350fdc461cb1d52cf/src/Filter/StringFilter.php#L72-L100

I understand you may not want to implement this on ApiPlatform side, but currently I don't even find a way to implement my own filter by extending/composition with the existing PartialSearchFilter, so it would require myself to copy paste it juste to add this extra condition.

Could be a very good contribution! Maybe we can think about a design extensible also at this place. Inheritance is not the best option here but yes, something like a composition could be an option. It will be ok for you @soyuka (depending on the implementation)?

[VincentLanglet]

Could be a very good contribution!

I thought so and was ready to do it, but if I understood correctly @soyuka in #7651 (comment), he seems to prefer having the caseSensitive option in a custom filter on my project rather than added to the PartialSearchFilter.

Maybe we can think about a design extensible also at this place. Inheritance is not the best option here but yes, something like a composition could be an option. It will be ok for you @soyuka (depending on the implementation)?

The simplest solution I have in mind rely on inheritance (which I agree is not the best option). But it can still help to start a reflection about it. WDYT @vinceAmstoutz about something like:

abstract class AbstractComparisonFilter implements FilterInterface, OpenApiParameterFilterInterface
{
    use BackwardCompatibleFilterDescriptionTrait;
    use OpenApiFilterTrait;

    final public function apply(QueryBuilder $queryBuilder, QueryNameGeneratorInterface $queryNameGenerator, string $resourceClass, ?Operation $operation = null, array $context = []): void
    {
        $parameter = $context['parameter'];
        $property = $parameter->getProperty();
        $alias = $queryBuilder->getRootAliases()[0];
        $field = $alias.'.'.$property;

        $values = $parameter->getValue();
        if (!is_iterable($values)) {
            $values = [$values];
        }

        $expressions = [];
        foreach ($values as $val) {
            $parameterName = $queryNameGenerator->generateParameterName($property);
            $queryBuilder->setParameter($parameterName, $this->formatValue($val));

            $expressions[] = $this->createExpression($queryBuilder, $field, $parameterName);
        }

        $whereClause = $context['whereClause'] ?? 'andWhere';
        if (count($expressions) === 1) {
            $queryBuilder->{$whereClause}($expressions[0]);
        } else {
            $queryBuilder->{$whereClause}($queryBuilder->expr()->orX(...$expressions));
        }
    }

    abstract protected function createExpression(
        QueryBuilder $queryBuilder,
        string $field,
        string $parameterName,
    );

    protected function formatValue(string $value): string
    {
        return $value;
    }
}

final class PartialSearchFilter extends AbstractComparisonFilter
{
    protected function createExpression(
        QueryBuilder $queryBuilder,
        string $field,
        string $parameterName,
    ) {
        return $queryBuilder->expr()->like('LOWER('.$field.')', ':'.$parameterName);
    }

    protected function formatValue(string $value): string
    {
        return '%'.strtolower($value).'%';
    }
}

[soyuka]

Let's avoid an abstract filter it becomes a mess quite fast, its alright to duplicate some code into a PartialCaseSensitiveFilter but lets wait to see if more users need that, maybe let's keep this issue open ?