From 65bcdbf4c7bdf8620eb03eb4e32f233113ea5fb9 Mon Sep 17 00:00:00 2001 From: Matthieu Napoli Date: Mon, 4 Nov 2013 12:40:51 +0100 Subject: [PATCH 1/7] [DDC-2775] Tests reproducing DDC-2775 --- .../Tests/Models/DDC2775/AdminRole.php | 8 +++ .../Tests/Models/DDC2775/Authorization.php | 40 +++++++++++++++ tests/Doctrine/Tests/Models/DDC2775/Role.php | 49 +++++++++++++++++++ tests/Doctrine/Tests/Models/DDC2775/User.php | 40 +++++++++++++++ .../ORM/Functional/Ticket/DDC2775Test.php | 49 +++++++++++++++++++ .../Doctrine/Tests/OrmFunctionalTestCase.php | 6 +++ 6 files changed, 192 insertions(+) create mode 100644 tests/Doctrine/Tests/Models/DDC2775/AdminRole.php create mode 100644 tests/Doctrine/Tests/Models/DDC2775/Authorization.php create mode 100644 tests/Doctrine/Tests/Models/DDC2775/Role.php create mode 100644 tests/Doctrine/Tests/Models/DDC2775/User.php create mode 100644 tests/Doctrine/Tests/ORM/Functional/Ticket/DDC2775Test.php diff --git a/tests/Doctrine/Tests/Models/DDC2775/AdminRole.php b/tests/Doctrine/Tests/Models/DDC2775/AdminRole.php new file mode 100644 index 000000000..d40bb625a --- /dev/null +++ b/tests/Doctrine/Tests/Models/DDC2775/AdminRole.php @@ -0,0 +1,8 @@ +id; + } + + public function setUser(User $user) + { + $this->user = $user; + } + + public function setRole(Role $role) + { + $this->role = $role; + } +} diff --git a/tests/Doctrine/Tests/Models/DDC2775/Role.php b/tests/Doctrine/Tests/Models/DDC2775/Role.php new file mode 100644 index 000000000..7812b2e89 --- /dev/null +++ b/tests/Doctrine/Tests/Models/DDC2775/Role.php @@ -0,0 +1,49 @@ +id; + } + + public function getUser() + { + return $this->user; + } + + public function setUser(User $user) + { + $this->user = $user; + } + + public function addAuthorization(Authorization $authorization) + { + $this->authorizations[] = $authorization; + $authorization->setRole($this); + } +} diff --git a/tests/Doctrine/Tests/Models/DDC2775/User.php b/tests/Doctrine/Tests/Models/DDC2775/User.php new file mode 100644 index 000000000..07b1b09be --- /dev/null +++ b/tests/Doctrine/Tests/Models/DDC2775/User.php @@ -0,0 +1,40 @@ +id; + } + + public function addRole(Role $role) + { + $this->roles[] = $role; + $role->setUser($this); + } + + public function addAuthorization(Authorization $authorization) + { + $this->authorizations[] = $authorization; + $authorization->setUser($this); + } +} diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC2775Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC2775Test.php new file mode 100644 index 000000000..f8259f7c8 --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC2775Test.php @@ -0,0 +1,49 @@ + + */ +class DDC2775Test extends \Doctrine\Tests\OrmFunctionalTestCase +{ + protected function setUp() { + $this->useModelSet('ddc2775'); + parent::setUp(); + } + + /** + * @group DDC-2775 + */ + public function testIssueCascadeRemove() + { + $user = new User(); + + $role = new AdminRole(); + $user->addRole($role); + + $authorization = new Authorization(); + $user->addAuthorization($authorization); + $role->addAuthorization($authorization); + + $this->_em->persist($user); + $this->_em->flush(); + + // Need to clear so that associations are lazy-loaded + $this->_em->clear(); + + $user = $this->_em->find('Doctrine\Tests\Models\DDC2775\User', $user->getId()); + + $this->_em->remove($user); + $this->_em->flush(); + + // With the bug, the second flush throws an error because the cascade remove didn't work correctly + $this->_em->flush(); + } +} diff --git a/tests/Doctrine/Tests/OrmFunctionalTestCase.php b/tests/Doctrine/Tests/OrmFunctionalTestCase.php index d2a41cfb9..57fad7db7 100644 --- a/tests/Doctrine/Tests/OrmFunctionalTestCase.php +++ b/tests/Doctrine/Tests/OrmFunctionalTestCase.php @@ -162,6 +162,12 @@ abstract class OrmFunctionalTestCase extends OrmTestCase 'Doctrine\Tests\Models\Taxi\Car', 'Doctrine\Tests\Models\Taxi\Driver', ), + 'ddc2775' => array( + 'Doctrine\Tests\Models\DDC2775\User', + 'Doctrine\Tests\Models\DDC2775\Role', + 'Doctrine\Tests\Models\DDC2775\AdminRole', + 'Doctrine\Tests\Models\DDC2775\Authorization', + ), ); /** From 3829b9c28b433627a0ba3eb20dcfcef6aecf9308 Mon Sep 17 00:00:00 2001 From: Matthieu Napoli Date: Mon, 4 Nov 2013 12:42:23 +0100 Subject: [PATCH 2/7] [DDC-2775] Bugfix --- lib/Doctrine/ORM/UnitOfWork.php | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index e0df63958..266288c56 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -2233,6 +2233,10 @@ class UnitOfWork implements PropertyChangedListener function ($assoc) { return $assoc['isCascadeRemove']; } ); + $entitiesToCascade = array(); + + // We need to load all related entities beforehand so that lazy collection loading doesn't + // reload entities after they have been removed (bug DDC-2775) foreach ($associationMappings as $assoc) { if ($entity instanceof Proxy && !$entity->__isInitialized__) { $entity->__load(); @@ -2245,18 +2249,22 @@ class UnitOfWork implements PropertyChangedListener case (is_array($relatedEntities)): // If its a PersistentCollection initialization is intended! No unwrap! foreach ($relatedEntities as $relatedEntity) { - $this->doRemove($relatedEntity, $visited); + $entitiesToCascade[] = $relatedEntity; } break; case ($relatedEntities !== null): - $this->doRemove($relatedEntities, $visited); + $entitiesToCascade[] = $relatedEntities; break; default: // Do nothing } } + + foreach ($entitiesToCascade as $relatedEntity) { + $this->doRemove($relatedEntity, $visited); + } } /** From b59f495875017a91792cf9a39e40ef91fbb2810d Mon Sep 17 00:00:00 2001 From: Matthieu Napoli Date: Mon, 4 Nov 2013 12:53:18 +0100 Subject: [PATCH 3/7] Fixed tests for pgsql: was using reserved keyword as table name --- tests/Doctrine/Tests/Models/DDC2775/User.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Doctrine/Tests/Models/DDC2775/User.php b/tests/Doctrine/Tests/Models/DDC2775/User.php index 07b1b09be..de99a0aad 100644 --- a/tests/Doctrine/Tests/Models/DDC2775/User.php +++ b/tests/Doctrine/Tests/Models/DDC2775/User.php @@ -2,7 +2,7 @@ namespace Doctrine\Tests\Models\DDC2775; -/** @Entity */ +/** @Entity @Table(name="users") */ class User { /** From c5f66e6e7ff2759bfdd396381c4a3720e948404e Mon Sep 17 00:00:00 2001 From: Matthieu Napoli Date: Mon, 4 Nov 2013 16:01:05 +0100 Subject: [PATCH 4/7] Fixed tests failing in pgsql because of used of a reserved keyword --- tests/Doctrine/Tests/Models/DDC2775/Authorization.php | 2 +- tests/Doctrine/Tests/Models/DDC2775/User.php | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/Doctrine/Tests/Models/DDC2775/Authorization.php b/tests/Doctrine/Tests/Models/DDC2775/Authorization.php index 2e33eca67..25d104123 100644 --- a/tests/Doctrine/Tests/Models/DDC2775/Authorization.php +++ b/tests/Doctrine/Tests/Models/DDC2775/Authorization.php @@ -3,7 +3,7 @@ namespace Doctrine\Tests\Models\DDC2775; /** - * @Entity + * @Entity @Table(name="authorizations") */ class Authorization { diff --git a/tests/Doctrine/Tests/Models/DDC2775/User.php b/tests/Doctrine/Tests/Models/DDC2775/User.php index de99a0aad..67b46f25e 100644 --- a/tests/Doctrine/Tests/Models/DDC2775/User.php +++ b/tests/Doctrine/Tests/Models/DDC2775/User.php @@ -2,7 +2,9 @@ namespace Doctrine\Tests\Models\DDC2775; -/** @Entity @Table(name="users") */ +/** + * @Entity @Table(name="users") + */ class User { /** From 5ac111e5f883d12aa8b8c0435c8877a0e1a23835 Mon Sep 17 00:00:00 2001 From: Matthieu Napoli Date: Sat, 14 Dec 2013 19:57:53 +0100 Subject: [PATCH 5/7] Cleaned up tests for DDC-2775 --- .../Tests/Models/DDC2775/Authorization.php | 21 +++---------------- tests/Doctrine/Tests/Models/DDC2775/Role.php | 21 +++---------------- tests/Doctrine/Tests/Models/DDC2775/User.php | 11 +++------- .../ORM/Functional/Ticket/DDC2775Test.php | 5 +++-- 4 files changed, 12 insertions(+), 46 deletions(-) diff --git a/tests/Doctrine/Tests/Models/DDC2775/Authorization.php b/tests/Doctrine/Tests/Models/DDC2775/Authorization.php index 25d104123..caeae2b24 100644 --- a/tests/Doctrine/Tests/Models/DDC2775/Authorization.php +++ b/tests/Doctrine/Tests/Models/DDC2775/Authorization.php @@ -11,30 +11,15 @@ class Authorization * @Id @Column(type="integer") * @GeneratedValue */ - private $id; + public $id; /** * @ManyToOne(targetEntity="User", inversedBy="authorizations") */ - private $user; + public $user; /** * @ManyToOne(targetEntity="Role", inversedBy="authorizations") */ - private $role; - - public function getId() - { - return $this->id; - } - - public function setUser(User $user) - { - $this->user = $user; - } - - public function setRole(Role $role) - { - $this->role = $role; - } + public $role; } diff --git a/tests/Doctrine/Tests/Models/DDC2775/Role.php b/tests/Doctrine/Tests/Models/DDC2775/Role.php index 7812b2e89..4ef0ac462 100644 --- a/tests/Doctrine/Tests/Models/DDC2775/Role.php +++ b/tests/Doctrine/Tests/Models/DDC2775/Role.php @@ -14,36 +14,21 @@ abstract class Role * @Id @Column(type="integer") * @GeneratedValue */ - private $id; + public $id; /** * @ManyToOne(targetEntity="User", inversedBy="roles") */ - private $user; + public $user; /** * @OneToMany(targetEntity="Authorization", mappedBy="role", cascade={"all"}, orphanRemoval=true) */ public $authorizations; - public function getId() - { - return $this->id; - } - - public function getUser() - { - return $this->user; - } - - public function setUser(User $user) - { - $this->user = $user; - } - public function addAuthorization(Authorization $authorization) { $this->authorizations[] = $authorization; - $authorization->setRole($this); + $authorization->role = $this; } } diff --git a/tests/Doctrine/Tests/Models/DDC2775/User.php b/tests/Doctrine/Tests/Models/DDC2775/User.php index 67b46f25e..d42685679 100644 --- a/tests/Doctrine/Tests/Models/DDC2775/User.php +++ b/tests/Doctrine/Tests/Models/DDC2775/User.php @@ -11,7 +11,7 @@ class User * @Id @Column(type="integer") * @GeneratedValue(strategy="AUTO") */ - private $id; + public $id; /** * @OneToMany(targetEntity="Role", mappedBy="user", cascade={"all"}, orphanRemoval=true) @@ -23,20 +23,15 @@ class User */ public $authorizations; - public function getId() - { - return $this->id; - } - public function addRole(Role $role) { $this->roles[] = $role; - $role->setUser($this); + $role->user = $this; } public function addAuthorization(Authorization $authorization) { $this->authorizations[] = $authorization; - $authorization->setUser($this); + $authorization->user = $this; } } diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC2775Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC2775Test.php index f8259f7c8..6e2a61d19 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC2775Test.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC2775Test.php @@ -5,13 +5,14 @@ namespace Doctrine\Tests\ORM\Functional\Ticket; use Doctrine\Tests\Models\DDC2775\AdminRole; use Doctrine\Tests\Models\DDC2775\Authorization; use Doctrine\Tests\Models\DDC2775\User; +use Doctrine\Tests\OrmFunctionalTestCase; /** * Functional tests for cascade remove with class table inheritance. * * @author Matthieu Napoli */ -class DDC2775Test extends \Doctrine\Tests\OrmFunctionalTestCase +class DDC2775Test extends OrmFunctionalTestCase { protected function setUp() { $this->useModelSet('ddc2775'); @@ -38,7 +39,7 @@ class DDC2775Test extends \Doctrine\Tests\OrmFunctionalTestCase // Need to clear so that associations are lazy-loaded $this->_em->clear(); - $user = $this->_em->find('Doctrine\Tests\Models\DDC2775\User', $user->getId()); + $user = $this->_em->find('Doctrine\Tests\Models\DDC2775\User', $user->id); $this->_em->remove($user); $this->_em->flush(); From a89cc7abead086a644f60fde2e08ddb31a81d986 Mon Sep 17 00:00:00 2001 From: Matthieu Napoli Date: Mon, 23 Dec 2013 09:55:10 +0100 Subject: [PATCH 6/7] Inlined the model for the DCC2775 test case inside the test class --- lib/Doctrine/ORM/UnitOfWork.php | 2 - .../Tests/Models/DDC2775/AdminRole.php | 8 -- .../Tests/Models/DDC2775/Authorization.php | 25 ---- tests/Doctrine/Tests/Models/DDC2775/Role.php | 34 ------ tests/Doctrine/Tests/Models/DDC2775/User.php | 37 ------ .../ORM/Functional/Ticket/DDC2775Test.php | 108 +++++++++++++++++- .../Doctrine/Tests/OrmFunctionalTestCase.php | 6 - 7 files changed, 102 insertions(+), 118 deletions(-) delete mode 100644 tests/Doctrine/Tests/Models/DDC2775/AdminRole.php delete mode 100644 tests/Doctrine/Tests/Models/DDC2775/Authorization.php delete mode 100644 tests/Doctrine/Tests/Models/DDC2775/Role.php delete mode 100644 tests/Doctrine/Tests/Models/DDC2775/User.php diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index 266288c56..e982d6b8b 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -2235,8 +2235,6 @@ class UnitOfWork implements PropertyChangedListener $entitiesToCascade = array(); - // We need to load all related entities beforehand so that lazy collection loading doesn't - // reload entities after they have been removed (bug DDC-2775) foreach ($associationMappings as $assoc) { if ($entity instanceof Proxy && !$entity->__isInitialized__) { $entity->__load(); diff --git a/tests/Doctrine/Tests/Models/DDC2775/AdminRole.php b/tests/Doctrine/Tests/Models/DDC2775/AdminRole.php deleted file mode 100644 index d40bb625a..000000000 --- a/tests/Doctrine/Tests/Models/DDC2775/AdminRole.php +++ /dev/null @@ -1,8 +0,0 @@ -authorizations[] = $authorization; - $authorization->role = $this; - } -} diff --git a/tests/Doctrine/Tests/Models/DDC2775/User.php b/tests/Doctrine/Tests/Models/DDC2775/User.php deleted file mode 100644 index d42685679..000000000 --- a/tests/Doctrine/Tests/Models/DDC2775/User.php +++ /dev/null @@ -1,37 +0,0 @@ -roles[] = $role; - $role->user = $this; - } - - public function addAuthorization(Authorization $authorization) - { - $this->authorizations[] = $authorization; - $authorization->user = $this; - } -} diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC2775Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC2775Test.php index 6e2a61d19..bac183276 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC2775Test.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC2775Test.php @@ -2,9 +2,6 @@ namespace Doctrine\Tests\ORM\Functional\Ticket; -use Doctrine\Tests\Models\DDC2775\AdminRole; -use Doctrine\Tests\Models\DDC2775\Authorization; -use Doctrine\Tests\Models\DDC2775\User; use Doctrine\Tests\OrmFunctionalTestCase; /** @@ -14,9 +11,16 @@ use Doctrine\Tests\OrmFunctionalTestCase; */ class DDC2775Test extends OrmFunctionalTestCase { - protected function setUp() { - $this->useModelSet('ddc2775'); + protected function setUp() + { parent::setUp(); + + $this->setUpEntitySchema(array( + 'Doctrine\Tests\ORM\Functional\Ticket\User', + 'Doctrine\Tests\ORM\Functional\Ticket\Role', + 'Doctrine\Tests\ORM\Functional\Ticket\AdminRole', + 'Doctrine\Tests\ORM\Functional\Ticket\Authorization', + )); } /** @@ -39,7 +43,7 @@ class DDC2775Test extends OrmFunctionalTestCase // Need to clear so that associations are lazy-loaded $this->_em->clear(); - $user = $this->_em->find('Doctrine\Tests\Models\DDC2775\User', $user->id); + $user = $this->_em->find('Doctrine\Tests\ORM\Functional\Ticket\User', $user->id); $this->_em->remove($user); $this->_em->flush(); @@ -48,3 +52,95 @@ class DDC2775Test extends OrmFunctionalTestCase $this->_em->flush(); } } + +/** + * @Entity + * @InheritanceType("JOINED") + * @DiscriminatorColumn(name="role_type", type="string") + * @DiscriminatorMap({"admin"="AdminRole"}) + */ +abstract class Role +{ + /** + * @Id @Column(type="integer") + * @GeneratedValue + */ + public $id; + + /** + * @ManyToOne(targetEntity="User", inversedBy="roles") + */ + public $user; + + /** + * @OneToMany(targetEntity="Authorization", mappedBy="role", cascade={"all"}, orphanRemoval=true) + */ + public $authorizations; + + public function addAuthorization(Authorization $authorization) + { + $this->authorizations[] = $authorization; + $authorization->role = $this; + } +} + +/** @Entity */ +class AdminRole extends Role +{ +} + +/** + * @Entity @Table(name="authorizations") + */ +class Authorization +{ + /** + * @Id @Column(type="integer") + * @GeneratedValue + */ + public $id; + + /** + * @ManyToOne(targetEntity="User", inversedBy="authorizations") + */ + public $user; + + /** + * @ManyToOne(targetEntity="Role", inversedBy="authorizations") + */ + public $role; +} + +/** + * @Entity @Table(name="users") + */ +class User +{ + /** + * @Id @Column(type="integer") + * @GeneratedValue(strategy="AUTO") + */ + public $id; + + /** + * @OneToMany(targetEntity="Role", mappedBy="user", cascade={"all"}, orphanRemoval=true) + */ + public $roles; + + /** + * @OneToMany(targetEntity="Authorization", mappedBy="user", cascade={"all"}, orphanRemoval=true) + */ + public $authorizations; + + public function addRole(Role $role) + { + $this->roles[] = $role; + $role->user = $this; + } + + public function addAuthorization(Authorization $authorization) + { + $this->authorizations[] = $authorization; + $authorization->user = $this; + } +} diff --git a/tests/Doctrine/Tests/OrmFunctionalTestCase.php b/tests/Doctrine/Tests/OrmFunctionalTestCase.php index 57fad7db7..d2a41cfb9 100644 --- a/tests/Doctrine/Tests/OrmFunctionalTestCase.php +++ b/tests/Doctrine/Tests/OrmFunctionalTestCase.php @@ -162,12 +162,6 @@ abstract class OrmFunctionalTestCase extends OrmTestCase 'Doctrine\Tests\Models\Taxi\Car', 'Doctrine\Tests\Models\Taxi\Driver', ), - 'ddc2775' => array( - 'Doctrine\Tests\Models\DDC2775\User', - 'Doctrine\Tests\Models\DDC2775\Role', - 'Doctrine\Tests\Models\DDC2775\AdminRole', - 'Doctrine\Tests\Models\DDC2775\Authorization', - ), ); /** From b40866c62445ce3033d7df115c1cd7baf8360ba8 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Thu, 2 Jan 2014 23:11:07 +0100 Subject: [PATCH 7/7] [DDC-2775] cleanup test. --- .../Doctrine/Tests/ORM/Functional/Ticket/DDC2775Test.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC2775Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC2775Test.php index bac183276..e98ff91a3 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC2775Test.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/DDC2775Test.php @@ -54,7 +54,7 @@ class DDC2775Test extends OrmFunctionalTestCase } /** - * @Entity + * @Entity @Table(name="ddc2775_role") * @InheritanceType("JOINED") * @DiscriminatorColumn(name="role_type", type="string") * @DiscriminatorMap({"admin"="AdminRole"}) @@ -84,13 +84,13 @@ abstract class Role } } -/** @Entity */ +/** @Entity @Table(name="ddc2775_admin_role") */ class AdminRole extends Role { } /** - * @Entity @Table(name="authorizations") + * @Entity @Table(name="ddc2775_authorizations") */ class Authorization { @@ -112,7 +112,7 @@ class Authorization } /** - * @Entity @Table(name="users") + * @Entity @Table(name="ddc2775_users") */ class User {