From 16a14f223839f173b5058d4ac35915e30c6c659d Mon Sep 17 00:00:00 2001
From: Gabe van der Weijde <gabe.vanderweijde@triasinformatica.nl>
Date: Wed, 28 Jun 2017 20:40:25 +0200
Subject: [PATCH 1/8] -- Created test for validation issue #6499.
---
.../ORM/Functional/Ticket/DDC6499Test.php | 172 ++++++++++++++++++
1 file changed, 172 insertions(+)
create mode 100644 tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6499Test.php
diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6499Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6499Test.php
new file mode 100644
index 000000000..98b0e42a7
--- /dev/null
+++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6499Test.php
@@ -0,0 +1,172 @@
+<?php
+
+namespace Doctrine\Tests\Functional\Ticket;
+
+use Doctrine\Common\Collections\ArrayCollection;
+use Doctrine\Tests\OrmFunctionalTestCase;
+
+/**
+ * @group DDC-6499
+ */
+class DDC6499Test extends OrmFunctionalTestCase
+{
+ /**
+ * {@inheritDoc}
+ */
+ protected function setUp()
+ {
+ parent::setUp();
+
+ $this->_schemaTool->createSchema(
+ [
+ $this->_em->getClassMetadata(DDC6499A::class),
+ $this->_em->getClassMetadata(DDC6499B::class),
+ ]
+ );
+ }
+
+ /**
+ * Test for the bug described in issue #6499.
+ */
+ public function testIssue()
+ {
+ $a = new DDC6499A();
+ $this->_em->persist($a);
+
+ $b = new DDC6499B();
+ $a->setB($b);
+ $this->_em->persist($b);
+
+ $this->_em->flush();
+
+ // Issue #6499 will result in a Integrity constraint violation before reaching this point
+ $this->assertEquals(true, true);
+ }
+}
+
+/** @Entity */
+class DDC6499A
+{
+ /**
+ * @Id()
+ * @GeneratedValue(strategy="AUTO")
+ * @Column(name="id", type="integer")
+ */
+ private $id;
+
+ /**
+ * @OneToMany(targetEntity="DDC6499B", mappedBy="a", cascade={"persist", "remove"}, orphanRemoval=true)
+ */
+ private $bs;
+
+ /**
+ * @OneToOne(targetEntity="DDC6499B", cascade={"persist"})
+ * @JoinColumn(nullable=false)
+ */
+ private $b;
+
+ /**
+ * DDC6499A constructor.
+ */
+ public function __construct()
+ {
+ $this->bs = new ArrayCollection();
+ }
+
+ /**
+ * @return int
+ */
+ public function getId()
+ {
+ return $this->id;
+ }
+
+ /**
+ * @return DDC6499B[]|ArrayCollection
+ */
+ public function getBs()
+ {
+ return $this->bs;
+ }
+
+ /**
+ * @param DDC6499B $b
+ */
+ public function addB(DDC6499B $b)
+ {
+ if ($this->bs->contains($b)) return;
+
+ $this->bs->add($b);
+
+ // Update owning side
+ $b->setA($this);
+ }
+
+ /**
+ * @param DDC6499B $b
+ */
+ public function removeB(DDC6499B $b)
+ {
+ if (!$this->bs->contains($b)) return;
+
+ $this->bs->removeElement($b);
+
+ // Not updating owning side due to orphan removal
+ }
+
+ /**
+ * @return DDC6499B
+ */
+ public function getB()
+ {
+ return $this->b;
+ }
+
+ /**
+ * @param DDC6499B $b
+ */
+ public function setB(DDC6499B $b)
+ {
+ $this->b = $b;
+ }
+}
+
+/** @Entity */
+class DDC6499B
+{
+ /**
+ * @Id()
+ * @GeneratedValue(strategy="AUTO")
+ * @Column(name="id", type="integer")
+ */
+ private $id;
+
+ /**
+ * @ManyToOne(targetEntity="DDC6499A", inversedBy="bs", cascade={"persist"})
+ */
+ private $a;
+
+ /**
+ * @return int
+ */
+ public function getId()
+ {
+ return $this->id;
+ }
+
+ /**
+ * @return DDC6499A
+ */
+ public function getA()
+ {
+ return $this->a;
+ }
+
+ /**
+ * @param DDC6499A $a
+ */
+ public function setA(DDC6499A $a)
+ {
+ $this->a = $a;
+ }
+}
\ No newline at end of file
From db2530d6fdbda608d2eaada74c16d253c89eb59c Mon Sep 17 00:00:00 2001
From: Gabe van der Weijde <gabe.vanderweijde@triasinformatica.nl>
Date: Wed, 28 Jun 2017 20:55:58 +0200
Subject: [PATCH 2/8] -- Proposed fix due to logic error.
---
lib/Doctrine/ORM/UnitOfWork.php | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php
index c0b0dd8c3..0afabf805 100644
--- a/lib/Doctrine/ORM/UnitOfWork.php
+++ b/lib/Doctrine/ORM/UnitOfWork.php
@@ -1154,7 +1154,7 @@ class UnitOfWork implements PropertyChangedListener
$joinColumns = reset($assoc['joinColumns']);
- $calc->addDependency($targetClass->name, $class->name, (int)empty($joinColumns['nullable']));
+ $calc->addDependency($targetClass->name, $class->name, (int) ! ( ! isset($joinColumns['nullable']) || $joinColumns['nullable'] === true));
// If the target class has mapped subclasses, these share the same dependency.
if ( ! $targetClass->subClasses) {
From da3cd04993b107e6dff8e2e382d5769add8ed0b4 Mon Sep 17 00:00:00 2001
From: Gabe van der Weijde <gabe.vanderweijde@triasinformatica.nl>
Date: Thu, 29 Jun 2017 17:16:35 +0200
Subject: [PATCH 3/8] -- Transformed into a minimal example. -- Processed
Ocramius' feedback.
---
.../ORM/Functional/Ticket/DDC6499Test.php | 158 ++++++------------
1 file changed, 47 insertions(+), 111 deletions(-)
diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6499Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6499Test.php
index 98b0e42a7..3cee68cf8 100644
--- a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6499Test.php
+++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6499Test.php
@@ -13,7 +13,7 @@ class DDC6499Test extends OrmFunctionalTestCase
/**
* {@inheritDoc}
*/
- protected function setUp()
+ protected function setUp() : void
{
parent::setUp();
@@ -25,22 +25,56 @@ class DDC6499Test extends OrmFunctionalTestCase
);
}
+ /**
+ * {@inheritDoc}
+ */
+ protected function tearDown() : void
+ {
+ parent::tearDown();
+
+ $this->_schemaTool->dropSchema(
+ [
+ $this->_em->getClassMetadata(DDC6499A::class),
+ $this->_em->getClassMetadata(DDC6499B::class),
+ ]
+ );
+ }
+
/**
* Test for the bug described in issue #6499.
*/
- public function testIssue()
+ public function testIssue() : void
{
$a = new DDC6499A();
$this->_em->persist($a);
$b = new DDC6499B();
- $a->setB($b);
+ $a->b = $b;
$this->_em->persist($b);
$this->_em->flush();
+ $this->_em->clear();
- // Issue #6499 will result in a Integrity constraint violation before reaching this point
- $this->assertEquals(true, true);
+ self::assertEquals($this->_em->find(DDC6499A::class, $a->id)->b->id, $b->id, "Issue #6499 will result in a Integrity constraint violation before reaching this point.");
+ }
+
+ /**
+ * Test for the bug described in issue #6499 (reversed order).
+ */
+ public function testIssueReversed() : void
+ {
+ $a = new DDC6499A();
+
+ $b = new DDC6499B();
+ $a->b = $b;
+
+ $this->_em->persist($b);
+ $this->_em->persist($a);
+
+ $this->_em->flush();
+ $this->_em->clear();
+
+ self::assertEquals($this->_em->find(DDC6499A::class, $a->id)->b->id, $b->id, "Issue #6499 will result in a Integrity constraint violation before reaching this point.");
}
}
@@ -48,125 +82,27 @@ class DDC6499Test extends OrmFunctionalTestCase
class DDC6499A
{
/**
- * @Id()
- * @GeneratedValue(strategy="AUTO")
- * @Column(name="id", type="integer")
+ * @Id @Column(type="integer") @GeneratedValue
*/
- private $id;
+ public $id;
/**
- * @OneToMany(targetEntity="DDC6499B", mappedBy="a", cascade={"persist", "remove"}, orphanRemoval=true)
- */
- private $bs;
-
- /**
- * @OneToOne(targetEntity="DDC6499B", cascade={"persist"})
+ * @OneToOne(targetEntity="DDC6499B")
* @JoinColumn(nullable=false)
*/
- private $b;
-
- /**
- * DDC6499A constructor.
- */
- public function __construct()
- {
- $this->bs = new ArrayCollection();
- }
-
- /**
- * @return int
- */
- public function getId()
- {
- return $this->id;
- }
-
- /**
- * @return DDC6499B[]|ArrayCollection
- */
- public function getBs()
- {
- return $this->bs;
- }
-
- /**
- * @param DDC6499B $b
- */
- public function addB(DDC6499B $b)
- {
- if ($this->bs->contains($b)) return;
-
- $this->bs->add($b);
-
- // Update owning side
- $b->setA($this);
- }
-
- /**
- * @param DDC6499B $b
- */
- public function removeB(DDC6499B $b)
- {
- if (!$this->bs->contains($b)) return;
-
- $this->bs->removeElement($b);
-
- // Not updating owning side due to orphan removal
- }
-
- /**
- * @return DDC6499B
- */
- public function getB()
- {
- return $this->b;
- }
-
- /**
- * @param DDC6499B $b
- */
- public function setB(DDC6499B $b)
- {
- $this->b = $b;
- }
+ public $b;
}
/** @Entity */
class DDC6499B
{
/**
- * @Id()
- * @GeneratedValue(strategy="AUTO")
- * @Column(name="id", type="integer")
+ * @Id @Column(type="integer") @GeneratedValue
*/
- private $id;
+ public $id;
/**
- * @ManyToOne(targetEntity="DDC6499A", inversedBy="bs", cascade={"persist"})
+ * @ManyToOne(targetEntity="DDC6499A", inversedBy="bs")
*/
- private $a;
-
- /**
- * @return int
- */
- public function getId()
- {
- return $this->id;
- }
-
- /**
- * @return DDC6499A
- */
- public function getA()
- {
- return $this->a;
- }
-
- /**
- * @param DDC6499A $a
- */
- public function setA(DDC6499A $a)
- {
- $this->a = $a;
- }
+ public $a;
}
\ No newline at end of file
From b352cd3e222db4680a365a1408fafea27daabb90 Mon Sep 17 00:00:00 2001
From: Marco Pivetta <ocramius@gmail.com>
Date: Fri, 11 Aug 2017 21:54:30 +0200
Subject: [PATCH 4/8] #6499 #6533 minor CS fixes in the test
---
tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6499Test.php | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6499Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6499Test.php
index 3cee68cf8..4b27478c6 100644
--- a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6499Test.php
+++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6499Test.php
@@ -17,12 +17,10 @@ class DDC6499Test extends OrmFunctionalTestCase
{
parent::setUp();
- $this->_schemaTool->createSchema(
- [
+ $this->_schemaTool->createSchema([
$this->_em->getClassMetadata(DDC6499A::class),
$this->_em->getClassMetadata(DDC6499B::class),
- ]
- );
+ ]);
}
/**
From ebd521c56eef4a9ad9cec239997554182db1e9a4 Mon Sep 17 00:00:00 2001
From: Marco Pivetta <ocramius@gmail.com>
Date: Fri, 11 Aug 2017 21:55:02 +0200
Subject: [PATCH 5/8] #6499 #6533 minor CS fixes in the test
---
.../Tests/ORM/Functional/Ticket/DDC6499Test.php | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6499Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6499Test.php
index 4b27478c6..066c0b1b4 100644
--- a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6499Test.php
+++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6499Test.php
@@ -30,12 +30,10 @@ class DDC6499Test extends OrmFunctionalTestCase
{
parent::tearDown();
- $this->_schemaTool->dropSchema(
- [
- $this->_em->getClassMetadata(DDC6499A::class),
- $this->_em->getClassMetadata(DDC6499B::class),
- ]
- );
+ $this->_schemaTool->dropSchema([
+ $this->_em->getClassMetadata(DDC6499A::class),
+ $this->_em->getClassMetadata(DDC6499B::class),
+ ]);
}
/**
From 25829ea450f60fb9f1507444f339edc8b0915669 Mon Sep 17 00:00:00 2001
From: Marco Pivetta <ocramius@gmail.com>
Date: Fri, 11 Aug 2017 22:05:00 +0200
Subject: [PATCH 6/8] #6499 #6533 simplifying test scenario to the bone, adding
description of what happened at persistence-level
---
.../ORM/Functional/Ticket/DDC6499Test.php | 58 ++++++++-----------
1 file changed, 25 insertions(+), 33 deletions(-)
diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6499Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6499Test.php
index 066c0b1b4..6e8b25336 100644
--- a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6499Test.php
+++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6499Test.php
@@ -6,7 +6,12 @@ use Doctrine\Common\Collections\ArrayCollection;
use Doctrine\Tests\OrmFunctionalTestCase;
/**
- * @group DDC-6499
+ * @group #6499
+ *
+ *
+ * Specifically, DDC6499B has a dependency on DDC6499A, and DDC6499A
+ * has a dependency on DDC6499B. Since DDC6499A#b is not nullable,
+ * the DDC6499B should be inserted first.
*/
class DDC6499Test extends OrmFunctionalTestCase
{
@@ -36,69 +41,56 @@ class DDC6499Test extends OrmFunctionalTestCase
]);
}
- /**
- * Test for the bug described in issue #6499.
- */
public function testIssue() : void
{
- $a = new DDC6499A();
- $this->_em->persist($a);
-
$b = new DDC6499B();
- $a->b = $b;
+ $a = new DDC6499A($b);
+
+ $this->_em->persist($a);
$this->_em->persist($b);
$this->_em->flush();
- $this->_em->clear();
- self::assertEquals($this->_em->find(DDC6499A::class, $a->id)->b->id, $b->id, "Issue #6499 will result in a Integrity constraint violation before reaching this point.");
+ self::assertInternalType('integer', $a->id);
+ self::assertInternalType('integer', $b->id);
}
- /**
- * Test for the bug described in issue #6499 (reversed order).
- */
public function testIssueReversed() : void
{
- $a = new DDC6499A();
-
$b = new DDC6499B();
- $a->b = $b;
+ $a = new DDC6499A($b);
$this->_em->persist($b);
$this->_em->persist($a);
$this->_em->flush();
- $this->_em->clear();
- self::assertEquals($this->_em->find(DDC6499A::class, $a->id)->b->id, $b->id, "Issue #6499 will result in a Integrity constraint violation before reaching this point.");
+ self::assertInternalType('integer', $a->id);
+ self::assertInternalType('integer', $b->id);
}
}
/** @Entity */
class DDC6499A
{
- /**
- * @Id @Column(type="integer") @GeneratedValue
- */
+ /** @Id @Column(type="integer") @GeneratedValue */
public $id;
- /**
- * @OneToOne(targetEntity="DDC6499B")
- * @JoinColumn(nullable=false)
- */
+ /** @JoinColumn(nullable=false) @OneToOne(targetEntity=DDC6499B::class) */
public $b;
+
+ public function __construct(DDC6499B $b)
+ {
+ $this->b = $b;
+ }
}
/** @Entity */
class DDC6499B
{
- /**
- * @Id @Column(type="integer") @GeneratedValue
- */
+ /** @Id @Column(type="integer") @GeneratedValue */
public $id;
- /**
- * @ManyToOne(targetEntity="DDC6499A", inversedBy="bs")
- */
- public $a;
-}
\ No newline at end of file
+ /** @ManyToOne(targetEntity="DDC6499A") */
+ private $a;
+}
From 166c5816b6296fc14860149fe1a6d2cf3f40e092 Mon Sep 17 00:00:00 2001
From: Marco Pivetta <ocramius@gmail.com>
Date: Fri, 11 Aug 2017 22:10:02 +0200
Subject: [PATCH 7/8] #6499 #6533 calibrating test so that the association is
populated after persistence in some edge cases
---
.../Tests/ORM/Functional/Ticket/DDC6499Test.php | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6499Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6499Test.php
index 6e8b25336..0d86fffb0 100644
--- a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6499Test.php
+++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC6499Test.php
@@ -44,9 +44,12 @@ class DDC6499Test extends OrmFunctionalTestCase
public function testIssue() : void
{
$b = new DDC6499B();
- $a = new DDC6499A($b);
+ $a = new DDC6499A();
$this->_em->persist($a);
+
+ $a->b = $b;
+
$this->_em->persist($b);
$this->_em->flush();
@@ -58,7 +61,9 @@ class DDC6499Test extends OrmFunctionalTestCase
public function testIssueReversed() : void
{
$b = new DDC6499B();
- $a = new DDC6499A($b);
+ $a = new DDC6499A();
+
+ $a->b = $b;
$this->_em->persist($b);
$this->_em->persist($a);
@@ -78,11 +83,6 @@ class DDC6499A
/** @JoinColumn(nullable=false) @OneToOne(targetEntity=DDC6499B::class) */
public $b;
-
- public function __construct(DDC6499B $b)
- {
- $this->b = $b;
- }
}
/** @Entity */
From 1ede3c514f78837ffa5f4bb0621a188791d80cd0 Mon Sep 17 00:00:00 2001
From: Marco Pivetta <ocramius@gmail.com>
Date: Fri, 11 Aug 2017 22:13:20 +0200
Subject: [PATCH 8/8] #6499 #6533 simplifying nullable column check - null
coalesce operator makes it *SOMEWHAT* more readable (no miracles)
---
lib/Doctrine/ORM/UnitOfWork.php | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php
index 0afabf805..56615807f 100644
--- a/lib/Doctrine/ORM/UnitOfWork.php
+++ b/lib/Doctrine/ORM/UnitOfWork.php
@@ -1154,7 +1154,11 @@ class UnitOfWork implements PropertyChangedListener
$joinColumns = reset($assoc['joinColumns']);
- $calc->addDependency($targetClass->name, $class->name, (int) ! ( ! isset($joinColumns['nullable']) || $joinColumns['nullable'] === true));
+ $calc->addDependency(
+ $targetClass->name,
+ $class->name,
+ (int) (($joinColumns['nullable'] ?? true) === false)
+ );
// If the target class has mapped subclasses, these share the same dependency.
if ( ! $targetClass->subClasses) {