From 63a3fb5ad8f48c78990391b8dbceeeb4f8392f72 Mon Sep 17 00:00:00 2001 From: Alexander Date: Thu, 15 Sep 2011 21:34:51 +0200 Subject: [PATCH] [DDC-551] Moved SQLFilter logic to a separate FilterCollection class --- lib/Doctrine/ORM/EntityManager.php | 110 ++----------- .../ORM/Persisters/BasicEntityPersister.php | 2 +- .../ORM/Persisters/ManyToManyPersister.php | 2 +- .../ORM/Persisters/OneToManyPersister.php | 2 +- lib/Doctrine/ORM/Query.php | 2 +- lib/Doctrine/ORM/Query/Filter/SQLFilter.php | 2 +- lib/Doctrine/ORM/Query/FilterCollection.php | 153 ++++++++++++++++++ lib/Doctrine/ORM/Query/SqlWalker.php | 12 +- .../Tests/ORM/Functional/SQLFilterTest.php | 69 +++++--- 9 files changed, 223 insertions(+), 131 deletions(-) create mode 100644 lib/Doctrine/ORM/Query/FilterCollection.php diff --git a/lib/Doctrine/ORM/EntityManager.php b/lib/Doctrine/ORM/EntityManager.php index 96e013b36..dd7b4b218 100644 --- a/lib/Doctrine/ORM/EntityManager.php +++ b/lib/Doctrine/ORM/EntityManager.php @@ -27,7 +27,8 @@ use Closure, Exception, Doctrine\ORM\Mapping\ClassMetadata, Doctrine\ORM\Mapping\ClassMetadataFactory, Doctrine\ORM\Query\ResultSetMapping, - Doctrine\ORM\Proxy\ProxyFactory; + Doctrine\ORM\Proxy\ProxyFactory, + Doctrine\ORM\Query\FilterCollection; /** * The EntityManager is the central access point to ORM functionality. @@ -110,36 +111,12 @@ class EntityManager implements ObjectManager */ private $closed = false; - /* Filters data */ /** - * Instances of enabled filters. + * Collection of query filters. * - * @var array + * @var Doctrine\ORM\Query\FilterCollection */ - 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 */ + private $filterCollection; /** * Creates a new EntityManager that operates on the given database connection @@ -771,55 +748,13 @@ class EntityManager implements ObjectManager return new EntityManager($conn, $config, $conn->getEventManager()); } - /** @return SQLFilter[] */ - public function getEnabledFilters() + public function getFilters() { - return $this->enabledFilters; - } - - /** Throws exception if filter does not exist. No-op if the filter is alrady enabled. - * @return SQLFilter */ - public function enableFilter($name) - { - if(null === $filterClass = $this->config->getFilterClassName($name)) { - throw new \InvalidArgumentException("Filter '" . $name . "' does not exist."); + if(null === $this->filterCollection) { + $this->filterCollection = new FilterCollection($this); } - if(!isset($this->enabledFilters[$name])) { - $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]; - } - - /** Disable the filter, looses the state */ - public function disableFilter($name) - { - // Get the filter to return it - $filter = $this->getFilter($name); - - unset($this->enabledFilters[$name]); - - // Now the filter collection is dirty - $this->filtersState = self::FILTERS_STATE_DIRTY; - - return $filter; - } - - /** throws exception if not in enabled filters */ - public function getFilter($name) - { - if(!isset($this->enabledFilters[$name])) { - throw new \InvalidArgumentException("Filter '" . $name . "' is not enabled."); - } - - return $this->enabledFilters[$name]; + return $this->filterCollection; } /** @@ -827,31 +762,12 @@ class EntityManager implements ObjectManager */ public function isFiltersStateClean() { - return self::FILTERS_STATE_CLEAN === $this->filtersState; + return null === $this->filterCollection + || $this->filterCollection->setFiltersStateDirty(); } - public function setFiltersStateDirty() + public function hasFilters() { - $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; + return null !== $this->filterCollection; } } diff --git a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php index 1d9c03244..4294a7ad8 100644 --- a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php +++ b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php @@ -1448,7 +1448,7 @@ class BasicEntityPersister $filterSql = ''; $first = true; - foreach($this->_em->getEnabledFilters() as $filter) { + foreach($this->_em->getFilters()->getEnabledFilters() as $filter) { if("" !== $filterExpr = $filter->addFilterConstraint($targetEntity, $targetTableAlias)) { if ( ! $first) $sql .= ' AND '; else $first = false; $filterSql .= '(' . $filterExpr . ')'; diff --git a/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php b/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php index ee0b9e1b0..511971a4a 100644 --- a/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php +++ b/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php @@ -318,7 +318,7 @@ class ManyToManyPersister extends AbstractCollectionPersister // Get the SQL for the filters $filterSql = ''; - foreach($this->_em->getEnabledFilters() as $filter) { + foreach($this->_em->getFilters()->getEnabledFilters() as $filter) { if("" !== $filterExpr = $filter->addFilterConstraint($targetClass, 'te')) { $filterSql .= ' AND (' . $filterExpr . ')'; } diff --git a/lib/Doctrine/ORM/Persisters/OneToManyPersister.php b/lib/Doctrine/ORM/Persisters/OneToManyPersister.php index 633de1d94..1d3b03fae 100644 --- a/lib/Doctrine/ORM/Persisters/OneToManyPersister.php +++ b/lib/Doctrine/ORM/Persisters/OneToManyPersister.php @@ -146,7 +146,7 @@ class OneToManyPersister extends AbstractCollectionPersister $sql = "SELECT count(*) FROM " . $targetClass->getQuotedTableName($this->_conn->getDatabasePlatform()) . " t WHERE " . $where; // Apply the filters - foreach($this->_em->getEnabledFilters() as $filter) { + foreach($this->_em->getFilters()->getEnabledFilters() as $filter) { if("" !== $filterExpr = $filter->addFilterConstraint($targetClass, 't')) { $sql .= ' AND (' . $filterExpr . ')'; } diff --git a/lib/Doctrine/ORM/Query.php b/lib/Doctrine/ORM/Query.php index 5a4545ad9..977dbabe7 100644 --- a/lib/Doctrine/ORM/Query.php +++ b/lib/Doctrine/ORM/Query.php @@ -561,7 +561,7 @@ final class Query extends AbstractQuery return md5( $this->getDql() . var_export($this->_hints, true) . - $this->_em->getFilterHash() . + ($this->_em->hasFilters() ? $this->_em->getFilters()->getHash() : '') . '&firstResult=' . $this->_firstResult . '&maxResult=' . $this->_maxResults . '&hydrationMode='.$this->_hydrationMode.'DOCTRINE_QUERY_CACHE_SALT' ); diff --git a/lib/Doctrine/ORM/Query/Filter/SQLFilter.php b/lib/Doctrine/ORM/Query/Filter/SQLFilter.php index 5a66fb64d..1ea27df6e 100644 --- a/lib/Doctrine/ORM/Query/Filter/SQLFilter.php +++ b/lib/Doctrine/ORM/Query/Filter/SQLFilter.php @@ -51,7 +51,7 @@ abstract class SQLFilter ksort($this->parameters); // The filter collection of the EM is now dirty - $this->em->setFiltersStateDirty(); + $this->em->getFilters()->setFiltersStateDirty(); return $this; } diff --git a/lib/Doctrine/ORM/Query/FilterCollection.php b/lib/Doctrine/ORM/Query/FilterCollection.php new file mode 100644 index 000000000..c0a408d60 --- /dev/null +++ b/lib/Doctrine/ORM/Query/FilterCollection.php @@ -0,0 +1,153 @@ +. + */ + +namespace Doctrine\ORM\Query; + +use Doctrine\ORM\Configuration, + Doctrine\ORM\EntityManager; + +/** + * Collection class for all the query filters. + * + * @author Alexander + */ +class FilterCollection +{ + private $config; + private $em; + private $filters; + + /** + * Instances of enabled filters. + * + * @var array + */ + 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; + + final public function __construct(EntityManager $em) + { + $this->em = $em; + $this->config = $em->getConfiguration(); + } + + /** @return SQLFilter[] */ + public function getEnabledFilters() + { + return $this->enabledFilters; + } + + /** Throws exception if filter does not exist. No-op if the filter is alrady enabled. + * @return SQLFilter */ + public function enable($name) + { + if(null === $filterClass = $this->config->getFilterClassName($name)) { + throw new \InvalidArgumentException("Filter '" . $name . "' does not exist."); + } + + if(!isset($this->enabledFilters[$name])) { + $this->enabledFilters[$name] = new $filterClass($this->em); + + // 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]; + } + + /** Disable the filter, looses the state */ + public function disable($name) + { + // Get the filter to return it + $filter = $this->getFilter($name); + + unset($this->enabledFilters[$name]); + + // Now the filter collection is dirty + $this->filtersState = self::FILTERS_STATE_DIRTY; + + return $filter; + } + + /** throws exception if not in enabled filters */ + public function getFilter($name) + { + if(!isset($this->enabledFilters[$name])) { + throw new \InvalidArgumentException("Filter '" . $name . "' is not enabled."); + } + + return $this->enabledFilters[$name]; + } + + /** + * @return boolean True, if the filter collection is clean. + */ + public function isClean() + { + return self::FILTERS_STATE_CLEAN === $this->filtersState; + } + + /** + * Generates a string of currently enabled filters to use for the cache id. + * + * @return string + */ + public function getHash() + { + // 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; + } + + public function setFiltersStateDirty() + { + $this->filtersState = self::FILTERS_STATE_DIRTY; + } +} diff --git a/lib/Doctrine/ORM/Query/SqlWalker.php b/lib/Doctrine/ORM/Query/SqlWalker.php index 833f7d4d4..b12b65e17 100644 --- a/lib/Doctrine/ORM/Query/SqlWalker.php +++ b/lib/Doctrine/ORM/Query/SqlWalker.php @@ -354,11 +354,13 @@ class SqlWalker implements TreeWalker { $filterSql = ''; - $first = true; - foreach($this->_em->getEnabledFilters() as $filter) { - if("" !== $filterExpr = $filter->addFilterConstraint($targetEntity, $targetTableAlias)) { - if ( ! $first) $sql .= ' AND '; else $first = false; - $filterSql .= '(' . $filterExpr . ')'; + if($this->_em->hasFilters()) { + $first = true; + foreach($this->_em->getFilters()->getEnabledFilters() as $filter) { + if("" !== $filterExpr = $filter->addFilterConstraint($targetEntity, $targetTableAlias)) { + if ( ! $first) $sql .= ' AND '; else $first = false; + $filterSql .= '(' . $filterExpr . ')'; + } } } diff --git a/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php b/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php index fc6312d97..6954ef489 100644 --- a/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php @@ -58,17 +58,17 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase $this->configureFilters($em); // Enable an existing filter - $filter = $em->enableFilter("locale"); + $filter = $em->getFilters()->enable("locale"); $this->assertTrue($filter instanceof \Doctrine\Tests\ORM\Functional\MyLocaleFilter); // Enable the filter again - $filter2 = $em->enableFilter("locale"); + $filter2 = $em->getFilters()->enable("locale"); $this->assertEquals($filter, $filter2); // Enable a non-existing filter $exceptionThrown = false; try { - $filter = $em->enableFilter("foo"); + $filter = $em->getFilters()->enable("foo"); } catch (\InvalidArgumentException $e) { $exceptionThrown = true; } @@ -80,14 +80,14 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase $em = $this->_getEntityManager(); // No enabled filters - $this->assertEquals(array(), $em->getEnabledFilters()); + $this->assertEquals(array(), $em->getFilters()->getEnabledFilters()); $this->configureFilters($em); - $filter = $em->enableFilter("locale"); - $filter = $em->enableFilter("soft_delete"); + $filter = $em->getFilters()->enable("locale"); + $filter = $em->getFilters()->enable("soft_delete"); // Two enabled filters - $this->assertEquals(2, count($em->getEnabledFilters())); + $this->assertEquals(2, count($em->getFilters()->getEnabledFilters())); } @@ -97,16 +97,16 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase $this->configureFilters($em); // Enable the filter - $filter = $em->enableFilter("locale"); + $filter = $em->getFilters()->enable("locale"); // Disable it - $this->assertEquals($filter, $em->disableFilter("locale")); - $this->assertEquals(0, count($em->getEnabledFilters())); + $this->assertEquals($filter, $em->getFilters()->disable("locale")); + $this->assertEquals(0, count($em->getFilters()->getEnabledFilters())); // Disable a non-existing filter $exceptionThrown = false; try { - $filter = $em->disableFilter("foo"); + $filter = $em->getFilters()->disable("foo"); } catch (\InvalidArgumentException $e) { $exceptionThrown = true; } @@ -115,7 +115,7 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase // Disable a non-enabled filter $exceptionThrown = false; try { - $filter = $em->disableFilter("locale"); + $filter = $em->getFilters()->disable("locale"); } catch (\InvalidArgumentException $e) { $exceptionThrown = true; } @@ -128,15 +128,15 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase $this->configureFilters($em); // Enable the filter - $filter = $em->enableFilter("locale"); + $filter = $em->getFilters()->enable("locale"); // Get the filter - $this->assertEquals($filter, $em->getFilter("locale")); + $this->assertEquals($filter, $em->getFilters()->getFilter("locale")); // Get a non-enabled filter $exceptionThrown = false; try { - $filter = $em->getFilter("soft_delete"); + $filter = $em->getFilters()->getFilter("soft_delete"); } catch (\InvalidArgumentException $e) { $exceptionThrown = true; } @@ -171,6 +171,19 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase return $em; } + protected function addMockFilterCollection($em) + { + $filterCollection = $this->getMockBuilder('Doctrine\ORM\Query\FilterCollection') + ->disableOriginalConstructor() + ->getMock(); + + $em->expects($this->any()) + ->method('getFilters') + ->will($this->returnValue($filterCollection)); + + return $filterCollection; + } + public function testSQLFilterGetSetParameter() { // Setup mock connection @@ -185,6 +198,11 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase ->method('getConnection') ->will($this->returnValue($conn)); + $filterCollection = $this->addMockFilterCollection($em); + $filterCollection + ->expects($this->once()) + ->method('setFiltersStateDirty'); + $filter = new MyLocaleFilter($em); $filter->setParameter('locale', 'en', \Doctrine\DBAL\Types\Type::STRING); @@ -213,11 +231,14 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase public function testSQLFilterToString() { - $filter = new MyLocaleFilter($this->getMockEntityManager()); + $em = $this->getMockEntityManager(); + $filterCollection = $this->addMockFilterCollection($em); + + $filter = new MyLocaleFilter($em); $filter->setParameter('locale', 'en', \Doctrine\DBAL\Types\Type::STRING); $filter->setParameter('foo', 'bar', \Doctrine\DBAL\Types\Type::STRING); - $filter2 = new MyLocaleFilter($this->getMockEntityManager()); + $filter2 = new MyLocaleFilter($em); $filter2->setParameter('foo', 'bar', \Doctrine\DBAL\Types\Type::STRING); $filter2->setParameter('locale', 'en', \Doctrine\DBAL\Types\Type::STRING); @@ -242,7 +263,7 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase $conf = $this->_em->getConfiguration(); $conf->addFilter("locale", "\Doctrine\Tests\ORM\Functional\MyLocaleFilter"); - $this->_em->enableFilter("locale"); + $this->_em->getFilters()->enable("locale"); $query->getResult(); $this->assertEquals(2, count($cache->getIds())); @@ -259,7 +280,7 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase $conf = $this->_em->getConfiguration(); $conf->addFilter("country", "\Doctrine\Tests\ORM\Functional\CMSCountryFilter"); - $this->_em->enableFilter("country") + $this->_em->getFilters()->enable("country") ->setParameter("country", "en", \Doctrine\DBAL\Types\Type::getType(\Doctrine\DBAL\Types\Type::STRING)->getBindingType()); $this->assertNotEquals($firstSQLQuery, $query->getSQL()); @@ -277,7 +298,7 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase $conf = $this->_em->getConfiguration(); $conf->addFilter("country", "\Doctrine\Tests\ORM\Functional\CMSCountryFilter"); - $this->_em->enableFilter("country")->setParameter("country", "Germany", \Doctrine\DBAL\Types\Type::getType(\Doctrine\DBAL\Types\Type::STRING)->getBindingType()); + $this->_em->getFilters()->enable("country")->setParameter("country", "Germany", \Doctrine\DBAL\Types\Type::getType(\Doctrine\DBAL\Types\Type::STRING)->getBindingType()); // We get one user after enabling the filter $this->assertEquals(1, count($query->getResult())); @@ -293,7 +314,7 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase $conf = $this->_em->getConfiguration(); $conf->addFilter("group_prefix", "\Doctrine\Tests\ORM\Functional\CMSGroupPrefixFilter"); - $this->_em->enableFilter("group_prefix")->setParameter("prefix", "bar_%", \Doctrine\DBAL\Types\Type::getType(\Doctrine\DBAL\Types\Type::STRING)->getBindingType()); + $this->_em->getFilters()->enable("group_prefix")->setParameter("prefix", "bar_%", \Doctrine\DBAL\Types\Type::getType(\Doctrine\DBAL\Types\Type::STRING)->getBindingType()); // We get one user after enabling the filter $this->assertEquals(1, count($query->getResult())); @@ -310,7 +331,7 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase $conf = $this->_em->getConfiguration(); $conf->addFilter("group_prefix", "\Doctrine\Tests\ORM\Functional\CMSGroupPrefixFilter"); - $this->_em->enableFilter("group_prefix")->setParameter("prefix", "bar_%", \Doctrine\DBAL\Types\Type::getType(\Doctrine\DBAL\Types\Type::STRING)->getBindingType()); + $this->_em->getFilters()->enable("group_prefix")->setParameter("prefix", "bar_%", \Doctrine\DBAL\Types\Type::getType(\Doctrine\DBAL\Types\Type::STRING)->getBindingType()); // We get one user after enabling the filter $this->assertEquals(1, count($query->getResult())); @@ -329,7 +350,7 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase { $conf = $this->_em->getConfiguration(); $conf->addFilter("article_topic", "\Doctrine\Tests\ORM\Functional\CMSArticleTopicFilter"); - $this->_em->enableFilter("article_topic")->setParameter("topic", "Test1", \Doctrine\DBAL\Types\Type::getType(\Doctrine\DBAL\Types\Type::STRING)->getBindingType()); + $this->_em->getFilters()->enable("article_topic")->setParameter("topic", "Test1", \Doctrine\DBAL\Types\Type::getType(\Doctrine\DBAL\Types\Type::STRING)->getBindingType()); } public function testOneToMany_ExtraLazyCountWithFilter() @@ -376,7 +397,7 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase { $conf = $this->_em->getConfiguration(); $conf->addFilter("group_prefix", "\Doctrine\Tests\ORM\Functional\CMSGroupPrefixFilter"); - $this->_em->enableFilter("group_prefix")->setParameter("prefix", "foo%", \Doctrine\DBAL\Types\Type::getType(\Doctrine\DBAL\Types\Type::STRING)->getBindingType()); + $this->_em->getFilters()->enable("group_prefix")->setParameter("prefix", "foo%", \Doctrine\DBAL\Types\Type::getType(\Doctrine\DBAL\Types\Type::STRING)->getBindingType()); } public function testManyToMany_ExtraLazyCountWithFilter()