[DDC-551] Added state of the complete filter collection to the EM

Previously it was sufficient to use the old parser result for a Query if
the DQL didn't change (Query::STATE_CLEAN), but now there must also be
no changes to the filter collection of the EntityManager.

In the old situation each Query object would create a hash of all the
filter objects on it's own. That was not very efficient.

This commit adds the state of the current filter collection to the
EntityManager. The state will be set to FILTERS_STATE_DIRTY as a filter
is enabled, removed or a parameter is set on a filter. The hash is also
computed by the EntityManager, but only if the filter collection is
dirty. This will prevent recalculation of the hash with each query.

lib/Doctrine/ORM/EntityManager.php

@@ -110,6 +110,7 @@ class EntityManager implements ObjectManager
      */
     private $closed = false;
 
+    /* Filters data */
     /**
      * Instances of enabled filters.
      *
@@ -117,6 +118,29 @@ class EntityManager implements ObjectManager
      */
     private $enabledFilters = array();
 
+    /**
+     * @var string The filter hash from the last time the query was parsed.
+     */
+    private $filterHash;
+
+    /* Filter STATES */
+    /**
+     * A filter object is in CLEAN state when it has no changed parameters.
+     */
+    const FILTERS_STATE_CLEAN  = 1;
+
+    /**
+     * A filter object is in DIRTY state when it has changed parameters.
+     */
+    const FILTERS_STATE_DIRTY = 2;
+
+    /**
+     * @var integer $state   The current state of this filter
+     */
+    private $filtersState = self::FILTERS_STATE_CLEAN;
+
+    /* End filters data */
+
     /**
      * Creates a new EntityManager that operates on the given database connection
      * and uses the given Configuration and EventManager implementations.
@@ -762,10 +786,13 @@ class EntityManager implements ObjectManager
         }
 
         if(!isset($this->enabledFilters[$name])) {
-            $this->enabledFilters[$name] = new $filterClass($this->conn);
+            $this->enabledFilters[$name] = new $filterClass($this);
 
             // Keep the enabled filters sorted for the hash
             ksort($this->enabledFilters);
+
+            // Now the filter collection is dirty
+            $this->filtersState = self::FILTERS_STATE_DIRTY;
         }
 
         return $this->enabledFilters[$name];
@@ -779,6 +806,9 @@ class EntityManager implements ObjectManager
 
         unset($this->enabledFilters[$name]);
 
+        // Now the filter collection is dirty
+        $this->filtersState = self::FILTERS_STATE_DIRTY;
+
         return $filter;
     }
 
@@ -791,4 +821,37 @@ class EntityManager implements ObjectManager
 
         return $this->enabledFilters[$name];
     }
+
+    /**
+     * @return boolean True, if the filter collection is clean.
+     */
+    public function isFiltersStateClean()
+    {
+        return self::FILTERS_STATE_CLEAN === $this->filtersState;
+    }
+
+    public function setFiltersStateDirty()
+    {
+        $this->filtersState = self::FILTERS_STATE_DIRTY;
+    }
+
+    /**
+     * Generates a string of currently enabled filters to use for the cache id.
+     *
+     * @return string
+     */
+    public function getFilterHash()
+    {
+        // If there are only clean filters, the previous hash can be returned
+        if(self::FILTERS_STATE_CLEAN === $this->filtersState) {
+            return $this->filterHash;
+        }
+
+        $filterHash = '';
+        foreach($this->enabledFilters as $name => $filter) {
+            $filterHash .= $name . $filter;
+        }
+
+        return $filterHash;
+    }
 }

lib/Doctrine/ORM/Query.php

