From eedf85cbdb3af364486c1b03cbaf039ea206be47 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sun, 20 Jan 2013 20:31:22 +0100 Subject: [PATCH] [DDC-2243] Fix bug where a bigint identifier would be casted to an integer, causing inconsistency with the string handling. --- .../ORM/Id/BigIntegerIdentityGenerator.php | 66 +++++++++++++++++++ lib/Doctrine/ORM/Id/IdentityGenerator.php | 8 +-- .../ORM/Mapping/ClassMetadataFactory.php | 10 ++- .../ORM/Mapping/AbstractMappingDriverTest.php | 22 +++---- 4 files changed, 88 insertions(+), 18 deletions(-) create mode 100644 lib/Doctrine/ORM/Id/BigIntegerIdentityGenerator.php diff --git a/lib/Doctrine/ORM/Id/BigIntegerIdentityGenerator.php b/lib/Doctrine/ORM/Id/BigIntegerIdentityGenerator.php new file mode 100644 index 000000000..e387023b4 --- /dev/null +++ b/lib/Doctrine/ORM/Id/BigIntegerIdentityGenerator.php @@ -0,0 +1,66 @@ +. + */ + +namespace Doctrine\ORM\Id; + +use Doctrine\ORM\EntityManager; + +/** + * Id generator that obtains IDs from special "identity" columns. These are columns + * that automatically get a database-generated, auto-incremented identifier on INSERT. + * This generator obtains the last insert id after such an insert. + */ +class BigIntegerIdentityGenerator extends AbstractIdGenerator +{ + /** + * The name of the sequence to pass to lastInsertId(), if any. + * + * @var string + */ + private $sequenceName; + + /** + * Constructor. + * + * @param string|null $seqName The name of the sequence to pass to lastInsertId() + * to obtain the last generated identifier within the current + * database session/connection, if any. + */ + public function __construct($sequenceName = null) + { + $this->sequenceName = $sequenceName; + } + + /** + * {@inheritdoc} + */ + public function generate(EntityManager $em, $entity) + { + return (string)$em->getConnection()->lastInsertId($this->sequenceName); + } + + /** + * {@inheritdoc} + */ + public function isPostInsertGenerator() + { + return true; + } +} + diff --git a/lib/Doctrine/ORM/Id/IdentityGenerator.php b/lib/Doctrine/ORM/Id/IdentityGenerator.php index 014bb44b4..d0bf327bb 100644 --- a/lib/Doctrine/ORM/Id/IdentityGenerator.php +++ b/lib/Doctrine/ORM/Id/IdentityGenerator.php @@ -33,7 +33,7 @@ class IdentityGenerator extends AbstractIdGenerator * * @var string */ - private $_seqName; + private $sequenceName; /** * Constructor. @@ -42,9 +42,9 @@ class IdentityGenerator extends AbstractIdGenerator * to obtain the last generated identifier within the current * database session/connection, if any. */ - public function __construct($seqName = null) + public function __construct($sequenceName = null) { - $this->_seqName = $seqName; + $this->sequenceName = $sequenceName; } /** @@ -52,7 +52,7 @@ class IdentityGenerator extends AbstractIdGenerator */ public function generate(EntityManager $em, $entity) { - return (int)$em->getConnection()->lastInsertId($this->_seqName); + return (int)$em->getConnection()->lastInsertId($this->sequenceName); } /** diff --git a/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php b/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php index b5fb39f1b..a259efa84 100644 --- a/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php +++ b/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php @@ -28,6 +28,7 @@ use Doctrine\Common\Persistence\Mapping\ReflectionService; use Doctrine\Common\Persistence\Mapping\ClassMetadata as ClassMetadataInterface; use Doctrine\Common\Persistence\Mapping\AbstractClassMetadataFactory; use Doctrine\ORM\Id\IdentityGenerator; +use Doctrine\ORM\Id\BigIntegerIdentityGenerator; use Doctrine\ORM\Event\LoadClassMetadataEventArgs; /** @@ -440,9 +441,9 @@ class ClassMetadataFactory extends AbstractClassMetadataFactory // __seq in PostgreSQL for SERIAL columns. // Not pretty but necessary and the simplest solution that currently works. $sequenceName = null; + $fieldName = $class->identifier ? $class->getSingleIdentifierFieldName() : 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'; @@ -457,7 +458,12 @@ class ClassMetadataFactory extends AbstractClassMetadataFactory $sequenceName = $this->em->getConfiguration()->getQuoteStrategy()->getSequenceName($definition, $class, $this->targetPlatform); } - $class->setIdGenerator(new \Doctrine\ORM\Id\IdentityGenerator($sequenceName)); + $generator = ($fieldName && $class->fieldMappings[$fieldName]['type'] === "bigint") + ? new BigIntegerIdentityGenerator($sequenceName) + : new IdentityGenerator($sequenceName); + + $class->setIdGenerator($generator); + break; case ClassMetadata::GENERATOR_TYPE_SEQUENCE: diff --git a/tests/Doctrine/Tests/ORM/Mapping/AbstractMappingDriverTest.php b/tests/Doctrine/Tests/ORM/Mapping/AbstractMappingDriverTest.php index b994d4863..f752c292e 100644 --- a/tests/Doctrine/Tests/ORM/Mapping/AbstractMappingDriverTest.php +++ b/tests/Doctrine/Tests/ORM/Mapping/AbstractMappingDriverTest.php @@ -475,24 +475,22 @@ abstract class AbstractMappingDriverTest extends \Doctrine\Tests\OrmTestCase /** * @group DDC-889 - * @expectedException Doctrine\ORM\Mapping\MappingException - * @expectedExceptionMessage Class "Doctrine\Tests\Models\DDC889\DDC889Class" sub class of "Doctrine\Tests\Models\DDC889\DDC889SuperClass" is not a valid entity or mapped super class. */ public function testInvalidEntityOrMappedSuperClassShouldMentionParentClasses() { + $this->setExpectedException('Doctrine\ORM\Mapping\MappingException', 'Class "Doctrine\Tests\Models\DDC889\DDC889Class" sub class of "Doctrine\Tests\Models\DDC889\DDC889SuperClass" is not a valid entity or mapped super class.'); + $this->createClassMetadata('Doctrine\Tests\Models\DDC889\DDC889Class'); } /** * @group DDC-889 - * @expectedException Doctrine\ORM\Mapping\MappingException - * @expectedExceptionMessage No identifier/primary key specified for Entity "Doctrine\Tests\Models\DDC889\DDC889Entity" sub class of "Doctrine\Tests\Models\DDC889\DDC889SuperClass". Every Entity must have an identifier/primary key. */ public function testIdentifierRequiredShouldMentionParentClasses() { - $factory = $this->createClassMetadataFactory(); - + + $this->setExpectedException('Doctrine\ORM\Mapping\MappingException', 'No identifier/primary key specified for Entity "Doctrine\Tests\Models\DDC889\DDC889Entity" sub class of "Doctrine\Tests\Models\DDC889\DDC889SuperClass". Every Entity must have an identifier/primary key.'); $factory->getMetadataFor('Doctrine\Tests\Models\DDC889\DDC889Entity'); } @@ -509,7 +507,7 @@ abstract class AbstractMappingDriverTest extends \Doctrine\Tests\OrmTestCase */ public function testNamedNativeQuery() { - + $class = $this->createClassMetadata('Doctrine\Tests\Models\CMS\CmsAddress'); //named native query @@ -538,7 +536,7 @@ abstract class AbstractMappingDriverTest extends \Doctrine\Tests\OrmTestCase $this->assertArrayHasKey('mapping-count', $class->sqlResultSetMappings); $this->assertArrayHasKey('mapping-find-all', $class->sqlResultSetMappings); $this->assertArrayHasKey('mapping-without-fields', $class->sqlResultSetMappings); - + $findAllMapping = $class->getSqlResultSetMapping('mapping-find-all'); $this->assertEquals('mapping-find-all', $findAllMapping['name']); $this->assertEquals('Doctrine\Tests\Models\CMS\CmsAddress', $findAllMapping['entities'][0]['entityClass']); @@ -550,7 +548,7 @@ abstract class AbstractMappingDriverTest extends \Doctrine\Tests\OrmTestCase $this->assertEquals('mapping-without-fields', $withoutFieldsMapping['name']); $this->assertEquals('Doctrine\Tests\Models\CMS\CmsAddress', $withoutFieldsMapping['entities'][0]['entityClass']); $this->assertEquals(array(), $withoutFieldsMapping['entities'][0]['fields']); - + $countMapping = $class->getSqlResultSetMapping('mapping-count'); $this->assertEquals('mapping-count', $countMapping['name']); $this->assertEquals(array('name'=>'count'), $countMapping['columns'][0]); @@ -638,7 +636,7 @@ abstract class AbstractMappingDriverTest extends \Doctrine\Tests\OrmTestCase $adminMetadata = $factory->getMetadataFor('Doctrine\Tests\Models\DDC964\DDC964Admin'); $guestMetadata = $factory->getMetadataFor('Doctrine\Tests\Models\DDC964\DDC964Guest'); - + // assert groups association mappings $this->assertArrayHasKey('groups', $guestMetadata->associationMappings); $this->assertArrayHasKey('groups', $adminMetadata->associationMappings); @@ -697,7 +695,7 @@ abstract class AbstractMappingDriverTest extends \Doctrine\Tests\OrmTestCase $this->assertEquals($guestAddress['isCascadeRefresh'], $adminAddress['isCascadeRefresh']); $this->assertEquals($guestAddress['isCascadeMerge'], $adminAddress['isCascadeMerge']); $this->assertEquals($guestAddress['isCascadeDetach'], $adminAddress['isCascadeDetach']); - + // assert override $this->assertEquals('address_id', $guestAddress['joinColumns'][0]['name']); $this->assertEquals(array('address_id'=>'id'), $guestAddress['sourceToTargetKeyColumns']); @@ -1071,7 +1069,7 @@ class DDC807Entity * @GeneratedValue(strategy="NONE") **/ public $id; - + public static function loadMetadata(ClassMetadataInfo $metadata) { $metadata->mapField(array(