Orm: DDC-3174: Query Cache not correct working when using SQLFilter

[doctrinebot]

Jira issue originally created by user benno:

We have an SQLFilter to filter on entities with a specific Trait implemented. The filter is very easy:
bq. $res = $targetTableAlias . '.agency_id = ' . $this->getCurrentAgencyId();
On our system we have the query cache enabled, this works as long the "AgencyId" doesn't change. When the ID changes, the query cache seems to return the wrong (old cache) query.

[doctrinebot]

Comment created by @ocramius:

I'm not sure if this case should be contemplated by the ORM. Filters are low-level and supposed to be stateless (services).

[doctrinebot]

Comment created by benno:

OK, we can disable the query cache for this case. But then should at least the documentation be updated, which explicitly mentions to use filter for locales, which are also not stateless: http://doctrine-orm.readthedocs.org/en/latest/reference/filters.html#example-filter-class

Also in the query cache chapter: http://doctrine-orm.readthedocs.org/en/latest/reference/caching.html#query-cache
bq. It is highly recommended that in a production environment you cache the transformation of a DQL query to its SQL counterpart. It doesn’t make sense to do this parsing multiple times as it doesn’t change unless you alter the DQL query.

[doctrinebot]

Comment created by @ocramius:

[~benno] can you eventually provide a pull request?

[doctrinebot]

Comment created by jamesblizzardbrowserlondon:

I would just like to say that we're having exactly the same issue. I'd love some method (official or not) of having filters being taken into account in this situation.

[doctrinebot]

Comment created by telegu:

I have the same problem when is generated QueryCacheId It consider only the name of active filters and not the value of the filter
This is the code at line 646 of class \Doctrine\ORM\Query
protected function _getQueryCacheId()
{
ksort($this->_hints);

    return md5(
        $this->getDql() . var*export($this->*hints, true) .
        ($this->*em->hasFilters() ? $this->*em->getFilters()->getHash() : '') .
        '&firstResult=' . $this->*firstResult . '&maxResult=' . $this->*maxResults .
        '&hydrationMode='.$this->*hydrationMode.'DOCTRINE_QUERY_CACHE*SALT'
    );
}

[doctrinebot]

Comment created by odiaseo:

I also have the same issue, there are circumstances where filters are dynamic and not stateless particularly when dealing with multi-site / multi-lingual platforms. Is there an appetite for the ORM to take this into consideration.

[doctrinebot]

Comment created by bramstroker:

I have the same issue. We are using SQL filters a lot to filter entities by website and locale. It would be nice if the filter values can be taken into account as well. For now I disabled the query cache in the concerning repositories.

[doctrinebot]

Comment created by csolis:

Same issue here, we are using filters for soft deletion and it would be nice if we can use query cache.

[doctrinebot]

Comment created by tom.pryor:

We are currently working around this by naming the filter based on the value we apply in the filter. So in the agency_id example if we were filtering on an agency_id of 5 we'd name the filter something like 'agency_filter_5'. Would be good if Doctrine took into account the parameter values of the filters when generating the cache id though.

[PastisD]

Hello,

Old issue, I know, but I have the same issue. For now i just disabled the query cache.
Any news about that ? :)

[EmilePerron]

Unless I misunderstood the issue, I believe this has been fixed.

Doctrine takes into account the parameters that are passed to the filter when the query cache is enabled. However, you have to pass those parameters via the filter's setParameter() method, otherwise it will not work. Below is a working example:

# Enable the Doctrine SQL filter for entity locales
$filter = $this->em->getFilters()->enable('localeFilter');
$filter->setParameter('locale', $request->getLocale());

Then, in your filter constraint, you can get the value with the getParameter() method, as such:

public function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAlias)
{
    $locale = $this->getParameter('locale');

    return sprintf('%1$s.locale = %2$s OR %1$s.locale = "" OR %1$s.locale IS NULL', $targetTableAlias, $locale);
}

[DamienHarper]

