From 3b7d16c60f432fa94131171852ef2154e079cac6 Mon Sep 17 00:00:00 2001 From: Alexander Date: Mon, 5 Dec 2011 21:14:31 +0100 Subject: [PATCH] [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)