From 216c4662333780fb7f09b71795ed75478286d0fa Mon Sep 17 00:00:00 2001 From: bilouwan Date: Fri, 27 Nov 2015 17:28:45 +0100 Subject: [PATCH 1/8] Unit test & fix for merge versionned entity --- lib/Doctrine/ORM/UnitOfWork.php | 14 +++- .../Models/VersionedOneToMany/Article.php | 47 +++++++++++++ .../Models/VersionedOneToMany/Category.php | 49 ++++++++++++++ .../MergeVersionedOneToManyTest.php | 67 +++++++++++++++++++ 4 files changed, 176 insertions(+), 1 deletion(-) create mode 100644 tests/Doctrine/Tests/Models/VersionedOneToMany/Article.php create mode 100644 tests/Doctrine/Tests/Models/VersionedOneToMany/Category.php create mode 100644 tests/Doctrine/Tests/ORM/Functional/MergeVersionedOneToManyTest.php diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index 194e45e30..1818b86cd 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -1870,7 +1870,7 @@ class UnitOfWork implements PropertyChangedListener } } - if ($class->isVersioned) { + if ($class->isVersioned && !($this->isNotInitializedProxy($managedCopy) || $this->isNotInitializedProxy($entity))) { $reflField = $class->reflFields[$class->versionField]; $managedCopyVersion = $reflField->getValue($managedCopy); $entityVersion = $reflField->getValue($entity); @@ -1908,6 +1908,18 @@ class UnitOfWork implements PropertyChangedListener return $managedCopy; } + /** + * Tests if an entity is a non initialized proxy class + * + * @param $entity + * + * @return bool + */ + private function isNotInitializedProxy($entity) + { + return $entity instanceof Proxy && !$entity->__isInitialized(); + } + /** * Sets/adds associated managed copies into the previous entity's association field * diff --git a/tests/Doctrine/Tests/Models/VersionedOneToMany/Article.php b/tests/Doctrine/Tests/Models/VersionedOneToMany/Article.php new file mode 100644 index 000000000..c284fc15f --- /dev/null +++ b/tests/Doctrine/Tests/Models/VersionedOneToMany/Article.php @@ -0,0 +1,47 @@ +tags = new ArrayCollection(); + } +} diff --git a/tests/Doctrine/Tests/Models/VersionedOneToMany/Category.php b/tests/Doctrine/Tests/Models/VersionedOneToMany/Category.php new file mode 100644 index 000000000..5ace04603 --- /dev/null +++ b/tests/Doctrine/Tests/Models/VersionedOneToMany/Category.php @@ -0,0 +1,49 @@ +articles = new ArrayCollection(); + } + + +} diff --git a/tests/Doctrine/Tests/ORM/Functional/MergeVersionedOneToManyTest.php b/tests/Doctrine/Tests/ORM/Functional/MergeVersionedOneToManyTest.php new file mode 100644 index 000000000..c5f022c52 --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Functional/MergeVersionedOneToManyTest.php @@ -0,0 +1,67 @@ +_schemaTool->createSchema( + [ + $this->_em->getClassMetadata(Category::class), + $this->_em->getClassMetadata(Article::class), + ] + ); + } catch (ORMException $e) { + } + } + + /** + * This test case tests that a versionable entity, that has a oneToOne relationship as it's id can be created + * without this bug fix (DDC-3318), you could not do this + */ + public function testSetVersionOnCreate() + { + $category = new Category(); + $category->name = 'Category'; + + $article = new Article(); + $article->name = 'Article'; + $article->category = $category; + + $this->_em->persist($article); + $this->_em->flush(); + $this->_em->clear(); + + $mergeSucceed = false; + try { + $articleMerged = $this->_em->merge($article); + $mergeSucceed = true; + } catch (OptimisticLockException $e) { + } + $this->assertTrue($mergeSucceed); + + $articleMerged->name = 'Article Merged'; + + $flushSucceed = false; + try { + $this->_em->flush(); + $flushSucceed = true; + } catch (OptimisticLockException $e) { + } + $this->assertTrue($flushSucceed); + } +} From 7071984559f1afd3e74f53c510aeff5d89572961 Mon Sep 17 00:00:00 2001 From: bilouwan Date: Mon, 30 Nov 2015 10:35:42 +0100 Subject: [PATCH 2/8] Fix compatibility with php5.4 --- .../MergeVersionedOneToManyTest.php | 22 +++++-------------- 1 file changed, 5 insertions(+), 17 deletions(-) diff --git a/tests/Doctrine/Tests/ORM/Functional/MergeVersionedOneToManyTest.php b/tests/Doctrine/Tests/ORM/Functional/MergeVersionedOneToManyTest.php index c5f022c52..178e16ab0 100644 --- a/tests/Doctrine/Tests/ORM/Functional/MergeVersionedOneToManyTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/MergeVersionedOneToManyTest.php @@ -6,7 +6,6 @@ use Doctrine\ORM\OptimisticLockException; use Doctrine\ORM\ORMException; use Doctrine\Tests\Models\VersionedOneToMany\Article; use Doctrine\Tests\Models\VersionedOneToMany\Category; -use Doctrine\Tests\Models\VersionedOneToMany\Tag; /** * @@ -21,8 +20,8 @@ class MergeVersionedOneToManyTest extends \Doctrine\Tests\OrmFunctionalTestCase try { $this->_schemaTool->createSchema( [ - $this->_em->getClassMetadata(Category::class), - $this->_em->getClassMetadata(Article::class), + $this->_em->getClassMetadata('Doctrine\Tests\Models\VersionedOneToMany\Category'), + $this->_em->getClassMetadata('Doctrine\Tests\Models\VersionedOneToMany\Article'), ] ); } catch (ORMException $e) { @@ -46,22 +45,11 @@ class MergeVersionedOneToManyTest extends \Doctrine\Tests\OrmFunctionalTestCase $this->_em->flush(); $this->_em->clear(); - $mergeSucceed = false; - try { - $articleMerged = $this->_em->merge($article); - $mergeSucceed = true; - } catch (OptimisticLockException $e) { - } - $this->assertTrue($mergeSucceed); + $articleMerged = $this->_em->merge($article); $articleMerged->name = 'Article Merged'; - $flushSucceed = false; - try { - $this->_em->flush(); - $flushSucceed = true; - } catch (OptimisticLockException $e) { - } - $this->assertTrue($flushSucceed); + $this->_em->flush(); + $this->assertEquals(2, $articleMerged->version); } } From e173c930ec03bbbf270eba4dd085ad2351a114f0 Mon Sep 17 00:00:00 2001 From: bilouwan Date: Wed, 2 Dec 2015 14:09:14 +0100 Subject: [PATCH 3/8] Fix superflous whitespaces & empty lines --- tests/Doctrine/Tests/Models/VersionedOneToMany/Article.php | 2 -- tests/Doctrine/Tests/Models/VersionedOneToMany/Category.php | 4 ---- .../Tests/ORM/Functional/MergeVersionedOneToManyTest.php | 4 ++-- 3 files changed, 2 insertions(+), 8 deletions(-) diff --git a/tests/Doctrine/Tests/Models/VersionedOneToMany/Article.php b/tests/Doctrine/Tests/Models/VersionedOneToMany/Article.php index c284fc15f..bd0cfdf48 100644 --- a/tests/Doctrine/Tests/Models/VersionedOneToMany/Article.php +++ b/tests/Doctrine/Tests/Models/VersionedOneToMany/Article.php @@ -5,7 +5,6 @@ namespace Doctrine\Tests\Models\VersionedOneToMany; use Doctrine\Common\Collections\ArrayCollection; /** - * * @Entity * @Table(name="article") */ @@ -38,7 +37,6 @@ class Article /** * Category constructor. - * */ public function __construct() { diff --git a/tests/Doctrine/Tests/Models/VersionedOneToMany/Category.php b/tests/Doctrine/Tests/Models/VersionedOneToMany/Category.php index 5ace04603..d4dbdefbb 100644 --- a/tests/Doctrine/Tests/Models/VersionedOneToMany/Category.php +++ b/tests/Doctrine/Tests/Models/VersionedOneToMany/Category.php @@ -5,7 +5,6 @@ namespace Doctrine\Tests\Models\VersionedOneToMany; use Doctrine\Common\Collections\ArrayCollection; /** - * * @Entity * @Table(name="category") */ @@ -38,12 +37,9 @@ class Category /** * Category constructor. - * */ public function __construct() { $this->articles = new ArrayCollection(); } - - } diff --git a/tests/Doctrine/Tests/ORM/Functional/MergeVersionedOneToManyTest.php b/tests/Doctrine/Tests/ORM/Functional/MergeVersionedOneToManyTest.php index 178e16ab0..b51a24013 100644 --- a/tests/Doctrine/Tests/ORM/Functional/MergeVersionedOneToManyTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/MergeVersionedOneToManyTest.php @@ -29,8 +29,8 @@ class MergeVersionedOneToManyTest extends \Doctrine\Tests\OrmFunctionalTestCase } /** - * This test case tests that a versionable entity, that has a oneToOne relationship as it's id can be created - * without this bug fix (DDC-3318), you could not do this + * This test case asserts that a detached and unmodified entity could be merge without firing + * OptimisticLockException. */ public function testSetVersionOnCreate() { From 4148220f9ca6523ae6df858bb7560fa47ecb655e Mon Sep 17 00:00:00 2001 From: bilouwan Date: Fri, 4 Dec 2015 14:49:01 +0100 Subject: [PATCH 4/8] Refactor testing Proxy not initilized --- lib/Doctrine/ORM/UnitOfWork.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index 1818b86cd..fb35b1589 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -1870,7 +1870,7 @@ class UnitOfWork implements PropertyChangedListener } } - if ($class->isVersioned && !($this->isNotInitializedProxy($managedCopy) || $this->isNotInitializedProxy($entity))) { + if ($class->isVersioned && $this->isLoaded($managedCopy) && $this->isLoaded($entity)) { $reflField = $class->reflFields[$class->versionField]; $managedCopyVersion = $reflField->getValue($managedCopy); $entityVersion = $reflField->getValue($entity); @@ -1883,7 +1883,7 @@ class UnitOfWork implements PropertyChangedListener $visited[$oid] = $managedCopy; // mark visited - if (!($entity instanceof Proxy && ! $entity->__isInitialized())) { + if ($this->isLoaded($entity)) { if ($managedCopy instanceof Proxy && ! $managedCopy->__isInitialized()) { $managedCopy->__load(); } @@ -1909,15 +1909,15 @@ class UnitOfWork implements PropertyChangedListener } /** - * Tests if an entity is a non initialized proxy class + * Tests if an entity is loaded (Not a proxy or a non initialized proxy) * * @param $entity * * @return bool */ - private function isNotInitializedProxy($entity) + private function isLoaded($entity) { - return $entity instanceof Proxy && !$entity->__isInitialized(); + return !($entity instanceof Proxy) || $entity->__isInitialized(); } /** From d5c82094dfa0b3e57902db6fc9f6730597781973 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 11 Dec 2015 19:59:08 +0100 Subject: [PATCH 5/8] #1573 removing unused API --- .../Tests/Models/VersionedOneToMany/Article.php | 12 +----------- .../Tests/Models/VersionedOneToMany/Category.php | 15 --------------- 2 files changed, 1 insertion(+), 26 deletions(-) diff --git a/tests/Doctrine/Tests/Models/VersionedOneToMany/Article.php b/tests/Doctrine/Tests/Models/VersionedOneToMany/Article.php index bd0cfdf48..919480162 100644 --- a/tests/Doctrine/Tests/Models/VersionedOneToMany/Article.php +++ b/tests/Doctrine/Tests/Models/VersionedOneToMany/Article.php @@ -2,8 +2,6 @@ namespace Doctrine\Tests\Models\VersionedOneToMany; -use Doctrine\Common\Collections\ArrayCollection; - /** * @Entity * @Table(name="article") @@ -23,7 +21,7 @@ class Article public $name; /** - * @ManyToOne(targetEntity="Category", inversedBy="category", cascade={"merge", "persist"}) + * @ManyToOne(targetEntity="Category", cascade={"merge", "persist"}) */ public $category; @@ -34,12 +32,4 @@ class Article * @Version */ public $version; - - /** - * Category constructor. - */ - public function __construct() - { - $this->tags = new ArrayCollection(); - } } diff --git a/tests/Doctrine/Tests/Models/VersionedOneToMany/Category.php b/tests/Doctrine/Tests/Models/VersionedOneToMany/Category.php index d4dbdefbb..f5d293651 100644 --- a/tests/Doctrine/Tests/Models/VersionedOneToMany/Category.php +++ b/tests/Doctrine/Tests/Models/VersionedOneToMany/Category.php @@ -2,8 +2,6 @@ namespace Doctrine\Tests\Models\VersionedOneToMany; -use Doctrine\Common\Collections\ArrayCollection; - /** * @Entity * @Table(name="category") @@ -17,11 +15,6 @@ class Category */ public $id; - /** - * @OneToMany(targetEntity="Article", mappedBy="category", cascade={"merge", "persist"}) - */ - public $articles; - /** * @Column(name="name") */ @@ -34,12 +27,4 @@ class Category * @Version */ public $version; - - /** - * Category constructor. - */ - public function __construct() - { - $this->articles = new ArrayCollection(); - } } From 596e8957635a81e208f9482a6b9b718b75e4649b Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 11 Dec 2015 20:00:08 +0100 Subject: [PATCH 6/8] #1573 - correcting docblock arguments/description --- lib/Doctrine/ORM/UnitOfWork.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index fb35b1589..d27748a99 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -1909,9 +1909,9 @@ class UnitOfWork implements PropertyChangedListener } /** - * Tests if an entity is loaded (Not a proxy or a non initialized proxy) + * Tests if an entity is loaded - must either be a loaded proxy or not a proxy * - * @param $entity + * @param object $entity * * @return bool */ From 42691c21b46a055086c5a24056bcf04d9cd634a9 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 11 Dec 2015 20:00:59 +0100 Subject: [PATCH 7/8] Removing empty newline --- .../Tests/ORM/Functional/MergeVersionedOneToManyTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/Doctrine/Tests/ORM/Functional/MergeVersionedOneToManyTest.php b/tests/Doctrine/Tests/ORM/Functional/MergeVersionedOneToManyTest.php index b51a24013..f1c3145da 100644 --- a/tests/Doctrine/Tests/ORM/Functional/MergeVersionedOneToManyTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/MergeVersionedOneToManyTest.php @@ -8,7 +8,6 @@ use Doctrine\Tests\Models\VersionedOneToMany\Article; use Doctrine\Tests\Models\VersionedOneToMany\Category; /** - * * @group MergeVersionedOneToMany */ class MergeVersionedOneToManyTest extends \Doctrine\Tests\OrmFunctionalTestCase From 66770c5bfe4456ad9df456ae2927035c40ace0d7 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 11 Dec 2015 20:14:53 +0100 Subject: [PATCH 8/8] #1573 - correcting test asset namespace, removing unused properties and bi-directional association --- .../Article.php | 6 +++-- .../Category.php | 11 ++++----- ...st.php => MergeVersionedManyToOneTest.php} | 23 ++++++------------- .../Doctrine/Tests/OrmFunctionalTestCase.php | 9 ++++++++ 4 files changed, 24 insertions(+), 25 deletions(-) rename tests/Doctrine/Tests/Models/{VersionedOneToMany => VersionedManyToOne}/Article.php (77%) rename tests/Doctrine/Tests/Models/{VersionedOneToMany => VersionedManyToOne}/Category.php (68%) rename tests/Doctrine/Tests/ORM/Functional/{MergeVersionedOneToManyTest.php => MergeVersionedManyToOneTest.php} (56%) diff --git a/tests/Doctrine/Tests/Models/VersionedOneToMany/Article.php b/tests/Doctrine/Tests/Models/VersionedManyToOne/Article.php similarity index 77% rename from tests/Doctrine/Tests/Models/VersionedOneToMany/Article.php rename to tests/Doctrine/Tests/Models/VersionedManyToOne/Article.php index 919480162..0e5b1683f 100644 --- a/tests/Doctrine/Tests/Models/VersionedOneToMany/Article.php +++ b/tests/Doctrine/Tests/Models/VersionedManyToOne/Article.php @@ -1,13 +1,15 @@ useModelSet('versioned_many_to_one'); - try { - $this->_schemaTool->createSchema( - [ - $this->_em->getClassMetadata('Doctrine\Tests\Models\VersionedOneToMany\Category'), - $this->_em->getClassMetadata('Doctrine\Tests\Models\VersionedOneToMany\Article'), - ] - ); - } catch (ORMException $e) { - } + parent::setUp(); } /** @@ -34,10 +26,9 @@ class MergeVersionedOneToManyTest extends \Doctrine\Tests\OrmFunctionalTestCase public function testSetVersionOnCreate() { $category = new Category(); - $category->name = 'Category'; + $article = new Article(); - $article = new Article(); - $article->name = 'Article'; + $article->name = 'Article'; $article->category = $category; $this->_em->persist($article); diff --git a/tests/Doctrine/Tests/OrmFunctionalTestCase.php b/tests/Doctrine/Tests/OrmFunctionalTestCase.php index 855fac617..b36a83487 100644 --- a/tests/Doctrine/Tests/OrmFunctionalTestCase.php +++ b/tests/Doctrine/Tests/OrmFunctionalTestCase.php @@ -280,6 +280,10 @@ abstract class OrmFunctionalTestCase extends OrmTestCase 'Doctrine\Tests\Models\Pagination\User', 'Doctrine\Tests\Models\Pagination\User1', ), + 'versioned_many_to_one' => array( + 'Doctrine\Tests\Models\VersionedManyToOne\Category', + 'Doctrine\Tests\Models\VersionedManyToOne\Article', + ), ); /** @@ -535,6 +539,11 @@ abstract class OrmFunctionalTestCase extends OrmTestCase $conn->executeUpdate('DELETE FROM pagination_user'); } + if (isset($this->_usedModelSets['versioned_many_to_one'])) { + $conn->executeUpdate('DELETE FROM versioned_many_to_one_article'); + $conn->executeUpdate('DELETE FROM versioned_many_to_one_category'); + } + $this->_em->clear(); }