From 2d89ddfb1f7d6c3b369995a2c58d5d1b880217ed Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Tue, 28 Dec 2010 14:56:13 +0100 Subject: [PATCH] DDC-837 - Fix bug with associations of the same name not being possible in inheritance hierachies. --- .../AbstractEntityInheritancePersister.php | 33 ++- .../Persisters/JoinedSubclassPersister.php | 20 +- .../ORM/Persisters/SingleTablePersister.php | 10 +- .../ORM/Functional/Ticket/DDC837Test.php | 198 ++++++++++++++++++ 4 files changed, 238 insertions(+), 23 deletions(-) create mode 100644 tests/Doctrine/Tests/ORM/Functional/Ticket/DDC837Test.php diff --git a/lib/Doctrine/ORM/Persisters/AbstractEntityInheritancePersister.php b/lib/Doctrine/ORM/Persisters/AbstractEntityInheritancePersister.php index 9be6de464..bfe1e60d9 100644 --- a/lib/Doctrine/ORM/Persisters/AbstractEntityInheritancePersister.php +++ b/lib/Doctrine/ORM/Persisters/AbstractEntityInheritancePersister.php @@ -33,11 +33,18 @@ use Doctrine\ORM\Mapping\ClassMetadata, abstract class AbstractEntityInheritancePersister extends BasicEntityPersister { /** - * Map from column names to class names that declare the field the column is mapped to. + * Map from column names to class metadata instances that declare the field the column is mapped to. * * @var array */ - private $_declaringClassMap = array(); + private $declaringClassMap = array(); + + /** + * Map from column names to class names that declare the field the association with join column is mapped to. + * + * @var array + */ + private $declaringJoinColumnMap = array(); /** * {@inheritdoc} @@ -70,8 +77,8 @@ abstract class AbstractEntityInheritancePersister extends BasicEntityPersister unset($sqlResult[$discrColumnName]); foreach ($sqlResult as $column => $value) { $realColumnName = $this->_resultColumnNames[$column]; - if (isset($this->_declaringClassMap[$column])) { - $class = $this->_declaringClassMap[$column]; + if (isset($this->declaringClassMap[$column])) { + $class = $this->declaringClassMap[$column]; if ($class->name == $entityName || is_subclass_of($entityName, $class->name)) { $field = $class->fieldNames[$realColumnName]; if (isset($data[$field])) { @@ -81,6 +88,10 @@ abstract class AbstractEntityInheritancePersister extends BasicEntityPersister ->convertToPHPValue($value, $this->_platform); } } + } else if (isset($this->declaringJoinColumnMap[$column])) { + if ($this->declaringJoinColumnMap[$column] == $entityName || is_subclass_of($entityName, $this->declaringJoinColumnMap[$column])) { + $data[$realColumnName] = $value; + } } else { $data[$realColumnName] = $value; } @@ -99,9 +110,21 @@ abstract class AbstractEntityInheritancePersister extends BasicEntityPersister $columnAlias = $this->_platform->getSQLResultCasing($columnName . $this->_sqlAliasCounter++); if ( ! isset($this->_resultColumnNames[$columnAlias])) { $this->_resultColumnNames[$columnAlias] = $columnName; - $this->_declaringClassMap[$columnAlias] = $class; + $this->declaringClassMap[$columnAlias] = $class; } return "$sql AS $columnAlias"; } + + protected function getSelectJoinColumnSQL($tableAlias, $joinColumnName, $className) + { + $columnAlias = $joinColumnName . $this->_sqlAliasCounter++; + $resultColumnName = $this->_platform->getSQLResultCasing($columnAlias); + if ( ! isset($this->_resultColumnNames[$resultColumnName])) { + $this->_resultColumnNames[$resultColumnName] = $joinColumnName; + $this->declaringJoinColumnMap[$resultColumnName] = $className; + } + + return $tableAlias . ".$joinColumnName AS $columnAlias"; + } } \ No newline at end of file diff --git a/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php b/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php index e9750f8c3..d75656b06 100644 --- a/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php +++ b/lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php @@ -263,12 +263,10 @@ class JoinedSubclassPersister extends AbstractEntityInheritancePersister $this->_getSQLTableAlias($assoc2['inherited']) : $baseTableAlias; foreach ($assoc2['targetToSourceKeyColumns'] as $srcColumn) { - $columnAlias = $srcColumn . $this->_sqlAliasCounter++; - $columnList .= ", $tableAlias.$srcColumn AS $columnAlias"; - $resultColumnName = $this->_platform->getSQLResultCasing($columnAlias); - if ( ! isset($this->_resultColumnNames[$resultColumnName])) { - $this->_resultColumnNames[$resultColumnName] = $srcColumn; - } + if ($columnList != '') $columnList .= ', '; + $columnList .= $this->getSelectJoinColumnSQL($tableAlias, $srcColumn, + isset($assoc2['inherited']) ? $assoc2['inherited'] : $this->_class->name + ); } } } @@ -318,12 +316,10 @@ class JoinedSubclassPersister extends AbstractEntityInheritancePersister if ($assoc2['isOwningSide'] && $assoc2['type'] & ClassMetadata::TO_ONE && ! isset($assoc2['inherited'])) { foreach ($assoc2['targetToSourceKeyColumns'] as $srcColumn) { - $columnAlias = $srcColumn . $this->_sqlAliasCounter++; - $columnList .= ', ' . $tableAlias . ".$srcColumn AS $columnAlias"; - $resultColumnName = $this->_platform->getSQLResultCasing($columnAlias); - if ( ! isset($this->_resultColumnNames[$resultColumnName])) { - $this->_resultColumnNames[$resultColumnName] = $srcColumn; - } + if ($columnList != '') $columnList .= ', '; + $columnList .= $this->getSelectJoinColumnSQL($tableAlias, $srcColumn, + isset($assoc2['inherited']) ? $assoc2['inherited'] : $subClass->name + ); } } } diff --git a/lib/Doctrine/ORM/Persisters/SingleTablePersister.php b/lib/Doctrine/ORM/Persisters/SingleTablePersister.php index 856ff3485..0ffa93826 100644 --- a/lib/Doctrine/ORM/Persisters/SingleTablePersister.php +++ b/lib/Doctrine/ORM/Persisters/SingleTablePersister.php @@ -63,12 +63,10 @@ class SingleTablePersister extends AbstractEntityInheritancePersister foreach ($subClass->associationMappings as $assoc) { if ($assoc['isOwningSide'] && $assoc['type'] & ClassMetadata::TO_ONE && ! isset($assoc['inherited'])) { foreach ($assoc['targetToSourceKeyColumns'] as $srcColumn) { - $columnAlias = $srcColumn . $this->_sqlAliasCounter++; - $columnList .= ', ' . $tableAlias . ".$srcColumn AS $columnAlias"; - $resultColumnName = $this->_platform->getSQLResultCasing($columnAlias); - if ( ! isset($this->_resultColumnNames[$resultColumnName])) { - $this->_resultColumnNames[$resultColumnName] = $srcColumn; - } + if ($columnList != '') $columnList .= ', '; + $columnList .= $this->getSelectJoinColumnSQL($tableAlias, $srcColumn, + isset($assoc['inherited']) ? $assoc['inherited'] : $this->_class->name + ); } } } diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC837Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC837Test.php new file mode 100644 index 000000000..6afea88cd --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC837Test.php @@ -0,0 +1,198 @@ +_schemaTool->createSchema(array( + $this->_em->getClassMetadata(__NAMESPACE__ . '\DDC837Super'), + $this->_em->getClassMetadata(__NAMESPACE__ . '\DDC837Class1'), + $this->_em->getClassMetadata(__NAMESPACE__ . '\DDC837Class2'), + $this->_em->getClassMetadata(__NAMESPACE__ . '\DDC837Class3'), + $this->_em->getClassMetadata(__NAMESPACE__ . '\DDC837Aggregate'), + )); + } + + /** + * @group DDC-837 + */ + public function testIssue() + { + //$this->_em->getConnection()->getConfiguration()->setSQLLogger(new \Doctrine\DBAL\Logging\EchoSQLLogger); + + $c1 = new DDC837Class1(); + $c1->title = "Foo"; + $c1->description = "Foo"; + $aggregate1 = new DDC837Aggregate('test1'); + $c1->aggregate = $aggregate1; + + $c2 = new DDC837Class2(); + $c2->title = "Bar"; + $c2->description = "Bar"; + $c2->text = "Bar"; + $aggregate2 = new DDC837Aggregate('test2'); + $c2->aggregate = $aggregate2; + + $c3 = new DDC837Class3(); + $c3->apples = "Baz"; + $c3->bananas = "Baz"; + + $this->_em->persist($c1); + $this->_em->persist($aggregate1); + $this->_em->persist($c2); + $this->_em->persist($aggregate2); + $this->_em->persist($c3); + $this->_em->flush(); + $this->_em->clear(); + + // Test Class1 + $e1 = $this->_em->find('Doctrine\Tests\ORM\Functional\Ticket\DDC837Super', $c1->id); + + $this->assertType('Doctrine\Tests\ORM\Functional\Ticket\DDC837Class1', $e1); + $this->assertEquals('Foo', $e1->title); + $this->assertEquals('Foo', $e1->description); + $this->assertType(__NAMESPACE__ . '\DDC837Aggregate', $e1->aggregate); + $this->assertEquals('test1', $e1->aggregate->getSysname()); + + // Test Class 2 + $e2 = $this->_em->find('Doctrine\Tests\ORM\Functional\Ticket\DDC837Super', $c2->id); + + $this->assertType('Doctrine\Tests\ORM\Functional\Ticket\DDC837Class2', $e2); + $this->assertEquals('Bar', $e2->title); + $this->assertEquals('Bar', $e2->description); + $this->assertEquals('Bar', $e2->text); + $this->assertType(__NAMESPACE__ . '\DDC837Aggregate', $e2->aggregate); + $this->assertEquals('test2', $e2->aggregate->getSysname()); + + $all = $this->_em->getRepository(__NAMESPACE__.'\DDC837Super')->findAll(); + + foreach ($all as $obj) { + if ($obj instanceof DDC837Class1) { + $this->assertEquals('Foo', $obj->title); + $this->assertEquals('Foo', $obj->description); + } else if ($obj instanceof DDC837Class2) { + $this->assertTrue($e2 === $obj); + $this->assertEquals('Bar', $obj->title); + $this->assertEquals('Bar', $obj->description); + $this->assertEquals('Bar', $obj->text); + } else if ($obj instanceof DDC837Class3) { + $this->assertEquals('Baz', $obj->apples); + $this->assertEquals('Baz', $obj->bananas); + } else { + $this->fail('Instance of DDC837Class1, DDC837Class2 or DDC837Class3 expected.'); + } + } + } +} + +/** + * @Entity + * @Table(name="DDC837Super") + * @InheritanceType("JOINED") + * @DiscriminatorColumn(name="type", type="string") + * @DiscriminatorMap({"class1" = "DDC837Class1", "class2" = "DDC837Class2", "class3"="DDC837Class3"}) + */ +abstract class DDC837Super +{ + /** + * @Id @Column(name="id", type="integer") + * @GeneratedValue(strategy="AUTO") + */ + public $id; +} + +/** + * @Entity + */ +class DDC837Class1 extends DDC837Super +{ + /** + * @Column(name="title", type="string", length="150") + */ + public $title; + + /** + * @Column(name="content", type="string", length="500") + */ + public $description; + + /** + * @OneToOne(targetEntity="DDC837Aggregate") + */ + public $aggregate; +} + +/** + * @Entity + */ +class DDC837Class2 extends DDC837Super +{ + /** + * @Column(name="title", type="string", length="150") + */ + public $title; + + /** + * @Column(name="content", type="string", length="500") + */ + public $description; + + /** + * @Column(name="text", type="text") + */ + public $text; + + /** + * @OneToOne(targetEntity="DDC837Aggregate") + */ + public $aggregate; +} + +/** + * An extra class to demonstrate why title and description aren't in Super + * + * @Entity + */ +class DDC837Class3 extends DDC837Super +{ + /** + * @Column(name="title", type="string", length="150") + */ + public $apples; + + /** + * @Column(name="content", type="string", length="500") + */ + public $bananas; +} + +/** + * @Entity + */ +class DDC837Aggregate +{ + /** + * @Id @Column(name="id", type="integer") + * @GeneratedValue + */ + public $id; + + /** + * @Column(name="sysname", type="string") + */ + protected $sysname; + + public function __construct($sysname) + { + $this->sysname = $sysname; + } + + public function getSysname() + { + return $this->sysname; + } +} \ No newline at end of file