From c697a2d47f3c289276f2f73d741a2b930b0d6eea Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sat, 7 Aug 2010 19:33:54 +0200 Subject: [PATCH] Prototype hack of @ManyToOne + @Id support with two test-scenarios, composite association key only composite key, and a mixed key scenario. I think single foreign association would work also --- lib/Doctrine/ORM/Id/AssignedGenerator.php | 14 +- .../ORM/Mapping/ClassMetadataInfo.php | 12 ++ .../ORM/Mapping/Driver/AnnotationDriver.php | 8 + .../ORM/Persisters/BasicEntityPersister.php | 6 + lib/Doctrine/ORM/Tools/SchemaTool.php | 19 +- lib/Doctrine/ORM/UnitOfWork.php | 6 +- .../ORM/Functional/Ticket/DDC117Test.php | 196 ++++++++++++++++++ 7 files changed, 257 insertions(+), 4 deletions(-) create mode 100644 tests/Doctrine/Tests/ORM/Functional/Ticket/DDC117Test.php diff --git a/lib/Doctrine/ORM/Id/AssignedGenerator.php b/lib/Doctrine/ORM/Id/AssignedGenerator.php index f4bd3d631..e1981fe43 100644 --- a/lib/Doctrine/ORM/Id/AssignedGenerator.php +++ b/lib/Doctrine/ORM/Id/AssignedGenerator.php @@ -49,7 +49,12 @@ class AssignedGenerator extends AbstractIdGenerator foreach ($idFields as $idField) { $value = $class->getReflectionProperty($idField)->getValue($entity); if (isset($value)) { - $identifier[$idField] = $value; + if (is_object($value)) { + // TODO: Single Id only, i enforce that. Compoite Key as Foreign Keys Primary Key part sounds ugly + $identifier[$idField] = current($em->getUnitOfWork()->getEntityIdentifier($value)); + } else { + $identifier[$idField] = $value; + } } else { throw ORMException::entityMissingAssignedId($entity); } @@ -58,7 +63,12 @@ class AssignedGenerator extends AbstractIdGenerator $idField = $class->identifier[0]; $value = $class->reflFields[$idField]->getValue($entity); if (isset($value)) { - $identifier[$idField] = $value; + if (is_object($value)) { + // TODO: Single Id only, i enforce that. Compoite Key as Foreign Keys Primary Key part sounds ugly + $identifier[$idField] = current($em->getUnitOfWork()->getEntityIdentifier($value)); + } else { + $identifier[$idField] = $value; + } } else { throw ORMException::entityMissingAssignedId($entity); } diff --git a/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php b/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php index 8a016fd47..4edca896b 100644 --- a/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php +++ b/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php @@ -957,6 +957,18 @@ class ClassMetadataInfo if (isset($mapping['targetEntity']) && strpos($mapping['targetEntity'], '\\') === false && strlen($this->namespace) > 0) { $mapping['targetEntity'] = $this->namespace . '\\' . $mapping['targetEntity']; } + + // Complete id mapping + if (isset($mapping['id']) && $mapping['id'] === true) { + if ( ! in_array($mapping['fieldName'], $this->identifier)) { + $this->identifier[] = $mapping['fieldName']; + } + // Check for composite key + if ( ! $this->isIdentifierComposite && count($this->identifier) > 1) { + $this->isIdentifierComposite = true; + } + } + return $mapping; } diff --git a/lib/Doctrine/ORM/Mapping/Driver/AnnotationDriver.php b/lib/Doctrine/ORM/Mapping/Driver/AnnotationDriver.php index 6a61772a7..36df0a60d 100644 --- a/lib/Doctrine/ORM/Mapping/Driver/AnnotationDriver.php +++ b/lib/Doctrine/ORM/Mapping/Driver/AnnotationDriver.php @@ -283,6 +283,10 @@ class AnnotationDriver implements Driver throw MappingException::tableIdGeneratorNotImplemented($className); } } else if ($oneToOneAnnot = $this->_reader->getPropertyAnnotation($property, 'Doctrine\ORM\Mapping\OneToOne')) { + if ($idAnnot = $this->_reader->getPropertyAnnotation($property, 'Doctrine\ORM\Mapping\Id')) { + $mapping['id'] = true; + } + $mapping['targetEntity'] = $oneToOneAnnot->targetEntity; $mapping['joinColumns'] = $joinColumns; $mapping['mappedBy'] = $oneToOneAnnot->mappedBy; @@ -304,6 +308,10 @@ class AnnotationDriver implements Driver $metadata->mapOneToMany($mapping); } else if ($manyToOneAnnot = $this->_reader->getPropertyAnnotation($property, 'Doctrine\ORM\Mapping\ManyToOne')) { + if ($idAnnot = $this->_reader->getPropertyAnnotation($property, 'Doctrine\ORM\Mapping\Id')) { + $mapping['id'] = true; + } + $mapping['joinColumns'] = $joinColumns; $mapping['cascade'] = $manyToOneAnnot->cascade; $mapping['inversedBy'] = $manyToOneAnnot->inversedBy; diff --git a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php index 8fba4d000..1c47249f9 100644 --- a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php +++ b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php @@ -1118,6 +1118,12 @@ class BasicEntityPersister $conditionSql .= $this->_getSQLTableAlias($this->_class->name) . '.'; } $conditionSql .= $this->_class->getQuotedColumnName($field, $this->_platform); + } else if (isset($this->_class->associationMappings[$field])) { + // TODO: Inherited? + // TODO: Composite Keys as Foreign Key PK? That would be super ugly! And should probably be disallowed ;) + $conditionSql .= $this->_getSQLTableAlias($this->_class->name) . '.'; + + $conditionSql .= $this->_class->associationMappings[$field]->joinColumns[0]['name']; } else if ($assoc !== null) { if ($assoc->isManyToMany()) { $owningAssoc = $assoc->isOwningSide ? $assoc : $this->_em->getClassMetadata($assoc->targetEntityName) diff --git a/lib/Doctrine/ORM/Tools/SchemaTool.php b/lib/Doctrine/ORM/Tools/SchemaTool.php index 9d68f7c2d..e53311c4a 100644 --- a/lib/Doctrine/ORM/Tools/SchemaTool.php +++ b/lib/Doctrine/ORM/Tools/SchemaTool.php @@ -193,6 +193,22 @@ class SchemaTool $this->_gatherRelationsSql($class, $table, $schema); } + $pkColumns = array(); + foreach ($class->identifier AS $identifierField) { + if (isset($class->fieldMappings[$identifierField])) { + $pkColumns[] = $class->getQuotedColumnName($identifierField, $this->_platform); + } else if (isset($class->associationMappings[$identifierField])) { + /* @var $assoc \Doctrine\ORM\Mapping\OneToOne */ + $assoc = $class->associationMappings[$identifierField]; + foreach ($assoc->joinColumns AS $joinColumn) { + $pkColumns[] = $joinColumn['name']; + } + } + } + if (!$table->hasIndex('primary')) { + $table->setPrimaryKey($pkColumns); + } + if (isset($class->table['indexes'])) { foreach ($class->table['indexes'] AS $indexName => $indexData) { $table->addIndex($indexData['columns'], $indexName); @@ -275,10 +291,11 @@ class SchemaTool $pkColumns[] = $class->getQuotedColumnName($mapping['fieldName'], $this->_platform); } } + // For now, this is a hack required for single table inheritence, since this method is called // twice by single table inheritence relations if(!$table->hasIndex('primary')) { - $table->setPrimaryKey($pkColumns); + //$table->setPrimaryKey($pkColumns); } return $columns; diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index 2008cbe53..18f0dc617 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -1831,7 +1831,11 @@ class UnitOfWork implements PropertyChangedListener if ($class->isIdentifierComposite) { $id = array(); foreach ($class->identifier as $fieldName) { - $id[$fieldName] = $data[$fieldName]; + if (isset($class->associationMappings[$fieldName])) { + $id[$fieldName] = $data[$class->associationMappings[$fieldName]->joinColumns[0]['name']]; + } else { + $id[$fieldName] = $data[$fieldName]; + } } $idHash = implode(' ', $id); } else { diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC117Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC117Test.php new file mode 100644 index 000000000..4e32f00b4 --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC117Test.php @@ -0,0 +1,196 @@ +_em->getConnection()->getConfiguration()->setSQLLogger(new \Doctrine\DBAL\Logging\EchoSQLLogger); + + try { + $this->_schemaTool->createSchema(array( + $this->_em->getClassMetadata(__NAMESPACE__ . '\DDC117Article'), + $this->_em->getClassMetadata(__NAMESPACE__ . '\DDC117Reference'), + $this->_em->getClassMetadata(__NAMESPACE__ . '\DDC117Translation'), + )); + } catch(\Exception $e) { + + } + } + + /** + * @group DDC-117 + */ + public function testAssociationOnlyCompositeKey() + { + $article1 = new DDC117Article("Foo"); + $article2 = new DDC117Article("Bar"); + + $this->_em->persist($article1); + $this->_em->persist($article2); + $this->_em->flush(); + + $reference = new DDC117Reference($article1, $article2, "Test-Description"); + $this->_em->persist($reference); + $this->_em->flush(); + + $mapRef = $this->_em->find(__NAMESPACE__."\DDC117Reference", array('source' => 1, 'target' => 2)); + $this->assertSame($reference, $mapRef); + + $this->_em->clear(); + + $dql = "SELECT r, s FROM ".__NAMESPACE__."\DDC117Reference r JOIN r.source s WHERE r.source = ?1"; + $ref = $this->_em->createQuery($dql)->setParameter(1, 1)->getSingleResult(); + + $this->_em->clear(); + + $refRep = $this->_em->find(__NAMESPACE__."\DDC117Reference", array('source' => 1, 'target' => 2)); + + $this->assertType(__NAMESPACE__."\DDC117Reference", $refRep); + $this->assertType(__NAMESPACE__."\DDC117Article", $refRep->target()); + $this->assertType(__NAMESPACE__."\DDC117Article", $refRep->source()); + + $this->assertSame($refRep, $this->_em->find(__NAMESPACE__."\DDC117Reference", array('source' => 1, 'target' => 2))); + } + + /** + * @group DDC-117 + */ + public function testMixedCompositeKey() + { + $article1 = new DDC117Article("Foo"); + $this->_em->persist($article1); + $this->_em->flush(); + + $translation = new DDC117Translation($article1, "en", "Bar"); + $this->_em->persist($translation); + $this->_em->flush(); + + $this->assertSame($translation, $this->_em->find(__NAMESPACE__ . '\DDC117Translation', array('article' => $article1->id(), 'language' => 'en'))); + + $this->_em->clear(); + + $translation = $this->_em->find(__NAMESPACE__ . '\DDC117Translation', array('article' => $article1->id(), 'language' => 'en')); + $this->assertType(__NAMESPACE__ . '\DDC117Translation', $translation); + + $this->_em->clear(); + + $dql = 'SELECT t, a FROM ' . __NAMESPACE__ . '\DDC117Translation t JOIN t.article a WHERE t.article = ?1 AND t.language = ?2'; + $dqlTrans = $this->_em->createQuery($dql) + ->setParameter(1, $article1->id()) + ->setParameter(2, 'en') + ->getSingleResult(); + + $this->assertType(__NAMESPACE__ . '\DDC117Translation', $translation); + } +} + +/** + * @Entity + */ +class DDC117Article +{ + /** @Id @Column(type="integer") @GeneratedValue */ + private $id; + /** @Column */ + private $title; + + /** + * @OneToMany(targetEntity="DDC117Reference", mappedBy="source") + */ + private $references; + + public function __construct($title) + { + $this->title = $title; + $this->references = new \Doctrine\Common\Collections\ArrayCollection(); + } + + public function id() + { + return $this->id; + } + + public function addReference($reference) + { + $this->references[] = $reference; + } +} + +/** + * @Entity + */ +class DDC117Reference +{ + /** + * @Id @ManyToOne(targetEntity="DDC117Article") + */ + private $source; + + /** + * @Id @ManyToOne(targetEntity="DDC117Article") + */ + private $target; + + /** + * @column(type="string") + */ + private $description; + + /** + * @column(type="datetime") + */ + private $created; + + public function __construct($source, $target, $description) + { + $source->addReference($this); + $target->addReference($this); + + $this->source = $source; + $this->target = $target; + $this->description = $description; + $this->created = new \DateTime("now"); + } + + public function source() + { + return $this->source; + } + + public function target() + { + return $this->target; + } +} + +/** + * @Entity + */ +class DDC117Translation +{ + /** + * @Id @ManyToOne(targetEntity="DDC117Article") + */ + private $article; + + /** + * @Id @column(type="string") + */ + private $language; + + /** + * @column(type="string") + */ + private $title; + + public function __construct($article, $language, $title) + { + $this->article = $article; + $this->language = $language; + $this->title = $title; + } +} \ No newline at end of file