@@ -198,7 +198,10 @@ final class Query extends AbstractQuery
      */
     private function _parse()
     {
-        if ($this->_state === self::STATE_CLEAN) {
+        // Return previous parser result if the query and the filter collection are both clean
+        if ($this->_state === self::STATE_CLEAN
+            && $this->_em->isFiltersStateClean()
+        ) {
             return $this->_parserResult;
         }
 
@@ -556,32 +559,14 @@ final class Query extends AbstractQuery
     {
         ksort($this->_hints);
 
-
         return md5(
             $this->getDql() . var_export($this->_hints, true) .
-            $this->getFilterHash() .
+            $this->_em->getFilterHash() .
             '&firstResult=' . $this->_firstResult . '&maxResult=' . $this->_maxResults .
             '&hydrationMode='.$this->_hydrationMode.'DOCTRINE_QUERY_CACHE_SALT'
         );
     }
 
-    /**
-     * Generates a string of currently enabled filters to use for the cache id.
-     *
-     * @return string
-     */
-    protected function getFilterHash()
-    {
-        $filterHash = '';
-
-        $filters = $this->_em->getEnabledFilters();
-        foreach($filters as $name => $filter) {
-            $filterHash .= $name . $filter;
-        }
-
-        return $filterHash;
-    }
-
     /**
      * Cleanup Query resource when clone is called.
      *

lib/Doctrine/ORM/Query/Filter/SQLFilter.php

@@ -19,8 +19,8 @@
 
 namespace Doctrine\ORM\Query\Filter;
 
-use Doctrine\DBAL\Connection;
 use Doctrine\ORM\Mapping\ClassMetaData;
+use Doctrine\ORM\EntityManager;
 
 /**
  * The base class that user defined filters should extend.
@@ -33,13 +33,13 @@ use Doctrine\ORM\Mapping\ClassMetaData;
  */
 abstract class SQLFilter
 {
-    private $conn;
+    private $em;
 
     private $parameters;
 
-    final public function __construct(Connection $conn)
+    final public function __construct(EntityManager $em)
     {
-        $this->conn = $conn;
+        $this->em = $em;
     }
 
     final function setParameter($name, $value, $type)
@@ -50,6 +50,9 @@ abstract class SQLFilter
         // Keep the parameters sorted for the hash
         ksort($this->parameters);
 
+        // The filter collection of the EM is now dirty
+        $this->em->setFiltersStateDirty();
+
         return $this;
     }
 
@@ -59,7 +62,7 @@ abstract class SQLFilter
             throw new \InvalidArgumentException("Parameter '" . $name . "' does not exist.");
         }
 
-        return $this->conn->quote($this->parameters[$name]['value'], $this->parameters[$name]['type']);
+        return $this->em->getConnection()->quote($this->parameters[$name]['value'], $this->parameters[$name]['type']);
     }
 
     final function __toString()

tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php

@@ -151,6 +151,16 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase
         return $conn;
     }
 
+    protected function getMockEntityManager()
+    {
+        // Setup connection mock
+        $em = $this->getMockBuilder('Doctrine\ORM\EntityManager')
+            ->disableOriginalConstructor()
+            ->getMock();
+
+        return $em;
+    }
+
     public function testSQLFilterGetSetParameter()
     {
         // Setup mock connection
@@ -160,7 +170,12 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase
             ->with($this->equalTo('en'))
             ->will($this->returnValue("'en'"));
 
-        $filter = new MyLocaleFilter($conn);
+        $em = $this->getMockEntityManager($conn);
+        $em->expects($this->once())
+            ->method('getConnection')
+            ->will($this->returnValue($conn));
+
+        $filter = new MyLocaleFilter($em);
 
         $filter->setParameter('locale', 'en', \Doctrine\DBAL\Types\Type::STRING);
 
@@ -174,7 +189,7 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase
             ->disableOriginalConstructor()
             ->getMock();
 
-        $filter = new MySoftDeleteFilter($this->getMockConnection());
+        $filter = new MySoftDeleteFilter($this->getMockEntityManager());
 
         // Test for an entity that gets extra filter data
         $targetEntity->name = 'MyEntity\SoftDeleteNewsItem';
@@ -188,11 +203,11 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase
 
     public function testSQLFilterToString()
     {
-        $filter = new MyLocaleFilter($this->getMockConnection());
+        $filter = new MyLocaleFilter($this->getMockEntityManager());
         $filter->setParameter('locale', 'en', \Doctrine\DBAL\Types\Type::STRING);
         $filter->setParameter('foo', 'bar', \Doctrine\DBAL\Types\Type::STRING);
 
-        $filter2 = new MyLocaleFilter($this->getMockConnection());
+        $filter2 = new MyLocaleFilter($this->getMockEntityManager());
         $filter2->setParameter('foo', 'bar', \Doctrine\DBAL\Types\Type::STRING);
         $filter2->setParameter('locale', 'en', \Doctrine\DBAL\Types\Type::STRING);
 
@@ -227,6 +242,19 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase
         $this->assertEquals(2, count($cache->getIds()));
     }
 
+    public function testQueryGeneration_DependsOnFilters()
+    {
+        $query = $this->_em->createQuery('select a from Doctrine\Tests\Models\CMS\CmsAddress a');
+        $firstSQLQuery = $query->getSQL();
+
+        $conf = $this->_em->getConfiguration();
+        $conf->addFilter("country", "\Doctrine\Tests\ORM\Functional\CMSCountryFilter");
+        $this->_em->enableFilter("country")
+            ->setParameter("country", "en", \Doctrine\DBAL\Types\Type::getType(\Doctrine\DBAL\Types\Type::STRING)->getBindingType());
+
+        $this->assertNotEquals($firstSQLQuery, $query->getSQL());
+    }
+
     public function testToOneFilter()
     {
         //$this->_em->getConnection()->getConfiguration()->setSQLLogger(new \Doctrine\DBAL\Logging\EchoSQLLogger);