@beberlei is query cache really usable with doctrine filters as of now (think also about softdeletable filter with no parameter) ?
What about doctrine 3: will filters be available and resulting queries cacheable?

[SenseException]

@DamienHarper Does the example of @EmilePerron not work?

[DamienHarper]

@SenseException No it still doesn't work for me.

[beberlei]

The fix here is to introduce a new final method getHash on SQLFilter that generates a hash based on the called class name and all the parameters, then modify FilterCollection::getHash() to use that.

Tricky bit is probably, how to handle case where parameter is an object, because serializing that would be expensive, and since its part of the query cache, must be stable across multiple requests.

[oojacoboo]

Geez, this was a fun bug to run into. Luckily it hasn't been too dangerous, but could have been much worse, executing queries on wrong records.

Why is addFilterConstraint returning the correct SQL WHERE clause, yet Doctrine is using a different set from the query cache? I stepped through the execution for most of this and it's absolutely clear the filter is returning the correct SQL, but the final composed SQL statement includes another ID value which happens to be the one set after clearing the cache, aka from the first cache warming. Also, disabling the query cache resolves the issue.

We're just going to have to leave query cache disabled until this issue is resolved. Any other insights into this would be appreciated.

[FabienPapet]

I ran into the same issue, looks like disabling cache is the current way to fix this.

My problem was while adding a filter to add and user.company= :company depending on a user's company.

Here's how to reproduce the problem on a Symfony 5.1 with ApiPlatform v2.4

<?php

namespace App\Doctrine\Filter;

use App\Doctrine\Annotation\CompanyAware;
use App\Entity\User;
use Doctrine\Common\Annotations\Reader;
use Doctrine\ORM\Mapping\ClassMetadata;
use Doctrine\ORM\Query\Filter\SQLFilter;
use Symfony\Component\Security\Core\Security;

class CompanyFilter extends SQLFilter
{
    private ?Reader $reader = null;
    private ?Security $security = null;

    public function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAlias): string
    {
        $companyAware = $this->reader->getClassAnnotation($targetEntity->getReflectionClass(), CompanyAware::class);
        if (!$companyAware) {
            return '';
        }

        /** @var User $user */
        $user = $this->security->getUser();

        return sprintf('%s.company_id = %s', $targetTableAlias, $user->getCompany()->getId());
    }

    public function setSecurity(Security $security): void
    {
        $this->security = $security;
    }

    public function setAnnotationReader(Reader $reader): void
    {
        $this->reader = $reader;
    }
}

class CompanyFilterConfigurator
{
    private EntityManagerInterface $em;
    private Reader $reader;
    private Security $security;

    public function __construct(EntityManagerInterface $em, Security $security, Reader $reader)
    {
        $this->em = $em;
        $this->reader = $reader;
        $this->security = $security;
    }

    public function onKernelRequest(): void
    {
        /** @var CompanyFilter $filter */
        $filter = $this->em->getFilters()->enable('company_filter');
        $filter->setAnnotationReader($this->reader);
        $filter->setSecurity($this->security);
    }
}

doctrine configuration :

doctrine:
    orm:
        auto_generate_proxy_classes: false
        metadata_cache_driver:
            type: service
            id: doctrine.system_cache_provider
        query_cache_driver:
            type: service
            id: doctrine.system_cache_provider
        result_cache_driver:
            type: service
            id: doctrine.result_cache_provider

services:
    doctrine.result_cache_provider:
        class: Symfony\Component\Cache\DoctrineProvider
        public: false
        arguments:
            - '@doctrine.result_cache_pool'
    doctrine.system_cache_provider:
        class: Symfony\Component\Cache\DoctrineProvider
        public: false
        arguments:
            - '@doctrine.system_cache_pool'

framework:
    cache:
        pools:
            doctrine.result_cache_pool:
                adapter: cache.app
            doctrine.system_cache_pool:
                adapter: cache.system

[beberlei]

@oojacoboo @FabienPapet see my last comment please, it explains what the problem is and it details how this can be fixed. It should be relatively simple if you want to give it a try.