From 2d27a99a0bdea2bea3379a213fdbfc11e784234a Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Wed, 29 Dec 2010 01:02:21 +0100 Subject: [PATCH] DDC-117 - Began to fix some issues surrounding the DDC-881 report and references to composite fk entities. --- .../ORM/Mapping/ClassMetadataInfo.php | 40 ++-- lib/Doctrine/ORM/Mapping/MappingException.php | 12 ++ lib/Doctrine/ORM/Tools/SchemaTool.php | 49 ++++- .../ORM/Functional/Ticket/DDC881Test.php | 181 ++++++++++++++++++ 4 files changed, 245 insertions(+), 37 deletions(-) create mode 100644 tests/Doctrine/Tests/ORM/Functional/Ticket/DDC881Test.php diff --git a/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php b/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php index 53042d015..298d5c02d 100644 --- a/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php +++ b/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php @@ -713,6 +713,12 @@ class ClassMetadataInfo // Complete id mapping if (isset($mapping['id']) && $mapping['id'] === true) { if ( ! in_array($mapping['fieldName'], $this->identifier)) { + if (count($mapping['joinColumns']) >= 2) { + throw MappingException::cannotMapCompositePrimaryKeyEntitiesAsForeignId( + $mapping['targetEntity'], $this->name, $mapping['fieldName'] + ); + } + $this->identifier[] = $mapping['fieldName']; } // Check for composite key @@ -994,14 +1000,18 @@ class ClassMetadataInfo $columnNames = array(); foreach ($this->identifier as $idField) { if (isset($this->associationMappings[$idField])) { + // no composite pk as fk entity assumption: $columnNames[] = $this->associationMappings[$idField]['joinColumns'][0]['name']; } else { $columnNames[] = $this->fieldMappings[$idField]['columnName']; } } return $columnNames; - } else { + } else if(isset($this->fieldMappings[$this->identifier[0]])) { return array($this->fieldMappings[$this->identifier[0]]['columnName']); + } else { + // no composite pk as fk entity assumption: + return array($this->associationMappings[$this->identifier[0]]['joinColumns'][0]['name']); } } @@ -1265,34 +1275,6 @@ class ClassMetadataInfo $type == self::INHERITANCE_TYPE_TABLE_PER_CLASS; } - /** - * Makes some automatic additions to the association mapping to make the life - * easier for the user, and store join columns in the metadata. - * - * @param array $mapping - * @todo Pass param by ref? - */ - private function _completeAssociationMapping(array $mapping) - { - $mapping['sourceEntity'] = $this->name; - 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; - } - /** * Adds a mapped field to the class. * diff --git a/lib/Doctrine/ORM/Mapping/MappingException.php b/lib/Doctrine/ORM/Mapping/MappingException.php index 14bd9ca0b..3d0e5e1a9 100644 --- a/lib/Doctrine/ORM/Mapping/MappingException.php +++ b/lib/Doctrine/ORM/Mapping/MappingException.php @@ -231,4 +231,16 @@ class MappingException extends \Doctrine\ORM\ORMException { return new self("It is illegal to put a one-to-many or many-to-many association on mapped superclass '".$className."#".$field."'."); } + + /** + * @param string $className + * @param string $targetEntity + * @param string $targetField + * @return self + */ + public static function cannotMapCompositePrimaryKeyEntitiesAsForeignId($className, $targetEntity, $targetField) + { + return new self("It is not possible to map entity '".$className."' with a composite primary key ". + "as part of the primary key of another entity '".$targetEntity."#".$targetField."'."); + } } \ No newline at end of file diff --git a/lib/Doctrine/ORM/Tools/SchemaTool.php b/lib/Doctrine/ORM/Tools/SchemaTool.php index 30d6db25d..16042d398 100644 --- a/lib/Doctrine/ORM/Tools/SchemaTool.php +++ b/lib/Doctrine/ORM/Tools/SchemaTool.php @@ -422,13 +422,47 @@ class SchemaTool } } + /** + * Get the class metadata that is responsible for the definition of the referenced column name. + * + * Previously this was a simple task, but with DDC-117 this problem is actually recursive. If its + * not a simple field, go through all identifier field names that are associations recursivly and + * find that referenced column name. + * + * TODO: Is there any way to make this code more pleasing? + * + * @param ClassMetadata $class + * @param string $referencedColumnName + * @return array(ClassMetadata, referencedFieldName) + */ + private function getDefiningClass($class, $referencedColumnName) + { + $referencedFieldName = $class->getFieldName($referencedColumnName); + + if ($class->hasField($referencedFieldName)) { + return array($class, $referencedFieldName); + } else if (in_array($referencedColumnName, $class->getIdentifierColumnNames())) { + // it seems to be an entity as foreign key + foreach ($class->getIdentifierFieldNames() AS $fieldName) { + if ($class->hasAssociation($fieldName) && $class->associationMappings[$fieldName]['joinColumns'][0]['name'] == $referencedColumnName) { + return $this->getDefiningClass( + $this->_em->getClassMetadata($class->associationMappings[$fieldName]['targetEntity']), + $class->associationMappings[$fieldName]['joinColumns'][0]['referencedColumnName'] + ); + } + } + } + + return null; + } + /** * Gather columns and fk constraints that are required for one part of relationship. * * @param array $joinColumns * @param \Doctrine\DBAL\Schema\Table $theJoinTable * @param ClassMetadata $class - * @param \Doctrine\ORM\Mapping\AssociationMapping $mapping + * @param array $mapping * @param array $primaryKeyColumns * @param array $uniqueConstraints */ @@ -437,12 +471,13 @@ class SchemaTool $localColumns = array(); $foreignColumns = array(); $fkOptions = array(); + $foreignTableName = $class->getTableName(); foreach ($joinColumns as $joinColumn) { $columnName = $joinColumn['name']; - $referencedFieldName = $class->getFieldName($joinColumn['referencedColumnName']); + list($definingClass, $referencedFieldName) = $this->getDefiningClass($class, $joinColumn['referencedColumnName']); - if ( ! $class->hasField($referencedFieldName)) { + if (!$definingClass) { throw new \Doctrine\ORM\ORMException( "Column name `".$joinColumn['referencedColumnName']."` referenced for relation from ". $mapping['sourceEntity'] . " towards ". $mapping['targetEntity'] . " does not exist." @@ -458,7 +493,7 @@ class SchemaTool // It might exist already if the foreign key is mapped into a regular // property as well. - $fieldMapping = $class->getFieldMapping($referencedFieldName); + $fieldMapping = $definingClass->getFieldMapping($referencedFieldName); $columnDef = null; if (isset($joinColumn['columnDefinition'])) { @@ -477,9 +512,7 @@ class SchemaTool $columnOptions['precision'] = $fieldMapping['precision']; } - $theJoinTable->addColumn( - $columnName, $class->getTypeOfColumn($joinColumn['referencedColumnName']), $columnOptions - ); + $theJoinTable->addColumn($columnName, $fieldMapping['type'], $columnOptions); } if (isset($joinColumn['unique']) && $joinColumn['unique'] == true) { @@ -496,7 +529,7 @@ class SchemaTool } $theJoinTable->addUnnamedForeignKeyConstraint( - $class->getTableName(), $localColumns, $foreignColumns, $fkOptions + $foreignTableName, $localColumns, $foreignColumns, $fkOptions ); } diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC881Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC881Test.php new file mode 100644 index 000000000..8b8aba843 --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC881Test.php @@ -0,0 +1,181 @@ +_schemaTool->createSchema(array( + $this->_em->getClassMetadata(__NAMESPACE__ . '\DDC881User'), + $this->_em->getClassMetadata(__NAMESPACE__ . '\DDC881Phonenumber'), + $this->_em->getClassMetadata(__NAMESPACE__ . '\DDC881Phonecall'), + )); + } catch (\Exception $e) { + + } + } + + /** + * @group DDC-117 + * @group DDC-881 + */ + public function testIssue() + { + /* Create two test users: albert and alfons */ + $albert = new DDC881User; + $albert->setName("albert"); + $this->_em->persist($albert); + + $alfons = new DDC881User; + $alfons->setName("alfons"); + $this->_em->persist($alfons); + + $this->_em->flush(); + + /* Assign two phone numbers to each user */ + $phoneAlbert1 = new DDC881PhoneNumber(); + $phoneAlbert1->setUser($albert); + $phoneAlbert1->setId(1); + $phoneAlbert1->setPhoneNumber("albert home: 012345"); + $this->_em->persist($phoneAlbert1); + + $phoneAlbert2 = new DDC881PhoneNumber(); + $phoneAlbert2->setUser($albert); + $phoneAlbert2->setId(2); + $phoneAlbert2->setPhoneNumber("albert mobile: 67890"); + $this->_em->persist($phoneAlbert2); + + $phoneAlfons1 = new DDC881PhoneNumber(); + $phoneAlfons1->setId(1); + $phoneAlfons1->setUser($alfons); + $phoneAlfons1->setPhoneNumber("alfons home: 012345"); + $this->_em->persist($phoneAlfons1); + + $phoneAlfons2 = new DDC881PhoneNumber(); + $phoneAlfons2->setId(2); + $phoneAlfons2->setUser($alfons); + $phoneAlfons2->setPhoneNumber("alfons mobile: 67890"); + $this->_em->persist($phoneAlfons2); + + /* We call alfons and albert once on their mobile numbers */ + $call1 = new DDC881PhoneCall(); + $call1->setPhoneNumber($phoneAlfons2); + $this->_em->persist($call1); + + $call2 = new DDC881PhoneCall(); + $call2->setPhoneNumber($phoneAlbert2); + $this->_em->persist($call2); + + $this->_em->flush(); + } + +} + +/** + * @Entity + */ +class DDC881User +{ + + /** + * @Id + * @Column(type="integer") + * @GeneratedValue(strategy="AUTO") + */ + private $id; + /** + * @Column(type="string") + */ + private $name; + /** + * @OneToMany(targetEntity="DDC881PhoneNumber",mappedBy="id") + */ + private $phoneNumbers; + + public function getName() + { + return $this->name; + } + + public function setName($name) + { + $this->name = $name; + } +} + +/** + * @Entity + */ +class DDC881PhoneNumber +{ + + /** + * @Id + * @Column(type="integer") + */ + private $id; + /** + * @Id + * @ManyToOne(targetEntity="DDC881User",cascade={"all"}) + */ + private $user; + /** + * @Column(type="string") + */ + private $phonenumber; + + public function setId($id) + { + $this->id = $id; + } + + public function setUser(DDC881User $user) + { + $this->user = $user; + } + + public function setPhoneNumber($phoneNumber) + { + $this->phonenumber = $phoneNumber; + } + +} + +/** + * @Entity + */ +class DDC881PhoneCall +{ + + /** + * @Id + * @Column(type="integer") + * @GeneratedValue(strategy="AUTO") + */ + private $id; + /** + * @OneToOne(targetEntity="DDC881PhoneNumber",cascade={"all"}) + * @JoinColumns({ + * @JoinColumn(name="phonenumber_id", referencedColumnName="id"), + * @JoinColumn(name="user_id", referencedColumnName="user_id") + * }) + */ + private $phonenumber; + /** + * @Column(type="string",nullable=true) + */ + private $callDate; + + public function setPhoneNumber(DDC881PhoneNumber $phoneNumber) + { + $this->phonenumber = $phoneNumber; + } + +} \ No newline at end of file