Skip to content
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

Doctrine filters #1185

Closed
djoos opened this issue Jun 19, 2017 · 8 comments
Closed

Doctrine filters #1185

djoos opened this issue Jun 19, 2017 · 8 comments
Labels

Comments

@djoos
Copy link

djoos commented Jun 19, 2017

Hi there,

when implementing a Doctrine filter (eg. UserAware), the results get filtered correctly (hydra:member), however the paginator's query doesn't seem to pick up the filter, so the hydra:totalItems and hydra:view are incorrect...

    "hydra:member": [],
    "hydra:totalItems": 61,
    "hydra:view": {
        "@id": "/foos?page=1",
        "@type": "hydra:PartialCollectionView",
        "hydra:first": "/foos?page=1",
        "hydra:last": "/foos?page=6",
        "hydra:next": "/foos?page=2"
    }

I'll dig deeper into the issue, but any heads up is appreciated - thanks in advance!

Kind regards,
David

@soyuka
Copy link
Member

soyuka commented Jun 19, 2017

My first though was that extension priorities are wrong, but it looks fine (filters are applied before the pagination). To look further you'd need to inspect what's going on here.

Lmk how this turns out. We may want to start with a behat test too see if bug there is.

@djoos
Copy link
Author

djoos commented Jun 20, 2017

A quick update - after picking this up just now again...

I spotted in the PartialConnectionViewNormalizer that $data looked like:

Array
(
    [@context] => /contexts/Project
    [@id] => /projects
    [@type] => hydra:Collection
    [hydra:member] => Array
        (
        )

    [hydra:totalItems] => 61
)

So, hydra:member is correct while hydra:totalItems is incorrect.

Digging deeper, I spotted in the CollectionNormalizer that the $object->getTotalItems() differs from count($object): 61 (incorrect) vs. 0 (correct)

The paginator object used to instantiate the Paginator carries the issue with it already, as the $paginator->getQuery() doesn't respect the Doctrine filter, so the count($paginator) returns an incorrect (as in: unfiltered) count, causing the issues further downwards.

Not sure yet where the src/Bridge/Doctrine/Orm/Paginator-object gets instantiated, but that'll be my next step.

TBC!

@djoos
Copy link
Author

djoos commented Jun 20, 2017

The instantiation of the Paginator happens in the PaginationExtension.

The CollectionDataProvider lists the following extensions:

ApiPlatform\Core\Bridge\Doctrine\Orm\Extension\EagerLoadingExtension
ApiPlatform\Core\Bridge\Doctrine\Orm\Extension\FilterExtension
ApiPlatform\Core\Bridge\Doctrine\Orm\Extension\FilterEagerLoadingExtension
ApiPlatform\Core\Bridge\Doctrine\Orm\Extension\OrderExtension
ApiPlatform\Core\Bridge\Doctrine\Orm\Extension\PaginationExtension

...so, including the PaginationExtension, which kicks in and whose getResult-method is oblivious to the Doctrine filter.
That brings me back to the CollectionDataProvider where the query builder is instantiated...

TBC!

@djoos
Copy link
Author

djoos commented Jun 21, 2017

Some further inspection shows that the Doctrine filter is executed for pagination purposes before my Configurator is called (which sets the annotation reader as well as the field + value to filter upon), hooked up on kernel.request. Obviously the filter returns rather than adding the filter query as required data is not set - when I, instead of letting the Configurator do this upon kernel.request, hard-code values in the filter, the pagination is fixed and everything is ok.

So, why is the Doctrine filter called for pagination purposes in API Platforms' PaginationExtension before kernel.request? kernel.request which is documented as:
"The REQUEST event occurs at the very beginning of request dispatching. This event allows you to create a response for a request before any other code in the framework is executed." #puzzled

@djoos
Copy link
Author

djoos commented Jun 21, 2017

Aha!

API Platform's ReadListener kicks off the ChainCollectionDataProvider > CollectionDataProvider -> PaginationExtension -> Paginator upon kernel.request, with priority 4.

The Doctrine filter's configurator is also tagged with kernel.request, but without using any priority parameter. So, when using Doctrine filters you need to make sure to have it kick in prior to the API Platform's ReadListener: I've upped the priority to > 4 and... voila: the pagination is correct now!

tags:
    - { name: kernel.event_listener, event: kernel.request, priority: 5 } # priority > 4

Result:

    "hydra:member": [],
    "hydra:totalItems": 0,

And "hydra:view" is omitted, as expected.

Hope this helps someone in the future when dealing with Doctrine filters!

@djoos djoos closed this as completed Jun 21, 2017
@Simperfit
Copy link
Contributor

Thanks for this @djoos do you mind creating a PR on the docs about this ?

@djoos
Copy link
Author

djoos commented Jun 21, 2017

Sure, no problem!

dunglas pushed a commit to api-platform/docs that referenced this issue Dec 26, 2017
#228)

* Example Doctrine filter usage, flagging the issue on the priority for the filter to work as expected

More information on Doctrine filters can be found at http://docs.doctrine-project.org/projects/doctrine-orm/en/latest/reference/filters.html
Using a great tutorial on implementing a "UserAware" Doctrine filter in Symfony (http://blog.michaelperrin.fr/2014/12/05/doctrine-filters/), this information not only allows an API Platform dev to implement a Doctrine filter, but also sail around the issue where having a too low priority makes the Pagination go off, as discussed in api-platform/core#1185.
@soullivaneuh
Copy link
Contributor

Should not this priority be automatically set if not specified?

It would be error-less than a line on the doc, in my opinion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants