docs(adr): filtering system and query parameters

[soyuka]

Introduction of an ADR, few things must be discussed, I joined a few ideas we collected over the past few years. This must be seen as a research into providing better developer experience and more freedom when expressing filters with API Platform.

Friendly ping: @aegypius, @mrossard, @metaclass-nl, @helyakin, @jdeniau, @bendavies, @bpolaszek

if you have ideas, things we should think of, or opinions I'd be glad to hear them, thanks!

[mrossard]

Starting this i great, i've been thinking about it and have some ideas, but couldn't find the time...here are some quick unorganized thoughts :

The direction i wanted to go was to first implement an AST-based filter (independant from the actual query language), then specific filters for one or more query language which would just translate the query into an AST (I started looking at Symfony's Expression Language but anything would work).
It would then be pretty simple to rewrite the existing filters as "ast builders".

It would need to be broken into quite a lot of smaller parts (further thinking needed here ), for example dealing with individual operations ( = / < > ...), which could provide extension points, allowing devs to add their custom operations...

I think in the new system i'd be better to force explicit enabling of filtering on properties, and if the target property is a resource, only the IRI should be expected (explicitly filtering on resource.id would still be possible after all...) : you would either have stuff.property = /someResource/1 or stuff.property.id = 1, without any ambiguity.
This might require some work on the BC layer to create a correct AST, but it seems very doable (stuff.property=string would likely just need to be transformed into stuff.property=string or stuff.property.id=string).

That's about it for my thoughts on it as of right now!

[aegypius]

Most of my concerns and all the override, decorating and tweaking I encountered so far is due to the lack of proper definition of parameters. As you may know, there is no specification for filters because it's a server-side concern from the OpenApi standpoint.

But parameters are quite well described and spec'ed up. with this remark I would introduce a parameter attribute to allow to describe properly what is expected.

This may resolve most of my concerns:

• properly validate and parse values of parameter (allowing custom validation if needed)
• allow to define a parameter that is not directly a property on the resource (aliases, localization, etc...)

Each parameter could be linked to one or more "filters" regardless of what is supposed to (filter, sort, paginate...)
Parmeters could be defined a resource level or at operation level according to OpenApi spec.

From the DX standpoint, I think it would be appreciable to be able to rely on the php type system (like BackedEnums for example) to properly describe parameters.

A parameter could be declare lke this:

#[Parameter('page', processors: [PaginationFilter::class])]
#[Parameter('limit', processors: [PaginationFilter::class])]
class Book 
{
    #[\Parameter(filters: [SearchFilter::class]]
    #[\Parameter(name: 'color', filters: [SearchFilter::class])] // You're welcome 🇺🇸 ! 
    public ColourEnum $colour;
}

This could allow to introduce parameter parsers too, allowing to use some sort of expression like a range expression, negation filters, lucene query syntax...

As a bonus, I think it could avoid the breaking changes because it's a entirely new concept and I can imagine with a few tweaks we could use the two systems together, allowing the end users to choose which implementation is the best fit for his case.

[mrossard]

Having the possibility to specify the types / validation method for parameters would be a very nice improvement!

I'd still keep it inside the filter definition though, something that could look like this :

#[ApiFilter(SomeFilter::class, properties:[
        'exposedName'=> new FilterProperty(
            target: 'realPropertyPath',
            type: Type::BUILTIN_TYPE_STRING,
            parser: self:nameParser(...),
            validation: self::nameValidator(...),
        )
    ]
)]

[soyuka]

We should not have a strong link between a filter (parameter) and a property.

[mrossard]

We should not have a strong link between a filter (parameter) and a property.

well there needs to be a way to specify this some way or other, or we can't build the ORM query, can we? Or am i missing the point?

[soyuka]

Because it's locking filters to behave like a parameter is always tight to a property which isn't the case. Something preferable would be:

#[ApiFilter(SomeFilter::class, properties:[
        'exposedName'
    ], filters: [
        new FilterProperty(
            target: 'realPropertyPath',
            type: Type::BUILTIN_TYPE_STRING,
            parser: self:nameParser(...),
            validation: self::nameValidator(...),
        )
    ]
)]

