search

api-platform / core

The server component of API Platform: hypermedia and GraphQL APIs in minutes
https://api-platform.com
MIT License
2.39k stars 848 forks source link

[Filters] OrderFilter not working on Embedded object when using nulls_comparison #3440

Closed t-richard closed 3 years ago

t-richard commented 4 years ago

API Platform version(s) affected: 2.5.4

Description

When trying to sort on a field of an @ORM\Embedded object, it doesn't work if I use the nulls_comparison of the OrderFilter.

Here is an example (for the full code, see the provided reproducer below):

/**
 * @ApiResource()
 * @ORM\Entity()
 *
 * @ApiFilter(OrderFilter::class, properties = {
 *     "bar.luckyNumber" : { "nulls_comparison" : OrderFilter::NULLS_SMALLEST },
 *     "bar.secondLuckyNumber"
 * })
 */
class Foo

Here, if bar is Embedded, the request to /foos?order[bar.luckyNumber]=asc will fail with the error

[Syntax Error] line 0, col 80: Error: Expected Doctrine\ORM\Query\Lexer::T_FROM, got '.'

However, the request to /foos?order[bar.secondLuckyNumber]=asc will work as expected because it doesn't use the nulls_comparison option. It will use the default behavior of Postgres which is NULL considered as largest which is not what I want here.

How to reproduce

For simplicity sake, I've created a reproducer repository which uses the api-platform/api-platform distribution. Here it is : https://github.com/t-richard/order-filter-reproducer

I've also provided Doctrine migrations and Doctrine fixtures in the reproducer.

Once the project is up and the fixtures loaded, you can make an HTTP call to

Possible Solution

I've not managed to find where the problem comes from, but I may be able to provide a PR if you guys can help me out to identify the problem.

Let me know if I can be of any help !

soyuka commented 4 years ago

Could you give a bit more stack trace from the [Syntax Error] line 0, col 80: Error: Expected Doctrine\ORM\Query\Lexer::T_FROM, got '.' error? I suspect that the relation isn't parsed correctly and the DQL isn't properly build.

t-richard commented 4 years ago

Hi @soyuka !

Here is the DQL generated 

SELECT o, CASE WHEN o.bar.luckyNumber IS NULL THEN 0 ELSE 1 END AS HIDDEN _o_bar.luckyNumber_null_rank FROM App\Entity\Foo o ORDER BY _o_bar.luckyNumber_null_rank ASC, o.bar.luckyNumber ASC, o.id ASC

and here is the full trace I get when making the request.

Stack trace

