From 5f99cad66916664d810655e5444cac917131e2ea Mon Sep 17 00:00:00 2001 From: Jan Langer Date: Fri, 6 Oct 2017 17:18:56 +0200 Subject: [PATCH 1/7] Failing test - inverse side OneToOne loaded without identifier condition --- ...eToOneInverseSideLoadAfterDqlQueryTest.php | 109 ++++++++++++++++++ 1 file changed, 109 insertions(+) create mode 100644 tests/Doctrine/Tests/ORM/Functional/OneToOneInverseSideLoadAfterDqlQueryTest.php diff --git a/tests/Doctrine/Tests/ORM/Functional/OneToOneInverseSideLoadAfterDqlQueryTest.php b/tests/Doctrine/Tests/ORM/Functional/OneToOneInverseSideLoadAfterDqlQueryTest.php new file mode 100644 index 000000000..34439c373 --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Functional/OneToOneInverseSideLoadAfterDqlQueryTest.php @@ -0,0 +1,109 @@ +_em); + try { + $schemaTool->createSchema( + [ + $this->_em->getClassMetadata(Bus::class), + $this->_em->getClassMetadata(BusOwner::class), + ] + ); + } catch(\Exception $e) {} + } + + public function testInverseSideOneToOneLoadedAfterDqlQuery(): void + { + $owner = new BusOwner('Alexander'); + $bus = new Bus($owner); + + $this->_em->persist($bus); + $this->_em->flush(); + $this->_em->clear(); + + $bus = $this->_em->createQueryBuilder() + ->select('to') + ->from(BusOwner::class, 'to') + ->andWhere('to.id = :id') + ->setParameter('id', $owner->id) + ->getQuery() + ->getResult(); + + $this->assertSQLEquals( + "SELECT b0_.id AS id_0, b0_.name AS name_1 FROM BusOwner b0_ WHERE b0_.id = ?", + $this->_sqlLoggerStack->queries[$this->_sqlLoggerStack->currentQuery - 1]['sql'] + ); + + $this->assertSQLEquals( + "SELECT t0.id AS id_1, t0.owner AS owner_2 FROM Bus t0 WHERE t0.owner = ?", + $this->_sqlLoggerStack->queries[$this->_sqlLoggerStack->currentQuery]['sql'] + ); + } + +} + + +/** + * @Entity + */ +class Bus +{ + /** + * @id @column(type="integer") @generatedValue + * @var int + */ + public $id; + /** + * Owning side + * @OneToOne(targetEntity="BusOwner", inversedBy="bus", cascade={"persist"}) + * @JoinColumn(nullable=false, name="owner") + */ + public $owner; + + public function __construct(BusOwner $owner) + { + $this->owner = $owner; + } + +} + +/** + * @Entity + */ +class BusOwner +{ + /** + * @Id + * @Column(type="integer") + * @GeneratedValue + */ + public $id; + + /** @column(type="string") */ + public $name; + /** + * Inverse side + * @OneToOne(targetEntity="Bus", mappedBy="owner") + */ + public $bus; + + public function __construct($name) + { + $this->name = $name; + } + + public function setBus(Bus $t) + { + $this->bus = $t; + } +} From 6ba2d1c3175ec205091968a5158f3ab2a954905a Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sat, 7 Oct 2017 10:09:44 +0200 Subject: [PATCH 2/7] #6759 cleaning up test case body --- ...eToOneInverseSideLoadAfterDqlQueryTest.php | 25 +++++++++++-------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/tests/Doctrine/Tests/ORM/Functional/OneToOneInverseSideLoadAfterDqlQueryTest.php b/tests/Doctrine/Tests/ORM/Functional/OneToOneInverseSideLoadAfterDqlQueryTest.php index 34439c373..8bfadc312 100644 --- a/tests/Doctrine/Tests/ORM/Functional/OneToOneInverseSideLoadAfterDqlQueryTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/OneToOneInverseSideLoadAfterDqlQueryTest.php @@ -3,6 +3,7 @@ namespace Doctrine\Tests\ORM\Functional; use Doctrine\ORM\Tools\SchemaTool; +use Doctrine\ORM\Tools\ToolsException; use Doctrine\Tests\OrmFunctionalTestCase; class OneToOneInverseSideLoadAfterDqlQueryTest extends OrmFunctionalTestCase @@ -11,15 +12,15 @@ class OneToOneInverseSideLoadAfterDqlQueryTest extends OrmFunctionalTestCase protected function setUp() { parent::setUp(); - $schemaTool = new SchemaTool($this->_em); + try { - $schemaTool->createSchema( - [ - $this->_em->getClassMetadata(Bus::class), - $this->_em->getClassMetadata(BusOwner::class), - ] - ); - } catch(\Exception $e) {} + $this->_schemaTool->createSchema([ + $this->_em->getClassMetadata(Bus::class), + $this->_em->getClassMetadata(BusOwner::class), + ]); + } catch(ToolsException $e) { + // ignored + } } public function testInverseSideOneToOneLoadedAfterDqlQuery(): void @@ -31,7 +32,9 @@ class OneToOneInverseSideLoadAfterDqlQueryTest extends OrmFunctionalTestCase $this->_em->flush(); $this->_em->clear(); - $bus = $this->_em->createQueryBuilder() + $bus = $this + ->_em + ->createQueryBuilder() ->select('to') ->from(BusOwner::class, 'to') ->andWhere('to.id = :id') @@ -40,12 +43,12 @@ class OneToOneInverseSideLoadAfterDqlQueryTest extends OrmFunctionalTestCase ->getResult(); $this->assertSQLEquals( - "SELECT b0_.id AS id_0, b0_.name AS name_1 FROM BusOwner b0_ WHERE b0_.id = ?", + 'SELECT b0_.id AS id_0, b0_.name AS name_1 FROM BusOwner b0_ WHERE b0_.id = ?', $this->_sqlLoggerStack->queries[$this->_sqlLoggerStack->currentQuery - 1]['sql'] ); $this->assertSQLEquals( - "SELECT t0.id AS id_1, t0.owner AS owner_2 FROM Bus t0 WHERE t0.owner = ?", + 'SELECT t0.id AS id_1, t0.owner AS owner_2 FROM Bus t0 WHERE t0.owner = ?', $this->_sqlLoggerStack->queries[$this->_sqlLoggerStack->currentQuery]['sql'] ); } From d831f4fd9f963a1c88da171d86a9dca675ef43d6 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sat, 7 Oct 2017 12:48:19 +0200 Subject: [PATCH 3/7] #6759 segregating test models into their own namespace --- .../OneToOneInverseSideLoad/InverseSide.php | 26 +++++++++++++++++ .../OneToOneInverseSideLoad/OwningSide.php | 29 +++++++++++++++++++ 2 files changed, 55 insertions(+) create mode 100644 tests/Doctrine/Tests/Models/OneToOneInverseSideLoad/InverseSide.php create mode 100644 tests/Doctrine/Tests/Models/OneToOneInverseSideLoad/OwningSide.php diff --git a/tests/Doctrine/Tests/Models/OneToOneInverseSideLoad/InverseSide.php b/tests/Doctrine/Tests/Models/OneToOneInverseSideLoad/InverseSide.php new file mode 100644 index 000000000..1b7d844b2 --- /dev/null +++ b/tests/Doctrine/Tests/Models/OneToOneInverseSideLoad/InverseSide.php @@ -0,0 +1,26 @@ + Date: Sat, 7 Oct 2017 12:52:34 +0200 Subject: [PATCH 4/7] #6759 cleaning up test case, using new models from the isolated namespace --- ...eToOneInverseSideLoadAfterDqlQueryTest.php | 102 ++++++------------ 1 file changed, 30 insertions(+), 72 deletions(-) diff --git a/tests/Doctrine/Tests/ORM/Functional/OneToOneInverseSideLoadAfterDqlQueryTest.php b/tests/Doctrine/Tests/ORM/Functional/OneToOneInverseSideLoadAfterDqlQueryTest.php index 8bfadc312..77cb937f4 100644 --- a/tests/Doctrine/Tests/ORM/Functional/OneToOneInverseSideLoadAfterDqlQueryTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/OneToOneInverseSideLoadAfterDqlQueryTest.php @@ -1,112 +1,70 @@ _schemaTool->createSchema([ - $this->_em->getClassMetadata(Bus::class), - $this->_em->getClassMetadata(BusOwner::class), + $this->_em->getClassMetadata(OwningSide::class), + $this->_em->getClassMetadata(InverseSide::class), ]); } catch(ToolsException $e) { // ignored } } + /** + * @group #6759 + */ public function testInverseSideOneToOneLoadedAfterDqlQuery(): void { - $owner = new BusOwner('Alexander'); - $bus = new Bus($owner); + $owner = new OwningSide(); + $inverse = new InverseSide(); - $this->_em->persist($bus); + $owner->id = 'owner'; + $inverse->id = 'inverse'; + $owner->inverse = $inverse; + $inverse->owning = $owner; + + $this->_em->persist($owner); + $this->_em->persist($inverse); $this->_em->flush(); $this->_em->clear(); - $bus = $this + /* @var $fetchedInverse InverseSide */ + $fetchedInverse = $this ->_em ->createQueryBuilder() - ->select('to') - ->from(BusOwner::class, 'to') - ->andWhere('to.id = :id') - ->setParameter('id', $owner->id) + ->select('inverse') + ->from(InverseSide::class, 'inverse') + ->andWhere('inverse.id = :id') + ->setParameter('id', 'inverse') ->getQuery() - ->getResult(); + ->getSingleResult(); + + self::assertInstanceOf(InverseSide::class, $fetchedInverse); + self::assertInstanceOf(OwningSide::class, $fetchedInverse->owning); $this->assertSQLEquals( - 'SELECT b0_.id AS id_0, b0_.name AS name_1 FROM BusOwner b0_ WHERE b0_.id = ?', + 'select o0_.id as id_0 from one_to_one_inverse_side_load_inverse o0_ where o0_.id = ?', $this->_sqlLoggerStack->queries[$this->_sqlLoggerStack->currentQuery - 1]['sql'] ); $this->assertSQLEquals( - 'SELECT t0.id AS id_1, t0.owner AS owner_2 FROM Bus t0 WHERE t0.owner = ?', + 'select t0.id as id_1, t0.inverse as inverse_2 from one_to_one_inverse_side_load_owning t0 WHERE t0.inverse = ?', $this->_sqlLoggerStack->queries[$this->_sqlLoggerStack->currentQuery]['sql'] ); } - -} - - -/** - * @Entity - */ -class Bus -{ - /** - * @id @column(type="integer") @generatedValue - * @var int - */ - public $id; - /** - * Owning side - * @OneToOne(targetEntity="BusOwner", inversedBy="bus", cascade={"persist"}) - * @JoinColumn(nullable=false, name="owner") - */ - public $owner; - - public function __construct(BusOwner $owner) - { - $this->owner = $owner; - } - -} - -/** - * @Entity - */ -class BusOwner -{ - /** - * @Id - * @Column(type="integer") - * @GeneratedValue - */ - public $id; - - /** @column(type="string") */ - public $name; - /** - * Inverse side - * @OneToOne(targetEntity="Bus", mappedBy="owner") - */ - public $bus; - - public function __construct($name) - { - $this->name = $name; - } - - public function setBus(Bus $t) - { - $this->bus = $t; - } } From dd12ba88ee8fb43222e1dac73c93d5f768254c2d Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sat, 7 Oct 2017 12:54:36 +0200 Subject: [PATCH 5/7] #6759 avoiding reuse of the `$identifier` variable when constructing an identifier from the owning side value Fixes #6759 --- .../ORM/Persisters/Entity/BasicEntityPersister.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php b/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php index b9fb045e9..6490d0bae 100644 --- a/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php +++ b/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php @@ -792,6 +792,8 @@ class BasicEntityPersister implements EntityPersister $sourceClass = $this->em->getClassMetadata($assoc['sourceEntity']); $owningAssoc = $targetClass->getAssociationMapping($assoc['mappedBy']); + $computedIdentifier = []; + // TRICKY: since the association is specular source and target are flipped foreach ($owningAssoc['targetToSourceKeyColumns'] as $sourceKeyColumn => $targetKeyColumn) { if ( ! isset($sourceClass->fieldNames[$sourceKeyColumn])) { @@ -802,13 +804,11 @@ class BasicEntityPersister implements EntityPersister // unset the old value and set the new sql aliased value here. By definition // unset($identifier[$targetKeyColumn] works here with how UnitOfWork::createEntity() calls this method. - $identifier[$targetClass->getFieldForColumn($targetKeyColumn)] = + $computedIdentifier[$targetClass->getFieldForColumn($targetKeyColumn)] = $sourceClass->reflFields[$sourceClass->fieldNames[$sourceKeyColumn]]->getValue($sourceEntity); - - unset($identifier[$targetKeyColumn]); } - $targetEntity = $this->load($identifier, null, $assoc); + $targetEntity = $this->load($computedIdentifier, null, $assoc); if ($targetEntity !== null) { $targetClass->setFieldValue($targetEntity, $assoc['mappedBy'], $sourceEntity); From 3dd7eb58888eb44d56de81f8fde284cd9f3dc64e Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sun, 8 Oct 2017 10:57:33 +0200 Subject: [PATCH 6/7] #6759 removing outdated comment as per @alcaeus' review Ref: https://github.com/doctrine/doctrine2/pull/6760#discussion_r143347881 --- lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php b/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php index 6490d0bae..6152c4176 100644 --- a/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php +++ b/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php @@ -802,8 +802,6 @@ class BasicEntityPersister implements EntityPersister ); } - // unset the old value and set the new sql aliased value here. By definition - // unset($identifier[$targetKeyColumn] works here with how UnitOfWork::createEntity() calls this method. $computedIdentifier[$targetClass->getFieldForColumn($targetKeyColumn)] = $sourceClass->reflFields[$sourceClass->fieldNames[$sourceKeyColumn]]->getValue($sourceEntity); } From 66f903a38fe369d8032626167fff7c7df2ba2294 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sun, 8 Oct 2017 14:26:45 +0200 Subject: [PATCH 7/7] #6759 remove `#` symbol from `@group` annotation as per @lcobucci's review Ref: https://github.com/doctrine/doctrine2/pull/6760#discussion_r143353225 --- .../ORM/Functional/OneToOneInverseSideLoadAfterDqlQueryTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Doctrine/Tests/ORM/Functional/OneToOneInverseSideLoadAfterDqlQueryTest.php b/tests/Doctrine/Tests/ORM/Functional/OneToOneInverseSideLoadAfterDqlQueryTest.php index 77cb937f4..85e518efa 100644 --- a/tests/Doctrine/Tests/ORM/Functional/OneToOneInverseSideLoadAfterDqlQueryTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/OneToOneInverseSideLoadAfterDqlQueryTest.php @@ -26,7 +26,7 @@ class OneToOneInverseSideLoadAfterDqlQueryTest extends OrmFunctionalTestCase } /** - * @group #6759 + * @group 6759 */ public function testInverseSideOneToOneLoadedAfterDqlQuery(): void {