Default order + OrderFilter produce duplicate columns in orderby

[mweidt]

Hi,

I'm using ApiPlatform 2.3.5 with MS-SQL-Server as DBMS and Doctrine. In this setup I encountered the following problem.

I defined an entity with overriding the default order - e.g. @ApiResource(attributes={"order"={"foo": "ASC"}}) - and allowed an OrderFilter on the same field - e.g. @APIFilter(OrderFilter::class, properties={"foo": "ASC"}).

The request on "/entities?order[foo]=asc" produces an error with MS-SQL-Server:

A column has been specified more than once in the order by list. Columns in the order by list must be unique.

Here is the sql

... ORDER BY d0_.foo ASC, d0_.foo ASC

On MySQL the same sql is produced - but no error occures because MySQL is able to optimize the code.

In my opinion, ApiPlatform should add the default orderby only if it's not already added by OrderFilter.

Thanks for your work and your support

[soyuka]

From what I see you're declaring 2 filters, can't you just use 1? Anyway, there may be some room for improvements on how filters are applied (first property level, then resource level) if it's not already the case.

[mweidt]

I think it's useful to define a default order in case a API caller omits any order parameter. Therefore I defined it like this.

/**
 * @ApiResource(
 *     attributes={
 *        "order"={"isbn": "ASC"}
 *     }
 * )
 * @ApiFilter(
 *     OrderFilter::class, properties={
 *        "isbn": "ASC",
 *        "title": "ASC"
 *     }
 * )
 *
 * @ORM\Table(name="books")
 * @ORM\Entity
 */
class Book
{
	...
}

Requesting /books?order[isbn]=asc will produce the duplicate order by in SQL ... ORDER BY d0_.isbn ASC, d0_.isbn ASC.

Of course I could remove the configuration for ApiResource. Then all callers of the API have to provide a order parameter to get an ordered collection.
But I think in this case ApiPlatform will also add a default order on the identifier field. So the error will occure if I allow the identifier field as OrderFilter...

[mweidt]

What would be the correct solution in your optinion? Apply the default order only if there isn't any order by criteria?

Or should it be the job of doctrine to omit duplicate order by entries - if someone calls addOrderBy with a field more than once?

[soyuka]

IMO just set the default order on the resource, you don't need one on the property. We could add a fix in the orderfilter but merging configurations is always a new source of issues.

[mweidt]

Ok - but then it's not possible to request /books?order[isbn]=desc ...