```json { "@context": "/contexts/Error", "@type": "hydra:Error", "hydra:title": "An error occurred", "hydra:description": "[Syntax Error] line 0, col 80: Error: Expected Doctrine\\ORM\\Query\\Lexer::T_FROM, got '.'", "trace": [ { "namespace": "", "short_class": "", "class": "", "type": "", "function": "", "file": "/srv/api/vendor/doctrine/orm/lib/Doctrine/ORM/Query/QueryException.php", "line": 54, "args": [] }, { "namespace": "Doctrine\\ORM\\Query", "short_class": "QueryException", "class": "Doctrine\\ORM\\Query\\QueryException", "type": "::", "function": "syntaxError", "file": "/srv/api/vendor/doctrine/orm/lib/Doctrine/ORM/Query/Parser.php", "line": 457, "args": [ [ "string", "line 0, col 80: Error: Expected Doctrine\\ORM\\Query\\Lexer::T_FROM, got '.'" ], [ "object", "Doctrine\\ORM\\Query\\QueryException" ] ] }, { "namespace": "Doctrine\\ORM\\Query", "short_class": "Parser", "class": "Doctrine\\ORM\\Query\\Parser", "type": "->", "function": "syntaxError", "file": "/srv/api/vendor/doctrine/orm/lib/Doctrine/ORM/Query/Parser.php", "line": 320, "args": [ [ "string", "Doctrine\\ORM\\Query\\Lexer::T_FROM" ] ] }, { "namespace": "Doctrine\\ORM\\Query", "short_class": "Parser", "class": "Doctrine\\ORM\\Query\\Parser", "type": "->", "function": "match", "file": "/srv/api/vendor/doctrine/orm/lib/Doctrine/ORM/Query/Parser.php", "line": 1317, "args": [ [ "integer", 221 ] ] }, { "namespace": "Doctrine\\ORM\\Query", "short_class": "Parser", "class": "Doctrine\\ORM\\Query\\Parser", "type": "->", "function": "FromClause", "file": "/srv/api/vendor/doctrine/orm/lib/Doctrine/ORM/Query/Parser.php", "line": 879, "args": [] }, { "namespace": "Doctrine\\ORM\\Query", "short_class": "Parser", "class": "Doctrine\\ORM\\Query\\Parser", "type": "->", "function": "SelectStatement", "file": "/srv/api/vendor/doctrine/orm/lib/Doctrine/ORM/Query/Parser.php", "line": 848, "args": [] }, { "namespace": "Doctrine\\ORM\\Query", "short_class": "Parser", "class": "Doctrine\\ORM\\Query\\Parser", "type": "->", "function": "QueryLanguage", "file": "/srv/api/vendor/doctrine/orm/lib/Doctrine/ORM/Query/Parser.php", "line": 261, "args": [] }, { "namespace": "Doctrine\\ORM\\Query", "short_class": "Parser", "class": "Doctrine\\ORM\\Query\\Parser", "type": "->", "function": "getAST", "file": "/srv/api/vendor/doctrine/orm/lib/Doctrine/ORM/Query/Parser.php", "line": 360, "args": [] }, { "namespace": "Doctrine\\ORM\\Query", "short_class": "Parser", "class": "Doctrine\\ORM\\Query\\Parser", "type": "->", "function": "parse", "file": "/srv/api/vendor/doctrine/orm/lib/Doctrine/ORM/Query.php", "line": 286, "args": [] }, { "namespace": "Doctrine\\ORM", "short_class": "Query", "class": "Doctrine\\ORM\\Query", "type": "->", "function": "_parse", "file": "/srv/api/vendor/doctrine/orm/lib/Doctrine/ORM/Query.php", "line": 298, "args": [] }, { "namespace": "Doctrine\\ORM", "short_class": "Query", "class": "Doctrine\\ORM\\Query", "type": "->", "function": "_doExecute", "file": "/srv/api/vendor/doctrine/orm/lib/Doctrine/ORM/AbstractQuery.php", "line": 992, "args": [] }, { "namespace": "Doctrine\\ORM", "short_class": "AbstractQuery", "class": "Doctrine\\ORM\\AbstractQuery", "type": "->", "function": "executeIgnoreQueryCache", "file": "/srv/api/vendor/doctrine/orm/lib/Doctrine/ORM/AbstractQuery.php", "line": 947, "args": [ [ "null", null ], [ "integer", 1 ] ] }, { "namespace": "Doctrine\\ORM", "short_class": "AbstractQuery", "class": "Doctrine\\ORM\\AbstractQuery", "type": "->", "function": "execute", "file": "/srv/api/vendor/doctrine/orm/lib/Doctrine/ORM/AbstractQuery.php", "line": 750, "args": [ [ "null", null ], [ "integer", 1 ] ] }, { "namespace": "Doctrine\\ORM", "short_class": "AbstractQuery", "class": "Doctrine\\ORM\\AbstractQuery", "type": "->", "function": "getResult", "file": "/srv/api/vendor/doctrine/orm/lib/Doctrine/ORM/Tools/Pagination/Paginator.php", "line": 177, "args": [ [ "integer", 1 ] ] }, { "namespace": "Doctrine\\ORM\\Tools\\Pagination", "short_class": "Paginator", "class": "Doctrine\\ORM\\Tools\\Pagination\\Paginator", "type": "->", "function": "getIterator", "file": "/srv/api/vendor/api-platform/core/src/Bridge/Doctrine/Orm/AbstractPaginator.php", "line": 69, "args": [] }, { "namespace": "ApiPlatform\\Core\\Bridge\\Doctrine\\Orm", "short_class": "AbstractPaginator", "class": "ApiPlatform\\Core\\Bridge\\Doctrine\\Orm\\AbstractPaginator", "type": "->", "function": "getIterator", "file": "/srv/api/vendor/api-platform/core/src/Hydra/Serializer/CollectionNormalizer.php", "line": 86, "args": [] }, { "namespace": "ApiPlatform\\Core\\Hydra\\Serializer", "short_class": "CollectionNormalizer", "class": "ApiPlatform\\Core\\Hydra\\Serializer\\CollectionNormalizer", "type": "->", "function": "normalize", "file": "/srv/api/vendor/api-platform/core/src/Hydra/Serializer/PartialCollectionViewNormalizer.php", "line": 55, "args": [ [ "object", "ApiPlatform\\Core\\Bridge\\Doctrine\\Orm\\Paginator" ], [ "string", "jsonld" ], [ "array", { "operation_type": [ "string", "collection" ], "collection_operation_name": [ "string", "get" ], "resource_class": [ "string", "App\\Entity\\Foo" ], "input": [ "null", null ], "output": [ "null", null ], "request_uri": [ "string", "/foos?order[bar.luckyNumber]=asc&page=1" ], "uri": [ "string", "https://localhost:8443/foos?order%5Bbar.luckyNumber%5D=asc&page=1" ], "skip_null_values": [ "boolean", true ], "resources": [ "object", "ApiPlatform\\Core\\Serializer\\ResourceList" ], "exclude_from_cache_key": [ "array", [ [ "string", "resources" ], [ "string", "resources_to_push" ] ] ], "resources_to_push": [ "object", "ApiPlatform\\Core\\Serializer\\ResourceList" ], "api_sub_level": [ "boolean", true ], "jsonld_has_context": [ "boolean", true ] } ] ] }, { "namespace": "ApiPlatform\\Core\\Hydra\\Serializer", "short_class": "PartialCollectionViewNormalizer", "class": "ApiPlatform\\Core\\Hydra\\Serializer\\PartialCollectionViewNormalizer", "type": "->", "function": "normalize", "file": "/srv/api/vendor/api-platform/core/src/Hydra/Serializer/CollectionFiltersNormalizer.php", "line": 73, "args": [ [ "object", "ApiPlatform\\Core\\Bridge\\Doctrine\\Orm\\Paginator" ], [ "string", "jsonld" ], [ "array", { "operation_type": [ "string", "collection" ], "collection_operation_name": [ "string", "get" ], "resource_class": [ "string", "App\\Entity\\Foo" ], "input": [ "null", null ], "output": [ "null", null ], "request_uri": [ "string", "/foos?order[bar.luckyNumber]=asc&page=1" ], "uri": [ "string", "https://localhost:8443/foos?order%5Bbar.luckyNumber%5D=asc&page=1" ], "skip_null_values": [ "boolean", true ], "resources": [ "object", "ApiPlatform\\Core\\Serializer\\ResourceList" ], "exclude_from_cache_key": [ "array", [ [ "string", "resources" ], [ "string", "resources_to_push" ] ] ], "resources_to_push": [ "object", "ApiPlatform\\Core\\Serializer\\ResourceList" ] } ] ] }, { "namespace": "ApiPlatform\\Core\\Hydra\\Serializer", "short_class": "CollectionFiltersNormalizer", "class": "ApiPlatform\\Core\\Hydra\\Serializer\\CollectionFiltersNormalizer", "type": "->", "function": "normalize", "file": "/srv/api/vendor/symfony/serializer/Serializer.php", "line": 152, "args": [ [ "object", "ApiPlatform\\Core\\Bridge\\Doctrine\\Orm\\Paginator" ], [ "string", "jsonld" ], [ "array", { "operation_type": [ "string", "collection" ], "collection_operation_name": [ "string", "get" ], "resource_class": [ "string", "App\\Entity\\Foo" ], "input": [ "null", null ], "output": [ "null", null ], "request_uri": [ "string", "/foos?order[bar.luckyNumber]=asc&page=1" ], "uri": [ "string", "https://localhost:8443/foos?order%5Bbar.luckyNumber%5D=asc&page=1" ], "skip_null_values": [ "boolean", true ], "resources": [ "object", "ApiPlatform\\Core\\Serializer\\ResourceList" ], "exclude_from_cache_key": [ "array", [ [ "string", "resources" ], [ "string", "resources_to_push" ] ] ], "resources_to_push": [ "object", "ApiPlatform\\Core\\Serializer\\ResourceList" ] } ] ] }, { "namespace": "Symfony\\Component\\Serializer", "short_class": "Serializer", "class": "Symfony\\Component\\Serializer\\Serializer", "type": "->", "function": "normalize", "file": "/srv/api/vendor/symfony/serializer/Serializer.php", "line": 125, "args": [ [ "object", "ApiPlatform\\Core\\Bridge\\Doctrine\\Orm\\Paginator" ], [ "string", "jsonld" ], [ "array", { "operation_type": [ "string", "collection" ], "collection_operation_name": [ "string", "get" ], "resource_class": [ "string", "App\\Entity\\Foo" ], "input": [ "null", null ], "output": [ "null", null ], "request_uri": [ "string", "/foos?order[bar.luckyNumber]=asc&page=1" ], "uri": [ "string", "https://localhost:8443/foos?order%5Bbar.luckyNumber%5D=asc&page=1" ], "skip_null_values": [ "boolean", true ], "resources": [ "object", "ApiPlatform\\Core\\Serializer\\ResourceList" ], "exclude_from_cache_key": [ "array", [ [ "string", "resources" ], [ "string", "resources_to_push" ] ] ], "resources_to_push": [ "object", "ApiPlatform\\Core\\Serializer\\ResourceList" ] } ] ] }, { "namespace": "Symfony\\Component\\Serializer", "short_class": "Serializer", "class": "Symfony\\Component\\Serializer\\Serializer", "type": "->", "function": "serialize", "file": "/srv/api/vendor/api-platform/core/src/EventListener/SerializeListener.php", "line": 95, "args": [ [ "object", "ApiPlatform\\Core\\Bridge\\Doctrine\\Orm\\Paginator" ], [ "string", "jsonld" ], [ "array", { "operation_type": [ "string", "collection" ], "collection_operation_name": [ "string", "get" ], "resource_class": [ "string", "App\\Entity\\Foo" ], "input": [ "null", null ], "output": [ "null", null ], "request_uri": [ "string", "/foos?order[bar.luckyNumber]=asc&page=1" ], "uri": [ "string", "https://localhost:8443/foos?order%5Bbar.luckyNumber%5D=asc&page=1" ], "skip_null_values": [ "boolean", true ], "resources": [ "object", "ApiPlatform\\Core\\Serializer\\ResourceList" ], "exclude_from_cache_key": [ "array", [ [ "string", "resources" ], [ "string", "resources_to_push" ] ] ], "resources_to_push": [ "object", "ApiPlatform\\Core\\Serializer\\ResourceList" ] } ] ] }, { "namespace": "ApiPlatform\\Core\\EventListener", "short_class": "SerializeListener", "class": "ApiPlatform\\Core\\EventListener\\SerializeListener", "type": "->", "function": "onKernelView", "file": "/srv/api/vendor/symfony/event-dispatcher/Debug/WrappedListener.php", "line": 126, "args": [ [ "object", "Symfony\\Component\\HttpKernel\\Event\\ViewEvent" ], [ "string", "kernel.view" ], [ "object", "Symfony\\Component\\HttpKernel\\Debug\\TraceableEventDispatcher" ] ] }, { "namespace": "Symfony\\Component\\EventDispatcher\\Debug", "short_class": "WrappedListener", "class": "Symfony\\Component\\EventDispatcher\\Debug\\WrappedListener", "type": "->", "function": "__invoke", "file": "/srv/api/vendor/symfony/event-dispatcher/EventDispatcher.php", "line": 264, "args": [ [ "object", "Symfony\\Component\\HttpKernel\\Event\\ViewEvent" ], [ "string", "kernel.view" ], [ "object", "Symfony\\Component\\HttpKernel\\Debug\\TraceableEventDispatcher" ] ] }, { "namespace": "Symfony\\Component\\EventDispatcher", "short_class": "EventDispatcher", "class": "Symfony\\Component\\EventDispatcher\\EventDispatcher", "type": "->", "function": "doDispatch", "file": "/srv/api/vendor/symfony/event-dispatcher/EventDispatcher.php", "line": 239, "args": [ [ "array", [ [ "object", "Symfony\\Component\\EventDispatcher\\Debug\\WrappedListener" ], [ "object", "Symfony\\Component\\EventDispatcher\\Debug\\WrappedListener" ], [ "object", "Symfony\\Component\\EventDispatcher\\Debug\\WrappedListener" ], [ "object", "Symfony\\Component\\EventDispatcher\\Debug\\WrappedListener" ] ] ], [ "string", "kernel.view" ], [ "object", "Symfony\\Component\\HttpKernel\\Event\\ViewEvent" ] ] }, { "namespace": "Symfony\\Component\\EventDispatcher", "short_class": "EventDispatcher", "class": "Symfony\\Component\\EventDispatcher\\EventDispatcher", "type": "->", "function": "callListeners", "file": "/srv/api/vendor/symfony/event-dispatcher/EventDispatcher.php", "line": 73, "args": [ [ "array", [ [ "object", "Symfony\\Component\\EventDispatcher\\Debug\\WrappedListener" ], [ "object", "Symfony\\Component\\EventDispatcher\\Debug\\WrappedListener" ], [ "object", "Symfony\\Component\\EventDispatcher\\Debug\\WrappedListener" ], [ "object", "Symfony\\Component\\EventDispatcher\\Debug\\WrappedListener" ] ] ], [ "string", "kernel.view" ], [ "object", "Symfony\\Component\\HttpKernel\\Event\\ViewEvent" ] ] }, { "namespace": "Symfony\\Component\\EventDispatcher", "short_class": "EventDispatcher", "class": "Symfony\\Component\\EventDispatcher\\EventDispatcher", "type": "->", "function": "dispatch", "file": "/srv/api/vendor/symfony/event-dispatcher/Debug/TraceableEventDispatcher.php", "line": 168, "args": [ [ "object", "Symfony\\Component\\HttpKernel\\Event\\ViewEvent" ], [ "string", "kernel.view" ] ] }, { "namespace": "Symfony\\Component\\EventDispatcher\\Debug", "short_class": "TraceableEventDispatcher", "class": "Symfony\\Component\\EventDispatcher\\Debug\\TraceableEventDispatcher", "type": "->", "function": "dispatch", "file": "/srv/api/vendor/symfony/http-kernel/HttpKernel.php", "line": 151, "args": [ [ "object", "Symfony\\Component\\HttpKernel\\Event\\ViewEvent" ], [ "string", "kernel.view" ] ] }, { "namespace": "Symfony\\Component\\HttpKernel", "short_class": "HttpKernel", "class": "Symfony\\Component\\HttpKernel\\HttpKernel", "type": "->", "function": "handleRaw", "file": "/srv/api/vendor/symfony/http-kernel/HttpKernel.php", "line": 68, "args": [ [ "object", "Symfony\\Component\\HttpFoundation\\Request" ], [ "integer", 1 ] ] }, { "namespace": "Symfony\\Component\\HttpKernel", "short_class": "HttpKernel", "class": "Symfony\\Component\\HttpKernel\\HttpKernel", "type": "->", "function": "handle", "file": "/srv/api/vendor/symfony/http-kernel/Kernel.php", "line": 201, "args": [ [ "object", "Symfony\\Component\\HttpFoundation\\Request" ], [ "integer", 1 ], [ "boolean", true ] ] }, { "namespace": "Symfony\\Component\\HttpKernel", "short_class": "Kernel", "class": "Symfony\\Component\\HttpKernel\\Kernel", "type": "->", "function": "handle", "file": "/srv/api/public/index.php", "line": 25, "args": [ [ "object", "Symfony\\Component\\HttpFoundation\\Request" ] ] } ] } ```

