From 2653d735e230d19b0fcb703800008f3abdb7a3ba Mon Sep 17 00:00:00 2001 From: Alexander Date: Tue, 16 Aug 2011 13:23:53 +0200 Subject: [PATCH] [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 | 65 ++++++++++++++++++- lib/Doctrine/ORM/Query.php | 29 ++------- lib/Doctrine/ORM/Query/Filter/SQLFilter.php | 13 ++-- .../Tests/ORM/Functional/SQLFilterTest.php | 36 ++++++++-- 4 files changed, 111 insertions(+), 32 deletions(-) diff --git a/lib/Doctrine/ORM/EntityManager.php b/lib/Doctrine/ORM/EntityManager.php index 558477ca4..96e013b36 100644 --- a/lib/Doctrine/ORM/EntityManager.php +++ b/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; + } } diff --git a/lib/Doctrine/ORM/Query.php b/lib/Doctrine/ORM/Query.php index 0f4cbf1c1..5a4545ad9 100644 --- a/lib/Doctrine/ORM/Query.php +++ b/lib/Doctrine/ORM/Query.php @@ -198,10 +198,13 @@ 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; } - + // Check query cache. if ($this->_useQueryCache && ($queryCache = $this->getQueryCacheDriver())) { $hash = $this->_getQueryCacheId(); @@ -556,32 +559,14 @@ final class Query extends AbstractQuery { ksort($this->_hints); - return md5( - $this->getDql() . var_export($this->_hints, true) . - $this->getFilterHash() . + $this->getDql() . var_export($this->_hints, true) . + $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. * diff --git a/lib/Doctrine/ORM/Query/Filter/SQLFilter.php b/lib/Doctrine/ORM/Query/Filter/SQLFilter.php index dd90f1789..5a66fb64d 100644 --- a/lib/Doctrine/ORM/Query/Filter/SQLFilter.php +++ b/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() diff --git a/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php b/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php index 997032cbc..f9afc130d 100644 --- a/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php +++ b/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);