[mrossard]

Because it's locking filters to behave like a parameter is always tight to a property which isn't the case. Something preferable would be:

#[ApiFilter(SomeFilter::class, properties:[
        'exposedName'
    ], filters: [
        new FilterProperty(
            target: 'realPropertyPath',
            type: Type::BUILTIN_TYPE_STRING,
            parser: self:nameParser(...),
            validation: self::nameValidator(...),
        )
    ]
)]

Then where do you see the link between the "exposedName" query parameter and the "realPropertyPath" being made? I'm a bit confused here.
When you're trying to build the backend query you'll need to know somehow that the "exposedName=someValue" in the query means you have to filter on records that match "someValue" for "realPropertyPath"...how would you do that?

[soyuka]

This is why I have a property but also an array attributes in my Parameter suggestion so that we can add a link, but the link itself is not at the heart of the filter definition. This will help with #5495 for example, which right now is quite hard to fix.

[mrossard]

This is why I have a property but also an array attributes in my Parameter suggestion so that we can add a link, but the link itself is not at the heart of the filter definition. This will help with #5495 for example, which right now is quite hard to fix.

I think I'm actually seing things very differently from what you envision, which explains how i have trouble wrapping my head around the logic of it.
For example I don't get the need for multiple filters, if the target is some query language. I was seeing just one SearchFilter with multiple query language parsers, since the call would be something like GET "/my_resources?query=(stuff=value or otherstuff=whatever)".
What i thought we'd need would look more like this :

#[ApiFilter(NewSearchFilter::class,
    properties         : [
        'name' => new FilterProperty(
            target: 'searchableName',
            type  : Type::BUILTIN_TYPE_STRING,
            parser: NameStringSearchFormatter::class
        ),
        'linkedResource' => new FilterProperty(
            target: 'otherResourceCollection',
            type  : Type::BUILTIN_TYPE_STRING,
            parser: IriToOtherResourceConverter::class
        )],
    queryLanguageParser: new ExpressionLanguageParser(),
    parameterName: 'query'
)]
class SomeResource
{
    public int $id;
    public string $name;
    public string $searchableName;
    public array $otherResourceCollection;
}

class NewSearchFilter
{
    function __construct(protected array $properties, protected QueryLanguageParserInterface $queryLanguageParser, protected string $parameterName)
    {
    }

    function filter($property, $value, QueryBuilder $queryBuilder, ...)
    {
        if($property !== $this->parameterName){
            return;
        }
        $ast = $this->queryLanguageParser->parse($value, $this->properties);
        return (new AstFilter($ast))->filter($queryBuilder,...);
    }
}

The FilterProperty could also have a link property instead of the target, but I feel like it would/will be quite a headache to implement - the correct way to establish the link on the ORM depends on the "state" of the AST (see the code i sent you on slack, typically you're doing simple joins or left joins depending on what happened before).

[mrossard]

...and i'm actually only talking about search filter when you're talking about all filters, which can't help 😅

[soyuka]

Not a problem, in fact I want to cover most of the possibilities and doing a search filter like you have in mind should be possible but there are some things that aren't portable in your example. For example queryLanguageParser, parameterName are things that should belong to your filter NewSearchFilter and not to our ApiFilter. This makes a huge difference.

For API Platform core, we have a SearchFilter, but I'd like to improve the current search filter by splitting the logic, mainly to resolve typing issues for example our current search filter does:

• IRI filtering
• id filtering (should work with uids)
• text match (starts with, ends with, case sensitive etc. these are options to apply for each parameter)
• exact match
• date doesn't work as it should use the date range filter, but why couldn't it be used when we found a date type?

In my opinion, we should find a way to compose with filters working on a single parameter (search) or multiple parameters (parameterName equals propertyName). This is why I don't want to correlate a property with a filter, but instead work on a Parameter (or list of parameters) each applying a list of filters.

[aegypius]

I think the queryLanguageParser name is not appropriate in all cases. A parameter can be found in path, query are header according to OpenAPI Spec the way we parse value can be the same for every location.

I am still advocating to split the logic between parameter concept and filters.

The nature of the value of the parameter can in some case use a filter or an other, for exemple

?createdAt=2023-11-28

can be translated to : [2023-11-28T00:00:00, 2023-11-28T23:59:59] maybe a range filter is more appropriate.

Declaring this type of filter using a Filter centric approche is quite cumbersome.

Does it make sense ?

[mrossard]

Not a problem, in fact I want to cover most of the possibilities and doing a search filter like you have in mind should be possible but there are some things that aren't portable in your example. For example queryLanguageParser, parameterName are things that should belong to your filter NewSearchFilter and not to our ApiFilter. This makes a huge difference.

indeed, that was a mistake - i intended something like #[NewSearchfilter(..)]

For API Platform core, we have a SearchFilter, but I'd like to improve the current search filter by splitting the logic, mainly to resolve typing issues for example our current search filter does:

• IRI filtering
• id filtering (should work with uids)
• text match (starts with, ends with, case sensitive etc. these are options to apply for each parameter)
• exact match
• date doesn't work as it should use the date range filter, but why couldn't it be used when we found a date type?

That's where the explicit typing would be very useful, yes, with some kind of parser/transformer specified to transform the text representation from the query into what we actually need (that's what i intended with the "parser" parameter).

Exact match/starts with/... would be different operators in the AST, which would make it fairly easy to have separate logic for them.

In my opinion, we should find a way to compose with filters working on a single parameter (search) or multiple parameters (parameterName equals propertyName).

If I understand what you mean correctly, the second one is easily translated into the first one - /resources?field=value&field2=value2 is just /resources?search=(field=value and field2=value2). It could use the same underlying code without problems

This is why I don't want to correlate a property with a filter, but instead work on a Parameter (or list of parameters) each applying a list of filters.

Then you'd just add some #[Parameter('someparam', filters:[...]) to your resources? It feels weird to me to separate the declaration of the filters (#[Get(filters: [new SearchFilter(), new UidFilter()])) and the definition of what thoses filters can use.

[mrossard]

I think the queryLanguageParser name is not appropriate in all cases. A parameter can be found in path, query are header according to OpenAPI Spec the way we parse value can be the same for every location.

the "query" in "queryLanguageParser" was not meant for how the parameter is passed, but to specify which "query language" is to be used, if several are supported.

I am still advocating to split the logic between parameter concept and filters.

I must be missing something...what other uses will the parameters have? If they're only used as filter parameters i'd rather have them tied to the filters that use them, this way one look is enough to know what a filter can do. It feels like better DX to me, but i've been known not to have the most popular opinion on such things... ;)

The nature of the value of the parameter can in some case use a filter or an other, for exemple

?createdAt=2023-11-28

can be translated to : [2023-11-28T00:00:00, 2023-11-28T23:59:59] maybe a range filter is more appropriate.

Declaring this type of filter using a Filter centric approche is quite cumbersome.

To me it should be a specific operation, there is no clear indication as to whether a date without time should match any time for that date, or just that date at 00:00:00, or...?
There are probably multiple ways to deal with that (typing with separate "Date" and "Datetime" types for example, matching a "Date" parameter to a "Datetime" field could trigger a "between 0:00 and 23:59" logic), but i don't see how this impacts the way it is declared...?

[aegypius]

We renamed the component here #6080

Suggested change

	3. Validate parameters through the QueryParameterValidator.
	3. Validate parameters through the ParameterValidator component (formerly known as QueryParameterValidator).

[dunglas]

Good work!

[dunglas]

Maybe could we allow inline filter definitions?

Suggested change

	public string $filter;
	public string|callable $filter;

[soyuka]

I thought about that but with which interface? at first I wanted something like mixed $value, Parameter $parameter, array $context but it doesn't ally with FilterInterface