From a75c672ee73569d09d13d513134f3c30e5fecb72 Mon Sep 17 00:00:00 2001 From: "Fabio B. Silva" Date: Tue, 5 Jun 2012 17:10:44 -0300 Subject: [PATCH] fix sequence and join columns --- .../ORM/Mapping/ClassMetadataFactory.php | 35 ++++-- .../ORM/Mapping/ClassMetadataInfo.php | 10 +- .../ORM/Mapping/DefaultQuoteStrategy.php | 31 +++++ lib/Doctrine/ORM/Mapping/QuoteStrategy.php | 19 +++ .../ORM/Persisters/BasicEntityPersister.php | 7 +- lib/Doctrine/ORM/Tools/SchemaTool.php | 21 ++-- tests/Doctrine/Tests/Models/Quote/Group.php | 11 ++ .../ORM/Functional/Ticket/DDC1151Test.php | 4 +- .../ORM/Functional/Ticket/DDC1360Test.php | 2 +- .../ORM/Functional/Ticket/DDC1843Test.php | 111 ++++++++++++++++++ .../ORM/Mapping/ClassMetadataFactoryTest.php | 6 +- 11 files changed, 229 insertions(+), 28 deletions(-) create mode 100644 tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1843Test.php diff --git a/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php b/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php index 1bfa99d74..c0bb0c66d 100644 --- a/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php +++ b/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php @@ -648,23 +648,40 @@ class ClassMetadataFactory implements ClassMetadataFactoryInterface // For PostgreSQL IDENTITY (SERIAL) we need a sequence name. It defaults to // __seq in PostgreSQL for SERIAL columns. // Not pretty but necessary and the simplest solution that currently works. - $seqName = $this->targetPlatform instanceof Platforms\PostgreSQLPlatform ? - $class->getTableName() . '_' . $class->columnNames[$class->identifier[0]] . '_seq' : - null; - $class->setIdGenerator(new \Doctrine\ORM\Id\IdentityGenerator($seqName)); + $sequenceName = null; + if($this->targetPlatform instanceof Platforms\PostgreSQLPlatform) { + $fieldName = $class->getSingleIdentifierFieldName(); + $columnName = $class->getSingleIdentifierColumnName(); + $quoted = isset($class->fieldMappings[$fieldName]['quoted']) || isset($class->table['quoted']); + $sequenceName = $class->getTableName() . '_' . $columnName . '_seq'; + $definition = array( + 'sequenceName' => $this->targetPlatform->fixSchemaElementName($sequenceName) + ); + if ($quoted) { + $definition['quoted'] = true; + } + $sequenceName = $this->em->getQuoteStrategy()->getSequenceName($definition, $class); + } + $class->setIdGenerator(new \Doctrine\ORM\Id\IdentityGenerator($sequenceName)); break; case ClassMetadata::GENERATOR_TYPE_SEQUENCE: // If there is no sequence definition yet, create a default definition $definition = $class->sequenceGeneratorDefinition; if ( ! $definition) { - $sequenceName = $class->getTableName() . '_' . $class->getSingleIdentifierColumnName() . '_seq'; - $definition['sequenceName'] = $this->targetPlatform->fixSchemaElementName($sequenceName); - $definition['allocationSize'] = 1; - $definition['initialValue'] = 1; + $fieldName = $class->getSingleIdentifierFieldName(); + $columnName = $class->getSingleIdentifierColumnName(); + $quoted = isset($class->fieldMappings[$fieldName]['quoted']) || isset($class->table['quoted']); + $sequenceName = $class->getTableName() . '_' . $columnName . '_seq'; + $definition['sequenceName'] = $this->targetPlatform->fixSchemaElementName($sequenceName); + $definition['allocationSize'] = 1; + $definition['initialValue'] = 1; + if ($quoted) { + $definition['quoted'] = true; + } $class->setSequenceGeneratorDefinition($definition); } $sequenceGenerator = new \Doctrine\ORM\Id\SequenceGenerator( - $definition['sequenceName'], + $this->em->getQuoteStrategy()->getSequenceName($definition, $class), $definition['allocationSize'] ); $class->setIdGenerator($sequenceGenerator); diff --git a/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php b/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php index 69d8c20cc..5e860be10 100644 --- a/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php +++ b/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php @@ -2567,9 +2567,10 @@ class ClassMetadataInfo implements ClassMetadata * The definition must have the following structure: * * array( - * 'sequenceName' => 'name', + * 'sequenceName' => 'name', * 'allocationSize' => 20, - * 'initialValue' => 1 + * 'initialValue' => 1 + * 'quoted' => 1 * ) * * @@ -2577,6 +2578,11 @@ class ClassMetadataInfo implements ClassMetadata */ public function setSequenceGeneratorDefinition(array $definition) { + if (isset($definition['name']) && $definition['name'] == '`') { + $definition['name'] = trim($definition['name'], '`'); + $definition['quoted'] = true; + } + $this->sequenceGeneratorDefinition = $definition; } diff --git a/lib/Doctrine/ORM/Mapping/DefaultQuoteStrategy.php b/lib/Doctrine/ORM/Mapping/DefaultQuoteStrategy.php index 3e09f1929..f1c6fa5d2 100644 --- a/lib/Doctrine/ORM/Mapping/DefaultQuoteStrategy.php +++ b/lib/Doctrine/ORM/Mapping/DefaultQuoteStrategy.php @@ -49,6 +49,37 @@ class DefaultQuoteStrategy extends QuoteStrategy : $class->table['name']; } + /** + * {@inheritdoc} + */ + public function getSequenceName(array $definition, ClassMetadata $class) + { + return isset($definition['quoted']) + ? $this->platform->quoteSingleIdentifier($definition['sequenceName']) + : $definition['sequenceName']; + } + + /** + * {@inheritdoc} + */ + public function getJoinColumnName($columnName, array $association, ClassMetadata $class) + { + if( !isset($association['joinColumns'])) { + return $columnName; + } + + foreach ($association['joinColumns'] as $joinColumn) { + if($joinColumn['name'] === $columnName) { + if (isset($joinColumn['quoted'])) { + return $this->platform->quoteIdentifier($columnName); + } + return $columnName; + } + } + + return $columnName; + } + /** * {@inheritdoc} */ diff --git a/lib/Doctrine/ORM/Mapping/QuoteStrategy.php b/lib/Doctrine/ORM/Mapping/QuoteStrategy.php index 5882783ad..aacf1c03d 100644 --- a/lib/Doctrine/ORM/Mapping/QuoteStrategy.php +++ b/lib/Doctrine/ORM/Mapping/QuoteStrategy.php @@ -60,14 +60,33 @@ abstract class QuoteStrategy */ abstract public function getTableName(ClassMetadata $class); + /** + * Gets the (possibly quoted) sequence name for safe use in an SQL statement. + * + * @param array $definition + * @param ClassMetadata $class + * @return string + */ + abstract public function getSequenceName(array $definition, ClassMetadata $class); + /** * Gets the (possibly quoted) name of the join table. * + * @param array $association * @param ClassMetadata $class * @return string */ abstract public function getJoinTableName(array $association, ClassMetadata $class); + /** + * Gets the (possibly quoted) join column name. + * + * @param array $association + * @param ClassMetadata $class + * @return string + */ + abstract public function getJoinColumnName($columnName, array $association, ClassMetadata $class); + /** * Gets the (possibly quoted) identifier column names for safe use in an SQL statement. * diff --git a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php index 6ff0568cf..400d0f77e 100644 --- a/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php +++ b/lib/Doctrine/ORM/Persisters/BasicEntityPersister.php @@ -1080,10 +1080,11 @@ class BasicEntityPersister foreach ($assoc['targetToSourceKeyColumns'] as $srcColumn) { if ($columnList) $columnList .= ', '; + $quotedColumn = $this->quoteStrategy->getJoinColumnName($srcColumn, $assoc, $this->_class); $resultColumnName = $this->getSQLColumnAlias($srcColumn); $columnList .= $this->_getSQLTableAlias($class->name, ($alias == 'r' ? '' : $alias) ) - . '.' . $srcColumn . ' AS ' . $resultColumnName; - $this->_rsm->addMetaResult($alias, $resultColumnName, $srcColumn, isset($assoc['id']) && $assoc['id'] === true); + . '.' . $quotedColumn . ' AS ' . $resultColumnName; + $this->_rsm->addMetaResult($alias, $resultColumnName, $quotedColumn, isset($assoc['id']) && $assoc['id'] === true); } } @@ -1191,7 +1192,7 @@ class BasicEntityPersister if ($assoc['isOwningSide'] && $assoc['type'] & ClassMetadata::TO_ONE) { foreach ($assoc['targetToSourceKeyColumns'] as $sourceCol) { - $columns[] = $sourceCol; + $columns[] = $this->quoteStrategy->getJoinColumnName($sourceCol, $assoc, $this->_class); } } } else if ($this->_class->generatorType != ClassMetadata::GENERATOR_TYPE_IDENTITY || $this->_class->identifier[0] != $name) { diff --git a/lib/Doctrine/ORM/Tools/SchemaTool.php b/lib/Doctrine/ORM/Tools/SchemaTool.php index 1f69e0057..04796e6d3 100644 --- a/lib/Doctrine/ORM/Tools/SchemaTool.php +++ b/lib/Doctrine/ORM/Tools/SchemaTool.php @@ -217,12 +217,12 @@ class SchemaTool $pkColumns = array(); foreach ($class->identifier as $identifierField) { if (isset($class->fieldMappings[$identifierField])) { - $pkColumns[] = $class->getColumnName($identifierField); + $pkColumns[] = $this->quoteStrategy->getColumnName($identifierField, $class); } else if (isset($class->associationMappings[$identifierField])) { /* @var $assoc \Doctrine\ORM\Mapping\OneToOne */ $assoc = $class->associationMappings[$identifierField]; foreach ($assoc['joinColumns'] as $joinColumn) { - $pkColumns[] = $joinColumn['name']; + $pkColumns[] = $this->quoteStrategy->getJoinColumnName($joinColumn['name'], $assoc, $class); } } } @@ -252,11 +252,11 @@ class SchemaTool $processedClasses[$class->name] = true; if ($class->isIdGeneratorSequence() && $class->name == $class->rootEntityName) { - $seqDef = $class->sequenceGeneratorDefinition; - - if (!$schema->hasSequence($seqDef['sequenceName'])) { + $seqDef = $class->sequenceGeneratorDefinition; + $quotedName = $this->quoteStrategy->getSequenceName($seqDef, $class); + if ( ! $schema->hasSequence($quotedName)) { $schema->createSequence( - $seqDef['sequenceName'], + $quotedName, $seqDef['allocationSize'], $seqDef['initialValue'] ); @@ -523,9 +523,10 @@ class SchemaTool ); } - $primaryKeyColumns[] = $columnName; - $localColumns[] = $columnName; - $foreignColumns[] = $joinColumn['referencedColumnName']; + $primaryKeyColumns[] = $columnName; + $localColumns[] = $columnName; + $foreignColumns[] = $joinColumn['referencedColumnName']; + $quotedColumnName = $this->quoteStrategy->getJoinColumnName($columnName, $mapping, $class); if ( ! $theJoinTable->hasColumn($joinColumn['name'])) { // Only add the column to the table if it does not exist already. @@ -551,7 +552,7 @@ class SchemaTool $columnOptions['precision'] = $fieldMapping['precision']; } - $theJoinTable->addColumn($columnName, $fieldMapping['type'], $columnOptions); + $theJoinTable->addColumn($quotedColumnName, $fieldMapping['type'], $columnOptions); } if (isset($joinColumn['unique']) && $joinColumn['unique'] == true) { diff --git a/tests/Doctrine/Tests/Models/Quote/Group.php b/tests/Doctrine/Tests/Models/Quote/Group.php index 1540a1d02..2117f4b69 100644 --- a/tests/Doctrine/Tests/Models/Quote/Group.php +++ b/tests/Doctrine/Tests/Models/Quote/Group.php @@ -21,9 +21,20 @@ class Group */ public $name; + /** + * @ManyToOne(targetEntity="Group", cascade={"persist"}) + * @JoinColumn(name="`parent-id`", referencedColumnName="`group-id`") + */ + public $parent; + /** * @ManyToMany(targetEntity="User", mappedBy="groups") */ public $users; + public function __construct($name = null, Group $parent = null) + { + $this->name = $name; + $this->parent = $parent; + } } \ No newline at end of file diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1151Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1151Test.php index 51a2d4555..fe975f69d 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1151Test.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1151Test.php @@ -25,8 +25,8 @@ class DDC1151Test extends \Doctrine\Tests\OrmFunctionalTestCase $this->assertEquals("CREATE INDEX IDX_88A3259AC5AD08A ON ddc1151user_ddc1151group (ddc1151user_id)", $sql[2]); $this->assertEquals("CREATE INDEX IDX_88A32597357E0B1 ON ddc1151user_ddc1151group (ddc1151group_id)", $sql[3]); $this->assertEquals("CREATE TABLE \"Group\" (id INT NOT NULL, PRIMARY KEY(id))", $sql[4]); - $this->assertEquals("CREATE SEQUENCE User_id_seq INCREMENT BY 1 MINVALUE 1 START 1", $sql[5]); - $this->assertEquals("CREATE SEQUENCE Group_id_seq INCREMENT BY 1 MINVALUE 1 START 1", $sql[6]); + $this->assertEquals("CREATE SEQUENCE \"User_id_seq\" INCREMENT BY 1 MINVALUE 1 START 1", $sql[5]); + $this->assertEquals("CREATE SEQUENCE \"Group_id_seq\" INCREMENT BY 1 MINVALUE 1 START 1", $sql[6]); $this->assertEquals("ALTER TABLE ddc1151user_ddc1151group ADD CONSTRAINT FK_88A3259AC5AD08A FOREIGN KEY (ddc1151user_id) REFERENCES \"User\" (id) ON DELETE CASCADE NOT DEFERRABLE INITIALLY IMMEDIATE", $sql[7]); $this->assertEquals("ALTER TABLE ddc1151user_ddc1151group ADD CONSTRAINT FK_88A32597357E0B1 FOREIGN KEY (ddc1151group_id) REFERENCES \"Group\" (id) ON DELETE CASCADE NOT DEFERRABLE INITIALLY IMMEDIATE", $sql[8]); } diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1360Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1360Test.php index f95f77eb4..38f5837e5 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1360Test.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1360Test.php @@ -21,7 +21,7 @@ class DDC1360Test extends OrmFunctionalTestCase $this->assertEquals(array( 'CREATE TABLE "user"."user" (id INT NOT NULL, PRIMARY KEY(id))', - 'CREATE SEQUENCE "user".user_id_seq INCREMENT BY 1 MINVALUE 1 START 1', + 'CREATE SEQUENCE "user"."user_id_seq" INCREMENT BY 1 MINVALUE 1 START 1', ), $sql); } } diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1843Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1843Test.php new file mode 100644 index 000000000..4b28f66e2 --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC1843Test.php @@ -0,0 +1,111 @@ +markTestIncomplete(); + parent::setUp(); + + $this->_em->getConnection()->getConfiguration()->setSQLLogger(new \Doctrine\DBAL\Logging\EchoSQLLogger); + + try { + $this->_schemaTool->createSchema(array( + $this->_em->getClassMetadata(self::CLASS_NAME), + )); + } catch(\Exception $e) { + $this->fail($e->getMessage()); + } + } + + public function testCreateRetreaveUpdateDelete() + { + + $e1 = new Group('Parent Bar 1'); + $e2 = new Group('Parent Foo 2'); + + $this->_em->persist($e1); + $this->_em->persist($e2); + $this->_em->flush(); + + $e3 = new Group('Bar 3', $e1); + $e4 = new Group('Foo 4', $e2); + + // Create + $this->_em->persist($e3); + $this->_em->persist($e4); + $this->_em->flush(); + $this->_em->clear(); + + $e1Id = $e1->id; + $e2Id = $e2->id; + $e3Id = $e3->id; + $e4Id = $e4->id; + + // Retreave + $e1 = $this->_em->find(self::CLASS_NAME, $e1Id); + $e2 = $this->_em->find(self::CLASS_NAME, $e2Id); + $e3 = $this->_em->find(self::CLASS_NAME, $e3Id); + $e4 = $this->_em->find(self::CLASS_NAME, $e4Id); + + $this->assertInstanceOf(self::CLASS_NAME, $e1); + $this->assertInstanceOf(self::CLASS_NAME, $e2); + $this->assertInstanceOf(self::CLASS_NAME, $e3); + $this->assertInstanceOf(self::CLASS_NAME, $e4); + + $this->assertEquals($e1Id, $e1->id); + $this->assertEquals($e2Id, $e2->id); + $this->assertEquals($e3Id, $e3->id); + $this->assertEquals($e4Id, $e4->id); + + return; + + $this->assertEquals('Bar 1', $e1->value); + $this->assertEquals('Foo 1', $e2->value); + + $e1->value = 'Bar 2'; + $e2->value = 'Foo 2'; + + // Update + $this->_em->persist($e1); + $this->_em->persist($e2); + $this->_em->flush(); + + $this->assertEquals('Bar 2', $e1->value); + $this->assertEquals('Foo 2', $e2->value); + + $this->assertInstanceOf(self::CLASS_NAME, $e1); + $this->assertInstanceOf(self::CLASS_NAME, $e2); + + $this->assertEquals($e1Id, $e1->id); + $this->assertEquals($e2Id, $e2->id); + + $this->assertEquals('Bar 2', $e1->value); + $this->assertEquals('Foo 2', $e2->value); + + // Delete + $this->_em->remove($e1); + $this->_em->remove($e2); + $this->_em->flush(); + + + $e1 = $this->_em->find(self::CLASS_NAME, $e1Id); + $e2 = $this->_em->find(self::CLASS_NAME, $e2Id); + + $this->assertNull($e1); + $this->assertNull($e2); + } + +} \ No newline at end of file diff --git a/tests/Doctrine/Tests/ORM/Mapping/ClassMetadataFactoryTest.php b/tests/Doctrine/Tests/ORM/Mapping/ClassMetadataFactoryTest.php index d36c366d0..f1654dcfd 100644 --- a/tests/Doctrine/Tests/ORM/Mapping/ClassMetadataFactoryTest.php +++ b/tests/Doctrine/Tests/ORM/Mapping/ClassMetadataFactoryTest.php @@ -272,13 +272,17 @@ class ClassMetadataFactoryTest extends \Doctrine\Tests\OrmTestCase - // User Class Metadata + // User Group Metadata $this->assertTrue($groupMetadata->fieldMappings['id']['quoted']); $this->assertTrue($groupMetadata->fieldMappings['name']['quoted']); $this->assertEquals('user-id', $userMetadata->fieldMappings['id']['columnName']); $this->assertEquals('user-name', $userMetadata->fieldMappings['name']['columnName']); + $user = $groupMetadata->associationMappings['parent']; + $this->assertTrue($user['joinColumns'][0]['quoted']); + $this->assertEquals('parent-id', $user['joinColumns'][0]['name']); + $this->assertEquals('group-id', $user['joinColumns'][0]['referencedColumnName']); // Address Class Metadata