Thanks for the support !

soyuka commented 4 years ago 
o.bar.luckyNumber

Is a bad alias, this needs to be fixed somehow.

vsalvans commented 3 years ago

I've found a solution for that issue and a PR is needed, but the work arround I'm using is to clone the ApiPlatform\Core\Bridge\Doctrine\Orm\Filter\OrderFilter class to a new one in my project namespace and apply this changes

@@ -105,7 +105,7 @@ class OrderFilter extends AbstractContextAwareFilter implements OrderFilterInter
         if (null !== $nullsComparison = $this->properties[$property]['nulls_comparison'] ?? null) {
             $nullsDirection = self::NULLS_DIRECTION_MAP[$nullsComparison][$direction];

-            $nullRankHiddenField = sprintf('_%s_%s_null_rank', $alias, str_replace('.', '_', $field));
+            $nullRankHiddenField = sprintf('_%s_%s_null_rank', $alias, $field);

             $queryBuilder->addSelect(sprintf('CASE WHEN %s.%s IS NULL THEN 0 ELSE 1 END AS HIDDEN %s', $alias, $field, $nullRankHiddenField));
             $queryBuilder->addOrderBy($nullRankHiddenField, $nullsDirection);

And then register the service like so:

    app.doctrine.orm.order_filter:
        class: App\Core\Api\Filter\OrderFilter
        public: false
        arguments:
            - '@doctrine'
            - ~
            - '%api_platform.collection.order_parameter_name%'
            - '@logger'
            - ~
            - '@api_platform.name_converter'

And in case you need to use nulls_comparison in embedded objects try something like this:

    app.customer.order_filter:
        parent: 'app.doctrine.orm.order_filter'
        arguments: { $properties: {
            someEmbeddedValueObject.someDate: :{ nulls_comparison: 'nulls_smallest'},
        }}
        tags: [ { name: 'api_platform.filter', id: 'customer.order_filter' } ]
        autowire:      false
        autoconfigure: false

This is something very temporary, so I need to know how to make a full PR but if someone can do this faster by testing these changes:

--- a/src/Bridge/Doctrine/Orm/Filter/OrderFilter.php
+++ b/src/Bridge/Doctrine/Orm/Filter/OrderFilter.php
@@ -105,7 +105,7 @@ class OrderFilter extends AbstractContextAwareFilter implements OrderFilterInter
         if (null !== $nullsComparison = $this->properties[$property]['nulls_comparison'] ?? null) {
             $nullsDirection = self::NULLS_DIRECTION_MAP[$nullsComparison][$direction];

-            $nullRankHiddenField = sprintf('_%s_%s_null_rank', $alias, str_replace('.', '_', $field));
+            $nullRankHiddenField = sprintf('_%s_%s_null_rank', $alias, $field);

             $queryBuilder->addSelect(sprintf('CASE WHEN %s.%s IS NULL THEN 0 ELSE 1 END AS HIDDEN %s', $alias, $field, $nullRankHiddenField));
             $queryBuilder->addOrderBy($nullRankHiddenField, $nullsDirection);

Hope it helps to someone at the moment!

walva commented 3 years ago

Thank you @vsalvans ! It is helpful !

alanpoulain commented 3 years ago

Fixed by https://github.com/api-platform/core/pull/4151.