From 4058ad39585eaa77421697419d390df5d0dd29e6 Mon Sep 17 00:00:00 2001 From: Adrien Brault Date: Fri, 7 Nov 2014 14:14:54 +0100 Subject: [PATCH 1/6] Add test exposing UnitOfWork merge bug --- .../Functional/MergeCompositeToOneKeyTest.php | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 tests/Doctrine/Tests/ORM/Functional/MergeCompositeToOneKeyTest.php diff --git a/tests/Doctrine/Tests/ORM/Functional/MergeCompositeToOneKeyTest.php b/tests/Doctrine/Tests/ORM/Functional/MergeCompositeToOneKeyTest.php new file mode 100644 index 000000000..007d55fe6 --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Functional/MergeCompositeToOneKeyTest.php @@ -0,0 +1,62 @@ +_schemaTool->createSchema(array( + $this->_em->getClassMetadata(__NAMESPACE__ . '\MergeCompositeToOneKeyCountry'), + $this->_em->getClassMetadata(__NAMESPACE__ . '\MergeCompositeToOneKeyState'), + )); + } + + public function testIssue() + { + $country = new MergeCompositeToOneKeyCountry(); + $country->country = 'US'; + $state = new MergeCompositeToOneKeyState(); + $state->state = 'CA'; + $state->country = $country; + + $this->_em->merge($country); + $this->_em->merge($state); + } +} + +/** + * @Entity + */ +class MergeCompositeToOneKeyCountry +{ + /** + * @Id + * @Column(type="string", name="country") + * @GeneratedValue(strategy="NONE") + */ + public $country; +} + +/** + * @Entity + */ +class MergeCompositeToOneKeyState +{ + /** + * @Id + * @Column(type="string") + * @GeneratedValue(strategy="NONE") + */ + public $state; + + /** + * @Id + * @ManyToOne(targetEntity="MergeCompositeToOneKeyCountry") + * @JoinColumn(name="country", referencedColumnName="country") + */ + public $country; +} From fc8191f557203c447963dab9b1a63a65a3d9bcf9 Mon Sep 17 00:00:00 2001 From: Adrien Brault Date: Fri, 7 Nov 2014 15:00:50 +0100 Subject: [PATCH 2/6] Naive fix --- lib/Doctrine/ORM/Id/AssignedGenerator.php | 6 +----- lib/Doctrine/ORM/Utility/IdentifierFlattener.php | 4 +++- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/lib/Doctrine/ORM/Id/AssignedGenerator.php b/lib/Doctrine/ORM/Id/AssignedGenerator.php index 3ad2cbff7..9bdaa0efc 100644 --- a/lib/Doctrine/ORM/Id/AssignedGenerator.php +++ b/lib/Doctrine/ORM/Id/AssignedGenerator.php @@ -55,12 +55,8 @@ class AssignedGenerator extends AbstractIdGenerator } if (isset($class->associationMappings[$idField])) { - if ( ! $em->getUnitOfWork()->isInIdentityMap($value)) { - throw ORMException::entityMissingForeignAssignedId($entity, $value); - } - // NOTE: Single Columns as associated identifiers only allowed - this constraint it is enforced. - $value = current($em->getUnitOfWork()->getEntityIdentifier($value)); + $value = $em->getUnitOfWork()->getSingleIdentifierValue($value); } $identifier[$idField] = $value; diff --git a/lib/Doctrine/ORM/Utility/IdentifierFlattener.php b/lib/Doctrine/ORM/Utility/IdentifierFlattener.php index bb84c6151..d16da3255 100644 --- a/lib/Doctrine/ORM/Utility/IdentifierFlattener.php +++ b/lib/Doctrine/ORM/Utility/IdentifierFlattener.php @@ -76,7 +76,9 @@ final class IdentifierFlattener $class->associationMappings[$idField]['targetEntity'] ); - $associatedId = $this->unitOfWork->getEntityIdentifier($idValue); + $associatedId = $this->unitOfWork->isInIdentityMap($idValue) + ? $this->unitOfWork->getEntityIdentifier($idValue) + : $targetClassMetadata->getIdentifierValues($idValue); $flatId[$idField] = $associatedId[$targetClassMetadata->identifier[0]]; } else { From f189c1aaf042dbcff5fd544597165d2aca9a768e Mon Sep 17 00:00:00 2001 From: Adrien Brault Date: Fri, 7 Nov 2014 15:19:37 +0100 Subject: [PATCH 3/6] Update test to valid use case --- .../Tests/ORM/Functional/MergeCompositeToOneKeyTest.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/Doctrine/Tests/ORM/Functional/MergeCompositeToOneKeyTest.php b/tests/Doctrine/Tests/ORM/Functional/MergeCompositeToOneKeyTest.php index 007d55fe6..99c2f349d 100644 --- a/tests/Doctrine/Tests/ORM/Functional/MergeCompositeToOneKeyTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/MergeCompositeToOneKeyTest.php @@ -23,7 +23,6 @@ class MergeCompositeToOneKeyTest extends \Doctrine\Tests\OrmFunctionalTestCase $state->state = 'CA'; $state->country = $country; - $this->_em->merge($country); $this->_em->merge($state); } } @@ -55,7 +54,7 @@ class MergeCompositeToOneKeyState /** * @Id - * @ManyToOne(targetEntity="MergeCompositeToOneKeyCountry") + * @ManyToOne(targetEntity="MergeCompositeToOneKeyCountry", cascade={"MERGE"}) * @JoinColumn(name="country", referencedColumnName="country") */ public $country; From 8987c9ab37e89b54f802529a9b24dc39b93e403b Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Thu, 22 Jan 2015 12:07:42 +0100 Subject: [PATCH 4/6] #1176 DDC-3378 - moved test asset entities to proper models directory --- .../CompositeToOneKeyState.php | 23 +++++++++++++++++++ .../Models/MixedToOneIdentity/Country.php | 12 ++++++++++ 2 files changed, 35 insertions(+) create mode 100644 tests/Doctrine/Tests/Models/MixedToOneIdentity/CompositeToOneKeyState.php create mode 100644 tests/Doctrine/Tests/Models/MixedToOneIdentity/Country.php diff --git a/tests/Doctrine/Tests/Models/MixedToOneIdentity/CompositeToOneKeyState.php b/tests/Doctrine/Tests/Models/MixedToOneIdentity/CompositeToOneKeyState.php new file mode 100644 index 000000000..b4b02e158 --- /dev/null +++ b/tests/Doctrine/Tests/Models/MixedToOneIdentity/CompositeToOneKeyState.php @@ -0,0 +1,23 @@ + Date: Thu, 22 Jan 2015 12:10:39 +0100 Subject: [PATCH 5/6] #1176 DDC-3378 - refactored test logic to use the newly introduced test assets --- .../Functional/MergeCompositeToOneKeyTest.php | 65 +++++++------------ 1 file changed, 24 insertions(+), 41 deletions(-) diff --git a/tests/Doctrine/Tests/ORM/Functional/MergeCompositeToOneKeyTest.php b/tests/Doctrine/Tests/ORM/Functional/MergeCompositeToOneKeyTest.php index 99c2f349d..6c6198520 100644 --- a/tests/Doctrine/Tests/ORM/Functional/MergeCompositeToOneKeyTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/MergeCompositeToOneKeyTest.php @@ -2,60 +2,43 @@ namespace Doctrine\Tests\ORM\Functional; -use Doctrine\ORM\Mapping\JoinColumn; +use Doctrine\Tests\Models\MixedToOneIdentity\CompositeToOneKeyState; +use Doctrine\Tests\Models\MixedToOneIdentity\Country; class MergeCompositeToOneKeyTest extends \Doctrine\Tests\OrmFunctionalTestCase { + /** + * {@inheritDoc} + */ protected function setUp() { parent::setUp(); + $this->_schemaTool->createSchema(array( - $this->_em->getClassMetadata(__NAMESPACE__ . '\MergeCompositeToOneKeyCountry'), - $this->_em->getClassMetadata(__NAMESPACE__ . '\MergeCompositeToOneKeyState'), + $this->_em->getClassMetadata(Country::CLASSNAME), + $this->_em->getClassMetadata(CompositeToOneKeyState::CLASSNAME), )); } - public function testIssue() + /** + * @group DDC-3378 + * @group 1176 + */ + public function testMergingOfEntityWithCompositeIdentifierContainingToOneAssociation() { - $country = new MergeCompositeToOneKeyCountry(); + $country = new Country(); $country->country = 'US'; - $state = new MergeCompositeToOneKeyState(); - $state->state = 'CA'; + + $state = new CompositeToOneKeyState(); + $state->state = 'CA'; $state->country = $country; - $this->_em->merge($state); + /* @var $merged CompositeToOneKeyState */ + $merged = $this->_em->merge($state); + + $this->assertInstanceOf(CompositeToOneKeyState::CLASSNAME, $state); + $this->assertNotSame($state, $merged); + $this->assertInstanceOf(Country::CLASSNAME, $merged->country); + $this->assertNotSame($country, $merged->country); } } - -/** - * @Entity - */ -class MergeCompositeToOneKeyCountry -{ - /** - * @Id - * @Column(type="string", name="country") - * @GeneratedValue(strategy="NONE") - */ - public $country; -} - -/** - * @Entity - */ -class MergeCompositeToOneKeyState -{ - /** - * @Id - * @Column(type="string") - * @GeneratedValue(strategy="NONE") - */ - public $state; - - /** - * @Id - * @ManyToOne(targetEntity="MergeCompositeToOneKeyCountry", cascade={"MERGE"}) - * @JoinColumn(name="country", referencedColumnName="country") - */ - public $country; -} From a67332fb51e0572b3d8edba4ae6a1c9b979ac8fb Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Thu, 22 Jan 2015 12:11:03 +0100 Subject: [PATCH 6/6] #1176 DDC-3378 - minor CS fixes (imports, spacing, IDE hints) --- lib/Doctrine/ORM/Id/AssignedGenerator.php | 3 +-- lib/Doctrine/ORM/Utility/IdentifierFlattener.php | 14 ++++++++------ 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/lib/Doctrine/ORM/Id/AssignedGenerator.php b/lib/Doctrine/ORM/Id/AssignedGenerator.php index 9bdaa0efc..447dbd6d5 100644 --- a/lib/Doctrine/ORM/Id/AssignedGenerator.php +++ b/lib/Doctrine/ORM/Id/AssignedGenerator.php @@ -40,8 +40,7 @@ class AssignedGenerator extends AbstractIdGenerator * * @throws \Doctrine\ORM\ORMException */ - public function generate( - EntityManager $em, $entity) + public function generate(EntityManager $em, $entity) { $class = $em->getClassMetadata(get_class($entity)); $idFields = $class->getIdentifierFieldNames(); diff --git a/lib/Doctrine/ORM/Utility/IdentifierFlattener.php b/lib/Doctrine/ORM/Utility/IdentifierFlattener.php index d16da3255..166e0acb6 100644 --- a/lib/Doctrine/ORM/Utility/IdentifierFlattener.php +++ b/lib/Doctrine/ORM/Utility/IdentifierFlattener.php @@ -36,22 +36,22 @@ final class IdentifierFlattener /** * The UnitOfWork used to coordinate object-level transactions. * - * @var \Doctrine\ORM\UnitOfWork + * @var UnitOfWork */ private $unitOfWork; /** * The metadata factory, used to retrieve the ORM metadata of entity classes. * - * @var \Doctrine\Common\Persistence\Mapping\ClassMetadataFactory + * @var ClassMetadataFactory */ private $metadataFactory; /** * Initializes a new IdentifierFlattener instance, bound to the given EntityManager. * - * @param \Doctrine\ORM\UnitOfWork $unitOfWork - * @param \Doctrine\Common\Persistence\Mapping\ClassMetadataFactory $metadataFactory + * @param UnitOfWork $unitOfWork + * @param ClassMetadataFactory $metadataFactory */ public function __construct(UnitOfWork $unitOfWork, ClassMetadataFactory $metadataFactory) { @@ -62,8 +62,9 @@ final class IdentifierFlattener /** * convert foreign identifiers into scalar foreign key values to avoid object to string conversion failures. * - * @param \Doctrine\ORM\Mapping\ClassMetadata $class - * @param array $id + * @param ClassMetadata $class + * @param array $id + * * @return array */ public function flattenIdentifier(ClassMetadata $class, array $id) @@ -72,6 +73,7 @@ final class IdentifierFlattener foreach ($id as $idField => $idValue) { if (isset($class->associationMappings[$idField]) && is_object($idValue)) { + /* @var $targetClassMetadata ClassMetadata */ $targetClassMetadata = $this->metadataFactory->getMetadataFor( $class->associationMappings[$idField]['targetEntity'] );