From 31b0705ed5454bb369859e0a6267a440f48ddbe0 Mon Sep 17 00:00:00 2001 From: beberlei Date: Mon, 15 Feb 2010 22:50:35 +0000 Subject: [PATCH] [2.0] DDC-336 DDC-337 - Changed @OrderBy annotation to take a DQL not SQL snippet, restrict it to field names and positional orderings. Changed all test-cases and added a test-case that shows the behaviour for OneToMany JoinedSubclassPersister Managed Entities. --- .../ORM/Mapping/ManyToManyMapping.php | 19 ++- lib/Doctrine/ORM/Mapping/OneToManyMapping.php | 13 +- .../Persisters/JoinedSubclassPersister.php | 13 +- .../Persisters/StandardEntityPersister.php | 54 ++++++-- .../Tests/ORM/Functional/AllTests.php | 2 + .../ORM/Functional/OrderedCollectionTest.php | 2 +- ...edJoinedTableInheritanceCollectionTest.php | 121 ++++++++++++++++++ .../ORM/Mapping/AbstractMappingDriverTest.php | 4 +- .../Doctrine.Tests.ORM.Mapping.User.dcm.xml | 2 +- .../Doctrine.Tests.ORM.Mapping.User.dcm.yml | 2 +- 10 files changed, 211 insertions(+), 21 deletions(-) create mode 100644 tests/Doctrine/Tests/ORM/Functional/OrderedJoinedTableInheritanceCollectionTest.php diff --git a/lib/Doctrine/ORM/Mapping/ManyToManyMapping.php b/lib/Doctrine/ORM/Mapping/ManyToManyMapping.php index 0fbb4df8a..8ebb1c0b9 100644 --- a/lib/Doctrine/ORM/Mapping/ManyToManyMapping.php +++ b/lib/Doctrine/ORM/Mapping/ManyToManyMapping.php @@ -61,7 +61,11 @@ class ManyToManyMapping extends AssociationMapping //public $keyColumn; /** - * Order this collection by the given SQL snippet. + * Order this collection by the given DQL snippet. + * + * Only simple unqualified field names and ASC|DESC are allowed + * + * @var array */ public $orderBy = null; @@ -142,7 +146,18 @@ class ManyToManyMapping extends AssociationMapping } if (isset($mapping['orderBy'])) { - $this->orderBy = $mapping['orderBy']; + $parts = explode(",", $mapping['orderBy']); + $orderByGroup = array(); + foreach ($parts AS $part) { + $orderByItem = explode(" ", trim($part)); + if (count($orderByItem) == 1) { + $orderByGroup[$orderByItem[0]] = "ASC"; + } else { + $orderByGroup[$orderByItem[0]] = array_pop($orderByItem); + } + } + + $this->orderBy = $orderByGroup; } } diff --git a/lib/Doctrine/ORM/Mapping/OneToManyMapping.php b/lib/Doctrine/ORM/Mapping/OneToManyMapping.php index 0dabd0ec0..0de4dcdc7 100644 --- a/lib/Doctrine/ORM/Mapping/OneToManyMapping.php +++ b/lib/Doctrine/ORM/Mapping/OneToManyMapping.php @@ -86,7 +86,18 @@ class OneToManyMapping extends AssociationMapping (bool) $mapping['orphanRemoval'] : false; if (isset($mapping['orderBy'])) { - $this->orderBy = $mapping['orderBy']; + $parts = explode(",", $mapping['orderBy']); + $orderByGroup = array(); + foreach ($parts AS $part) { + $orderByItem = explode(" ", trim($part)); + if (count($orderByItem) == 1) { + $orderByGroup[$orderByItem[0]] = "ASC"; + } else { + $orderByGroup[$orderByItem[0]] = array_pop($orderByItem); + } + } + + $this->orderBy = $orderByGroup; } } diff --git a/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php b/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php index 7242b26b3..0879785cb 100644 --- a/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php +++ b/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php @@ -294,10 +294,12 @@ class JoinedSubclassPersister extends StandardEntityPersister * Gets the SELECT SQL to select one or more entities by a set of field criteria. * * @param array $criteria - * @return string The SQL. + * @param AssociationMapping $assoc + * @param string $orderBy + * @return string * @override */ - protected function _getSelectEntitiesSql(array &$criteria, $assoc = null) + protected function _getSelectEntitiesSql(array &$criteria, $assoc = null, $orderBy = null) { $tableAliases = array(); $aliasIndex = 1; @@ -419,10 +421,15 @@ class JoinedSubclassPersister extends StandardEntityPersister $conditionSql .= ' = ?'; } + $orderBySql = ''; + if ($orderBy !== null) { + $orderBySql = $this->_getCollectionOrderBySql($orderBy, $baseTableAlias, $tableAliases); + } + return 'SELECT ' . $columnList . ' FROM ' . $this->_class->getQuotedTableName($this->_platform) . ' ' . $baseTableAlias . $joinSql - . ($conditionSql != '' ? ' WHERE ' . $conditionSql : ''); + . ($conditionSql != '' ? ' WHERE ' . $conditionSql : '') . $orderBySql; } /** @override */ diff --git a/lib/Doctrine/ORM/Persisters/StandardEntityPersister.php b/lib/Doctrine/ORM/Persisters/StandardEntityPersister.php index 7e0e4a845..50c7b5760 100644 --- a/lib/Doctrine/ORM/Persisters/StandardEntityPersister.php +++ b/lib/Doctrine/ORM/Persisters/StandardEntityPersister.php @@ -548,11 +548,7 @@ class StandardEntityPersister { $owningAssoc = $this->_class->associationMappings[$coll->getMapping()->mappedByFieldName]; - $sql = $this->_getSelectEntitiesSql($criteria, $owningAssoc); - - if ($assoc->orderBy !== null) { - $sql .= ' ORDER BY '.str_replace('%alias%', $this->_class->getTableName(), $assoc->orderBy); - } + $sql = $this->_getSelectEntitiesSql($criteria, $owningAssoc, $assoc->orderBy); $params = array_values($criteria); @@ -653,9 +649,11 @@ class StandardEntityPersister * Gets the SELECT SQL to select one or more entities by a set of field criteria. * * @param array $criteria - * @return string The SQL. + * @param AssociationMapping $assoc + * @param string $orderBy + * @return string */ - protected function _getSelectEntitiesSql(array &$criteria, $assoc = null) + protected function _getSelectEntitiesSql(array &$criteria, $assoc = null, $orderBy = null) { // Construct WHERE conditions $conditionSql = ''; @@ -676,9 +674,43 @@ class StandardEntityPersister $conditionSql .= ' = ?'; } + $orderBySql = ''; + if ($orderBy !== null) { + $orderBySql = $this->_getCollectionOrderBySql( + $orderBy, $this->_class->getQuotedTableName($this->_platform) + ); + } + return 'SELECT ' . $this->_getSelectColumnList() . ' FROM ' . $this->_class->getQuotedTableName($this->_platform) - . ($conditionSql ? ' WHERE ' . $conditionSql : ''); + . ($conditionSql ? ' WHERE ' . $conditionSql : '') . $orderBySql; + } + + /** + * Generate ORDER BY Sql Snippet for ordered collections + * + * @param array $orderBy + * @return string + */ + protected function _getCollectionOrderBySql(array $orderBy, $baseTableAlias, $tableAliases = array()) + { + $orderBySql = ''; + foreach ($orderBy AS $fieldName => $orientation) { + if (!isset($this->_class->fieldMappings[$fieldName])) { + ORMException::unrecognizedField($fieldName); + } + + $tableAlias = isset($this->_class->fieldMappings['inherited']) ? + $tableAliases[$this->_class->fieldMappings['inherited']] : $baseTableAlias; + $columnName = $this->_class->getQuotedColumnName($fieldName, $this->_platform); + if ($orderBySql != '') { + $orderBySql .= ', '; + } else { + $orderBySql = ' ORDER BY '; + } + $orderBySql .= $tableAlias . '.' . $columnName . ' '.$orientation; + } + return $orderBySql; } /** @@ -724,7 +756,7 @@ class StandardEntityPersister /** * Gets the SQL to select a collection of entities in a many-many association. * - * @param ManyToManyMapping $assoc + * @param ManyToManyMapping $manyToMany * @param array $criteria * @return string */ @@ -761,7 +793,9 @@ class StandardEntityPersister $orderBySql = ''; if ($manyToMany->orderBy !== null) { - $orderBySql = ' ORDER BY '.str_replace('%alias%', $this->_class->getTableName(), $manyToMany->orderBy); + $orderBySql = $this->_getCollectionOrderBySql( + $manyToMany->orderBy, $this->_class->getQuotedTableName($this->_platform) + ); } return 'SELECT ' . $this->_getSelectColumnList() diff --git a/tests/Doctrine/Tests/ORM/Functional/AllTests.php b/tests/Doctrine/Tests/ORM/Functional/AllTests.php index e3c921778..81b67c1f2 100644 --- a/tests/Doctrine/Tests/ORM/Functional/AllTests.php +++ b/tests/Doctrine/Tests/ORM/Functional/AllTests.php @@ -41,6 +41,8 @@ class AllTests $suite->addTestSuite('Doctrine\Tests\ORM\Functional\OneToOneSelfReferentialAssociationTest'); $suite->addTestSuite('Doctrine\Tests\ORM\Functional\OneToManySelfReferentialAssociationTest'); $suite->addTestSuite('Doctrine\Tests\ORM\Functional\ManyToManySelfReferentialAssociationTest'); + $suite->addTestSuite('Doctrine\Tests\ORM\Functional\OrderedCollectionTest'); + $suite->addTestSuite('Doctrine\Tests\ORM\Functional\OrderedJoinedTableInheritanceCollectionTest'); $suite->addTestSuite('Doctrine\Tests\ORM\Functional\ReferenceProxyTest'); $suite->addTestSuite('Doctrine\Tests\ORM\Functional\LifecycleCallbackTest'); $suite->addTestSuite('Doctrine\Tests\ORM\Functional\StandardEntityPersisterTest'); diff --git a/tests/Doctrine/Tests/ORM/Functional/OrderedCollectionTest.php b/tests/Doctrine/Tests/ORM/Functional/OrderedCollectionTest.php index d67e1f1ec..f44fe4798 100644 --- a/tests/Doctrine/Tests/ORM/Functional/OrderedCollectionTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/OrderedCollectionTest.php @@ -9,7 +9,7 @@ use Doctrine\Tests\Models\Routing\RoutingRouteBooking; require_once __DIR__ . '/../../TestInit.php'; -class OrderedAssociationTest extends \Doctrine\Tests\OrmFunctionalTestCase +class OrderedCollectionTest extends \Doctrine\Tests\OrmFunctionalTestCase { protected $locations = array(); diff --git a/tests/Doctrine/Tests/ORM/Functional/OrderedJoinedTableInheritanceCollectionTest.php b/tests/Doctrine/Tests/ORM/Functional/OrderedJoinedTableInheritanceCollectionTest.php new file mode 100644 index 000000000..4ede6056a --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Functional/OrderedJoinedTableInheritanceCollectionTest.php @@ -0,0 +1,121 @@ + + */ +class OrderedJoinedTableInheritanceCollectionTest extends \Doctrine\Tests\OrmFunctionalTestCase +{ + protected function setUp() { + parent::setUp(); + try { + $this->_schemaTool->createSchema(array( + $this->_em->getClassMetadata('Doctrine\Tests\ORM\Functional\OJTIC_Pet'), + $this->_em->getClassMetadata('Doctrine\Tests\ORM\Functional\OJTIC_Cat'), + $this->_em->getClassMetadata('Doctrine\Tests\ORM\Functional\OJTIC_Dog'), + )); + } catch (\Exception $e) { + // Swallow all exceptions. We do not test the schema tool here. + } + } + + public function testOrderdOneToManyCollection() + { + $dog = new OJTIC_Dog(); + $dog->name = "Poofy"; + + $dog1 = new OJTIC_Dog(); + $dog1->name = "Zampa"; + $dog2 = new OJTIC_Dog(); + $dog2->name = "Aari"; + + $dog1->mother = $dog; + $dog2->mother = $dog; + + $dog->children[] = $dog1; + $dog->children[] = $dog2; + + $this->_em->persist($dog); + $this->_em->persist($dog1); + $this->_em->persist($dog2); + $this->_em->flush(); + $this->_em->clear(); + + $poofy = $this->_em->createQuery("SELECT p FROM Doctrine\Tests\ORM\Functional\OJTIC_Pet p WHERE p.name = 'Poofy'")->getSingleResult(); + + $this->assertEquals('Aari', $poofy->children[0]->getName()); + $this->assertEquals('Zampa', $poofy->children[1]->getName()); + } +} + +/** + * @Entity + * @InheritanceType("JOINED") + * @DiscriminatorColumn(name="discr", type="string") + * @DiscriminatorMap({ + * "cat" = "OJTIC_Cat", + * "dog" = "OJTIC_Dog"}) + */ +abstract class OJTIC_Pet +{ + /** + * @Id + * @column(type="integer") + * @generatedValue(strategy="AUTO") + */ + public $id; + + /** + * + * @Column + */ + public $name; + + /** + * @ManyToOne(targetEntity="OJTIC_PET") + */ + public $mother; + + /** + * @OneToMany(targetEntity="OJTIC_Pet", mappedBy="mother") + * @OrderBy("name ASC") + */ + public $children; + + /** + * @ManyToMany(targetEntity="OJTIC_Pet") + * @JoinTable(name="OTJIC_Pet_Friends", + * joinColumns={@JoinColumn(name="pet_id", referencedColumnName="id")}, + * inverseJoinColumns={@JoinColumn(name="friend_id", referencedColumnName="id")}) + * @OrderBy("name ASC") + */ + public $friends; + + public function getName() + { + return $this->name; + } +} + +/** + * @Entity + */ +class OJTIC_Cat extends OJTIC_Pet +{ + +} + +/** + * @Entity + */ +class OJTIC_Dog extends OJTIC_Pet +{ + +} \ No newline at end of file diff --git a/tests/Doctrine/Tests/ORM/Mapping/AbstractMappingDriverTest.php b/tests/Doctrine/Tests/ORM/Mapping/AbstractMappingDriverTest.php index b7c9b108f..a41c4a22a 100644 --- a/tests/Doctrine/Tests/ORM/Mapping/AbstractMappingDriverTest.php +++ b/tests/Doctrine/Tests/ORM/Mapping/AbstractMappingDriverTest.php @@ -114,7 +114,7 @@ abstract class AbstractMappingDriverTest extends \Doctrine\Tests\OrmTestCase $this->assertFalse($class->associationMappings['phonenumbers']->isCascadeMerge); // Test Order By - $this->assertEquals('%alias%.number ASC', $class->associationMappings['phonenumbers']->orderBy); + $this->assertEquals(array('number' => 'ASC'), $class->associationMappings['phonenumbers']->orderBy); return $class; } @@ -207,7 +207,7 @@ class User /** * * @OneToMany(targetEntity="Phonenumber", mappedBy="user", cascade={"persist"}) - * @OrderBy("%alias%.number ASC") + * @OrderBy("number ASC") */ public $phonenumbers; diff --git a/tests/Doctrine/Tests/ORM/Mapping/xml/Doctrine.Tests.ORM.Mapping.User.dcm.xml b/tests/Doctrine/Tests/ORM/Mapping/xml/Doctrine.Tests.ORM.Mapping.User.dcm.xml index 1a344790e..c28baa48e 100644 --- a/tests/Doctrine/Tests/ORM/Mapping/xml/Doctrine.Tests.ORM.Mapping.User.dcm.xml +++ b/tests/Doctrine/Tests/ORM/Mapping/xml/Doctrine.Tests.ORM.Mapping.User.dcm.xml @@ -24,7 +24,7 @@ - + diff --git a/tests/Doctrine/Tests/ORM/Mapping/yaml/Doctrine.Tests.ORM.Mapping.User.dcm.yml b/tests/Doctrine/Tests/ORM/Mapping/yaml/Doctrine.Tests.ORM.Mapping.User.dcm.yml index 450dfd7e5..2e9ed0302 100644 --- a/tests/Doctrine/Tests/ORM/Mapping/yaml/Doctrine.Tests.ORM.Mapping.User.dcm.yml +++ b/tests/Doctrine/Tests/ORM/Mapping/yaml/Doctrine.Tests.ORM.Mapping.User.dcm.yml @@ -27,7 +27,7 @@ Doctrine\Tests\ORM\Mapping\User: phonenumbers: targetEntity: Phonenumber mappedBy: user - orderBy: %alias%.number ASC + orderBy: number ASC cascade: [ persist ] manyToMany: groups: