Core: Prefer order filter on canonical field

Created on 11 Jun 2017  ยท  9Comments  ยท  Source: api-platform/core

When using the Doctrine ORM OrderFilter, I had a case when I had to sort on a canonical version of a field for valid results. (And, I hope, faster queries with proper indexes)

I created a custom version of OrderFilter, checking if the sorted field has a canonical column (eg: name => name_canonical) and uses it in the orderBy instead of the normal one.

It is something you would accept if I do a PR ?

enhancement question โญ EU-FOSSA Hackathon

All 9 comments

You may write a custom filter which extends the OrderFilter. Specifically, you only have to override the filterProperty method.

@teohhanhui Thanks for the reply.

I already did that, I was wondering if this something we could add to api-platform/core.
For example, we could use ApiProperty to specify the canonical version of a property, to be used in OrderFilter.

I don't mind if we implement this feature, it can be useful IMHO.
@tseho do you want to implement it ?

@Simperfit Sure ! It's a funny coincidence, I finally upgraded my app last week to use the ApiFilter annotation everywhere and I updated the way my CanonicalOrderFilter works.

The resulting syntax is the following:

    /**
     * @ORM\Column(type="string")
     * @ApiFilter(CanonicalOrderFilter::class)
     */
    protected $email;

    /**
     * @ORM\Column(type="string")
     */
    protected $emailCanonical;

I will make a PR tonight so you can check it.

@tseho That's one of my ability, if you are a french people, I can read though your mind just by reading the issue :p.

Mhh looks to me that this is a specific use case. I don't see a fit in the core though.

@soyuka It's not really my call :)
Canonical fields are not that rare, there are some in FOSUserBundle. If this feature shouldn't be part of the core, I don't mind providing an example and some documentation on how to do it.

@Simperfit I have been running more tests and found out that having the CanonicalOrderFilter on a field is against the spirit of OrderFilter because you cannot control the order on which the filters are applied.

I think a proper solution would be to add a new option:

/**
 * @ApiResource
 * @ApiFilter(OrderFilter::class, properties={"email": { "canonical": "emailCanonical", "default_direction": "ASC" }})
 */
class Offer
{
    // ...
}

The change to OrderFilter could be then something like this:

protected function filterProperty(string $property, $direction, QueryBuilder $queryBuilder, QueryNameGeneratorInterface $queryNameGenerator, string $resourceClass, string $operationName = null)
{
    // ...

    if (null !== $canonical = $this->properties[$property]['canonical'] ?? null) {
        $queryBuilder->addOrderBy(sprintf('%s.%s', $alias, $canonical), $direction);
    }

    $queryBuilder->addOrderBy(sprintf('%s.%s', $alias, $field), $direction);
}

Hey @soyuka, @Simperfit, should I open a PR with my last proposal ?

Was this page helpful?
0 / 5 - 0 ratings

Related issues

ghost picture ghost  ยท  3Comments

silverbackdan picture silverbackdan  ยท  3Comments

rockyweng picture rockyweng  ยท  3Comments

tezvi picture tezvi  ยท  3Comments

stipic picture stipic  ยท  3Comments