-
-
Notifications
You must be signed in to change notification settings - Fork 892
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[RFC] Filter composition #2400
Comments
There are 2 other popular syntaxes:
Both already have a lot of compatible client tools (including React Admin providers IIRC) and would be fine to me. |
I actually like the loopback syntax because it looks very PHPish. |
My suggestion here has nothing to do with complex querying from the client. These composite filter parameters are defined by the API. In fact, these 2 approaches could coexist. Also, this would make it easier to write custom filters by being able to use composition of filter primitives. Whether we want to expose these primitives through configuration is another story. |
regarding query syntax, https://github.com/krakphp/aql is a solution I built that basically allows sql type expressions in the API, but it's parsed and validated and can contain certain constraints, it might be something of use here for allowing more complex querying/flexibility |
I'm strongly -1 for giving too much control to the client (there's GraphQL for that lol, at least it's better than inventing yet another format / syntax) (or OData if we go that route). Anyway, it's irrelevant to this RFC, which is about filter composition on the server side only. |
ah, i guess I misunderstood your original post, the idea is that if we do:
We could define a proper query portion in the resource of what q maps to. So instead of q mapping to a simple field, we could build up a complex expression for q is searched on. Is that understanding correct? |
@ragboyjr Yes 😄 |
ah, ok, cool beans, y, this would be awesome, part of me wonders if maybe just making the custom filter API easier would fix this. Like, if making filters was just more composable already, then we could easily have filter factories that can be used to create filters instead of building our own expression parser or something on the backend. |
@ragboyjr Could you elaborate? I'm not sure I get what you mean, about the composable filter API and filter factories. |
Sure, The basic idea would be similar to this: https://github.com/krakphp/php-inc/blob/master/src/php-inc.php#L30 Where we have filter classes that decorate certain things Then in the filter config for a resource, we'd define structured config like:
The config is a bit verbose, but the idea is that we'd introduce a few new filters which handled one thing: OrFilter that takes a set of filters in its constructor. PropertyFilter, that takes on property key and the filter to apply on the property value. And a MatchFilter which is basically our search filter which takes some configuration parameters on if it's caseless and a partial match or not. |
Please see my examples in the first post. We could use expression language. No need for verbose config. But the real problem is Doctrine's QueryBuilder. It's not composable at all. |
any update about this feature ? :-) |
Not unless we are able to switch to a composable query builder (on top of / as an alternative to Doctrine's QueryBuilder). Perhaps it need not (and should not) support the same expressiveness, which might be an impediment to composability indeed. |
IMHO the problem is not in the QueryBuilder but the filters not separating two concerns:
They can be made them composable by implementing an extra interface. The behavior of ::apply does not change so all tests keep running. But custom subclasses may break because ::filterProperty no longer adds the expressions to the QueryBuilder but instead returns them. You can see the refactoring here. (still needs workarounds if combining filtering by nested properties through OR) Here is an example of composition in php code using the interface and here one of a WhereFilter (FilterLogic). Also see my comment on Filters RFC #1724. |
I was working in Proof-of-Concept of a bundle for provide a filter that supports expressions for the filter logic (you can see for yourself), based on the commentaries of @teohhanhui and the separation of the generating query logic and the documentation inspired by the work of @metaclass-nl. In summary you can do the following with Api Platform with this bundle: use ApiPlatform\Core\Annotation\ApiFilter;
use ApiPlatform\Core\Annotation\ApiResource;
use Doctrine\ORM\Mapping as ORM;
use Instacar\ExtraFiltersBundle\Doctrine\Orm\Filter\ExpressionFilter;
#[ApiResource]
#[ApiFilter(ExpressionFilter::class, properties: ['search' => 'orWhere(match("name", "ipartial"), match("author.name", "ipartial"), match("year", "exact"))'])]
#[ORM\Entity]
class Book {
// real implementation
} You can extend the filter composability implementing a interface and creating your own operators. I'd love to work in a similar functionality for Api Platform, but I'm not sure if this is what you want. For my part, I'm going to continue developing the Bundle for Symfony. |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
still relevant |
Is this subject still ongoing / in reflexion? I was trying to find a good way to do this and actually started toying with symfony's ExpressionLanguage. It felt like a pretty good solution since it was already "in the ecosystem" and could provide the AST from the textual expression, so getting a rough POC for basic stuff (and/or and ==) was fairly easy. Is there any point to keep digging in that direction? |
|
I'll take a look at that, thanks. And then maybe start an "exploratory" PR based on what I did; I feel like we have the means to build something pretty neat, but it would likely come with a big rework of what is currently available. |
Yeah and for now we had more urgent work. I think that #5732 is a good extension point to have for "selecting the data you want". Now we either need to choose what query parameters specification to pick (or create one), or we need to provide an extension point to handle that? Anyways probably that creating a filter using expression language would work. Also, I'd like to make some order in these parts of the code:
@metaclass-nl thanks for your input it's a valuable comment and I often look at it! |
I feel like chosing the exact query parameter specification is not actually that important an issue : we could actually support multiple specs by simply having an internal AST representation of the query and use that to build orm queries. Then for any query parameter spec there's only a need to implement the transformation into that representation, which shouldn't be too difficult. As for the rest...well yes, lots of stuff need looking at! |
Instead of giving the client the ability to construct arbitrary queries / filtering (something better served by existing standards / specifications, e.g. OData and GraphQL), we could allow predefined composite filters in the API, each composed of one or more filter primitives.
For example:
/users
q
:match('username', 'ipartial') or match('email', 'ipartial') or match('firstName', 'iword_start') or match('lastName', 'iword_start')
/products
q
:match('sku', 'iexact') or match('name', 'iword_start') or match('description', 'iword_start')
This also supports the aliasing use case where you could easily define multiple filter parameters for the same property, e.g.:
/users
email
:match('email', 'iexact')
email_partial
:match('email', 'ipartial')
/products
sku
:match('sku', 'iexact')
sku_partial
:match('sku', 'ipartial')
Of course, this also forces us to have decoupled code between the declaration of filter parameters (currently the
$properties
argument) and the actual filtering (currently thefilterProperty
function). It'd be a good opportunity to get rid of inheritance of filter classes (mark as deprecated; to be removed in API Platform 3.0).Related:
The text was updated successfully, but these errors were encountered: