diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index c54d97a0b..7ac24cc60 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -1398,12 +1398,13 @@ class UnitOfWork implements PropertyChangedListener public function addToIdentityMap($entity) { $classMetadata = $this->em->getClassMetadata(get_class($entity)); - $idHash = implode(' ', $this->entityIdentifiers[spl_object_hash($entity)]); + $identifier = $this->entityIdentifiers[spl_object_hash($entity)]; - if ($idHash === '') { + if (empty($identifier) || in_array(null, $identifier, true)) { throw ORMInvalidArgumentException::entityWithoutIdentity($classMetadata->name, $entity); } + $idHash = implode(' ', $identifier); $className = $classMetadata->rootEntityName; if (isset($this->identityMap[$className][$idHash])) { diff --git a/tests/Doctrine/Tests/Models/Cache/Action.php b/tests/Doctrine/Tests/Models/Cache/Action.php index 25609abed..d23ab494b 100644 --- a/tests/Doctrine/Tests/Models/Cache/Action.php +++ b/tests/Doctrine/Tests/Models/Cache/Action.php @@ -14,13 +14,8 @@ class Action /** * @Id - * @GeneratedValue - * @Column(type="integer") - */ - public $id; - - /** - * @Column + * @Column(type="string") + * @GeneratedValue(strategy="NONE") */ public $name; diff --git a/tests/Doctrine/Tests/Models/Cache/ComplexAction.php b/tests/Doctrine/Tests/Models/Cache/ComplexAction.php index 46d0414d3..5f5faf6dd 100644 --- a/tests/Doctrine/Tests/Models/Cache/ComplexAction.php +++ b/tests/Doctrine/Tests/Models/Cache/ComplexAction.php @@ -20,14 +20,14 @@ class ComplexAction /** * @Id * @OneToOne(targetEntity="Action", cascade={"persist", "remove"}) - * @JoinColumn(name="action1_id", referencedColumnName="id") + * @JoinColumn(name="action1_name", referencedColumnName="name") */ public $action1; /** * @Id * @OneToOne(targetEntity="Action", cascade={"persist", "remove"}) - * @JoinColumn(name="action2_id", referencedColumnName="id") + * @JoinColumn(name="action2_name", referencedColumnName="name") */ public $action2; diff --git a/tests/Doctrine/Tests/Models/Cache/Token.php b/tests/Doctrine/Tests/Models/Cache/Token.php index f6c712e36..ba3fdb182 100644 --- a/tests/Doctrine/Tests/Models/Cache/Token.php +++ b/tests/Doctrine/Tests/Models/Cache/Token.php @@ -36,7 +36,7 @@ class Token /** * @ManyToOne(targetEntity="Action", cascade={"persist", "remove"}, inversedBy="tokens") - * @JoinColumn(name="action_id", referencedColumnName="id") + * @JoinColumn(name="action_name", referencedColumnName="name") * @var array */ public $action; @@ -44,8 +44,8 @@ class Token /** * @ManyToOne(targetEntity="ComplexAction", cascade={"persist", "remove"}, inversedBy="tokens") * @JoinColumns({ - * @JoinColumn(name="complex_action1_id", referencedColumnName="action1_id"), - * @JoinColumn(name="complex_action2_id", referencedColumnName="action2_id") + * @JoinColumn(name="complex_action1_name", referencedColumnName="action1_name"), + * @JoinColumn(name="complex_action2_name", referencedColumnName="action2_name") * }) * @var ComplexAction */ diff --git a/tests/Doctrine/Tests/ORM/Functional/SecondLevelCacheManyToOneTest.php b/tests/Doctrine/Tests/ORM/Functional/SecondLevelCacheManyToOneTest.php index 00fa40973..57215f8e0 100644 --- a/tests/Doctrine/Tests/ORM/Functional/SecondLevelCacheManyToOneTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/SecondLevelCacheManyToOneTest.php @@ -193,7 +193,7 @@ class SecondLevelCacheManyToOneTest extends SecondLevelCacheAbstractTest $this->_em->clear(); $this->assertTrue($this->cache->containsEntity(Token::CLASSNAME, $token->token)); - $this->assertFalse($this->cache->containsEntity(Token::CLASSNAME, $action->id)); + $this->assertFalse($this->cache->containsEntity(Token::CLASSNAME, $action->name)); $queryCount = $this->getCurrentQueryCount(); $entity = $this->_em->find(Token::CLASSNAME, $token->token); @@ -204,7 +204,7 @@ class SecondLevelCacheManyToOneTest extends SecondLevelCacheAbstractTest $this->assertInstanceOf(Action::CLASSNAME, $entity->getAction()); $this->assertEquals('exec', $entity->getAction()->name); - $this->assertEquals($queryCount + 1, $this->getCurrentQueryCount()); + $this->assertEquals($queryCount, $this->getCurrentQueryCount()); } public function testPutAndLoadNonCacheableCompositeManyToOne() @@ -231,9 +231,9 @@ class SecondLevelCacheManyToOneTest extends SecondLevelCacheAbstractTest $this->_em->clear(); $this->assertTrue($this->cache->containsEntity(Token::CLASSNAME, $token->token)); - $this->assertFalse($this->cache->containsEntity(Action::CLASSNAME, $action1->id)); - $this->assertFalse($this->cache->containsEntity(Action::CLASSNAME, $action2->id)); - $this->assertFalse($this->cache->containsEntity(Action::CLASSNAME, $action3->id)); + $this->assertFalse($this->cache->containsEntity(Action::CLASSNAME, $action1->name)); + $this->assertFalse($this->cache->containsEntity(Action::CLASSNAME, $action2->name)); + $this->assertFalse($this->cache->containsEntity(Action::CLASSNAME, $action3->name)); $queryCount = $this->getCurrentQueryCount(); /** @@ -255,8 +255,8 @@ class SecondLevelCacheManyToOneTest extends SecondLevelCacheAbstractTest $this->assertEquals($queryCount + 1, $this->getCurrentQueryCount()); $this->assertEquals('login', $entity->getComplexAction()->getAction1()->name); - $this->assertEquals($queryCount + 2, $this->getCurrentQueryCount()); + $this->assertEquals($queryCount + 1, $this->getCurrentQueryCount()); $this->assertEquals('rememberme', $entity->getComplexAction()->getAction2()->name); - $this->assertEquals($queryCount + 3, $this->getCurrentQueryCount()); + $this->assertEquals($queryCount + 1, $this->getCurrentQueryCount()); } } \ No newline at end of file diff --git a/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php b/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php index 9b0c6a8f5..6a2c882e8 100644 --- a/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php +++ b/tests/Doctrine/Tests/ORM/UnitOfWorkTest.php @@ -6,6 +6,7 @@ use Doctrine\Common\Collections\ArrayCollection; use Doctrine\Common\NotifyPropertyChanged; use Doctrine\Common\PropertyChangedListener; use Doctrine\ORM\Mapping\ClassMetadata; +use Doctrine\ORM\ORMInvalidArgumentException; use Doctrine\ORM\UnitOfWork; use Doctrine\Tests\Mocks\ConnectionMock; use Doctrine\Tests\Mocks\DriverMock; @@ -363,6 +364,101 @@ class UnitOfWorkTest extends OrmTestCase [new ArrayCollection()], ]; } + + /** + * @dataProvider entitiesWithValidIdentifiersProvider + * + * @param object $entity + * @param string $idHash + * + * @return void + */ + public function testAddToIdentityMapValidIdentifiers($entity, $idHash) + { + $this->_unitOfWork->persist($entity); + $this->_unitOfWork->addToIdentityMap($entity); + + self::assertSame($entity, $this->_unitOfWork->getByIdHash($idHash, get_class($entity))); + } + + public function entitiesWithValidIdentifiersProvider() + { + $emptyString = new EntityWithStringIdentifier(); + + $emptyString->id = ''; + + $nonEmptyString = new EntityWithStringIdentifier(); + + $nonEmptyString->id = uniqid('id', true); + + $emptyStrings = new EntityWithCompositeStringIdentifier(); + + $emptyStrings->id1 = ''; + $emptyStrings->id2 = ''; + + $nonEmptyStrings = new EntityWithCompositeStringIdentifier(); + + $nonEmptyStrings->id1 = uniqid('id1', true); + $nonEmptyStrings->id2 = uniqid('id2', true); + + $booleanTrue = new EntityWithBooleanIdentifier(); + + $booleanTrue->id = true; + + $booleanFalse = new EntityWithBooleanIdentifier(); + + $booleanFalse->id = false; + + return [ + 'empty string, single field' => [$emptyString, ''], + 'non-empty string, single field' => [$nonEmptyString, $nonEmptyString->id], + 'empty strings, two fields' => [$emptyStrings, ' '], + 'non-empty strings, two fields' => [$nonEmptyStrings, $nonEmptyStrings->id1 . ' ' . $nonEmptyStrings->id2], + 'boolean true' => [$booleanTrue, '1'], + 'boolean false' => [$booleanFalse, ''], + ]; + } + + public function testRegisteringAManagedInstanceRequiresANonEmptyIdentifier() + { + $this->expectException(ORMInvalidArgumentException::class); + + $this->_unitOfWork->registerManaged(new EntityWithBooleanIdentifier(), [], []); + } + + /** + * @dataProvider entitiesWithInvalidIdentifiersProvider + * + * @param object $entity + * @param array $identifier + * + * @return void + */ + public function testAddToIdentityMapInvalidIdentifiers($entity, array $identifier) + { + $this->expectException(ORMInvalidArgumentException::class); + + $this->_unitOfWork->registerManaged($entity, $identifier, []); + } + + + public function entitiesWithInvalidIdentifiersProvider() + { + $firstNullString = new EntityWithCompositeStringIdentifier(); + + $firstNullString->id2 = uniqid('id2', true); + + $secondNullString = new EntityWithCompositeStringIdentifier(); + + $secondNullString->id1 = uniqid('id1', true); + + return [ + 'null string, single field' => [new EntityWithStringIdentifier(), ['id' => null]], + 'null strings, two fields' => [new EntityWithCompositeStringIdentifier(), ['id1' => null, 'id2' => null]], + 'first null string, two fields' => [$firstNullString, ['id1' => null, 'id2' => $firstNullString->id2]], + 'second null string, two fields' => [$secondNullString, ['id1' => $secondNullString->id1, 'id2' => null]], + ]; + } } /** @@ -469,3 +565,43 @@ class VersionedAssignedIdentifierEntity */ public $version; } + +/** @Entity */ +class EntityWithStringIdentifier +{ + /** + * @Id @Column(type="string") + * + * @var string|null + */ + public $id; +} + +/** @Entity */ +class EntityWithBooleanIdentifier +{ + /** + * @Id @Column(type="boolean") + * + * @var bool|null + */ + public $id; +} + +/** @Entity */ +class EntityWithCompositeStringIdentifier +{ + /** + * @Id @Column(type="string") + * + * @var string|null + */ + public $id1; + + /** + * @Id @Column(type="string") + * + * @var string|null + */ + public $id2; +} diff --git a/tests/Doctrine/Tests/OrmFunctionalTestCase.php b/tests/Doctrine/Tests/OrmFunctionalTestCase.php index d18b94965..6e17a92ad 100644 --- a/tests/Doctrine/Tests/OrmFunctionalTestCase.php +++ b/tests/Doctrine/Tests/OrmFunctionalTestCase.php @@ -460,8 +460,8 @@ abstract class OrmFunctionalTestCase extends OrmTestCase $conn->executeUpdate('DELETE FROM cache_state'); $conn->executeUpdate('DELETE FROM cache_country'); $conn->executeUpdate('DELETE FROM cache_login'); - $conn->executeUpdate('DELETE FROM cache_complex_action'); $conn->executeUpdate('DELETE FROM cache_token'); + $conn->executeUpdate('DELETE FROM cache_complex_action'); $conn->executeUpdate('DELETE FROM cache_action'); $conn->executeUpdate('DELETE FROM cache_client'); }