From a85902b08d8b7a55fb8c61369baf4af779a8ab25 Mon Sep 17 00:00:00 2001 From: Alexander Date: Fri, 22 Jul 2011 12:01:33 +0200 Subject: [PATCH 01/49] [DDC-551] Initial code for filter functionality --- lib/Doctrine/ORM/Configuration.php | 14 ++++- lib/Doctrine/ORM/EntityManager.php | 49 +++++++++++++++++ lib/Doctrine/ORM/Query/Filter/SQLFilter.php | 61 +++++++++++++++++++++ 3 files changed, 123 insertions(+), 1 deletion(-) create mode 100644 lib/Doctrine/ORM/Query/Filter/SQLFilter.php diff --git a/lib/Doctrine/ORM/Configuration.php b/lib/Doctrine/ORM/Configuration.php index a7219da8b..6e48ac19b 100644 --- a/lib/Doctrine/ORM/Configuration.php +++ b/lib/Doctrine/ORM/Configuration.php @@ -515,4 +515,16 @@ class Configuration extends \Doctrine\DBAL\Configuration } return $this->_attributes['classMetadataFactoryName']; } -} \ No newline at end of file + + public function addFilter($name, $className) + { + $this->_attributes['filters'][strtolower($name)] = $className; + } + + public function getFilterClassName($name) + { + $name = strtolower($name); + return isset($this->_attributes['filters'][$name]) ? + $this->_attributes['filters'][$name] : null; + } +} diff --git a/lib/Doctrine/ORM/EntityManager.php b/lib/Doctrine/ORM/EntityManager.php index 0379cc435..c127eefff 100644 --- a/lib/Doctrine/ORM/EntityManager.php +++ b/lib/Doctrine/ORM/EntityManager.php @@ -110,6 +110,13 @@ class EntityManager implements ObjectManager */ private $closed = false; + /** + * Instances of enabled filters. + * + * @var array + */ + private $enabledFilters; + /** * Creates a new EntityManager that operates on the given database connection * and uses the given Configuration and EventManager implementations. @@ -739,4 +746,46 @@ class EntityManager implements ObjectManager return new EntityManager($conn, $config, $conn->getEventManager()); } + + /** @return SQLFilter[] */ + public function getEnabledFilters() + { + return $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->getFilter($name)) { + throw new \InvalidArgumentException("Filter '" . $name . "' is does not exist."); + } + + if(!isset($this->enabledFilters[$name])) { + $this->enabledFilters[$name] = new $filterClass($this->conn); + } + + 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]); + + 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]; + } } diff --git a/lib/Doctrine/ORM/Query/Filter/SQLFilter.php b/lib/Doctrine/ORM/Query/Filter/SQLFilter.php new file mode 100644 index 000000000..454c554c1 --- /dev/null +++ b/lib/Doctrine/ORM/Query/Filter/SQLFilter.php @@ -0,0 +1,61 @@ +. + */ + +namespace Doctrine\ORM\Query\Filter; + +use Doctrine\DBAL\Connection; + +/** + * The base class that user defined filters should extend. + * + * Handles the setting and escaping of parameters. + * + * @author Alexander + * @author Benjamin Eberlei + * @abstract + */ +abstract class SQLFilter +{ + private $conn; + + private $parameters; + + final public function __construct(Connection $conn) + { + $this->conn = $conn; + } + + final function setParameter($name, $value, $type) + { + // @todo: check for a valid type? + $parameters[$name] = array('value' => $value, 'type' => $type); + } + + final function getParameter($name) + { + if(!isset($parameters[$name])) { + throw new \InvalidArgumentException("Parameter '" . $name . "' is does not exist."); + } + + // @todo: espace the parameter + return $paramaters[$name]['value']; + } + + abstract function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAlias); +} From b867744f15b40e19604056481c388a7606f1a728 Mon Sep 17 00:00:00 2001 From: Alexander Date: Fri, 22 Jul 2011 13:08:49 +0200 Subject: [PATCH 02/49] [DDC-551] Added tests for SQLFilter functionality + small fixes --- lib/Doctrine/ORM/EntityManager.php | 8 +- lib/Doctrine/ORM/Query/Filter/SQLFilter.php | 1 + .../Tests/ORM/Functional/SQLFilterTest.php | 154 ++++++++++++++++++ 3 files changed, 159 insertions(+), 4 deletions(-) create mode 100644 tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php diff --git a/lib/Doctrine/ORM/EntityManager.php b/lib/Doctrine/ORM/EntityManager.php index c127eefff..2605e0cec 100644 --- a/lib/Doctrine/ORM/EntityManager.php +++ b/lib/Doctrine/ORM/EntityManager.php @@ -115,7 +115,7 @@ class EntityManager implements ObjectManager * * @var array */ - private $enabledFilters; + private $enabledFilters = array(); /** * Creates a new EntityManager that operates on the given database connection @@ -750,15 +750,15 @@ class EntityManager implements ObjectManager /** @return SQLFilter[] */ public function getEnabledFilters() { - return $enabledFilters; + 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->getFilter($name)) { - throw new \InvalidArgumentException("Filter '" . $name . "' is does not exist."); + if(null === $filterClass = $this->config->getFilterClassName($name)) { + throw new \InvalidArgumentException("Filter '" . $name . "' does not exist."); } if(!isset($this->enabledFilters[$name])) { diff --git a/lib/Doctrine/ORM/Query/Filter/SQLFilter.php b/lib/Doctrine/ORM/Query/Filter/SQLFilter.php index 454c554c1..9f7307445 100644 --- a/lib/Doctrine/ORM/Query/Filter/SQLFilter.php +++ b/lib/Doctrine/ORM/Query/Filter/SQLFilter.php @@ -20,6 +20,7 @@ namespace Doctrine\ORM\Query\Filter; use Doctrine\DBAL\Connection; +use Doctrine\ORM\Mapping\ClassMetaData; /** * The base class that user defined filters should extend. diff --git a/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php b/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php new file mode 100644 index 000000000..3cb3ce426 --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php @@ -0,0 +1,154 @@ + + */ +class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase +{ + public function setUp() + { + parent::setUp(); + } + + public function testConfigureFilter() + { + $config = new \Doctrine\ORM\Configuration(); + + $config->addFilter("locale", "\Doctrine\Tests\ORM\Functional\MyLocaleFilter"); + + $this->assertEquals("\Doctrine\Tests\ORM\Functional\MyLocaleFilter", $config->getFilterClassName("locale")); + $this->assertNull($config->getFilterClassName("foo")); + } + + public function testEntityManagerEnableFilter() + { + $em = $this->_getEntityManager(); + $this->configureFilters($em); + + // Enable an existing filter + $filter = $em->enableFilter("locale"); + $this->assertTrue($filter instanceof \Doctrine\Tests\ORM\Functional\MyLocaleFilter); + + // Enable the filter again + $filter2 = $em->enableFilter("locale"); + $this->assertEquals($filter, $filter2); + + // Enable a non-existing filter + $exceptionThrown = false; + try { + $filter = $em->enableFilter("foo"); + } catch (\InvalidArgumentException $e) { + $exceptionThrown = true; + } + $this->assertTrue($exceptionThrown); + } + + public function testEntityManagerEnabledFilters() + { + $em = $this->_getEntityManager(); + + // No enabled filters + $this->assertEquals(array(), $em->getEnabledFilters()); + + $this->configureFilters($em); + $filter = $em->enableFilter("locale"); + $filter = $em->enableFilter("soft_delete"); + + // Two enabled filters + $this->assertEquals(2, count($em->getEnabledFilters())); + + } + + public function testEntityManagerDisableFilter() + { + $em = $this->_getEntityManager(); + $this->configureFilters($em); + + // Enable the filter + $filter = $em->enableFilter("locale"); + + // Disable it + $this->assertEquals($filter, $em->disableFilter("locale")); + $this->assertEquals(0, count($em->getEnabledFilters())); + + // Disable a non-existing filter + $exceptionThrown = false; + try { + $filter = $em->disableFilter("foo"); + } catch (\InvalidArgumentException $e) { + $exceptionThrown = true; + } + $this->assertTrue($exceptionThrown); + + // Disable a non-enabled filter + $exceptionThrown = false; + try { + $filter = $em->disableFilter("locale"); + } catch (\InvalidArgumentException $e) { + $exceptionThrown = true; + } + $this->assertTrue($exceptionThrown); + } + + public function testEntityManagerGetFilter() + { + $em = $this->_getEntityManager(); + $this->configureFilters($em); + + // Enable the filter + $filter = $em->enableFilter("locale"); + + // Get the filter + $this->assertEquals($filter, $em->getFilter("locale")); + + // Get a non-enabled filter + $exceptionThrown = false; + try { + $filter = $em->getFilter("soft_delete"); + } catch (\InvalidArgumentException $e) { + $exceptionThrown = true; + } + $this->assertTrue($exceptionThrown); + } + + protected function configureFilters($em) + { + // Add filters to the configuration of the EM + $config = $em->getConfiguration(); + $config->addFilter("locale", "\Doctrine\Tests\ORM\Functional\MyLocaleFilter"); + $config->addFilter("soft_delete", "\Doctrine\Tests\ORM\Functional\MySoftDeleteFilter"); + } +} + +class MySoftDeleteFilter extends SQLFilter +{ + public function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAlias) + { + if ($targetEntity->name != "MyEntity\SoftDeleteNewsItem") { + return ""; + } + + return $targetTableAlias.'.deleted = 0'; + } +} + +class MyLocaleFilter extends SQLFilter +{ + public function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAlias) + { + if (!in_array("LocaleAware", $targetEntity->reflClass->getInterfaceNames())) { + return ""; + } + + return $targetTableAlias.'.locale = ' . $this->getParam('locale'); // getParam uses connection to quote the value. + } +} From 277fc751b6cbcc906e1986947b2b4fee3ff6f382 Mon Sep 17 00:00:00 2001 From: Alexander Date: Fri, 22 Jul 2011 13:51:26 +0200 Subject: [PATCH 03/49] [DDC-551] Added tests for SQLFilter --- lib/Doctrine/ORM/Query/Filter/SQLFilter.php | 10 ++-- .../Tests/ORM/Functional/SQLFilterTest.php | 46 +++++++++++++++++++ 2 files changed, 52 insertions(+), 4 deletions(-) diff --git a/lib/Doctrine/ORM/Query/Filter/SQLFilter.php b/lib/Doctrine/ORM/Query/Filter/SQLFilter.php index 9f7307445..9477636e3 100644 --- a/lib/Doctrine/ORM/Query/Filter/SQLFilter.php +++ b/lib/Doctrine/ORM/Query/Filter/SQLFilter.php @@ -45,17 +45,19 @@ abstract class SQLFilter final function setParameter($name, $value, $type) { // @todo: check for a valid type? - $parameters[$name] = array('value' => $value, 'type' => $type); + $this->parameters[$name] = array('value' => $value, 'type' => $type); + + return $this; } final function getParameter($name) { - if(!isset($parameters[$name])) { - throw new \InvalidArgumentException("Parameter '" . $name . "' is does not exist."); + if(!isset($this->parameters[$name])) { + throw new \InvalidArgumentException("Parameter '" . $name . "' does not exist."); } // @todo: espace the parameter - return $paramaters[$name]['value']; + return $this->conn->convertToDatabaseValue($this->parameters[$name]['value'], $this->parameters[$name]['type']); } abstract function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAlias); diff --git a/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php b/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php index 3cb3ce426..482ddb534 100644 --- a/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php @@ -4,6 +4,7 @@ namespace Doctrine\Tests\ORM\Functional; use Doctrine\ORM\Query\Filter\SQLFilter; use Doctrine\ORM\Mapping\ClassMetaData; +use Doctrine\DBAL\Types\Type; require_once __DIR__ . '/../../TestInit.php'; @@ -127,6 +128,51 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase $config->addFilter("locale", "\Doctrine\Tests\ORM\Functional\MyLocaleFilter"); $config->addFilter("soft_delete", "\Doctrine\Tests\ORM\Functional\MySoftDeleteFilter"); } + + protected function getMockConnection() + { + // Setup connection mock + $conn = $this->getMockBuilder('Doctrine\DBAL\Connection') + ->disableOriginalConstructor() + ->getMock(); + + return $conn; + } + + public function testSQLFilterGetSetParameter() + { + // Setup mock connection + $conn = $this->getMockConnection(); + $conn->expects($this->once()) + ->method('convertToDatabaseValue') + ->with($this->equalTo('en')) + ->will($this->returnValue("'en'")); + + $filter = new MyLocaleFilter($conn); + + $filter->setParameter('locale', 'en', Type::STRING); + + $this->assertEquals("'en'", $filter->getParameter('locale')); + } + + public function testSQLFilterAddConstraint() + { + // Set up metadata mock + $targetEntity = $this->getMockBuilder('Doctrine\ORM\Mapping\ClassMetadata') + ->disableOriginalConstructor() + ->getMock(); + + $filter = new MySoftDeleteFilter($this->getMockConnection()); + + // Test for an entity that gets extra filter data + $targetEntity->name = 'MyEntity\SoftDeleteNewsItem'; + $this->assertEquals('t1_.deleted = 0', $filter->addFilterConstraint($targetEntity, 't1_')); + + // Test for an entity that doesn't get extra filter data + $targetEntity->name = 'MyEntity\NoSoftDeleteNewsItem'; + $this->assertEquals('', $filter->addFilterConstraint($targetEntity, 't1_')); + + } } class MySoftDeleteFilter extends SQLFilter From d1908f7207565a3cdd47ee42dd73a38642cde519 Mon Sep 17 00:00:00 2001 From: Alexander Date: Fri, 22 Jul 2011 14:20:36 +0200 Subject: [PATCH 04/49] [DDC-551] Keep filter parameters and enabled filters sorted for hashing --- lib/Doctrine/ORM/EntityManager.php | 3 +++ lib/Doctrine/ORM/Query/Filter/SQLFilter.php | 3 +++ 2 files changed, 6 insertions(+) diff --git a/lib/Doctrine/ORM/EntityManager.php b/lib/Doctrine/ORM/EntityManager.php index 2605e0cec..558477ca4 100644 --- a/lib/Doctrine/ORM/EntityManager.php +++ b/lib/Doctrine/ORM/EntityManager.php @@ -763,6 +763,9 @@ class EntityManager implements ObjectManager if(!isset($this->enabledFilters[$name])) { $this->enabledFilters[$name] = new $filterClass($this->conn); + + // Keep the enabled filters sorted for the hash + ksort($this->enabledFilters); } return $this->enabledFilters[$name]; diff --git a/lib/Doctrine/ORM/Query/Filter/SQLFilter.php b/lib/Doctrine/ORM/Query/Filter/SQLFilter.php index 9477636e3..16887fd4d 100644 --- a/lib/Doctrine/ORM/Query/Filter/SQLFilter.php +++ b/lib/Doctrine/ORM/Query/Filter/SQLFilter.php @@ -47,6 +47,9 @@ abstract class SQLFilter // @todo: check for a valid type? $this->parameters[$name] = array('value' => $value, 'type' => $type); + // Keep the parameters sorted for the hash + ksort($this->parameters); + return $this; } From 4cf63a4e83665ab674c2e95630f031490d6c42a6 Mon Sep 17 00:00:00 2001 From: Alexander Date: Fri, 22 Jul 2011 14:48:20 +0200 Subject: [PATCH 05/49] [DDC-551] Fixed the escaping of filter parameters --- lib/Doctrine/ORM/Query/Filter/SQLFilter.php | 3 +-- tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/Doctrine/ORM/Query/Filter/SQLFilter.php b/lib/Doctrine/ORM/Query/Filter/SQLFilter.php index 16887fd4d..4ca31d64d 100644 --- a/lib/Doctrine/ORM/Query/Filter/SQLFilter.php +++ b/lib/Doctrine/ORM/Query/Filter/SQLFilter.php @@ -59,8 +59,7 @@ abstract class SQLFilter throw new \InvalidArgumentException("Parameter '" . $name . "' does not exist."); } - // @todo: espace the parameter - return $this->conn->convertToDatabaseValue($this->parameters[$name]['value'], $this->parameters[$name]['type']); + return $this->conn->quote($this->parameters[$name]['value'], $this->parameters[$name]['type']); } abstract function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAlias); diff --git a/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php b/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php index 482ddb534..a58fef3bf 100644 --- a/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php @@ -144,7 +144,7 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase // Setup mock connection $conn = $this->getMockConnection(); $conn->expects($this->once()) - ->method('convertToDatabaseValue') + ->method('quote') ->with($this->equalTo('en')) ->will($this->returnValue("'en'")); From 4266ab77b219242455c8d46a93f81e2dcf13b143 Mon Sep 17 00:00:00 2001 From: Alexander Date: Fri, 22 Jul 2011 14:55:00 +0200 Subject: [PATCH 06/49] [DDC-551] Added __toString() method to SQLFilter --- lib/Doctrine/ORM/Query/Filter/SQLFilter.php | 5 +++++ .../Tests/ORM/Functional/SQLFilterTest.php | 19 +++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/lib/Doctrine/ORM/Query/Filter/SQLFilter.php b/lib/Doctrine/ORM/Query/Filter/SQLFilter.php index 4ca31d64d..2ac413788 100644 --- a/lib/Doctrine/ORM/Query/Filter/SQLFilter.php +++ b/lib/Doctrine/ORM/Query/Filter/SQLFilter.php @@ -62,5 +62,10 @@ abstract class SQLFilter return $this->conn->quote($this->parameters[$name]['value'], $this->parameters[$name]['type']); } + final function __toString() + { + return serialize($this->parameters); + } + abstract function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAlias); } diff --git a/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php b/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php index a58fef3bf..6cc1a75be 100644 --- a/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php @@ -173,6 +173,25 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase $this->assertEquals('', $filter->addFilterConstraint($targetEntity, 't1_')); } + + public function testSQLFilterToString() + { + $filter = new MyLocaleFilter($this->getMockConnection()); + $filter->setParameter('locale', 'en', TYPE::STRING); + $filter->setParameter('foo', 'bar', TYPE::STRING); + + $filter2 = new MyLocaleFilter($this->getMockConnection()); + $filter2->setParameter('foo', 'bar', TYPE::STRING); + $filter2->setParameter('locale', 'en', TYPE::STRING); + + $parameters = array( + 'foo' => array('value' => 'bar', 'type' => TYPE::STRING), + 'locale' => array('value' => 'en', 'type' => TYPE::STRING), + ); + + $this->assertEquals(serialize($parameters), ''.$filter); + $this->assertEquals(''.$filter, ''.$filter2); + } } class MySoftDeleteFilter extends SQLFilter From afd7a540a7f5c01767d15f18a519978ff1562028 Mon Sep 17 00:00:00 2001 From: Alexander Date: Fri, 22 Jul 2011 15:10:31 +0200 Subject: [PATCH 07/49] [DDC-551] Removed 'use ..DBAL\..\Type', causing full testsuite to fail --- .../Tests/ORM/Functional/SQLFilterTest.php | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php b/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php index 6cc1a75be..7e6470cfc 100644 --- a/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php @@ -4,7 +4,6 @@ namespace Doctrine\Tests\ORM\Functional; use Doctrine\ORM\Query\Filter\SQLFilter; use Doctrine\ORM\Mapping\ClassMetaData; -use Doctrine\DBAL\Types\Type; require_once __DIR__ . '/../../TestInit.php'; @@ -150,7 +149,7 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase $filter = new MyLocaleFilter($conn); - $filter->setParameter('locale', 'en', Type::STRING); + $filter->setParameter('locale', 'en', \Doctrine\DBAL\Types\Type::STRING); $this->assertEquals("'en'", $filter->getParameter('locale')); } @@ -177,16 +176,16 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase public function testSQLFilterToString() { $filter = new MyLocaleFilter($this->getMockConnection()); - $filter->setParameter('locale', 'en', TYPE::STRING); - $filter->setParameter('foo', 'bar', TYPE::STRING); + $filter->setParameter('locale', 'en', \Doctrine\DBAL\Types\Type::STRING); + $filter->setParameter('foo', 'bar', \Doctrine\DBAL\Types\Type::STRING); $filter2 = new MyLocaleFilter($this->getMockConnection()); - $filter2->setParameter('foo', 'bar', TYPE::STRING); - $filter2->setParameter('locale', 'en', TYPE::STRING); + $filter2->setParameter('foo', 'bar', \Doctrine\DBAL\Types\Type::STRING); + $filter2->setParameter('locale', 'en', \Doctrine\DBAL\Types\Type::STRING); $parameters = array( - 'foo' => array('value' => 'bar', 'type' => TYPE::STRING), - 'locale' => array('value' => 'en', 'type' => TYPE::STRING), + 'foo' => array('value' => 'bar', 'type' => \Doctrine\DBAL\Types\Type::STRING), + 'locale' => array('value' => 'en', 'type' => \Doctrine\DBAL\Types\Type::STRING), ); $this->assertEquals(serialize($parameters), ''.$filter); From 6163d9d932104fcf17912674e40926310b4a5c88 Mon Sep 17 00:00:00 2001 From: Alexander Date: Fri, 22 Jul 2011 15:26:03 +0200 Subject: [PATCH 08/49] [DDC-551] Added enabled filters to Query hash --- lib/Doctrine/ORM/Query.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/Doctrine/ORM/Query.php b/lib/Doctrine/ORM/Query.php index 820711d55..e413a9260 100644 --- a/lib/Doctrine/ORM/Query.php +++ b/lib/Doctrine/ORM/Query.php @@ -558,6 +558,7 @@ final class Query extends AbstractQuery return md5( $this->getDql() . var_export($this->_hints, true) . + var_export($this->_em->getEnabledFilters(), true) . '&firstResult=' . $this->_firstResult . '&maxResult=' . $this->_maxResults . '&hydrationMode='.$this->_hydrationMode.'DOCTRINE_QUERY_CACHE_SALT' ); @@ -573,4 +574,4 @@ final class Query extends AbstractQuery parent::__clone(); $this->_state = self::STATE_DIRTY; } -} \ No newline at end of file +} From e3dcfa870257fdb1965e5d6ac74f7b1d91684b6d Mon Sep 17 00:00:00 2001 From: Alexander Date: Fri, 22 Jul 2011 17:01:18 +0200 Subject: [PATCH 09/49] [DDC-551] Added filters to query hash + tests for hash --- lib/Doctrine/ORM/Query.php | 20 +++++++++++++++- .../Tests/ORM/Functional/SQLFilterTest.php | 24 +++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/lib/Doctrine/ORM/Query.php b/lib/Doctrine/ORM/Query.php index e413a9260..0f4cbf1c1 100644 --- a/lib/Doctrine/ORM/Query.php +++ b/lib/Doctrine/ORM/Query.php @@ -556,14 +556,32 @@ final class Query extends AbstractQuery { ksort($this->_hints); + return md5( $this->getDql() . var_export($this->_hints, true) . - var_export($this->_em->getEnabledFilters(), true) . + $this->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/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php b/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php index 7e6470cfc..ac9bae0c7 100644 --- a/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php @@ -4,6 +4,7 @@ namespace Doctrine\Tests\ORM\Functional; use Doctrine\ORM\Query\Filter\SQLFilter; use Doctrine\ORM\Mapping\ClassMetaData; +use Doctrine\Common\Cache\ArrayCache; require_once __DIR__ . '/../../TestInit.php'; @@ -16,6 +17,7 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase { public function setUp() { + $this->useModelSet('cms'); parent::setUp(); } @@ -191,6 +193,28 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase $this->assertEquals(serialize($parameters), ''.$filter); $this->assertEquals(''.$filter, ''.$filter2); } + + public function testQueryCache_DependsOnFilters() + { + $query = $this->_em->createQuery('select ux from Doctrine\Tests\Models\CMS\CmsUser ux'); + + $cache = new ArrayCache(); + $query->setQueryCacheDriver($cache); + + $query->getResult(); + $this->assertEquals(1, count($cache->getIds())); + + $conf = $this->_em->getConfiguration(); + $conf->addFilter("locale", "\Doctrine\Tests\ORM\Functional\MyLocaleFilter"); + $this->_em->enableFilter("locale"); + + $query->getResult(); + $this->assertEquals(2, count($cache->getIds())); + + return $query; + } + + } class MySoftDeleteFilter extends SQLFilter From 3b1ddb034675b80baef427c8cea0f63b91f3a372 Mon Sep 17 00:00:00 2001 From: Alexander Date: Tue, 16 Aug 2011 12:03:47 +0200 Subject: [PATCH 10/49] [DDC-551] Added filters to SQLWalker --- lib/Doctrine/ORM/Query/Filter/SQLFilter.php | 3 + lib/Doctrine/ORM/Query/SqlWalker.php | 46 +++++ .../Tests/ORM/Functional/SQLFilterTest.php | 164 +++++++++++++++++- 3 files changed, 211 insertions(+), 2 deletions(-) diff --git a/lib/Doctrine/ORM/Query/Filter/SQLFilter.php b/lib/Doctrine/ORM/Query/Filter/SQLFilter.php index 2ac413788..dd90f1789 100644 --- a/lib/Doctrine/ORM/Query/Filter/SQLFilter.php +++ b/lib/Doctrine/ORM/Query/Filter/SQLFilter.php @@ -67,5 +67,8 @@ abstract class SQLFilter return serialize($this->parameters); } + /** + * @return string The contstraint if there is one, empty string otherwise + */ abstract function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAlias); } diff --git a/lib/Doctrine/ORM/Query/SqlWalker.php b/lib/Doctrine/ORM/Query/SqlWalker.php index 7393ba893..8810aed87 100644 --- a/lib/Doctrine/ORM/Query/SqlWalker.php +++ b/lib/Doctrine/ORM/Query/SqlWalker.php @@ -350,6 +350,20 @@ class SqlWalker implements TreeWalker return ($encapsulate) ? '(' . $sql . ')' : $sql; } + private function generateFilterConditionSQL(ClassMetadata $targetEntity, $targetTableAlias) + { + $filterSql = ''; + + $first = true; + foreach($this->_em->getEnabledFilters() as $filter) { + if("" !== $filterExpr = $filter->addFilterConstraint($targetEntity, $targetTableAlias)) { + if ( ! $first) $sql .= ' AND '; else $first = false; + $filterSql .= '(' . $filterExpr . ')'; + } + } + + return $filterSql; + } /** * Walks down a SelectStatement AST node, thereby generating the appropriate SQL. * @@ -771,6 +785,12 @@ class SqlWalker implements TreeWalker $sql .= $sourceTableAlias . '.' . $quotedTargetColumn . ' = ' . $targetTableAlias . '.' . $sourceColumn; } } + + // Apply the filters + if("" !== $filterExpr = $this->generateFilterConditionSQL($targetClass, $targetTableAlias)) { + if ( ! $first) $sql .= ' AND '; else $first = false; + $sql .= $filterExpr; + } } else if ($assoc['type'] == ClassMetadata::MANY_TO_MANY) { // Join relation table $joinTable = $assoc['joinTable']; @@ -835,6 +855,12 @@ class SqlWalker implements TreeWalker $sql .= $targetTableAlias . '.' . $quotedTargetColumn . ' = ' . $joinTableAlias . '.' . $relationColumn; } } + + // Apply the filters + if("" !== $filterExpr = $this->generateFilterConditionSQL($targetClass, $targetTableAlias)) { + if ( ! $first) $sql .= ' AND '; else $first = false; + $sql .= $filterExpr; + } } // Handle WITH clause @@ -1414,6 +1440,26 @@ class SqlWalker implements TreeWalker $condSql = null !== $whereClause ? $this->walkConditionalExpression($whereClause->conditionalExpression) : ''; $discrSql = $this->_generateDiscriminatorColumnConditionSql($this->_rootAliases); + $filterSql = ''; + + // Apply the filters for all entities in FROM + $first = true; + foreach ($this->_rootAliases as $dqlAlias) { + $class = $this->_queryComponents[$dqlAlias]['metadata']; + $tableAlias = $this->getSQLTableAlias($class->table['name'], $dqlAlias); + + if("" !== $filterExpr = $this->generateFilterConditionSQL($class, $tableAlias)) { + if ( ! $first) $filterSql .= ' AND '; else $first = false; + $filterSql .= $filterExpr; + } + } + + if ('' !== $filterSql) { + if($condSql) $condSql .= ' AND '; + + $condSql .= $filterSql; + } + if ($condSql) { return ' WHERE ' . (( ! $discrSql) ? $condSql : '(' . $condSql . ') AND ' . $discrSql); } else if ($discrSql) { diff --git a/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php b/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php index ac9bae0c7..997032cbc 100644 --- a/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php @@ -6,6 +6,15 @@ use Doctrine\ORM\Query\Filter\SQLFilter; use Doctrine\ORM\Mapping\ClassMetaData; use Doctrine\Common\Cache\ArrayCache; +use Doctrine\ORM\Mapping\ClassMetadataInfo; + +use Doctrine\Tests\Models\CMS\CmsUser; +use Doctrine\Tests\Models\CMS\CmsPhonenumber; +use Doctrine\Tests\Models\CMS\CmsAddress; +use Doctrine\Tests\Models\CMS\CmsGroup; +use Doctrine\Tests\Models\CMS\CmsArticle; +use Doctrine\Tests\Models\CMS\CmsComment; + require_once __DIR__ . '/../../TestInit.php'; /** @@ -15,6 +24,8 @@ require_once __DIR__ . '/../../TestInit.php'; */ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase { + private $userId, $userId2, $articleId, $articleId2; + public function setUp() { $this->useModelSet('cms'); @@ -211,10 +222,124 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase $query->getResult(); $this->assertEquals(2, count($cache->getIds())); - return $query; + // Another time doesn't add another cache entry + $query->getResult(); + $this->assertEquals(2, count($cache->getIds())); + } + + public function testToOneFilter() + { + //$this->_em->getConnection()->getConfiguration()->setSQLLogger(new \Doctrine\DBAL\Logging\EchoSQLLogger); + $this->loadFixtureData(); + + $query = $this->_em->createQuery('select ux, ua from Doctrine\Tests\Models\CMS\CmsUser ux JOIN ux.address ua'); + + // We get two users before enabling the filter + $this->assertEquals(2, count($query->getResult())); + + $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()); + + // We get one user after enabling the filter + $this->assertEquals(1, count($query->getResult())); + } + + public function testManyToManyFilter() + { + $this->loadFixtureData(); + $query = $this->_em->createQuery('select ux, ug from Doctrine\Tests\Models\CMS\CmsUser ux JOIN ux.groups ug'); + + // We get two users before enabling the filter + $this->assertEquals(2, count($query->getResult())); + + $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()); + + // We get one user after enabling the filter + $this->assertEquals(1, count($query->getResult())); + + } + + public function testWhereFilter() + { + $this->loadFixtureData(); + $query = $this->_em->createQuery('select ug from Doctrine\Tests\Models\CMS\CmsGroup ug WHERE 1=1'); + + // We get two users before enabling the filter + $this->assertEquals(2, count($query->getResult())); + + $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()); + + // We get one user after enabling the filter + $this->assertEquals(1, count($query->getResult())); } + private function loadFixtureData() + { + $user = new CmsUser; + $user->name = 'Roman'; + $user->username = 'romanb'; + $user->status = 'developer'; + + $address = new CmsAddress; + $address->country = 'Germany'; + $address->city = 'Berlin'; + $address->zip = '12345'; + + $user->address = $address; // inverse side + $address->user = $user; // owning side! + + $group = new CmsGroup; + $group->name = 'foo_group'; + $user->addGroup($group); + + $article1 = new CmsArticle; + $article1->topic = "Test1"; + $article1->text = "Test"; + $article1->setAuthor($user); + + $article2 = new CmsArticle; + $article2->topic = "Test2"; + $article2->text = "Test"; + $article2->setAuthor($user); + + $this->_em->persist($article1); + $this->_em->persist($article2); + + $this->_em->persist($user); + + $user2 = new CmsUser; + $user2->name = 'Guilherme'; + $user2->username = 'gblanco'; + $user2->status = 'developer'; + + $address2 = new CmsAddress; + $address2->country = 'France'; + $address2->city = 'Paris'; + $address2->zip = '12345'; + + $user->address = $address2; // inverse side + $address2->user = $user2; // owning side! + + $user2->addGroup($group); + $group2 = new CmsGroup; + $group2->name = 'bar_group'; + $user2->addGroup($group2); + + $this->_em->persist($user2); + $this->_em->flush(); + $this->_em->clear(); + + $this->userId = $user->getId(); + $this->userId2 = $user2->getId(); + $this->articleId = $article1->id; + $this->articleId2 = $article2->id; + } } class MySoftDeleteFilter extends SQLFilter @@ -237,6 +362,41 @@ class MyLocaleFilter extends SQLFilter return ""; } - return $targetTableAlias.'.locale = ' . $this->getParam('locale'); // getParam uses connection to quote the value. + return $targetTableAlias.'.locale = ' . $this->getParameter('locale'); // getParam uses connection to quote the value. + } +} + +class CMSCountryFilter extends SQLFilter +{ + public function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAlias) + { + if ($targetEntity->name != "Doctrine\Tests\Models\CMS\CmsAddress") { + return ""; + } + + return $targetTableAlias.'.country = ' . $this->getParameter('country'); // getParam uses connection to quote the value. + } +} + +class CMSGroupPrefixFilter extends SQLFilter +{ + public function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAlias) + { + if ($targetEntity->name != "Doctrine\Tests\Models\CMS\CmsGroup") { + return ""; + } + + return $targetTableAlias.'.name LIKE ' . $this->getParameter('prefix'); // getParam uses connection to quote the value. + } +} +class CMSArticleTopicFilter extends SQLFilter +{ + public function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAlias) + { + if ($targetEntity->name != "Doctrine\Tests\Models\CMS\CmsArticle") { + return ""; + } + + return $targetTableAlias.'.topic = ' . $this->getParameter('topic'); // getParam uses connection to quote the value. } } From 2653d735e230d19b0fcb703800008f3abdb7a3ba Mon Sep 17 00:00:00 2001 From: Alexander Date: Tue, 16 Aug 2011 13:23:53 +0200 Subject: [PATCH 11/49] [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); From 380058194736e3eb6f219195a06fe60e1480039e Mon Sep 17 00:00:00 2001 From: Alexander Date: Tue, 16 Aug 2011 16:24:50 +0200 Subject: [PATCH 12/49] [DDC-551] Altered persisters to make filters work with EXTRA_LAZY associations --- .../ORM/Persisters/BasicEntityPersister.php | 35 +++++- .../ORM/Persisters/ManyToManyPersister.php | 51 +++++++- .../ORM/Persisters/OneToManyPersister.php | 12 +- .../Tests/ORM/Functional/SQLFilterTest.php | 115 ++++++++++++++++++ 4 files changed, 205 insertions(+), 8 deletions(-) diff --git a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php index 19da2e200..9da8e95fc 100644 --- a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php +++ b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php @@ -861,9 +861,17 @@ class BasicEntityPersister $lockSql = ' ' . $this->_platform->getWriteLockSql(); } + $alias = $this->_getSQLTableAlias($this->_class->name); + + $filterSql = $this->generateFilterConditionSQL($this->_class, $alias); + if('' !== $filterSql) { + if($conditionSql) $conditionSql .= ' AND '; + $conditionSql .= $filterSql; + } + return $this->_platform->modifyLimitQuery('SELECT ' . $this->_getSelectColumnListSQL() . $this->_platform->appendLockHint(' FROM ' . $this->_class->getQuotedTableName($this->_platform) . ' ' - . $this->_getSQLTableAlias($this->_class->name), $lockMode) + . $alias, $lockMode) . $this->_selectJoinSql . $joinSql . ($conditionSql ? ' WHERE ' . $conditionSql : '') . $orderBySql, $limit, $offset) @@ -1340,10 +1348,33 @@ class BasicEntityPersister $criteria = array_merge($criteria, $extraConditions); } + $alias = $this->_getSQLTableAlias($this->_class->name); + + $sql = 'SELECT 1 FROM ' . $this->_class->getQuotedTableName($this->_platform) - . ' ' . $this->_getSQLTableAlias($this->_class->name) + . ' ' . $alias . ' WHERE ' . $this->_getSelectConditionSQL($criteria); + $filterSql = $this->generateFilterConditionSQL($this->_class, $alias); + if('' !== $filterSql) { + $sql .= ' AND ' . $filterSql; + } + return (bool) $this->_conn->fetchColumn($sql, array_values($criteria)); } + + private function generateFilterConditionSQL(ClassMetadata $targetEntity, $targetTableAlias) + { + $filterSql = ''; + + $first = true; + foreach($this->_em->getEnabledFilters() as $filter) { + if("" !== $filterExpr = $filter->addFilterConstraint($targetEntity, $targetTableAlias)) { + if ( ! $first) $sql .= ' AND '; else $first = false; + $filterSql .= '(' . $filterExpr . ')'; + } + } + + return $filterSql; + } } diff --git a/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php b/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php index 6ca2b15a5..ee0b9e1b0 100644 --- a/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php +++ b/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php @@ -208,7 +208,7 @@ class ManyToManyPersister extends AbstractCollectionPersister if ($whereClause !== '') { $whereClause .= ' AND '; } - $whereClause .= "$joinTableColumn = ?"; + $whereClause .= "t.$joinTableColumn = ?"; if ($class->containsForeignIdentifier) { $params[] = $id[$class->getFieldForColumn($joinColumns[$joinTableColumn])]; @@ -217,7 +217,14 @@ class ManyToManyPersister extends AbstractCollectionPersister } } } - $sql = 'SELECT count(*) FROM ' . $joinTable['name'] . ' WHERE ' . $whereClause; + + list($joinTargetEntitySQL, $filterSql) = $this->getFilterSql($mapping); + + $sql = 'SELECT count(*)' + . ' FROM ' . $joinTable['name'] . ' t' + . $joinTargetEntitySQL + . ' WHERE ' . $whereClause + . $filterSql; return $this->_conn->fetchColumn($sql, $params); } @@ -293,8 +300,44 @@ class ManyToManyPersister extends AbstractCollectionPersister } } } - $sql = 'SELECT 1 FROM ' . $joinTable['name'] . ' WHERE ' . $whereClause; + + list($joinTargetEntitySQL, $filterSql) = $this->getFilterSql($mapping); + + $sql = 'SELECT 1' + . ' FROM ' . $joinTable['name'] . ' t' + . $joinTargetEntitySQL + . ' WHERE ' . $whereClause + . $filterSql; return (bool)$this->_conn->fetchColumn($sql, $params); } -} \ No newline at end of file + + public function getFilterSql($mapping) + { + $targetClass = $this->_em->getClassMetadata($mapping['targetEntity']); + + // Get the SQL for the filters + $filterSql = ''; + foreach($this->_em->getEnabledFilters() as $filter) { + if("" !== $filterExpr = $filter->addFilterConstraint($targetClass, 'te')) { + $filterSql .= ' AND (' . $filterExpr . ')'; + } + } + + // A join is needed if there is filtering on the target entity + $joinTargetEntitySQL = ''; + if('' !== $filterSql) { + $joinTargetEntitySQL = ' JOIN ' + . $targetClass->getQuotedTableName($this->_conn->getDatabasePlatform()) . ' te' + . ' ON'; + + $first = true; + foreach($mapping['relationToTargetKeyColumns'] as $joinTableColumn => $targetTableColumn) { + if(!$first) $joinTargetEntitySQL .= ' AND '; else $first = false; + $joinTargetEntitySQL .= ' t.' . $joinTableColumn . ' = ' . 'te.' . $targetTableColumn; + } + } + + return array($joinTargetEntitySQL, $filterSql); + } +} diff --git a/lib/Doctrine/ORM/Persisters/OneToManyPersister.php b/lib/Doctrine/ORM/Persisters/OneToManyPersister.php index 5e889ddb9..2dfd18e79 100644 --- a/lib/Doctrine/ORM/Persisters/OneToManyPersister.php +++ b/lib/Doctrine/ORM/Persisters/OneToManyPersister.php @@ -133,7 +133,7 @@ class OneToManyPersister extends AbstractCollectionPersister if ($where != '') { $where .= ' AND '; } - $where .= $joinColumn['name'] . " = ?"; + $where .= 't.' . $joinColumn['name'] . " = ?"; if ($class->containsForeignIdentifier) { $params[] = $id[$class->getFieldForColumn($joinColumn['referencedColumnName'])]; } else { @@ -141,7 +141,15 @@ class OneToManyPersister extends AbstractCollectionPersister } } - $sql = "SELECT count(*) FROM " . $class->getQuotedTableName($this->_conn->getDatabasePlatform()) . " WHERE " . $where; + $sql = "SELECT count(*) FROM " . $class->getQuotedTableName($this->_conn->getDatabasePlatform()) . " t WHERE " . $where; + + // Apply the filters + foreach($this->_em->getEnabledFilters() as $filter) { + if("" !== $filterExpr = $filter->addFilterConstraint($class, 't')) { + $sql .= ' AND (' . $filterExpr . ')'; + } + } + return $this->_conn->fetchColumn($sql, $params); } diff --git a/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php b/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php index f9afc130d..fc6312d97 100644 --- a/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php @@ -25,6 +25,7 @@ require_once __DIR__ . '/../../TestInit.php'; class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase { private $userId, $userId2, $articleId, $articleId2; + private $groupId, $groupId2; public function setUp() { @@ -32,6 +33,15 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase parent::setUp(); } + public function tearDown() + { + parent::tearDown(); + + $class = $this->_em->getClassMetadata('Doctrine\Tests\Models\CMS\CmsUser'); + $class->associationMappings['groups']['fetch'] = ClassMetadataInfo::FETCH_LAZY; + $class->associationMappings['articles']['fetch'] = ClassMetadataInfo::FETCH_LAZY; + } + public function testConfigureFilter() { $config = new \Doctrine\ORM\Configuration(); @@ -307,6 +317,109 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase } + private function loadLazyFixtureData() + { + $class = $this->_em->getClassMetadata('Doctrine\Tests\Models\CMS\CmsUser'); + $class->associationMappings['articles']['fetch'] = ClassMetadataInfo::FETCH_EXTRA_LAZY; + $class->associationMappings['groups']['fetch'] = ClassMetadataInfo::FETCH_EXTRA_LAZY; + $this->loadFixtureData(); + } + + private function useCMSArticleTopicFilter() + { + $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()); + } + + public function testOneToMany_ExtraLazyCountWithFilter() + { + $this->loadLazyFixtureData(); + $user = $this->_em->find('Doctrine\Tests\Models\CMS\CmsUser', $this->userId); + + $this->assertFalse($user->articles->isInitialized()); + $this->assertEquals(2, count($user->articles)); + + $this->useCMSArticleTopicFilter(); + + $this->assertEquals(1, count($user->articles)); + } + + public function testOneToMany_ExtraLazyContainsWithFilter() + { + $this->loadLazyFixtureData(); + $user = $this->_em->find('Doctrine\Tests\Models\CMS\CmsUser', $this->userId); + $filteredArticle = $this->_em->find('Doctrine\Tests\Models\CMS\CmsArticle', $this->articleId2); + + $this->assertFalse($user->articles->isInitialized()); + $this->assertTrue($user->articles->contains($filteredArticle)); + + $this->useCMSArticleTopicFilter(); + + $this->assertFalse($user->articles->contains($filteredArticle)); + } + + public function testOneToMany_ExtraLazySliceWithFilter() + { + $this->loadLazyFixtureData(); + $user = $this->_em->find('Doctrine\Tests\Models\CMS\CmsUser', $this->userId); + + $this->assertFalse($user->articles->isInitialized()); + $this->assertEquals(2, count($user->articles->slice(0,10))); + + $this->useCMSArticleTopicFilter(); + + $this->assertEquals(1, count($user->articles->slice(0,10))); + } + + private function useCMSGroupPrefixFilter() + { + $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()); + } + + public function testManyToMany_ExtraLazyCountWithFilter() + { + $this->loadLazyFixtureData(); + + $user = $this->_em->find('Doctrine\Tests\Models\CMS\CmsUser', $this->userId2); + + $this->assertFalse($user->groups->isInitialized()); + $this->assertEquals(2, count($user->groups)); + + $this->useCMSGroupPrefixFilter(); + + $this->assertEquals(1, count($user->groups)); + } + + public function testManyToMany_ExtraLazyContainsWithFilter() + { + $this->loadLazyFixtureData(); + $user = $this->_em->find('Doctrine\Tests\Models\CMS\CmsUser', $this->userId2); + $filteredArticle = $this->_em->find('Doctrine\Tests\Models\CMS\CmsGroup', $this->groupId2); + + $this->assertFalse($user->groups->isInitialized()); + $this->assertTrue($user->groups->contains($filteredArticle)); + + $this->useCMSGroupPrefixFilter(); + + $this->assertFalse($user->groups->contains($filteredArticle)); + } + + public function testManyToMany_ExtraLazySliceWithFilter() + { + $this->loadLazyFixtureData(); + $user = $this->_em->find('Doctrine\Tests\Models\CMS\CmsUser', $this->userId2); + + $this->assertFalse($user->groups->isInitialized()); + $this->assertEquals(2, count($user->groups->slice(0,10))); + + $this->useCMSGroupPrefixFilter(); + + $this->assertEquals(1, count($user->groups->slice(0,10))); + } + private function loadFixtureData() { $user = new CmsUser; @@ -367,6 +480,8 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase $this->userId2 = $user2->getId(); $this->articleId = $article1->id; $this->articleId2 = $article2->id; + $this->groupId = $group->id; + $this->groupId2 = $group2->id; } } From 63a3fb5ad8f48c78990391b8dbceeeb4f8392f72 Mon Sep 17 00:00:00 2001 From: Alexander Date: Thu, 15 Sep 2011 21:34:51 +0200 Subject: [PATCH 13/49] [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() From 58b381bf24087d18e0952d8c3267a4f1cbc395dc Mon Sep 17 00:00:00 2001 From: Alexander Date: Sat, 15 Oct 2011 22:31:20 +0200 Subject: [PATCH 14/49] [DDC-551] use isClean to check the filterCollection state.. --- lib/Doctrine/ORM/EntityManager.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Doctrine/ORM/EntityManager.php b/lib/Doctrine/ORM/EntityManager.php index fba3e2de4..c5e89c9ae 100644 --- a/lib/Doctrine/ORM/EntityManager.php +++ b/lib/Doctrine/ORM/EntityManager.php @@ -764,7 +764,7 @@ class EntityManager implements ObjectManager public function isFiltersStateClean() { return null === $this->filterCollection - || $this->filterCollection->setFiltersStateDirty(); + || $this->filterCollection->isClean(); } public function hasFilters() From 9ccce8ed742067270a5ec0436cd0d46127296633 Mon Sep 17 00:00:00 2001 From: Alexander Date: Wed, 26 Oct 2011 16:11:27 +0200 Subject: [PATCH 15/49] [DDC-551] Add filters to eagerly joined entities in the persisters --- lib/Doctrine/ORM/Persisters/BasicEntityPersister.php | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php index 51cce15a4..737a7c1de 100644 --- a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php +++ b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php @@ -1020,10 +1020,17 @@ class BasicEntityPersister $this->_selectJoinSql .= ' AND '; } + $tableAlias = $this->_getSQLTableAlias($assoc['targetEntity'], $assocAlias); $this->_selectJoinSql .= $this->_getSQLTableAlias($assoc['sourceEntity']) . '.' . $sourceCol . ' = ' - . $this->_getSQLTableAlias($assoc['targetEntity'], $assocAlias) . '.' . $targetCol . ' '; + . $tableAlias . '.' . $targetCol . ' '; + $first = false; } + + // Add filter SQL + if('' !== $filterSql = $this->generateFilterConditionSQL($eagerEntity, $tableAlias)) { + $this->_selectJoinSql .= ' AND ' . $filterSql; + } } else { $eagerEntity = $this->_em->getClassMetadata($assoc['targetEntity']); $owningAssoc = $eagerEntity->getAssociationMapping($assoc['mappedBy']); From 53055f1fb2f0d9f825876b73ffaf403ebc7ff8fe Mon Sep 17 00:00:00 2001 From: Alexander Date: Tue, 1 Nov 2011 12:36:06 +0100 Subject: [PATCH 16/49] [DDC-551] Fixed a bug in the sql generation for filters --- lib/Doctrine/ORM/Persisters/BasicEntityPersister.php | 2 +- lib/Doctrine/ORM/Query/SqlWalker.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php index 737a7c1de..7b234ca58 100644 --- a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php +++ b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php @@ -1529,7 +1529,7 @@ class BasicEntityPersister $first = true; foreach($this->_em->getFilters()->getEnabledFilters() as $filter) { if("" !== $filterExpr = $filter->addFilterConstraint($targetEntity, $targetTableAlias)) { - if ( ! $first) $sql .= ' AND '; else $first = false; + if ( ! $first) $filterSql .= ' AND '; else $first = false; $filterSql .= '(' . $filterExpr . ')'; } } diff --git a/lib/Doctrine/ORM/Query/SqlWalker.php b/lib/Doctrine/ORM/Query/SqlWalker.php index 485708386..fa994da2c 100644 --- a/lib/Doctrine/ORM/Query/SqlWalker.php +++ b/lib/Doctrine/ORM/Query/SqlWalker.php @@ -358,7 +358,7 @@ class SqlWalker implements TreeWalker $first = true; foreach($this->_em->getFilters()->getEnabledFilters() as $filter) { if("" !== $filterExpr = $filter->addFilterConstraint($targetEntity, $targetTableAlias)) { - if ( ! $first) $sql .= ' AND '; else $first = false; + if ( ! $first) $filterSql .= ' AND '; else $first = false; $filterSql .= '(' . $filterExpr . ')'; } } From 4c94a7ccc5404f1288603c341ddab02c6dd78ba3 Mon Sep 17 00:00:00 2001 From: Alexander Date: Wed, 30 Nov 2011 16:40:55 +0100 Subject: [PATCH 17/49] [DDC-551] Various minor fixes after merge and cleanup --- .../ORM/Persisters/BasicEntityPersister.php | 5 +- .../Persisters/JoinedSubclassPersister.php | 4 +- lib/Doctrine/ORM/Query/Filter/SQLFilter.php | 10 +-- lib/Doctrine/ORM/Query/FilterCollection.php | 71 ++++++++++++++----- .../ORM/Functional/Ticket/DDC633Test.php | 2 +- 5 files changed, 64 insertions(+), 28 deletions(-) diff --git a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php index 77cab7c5b..1f4550d8c 100644 --- a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php +++ b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php @@ -1020,6 +1020,7 @@ class BasicEntityPersister if ($assoc['isOwningSide']) { $this->_selectJoinSql .= ' ' . $this->getJoinSQLForJoinColumns($assoc['joinColumns']); + $this->_selectJoinSql .= ' ' . $eagerEntity->getQuotedTableName($this->_platform) . ' ' . $this->_getSQLTableAlias($eagerEntity->name, $assocAlias) .' ON '; foreach ($assoc['sourceToTargetKeyColumns'] AS $sourceCol => $targetCol) { if ( ! $first) { @@ -1027,7 +1028,7 @@ class BasicEntityPersister } $tableAlias = $this->_getSQLTableAlias($assoc['targetEntity'], $assocAlias); $this->_selectJoinSql .= $this->_getSQLTableAlias($assoc['sourceEntity']) . '.' . $sourceCol . ' = ' - . $tableAlias . '.' . $targetCol . ' '; + . $tableAlias . '.' . $targetCol; $first = false; } @@ -1536,7 +1537,7 @@ class BasicEntityPersister $alias = $this->_getSQLTableAlias($this->_class->name); - $sql = 'SELECT 1' + $sql = 'SELECT 1 ' . $this->getLockTablesSql($alias) . ' WHERE ' . $this->_getSelectConditionSQL($criteria); diff --git a/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php b/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php index fb60d5e32..d7e79e285 100644 --- a/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php +++ b/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php @@ -401,10 +401,10 @@ class JoinedSubclassPersister extends AbstractEntityInheritancePersister * * @return string */ - public function getLockTablesSql() + public function getLockTablesSql($alias = null) { $idColumns = $this->_class->getIdentifierColumnNames(); - $baseTableAlias = $this->_getSQLTableAlias($this->_class->name); + $baseTableAlias = null !== $alias ? $alias : $this->_getSQLTableAlias($this->_class->name); // INNER JOIN parent tables $joinSql = ''; diff --git a/lib/Doctrine/ORM/Query/Filter/SQLFilter.php b/lib/Doctrine/ORM/Query/Filter/SQLFilter.php index 1ea27df6e..81cae335a 100644 --- a/lib/Doctrine/ORM/Query/Filter/SQLFilter.php +++ b/lib/Doctrine/ORM/Query/Filter/SQLFilter.php @@ -42,7 +42,7 @@ abstract class SQLFilter $this->em = $em; } - final function setParameter($name, $value, $type) + final public function setParameter($name, $value, $type) { // @todo: check for a valid type? $this->parameters[$name] = array('value' => $value, 'type' => $type); @@ -56,7 +56,7 @@ abstract class SQLFilter return $this; } - final function getParameter($name) + final public function getParameter($name) { if(!isset($this->parameters[$name])) { throw new \InvalidArgumentException("Parameter '" . $name . "' does not exist."); @@ -65,13 +65,13 @@ abstract class SQLFilter return $this->em->getConnection()->quote($this->parameters[$name]['value'], $this->parameters[$name]['type']); } - final function __toString() + final public function __toString() { return serialize($this->parameters); } /** - * @return string The contstraint if there is one, empty string otherwise + * @return string The constraint if there is one, empty string otherwise */ - abstract function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAlias); + abstract public function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAlias); } diff --git a/lib/Doctrine/ORM/Query/FilterCollection.php b/lib/Doctrine/ORM/Query/FilterCollection.php index c0a408d60..72b5a757c 100644 --- a/lib/Doctrine/ORM/Query/FilterCollection.php +++ b/lib/Doctrine/ORM/Query/FilterCollection.php @@ -29,6 +29,17 @@ use Doctrine\ORM\Configuration, */ class FilterCollection { + /* 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; + private $config; private $em; private $filters; @@ -45,36 +56,41 @@ class FilterCollection */ 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 + * @var integer $state The current state of this filter */ private $filtersState = self::FILTERS_STATE_CLEAN; - final public function __construct(EntityManager $em) + /** + * Constructor. + * + * @param EntityManager $em + */ + public function __construct(EntityManager $em) { $this->em = $em; $this->config = $em->getConfiguration(); } - /** @return SQLFilter[] */ + /** + * Get all the enabled filters. + * + * @return array The enabled filters. + */ public function getEnabledFilters() { return $this->enabledFilters; } - /** Throws exception if filter does not exist. No-op if the filter is alrady enabled. - * @return SQLFilter */ + /** + * Enables a filter from the collection. + * + * @param mixed $name Name of the filter. + * + * @throws \InvalidArgumentException If the filter does not exist. + * + * @return SQLFilter The enabled filter. + */ public function enable($name) { if(null === $filterClass = $this->config->getFilterClassName($name)) { @@ -94,7 +110,15 @@ class FilterCollection return $this->enabledFilters[$name]; } - /** Disable the filter, looses the state */ + /** + * Disables a filter. + * + * @param mixed $name Name of the filter. + * + * @return SQLFilter The disabled filter. + * + * @throws \InvalidArgumentException If the filter does not exist. + */ public function disable($name) { // Get the filter to return it @@ -108,7 +132,15 @@ class FilterCollection return $filter; } - /** throws exception if not in enabled filters */ + /** + * Get an enabled filter from the collection. + * + * @param mixed $name Name of the filter. + * + * @return SQLFilter The filter. + * + * @throws \InvalidArgumentException If the filter is not enabled. + */ public function getFilter($name) { if(!isset($this->enabledFilters[$name])) { @@ -146,6 +178,9 @@ class FilterCollection return $filterHash; } + /** + * Set the filter state to dirty. + */ public function setFiltersStateDirty() { $this->filtersState = self::FILTERS_STATE_DIRTY; diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC633Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC633Test.php index fa7e0af2f..0dd24d50c 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC633Test.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC633Test.php @@ -99,4 +99,4 @@ class DDC633Patient * @OneToOne(targetEntity="DDC633Appointment", mappedBy="patient") */ public $appointment; -} \ No newline at end of file +} From f6d5f0481ee1c3d9723e172ce1de8b25dba161ce Mon Sep 17 00:00:00 2001 From: Alexander Date: Wed, 30 Nov 2011 23:01:10 +0100 Subject: [PATCH 18/49] [DDC-551] Initial support for filters in the JoinedSubclassPersister Still some things to do. --- .../ORM/Persisters/BasicEntityPersister.php | 2 +- .../Persisters/JoinedSubclassPersister.php | 31 ++++++ lib/Doctrine/ORM/Query/Filter/SQLFilter.php | 2 +- .../Tests/ORM/Functional/SQLFilterTest.php | 97 ++++++++++++++++++- 4 files changed, 125 insertions(+), 7 deletions(-) diff --git a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php index 1f4550d8c..08252c420 100644 --- a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php +++ b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php @@ -1584,7 +1584,7 @@ class BasicEntityPersister ); } - private function generateFilterConditionSQL(ClassMetadata $targetEntity, $targetTableAlias) + private function generateFilterConditionSQL(ClassMetadata $targetEntity, $targetTableAlias, $targetTable = '') { $filterSql = ''; diff --git a/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php b/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php index d7e79e285..1a5bd8e51 100644 --- a/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php +++ b/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php @@ -328,8 +328,17 @@ class JoinedSubclassPersister extends AbstractEntityInheritancePersister $joinSql .= $baseTableAlias . '.' . $idColumn . ' = ' . $tableAlias . '.' . $idColumn; } + + // Filters for each parent table + $filterSql = $this->generateFilterConditionSQL($parentClass, $tableAlias, $parentClass->table['name']); + if('' !== $filterSql) { + if (!$first) $joinSql .= ' AND '; + $joinSql .= $filterSql; + } } + //@todo: filters for the sub tables of childs + // OUTER JOIN sub tables foreach ($this->_class->subClasses as $subClassName) { $subClass = $this->_em->getClassMetadata($subClassName); @@ -389,6 +398,13 @@ class JoinedSubclassPersister extends AbstractEntityInheritancePersister $lockSql = ' ' . $this->_platform->getWriteLockSql(); } + // Filters for the base table + $filterSql = $this->generateFilterConditionSQL($this->_class, $baseTableAlias, $this->_class->table['name']); + if('' !== $filterSql) { + if($conditionSql) $conditionSql .= ' AND '; + $conditionSql .= $filterSql; + } + return $this->_platform->modifyLimitQuery('SELECT ' . $this->_selectColumnListSql . ' FROM ' . $this->_class->getQuotedTableName($this->_platform) . ' ' . $baseTableAlias . $joinSql @@ -473,4 +489,19 @@ class JoinedSubclassPersister extends AbstractEntityInheritancePersister $value = $this->fetchVersionValue($this->_getVersionedClassMetadata(), $id); $this->_class->setFieldValue($entity, $this->_class->versionField, $value); } + + private function generateFilterConditionSQL(ClassMetadata $targetEntity, $targetTableAlias, $targetTable = '') + { + $filterSql = ''; + + $first = true; + foreach($this->_em->getFilters()->getEnabledFilters() as $filter) { + if("" !== $filterExpr = $filter->addFilterConstraint($targetEntity, $targetTableAlias, $targetTable)) { + if ( ! $first) $filterSql .= ' AND '; else $first = false; + $filterSql .= '(' . $filterExpr . ')'; + } + } + + return $filterSql; + } } diff --git a/lib/Doctrine/ORM/Query/Filter/SQLFilter.php b/lib/Doctrine/ORM/Query/Filter/SQLFilter.php index 81cae335a..6ffe2b6de 100644 --- a/lib/Doctrine/ORM/Query/Filter/SQLFilter.php +++ b/lib/Doctrine/ORM/Query/Filter/SQLFilter.php @@ -73,5 +73,5 @@ abstract class SQLFilter /** * @return string The constraint if there is one, empty string otherwise */ - abstract public function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAlias); + abstract public function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAlias, $targetTable = ''); } diff --git a/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php b/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php index aed2dbaf1..92016bc69 100644 --- a/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php @@ -15,6 +15,10 @@ use Doctrine\Tests\Models\CMS\CmsGroup; use Doctrine\Tests\Models\CMS\CmsArticle; use Doctrine\Tests\Models\CMS\CmsComment; +use Doctrine\Tests\Models\Company\CompanyPerson; +use Doctrine\Tests\Models\Company\CompanyManager; +use Doctrine\Tests\Models\Company\CompanyEmployee; + require_once __DIR__ . '/../../TestInit.php'; /** @@ -30,6 +34,7 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase public function setUp() { $this->useModelSet('cms'); + $this->useModelSet('company'); parent::setUp(); } @@ -444,6 +449,44 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase $this->assertEquals(1, count($user->groups->slice(0,10))); } + public function testJoinSubclassPersister_FilterOnBaseTable() + { + $this->loadCompanyFixtureData(); + $this->assertEquals(2, count($this->_em->getRepository('Doctrine\Tests\Models\Company\CompanyManager')->findAll())); + + // Enable the filter + $conf = $this->_em->getConfiguration(); + $conf->addFilter("manager_title", "\Doctrine\Tests\ORM\Functional\CompanyManagerTitleFilter"); + $this->_em->getFilters() + ->enable("manager_title") + ->setParameter("title", "devlead", \Doctrine\DBAL\Types\Type::getType(\Doctrine\DBAL\Types\Type::STRING)->getBindingType()); + + $this->_em->getConnection()->getConfiguration()->setSQLLogger(new \Doctrine\DBAL\Logging\EchoSQLLogger); + + $managers = $this->_em->getRepository('Doctrine\Tests\Models\Company\CompanyManager')->findAll(); + $this->assertEquals(1, count($managers)); + $this->assertEquals("Guilherme", $managers[0]->getName()); + } + + public function testJoinSubclassPersister_FilterOnParentTable() + { + $this->loadCompanyFixtureData(); + $this->assertEquals(2, count($this->_em->getRepository('Doctrine\Tests\Models\Company\CompanyManager')->findAll())); + + // Enable the filter + $conf = $this->_em->getConfiguration(); + $conf->addFilter("employee_department", "\Doctrine\Tests\ORM\Functional\CompanyEmployeeDepartmentFilter"); + $this->_em->getFilters() + ->enable("employee_department") + ->setParameter("department", "parsers", \Doctrine\DBAL\Types\Type::getType(\Doctrine\DBAL\Types\Type::STRING)->getBindingType()); + + $this->_em->getConnection()->getConfiguration()->setSQLLogger(new \Doctrine\DBAL\Logging\EchoSQLLogger); + + $managers = $this->_em->getRepository('Doctrine\Tests\Models\Company\CompanyManager')->findAll(); + $this->assertEquals(1, count($managers)); + $this->assertEquals("Guilherme", $managers[0]->getName()); + } + private function loadFixtureData() { $user = new CmsUser; @@ -507,11 +550,31 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase $this->groupId = $group->id; $this->groupId2 = $group2->id; } + + private function loadCompanyFixtureData() + { + $manager = new CompanyManager; + $manager->setName('Roman'); + $manager->setTitle('testlead'); + $manager->setSalary(42); + $manager->setDepartment('persisters'); + + $manager2 = new CompanyManager; + $manager2->setName('Guilherme'); + $manager2->setTitle('devlead'); + $manager2->setSalary(42); + $manager2->setDepartment('parsers'); + + $this->_em->persist($manager); + $this->_em->persist($manager2); + $this->_em->flush(); + $this->_em->clear(); + } } class MySoftDeleteFilter extends SQLFilter { - public function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAlias) + public function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAlias, $targetTable = '') { if ($targetEntity->name != "MyEntity\SoftDeleteNewsItem") { return ""; @@ -523,7 +586,7 @@ class MySoftDeleteFilter extends SQLFilter class MyLocaleFilter extends SQLFilter { - public function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAlias) + public function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAlias, $targetTable = '') { if (!in_array("LocaleAware", $targetEntity->reflClass->getInterfaceNames())) { return ""; @@ -535,7 +598,7 @@ class MyLocaleFilter extends SQLFilter class CMSCountryFilter extends SQLFilter { - public function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAlias) + public function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAlias, $targetTable = '') { if ($targetEntity->name != "Doctrine\Tests\Models\CMS\CmsAddress") { return ""; @@ -547,7 +610,7 @@ class CMSCountryFilter extends SQLFilter class CMSGroupPrefixFilter extends SQLFilter { - public function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAlias) + public function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAlias, $targetTable = '') { if ($targetEntity->name != "Doctrine\Tests\Models\CMS\CmsGroup") { return ""; @@ -558,7 +621,7 @@ class CMSGroupPrefixFilter extends SQLFilter } class CMSArticleTopicFilter extends SQLFilter { - public function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAlias) + public function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAlias, $targetTable = '') { if ($targetEntity->name != "Doctrine\Tests\Models\CMS\CmsArticle") { return ""; @@ -567,3 +630,27 @@ class CMSArticleTopicFilter extends SQLFilter return $targetTableAlias.'.topic = ' . $this->getParameter('topic'); // getParam uses connection to quote the value. } } + +class CompanyManagerTitleFilter extends SQLFilter +{ + public function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAlias, $targetTable = '') + { + if ($targetEntity->name != "Doctrine\Tests\Models\Company\CompanyManager") { + return ""; + } + + return $targetTableAlias.'.title = ' . $this->getParameter('title'); // getParam uses connection to quote the value. + } +} + +class CompanyEmployeeDepartmentFilter extends SQLFilter +{ + public function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAlias, $targetTable = '') + { + if ($targetEntity->name != "Doctrine\Tests\Models\Company\CompanyEmployee") { + return ""; + } + + return $targetTableAlias.'.department = ' . $this->getParameter('department'); // getParam uses connection to quote the value. + } +} From bf1cc29a2a5b4e8374973a41b293e3dbb7e91ca8 Mon Sep 17 00:00:00 2001 From: Alexander Date: Thu, 1 Dec 2011 09:46:02 +0100 Subject: [PATCH 19/49] [DDC-551] Fixed some comments --- lib/Doctrine/ORM/Query/FilterCollection.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/Doctrine/ORM/Query/FilterCollection.php b/lib/Doctrine/ORM/Query/FilterCollection.php index 72b5a757c..c28f54863 100644 --- a/lib/Doctrine/ORM/Query/FilterCollection.php +++ b/lib/Doctrine/ORM/Query/FilterCollection.php @@ -85,7 +85,7 @@ class FilterCollection /** * Enables a filter from the collection. * - * @param mixed $name Name of the filter. + * @param string $name Name of the filter. * * @throws \InvalidArgumentException If the filter does not exist. * @@ -113,7 +113,7 @@ class FilterCollection /** * Disables a filter. * - * @param mixed $name Name of the filter. + * @param string $name Name of the filter. * * @return SQLFilter The disabled filter. * @@ -135,7 +135,7 @@ class FilterCollection /** * Get an enabled filter from the collection. * - * @param mixed $name Name of the filter. + * @param string $name Name of the filter. * * @return SQLFilter The filter. * From e98c775f0d4143d6e8b2877f5311ea09873b02e7 Mon Sep 17 00:00:00 2001 From: Alexander Date: Mon, 5 Dec 2011 16:14:04 +0100 Subject: [PATCH 20/49] Revert "[DDC-551] Initial support for filters in the JoinedSubclassPersister" This reverts commit f6d5f0481ee1c3d9723e172ce1de8b25dba161ce. --- .../ORM/Persisters/BasicEntityPersister.php | 2 +- .../Persisters/JoinedSubclassPersister.php | 20 +----- lib/Doctrine/ORM/Query/Filter/SQLFilter.php | 2 +- .../Tests/ORM/Functional/SQLFilterTest.php | 72 ++----------------- 4 files changed, 9 insertions(+), 87 deletions(-) diff --git a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php index 08252c420..1f4550d8c 100644 --- a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php +++ b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php @@ -1584,7 +1584,7 @@ class BasicEntityPersister ); } - private function generateFilterConditionSQL(ClassMetadata $targetEntity, $targetTableAlias, $targetTable = '') + private function generateFilterConditionSQL(ClassMetadata $targetEntity, $targetTableAlias) { $filterSql = ''; diff --git a/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php b/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php index 1a5bd8e51..190ae7be6 100644 --- a/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php +++ b/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php @@ -328,17 +328,8 @@ class JoinedSubclassPersister extends AbstractEntityInheritancePersister $joinSql .= $baseTableAlias . '.' . $idColumn . ' = ' . $tableAlias . '.' . $idColumn; } - - // Filters for each parent table - $filterSql = $this->generateFilterConditionSQL($parentClass, $tableAlias, $parentClass->table['name']); - if('' !== $filterSql) { - if (!$first) $joinSql .= ' AND '; - $joinSql .= $filterSql; - } } - //@todo: filters for the sub tables of childs - // OUTER JOIN sub tables foreach ($this->_class->subClasses as $subClassName) { $subClass = $this->_em->getClassMetadata($subClassName); @@ -398,13 +389,6 @@ class JoinedSubclassPersister extends AbstractEntityInheritancePersister $lockSql = ' ' . $this->_platform->getWriteLockSql(); } - // Filters for the base table - $filterSql = $this->generateFilterConditionSQL($this->_class, $baseTableAlias, $this->_class->table['name']); - if('' !== $filterSql) { - if($conditionSql) $conditionSql .= ' AND '; - $conditionSql .= $filterSql; - } - return $this->_platform->modifyLimitQuery('SELECT ' . $this->_selectColumnListSql . ' FROM ' . $this->_class->getQuotedTableName($this->_platform) . ' ' . $baseTableAlias . $joinSql @@ -490,13 +474,13 @@ class JoinedSubclassPersister extends AbstractEntityInheritancePersister $this->_class->setFieldValue($entity, $this->_class->versionField, $value); } - private function generateFilterConditionSQL(ClassMetadata $targetEntity, $targetTableAlias, $targetTable = '') + private function generateFilterConditionSQL(ClassMetadata $targetEntity, $targetTableAlias) { $filterSql = ''; $first = true; foreach($this->_em->getFilters()->getEnabledFilters() as $filter) { - if("" !== $filterExpr = $filter->addFilterConstraint($targetEntity, $targetTableAlias, $targetTable)) { + if("" !== $filterExpr = $filter->addFilterConstraint($targetEntity, $targetTableAlias)) { if ( ! $first) $filterSql .= ' AND '; else $first = false; $filterSql .= '(' . $filterExpr . ')'; } diff --git a/lib/Doctrine/ORM/Query/Filter/SQLFilter.php b/lib/Doctrine/ORM/Query/Filter/SQLFilter.php index 6ffe2b6de..81cae335a 100644 --- a/lib/Doctrine/ORM/Query/Filter/SQLFilter.php +++ b/lib/Doctrine/ORM/Query/Filter/SQLFilter.php @@ -73,5 +73,5 @@ abstract class SQLFilter /** * @return string The constraint if there is one, empty string otherwise */ - abstract public function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAlias, $targetTable = ''); + abstract public function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAlias); } diff --git a/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php b/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php index 92016bc69..107b4ca84 100644 --- a/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php @@ -449,44 +449,6 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase $this->assertEquals(1, count($user->groups->slice(0,10))); } - public function testJoinSubclassPersister_FilterOnBaseTable() - { - $this->loadCompanyFixtureData(); - $this->assertEquals(2, count($this->_em->getRepository('Doctrine\Tests\Models\Company\CompanyManager')->findAll())); - - // Enable the filter - $conf = $this->_em->getConfiguration(); - $conf->addFilter("manager_title", "\Doctrine\Tests\ORM\Functional\CompanyManagerTitleFilter"); - $this->_em->getFilters() - ->enable("manager_title") - ->setParameter("title", "devlead", \Doctrine\DBAL\Types\Type::getType(\Doctrine\DBAL\Types\Type::STRING)->getBindingType()); - - $this->_em->getConnection()->getConfiguration()->setSQLLogger(new \Doctrine\DBAL\Logging\EchoSQLLogger); - - $managers = $this->_em->getRepository('Doctrine\Tests\Models\Company\CompanyManager')->findAll(); - $this->assertEquals(1, count($managers)); - $this->assertEquals("Guilherme", $managers[0]->getName()); - } - - public function testJoinSubclassPersister_FilterOnParentTable() - { - $this->loadCompanyFixtureData(); - $this->assertEquals(2, count($this->_em->getRepository('Doctrine\Tests\Models\Company\CompanyManager')->findAll())); - - // Enable the filter - $conf = $this->_em->getConfiguration(); - $conf->addFilter("employee_department", "\Doctrine\Tests\ORM\Functional\CompanyEmployeeDepartmentFilter"); - $this->_em->getFilters() - ->enable("employee_department") - ->setParameter("department", "parsers", \Doctrine\DBAL\Types\Type::getType(\Doctrine\DBAL\Types\Type::STRING)->getBindingType()); - - $this->_em->getConnection()->getConfiguration()->setSQLLogger(new \Doctrine\DBAL\Logging\EchoSQLLogger); - - $managers = $this->_em->getRepository('Doctrine\Tests\Models\Company\CompanyManager')->findAll(); - $this->assertEquals(1, count($managers)); - $this->assertEquals("Guilherme", $managers[0]->getName()); - } - private function loadFixtureData() { $user = new CmsUser; @@ -574,7 +536,7 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase class MySoftDeleteFilter extends SQLFilter { - public function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAlias, $targetTable = '') + public function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAlias) { if ($targetEntity->name != "MyEntity\SoftDeleteNewsItem") { return ""; @@ -586,7 +548,7 @@ class MySoftDeleteFilter extends SQLFilter class MyLocaleFilter extends SQLFilter { - public function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAlias, $targetTable = '') + public function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAlias) { if (!in_array("LocaleAware", $targetEntity->reflClass->getInterfaceNames())) { return ""; @@ -598,7 +560,7 @@ class MyLocaleFilter extends SQLFilter class CMSCountryFilter extends SQLFilter { - public function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAlias, $targetTable = '') + public function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAlias) { if ($targetEntity->name != "Doctrine\Tests\Models\CMS\CmsAddress") { return ""; @@ -610,7 +572,7 @@ class CMSCountryFilter extends SQLFilter class CMSGroupPrefixFilter extends SQLFilter { - public function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAlias, $targetTable = '') + public function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAlias) { if ($targetEntity->name != "Doctrine\Tests\Models\CMS\CmsGroup") { return ""; @@ -621,7 +583,7 @@ class CMSGroupPrefixFilter extends SQLFilter } class CMSArticleTopicFilter extends SQLFilter { - public function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAlias, $targetTable = '') + public function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAlias) { if ($targetEntity->name != "Doctrine\Tests\Models\CMS\CmsArticle") { return ""; @@ -630,27 +592,3 @@ class CMSArticleTopicFilter extends SQLFilter return $targetTableAlias.'.topic = ' . $this->getParameter('topic'); // getParam uses connection to quote the value. } } - -class CompanyManagerTitleFilter extends SQLFilter -{ - public function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAlias, $targetTable = '') - { - if ($targetEntity->name != "Doctrine\Tests\Models\Company\CompanyManager") { - return ""; - } - - return $targetTableAlias.'.title = ' . $this->getParameter('title'); // getParam uses connection to quote the value. - } -} - -class CompanyEmployeeDepartmentFilter extends SQLFilter -{ - public function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAlias, $targetTable = '') - { - if ($targetEntity->name != "Doctrine\Tests\Models\Company\CompanyEmployee") { - return ""; - } - - return $targetTableAlias.'.department = ' . $this->getParameter('department'); // getParam uses connection to quote the value. - } -} From 752b5023260527dcf458e0b66778518a22f90096 Mon Sep 17 00:00:00 2001 From: Alexander Date: Mon, 5 Dec 2011 18:26:56 +0100 Subject: [PATCH 21/49] [DDC-551] Add filters only on root entities in JoinedSubclassPersister --- .../Persisters/JoinedSubclassPersister.php | 17 +++++++ .../Tests/ORM/Functional/SQLFilterTest.php | 50 +++++++++++++++++++ 2 files changed, 67 insertions(+) diff --git a/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php b/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php index 190ae7be6..9e9477f0a 100644 --- a/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php +++ b/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php @@ -327,6 +327,14 @@ class JoinedSubclassPersister extends AbstractEntityInheritancePersister if ($first) $first = false; else $joinSql .= ' AND '; $joinSql .= $baseTableAlias . '.' . $idColumn . ' = ' . $tableAlias . '.' . $idColumn; + + if($parentClass->name === $this->_class->rootEntityName) { + // Add filters on the root class + $filterSql = $this->generateFilterConditionSQL($parentClass, $tableAlias); + if('' !== $filterSql) { + $joinSql .= ' AND ' . $filterSql; + } + } } } @@ -374,6 +382,15 @@ class JoinedSubclassPersister extends AbstractEntityInheritancePersister $conditionSql = $this->_getSelectConditionSQL($criteria, $assoc); + // If the current class in the root entity, add the filters + if($this->_class->name === $this->_class->rootEntityName) { + $filterSql = $this->generateFilterConditionSQL($this->_class, $baseTableAlias); + if('' !== $filterSql) { + if($conditionSql) $conditionSql .= ' AND '; + $conditionSql .= $filterSql; + } + } + $orderBy = ($assoc !== null && isset($assoc['orderBy'])) ? $assoc['orderBy'] : $orderBy; $orderBySql = $orderBy ? $this->_getOrderBySQL($orderBy, $baseTableAlias) : ''; diff --git a/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php b/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php index 107b4ca84..5d9803277 100644 --- a/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php @@ -513,6 +513,40 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase $this->groupId2 = $group2->id; } + public function testJoinSubclassPersister_FilterOnlyOnRootTableWhenFetchingSubEntity() + { + $this->loadCompanyFixtureData(); + $this->assertEquals(2, count($this->_em->getRepository('Doctrine\Tests\Models\Company\CompanyManager')->findAll())); + + // Enable the filter + $conf = $this->_em->getConfiguration(); + $conf->addFilter("person_name", "\Doctrine\Tests\ORM\Functional\CompanyPersonNameFilter"); + $this->_em->getFilters() + ->enable("person_name") + ->setParameter("name", "Guilh%", \Doctrine\DBAL\Types\Type::getType(\Doctrine\DBAL\Types\Type::STRING)->getBindingType()); + + $managers = $this->_em->getRepository('Doctrine\Tests\Models\Company\CompanyManager')->findAll(); + $this->assertEquals(1, count($managers)); + $this->assertEquals("Guilherme", $managers[0]->getName()); + } + + public function testJoinSubclassPersister_FilterOnlyOnRootTableWhenFetchingRootEntity() + { + $this->loadCompanyFixtureData(); + $this->assertEquals(3, count($this->_em->getRepository('Doctrine\Tests\Models\Company\CompanyPerson')->findAll())); + + // Enable the filter + $conf = $this->_em->getConfiguration(); + $conf->addFilter("person_name", "\Doctrine\Tests\ORM\Functional\CompanyPersonNameFilter"); + $this->_em->getFilters() + ->enable("person_name") + ->setParameter("name", "Guilh%", \Doctrine\DBAL\Types\Type::getType(\Doctrine\DBAL\Types\Type::STRING)->getBindingType()); + + $managers = $this->_em->getRepository('Doctrine\Tests\Models\Company\CompanyPerson')->findAll(); + $this->assertEquals(1, count($managers)); + $this->assertEquals("Guilherme", $managers[0]->getName()); + } + private function loadCompanyFixtureData() { $manager = new CompanyManager; @@ -527,8 +561,12 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase $manager2->setSalary(42); $manager2->setDepartment('parsers'); + $person = new CompanyPerson; + $person->setName('Benjamin'); + $this->_em->persist($manager); $this->_em->persist($manager2); + $this->_em->persist($person); $this->_em->flush(); $this->_em->clear(); } @@ -592,3 +630,15 @@ class CMSArticleTopicFilter extends SQLFilter return $targetTableAlias.'.topic = ' . $this->getParameter('topic'); // getParam uses connection to quote the value. } } + +class CompanyPersonNameFilter extends SQLFilter +{ + public function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAlias, $targetTable = '') + { + if ($targetEntity->name != "Doctrine\Tests\Models\Company\CompanyPerson") { + return ""; + } + + return $targetTableAlias.'.name LIKE ' . $this->getParameter('name'); + } +} From 4c842974b47812cd7bff4e5b28effc46867ee570 Mon Sep 17 00:00:00 2001 From: Alexander Date: Mon, 5 Dec 2011 18:56:44 +0100 Subject: [PATCH 22/49] [DDC-551] Add filters only on root entities in SingleTablePersister --- .../ORM/Persisters/BasicEntityPersister.php | 10 ++- .../Persisters/JoinedSubclassPersister.php | 14 ---- .../ORM/Persisters/SingleTablePersister.php | 12 +++ .../Tests/ORM/Functional/SQLFilterTest.php | 73 +++++++++++++++++-- 4 files changed, 88 insertions(+), 21 deletions(-) diff --git a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php index 1f4550d8c..af7cf302c 100644 --- a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php +++ b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php @@ -1584,7 +1584,15 @@ class BasicEntityPersister ); } - private function generateFilterConditionSQL(ClassMetadata $targetEntity, $targetTableAlias) + /** + * Generates the filter SQL for a given entity and table alias. + * + * @param ClassMetadata $targetEntity Metadata of the target entity. + * @param string $targetTableAlias The table alias of the joined/selected table. + * + * @return string The SQL query part to add to a query. + */ + protected function generateFilterConditionSQL(ClassMetadata $targetEntity, $targetTableAlias) { $filterSql = ''; diff --git a/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php b/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php index 9e9477f0a..73e65797c 100644 --- a/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php +++ b/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php @@ -491,18 +491,4 @@ class JoinedSubclassPersister extends AbstractEntityInheritancePersister $this->_class->setFieldValue($entity, $this->_class->versionField, $value); } - private function generateFilterConditionSQL(ClassMetadata $targetEntity, $targetTableAlias) - { - $filterSql = ''; - - $first = true; - foreach($this->_em->getFilters()->getEnabledFilters() as $filter) { - if("" !== $filterExpr = $filter->addFilterConstraint($targetEntity, $targetTableAlias)) { - if ( ! $first) $filterSql .= ' AND '; else $first = false; - $filterSql .= '(' . $filterExpr . ')'; - } - } - - return $filterSql; - } } diff --git a/lib/Doctrine/ORM/Persisters/SingleTablePersister.php b/lib/Doctrine/ORM/Persisters/SingleTablePersister.php index 0f1b9e3de..d11486e93 100644 --- a/lib/Doctrine/ORM/Persisters/SingleTablePersister.php +++ b/lib/Doctrine/ORM/Persisters/SingleTablePersister.php @@ -131,4 +131,16 @@ class SingleTablePersister extends AbstractEntityInheritancePersister return $conditionSql; } + + /** {@inheritdoc} */ + protected function generateFilterConditionSQL(ClassMetadata $targetEntity, $targetTableAlias) + { + // Ensure that the filters are applied to the root entity of the inheritance tree + $realTargetEntity = $targetEntity; + if($targetEntity->name !== $targetEntity->rootEntityName) { + $realTargetEntity = $this->_em->getClassMetadata($targetEntity->rootEntityName); + } + + return parent::generateFilterConditionSQL($realTargetEntity, $targetTableAlias); + } } diff --git a/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php b/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php index 5d9803277..2353814b0 100644 --- a/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php @@ -19,6 +19,9 @@ use Doctrine\Tests\Models\Company\CompanyPerson; use Doctrine\Tests\Models\Company\CompanyManager; use Doctrine\Tests\Models\Company\CompanyEmployee; +use Doctrine\Tests\Models\Company\CompanyFlexContract; +use Doctrine\Tests\Models\Company\CompanyFlexUltraContract; + require_once __DIR__ . '/../../TestInit.php'; /** @@ -515,7 +518,7 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase public function testJoinSubclassPersister_FilterOnlyOnRootTableWhenFetchingSubEntity() { - $this->loadCompanyFixtureData(); + $this->loadCompanyJoinedSubclassFixtureData(); $this->assertEquals(2, count($this->_em->getRepository('Doctrine\Tests\Models\Company\CompanyManager')->findAll())); // Enable the filter @@ -532,7 +535,7 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase public function testJoinSubclassPersister_FilterOnlyOnRootTableWhenFetchingRootEntity() { - $this->loadCompanyFixtureData(); + $this->loadCompanyJoinedSubclassFixtureData(); $this->assertEquals(3, count($this->_em->getRepository('Doctrine\Tests\Models\Company\CompanyPerson')->findAll())); // Enable the filter @@ -542,12 +545,12 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase ->enable("person_name") ->setParameter("name", "Guilh%", \Doctrine\DBAL\Types\Type::getType(\Doctrine\DBAL\Types\Type::STRING)->getBindingType()); - $managers = $this->_em->getRepository('Doctrine\Tests\Models\Company\CompanyPerson')->findAll(); - $this->assertEquals(1, count($managers)); - $this->assertEquals("Guilherme", $managers[0]->getName()); + $persons = $this->_em->getRepository('Doctrine\Tests\Models\Company\CompanyPerson')->findAll(); + $this->assertEquals(1, count($persons)); + $this->assertEquals("Guilherme", $persons[0]->getName()); } - private function loadCompanyFixtureData() + private function loadCompanyJoinedSubclassFixtureData() { $manager = new CompanyManager; $manager->setName('Roman'); @@ -570,6 +573,52 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase $this->_em->flush(); $this->_em->clear(); } + + public function testSingleTableInheritance_FilterOnlyOnRootTableWhenFetchingSubEntity() + { + $this->loadCompanySingleTableInheritanceFixtureData(); + $this->assertEquals(2, count($this->_em->getRepository('Doctrine\Tests\Models\Company\CompanyFlexUltraContract')->findAll())); + + // Enable the filter + $conf = $this->_em->getConfiguration(); + $conf->addFilter("completed_contract", "\Doctrine\Tests\ORM\Functional\CompletedContractFilter"); + $this->_em->getFilters() + ->enable("completed_contract"); + + $this->assertEquals(1, count($this->_em->getRepository('Doctrine\Tests\Models\Company\CompanyFlexUltraContract')->findAll())); + } + + public function testSingleTableInheritance_FilterOnlyOnRootTableWhenFetchingRootEntity() + { + $this->loadCompanySingleTableInheritanceFixtureData(); + $this->assertEquals(4, count($this->_em->getRepository('Doctrine\Tests\Models\Company\CompanyFlexContract')->findAll())); + + // Enable the filter + $conf = $this->_em->getConfiguration(); + $conf->addFilter("completed_contract", "\Doctrine\Tests\ORM\Functional\CompletedContractFilter"); + $this->_em->getFilters() + ->enable("completed_contract"); + + $this->assertEquals(2, count($this->_em->getRepository('Doctrine\Tests\Models\Company\CompanyFlexContract')->findAll())); + } + + private function loadCompanySingleTableInheritanceFixtureData() + { + $contract1 = new CompanyFlexUltraContract; + $contract2 = new CompanyFlexUltraContract; + $contract2->markCompleted(); + + $contract3 = new CompanyFlexContract; + $contract4 = new CompanyFlexContract; + $contract4->markCompleted(); + + $this->_em->persist($contract1); + $this->_em->persist($contract2); + $this->_em->persist($contract3); + $this->_em->persist($contract4); + $this->_em->flush(); + $this->_em->clear(); + } } class MySoftDeleteFilter extends SQLFilter @@ -642,3 +691,15 @@ class CompanyPersonNameFilter extends SQLFilter return $targetTableAlias.'.name LIKE ' . $this->getParameter('name'); } } + +class CompletedContractFilter extends SQLFilter +{ + public function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAlias, $targetTable = '') + { + if ($targetEntity->name != "Doctrine\Tests\Models\Company\CompanyContract") { + return ""; + } + + return $targetTableAlias.'.completed = 1'; + } +} From 3b7d16c60f432fa94131171852ef2154e079cac6 Mon Sep 17 00:00:00 2001 From: Alexander Date: Mon, 5 Dec 2011 21:14:31 +0100 Subject: [PATCH 23/49] [DDC-551] General cleanup of the code. --- lib/Doctrine/ORM/Configuration.php | 14 ++++++ lib/Doctrine/ORM/EntityManager.php | 15 ++++++- .../ORM/Persisters/BasicEntityPersister.php | 10 ++--- .../Persisters/JoinedSubclassPersister.php | 6 +-- .../ORM/Persisters/ManyToManyPersister.php | 41 ++++++++++++----- lib/Doctrine/ORM/Query/Filter/SQLFilter.php | 45 ++++++++++++++++++- lib/Doctrine/ORM/Query/FilterCollection.php | 12 ++++- lib/Doctrine/ORM/Query/SqlWalker.php | 10 ++++- .../Tests/ORM/Functional/SQLFilterTest.php | 1 + 9 files changed, 130 insertions(+), 24 deletions(-) diff --git a/lib/Doctrine/ORM/Configuration.php b/lib/Doctrine/ORM/Configuration.php index 50a424256..5f5a58b30 100644 --- a/lib/Doctrine/ORM/Configuration.php +++ b/lib/Doctrine/ORM/Configuration.php @@ -496,11 +496,25 @@ class Configuration extends \Doctrine\DBAL\Configuration return $this->_attributes['classMetadataFactoryName']; } + /** + * Add a filter to the list of possible filters. + * + * @param string $name The name of the filter. + * @param string $className The class name of the filter. + */ public function addFilter($name, $className) { $this->_attributes['filters'][strtolower($name)] = $className; } + /** + * Gets the class name for a given filter name. + * + * @param string $name The name of the filter. + * + * @return string The class name of the filter, or null of it is not + * defined. + */ public function getFilterClassName($name) { $name = strtolower($name); diff --git a/lib/Doctrine/ORM/EntityManager.php b/lib/Doctrine/ORM/EntityManager.php index 2916caf3a..80fd6685d 100644 --- a/lib/Doctrine/ORM/EntityManager.php +++ b/lib/Doctrine/ORM/EntityManager.php @@ -762,6 +762,12 @@ class EntityManager implements ObjectManager return new EntityManager($conn, $config, $conn->getEventManager()); } + /** + * Gets the enabled filters. + * + * @access public + * @return FilterCollection The active filter collection. + */ public function getFilters() { if(null === $this->filterCollection) { @@ -772,7 +778,9 @@ class EntityManager implements ObjectManager } /** - * @return boolean True, if the filter collection is clean. + * Checks whether the state of the filter collection is clean. + * + * @return boolean True, iff the filter collection is clean. */ public function isFiltersStateClean() { @@ -780,6 +788,11 @@ class EntityManager implements ObjectManager || $this->filterCollection->isClean(); } + /** + * Checks whether the Entity Manager has filters. + * + * @return True, iff the EM has a filter collection. + */ public function hasFilters() { return null !== $this->filterCollection; diff --git a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php index af7cf302c..ec725693f 100644 --- a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php +++ b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php @@ -902,8 +902,7 @@ class BasicEntityPersister $alias = $this->_getSQLTableAlias($this->_class->name); - $filterSql = $this->generateFilterConditionSQL($this->_class, $alias); - if('' !== $filterSql) { + if('' !== $filterSql = $this->generateFilterConditionSQL($this->_class, $alias)) { if($conditionSql) $conditionSql .= ' AND '; $conditionSql .= $filterSql; } @@ -1541,8 +1540,7 @@ class BasicEntityPersister . $this->getLockTablesSql($alias) . ' WHERE ' . $this->_getSelectConditionSQL($criteria); - $filterSql = $this->generateFilterConditionSQL($this->_class, $alias); - if('' !== $filterSql) { + if('' !== $filterSql = $this->generateFilterConditionSQL($this->_class, $alias)) { $sql .= ' AND ' . $filterSql; } @@ -1598,8 +1596,8 @@ class BasicEntityPersister $first = true; foreach($this->_em->getFilters()->getEnabledFilters() as $filter) { - if("" !== $filterExpr = $filter->addFilterConstraint($targetEntity, $targetTableAlias)) { - if ( ! $first) $filterSql .= ' AND '; else $first = false; + if('' !== $filterExpr = $filter->addFilterConstraint($targetEntity, $targetTableAlias)) { + if (!$first) $filterSql .= ' AND '; else $first = false; $filterSql .= '(' . $filterExpr . ')'; } } diff --git a/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php b/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php index 73e65797c..daed5b01d 100644 --- a/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php +++ b/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php @@ -330,8 +330,7 @@ class JoinedSubclassPersister extends AbstractEntityInheritancePersister if($parentClass->name === $this->_class->rootEntityName) { // Add filters on the root class - $filterSql = $this->generateFilterConditionSQL($parentClass, $tableAlias); - if('' !== $filterSql) { + if('' !== $filterSql = $this->generateFilterConditionSQL($parentClass, $tableAlias)) { $joinSql .= ' AND ' . $filterSql; } } @@ -384,8 +383,7 @@ class JoinedSubclassPersister extends AbstractEntityInheritancePersister // If the current class in the root entity, add the filters if($this->_class->name === $this->_class->rootEntityName) { - $filterSql = $this->generateFilterConditionSQL($this->_class, $baseTableAlias); - if('' !== $filterSql) { + if('' !== $filterSql = $this->generateFilterConditionSQL($this->_class, $baseTableAlias)) { if($conditionSql) $conditionSql .= ' AND '; $conditionSql .= $filterSql; } diff --git a/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php b/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php index 8a1b6cc32..2a97e32c9 100644 --- a/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php +++ b/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php @@ -21,7 +21,8 @@ namespace Doctrine\ORM\Persisters; -use Doctrine\ORM\PersistentCollection, +use Doctrine\ORM\Mapping\ClassMetadata, + Doctrine\ORM\PersistentCollection, Doctrine\ORM\UnitOfWork; /** @@ -315,21 +316,20 @@ class ManyToManyPersister extends AbstractCollectionPersister return (bool) $this->_conn->fetchColumn($sql, $params); } + /** + * Generates the filter SQL for a given mapping. + * + * @param array $targetEntity Array containing mapping information. + * + * @return string The SQL query part to add to a query. + */ public function getFilterSql($mapping) { $targetClass = $this->_em->getClassMetadata($mapping['targetEntity']); - // Get the SQL for the filters - $filterSql = ''; - foreach($this->_em->getFilters()->getEnabledFilters() as $filter) { - if("" !== $filterExpr = $filter->addFilterConstraint($targetClass, 'te')) { - $filterSql .= ' AND (' . $filterExpr . ')'; - } - } - // A join is needed if there is filtering on the target entity $joinTargetEntitySQL = ''; - if('' !== $filterSql) { + if('' !== $filterSql = $this->generateFilterConditionSQL($targetClass, 'te')) { $joinTargetEntitySQL = ' JOIN ' . $targetClass->getQuotedTableName($this->_conn->getDatabasePlatform()) . ' te' . ' ON'; @@ -343,4 +343,25 @@ class ManyToManyPersister extends AbstractCollectionPersister return array($joinTargetEntitySQL, $filterSql); } + + /** + * Generates the filter SQL for a given entity and table alias. + * + * @param ClassMetadata $targetEntity Metadata of the target entity. + * @param string $targetTableAlias The table alias of the joined/selected table. + * + * @return string The SQL query part to add to a query. + */ + protected function generateFilterConditionSQL(ClassMetadata $targetEntity, $targetTableAlias) + { + $filterSql = ''; + + foreach($this->_em->getFilters()->getEnabledFilters() as $filter) { + if('' !== $filterExpr = $filter->addFilterConstraint($targetEntity, $targetTableAlias)) { + $filterSql .= 'AND (' . $filterExpr . ')'; + } + } + + return $filterSql; + } } diff --git a/lib/Doctrine/ORM/Query/Filter/SQLFilter.php b/lib/Doctrine/ORM/Query/Filter/SQLFilter.php index 81cae335a..9be8628a2 100644 --- a/lib/Doctrine/ORM/Query/Filter/SQLFilter.php +++ b/lib/Doctrine/ORM/Query/Filter/SQLFilter.php @@ -33,15 +33,39 @@ use Doctrine\ORM\EntityManager; */ abstract class SQLFilter { + /** + * The entity manager. + * @var EntityManager + */ private $em; + /** + * Parameters for the filter. + * @var array + */ private $parameters; + /** + * Constructs the SQLFilter object. + * + * @param EntityManager $em The EM + * @final + */ final public function __construct(EntityManager $em) { $this->em = $em; } + /** + * Sets a parameter that can be used by the filter. + * + * @param string $name Name of the parameter. + * @param string $value Value of the parameter. + * @param string $type Type of the parameter. + * + * @final + * @return SQLFilter The current SQL filter. + */ final public function setParameter($name, $value, $type) { // @todo: check for a valid type? @@ -56,6 +80,17 @@ abstract class SQLFilter return $this; } + /** + * Gets a parameter to use in a query. + * + * The function is responsible for the right output escaping to use the + * value in a query. + * + * @param string $name Name of the parameter. + * + * @final + * @return string The SQL escaped parameter to use in a query. + */ final public function getParameter($name) { if(!isset($this->parameters[$name])) { @@ -65,13 +100,21 @@ abstract class SQLFilter return $this->em->getConnection()->quote($this->parameters[$name]['value'], $this->parameters[$name]['type']); } + /** + * Returns as string representation of the SQLFilter parameters (the state). + * + * @final + * @return string String representation of the the SQLFilter. + */ final public function __toString() { return serialize($this->parameters); } /** - * @return string The constraint if there is one, empty string otherwise + * Gets the SQL query part to add to a query. + * + * @return string The constraint SQL if there is available, empty string otherwise */ abstract public function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAlias); } diff --git a/lib/Doctrine/ORM/Query/FilterCollection.php b/lib/Doctrine/ORM/Query/FilterCollection.php index c28f54863..8e055ecdb 100644 --- a/lib/Doctrine/ORM/Query/FilterCollection.php +++ b/lib/Doctrine/ORM/Query/FilterCollection.php @@ -40,9 +40,19 @@ class FilterCollection */ const FILTERS_STATE_DIRTY = 2; + /** + * The used Configuration. + * + * @var Doctrine\ORM\Configuration + */ private $config; + + /** + * The EntityManager that "owns" this FilterCollection instance. + * + * @var Doctrine\ORM\EntityManager + */ private $em; - private $filters; /** * Instances of enabled filters. diff --git a/lib/Doctrine/ORM/Query/SqlWalker.php b/lib/Doctrine/ORM/Query/SqlWalker.php index 0cbecb99d..e95cb400f 100644 --- a/lib/Doctrine/ORM/Query/SqlWalker.php +++ b/lib/Doctrine/ORM/Query/SqlWalker.php @@ -352,12 +352,20 @@ class SqlWalker implements TreeWalker return (count($sqlParts) > 1) ? '(' . $sql . ')' : $sql; } + /** + * Generates the filter SQL for a given entity and table alias. + * + * @param ClassMetadata $targetEntity Metadata of the target entity. + * @param string $targetTableAlias The table alias of the joined/selected table. + * + * @return string The SQL query part to add to a query. + */ private function generateFilterConditionSQL(ClassMetadata $targetEntity, $targetTableAlias) { $filterSql = ''; if($this->_em->hasFilters()) { - $first = true; + $first = true; foreach($this->_em->getFilters()->getEnabledFilters() as $filter) { if("" !== $filterExpr = $filter->addFilterConstraint($targetEntity, $targetTableAlias)) { if ( ! $first) $filterSql .= ' AND '; else $first = false; diff --git a/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php b/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php index 2353814b0..ff9332373 100644 --- a/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php @@ -668,6 +668,7 @@ class CMSGroupPrefixFilter extends SQLFilter return $targetTableAlias.'.name LIKE ' . $this->getParameter('prefix'); // getParam uses connection to quote the value. } } + class CMSArticleTopicFilter extends SQLFilter { public function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAlias) From e8d30068e25e769a2c1bcf63c651def577fa541c Mon Sep 17 00:00:00 2001 From: Alexander Date: Mon, 5 Dec 2011 22:05:42 +0100 Subject: [PATCH 24/49] [DDC-551] Various refactorings --- lib/Doctrine/ORM/Persisters/BasicEntityPersister.php | 9 ++++----- lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php | 1 + lib/Doctrine/ORM/Persisters/ManyToManyPersister.php | 9 ++++----- lib/Doctrine/ORM/Persisters/OneToManyPersister.php | 1 + lib/Doctrine/ORM/Persisters/SingleTablePersister.php | 1 + lib/Doctrine/ORM/Query/SqlWalker.php | 9 ++++----- 6 files changed, 15 insertions(+), 15 deletions(-) diff --git a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php index ec725693f..c58c4c3bf 100644 --- a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php +++ b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php @@ -72,6 +72,7 @@ use PDO, * @author Roman Borschel * @author Giorgio Sironi * @author Benjamin Eberlei + * @author Alexander * @since 2.0 */ class BasicEntityPersister @@ -1592,16 +1593,14 @@ class BasicEntityPersister */ protected function generateFilterConditionSQL(ClassMetadata $targetEntity, $targetTableAlias) { - $filterSql = ''; + $filterClauses = array(); - $first = true; foreach($this->_em->getFilters()->getEnabledFilters() as $filter) { if('' !== $filterExpr = $filter->addFilterConstraint($targetEntity, $targetTableAlias)) { - if (!$first) $filterSql .= ' AND '; else $first = false; - $filterSql .= '(' . $filterExpr . ')'; + $filterClauses[] = '(' . $filterExpr . ')'; } } - return $filterSql; + return implode(' AND ', $filterClauses); } } diff --git a/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php b/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php index daed5b01d..ad34f5c31 100644 --- a/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php +++ b/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php @@ -31,6 +31,7 @@ use Doctrine\ORM\ORMException, * * @author Roman Borschel * @author Benjamin Eberlei + * @author Alexander * @since 2.0 * @see http://martinfowler.com/eaaCatalog/classTableInheritance.html */ diff --git a/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php b/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php index 41d7b24a9..f765b77f1 100644 --- a/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php +++ b/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php @@ -30,6 +30,7 @@ use Doctrine\ORM\Mapping\ClassMetadata, * * @author Roman Borschel * @author Guilherme Blanco + * @author Alexander * @since 2.0 */ class ManyToManyPersister extends AbstractCollectionPersister @@ -379,16 +380,14 @@ class ManyToManyPersister extends AbstractCollectionPersister */ protected function generateFilterConditionSQL(ClassMetadata $targetEntity, $targetTableAlias) { - $filterSql = ''; + $filterClauses = array(); - $first = true; foreach($this->_em->getFilters()->getEnabledFilters() as $filter) { if('' !== $filterExpr = $filter->addFilterConstraint($targetEntity, $targetTableAlias)) { - if($first) $first = false; else $filterSql .= ' AND '; - $filterSql .= '(' . $filterExpr . ')'; + $filterClauses[] = '(' . $filterExpr . ')'; } } - return $filterSql; + return implode(' AND ', $filterClauses); } } diff --git a/lib/Doctrine/ORM/Persisters/OneToManyPersister.php b/lib/Doctrine/ORM/Persisters/OneToManyPersister.php index 0cc380337..17575aaac 100644 --- a/lib/Doctrine/ORM/Persisters/OneToManyPersister.php +++ b/lib/Doctrine/ORM/Persisters/OneToManyPersister.php @@ -29,6 +29,7 @@ use Doctrine\ORM\PersistentCollection, * * @author Roman Borschel * @author Guilherme Blanco + * @author Alexander * @since 2.0 */ class OneToManyPersister extends AbstractCollectionPersister diff --git a/lib/Doctrine/ORM/Persisters/SingleTablePersister.php b/lib/Doctrine/ORM/Persisters/SingleTablePersister.php index d11486e93..cb005575f 100644 --- a/lib/Doctrine/ORM/Persisters/SingleTablePersister.php +++ b/lib/Doctrine/ORM/Persisters/SingleTablePersister.php @@ -27,6 +27,7 @@ use Doctrine\ORM\Mapping\ClassMetadata; * * @author Roman Borschel * @author Benjamin Eberlei + * @author Alexander * @since 2.0 * @link http://martinfowler.com/eaaCatalog/singleTableInheritance.html */ diff --git a/lib/Doctrine/ORM/Query/SqlWalker.php b/lib/Doctrine/ORM/Query/SqlWalker.php index e3d412df1..cf20f747b 100644 --- a/lib/Doctrine/ORM/Query/SqlWalker.php +++ b/lib/Doctrine/ORM/Query/SqlWalker.php @@ -33,6 +33,7 @@ use Doctrine\DBAL\LockMode, * @author Guilherme Blanco * @author Roman Borschel * @author Benjamin Eberlei + * @author Alexander * @since 2.0 * @todo Rename: SQLWalker */ @@ -362,19 +363,17 @@ class SqlWalker implements TreeWalker */ private function generateFilterConditionSQL(ClassMetadata $targetEntity, $targetTableAlias) { - $filterSql = ''; + $filterClauses = array(); if($this->_em->hasFilters()) { - $first = true; foreach($this->_em->getFilters()->getEnabledFilters() as $filter) { if("" !== $filterExpr = $filter->addFilterConstraint($targetEntity, $targetTableAlias)) { - if ( ! $first) $filterSql .= ' AND '; else $first = false; - $filterSql .= '(' . $filterExpr . ')'; + $filterClauses[] = '(' . $filterExpr . ')'; } } } - return $filterSql; + return implode(' AND ', $filterClauses); } /** * Walks down a SelectStatement AST node, thereby generating the appropriate SQL. From f4663f45127e0dcd029b992dedc426a3cfa6fd1d Mon Sep 17 00:00:00 2001 From: Alexander Date: Mon, 5 Dec 2011 22:19:54 +0100 Subject: [PATCH 25/49] [DDC-551] Another batch of small refactorings --- lib/Doctrine/ORM/EntityManager.php | 1 - lib/Doctrine/ORM/Persisters/ManyToManyPersister.php | 7 ++++--- lib/Doctrine/ORM/Query/SqlWalker.php | 11 ++++------- 3 files changed, 8 insertions(+), 11 deletions(-) diff --git a/lib/Doctrine/ORM/EntityManager.php b/lib/Doctrine/ORM/EntityManager.php index 9adf82301..3f9e477c7 100644 --- a/lib/Doctrine/ORM/EntityManager.php +++ b/lib/Doctrine/ORM/EntityManager.php @@ -800,7 +800,6 @@ class EntityManager implements ObjectManager /** * Gets the enabled filters. * - * @access public * @return FilterCollection The active filter collection. */ public function getFilters() diff --git a/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php b/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php index f765b77f1..e9f4b9820 100644 --- a/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php +++ b/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php @@ -360,11 +360,12 @@ class ManyToManyPersister extends AbstractCollectionPersister . $targetClass->getQuotedTableName($this->_conn->getDatabasePlatform()) . ' te' . ' ON'; - $first = true; + $joinTargetEntitySQLClauses = array(); foreach($mapping['relationToTargetKeyColumns'] as $joinTableColumn => $targetTableColumn) { - if(!$first) $joinTargetEntitySQL .= ' AND '; else $first = false; - $joinTargetEntitySQL .= ' t.' . $joinTableColumn . ' = ' . 'te.' . $targetTableColumn; + $joinTargetEntitySQLClauses[] = ' t.' . $joinTableColumn . ' = ' . 'te.' . $targetTableColumn; } + + $joinTargetEntitySQL .= implode(' AND ', $joinTargetEntitySQLClauses); } return array($joinTargetEntitySQL, $filterSql); diff --git a/lib/Doctrine/ORM/Query/SqlWalker.php b/lib/Doctrine/ORM/Query/SqlWalker.php index cf20f747b..e6a08bf5a 100644 --- a/lib/Doctrine/ORM/Query/SqlWalker.php +++ b/lib/Doctrine/ORM/Query/SqlWalker.php @@ -1502,24 +1502,21 @@ class SqlWalker implements TreeWalker $condSql = null !== $whereClause ? $this->walkConditionalExpression($whereClause->conditionalExpression) : ''; $discrSql = $this->_generateDiscriminatorColumnConditionSql($this->_rootAliases); - $filterSql = ''; - // Apply the filters for all entities in FROM - $first = true; + $filterClauses = array(); foreach ($this->_rootAliases as $dqlAlias) { $class = $this->_queryComponents[$dqlAlias]['metadata']; $tableAlias = $this->getSQLTableAlias($class->table['name'], $dqlAlias); if("" !== $filterExpr = $this->generateFilterConditionSQL($class, $tableAlias)) { - if ( ! $first) $filterSql .= ' AND '; else $first = false; - $filterSql .= $filterExpr; + $filterClauses[] = $filterExpr; } } - if ('' !== $filterSql) { + if (count($filterClauses)) { if($condSql) $condSql .= ' AND '; - $condSql .= $filterSql; + $condSql .= implode(' AND ', $filterClauses); } if ($condSql) { From efe7a014828508d46a06cbcb381a3c60e4b13c38 Mon Sep 17 00:00:00 2001 From: Alexander Date: Mon, 5 Dec 2011 23:00:52 +0100 Subject: [PATCH 26/49] [DDC-551] Fixed CS, comments by @stof --- lib/Doctrine/ORM/EntityManager.php | 6 ++--- .../ORM/Persisters/BasicEntityPersister.php | 19 +++++++++------- .../Persisters/JoinedSubclassPersister.php | 13 ++++++----- .../ORM/Persisters/ManyToManyPersister.php | 14 ++++++------ .../ORM/Persisters/OneToManyPersister.php | 4 ++-- .../ORM/Persisters/SingleTablePersister.php | 2 +- lib/Doctrine/ORM/Query/Filter/SQLFilter.php | 8 ++----- lib/Doctrine/ORM/Query/FilterCollection.php | 10 ++++----- lib/Doctrine/ORM/Query/SqlWalker.php | 22 +++++++++---------- 9 files changed, 50 insertions(+), 48 deletions(-) diff --git a/lib/Doctrine/ORM/EntityManager.php b/lib/Doctrine/ORM/EntityManager.php index 3f9e477c7..b353828ae 100644 --- a/lib/Doctrine/ORM/EntityManager.php +++ b/lib/Doctrine/ORM/EntityManager.php @@ -804,7 +804,7 @@ class EntityManager implements ObjectManager */ public function getFilters() { - if(null === $this->filterCollection) { + if (null === $this->filterCollection) { $this->filterCollection = new FilterCollection($this); } @@ -814,7 +814,7 @@ class EntityManager implements ObjectManager /** * Checks whether the state of the filter collection is clean. * - * @return boolean True, iff the filter collection is clean. + * @return boolean True, if the filter collection is clean. */ public function isFiltersStateClean() { @@ -825,7 +825,7 @@ class EntityManager implements ObjectManager /** * Checks whether the Entity Manager has filters. * - * @return True, iff the EM has a filter collection. + * @return True, if the EM has a filter collection. */ public function hasFilters() { diff --git a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php index c58c4c3bf..9ce8acc69 100644 --- a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php +++ b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php @@ -903,8 +903,11 @@ class BasicEntityPersister $alias = $this->_getSQLTableAlias($this->_class->name); - if('' !== $filterSql = $this->generateFilterConditionSQL($this->_class, $alias)) { - if($conditionSql) $conditionSql .= ' AND '; + if ('' !== $filterSql = $this->generateFilterConditionSQL($this->_class, $alias)) { + if ($conditionSql) { + $conditionSql .= ' AND '; + } + $conditionSql .= $filterSql; } @@ -1033,7 +1036,7 @@ class BasicEntityPersister } // Add filter SQL - if('' !== $filterSql = $this->generateFilterConditionSQL($eagerEntity, $tableAlias)) { + if ('' !== $filterSql = $this->generateFilterConditionSQL($eagerEntity, $tableAlias)) { $this->_selectJoinSql .= ' AND ' . $filterSql; } } else { @@ -1541,7 +1544,7 @@ class BasicEntityPersister . $this->getLockTablesSql($alias) . ' WHERE ' . $this->_getSelectConditionSQL($criteria); - if('' !== $filterSql = $this->generateFilterConditionSQL($this->_class, $alias)) { + if ('' !== $filterSql = $this->generateFilterConditionSQL($this->_class, $alias)) { $sql .= ' AND ' . $filterSql; } @@ -1559,8 +1562,8 @@ class BasicEntityPersister protected function getJoinSQLForJoinColumns($joinColumns) { // if one of the join columns is nullable, return left join - foreach($joinColumns as $joinColumn) { - if(isset($joinColumn['nullable']) && $joinColumn['nullable']){ + foreach ($joinColumns as $joinColumn) { + if (isset($joinColumn['nullable']) && $joinColumn['nullable']) { return 'LEFT JOIN'; } } @@ -1595,8 +1598,8 @@ class BasicEntityPersister { $filterClauses = array(); - foreach($this->_em->getFilters()->getEnabledFilters() as $filter) { - if('' !== $filterExpr = $filter->addFilterConstraint($targetEntity, $targetTableAlias)) { + foreach ($this->_em->getFilters()->getEnabledFilters() as $filter) { + if ('' !== $filterExpr = $filter->addFilterConstraint($targetEntity, $targetTableAlias)) { $filterClauses[] = '(' . $filterExpr . ')'; } } diff --git a/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php b/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php index ad34f5c31..02d6ab75c 100644 --- a/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php +++ b/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php @@ -329,9 +329,9 @@ class JoinedSubclassPersister extends AbstractEntityInheritancePersister $joinSql .= $baseTableAlias . '.' . $idColumn . ' = ' . $tableAlias . '.' . $idColumn; - if($parentClass->name === $this->_class->rootEntityName) { + if ($parentClass->name === $this->_class->rootEntityName) { // Add filters on the root class - if('' !== $filterSql = $this->generateFilterConditionSQL($parentClass, $tableAlias)) { + if ('' !== $filterSql = $this->generateFilterConditionSQL($parentClass, $tableAlias)) { $joinSql .= ' AND ' . $filterSql; } } @@ -383,9 +383,12 @@ class JoinedSubclassPersister extends AbstractEntityInheritancePersister $conditionSql = $this->_getSelectConditionSQL($criteria, $assoc); // If the current class in the root entity, add the filters - if($this->_class->name === $this->_class->rootEntityName) { - if('' !== $filterSql = $this->generateFilterConditionSQL($this->_class, $baseTableAlias)) { - if($conditionSql) $conditionSql .= ' AND '; + if ($this->_class->name === $this->_class->rootEntityName) { + if ('' !== $filterSql = $this->generateFilterConditionSQL($this->_class, $baseTableAlias)) { + if ($conditionSql) { + $conditionSql .= ' AND '; + } + $conditionSql .= $filterSql; } } diff --git a/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php b/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php index e9f4b9820..bfb6ef08e 100644 --- a/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php +++ b/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php @@ -219,7 +219,7 @@ class ManyToManyPersister extends AbstractCollectionPersister } list($joinTargetEntitySQL, $filterSql) = $this->getFilterSql($mapping); - if('' !== $filterSql) { + if ('' !== $filterSql) { $whereClauses[] = $filterSql; } @@ -331,9 +331,9 @@ class ManyToManyPersister extends AbstractCollectionPersister : $sourceId[$sourceClass->fieldNames[$mapping['relationToSourceKeyColumns'][$joinTableColumn]]]; } - if($addFilters) { + if ($addFilters) { list($joinTargetEntitySQL, $filterSql) = $this->getFilterSql($mapping); - if('' !== $filterSql) { + if ('' !== $filterSql) { $quotedJoinTable .= ' t ' . $joinTargetEntitySQL; $whereClauses[] = $filterSql; } @@ -355,13 +355,13 @@ class ManyToManyPersister extends AbstractCollectionPersister // A join is needed if there is filtering on the target entity $joinTargetEntitySQL = ''; - if('' !== $filterSql = $this->generateFilterConditionSQL($targetClass, 'te')) { + if ('' !== $filterSql = $this->generateFilterConditionSQL($targetClass, 'te')) { $joinTargetEntitySQL = ' JOIN ' . $targetClass->getQuotedTableName($this->_conn->getDatabasePlatform()) . ' te' . ' ON'; $joinTargetEntitySQLClauses = array(); - foreach($mapping['relationToTargetKeyColumns'] as $joinTableColumn => $targetTableColumn) { + foreach ($mapping['relationToTargetKeyColumns'] as $joinTableColumn => $targetTableColumn) { $joinTargetEntitySQLClauses[] = ' t.' . $joinTableColumn . ' = ' . 'te.' . $targetTableColumn; } @@ -383,8 +383,8 @@ class ManyToManyPersister extends AbstractCollectionPersister { $filterClauses = array(); - foreach($this->_em->getFilters()->getEnabledFilters() as $filter) { - if('' !== $filterExpr = $filter->addFilterConstraint($targetEntity, $targetTableAlias)) { + foreach ($this->_em->getFilters()->getEnabledFilters() as $filter) { + if ('' !== $filterExpr = $filter->addFilterConstraint($targetEntity, $targetTableAlias)) { $filterClauses[] = '(' . $filterExpr . ')'; } } diff --git a/lib/Doctrine/ORM/Persisters/OneToManyPersister.php b/lib/Doctrine/ORM/Persisters/OneToManyPersister.php index 17575aaac..2fe1d5acb 100644 --- a/lib/Doctrine/ORM/Persisters/OneToManyPersister.php +++ b/lib/Doctrine/ORM/Persisters/OneToManyPersister.php @@ -121,8 +121,8 @@ class OneToManyPersister extends AbstractCollectionPersister : $id[$sourceClass->fieldNames[$joinColumn['referencedColumnName']]]; } - foreach($this->_em->getFilters()->getEnabledFilters() as $filter) { - if("" !== $filterExpr = $filter->addFilterConstraint($targetClass, 't')) { + foreach ($this->_em->getFilters()->getEnabledFilters() as $filter) { + if ('' !== $filterExpr = $filter->addFilterConstraint($targetClass, 't')) { $whereClauses[] = '(' . $filterExpr . ')'; } } diff --git a/lib/Doctrine/ORM/Persisters/SingleTablePersister.php b/lib/Doctrine/ORM/Persisters/SingleTablePersister.php index cb005575f..39a6b4954 100644 --- a/lib/Doctrine/ORM/Persisters/SingleTablePersister.php +++ b/lib/Doctrine/ORM/Persisters/SingleTablePersister.php @@ -138,7 +138,7 @@ class SingleTablePersister extends AbstractEntityInheritancePersister { // Ensure that the filters are applied to the root entity of the inheritance tree $realTargetEntity = $targetEntity; - if($targetEntity->name !== $targetEntity->rootEntityName) { + if ($targetEntity->name !== $targetEntity->rootEntityName) { $realTargetEntity = $this->_em->getClassMetadata($targetEntity->rootEntityName); } diff --git a/lib/Doctrine/ORM/Query/Filter/SQLFilter.php b/lib/Doctrine/ORM/Query/Filter/SQLFilter.php index 9be8628a2..772ef683b 100644 --- a/lib/Doctrine/ORM/Query/Filter/SQLFilter.php +++ b/lib/Doctrine/ORM/Query/Filter/SQLFilter.php @@ -49,7 +49,6 @@ abstract class SQLFilter * Constructs the SQLFilter object. * * @param EntityManager $em The EM - * @final */ final public function __construct(EntityManager $em) { @@ -63,7 +62,6 @@ abstract class SQLFilter * @param string $value Value of the parameter. * @param string $type Type of the parameter. * - * @final * @return SQLFilter The current SQL filter. */ final public function setParameter($name, $value, $type) @@ -88,12 +86,11 @@ abstract class SQLFilter * * @param string $name Name of the parameter. * - * @final * @return string The SQL escaped parameter to use in a query. */ final public function getParameter($name) { - if(!isset($this->parameters[$name])) { + if (!isset($this->parameters[$name])) { throw new \InvalidArgumentException("Parameter '" . $name . "' does not exist."); } @@ -103,8 +100,7 @@ abstract class SQLFilter /** * Returns as string representation of the SQLFilter parameters (the state). * - * @final - * @return string String representation of the the SQLFilter. + * @return string String representation of the SQLFilter. */ final public function __toString() { diff --git a/lib/Doctrine/ORM/Query/FilterCollection.php b/lib/Doctrine/ORM/Query/FilterCollection.php index 8e055ecdb..35c8d043c 100644 --- a/lib/Doctrine/ORM/Query/FilterCollection.php +++ b/lib/Doctrine/ORM/Query/FilterCollection.php @@ -103,11 +103,11 @@ class FilterCollection */ public function enable($name) { - if(null === $filterClass = $this->config->getFilterClassName($name)) { + if (null === $filterClass = $this->config->getFilterClassName($name)) { throw new \InvalidArgumentException("Filter '" . $name . "' does not exist."); } - if(!isset($this->enabledFilters[$name])) { + if (!isset($this->enabledFilters[$name])) { $this->enabledFilters[$name] = new $filterClass($this->em); // Keep the enabled filters sorted for the hash @@ -153,7 +153,7 @@ class FilterCollection */ public function getFilter($name) { - if(!isset($this->enabledFilters[$name])) { + if (!isset($this->enabledFilters[$name])) { throw new \InvalidArgumentException("Filter '" . $name . "' is not enabled."); } @@ -176,12 +176,12 @@ class FilterCollection public function getHash() { // If there are only clean filters, the previous hash can be returned - if(self::FILTERS_STATE_CLEAN === $this->filtersState) { + if (self::FILTERS_STATE_CLEAN === $this->filtersState) { return $this->filterHash; } $filterHash = ''; - foreach($this->enabledFilters as $name => $filter) { + foreach ($this->enabledFilters as $name => $filter) { $filterHash .= $name . $filter; } diff --git a/lib/Doctrine/ORM/Query/SqlWalker.php b/lib/Doctrine/ORM/Query/SqlWalker.php index e6a08bf5a..ea834fc6a 100644 --- a/lib/Doctrine/ORM/Query/SqlWalker.php +++ b/lib/Doctrine/ORM/Query/SqlWalker.php @@ -365,9 +365,9 @@ class SqlWalker implements TreeWalker { $filterClauses = array(); - if($this->_em->hasFilters()) { - foreach($this->_em->getFilters()->getEnabledFilters() as $filter) { - if("" !== $filterExpr = $filter->addFilterConstraint($targetEntity, $targetTableAlias)) { + if ($this->_em->hasFilters()) { + foreach ($this->_em->getFilters()->getEnabledFilters() as $filter) { + if ('' !== $filterExpr = $filter->addFilterConstraint($targetEntity, $targetTableAlias)) { $filterClauses[] = '(' . $filterExpr . ')'; } } @@ -827,9 +827,8 @@ class SqlWalker implements TreeWalker } // Apply the filters - if("" !== $filterExpr = $this->generateFilterConditionSQL($targetClass, $targetTableAlias)) { - if ( ! $first) $sql .= ' AND '; else $first = false; - $sql .= $filterExpr; + if ('' !== $filterExpr = $this->generateFilterConditionSQL($targetClass, $targetTableAlias)) { + $sql .= ' AND ' . $filterExpr; } } else if ($assoc['type'] == ClassMetadata::MANY_TO_MANY) { // Join relation table @@ -896,9 +895,8 @@ class SqlWalker implements TreeWalker } // Apply the filters - if("" !== $filterExpr = $this->generateFilterConditionSQL($targetClass, $targetTableAlias)) { - if ( ! $first) $sql .= ' AND '; else $first = false; - $sql .= $filterExpr; + if ('' !== $filterExpr = $this->generateFilterConditionSQL($targetClass, $targetTableAlias)) { + $sql .= ' AND ' . $filterExpr; } } @@ -1508,13 +1506,15 @@ class SqlWalker implements TreeWalker $class = $this->_queryComponents[$dqlAlias]['metadata']; $tableAlias = $this->getSQLTableAlias($class->table['name'], $dqlAlias); - if("" !== $filterExpr = $this->generateFilterConditionSQL($class, $tableAlias)) { + if ('' !== $filterExpr = $this->generateFilterConditionSQL($class, $tableAlias)) { $filterClauses[] = $filterExpr; } } if (count($filterClauses)) { - if($condSql) $condSql .= ' AND '; + if ($condSql) { + $condSql .= ' AND '; + } $condSql .= implode(' AND ', $filterClauses); } From 5e91f0c1ca473f2c899c329c3a844257e7f48cd5 Mon Sep 17 00:00:00 2001 From: Alexander Date: Wed, 7 Dec 2011 10:02:15 +0100 Subject: [PATCH 27/49] [DDC-551] Update SQLWalker to reflect filter requirements for inheritance --- lib/Doctrine/ORM/Query/SqlWalker.php | 37 ++++++++++++++++--- .../Tests/ORM/Functional/SQLFilterTest.php | 14 +++++++ 2 files changed, 46 insertions(+), 5 deletions(-) diff --git a/lib/Doctrine/ORM/Query/SqlWalker.php b/lib/Doctrine/ORM/Query/SqlWalker.php index ea834fc6a..69bf903be 100644 --- a/lib/Doctrine/ORM/Query/SqlWalker.php +++ b/lib/Doctrine/ORM/Query/SqlWalker.php @@ -268,6 +268,11 @@ class SqlWalker implements TreeWalker $sqlParts[] = $baseTableAlias . '.' . $columnName . ' = ' . $tableAlias . '.' . $columnName; } + // Add filters on the root class + if ('' !== $filterSql = $this->generateFilterConditionSQL($parentClass, $tableAlias)) { + $sqlParts[] = $filterSql; + } + $sql .= implode(' AND ', $sqlParts); } @@ -363,13 +368,35 @@ class SqlWalker implements TreeWalker */ private function generateFilterConditionSQL(ClassMetadata $targetEntity, $targetTableAlias) { - $filterClauses = array(); + if (!$this->_em->hasFilters()) { + return ''; + } - if ($this->_em->hasFilters()) { - foreach ($this->_em->getFilters()->getEnabledFilters() as $filter) { - if ('' !== $filterExpr = $filter->addFilterConstraint($targetEntity, $targetTableAlias)) { - $filterClauses[] = '(' . $filterExpr . ')'; + switch($targetEntity->inheritanceType) { + case ClassMetadata::INHERITANCE_TYPE_NONE: + break; + case ClassMetadata::INHERITANCE_TYPE_JOINED: + // The classes in the inheritance will be added to the query one by one, + // but only the root node is getting filtered + if ($targetEntity->name !== $targetEntity->rootEntityName) { + return ''; } + break; + case ClassMetadata::INHERITANCE_TYPE_SINGLE_TABLE: + // With STI the table will only be queried once, make sure that the filters + // are added to the root entity + $targetEntity = $this->_em->getClassMetadata($targetEntity->rootEntityName); + break; + default: + //@todo: throw exception? + return ''; + break; + } + + $filterClauses = array(); + foreach ($this->_em->getFilters()->getEnabledFilters() as $filter) { + if ('' !== $filterExpr = $filter->addFilterConstraint($targetEntity, $targetTableAlias)) { + $filterClauses[] = '(' . $filterExpr . ')'; } } diff --git a/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php b/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php index ff9332373..d56ddb99b 100644 --- a/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php @@ -519,7 +519,10 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase public function testJoinSubclassPersister_FilterOnlyOnRootTableWhenFetchingSubEntity() { $this->loadCompanyJoinedSubclassFixtureData(); + // Persister $this->assertEquals(2, count($this->_em->getRepository('Doctrine\Tests\Models\Company\CompanyManager')->findAll())); + // SQLWalker + $this->assertEquals(2, count($this->_em->createQuery("SELECT cm FROM Doctrine\Tests\Models\Company\CompanyManager cm")->getResult())); // Enable the filter $conf = $this->_em->getConfiguration(); @@ -531,12 +534,15 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase $managers = $this->_em->getRepository('Doctrine\Tests\Models\Company\CompanyManager')->findAll(); $this->assertEquals(1, count($managers)); $this->assertEquals("Guilherme", $managers[0]->getName()); + + $this->assertEquals(1, count($this->_em->createQuery("SELECT cm FROM Doctrine\Tests\Models\Company\CompanyManager cm")->getResult())); } public function testJoinSubclassPersister_FilterOnlyOnRootTableWhenFetchingRootEntity() { $this->loadCompanyJoinedSubclassFixtureData(); $this->assertEquals(3, count($this->_em->getRepository('Doctrine\Tests\Models\Company\CompanyPerson')->findAll())); + $this->assertEquals(3, count($this->_em->createQuery("SELECT cp FROM Doctrine\Tests\Models\Company\CompanyPerson cp")->getResult())); // Enable the filter $conf = $this->_em->getConfiguration(); @@ -548,6 +554,8 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase $persons = $this->_em->getRepository('Doctrine\Tests\Models\Company\CompanyPerson')->findAll(); $this->assertEquals(1, count($persons)); $this->assertEquals("Guilherme", $persons[0]->getName()); + + $this->assertEquals(1, count($this->_em->createQuery("SELECT cp FROM Doctrine\Tests\Models\Company\CompanyPerson cp")->getResult())); } private function loadCompanyJoinedSubclassFixtureData() @@ -577,7 +585,10 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase public function testSingleTableInheritance_FilterOnlyOnRootTableWhenFetchingSubEntity() { $this->loadCompanySingleTableInheritanceFixtureData(); + // Persister $this->assertEquals(2, count($this->_em->getRepository('Doctrine\Tests\Models\Company\CompanyFlexUltraContract')->findAll())); + // SQLWalker + $this->assertEquals(2, count($this->_em->createQuery("SELECT cfc FROM Doctrine\Tests\Models\Company\CompanyFlexUltraContract cfc")->getResult())); // Enable the filter $conf = $this->_em->getConfiguration(); @@ -586,12 +597,14 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase ->enable("completed_contract"); $this->assertEquals(1, count($this->_em->getRepository('Doctrine\Tests\Models\Company\CompanyFlexUltraContract')->findAll())); + $this->assertEquals(1, count($this->_em->createQuery("SELECT cfc FROM Doctrine\Tests\Models\Company\CompanyFlexUltraContract cfc")->getResult())); } public function testSingleTableInheritance_FilterOnlyOnRootTableWhenFetchingRootEntity() { $this->loadCompanySingleTableInheritanceFixtureData(); $this->assertEquals(4, count($this->_em->getRepository('Doctrine\Tests\Models\Company\CompanyFlexContract')->findAll())); + $this->assertEquals(4, count($this->_em->createQuery("SELECT cfc FROM Doctrine\Tests\Models\Company\CompanyFlexContract cfc")->getResult())); // Enable the filter $conf = $this->_em->getConfiguration(); @@ -600,6 +613,7 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase ->enable("completed_contract"); $this->assertEquals(2, count($this->_em->getRepository('Doctrine\Tests\Models\Company\CompanyFlexContract')->findAll())); + $this->assertEquals(2, count($this->_em->createQuery("SELECT cfc FROM Doctrine\Tests\Models\Company\CompanyFlexContract cfc")->getResult())); } private function loadCompanySingleTableInheritanceFixtureData() From f7175c229ee85e33102066d9740d5c385cd683f7 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sun, 11 Dec 2011 18:39:04 +0100 Subject: [PATCH 28/49] DDC-551 - Fix some ugly yoda conditions and a wrong nesting. --- lib/Doctrine/ORM/Persisters/BasicEntityPersister.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php index 9ce8acc69..40ccf91d0 100644 --- a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php +++ b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php @@ -903,7 +903,7 @@ class BasicEntityPersister $alias = $this->_getSQLTableAlias($this->_class->name); - if ('' !== $filterSql = $this->generateFilterConditionSQL($this->_class, $alias)) { + if ($filterSql = $this->generateFilterConditionSQL($this->_class, $alias)) { if ($conditionSql) { $conditionSql .= ' AND '; } @@ -1025,11 +1025,11 @@ class BasicEntityPersister $this->_selectJoinSql .= ' ' . $this->getJoinSQLForJoinColumns($assoc['joinColumns']); $this->_selectJoinSql .= ' ' . $eagerEntity->getQuotedTableName($this->_platform) . ' ' . $this->_getSQLTableAlias($eagerEntity->name, $assocAlias) .' ON '; + $tableAlias = $this->_getSQLTableAlias($assoc['targetEntity'], $assocAlias); foreach ($assoc['sourceToTargetKeyColumns'] AS $sourceCol => $targetCol) { if ( ! $first) { $this->_selectJoinSql .= ' AND '; } - $tableAlias = $this->_getSQLTableAlias($assoc['targetEntity'], $assocAlias); $this->_selectJoinSql .= $this->_getSQLTableAlias($assoc['sourceEntity']) . '.' . $sourceCol . ' = ' . $tableAlias . '.' . $targetCol; $first = false; From 69b1eb5c6490ecea4ea181cbd6a75ea1fb2771e1 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sun, 11 Dec 2011 18:46:57 +0100 Subject: [PATCH 29/49] DDC-551 - Fix locking mess with filters --- lib/Doctrine/ORM/Persisters/BasicEntityPersister.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php index 40ccf91d0..4cec1314d 100644 --- a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php +++ b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php @@ -1036,7 +1036,7 @@ class BasicEntityPersister } // Add filter SQL - if ('' !== $filterSql = $this->generateFilterConditionSQL($eagerEntity, $tableAlias)) { + if ($filterSql = $this->generateFilterConditionSQL($eagerEntity, $tableAlias)) { $this->_selectJoinSql .= ' AND ' . $filterSql; } } else { @@ -1284,10 +1284,10 @@ class BasicEntityPersister * * @return string */ - protected function getLockTablesSql($alias = null) + protected function getLockTablesSql() { return 'FROM ' . $this->_class->getQuotedTableName($this->_platform) . ' ' - . (null !== $alias ? $alias : $this->_getSQLTableAlias($this->_class->name)); + . $this->_getSQLTableAlias($this->_class->name); } /** @@ -1541,10 +1541,10 @@ class BasicEntityPersister $alias = $this->_getSQLTableAlias($this->_class->name); $sql = 'SELECT 1 ' - . $this->getLockTablesSql($alias) + . $this->getLockTablesSql() . ' WHERE ' . $this->_getSelectConditionSQL($criteria); - if ('' !== $filterSql = $this->generateFilterConditionSQL($this->_class, $alias)) { + if ($filterSql = $this->generateFilterConditionSQL($this->_class, $alias)) { $sql .= ' AND ' . $filterSql; } From ca5dbb182a92466fbda9e524c0452e589a866100 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sun, 11 Dec 2011 19:27:50 +0100 Subject: [PATCH 30/49] DDC-551 - Make filters case-sensitive everywhere --- lib/Doctrine/ORM/Configuration.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/Doctrine/ORM/Configuration.php b/lib/Doctrine/ORM/Configuration.php index 5f5a58b30..874d3725f 100644 --- a/lib/Doctrine/ORM/Configuration.php +++ b/lib/Doctrine/ORM/Configuration.php @@ -504,7 +504,7 @@ class Configuration extends \Doctrine\DBAL\Configuration */ public function addFilter($name, $className) { - $this->_attributes['filters'][strtolower($name)] = $className; + $this->_attributes['filters'][$name] = $className; } /** @@ -517,7 +517,6 @@ class Configuration extends \Doctrine\DBAL\Configuration */ public function getFilterClassName($name) { - $name = strtolower($name); return isset($this->_attributes['filters'][$name]) ? $this->_attributes['filters'][$name] : null; } From ad6130b02d5dd2ae37ee5621ed67e5b84e3199cb Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sun, 11 Dec 2011 19:29:36 +0100 Subject: [PATCH 31/49] DDC-551 - Cleanup filters branch, especially inheritance related code and yoda conditions and some inconsistencies --- .../ORM/Persisters/BasicEntityPersister.php | 3 ++- .../Persisters/JoinedSubclassPersister.php | 23 ++++++------------- .../ORM/Persisters/ManyToManyPersister.php | 18 +++++++++++---- .../ORM/Persisters/OneToManyPersister.php | 4 +--- .../ORM/Persisters/SingleTablePersister.php | 8 +++---- lib/Doctrine/ORM/Query/Filter/SQLFilter.php | 1 - 6 files changed, 26 insertions(+), 31 deletions(-) diff --git a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php index 4cec1314d..ea9e8cba4 100644 --- a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php +++ b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php @@ -1604,6 +1604,7 @@ class BasicEntityPersister } } - return implode(' AND ', $filterClauses); + $sql = implode(' AND ', $filterClauses); + return $sql ? "(" . $sql . ")" : ""; // Wrap again to avoid "X or Y and FilterConditionSQL" } } diff --git a/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php b/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php index 02d6ab75c..aca4d14b4 100644 --- a/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php +++ b/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php @@ -328,13 +328,6 @@ class JoinedSubclassPersister extends AbstractEntityInheritancePersister if ($first) $first = false; else $joinSql .= ' AND '; $joinSql .= $baseTableAlias . '.' . $idColumn . ' = ' . $tableAlias . '.' . $idColumn; - - if ($parentClass->name === $this->_class->rootEntityName) { - // Add filters on the root class - if ('' !== $filterSql = $this->generateFilterConditionSQL($parentClass, $tableAlias)) { - $joinSql .= ' AND ' . $filterSql; - } - } } } @@ -383,14 +376,12 @@ class JoinedSubclassPersister extends AbstractEntityInheritancePersister $conditionSql = $this->_getSelectConditionSQL($criteria, $assoc); // If the current class in the root entity, add the filters - if ($this->_class->name === $this->_class->rootEntityName) { - if ('' !== $filterSql = $this->generateFilterConditionSQL($this->_class, $baseTableAlias)) { - if ($conditionSql) { - $conditionSql .= ' AND '; - } - - $conditionSql .= $filterSql; + if ($filterSql = $this->generateFilterConditionSQL($this->_em->getClassMetadata($this->_class->rootEntityName), $this->_getSQLTableAlias($this->_class->rootEntityName))) { + if ($conditionSql) { + $conditionSql .= ' AND '; } + + $conditionSql .= $filterSql; } $orderBy = ($assoc !== null && isset($assoc['orderBy'])) ? $assoc['orderBy'] : $orderBy; @@ -420,10 +411,10 @@ class JoinedSubclassPersister extends AbstractEntityInheritancePersister * * @return string */ - public function getLockTablesSql($alias = null) + public function getLockTablesSql() { $idColumns = $this->_class->getIdentifierColumnNames(); - $baseTableAlias = null !== $alias ? $alias : $this->_getSQLTableAlias($this->_class->name); + $baseTableAlias = $this->_getSQLTableAlias($this->_class->name); // INNER JOIN parent tables $joinSql = ''; diff --git a/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php b/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php index bfb6ef08e..99391d9fa 100644 --- a/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php +++ b/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php @@ -219,7 +219,7 @@ class ManyToManyPersister extends AbstractCollectionPersister } list($joinTargetEntitySQL, $filterSql) = $this->getFilterSql($mapping); - if ('' !== $filterSql) { + if ($filterSql) { $whereClauses[] = $filterSql; } @@ -333,7 +333,7 @@ class ManyToManyPersister extends AbstractCollectionPersister if ($addFilters) { list($joinTargetEntitySQL, $filterSql) = $this->getFilterSql($mapping); - if ('' !== $filterSql) { + if ($filterSql) { $quotedJoinTable .= ' t ' . $joinTargetEntitySQL; $whereClauses[] = $filterSql; } @@ -345,6 +345,12 @@ class ManyToManyPersister extends AbstractCollectionPersister /** * Generates the filter SQL for a given mapping. * + * This method is not used for actually grabbing the related entities + * but when the extra-lazy collection methods are called on a filtered + * association. This is why besides the many to many table we also + * have to join in the actual entities table leading to additional + * JOIN. + * * @param array $targetEntity Array containing mapping information. * * @return string The SQL query part to add to a query. @@ -352,10 +358,11 @@ class ManyToManyPersister extends AbstractCollectionPersister public function getFilterSql($mapping) { $targetClass = $this->_em->getClassMetadata($mapping['targetEntity']); + $targetClass = $this->_em->getClassMetadata($targetClass->rootEntityName); // A join is needed if there is filtering on the target entity $joinTargetEntitySQL = ''; - if ('' !== $filterSql = $this->generateFilterConditionSQL($targetClass, 'te')) { + if ($filterSql = $this->generateFilterConditionSQL($targetClass, 'te')) { $joinTargetEntitySQL = ' JOIN ' . $targetClass->getQuotedTableName($this->_conn->getDatabasePlatform()) . ' te' . ' ON'; @@ -384,11 +391,12 @@ class ManyToManyPersister extends AbstractCollectionPersister $filterClauses = array(); foreach ($this->_em->getFilters()->getEnabledFilters() as $filter) { - if ('' !== $filterExpr = $filter->addFilterConstraint($targetEntity, $targetTableAlias)) { + if ($filterExpr = $filter->addFilterConstraint($targetEntity, $targetTableAlias)) { $filterClauses[] = '(' . $filterExpr . ')'; } } - return implode(' AND ', $filterClauses); + $sql = implode(' AND ', $filterClauses); + return $sql ? "(" . $sql . ")" : ""; } } diff --git a/lib/Doctrine/ORM/Persisters/OneToManyPersister.php b/lib/Doctrine/ORM/Persisters/OneToManyPersister.php index 2fe1d5acb..b78d0e561 100644 --- a/lib/Doctrine/ORM/Persisters/OneToManyPersister.php +++ b/lib/Doctrine/ORM/Persisters/OneToManyPersister.php @@ -1,7 +1,5 @@ _em->getFilters()->getEnabledFilters() as $filter) { - if ('' !== $filterExpr = $filter->addFilterConstraint($targetClass, 't')) { + if ($filterExpr = $filter->addFilterConstraint($targetClass, 't')) { $whereClauses[] = '(' . $filterExpr . ')'; } } diff --git a/lib/Doctrine/ORM/Persisters/SingleTablePersister.php b/lib/Doctrine/ORM/Persisters/SingleTablePersister.php index 39a6b4954..c2687c109 100644 --- a/lib/Doctrine/ORM/Persisters/SingleTablePersister.php +++ b/lib/Doctrine/ORM/Persisters/SingleTablePersister.php @@ -137,11 +137,9 @@ class SingleTablePersister extends AbstractEntityInheritancePersister protected function generateFilterConditionSQL(ClassMetadata $targetEntity, $targetTableAlias) { // Ensure that the filters are applied to the root entity of the inheritance tree - $realTargetEntity = $targetEntity; - if ($targetEntity->name !== $targetEntity->rootEntityName) { - $realTargetEntity = $this->_em->getClassMetadata($targetEntity->rootEntityName); - } + $targetEntity = $this->_em->getClassMetadata($targetEntity->rootEntityName); + // we dont care about the $targetTableAlias, in a STI there is only one table. - return parent::generateFilterConditionSQL($realTargetEntity, $targetTableAlias); + return parent::generateFilterConditionSQL($targetEntity, $targetTableAlias); } } diff --git a/lib/Doctrine/ORM/Query/Filter/SQLFilter.php b/lib/Doctrine/ORM/Query/Filter/SQLFilter.php index 772ef683b..1250b16d7 100644 --- a/lib/Doctrine/ORM/Query/Filter/SQLFilter.php +++ b/lib/Doctrine/ORM/Query/Filter/SQLFilter.php @@ -66,7 +66,6 @@ abstract class SQLFilter */ final public function setParameter($name, $value, $type) { - // @todo: check for a valid type? $this->parameters[$name] = array('value' => $value, 'type' => $type); // Keep the parameters sorted for the hash From b6d776f75de2820a40ad2dd0150ce0c08d75a2c2 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sun, 11 Dec 2011 21:14:09 +0100 Subject: [PATCH 32/49] DDC-551 - rework walker filtering --- lib/Doctrine/ORM/Query/SqlWalker.php | 43 +++++++++++++--------------- 1 file changed, 20 insertions(+), 23 deletions(-) diff --git a/lib/Doctrine/ORM/Query/SqlWalker.php b/lib/Doctrine/ORM/Query/SqlWalker.php index 69bf903be..8b6a61827 100644 --- a/lib/Doctrine/ORM/Query/SqlWalker.php +++ b/lib/Doctrine/ORM/Query/SqlWalker.php @@ -269,7 +269,7 @@ class SqlWalker implements TreeWalker } // Add filters on the root class - if ('' !== $filterSql = $this->generateFilterConditionSQL($parentClass, $tableAlias)) { + if ($filterSql = $this->generateFilterConditionSQL($parentClass, $tableAlias)) { $sqlParts[] = $filterSql; } @@ -853,10 +853,6 @@ class SqlWalker implements TreeWalker } } - // Apply the filters - if ('' !== $filterExpr = $this->generateFilterConditionSQL($targetClass, $targetTableAlias)) { - $sql .= ' AND ' . $filterExpr; - } } else if ($assoc['type'] == ClassMetadata::MANY_TO_MANY) { // Join relation table $joinTable = $assoc['joinTable']; @@ -920,11 +916,11 @@ class SqlWalker implements TreeWalker $sql .= $targetTableAlias . '.' . $quotedTargetColumn . ' = ' . $joinTableAlias . '.' . $relationColumn; } } + } - // Apply the filters - if ('' !== $filterExpr = $this->generateFilterConditionSQL($targetClass, $targetTableAlias)) { - $sql .= ' AND ' . $filterExpr; - } + // Apply the filters + if ($filterExpr = $this->generateFilterConditionSQL($targetClass, $targetTableAlias)) { + $sql .= ' AND ' . $filterExpr; } // Handle WITH clause @@ -1527,23 +1523,24 @@ class SqlWalker implements TreeWalker $condSql = null !== $whereClause ? $this->walkConditionalExpression($whereClause->conditionalExpression) : ''; $discrSql = $this->_generateDiscriminatorColumnConditionSql($this->_rootAliases); - // Apply the filters for all entities in FROM - $filterClauses = array(); - foreach ($this->_rootAliases as $dqlAlias) { - $class = $this->_queryComponents[$dqlAlias]['metadata']; - $tableAlias = $this->getSQLTableAlias($class->table['name'], $dqlAlias); + if ($this->_em->hasFilters()) { + $filterClauses = array(); + foreach ($this->_rootAliases as $dqlAlias) { + $class = $this->_queryComponents[$dqlAlias]['metadata']; + $tableAlias = $this->getSQLTableAlias($class->table['name'], $dqlAlias); - if ('' !== $filterExpr = $this->generateFilterConditionSQL($class, $tableAlias)) { - $filterClauses[] = $filterExpr; - } - } - - if (count($filterClauses)) { - if ($condSql) { - $condSql .= ' AND '; + if ($filterExpr = $this->generateFilterConditionSQL($class, $tableAlias)) { + $filterClauses[] = $filterExpr; + } } - $condSql .= implode(' AND ', $filterClauses); + if (count($filterClauses)) { + if ($condSql) { + $condSql .= ' AND '; + } + + $condSql .= implode(' AND ', $filterClauses); + } } if ($condSql) { From 017a7d889f109d9501808080d1b13961348c87f9 Mon Sep 17 00:00:00 2001 From: "Fabio B. Silva" Date: Thu, 15 Dec 2011 17:12:01 -0200 Subject: [PATCH 33/49] Fixed DDC-1468 --- .../ORM/Mapping/Driver/AbstractFileDriver.php | 3 +++ lib/Doctrine/ORM/Mapping/MappingException.php | 5 +++++ .../Tests/ORM/Mapping/XmlMappingDriverTest.php | 11 +++++++++++ .../Tests/ORM/Mapping/YamlMappingDriverTest.php | 11 +++++++++++ ...ts.Models.Generic.SerializationModel.dcm.xml | 17 +++++++++++++++++ ...ts.Models.Generic.SerializationModel.dcm.yml | 13 +++++++++++++ 6 files changed, 60 insertions(+) create mode 100644 tests/Doctrine/Tests/ORM/Mapping/xml/Doctrine.Tests.Models.Generic.SerializationModel.dcm.xml create mode 100644 tests/Doctrine/Tests/ORM/Mapping/yaml/Doctrine.Tests.Models.Generic.SerializationModel.dcm.yml diff --git a/lib/Doctrine/ORM/Mapping/Driver/AbstractFileDriver.php b/lib/Doctrine/ORM/Mapping/Driver/AbstractFileDriver.php index 457b7cda7..a92432db8 100644 --- a/lib/Doctrine/ORM/Mapping/Driver/AbstractFileDriver.php +++ b/lib/Doctrine/ORM/Mapping/Driver/AbstractFileDriver.php @@ -118,6 +118,9 @@ abstract class AbstractFileDriver implements Driver { $result = $this->_loadMappingFile($this->_findMappingFile($className)); + if(!isset($result[$className])){ + throw MappingException::invalidMappingFile($className, str_replace('\\', '.', $className) . $this->_fileExtension); + } return $result[$className]; } diff --git a/lib/Doctrine/ORM/Mapping/MappingException.php b/lib/Doctrine/ORM/Mapping/MappingException.php index 014714bf8..bf8da82e6 100644 --- a/lib/Doctrine/ORM/Mapping/MappingException.php +++ b/lib/Doctrine/ORM/Mapping/MappingException.php @@ -67,6 +67,11 @@ class MappingException extends \Doctrine\ORM\ORMException { return new self("No mapping file found named '$fileName' for class '$entityName'."); } + + public static function invalidMappingFile($entityName, $fileName) + { + return new self("Invalid mapping file '$fileName' for class '$entityName'."); + } public static function mappingNotFound($className, $fieldName) { diff --git a/tests/Doctrine/Tests/ORM/Mapping/XmlMappingDriverTest.php b/tests/Doctrine/Tests/ORM/Mapping/XmlMappingDriverTest.php index 9ba8748f4..980a720df 100644 --- a/tests/Doctrine/Tests/ORM/Mapping/XmlMappingDriverTest.php +++ b/tests/Doctrine/Tests/ORM/Mapping/XmlMappingDriverTest.php @@ -51,6 +51,17 @@ class XmlMappingDriverTest extends AbstractMappingDriverTest $this->assertTrue($class->associationMappings['article']['id']); } + /** + * @group DDC-1468 + * + * @expectedException Doctrine\ORM\Mapping\MappingException + * @expectedExceptionMessage Invalid mapping file 'Doctrine.Tests.Models.Generic.SerializationModel.dcm.xml' for class 'Doctrine\Tests\Models\Generic\SerializationModel'. + */ + public function testInvalidMappingFileException() + { + $this->createClassMetadata('Doctrine\Tests\Models\Generic\SerializationModel'); + } + /** * @param string $xmlMappingFile * @dataProvider dataValidSchema diff --git a/tests/Doctrine/Tests/ORM/Mapping/YamlMappingDriverTest.php b/tests/Doctrine/Tests/ORM/Mapping/YamlMappingDriverTest.php index 34c3fa810..2757259a4 100644 --- a/tests/Doctrine/Tests/ORM/Mapping/YamlMappingDriverTest.php +++ b/tests/Doctrine/Tests/ORM/Mapping/YamlMappingDriverTest.php @@ -43,4 +43,15 @@ class YamlMappingDriverTest extends AbstractMappingDriverTest $this->assertEquals('Doctrine\Tests\Models\DirectoryTree\Directory', $classDirectory->associationMappings['parentDirectory']['sourceEntity']); } + /** + * @group DDC-1468 + * + * @expectedException Doctrine\ORM\Mapping\MappingException + * @expectedExceptionMessage Invalid mapping file 'Doctrine.Tests.Models.Generic.SerializationModel.dcm.yml' for class 'Doctrine\Tests\Models\Generic\SerializationModel'. + */ + public function testInvalidMappingFileException() + { + $this->createClassMetadata('Doctrine\Tests\Models\Generic\SerializationModel'); + } + } diff --git a/tests/Doctrine/Tests/ORM/Mapping/xml/Doctrine.Tests.Models.Generic.SerializationModel.dcm.xml b/tests/Doctrine/Tests/ORM/Mapping/xml/Doctrine.Tests.Models.Generic.SerializationModel.dcm.xml new file mode 100644 index 000000000..36af855f2 --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Mapping/xml/Doctrine.Tests.Models.Generic.SerializationModel.dcm.xml @@ -0,0 +1,17 @@ + + + + + + + + + + + + + + \ No newline at end of file diff --git a/tests/Doctrine/Tests/ORM/Mapping/yaml/Doctrine.Tests.Models.Generic.SerializationModel.dcm.yml b/tests/Doctrine/Tests/ORM/Mapping/yaml/Doctrine.Tests.Models.Generic.SerializationModel.dcm.yml new file mode 100644 index 000000000..64f74b55f --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Mapping/yaml/Doctrine.Tests.Models.Generic.SerializationModel.dcm.yml @@ -0,0 +1,13 @@ +\stdClass: + type: entity + id: + id: + type: integer + unsigned: true + generator: + strategy: AUTO + fields: + array: + type: array + object: + type: object \ No newline at end of file From 98bd5cae64560aea670069833c10b22d25df2b8b Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sat, 17 Dec 2011 11:36:23 +0100 Subject: [PATCH 34/49] Revert "Incorporated setAssociationTargetClass, which solves biggest problem of modular system in ZF. Any questions, pelase forward to @EvanDotPro." This reverts commit cac9928f2853208603a75d835d20ff2a35da9326. --- lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php b/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php index 0c3a339eb..d845e6f59 100644 --- a/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php +++ b/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php @@ -1849,21 +1849,6 @@ class ClassMetadataInfo implements ClassMetadata } } - /** - * Refine an association targetEntity class pointer to be consumed through loadMetadata event. - * - * @param string $assoc - * @param string $class - */ - public function setAssociationTargetClass($assocName, $class) - { - if ( ! isset($this->associationMappings[$assocName])) { - throw new \InvalidArgumentException("Association name expected, '" . $assocName ."' is not an association."); - } - - $this->associationMappings[$assocName]['targetEntity'] = $class; - } - /** * Sets whether this class is to be versioned for optimistic locking. * From 60152530646aef7029ddaed0fb338b0454604652 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sat, 17 Dec 2011 12:35:22 +0100 Subject: [PATCH 35/49] DDC-1524 - Add validation and error messages for annotation named query code. --- lib/Doctrine/ORM/Mapping/Driver/AnnotationDriver.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/lib/Doctrine/ORM/Mapping/Driver/AnnotationDriver.php b/lib/Doctrine/ORM/Mapping/Driver/AnnotationDriver.php index d469fd66e..bee84159f 100644 --- a/lib/Doctrine/ORM/Mapping/Driver/AnnotationDriver.php +++ b/lib/Doctrine/ORM/Mapping/Driver/AnnotationDriver.php @@ -192,7 +192,14 @@ class AnnotationDriver implements Driver if (isset($classAnnotations['Doctrine\ORM\Mapping\NamedQueries'])) { $namedQueriesAnnot = $classAnnotations['Doctrine\ORM\Mapping\NamedQueries']; + if (!is_array($namedQueriesAnnot->value)) { + throw new \UnexpectedValueException("@NamedQueries should contain an array of @NamedQuery annotations."); + } + foreach ($namedQueriesAnnot->value as $namedQuery) { + if (!($namedQuery instanceof \Doctrine\ORM\Mapping\NamedQuery)) { + throw new \UnexpectedValueException("@NamedQueries should contain an array of @NamedQuery annotations."); + } $metadata->addNamedQuery(array( 'name' => $namedQuery->name, 'query' => $namedQuery->query From cfe125940092e4c7403201703ae0428b943432f5 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sat, 17 Dec 2011 12:39:44 +0100 Subject: [PATCH 36/49] DDC-1541 - Fix wrong references in ClassMetadataBuilder --- .../ORM/Mapping/Builder/ClassMetadataBuilder.php | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/Doctrine/ORM/Mapping/Builder/ClassMetadataBuilder.php b/lib/Doctrine/ORM/Mapping/Builder/ClassMetadataBuilder.php index f7d5031cd..3765fbd93 100644 --- a/lib/Doctrine/ORM/Mapping/Builder/ClassMetadataBuilder.php +++ b/lib/Doctrine/ORM/Mapping/Builder/ClassMetadataBuilder.php @@ -298,7 +298,7 @@ class ClassMetadataBuilder $builder = $this->createManyToOne($name, $targetEntity); if ($inversedBy) { - $builder->setInversedBy($inversedBy); + $builder->inversedBy($inversedBy); } return $builder->build(); @@ -355,7 +355,7 @@ class ClassMetadataBuilder public function addInverseOneToOne($name, $targetEntity, $mappedBy) { $builder = $this->createOneToOne($name, $targetEntity); - $builder->setMappedBy($mappedBy); + $builder->mappedBy($mappedBy); return $builder->build(); } @@ -373,7 +373,7 @@ class ClassMetadataBuilder $builder = $this->createOneToOne($name, $targetEntity); if ($inversedBy) { - $builder->setInversedBy($inversedBy); + $builder->inversedBy($inversedBy); } return $builder->build(); @@ -411,7 +411,7 @@ class ClassMetadataBuilder $builder = $this->createManyToMany($name, $targetEntity); if ($inversedBy) { - $builder->setInversedBy($inversedBy); + $builder->inversedBy($inversedBy); } return $builder->build(); @@ -428,7 +428,7 @@ class ClassMetadataBuilder public function addInverseManyToMany($name, $targetEntity, $mappedBy) { $builder = $this->createManyToMany($name, $targetEntity); - $builder->setMappedBy($mappedBy); + $builder->mappedBy($mappedBy); return $builder->build(); } @@ -463,7 +463,7 @@ class ClassMetadataBuilder public function addOneToMany($name, $targetEntity, $mappedBy) { $builder = $this->createOneToMany($name, $targetEntity); - $builder->setMappedBy($mappedBy); + $builder->mappedBy($mappedBy); return $builder->build(); } From 267ce7df889eb971e2e27e260ce03b110ce1f7a3 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sat, 17 Dec 2011 19:35:10 +0100 Subject: [PATCH 37/49] DDC-1544 - Add ResolveTargetEntityListener --- .../ORM/Tools/ResolveTargetEntityListener.php | 92 +++++++++++++++++++ 1 file changed, 92 insertions(+) create mode 100644 lib/Doctrine/ORM/Tools/ResolveTargetEntityListener.php diff --git a/lib/Doctrine/ORM/Tools/ResolveTargetEntityListener.php b/lib/Doctrine/ORM/Tools/ResolveTargetEntityListener.php new file mode 100644 index 000000000..6151763da --- /dev/null +++ b/lib/Doctrine/ORM/Tools/ResolveTargetEntityListener.php @@ -0,0 +1,92 @@ +. + */ + +namespace Doctrine\ORM\Tools; + +use Doctrine\ORM\Events\LoadClassMetadataEventArgs; + +/** + * ResolveTargetEntityListener + * + * Mechanism to overwrite interfaces or classes specified as association + * targets. + * + * @author Benjamin Eberlei + * @since 2.2 + */ +class ResolveTargetEntityListener +{ + /** + * @var array + */ + private $resolveTargetEntities = array(); + + /** + * Add a target-entity class name to resolve to a new class name. + * + * @param string $originalEntity + * @param string $newEntity + * @param array $mapping + * @return void + */ + public function addResolveTargetEntity($originalEntity, $newEntity, array $mapping) + { + $mapping['targetEntity'] = ltrim($newEntity, "\\"); + $this->resolveTargetEntities[ltrim($originalEntity, "\\")] = $mapping; + } + + /** + * Process event and resolve new target entity names. + * + * @param LoadClassMetadataEventArgs $args + * @return void + */ + public function loadClassMetadata(LoadClassMetadataEventArgs $args) + { + $cm = $args->getClassMetadata(); + foreach ($cm->associationMappings as $assocName => $mapping) { + if (isset($this->resolveTargetEntities[$mapping['targetEntity']])) { + $this->remapAssociation($cm, $mapping); + } + } + } + + private function remapAssocation($classMetadata, $mapping) + { + $newMapping = $this->resolveTargetEntities[$mapping['targetEntity']]; + $newMapping['fieldName'] = $mapping['fieldName']; + unset($cm->associationMappings[$mapping['fieldName']]); + + switch ($mapping['type']) { + case ClassMetadata::MANY_TO_MANY: + $cm->mapManyToMany($newMapping); + break; + case ClassMetadata::MANY_TO_ONE: + $cm->mapManyToOne($newMapping); + break; + case ClassMetadata::ONE_TO_MANY: + $cm->mapOneToMany($newMapping); + break; + case ClassMetadata::ONE_TO_ONE: + $cm->mapOneToOne($newMapping); + break; + } + } +} + From 36a47e391cffa9a65b8fb80ce6f82b4a94d2d9dc Mon Sep 17 00:00:00 2001 From: Evan Coury Date: Sat, 17 Dec 2011 15:00:05 -0700 Subject: [PATCH 38/49] DDC-1544 - Add unit test and assertions for ResolveTargetEntityListener --- .../ORM/Tools/ResolveTargetEntityListener.php | 16 ++- .../Tools/ResolveTargetEntityListenerTest.php | 129 ++++++++++++++++++ 2 files changed, 138 insertions(+), 7 deletions(-) create mode 100644 tests/Doctrine/Tests/ORM/Tools/ResolveTargetEntityListenerTest.php diff --git a/lib/Doctrine/ORM/Tools/ResolveTargetEntityListener.php b/lib/Doctrine/ORM/Tools/ResolveTargetEntityListener.php index 6151763da..621214fdd 100644 --- a/lib/Doctrine/ORM/Tools/ResolveTargetEntityListener.php +++ b/lib/Doctrine/ORM/Tools/ResolveTargetEntityListener.php @@ -19,7 +19,8 @@ namespace Doctrine\ORM\Tools; -use Doctrine\ORM\Events\LoadClassMetadataEventArgs; +use Doctrine\ORM\Event\LoadClassMetadataEventArgs; +use Doctrine\ORM\Mapping\ClassMetadata; /** * ResolveTargetEntityListener @@ -67,24 +68,25 @@ class ResolveTargetEntityListener } } - private function remapAssocation($classMetadata, $mapping) + private function remapAssociation($classMetadata, $mapping) { $newMapping = $this->resolveTargetEntities[$mapping['targetEntity']]; + $newMapping = array_replace_recursive($mapping, $newMapping); $newMapping['fieldName'] = $mapping['fieldName']; - unset($cm->associationMappings[$mapping['fieldName']]); + unset($classMetadata->associationMappings[$mapping['fieldName']]); switch ($mapping['type']) { case ClassMetadata::MANY_TO_MANY: - $cm->mapManyToMany($newMapping); + $classMetadata->mapManyToMany($newMapping); break; case ClassMetadata::MANY_TO_ONE: - $cm->mapManyToOne($newMapping); + $classMetadata->mapManyToOne($newMapping); break; case ClassMetadata::ONE_TO_MANY: - $cm->mapOneToMany($newMapping); + $classMetadata->mapOneToMany($newMapping); break; case ClassMetadata::ONE_TO_ONE: - $cm->mapOneToOne($newMapping); + $classMetadata->mapOneToOne($newMapping); break; } } diff --git a/tests/Doctrine/Tests/ORM/Tools/ResolveTargetEntityListenerTest.php b/tests/Doctrine/Tests/ORM/Tools/ResolveTargetEntityListenerTest.php new file mode 100644 index 000000000..b794474a8 --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Tools/ResolveTargetEntityListenerTest.php @@ -0,0 +1,129 @@ +createAnnotationDriver(); + + $this->em = $this->_getTestEntityManager(); + $this->em->getConfiguration()->setMetadataDriverImpl($annotationDriver); + $this->factory = new ClassMetadataFactory; + $this->factory->setEntityManager($this->em); + $this->listener = new ResolveTargetEntityListener; + } + + /** + * @group DDC-1544 + */ + public function testResolveTargetEntityListenerCanResolveTargetEntity() + { + $evm = $this->em->getEventManager(); + $this->listener->addResolveTargetEntity( + 'Doctrine\Tests\ORM\Tools\ResolveTargetInterface', + 'Doctrine\Tests\ORM\Tools\ResolveTargetEntity', + array() + ); + $this->listener->addResolveTargetEntity( + 'Doctrine\Tests\ORM\Tools\TargetInterface', + 'Doctrine\Tests\ORM\Tools\TargetEntity', + array() + ); + $evm->addEventListener(Events::loadClassMetadata, $this->listener); + $cm = $this->factory->getMetadataFor('Doctrine\Tests\ORM\Tools\ResolveTargetEntity'); + $meta = $cm->associationMappings; + $this->assertSame('Doctrine\Tests\ORM\Tools\TargetEntity', $meta['manyToMany']['targetEntity']); + $this->assertSame('Doctrine\Tests\ORM\Tools\ResolveTargetEntity', $meta['manyToOne']['targetEntity']); + $this->assertSame('Doctrine\Tests\ORM\Tools\ResolveTargetEntity', $meta['oneToMany']['targetEntity']); + $this->assertSame('Doctrine\Tests\ORM\Tools\TargetEntity', $meta['oneToOne']['targetEntity']); + } +} + +interface ResolveTargetInterface +{ + public function getId(); +} + +interface TargetInterface extends ResolveTargetInterface +{ +} + +/** + * @Entity + */ +class ResolveTargetEntity implements ResolveTargetInterface +{ + /** + * @Id + * @Column(type="integer") + * @GeneratedValue(strategy="AUTO") + */ + private $id; + + /** + * @ManyToMany(targetEntity="Doctrine\Tests\ORM\Tools\TargetInterface") + */ + private $manyToMany; + + /** + * @ManyToOne(targetEntity="Doctrine\Tests\ORM\Tools\ResolveTargetInterface", inversedBy="oneToMany") + */ + private $manyToOne; + + /** + * @OneToMany(targetEntity="Doctrine\Tests\ORM\Tools\ResolveTargetInterface", mappedBy="manyToOne") + */ + private $oneToMany; + + /** + * @OneToOne(targetEntity="Doctrine\Tests\ORM\Tools\TargetInterface") + * @JoinColumn(name="target_entity_id", referencedColumnName="id") + */ + private $oneToOne; + + public function getId() + { + return $this->id; + } +} + +/** + * @Entity + */ +class TargetEntity implements TargetInterface +{ + /** + * @Id + * @Column(type="integer") + * @GeneratedValue(strategy="AUTO") + */ + private $id; + + public function getId() + { + return $this->id; + } +} From 170271fd72a276a06308693060d7752b3f221a8b Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sat, 17 Dec 2011 23:27:39 +0100 Subject: [PATCH 39/49] DDC-1368 - Improve schema validator --- lib/Doctrine/ORM/Tools/SchemaValidator.php | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/lib/Doctrine/ORM/Tools/SchemaValidator.php b/lib/Doctrine/ORM/Tools/SchemaValidator.php index cb3c9e515..f94234487 100644 --- a/lib/Doctrine/ORM/Tools/SchemaValidator.php +++ b/lib/Doctrine/ORM/Tools/SchemaValidator.php @@ -28,7 +28,6 @@ use Doctrine\ORM\Mapping\ClassMetadataInfo; * @license http://www.opensource.org/licenses/lgpl-license.php LGPL * @link www.doctrine-project.com * @since 1.0 - * @version $Revision$ * @author Benjamin Eberlei * @author Guilherme Blanco * @author Jonathan Wage @@ -139,6 +138,19 @@ class SchemaValidator $assoc['targetEntity'] . "#" . $assoc['inversedBy'] . " are ". "incosistent with each other."; } + + // Verify inverse side/owning side match each other + $targetAssoc = $targetMetadata->associationMappings[$assoc['inversedBy']]; + if ($assoc['type'] == ClassMetadata::ONE_TO_ONE && $targetAssoc['type'] !== ClassMetadata::ONE_TO_ONE){ + $ce[] = "If association " . $class->name . "#" . $fieldName . " is one-to-one, then the inversed " . + "side " . $targetMetadata->name . "#" . $assoc['inversedBy'] . " has to be one-to-one as well."; + } else if ($assoc['type'] == ClassMetadata::MANY_TO_ONE && $targetAssoc['type'] !== ClassMetadata::ONE_TO_MANY){ + $ce[] = "If association " . $class->name . "#" . $fieldName . " is many-to-one, then the inversed " . + "side " . $targetMetadata->name . "#" . $assoc['inversedBy'] . " has to be one-to-many."; + } else if ($assoc['type'] == ClassMetadata::MANY_TO_MANY && $targetAssoc['type'] !== ClassMetadata::MANY_TO_MANY){ + $ce[] = "If association " . $class->name . "#" . $fieldName . " is many-to-many, then the inversed " . + "side " . $targetMetadata->name . "#" . $assoc['inversedBy'] . " has to be many-to-many as well."; + } } if ($assoc['isOwningSide']) { From 072094f72252beda25dc20ab9597427907db2f92 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sat, 17 Dec 2011 23:38:39 +0100 Subject: [PATCH 40/49] DDC-1368 - Fix tests --- lib/Doctrine/ORM/Tools/SchemaValidator.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/Doctrine/ORM/Tools/SchemaValidator.php b/lib/Doctrine/ORM/Tools/SchemaValidator.php index f94234487..93a5013be 100644 --- a/lib/Doctrine/ORM/Tools/SchemaValidator.php +++ b/lib/Doctrine/ORM/Tools/SchemaValidator.php @@ -141,13 +141,13 @@ class SchemaValidator // Verify inverse side/owning side match each other $targetAssoc = $targetMetadata->associationMappings[$assoc['inversedBy']]; - if ($assoc['type'] == ClassMetadata::ONE_TO_ONE && $targetAssoc['type'] !== ClassMetadata::ONE_TO_ONE){ + if ($assoc['type'] == ClassMetadataInfo::ONE_TO_ONE && $targetAssoc['type'] !== ClassMetadataInfo::ONE_TO_ONE){ $ce[] = "If association " . $class->name . "#" . $fieldName . " is one-to-one, then the inversed " . "side " . $targetMetadata->name . "#" . $assoc['inversedBy'] . " has to be one-to-one as well."; - } else if ($assoc['type'] == ClassMetadata::MANY_TO_ONE && $targetAssoc['type'] !== ClassMetadata::ONE_TO_MANY){ + } else if ($assoc['type'] == ClassMetadataInfo::MANY_TO_ONE && $targetAssoc['type'] !== ClassMetadataInfo::ONE_TO_MANY){ $ce[] = "If association " . $class->name . "#" . $fieldName . " is many-to-one, then the inversed " . "side " . $targetMetadata->name . "#" . $assoc['inversedBy'] . " has to be one-to-many."; - } else if ($assoc['type'] == ClassMetadata::MANY_TO_MANY && $targetAssoc['type'] !== ClassMetadata::MANY_TO_MANY){ + } else if ($assoc['type'] == ClassMetadataInfo::MANY_TO_MANY && $targetAssoc['type'] !== ClassMetadataInfo::MANY_TO_MANY){ $ce[] = "If association " . $class->name . "#" . $fieldName . " is many-to-many, then the inversed " . "side " . $targetMetadata->name . "#" . $assoc['inversedBy'] . " has to be many-to-many as well."; } From 9cd8f85a8cef8d97c3d9dfe03d45b580998e0032 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sun, 18 Dec 2011 00:32:35 +0100 Subject: [PATCH 41/49] DDC-1456 - Disallow setting id generators on composite identifiers. --- lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php | 4 ++++ lib/Doctrine/ORM/Mapping/MappingException.php | 5 +++++ tests/Doctrine/Tests/ORM/Tools/SchemaValidatorTest.php | 4 +--- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php b/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php index 82ff14531..8801344ff 100644 --- a/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php +++ b/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php @@ -371,6 +371,10 @@ class ClassMetadataFactory implements ClassMetadataFactoryInterface // second condition is necessary for mapped superclasses in the middle of an inheritance hierachy throw MappingException::noInheritanceOnMappedSuperClass($class->name); } + + if ($class->usesIdGenerator() && $class->isIdentifierComposite) { + throw MappingException::compositeKeyAssignedIdGeneratorRequired($class->name); + } } /** diff --git a/lib/Doctrine/ORM/Mapping/MappingException.php b/lib/Doctrine/ORM/Mapping/MappingException.php index 014714bf8..82b6f138b 100644 --- a/lib/Doctrine/ORM/Mapping/MappingException.php +++ b/lib/Doctrine/ORM/Mapping/MappingException.php @@ -314,4 +314,9 @@ class MappingException extends \Doctrine\ORM\ORMException { return new self("Entity '" . $className . "' has a mapping with invalid fetch mode '" . $annotation . "'"); } + + public static function compositeKeyAssignedIdGeneratorRequired($className) + { + return new self("Entity '". $className . "' has a composite identifier but uses an ID generator other than manually assigning (Identity, Sequence). This is not supported."); + } } diff --git a/tests/Doctrine/Tests/ORM/Tools/SchemaValidatorTest.php b/tests/Doctrine/Tests/ORM/Tools/SchemaValidatorTest.php index 0fc3bb636..1d68a9bcb 100644 --- a/tests/Doctrine/Tests/ORM/Tools/SchemaValidatorTest.php +++ b/tests/Doctrine/Tests/ORM/Tools/SchemaValidatorTest.php @@ -141,13 +141,11 @@ class InvalidEntity2 { /** * @Id @Column - * @GeneratedValue(strategy="AUTO") */ protected $key3; /** * @Id @Column - * @GeneratedValue(strategy="AUTO") */ protected $key4; @@ -155,4 +153,4 @@ class InvalidEntity2 * @ManyToOne(targetEntity="InvalidEntity1") */ protected $assoc; -} \ No newline at end of file +} From 9b877499c778ef01379d8f391ab15f1c19c484ec Mon Sep 17 00:00:00 2001 From: Bernhard Schussek Date: Sun, 18 Dec 2011 12:13:58 +0100 Subject: [PATCH 42/49] Added test case for DDC-1545 --- .../ORM/Functional/Ticket/DDC1545Test.php | 203 ++++++++++++++++++ 1 file changed, 203 insertions(+) create mode 100644 tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1545Test.php diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1545Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1545Test.php new file mode 100644 index 000000000..6c028b5ac --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1545Test.php @@ -0,0 +1,203 @@ +useModelSet('cms'); + parent::setUp(); + } + + private function initDb($link) + { + $article = new CmsArticle(); + $article->topic = 'foo'; + $article->text = 'foo'; + + $user = new CmsUser(); + $user->status = 'foo'; + $user->username = 'foo'; + $user->name = 'foo'; + + $user2 = new CmsUser(); + $user2->status = 'bar'; + $user2->username = 'bar'; + $user2->name = 'bar'; + + if ($link) { + $article->user = $user; + } + + $this->_em->persist($article); + $this->_em->persist($user); + $this->_em->persist($user2); + $this->_em->flush(); + $this->_em->clear(); + + $this->articleId = $article->id; + $this->userId = $user->id; + $this->user2Id = $user2->id; + } + + public function testLinkObjects() + { + $this->initDb(false); + + // don't join association + $article = $this->_em->find('Doctrine\Tests\Models\Cms\CmsArticle', $this->articleId); + + $user = $this->_em->find('Doctrine\Tests\Models\Cms\CmsUser', $this->userId); + + $article->user = $user; + + $this->_em->flush(); + $this->_em->clear(); + + $article = $this->_em + ->createQuery('SELECT a, u FROM Doctrine\Tests\Models\Cms\CmsArticle a LEFT JOIN a.user u WHERE a.id = :id') + ->setParameter('id', $this->articleId) + ->getOneOrNullResult(); + + $this->assertNotNull($article->user); + $this->assertEquals($user->id, $article->user->id); + } + + public function testLinkObjectsWithAssociationLoaded() + { + $this->initDb(false); + + // join association + $article = $this->_em + ->createQuery('SELECT a, u FROM Doctrine\Tests\Models\Cms\CmsArticle a LEFT JOIN a.user u WHERE a.id = :id') + ->setParameter('id', $this->articleId) + ->getOneOrNullResult(); + + $user = $this->_em->find('Doctrine\Tests\Models\Cms\CmsUser', $this->userId); + + $article->user = $user; + + $this->_em->flush(); + $this->_em->clear(); + + $article = $this->_em + ->createQuery('SELECT a, u FROM Doctrine\Tests\Models\Cms\CmsArticle a LEFT JOIN a.user u WHERE a.id = :id') + ->setParameter('id', $this->articleId) + ->getOneOrNullResult(); + + $this->assertNotNull($article->user); + $this->assertEquals($user->id, $article->user->id); + } + + public function testUnlinkObjects() + { + $this->initDb(true); + + // don't join association + $article = $this->_em->find('Doctrine\Tests\Models\Cms\CmsArticle', $this->articleId); + + $article->user = null; + + $this->_em->flush(); + $this->_em->clear(); + + $article = $this->_em + ->createQuery('SELECT a, u FROM Doctrine\Tests\Models\Cms\CmsArticle a LEFT JOIN a.user u WHERE a.id = :id') + ->setParameter('id', $this->articleId) + ->getOneOrNullResult(); + + $this->assertNull($article->user); + } + + public function testUnlinkObjectsWithAssociationLoaded() + { + $this->initDb(true); + + // join association + $article = $this->_em + ->createQuery('SELECT a, u FROM Doctrine\Tests\Models\Cms\CmsArticle a LEFT JOIN a.user u WHERE a.id = :id') + ->setParameter('id', $this->articleId) + ->getOneOrNullResult(); + + $article->user = null; + + $this->_em->flush(); + $this->_em->clear(); + + $article = $this->_em + ->createQuery('SELECT a, u FROM Doctrine\Tests\Models\Cms\CmsArticle a LEFT JOIN a.user u WHERE a.id = :id') + ->setParameter('id', $this->articleId) + ->getOneOrNullResult(); + + $this->assertNull($article->user); + } + + public function testChangeLink() + { + $this->initDb(false); + + // don't join association + $article = $this->_em->find('Doctrine\Tests\Models\Cms\CmsArticle', $this->articleId); + + $user2 = $this->_em->find('Doctrine\Tests\Models\Cms\CmsUser', $this->user2Id); + + $article->user = $user2; + + $this->_em->flush(); + $this->_em->clear(); + + $article = $this->_em + ->createQuery('SELECT a, u FROM Doctrine\Tests\Models\Cms\CmsArticle a LEFT JOIN a.user u WHERE a.id = :id') + ->setParameter('id', $this->articleId) + ->getOneOrNullResult(); + + $this->assertNotNull($article->user); + $this->assertEquals($user2->id, $article->user->id); + } + + public function testChangeLinkWithAssociationLoaded() + { + $this->initDb(false); + + // join association + $article = $this->_em + ->createQuery('SELECT a, u FROM Doctrine\Tests\Models\Cms\CmsArticle a LEFT JOIN a.user u WHERE a.id = :id') + ->setParameter('id', $this->articleId) + ->getOneOrNullResult(); + + $user2 = $this->_em->find('Doctrine\Tests\Models\Cms\CmsUser', $this->user2Id); + + $article->user = $user2; + + $this->_em->flush(); + $this->_em->clear(); + + $article = $this->_em + ->createQuery('SELECT a, u FROM Doctrine\Tests\Models\Cms\CmsArticle a LEFT JOIN a.user u WHERE a.id = :id') + ->setParameter('id', $this->articleId) + ->getOneOrNullResult(); + + $this->assertNotNull($article->user); + $this->assertEquals($user2->id, $article->user->id); + } +} \ No newline at end of file From 5160bdc1a8278d79acd0732f1bceaebec1ea8c0c Mon Sep 17 00:00:00 2001 From: Alexander Date: Sun, 18 Dec 2011 19:58:32 +0100 Subject: [PATCH 43/49] [DDC-551] Fix testcase on pgsql --- tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php b/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php index d56ddb99b..1a20344a9 100644 --- a/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php @@ -594,7 +594,8 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase $conf = $this->_em->getConfiguration(); $conf->addFilter("completed_contract", "\Doctrine\Tests\ORM\Functional\CompletedContractFilter"); $this->_em->getFilters() - ->enable("completed_contract"); + ->enable("completed_contract") + ->setParameter("completed", true, \Doctrine\DBAL\Types\Type::getType(\Doctrine\DBAL\Types\Type::BOOLEAN)->getBindingType()); $this->assertEquals(1, count($this->_em->getRepository('Doctrine\Tests\Models\Company\CompanyFlexUltraContract')->findAll())); $this->assertEquals(1, count($this->_em->createQuery("SELECT cfc FROM Doctrine\Tests\Models\Company\CompanyFlexUltraContract cfc")->getResult())); @@ -610,7 +611,8 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase $conf = $this->_em->getConfiguration(); $conf->addFilter("completed_contract", "\Doctrine\Tests\ORM\Functional\CompletedContractFilter"); $this->_em->getFilters() - ->enable("completed_contract"); + ->enable("completed_contract") + ->setParameter("completed", true, \Doctrine\DBAL\Types\Type::getType(\Doctrine\DBAL\Types\Type::BOOLEAN)->getBindingType()); $this->assertEquals(2, count($this->_em->getRepository('Doctrine\Tests\Models\Company\CompanyFlexContract')->findAll())); $this->assertEquals(2, count($this->_em->createQuery("SELECT cfc FROM Doctrine\Tests\Models\Company\CompanyFlexContract cfc")->getResult())); @@ -715,6 +717,6 @@ class CompletedContractFilter extends SQLFilter return ""; } - return $targetTableAlias.'.completed = 1'; + return $targetTableAlias.'.completed = ' . $this->getParameter('completed'); } } From de769c6c3c0b55d4637cab2deba952cd98f5b0e7 Mon Sep 17 00:00:00 2001 From: Alexander Date: Sun, 18 Dec 2011 21:33:38 +0100 Subject: [PATCH 44/49] [DDC-1505] joinColumn "nullable" should be handled true by default --- .../ORM/Persisters/BasicEntityPersister.php | 2 +- .../Functional/OneToOneEagerLoadingTest.php | 23 ++++++++++++++++--- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php index 6bb3d74da..6845d984a 100644 --- a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php +++ b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php @@ -1563,7 +1563,7 @@ class BasicEntityPersister { // if one of the join columns is nullable, return left join foreach ($joinColumns as $joinColumn) { - if (isset($joinColumn['nullable']) && $joinColumn['nullable']) { + if (!isset($joinColumn['nullable']) || $joinColumn['nullable']) { return 'LEFT JOIN'; } } diff --git a/tests/Doctrine/Tests/ORM/Functional/OneToOneEagerLoadingTest.php b/tests/Doctrine/Tests/ORM/Functional/OneToOneEagerLoadingTest.php index 5fa55036c..8257176ab 100644 --- a/tests/Doctrine/Tests/ORM/Functional/OneToOneEagerLoadingTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/OneToOneEagerLoadingTest.php @@ -143,14 +143,27 @@ class OneToOneEagerLoadingTest extends \Doctrine\Tests\OrmFunctionalTestCase public function testEagerLoadWithNonNullableColumnsGeneratesInnerJoinOnOwningSide() { $waggon = new Waggon(); - $this->_em->persist($waggon); + + // It should have a train + $train = new Train(new TrainOwner("Alexander")); + $train->addWaggon($waggon); + + $this->_em->persist($train); $this->_em->flush(); $this->_em->clear(); $waggon = $this->_em->find(get_class($waggon), $waggon->id); + + // The last query is the eager loading of the owner of the train + $this->assertEquals( + "SELECT t0.id AS id1, t0.name AS name2, t3.id AS id4, t3.driver_id AS driver_id5, t3.owner_id AS owner_id6 FROM TrainOwner t0 LEFT JOIN Train t3 ON t3.owner_id = t0.id WHERE t0.id IN (?)", + $this->_sqlLoggerStack->queries[$this->_sqlLoggerStack->currentQuery]['sql'] + ); + + // The one before is the fetching of the waggon and train $this->assertEquals( "SELECT t0.id AS id1, t0.train_id AS train_id2, t3.id AS id4, t3.driver_id AS driver_id5, t3.owner_id AS owner_id6 FROM Waggon t0 INNER JOIN Train t3 ON t0.train_id = t3.id WHERE t0.id = ?", - $this->_sqlLoggerStack->queries[$this->_sqlLoggerStack->currentQuery]['sql'] + $this->_sqlLoggerStack->queries[$this->_sqlLoggerStack->currentQuery - 1]['sql'] ); } @@ -189,6 +202,7 @@ class Train /** * Owning side * @OneToOne(targetEntity="TrainOwner", inversedBy="train", fetch="EAGER", cascade={"persist"}) + * @JoinColumn(nullable=false) */ public $owner; /** @@ -280,7 +294,10 @@ class Waggon { /** @id @generatedValue @column(type="integer") */ public $id; - /** @ManyToOne(targetEntity="Train", inversedBy="waggons", fetch="EAGER") */ + /** + * @ManyToOne(targetEntity="Train", inversedBy="waggons", fetch="EAGER") + * @JoinColumn(nullable=false) + */ public $train; public function setTrain($train) From a8478d5766e2cc4185612af680b7f6bcd83af61e Mon Sep 17 00:00:00 2001 From: Guilherme Blanco Date: Mon, 19 Dec 2011 01:39:48 -0500 Subject: [PATCH 45/49] Fixed issue with fetched association not being considered during changeSet calculation. Fixes DDC-1545. --- lib/Doctrine/ORM/Query/SqlWalker.php | 6 +---- lib/Doctrine/ORM/UnitOfWork.php | 27 +++++++++++-------- .../Doctrine/Tests/Models/CMS/CmsArticle.php | 2 +- 3 files changed, 18 insertions(+), 17 deletions(-) diff --git a/lib/Doctrine/ORM/Query/SqlWalker.php b/lib/Doctrine/ORM/Query/SqlWalker.php index 0bc437a98..753669f92 100644 --- a/lib/Doctrine/ORM/Query/SqlWalker.php +++ b/lib/Doctrine/ORM/Query/SqlWalker.php @@ -978,17 +978,13 @@ class SqlWalker implements TreeWalker */ public function walkCoalesceExpression($coalesceExpression) { - $sql = 'COALESCE('; - $scalarExpressions = array(); foreach ($coalesceExpression->scalarExpressions as $scalarExpression) { $scalarExpressions[] = $this->walkSimpleArithmeticExpression($scalarExpression); } - $sql .= implode(', ', $scalarExpressions) . ')'; - - return $sql; + return 'COALESCE(' . implode(', ', $scalarExpressions) . ')'; } /** diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index a026d001a..1db6caa0c 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -705,7 +705,6 @@ class UnitOfWork implements PropertyChangedListener foreach ($unwrappedValue as $key => $entry) { $state = $this->getEntityState($entry, self::STATE_NEW); - $oid = spl_object_hash($entry); switch ($state) { case self::STATE_NEW: @@ -2293,13 +2292,14 @@ class UnitOfWork implements PropertyChangedListener $id = array($class->identifier[0] => $idHash); } + $overrideLocalValues = true; + if (isset($this->identityMap[$class->rootEntityName][$idHash])) { $entity = $this->identityMap[$class->rootEntityName][$idHash]; $oid = spl_object_hash($entity); if ($entity instanceof Proxy && ! $entity->__isInitialized__) { $entity->__isInitialized__ = true; - $overrideLocalValues = true; if ($entity instanceof NotifyPropertyChanged) { $entity->addPropertyChangedListener($this); @@ -2308,7 +2308,7 @@ class UnitOfWork implements PropertyChangedListener $overrideLocalValues = isset($hints[Query::HINT_REFRESH]); // If only a specific entity is set to refresh, check that it's the one - if(isset($hints[Query::HINT_REFRESH_ENTITY])) { + if (isset($hints[Query::HINT_REFRESH_ENTITY])) { $overrideLocalValues = $hints[Query::HINT_REFRESH_ENTITY] === $entity; // inject ObjectManager into just loaded proxies. @@ -2333,8 +2333,6 @@ class UnitOfWork implements PropertyChangedListener if ($entity instanceof NotifyPropertyChanged) { $entity->addPropertyChangedListener($this); } - - $overrideLocalValues = true; } if ( ! $overrideLocalValues) { @@ -2362,6 +2360,10 @@ class UnitOfWork implements PropertyChangedListener foreach ($class->associationMappings as $field => $assoc) { // Check if the association is not among the fetch-joined associations already. if (isset($hints['fetchAlias']) && isset($hints['fetched'][$hints['fetchAlias']][$field])) { + // DDC-1545: Fetched associations must have original entity data set. + // Define NULL value right now, since next iteration may fill it with actual value. + $this->originalEntityData[$oid][$field] = null; + continue; } @@ -2382,12 +2384,15 @@ class UnitOfWork implements PropertyChangedListener foreach ($assoc['targetToSourceKeyColumns'] as $targetColumn => $srcColumn) { $joinColumnValue = isset($data[$srcColumn]) ? $data[$srcColumn] : null; - if ($joinColumnValue !== null) { - if ($targetClass->containsForeignIdentifier) { - $associatedId[$targetClass->getFieldForColumn($targetColumn)] = $joinColumnValue; - } else { - $associatedId[$targetClass->fieldNames[$targetColumn]] = $joinColumnValue; - } + // Skip is join column value is null + if ($joinColumnValue === null) { + continue; + } + + if ($targetClass->containsForeignIdentifier) { + $associatedId[$targetClass->getFieldForColumn($targetColumn)] = $joinColumnValue; + } else { + $associatedId[$targetClass->fieldNames[$targetColumn]] = $joinColumnValue; } } diff --git a/tests/Doctrine/Tests/Models/CMS/CmsArticle.php b/tests/Doctrine/Tests/Models/CMS/CmsArticle.php index 5cc516735..266cbc662 100644 --- a/tests/Doctrine/Tests/Models/CMS/CmsArticle.php +++ b/tests/Doctrine/Tests/Models/CMS/CmsArticle.php @@ -36,7 +36,7 @@ class CmsArticle * @Version @column(type="integer") */ public $version; - + public function setAuthor(CmsUser $author) { $this->user = $author; } From bd07f8d3dd303891fa9c662c46fb702a8cac6bf9 Mon Sep 17 00:00:00 2001 From: Alexander Date: Mon, 19 Dec 2011 08:43:42 +0100 Subject: [PATCH 46/49] [DDC-551] Add type inference to SQLFilter::setParameter() + cleaned tests --- lib/Doctrine/ORM/Query/Filter/SQLFilter.php | 15 +++-- .../Tests/ORM/Functional/SQLFilterTest.php | 61 +++++++++++++------ 2 files changed, 55 insertions(+), 21 deletions(-) diff --git a/lib/Doctrine/ORM/Query/Filter/SQLFilter.php b/lib/Doctrine/ORM/Query/Filter/SQLFilter.php index 1250b16d7..a87dd841e 100644 --- a/lib/Doctrine/ORM/Query/Filter/SQLFilter.php +++ b/lib/Doctrine/ORM/Query/Filter/SQLFilter.php @@ -19,8 +19,9 @@ namespace Doctrine\ORM\Query\Filter; -use Doctrine\ORM\Mapping\ClassMetaData; -use Doctrine\ORM\EntityManager; +use Doctrine\ORM\EntityManager, + Doctrine\ORM\Mapping\ClassMetaData, + Doctrine\ORM\Query\ParameterTypeInferer; /** * The base class that user defined filters should extend. @@ -60,12 +61,18 @@ abstract class SQLFilter * * @param string $name Name of the parameter. * @param string $value Value of the parameter. - * @param string $type Type of the parameter. + * @param string $type The parameter type. If specified, the given value will be run through + * the type conversion of this type. This is usually not needed for + * strings and numeric types. * * @return SQLFilter The current SQL filter. */ - final public function setParameter($name, $value, $type) + final public function setParameter($name, $value, $type = null) { + if (null === $type) { + $type = ParameterTypeInferer::inferType($value); + } + $this->parameters[$name] = array('value' => $value, 'type' => $type); // Keep the parameters sorted for the hash diff --git a/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php b/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php index 1a20344a9..ebb2c66cc 100644 --- a/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php @@ -2,6 +2,7 @@ namespace Doctrine\Tests\ORM\Functional; +use Doctrine\DBAL\Types\Type; use Doctrine\ORM\Query\Filter\SQLFilter; use Doctrine\ORM\Mapping\ClassMetaData; use Doctrine\Common\Cache\ArrayCache; @@ -213,7 +214,33 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase $filter = new MyLocaleFilter($em); - $filter->setParameter('locale', 'en', \Doctrine\DBAL\Types\Type::STRING); + $filter->setParameter('locale', 'en', Type::STRING); + + $this->assertEquals("'en'", $filter->getParameter('locale')); + } + + public function testSQLFilterSetParameterInfersType() + { + // Setup mock connection + $conn = $this->getMockConnection(); + $conn->expects($this->once()) + ->method('quote') + ->with($this->equalTo('en')) + ->will($this->returnValue("'en'")); + + $em = $this->getMockEntityManager($conn); + $em->expects($this->once()) + ->method('getConnection') + ->will($this->returnValue($conn)); + + $filterCollection = $this->addMockFilterCollection($em); + $filterCollection + ->expects($this->once()) + ->method('setFiltersStateDirty'); + + $filter = new MyLocaleFilter($em); + + $filter->setParameter('locale', 'en'); $this->assertEquals("'en'", $filter->getParameter('locale')); } @@ -243,16 +270,16 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase $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); + $filter->setParameter('locale', 'en', Type::STRING); + $filter->setParameter('foo', 'bar', Type::STRING); $filter2 = new MyLocaleFilter($em); - $filter2->setParameter('foo', 'bar', \Doctrine\DBAL\Types\Type::STRING); - $filter2->setParameter('locale', 'en', \Doctrine\DBAL\Types\Type::STRING); + $filter2->setParameter('foo', 'bar', Type::STRING); + $filter2->setParameter('locale', 'en', Type::STRING); $parameters = array( - 'foo' => array('value' => 'bar', 'type' => \Doctrine\DBAL\Types\Type::STRING), - 'locale' => array('value' => 'en', 'type' => \Doctrine\DBAL\Types\Type::STRING), + 'foo' => array('value' => 'bar', 'type' => Type::STRING), + 'locale' => array('value' => 'en', 'type' => Type::STRING), ); $this->assertEquals(serialize($parameters), ''.$filter); @@ -292,7 +319,7 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase $conf = $this->_em->getConfiguration(); $conf->addFilter("country", "\Doctrine\Tests\ORM\Functional\CMSCountryFilter"); $this->_em->getFilters()->enable("country") - ->setParameter("country", "en", \Doctrine\DBAL\Types\Type::getType(\Doctrine\DBAL\Types\Type::STRING)->getBindingType()); + ->setParameter("country", "en", Type::STRING); $this->assertNotEquals($firstSQLQuery, $query->getSQL()); } @@ -309,7 +336,7 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase $conf = $this->_em->getConfiguration(); $conf->addFilter("country", "\Doctrine\Tests\ORM\Functional\CMSCountryFilter"); - $this->_em->getFilters()->enable("country")->setParameter("country", "Germany", \Doctrine\DBAL\Types\Type::getType(\Doctrine\DBAL\Types\Type::STRING)->getBindingType()); + $this->_em->getFilters()->enable("country")->setParameter("country", "Germany", Type::STRING); // We get one user after enabling the filter $this->assertEquals(1, count($query->getResult())); @@ -325,7 +352,7 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase $conf = $this->_em->getConfiguration(); $conf->addFilter("group_prefix", "\Doctrine\Tests\ORM\Functional\CMSGroupPrefixFilter"); - $this->_em->getFilters()->enable("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_%", Type::STRING); // We get one user after enabling the filter $this->assertEquals(1, count($query->getResult())); @@ -342,7 +369,7 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase $conf = $this->_em->getConfiguration(); $conf->addFilter("group_prefix", "\Doctrine\Tests\ORM\Functional\CMSGroupPrefixFilter"); - $this->_em->getFilters()->enable("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_%", Type::STRING); // We get one user after enabling the filter $this->assertEquals(1, count($query->getResult())); @@ -361,7 +388,7 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase { $conf = $this->_em->getConfiguration(); $conf->addFilter("article_topic", "\Doctrine\Tests\ORM\Functional\CMSArticleTopicFilter"); - $this->_em->getFilters()->enable("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", Type::STRING); } public function testOneToMany_ExtraLazyCountWithFilter() @@ -408,7 +435,7 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase { $conf = $this->_em->getConfiguration(); $conf->addFilter("group_prefix", "\Doctrine\Tests\ORM\Functional\CMSGroupPrefixFilter"); - $this->_em->getFilters()->enable("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%", Type::STRING); } public function testManyToMany_ExtraLazyCountWithFilter() @@ -529,7 +556,7 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase $conf->addFilter("person_name", "\Doctrine\Tests\ORM\Functional\CompanyPersonNameFilter"); $this->_em->getFilters() ->enable("person_name") - ->setParameter("name", "Guilh%", \Doctrine\DBAL\Types\Type::getType(\Doctrine\DBAL\Types\Type::STRING)->getBindingType()); + ->setParameter("name", "Guilh%", Type::STRING); $managers = $this->_em->getRepository('Doctrine\Tests\Models\Company\CompanyManager')->findAll(); $this->assertEquals(1, count($managers)); @@ -549,7 +576,7 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase $conf->addFilter("person_name", "\Doctrine\Tests\ORM\Functional\CompanyPersonNameFilter"); $this->_em->getFilters() ->enable("person_name") - ->setParameter("name", "Guilh%", \Doctrine\DBAL\Types\Type::getType(\Doctrine\DBAL\Types\Type::STRING)->getBindingType()); + ->setParameter("name", "Guilh%", Type::STRING); $persons = $this->_em->getRepository('Doctrine\Tests\Models\Company\CompanyPerson')->findAll(); $this->assertEquals(1, count($persons)); @@ -595,7 +622,7 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase $conf->addFilter("completed_contract", "\Doctrine\Tests\ORM\Functional\CompletedContractFilter"); $this->_em->getFilters() ->enable("completed_contract") - ->setParameter("completed", true, \Doctrine\DBAL\Types\Type::getType(\Doctrine\DBAL\Types\Type::BOOLEAN)->getBindingType()); + ->setParameter("completed", true, Type::BOOLEAN); $this->assertEquals(1, count($this->_em->getRepository('Doctrine\Tests\Models\Company\CompanyFlexUltraContract')->findAll())); $this->assertEquals(1, count($this->_em->createQuery("SELECT cfc FROM Doctrine\Tests\Models\Company\CompanyFlexUltraContract cfc")->getResult())); @@ -612,7 +639,7 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase $conf->addFilter("completed_contract", "\Doctrine\Tests\ORM\Functional\CompletedContractFilter"); $this->_em->getFilters() ->enable("completed_contract") - ->setParameter("completed", true, \Doctrine\DBAL\Types\Type::getType(\Doctrine\DBAL\Types\Type::BOOLEAN)->getBindingType()); + ->setParameter("completed", true, Type::BOOLEAN); $this->assertEquals(2, count($this->_em->getRepository('Doctrine\Tests\Models\Company\CompanyFlexContract')->findAll())); $this->assertEquals(2, count($this->_em->createQuery("SELECT cfc FROM Doctrine\Tests\Models\Company\CompanyFlexContract cfc")->getResult())); From 8c6c49a6ee33e4d6932149df753e8baea11eb634 Mon Sep 17 00:00:00 2001 From: Alexander Date: Mon, 19 Dec 2011 09:55:49 +0100 Subject: [PATCH 47/49] Fixed testsuite --- .../Tests/ORM/Functional/SQLFilterTest.php | 36 +++++++++---------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php b/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php index ebb2c66cc..98b3fafd4 100644 --- a/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php @@ -2,7 +2,7 @@ namespace Doctrine\Tests\ORM\Functional; -use Doctrine\DBAL\Types\Type; +use Doctrine\DBAL\Types\Type as DBALType; use Doctrine\ORM\Query\Filter\SQLFilter; use Doctrine\ORM\Mapping\ClassMetaData; use Doctrine\Common\Cache\ArrayCache; @@ -214,7 +214,7 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase $filter = new MyLocaleFilter($em); - $filter->setParameter('locale', 'en', Type::STRING); + $filter->setParameter('locale', 'en', DBALType::STRING); $this->assertEquals("'en'", $filter->getParameter('locale')); } @@ -270,16 +270,16 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase $filterCollection = $this->addMockFilterCollection($em); $filter = new MyLocaleFilter($em); - $filter->setParameter('locale', 'en', Type::STRING); - $filter->setParameter('foo', 'bar', Type::STRING); + $filter->setParameter('locale', 'en', DBALType::STRING); + $filter->setParameter('foo', 'bar', DBALType::STRING); $filter2 = new MyLocaleFilter($em); - $filter2->setParameter('foo', 'bar', Type::STRING); - $filter2->setParameter('locale', 'en', Type::STRING); + $filter2->setParameter('foo', 'bar', DBALType::STRING); + $filter2->setParameter('locale', 'en', DBALType::STRING); $parameters = array( - 'foo' => array('value' => 'bar', 'type' => Type::STRING), - 'locale' => array('value' => 'en', 'type' => Type::STRING), + 'foo' => array('value' => 'bar', 'type' => DBALType::STRING), + 'locale' => array('value' => 'en', 'type' => DBALType::STRING), ); $this->assertEquals(serialize($parameters), ''.$filter); @@ -319,7 +319,7 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase $conf = $this->_em->getConfiguration(); $conf->addFilter("country", "\Doctrine\Tests\ORM\Functional\CMSCountryFilter"); $this->_em->getFilters()->enable("country") - ->setParameter("country", "en", Type::STRING); + ->setParameter("country", "en", DBALType::STRING); $this->assertNotEquals($firstSQLQuery, $query->getSQL()); } @@ -336,7 +336,7 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase $conf = $this->_em->getConfiguration(); $conf->addFilter("country", "\Doctrine\Tests\ORM\Functional\CMSCountryFilter"); - $this->_em->getFilters()->enable("country")->setParameter("country", "Germany", Type::STRING); + $this->_em->getFilters()->enable("country")->setParameter("country", "Germany", DBALType::STRING); // We get one user after enabling the filter $this->assertEquals(1, count($query->getResult())); @@ -352,7 +352,7 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase $conf = $this->_em->getConfiguration(); $conf->addFilter("group_prefix", "\Doctrine\Tests\ORM\Functional\CMSGroupPrefixFilter"); - $this->_em->getFilters()->enable("group_prefix")->setParameter("prefix", "bar_%", Type::STRING); + $this->_em->getFilters()->enable("group_prefix")->setParameter("prefix", "bar_%", DBALType::STRING); // We get one user after enabling the filter $this->assertEquals(1, count($query->getResult())); @@ -369,7 +369,7 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase $conf = $this->_em->getConfiguration(); $conf->addFilter("group_prefix", "\Doctrine\Tests\ORM\Functional\CMSGroupPrefixFilter"); - $this->_em->getFilters()->enable("group_prefix")->setParameter("prefix", "bar_%", Type::STRING); + $this->_em->getFilters()->enable("group_prefix")->setParameter("prefix", "bar_%", DBALType::STRING); // We get one user after enabling the filter $this->assertEquals(1, count($query->getResult())); @@ -388,7 +388,7 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase { $conf = $this->_em->getConfiguration(); $conf->addFilter("article_topic", "\Doctrine\Tests\ORM\Functional\CMSArticleTopicFilter"); - $this->_em->getFilters()->enable("article_topic")->setParameter("topic", "Test1", Type::STRING); + $this->_em->getFilters()->enable("article_topic")->setParameter("topic", "Test1", DBALType::STRING); } public function testOneToMany_ExtraLazyCountWithFilter() @@ -435,7 +435,7 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase { $conf = $this->_em->getConfiguration(); $conf->addFilter("group_prefix", "\Doctrine\Tests\ORM\Functional\CMSGroupPrefixFilter"); - $this->_em->getFilters()->enable("group_prefix")->setParameter("prefix", "foo%", Type::STRING); + $this->_em->getFilters()->enable("group_prefix")->setParameter("prefix", "foo%", DBALType::STRING); } public function testManyToMany_ExtraLazyCountWithFilter() @@ -556,7 +556,7 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase $conf->addFilter("person_name", "\Doctrine\Tests\ORM\Functional\CompanyPersonNameFilter"); $this->_em->getFilters() ->enable("person_name") - ->setParameter("name", "Guilh%", Type::STRING); + ->setParameter("name", "Guilh%", DBALType::STRING); $managers = $this->_em->getRepository('Doctrine\Tests\Models\Company\CompanyManager')->findAll(); $this->assertEquals(1, count($managers)); @@ -576,7 +576,7 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase $conf->addFilter("person_name", "\Doctrine\Tests\ORM\Functional\CompanyPersonNameFilter"); $this->_em->getFilters() ->enable("person_name") - ->setParameter("name", "Guilh%", Type::STRING); + ->setParameter("name", "Guilh%", DBALType::STRING); $persons = $this->_em->getRepository('Doctrine\Tests\Models\Company\CompanyPerson')->findAll(); $this->assertEquals(1, count($persons)); @@ -622,7 +622,7 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase $conf->addFilter("completed_contract", "\Doctrine\Tests\ORM\Functional\CompletedContractFilter"); $this->_em->getFilters() ->enable("completed_contract") - ->setParameter("completed", true, Type::BOOLEAN); + ->setParameter("completed", true, DBALType::BOOLEAN); $this->assertEquals(1, count($this->_em->getRepository('Doctrine\Tests\Models\Company\CompanyFlexUltraContract')->findAll())); $this->assertEquals(1, count($this->_em->createQuery("SELECT cfc FROM Doctrine\Tests\Models\Company\CompanyFlexUltraContract cfc")->getResult())); @@ -639,7 +639,7 @@ class SQLFilterTest extends \Doctrine\Tests\OrmFunctionalTestCase $conf->addFilter("completed_contract", "\Doctrine\Tests\ORM\Functional\CompletedContractFilter"); $this->_em->getFilters() ->enable("completed_contract") - ->setParameter("completed", true, Type::BOOLEAN); + ->setParameter("completed", true, DBALType::BOOLEAN); $this->assertEquals(2, count($this->_em->getRepository('Doctrine\Tests\Models\Company\CompanyFlexContract')->findAll())); $this->assertEquals(2, count($this->_em->createQuery("SELECT cfc FROM Doctrine\Tests\Models\Company\CompanyFlexContract cfc")->getResult())); From 40800bd3cdbb0e932ae71bf050d158e965d8d808 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Mon, 19 Dec 2011 10:10:56 +0100 Subject: [PATCH 48/49] DDC-1530 - Validate field types in SchemaValidator --- lib/Doctrine/ORM/Tools/SchemaValidator.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/lib/Doctrine/ORM/Tools/SchemaValidator.php b/lib/Doctrine/ORM/Tools/SchemaValidator.php index 93a5013be..f6bcadb72 100644 --- a/lib/Doctrine/ORM/Tools/SchemaValidator.php +++ b/lib/Doctrine/ORM/Tools/SchemaValidator.php @@ -21,6 +21,7 @@ namespace Doctrine\ORM\Tools; use Doctrine\ORM\EntityManager; use Doctrine\ORM\Mapping\ClassMetadataInfo; +use Doctrine\DBAL\Types\Type; /** * Performs strict validation of the mapping schema @@ -87,6 +88,12 @@ class SchemaValidator $ce = array(); $cmf = $this->em->getMetadataFactory(); + foreach ($class->fieldMappings as $fieldName => $mapping) { + if (!Type::hasType($mapping['type'])) { + $ce[] = "The field '" . $class->name . "#" . $fieldName."' uses a non-existant type '" . $mapping['type'] . "'."; + } + } + foreach ($class->associationMappings AS $fieldName => $assoc) { if (!$cmf->hasMetadataFor($assoc['targetEntity'])) { $ce[] = "The target entity '" . $assoc['targetEntity'] . "' specified on " . $class->name . '#' . $fieldName . ' is unknown.'; From e035fe7949851a2162794467f7d236368f6d5ee7 Mon Sep 17 00:00:00 2001 From: Bernhard Schussek Date: Mon, 19 Dec 2011 11:20:37 +0100 Subject: [PATCH 49/49] Fixed class name of test for DDC-1545 --- tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1545Test.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1545Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1545Test.php index 6c028b5ac..e7b55eb1f 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1545Test.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1545Test.php @@ -13,9 +13,9 @@ use Doctrine\Tests\Models\CMS\CmsUser; require_once __DIR__ . '/../../../TestInit.php'; /** - * @group DDC-1040 + * @group DDC-1545 */ -class ATest extends \Doctrine\Tests\OrmFunctionalTestCase +class DDC1545Test extends \Doctrine\Tests\OrmFunctionalTestCase { private $articleId;