I also think the solution by merging configurations or current values is not trivial :-(

[soyuka]

I definitely need to investigate, I'm sure there is an easy solution. I'll get back to you.

[deancsmith]

We also encountered this problem on api-platform/core v2.5.0 running against MS SQL which, out of the box, made the OrderFilter unusable for our use-case of applying custom (user) filtering to different properties on a collection.

When adding an OrderFilter and then trying to apply ordering to the default property (id) we get a database exception because a default ASC filtering is applied by the API Platform in addition to our OrderFilter which results in the Doctrine SQL query looking like ...ORDER BY e0_.id DESC, e0_.id ASC... As stated earlier this is invalid on Microsoft SQL Server and results in the SQL error [SQL Server] A column has been specified more than once in the order by list. Columns in the order by list must be unique..

Our workaround has been to turn off the default resource ordering by supplying an empty config value so that we can apply our own. We do not care about the default ordering of the resource, as it is not being used elsewhere, but it actually appears to be ordering in ASC order by the primary key (id) anyway, so this may be good enough behaviour for anyone else if the default ordering is important (until there is a better way to handle this within API Platform itself).

/**
 * @ApiResource(
 *     ...
 *     attributes={
 *        "order"={}
 *     }
 * )
 * @ApiFilter(OrderFilter::class, properties={"id": "DESC", "createdAt": "DESC"})
 *
 * @ORM\Table(name="books")
 * @ORM\Entity
 */
class Book
{
    private $id;

    private $createdAt;

    ...
}

[LeandroLuccerini]

Same issue here with api-platform/core v2.5.7 and yaml configuration, i have duplicated ordering field that causes an error.
Yaml is used 'cos Acme\Ind\Entity are in a separate bundle.

config/services.yaml

############################################################
# FILTERS
############################################################
app.cliente_resource.order_filter:
    parent: 'api_platform.doctrine.orm.order_filter'
    arguments:
        $properties:
            codiceCliente: ~
    tags: [ { name: 'api_platform.filter', id: 'cliente.order_filter' } ]
    autowire: false # required, explained below
    autoconfigure: false # required, explained below

config/api_platform/cliente.yaml

resources:
  Acme\Ind\Entity\Cliente\Cliente:
    shortName: 'Cliente'
    description: 'Cliente di Acme'
    collectionOperations:
      get:
        path: /clienti
        filters: ['cliente.order_filter']
    itemOperations:
      get:
        path: cliente/{id}
        requirements:
          id: '\d{1,6}'
        openapi_context:
          summary: Recupera un cliente in base al codice Acme
          parameters:
            - in: path
              name: id
              schema:
                type: integer
              required: true
              description: Codice cliente, nominato id
    attributes: # optional
      input: App\Dto\Cliente\ClienteDtoInput
      output: App\Dto\Cliente\ClienteDtoOutput
      pagination_items_per_page: 25

Any idea how to solve?

EDIT
codiceCliente is the ID of the Entity

[natepage]

Hey there,
I know I'm a bit late to the party... But we've encountered this issue recently so maybe it can still somebody out there 😄

Having a quick look at api-platform code, this issue is caused by the OrderExtension modifying the queryBuilder if a default order is defined regardless if filters defined by the client are already applied or not. The FilterExtension is executed before the OrderExtension based on its priority, which means that any order filters defined by the client on the request will be applied on the queryBuilder first.

IMHO, I think that a fix could be to update this OrderExtension to apply the default order only if the queryBuilder doens't have any DQL part defined for orderBy already.

As a workaround, here is what we've done in our projects:

1. Create a QueryCollectionExtensionInterface which looks like this:

<?php
declare(strict_types=1);

namespace App\Api\Doctrine\Orm\Extension;

use ApiPlatform\Core\Bridge\Doctrine\Orm\Extension\QueryCollectionExtensionInterface;
use ApiPlatform\Core\Bridge\Doctrine\Orm\Util\QueryNameGeneratorInterface;
use Doctrine\ORM\QueryBuilder;

final class FixOrderByCollectionExtension implements QueryCollectionExtensionInterface
{
    public function applyToCollection(
        QueryBuilder $queryBuilder,
        QueryNameGeneratorInterface $queryNameGenerator,
        string $resourceClass,
        string $operationName = null
    ) {
        $orderByDqlPart = $queryBuilder->getDQLPart('orderBy');

        // Because I had to manually register this as a service to define its priority to make sure it runs after
        // the default order extension, it's actually twice in the collection of extensions...
        // Let's make sure the logic is executed ONLY ONCE, and after the order extension
        if ($orderByDqlPart === []) {
            return;
        }

        // ApiPlatform is adding the default order to the query regardless if the client is specifying the order filter
        // on the request which results in double order in this case...
        // If any order filter is provided as part of the request it would be first in the DQL part, let's remove
        // the last one which is default one added by ApiPlatform
        if (\count($orderByDqlPart) > 1) {
            \array_pop($orderByDqlPart);

            $queryBuilder->add('orderBy', $orderByDqlPart);
        }
    }
}

2. Register this new extension as a service explicitly to control its priority to make sure it runs after the OrderExtension of api-platform:

# config/services.yaml

services:
    # Other services...

    App\Api\Doctrine\Orm\Extension\FixOrderByCollectionExtension:
        tags:
            - { name: 'api_platform.doctrine.orm.query_extension.collection', priority: '-33' }

NOTE
Our solution is removing only the last element of the $orderByDqlPart as it assumes that the default order defined within the configuration contains only one property, so this part might have to be updated based on your own use-case

[alanpoulain]

Do you think you could make a PR with a fix to solve this issue?

[natepage]

@alanpoulain I've created the PR #4493, it fixes the issue for Doctrine ORM and MongoDbOdm, CI won't be green on it right now, sorry it's my first one on this project, but if you support the main idea then I will make sure it has all the green ticks 😄

[alanpoulain]

Nice, I will take a look 🙂

[natepage]

@alanpoulain Should we close this issue as #4493 has been merged?