From da8b983414e8e72f6407c576e53917314f5031ad Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 12 Dec 2014 16:28:17 +0100 Subject: [PATCH 1/7] Failing test case: proxy loading fails when objects referencing each other are loaded by the same `UnitOfWork#findMany()` call --- .../ODM/PHPCR/Functional/UnitOfWorkTest.php | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/tests/Doctrine/Tests/ODM/PHPCR/Functional/UnitOfWorkTest.php b/tests/Doctrine/Tests/ODM/PHPCR/Functional/UnitOfWorkTest.php index dc2181e88..4fee4f83c 100644 --- a/tests/Doctrine/Tests/ODM/PHPCR/Functional/UnitOfWorkTest.php +++ b/tests/Doctrine/Tests/ODM/PHPCR/Functional/UnitOfWorkTest.php @@ -147,4 +147,42 @@ public function testComputeChangeSetForTranslatableDocument() $this->assertCount(1, $this->uow->getScheduledInserts()); $this->assertCount(0, $this->uow->getScheduledUpdates()); } + + public function testFetchingMultipleHierarchicalObjectsWithChildIdFirst() + { + $root = $this->dm->find(null, 'functional'); + + $comment1 = new ParentTestObj(); + $comment1->nodename = 'parentComment'; + $comment1->name = 'parentComment'; + $comment1->parent = $root; + + $comment2 = new ParentTestObj(); + $comment2->nodename = 'parentComment'; + $comment2->name = 'parentComment'; + $comment2->parent = $comment1; + + $this->dm->persist($comment1); + $this->dm->persist($comment2); + + $comment1Id = $this->uow->getDocumentId($comment1); + $comment2Id = $this->uow->getDocumentId($comment2); + + $this->dm->flush(); + $this->dm->clear(); + + $documents = $this->dm->findMany( + 'Doctrine\Tests\Models\References\ParentTestObj', + array($comment2Id, $comment1Id) + ); + + $this->assertCount(2, $documents); + + /* @var $comment2 ParentTestObj */ + /* @var $comment1 ParentTestObj */ + $comment2 = $documents->first(); + $comment1 = $documents->last(); + + $this->assertSame($comment2->parent, $comment1); + } } From b7f68d0f240e8afa041c478582a153c764eafb48 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 12 Dec 2014 17:19:40 +0100 Subject: [PATCH 2/7] Asserting also on node name for clarity, simplifying tests, readability fixes --- .../Tests/ODM/PHPCR/Functional/UnitOfWorkTest.php | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/tests/Doctrine/Tests/ODM/PHPCR/Functional/UnitOfWorkTest.php b/tests/Doctrine/Tests/ODM/PHPCR/Functional/UnitOfWorkTest.php index 4fee4f83c..52063be58 100644 --- a/tests/Doctrine/Tests/ODM/PHPCR/Functional/UnitOfWorkTest.php +++ b/tests/Doctrine/Tests/ODM/PHPCR/Functional/UnitOfWorkTest.php @@ -150,16 +150,14 @@ public function testComputeChangeSetForTranslatableDocument() public function testFetchingMultipleHierarchicalObjectsWithChildIdFirst() { - $root = $this->dm->find(null, 'functional'); - $comment1 = new ParentTestObj(); $comment1->nodename = 'parentComment'; $comment1->name = 'parentComment'; - $comment1->parent = $root; + $comment1->parent = $this->dm->find(null, 'functional'); $comment2 = new ParentTestObj(); - $comment2->nodename = 'parentComment'; - $comment2->name = 'parentComment'; + $comment2->nodename = 'childComment'; + $comment2->name = 'childComment'; $comment2->parent = $comment1; $this->dm->persist($comment1); @@ -184,5 +182,6 @@ public function testFetchingMultipleHierarchicalObjectsWithChildIdFirst() $comment1 = $documents->last(); $this->assertSame($comment2->parent, $comment1); + $this->assertSame('parentComment', $comment1->nodename); } } From 23684b712c7ea9bfc56c63c68f03af2f0ae9ef14 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 12 Dec 2014 17:20:28 +0100 Subject: [PATCH 3/7] Applying hotfix: currently existing identifiers are used to check against the identity map during hydration of multiple hierarchical items referencing each other --- lib/Doctrine/ODM/PHPCR/UnitOfWork.php | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/Doctrine/ODM/PHPCR/UnitOfWork.php b/lib/Doctrine/ODM/PHPCR/UnitOfWork.php index 70dc0d863..215936d7a 100644 --- a/lib/Doctrine/ODM/PHPCR/UnitOfWork.php +++ b/lib/Doctrine/ODM/PHPCR/UnitOfWork.php @@ -437,12 +437,13 @@ public function getOrCreateDocuments($className, $nodes, array &$hints = array() } foreach ($nodes as $node) { - $id = $node->getPath(); - if (!isset($documents[$id])) { + $id = $node->getPath(); + $document = $this->getDocumentById($id) ?: (isset($documents[$id]) ? $documents[$id] : null); + + if (! $document) { continue; } - $document = $documents[$id]; $class = $this->dm->getClassMetadata(get_class($document)); $documentState = array(); From b031dd9b2dc3489e95c0ceac7c4befe3fe504e84 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 12 Dec 2014 17:22:22 +0100 Subject: [PATCH 4/7] Rewording test variables for clarity --- .../ODM/PHPCR/Functional/UnitOfWorkTest.php | 39 ++++++++++--------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/tests/Doctrine/Tests/ODM/PHPCR/Functional/UnitOfWorkTest.php b/tests/Doctrine/Tests/ODM/PHPCR/Functional/UnitOfWorkTest.php index 52063be58..ab51fd9d1 100644 --- a/tests/Doctrine/Tests/ODM/PHPCR/Functional/UnitOfWorkTest.php +++ b/tests/Doctrine/Tests/ODM/PHPCR/Functional/UnitOfWorkTest.php @@ -150,38 +150,39 @@ public function testComputeChangeSetForTranslatableDocument() public function testFetchingMultipleHierarchicalObjectsWithChildIdFirst() { - $comment1 = new ParentTestObj(); - $comment1->nodename = 'parentComment'; - $comment1->name = 'parentComment'; - $comment1->parent = $this->dm->find(null, 'functional'); + $parent = new ParentTestObj(); + $parent->nodename = 'parentComment'; + $parent->name = 'parentComment'; + $parent->parent = $this->dm->find(null, 'functional'); - $comment2 = new ParentTestObj(); - $comment2->nodename = 'childComment'; - $comment2->name = 'childComment'; - $comment2->parent = $comment1; + $child = new ParentTestObj(); + $child->nodename = 'childComment'; + $child->name = 'childComment'; + $child->parent = $parent; - $this->dm->persist($comment1); - $this->dm->persist($comment2); + $this->dm->persist($parent); + $this->dm->persist($child); - $comment1Id = $this->uow->getDocumentId($comment1); - $comment2Id = $this->uow->getDocumentId($comment2); + $parentId = $this->uow->getDocumentId($parent); + $childId = $this->uow->getDocumentId($child); $this->dm->flush(); $this->dm->clear(); + // this forces the objects to be loaded in an order where the $parent will become a proxy $documents = $this->dm->findMany( 'Doctrine\Tests\Models\References\ParentTestObj', - array($comment2Id, $comment1Id) + array($childId, $parentId) ); $this->assertCount(2, $documents); - /* @var $comment2 ParentTestObj */ - /* @var $comment1 ParentTestObj */ - $comment2 = $documents->first(); - $comment1 = $documents->last(); + /* @var $child ParentTestObj */ + /* @var $parent ParentTestObj */ + $child = $documents->first(); + $parent = $documents->last(); - $this->assertSame($comment2->parent, $comment1); - $this->assertSame('parentComment', $comment1->nodename); + $this->assertSame($child->parent, $parent); + $this->assertSame('parentComment', $parent->nodename); } } From 7d6a3ce36d5c04eb791352ce13b406551cdf2042 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 12 Dec 2014 17:25:23 +0100 Subject: [PATCH 5/7] Retrieved document instance should be the same one that is already in the identity map --- lib/Doctrine/ODM/PHPCR/UnitOfWork.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/Doctrine/ODM/PHPCR/UnitOfWork.php b/lib/Doctrine/ODM/PHPCR/UnitOfWork.php index 215936d7a..ab9a8fde2 100644 --- a/lib/Doctrine/ODM/PHPCR/UnitOfWork.php +++ b/lib/Doctrine/ODM/PHPCR/UnitOfWork.php @@ -444,7 +444,8 @@ public function getOrCreateDocuments($className, $nodes, array &$hints = array() continue; } - $class = $this->dm->getClassMetadata(get_class($document)); + $documents[$id] = $document; + $class = $this->dm->getClassMetadata(get_class($document)); $documentState = array(); $nonMappedData = array(); From 4a452d370d99d7d928e5335642154dd905f1adc2 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Fri, 12 Dec 2014 17:26:37 +0100 Subject: [PATCH 6/7] s/Comment// on `ParentTestObj` identifiers in tests, for clarity --- .../Tests/ODM/PHPCR/Functional/UnitOfWorkTest.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/Doctrine/Tests/ODM/PHPCR/Functional/UnitOfWorkTest.php b/tests/Doctrine/Tests/ODM/PHPCR/Functional/UnitOfWorkTest.php index ab51fd9d1..fd7531c8f 100644 --- a/tests/Doctrine/Tests/ODM/PHPCR/Functional/UnitOfWorkTest.php +++ b/tests/Doctrine/Tests/ODM/PHPCR/Functional/UnitOfWorkTest.php @@ -151,13 +151,13 @@ public function testComputeChangeSetForTranslatableDocument() public function testFetchingMultipleHierarchicalObjectsWithChildIdFirst() { $parent = new ParentTestObj(); - $parent->nodename = 'parentComment'; - $parent->name = 'parentComment'; + $parent->nodename = 'parent'; + $parent->name = 'parent'; $parent->parent = $this->dm->find(null, 'functional'); $child = new ParentTestObj(); - $child->nodename = 'childComment'; - $child->name = 'childComment'; + $child->nodename = 'child'; + $child->name = 'child'; $child->parent = $parent; $this->dm->persist($parent); From 985ceba8aaa4d588a7d6b128e46e188b0acd9ef5 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Sat, 13 Dec 2014 10:36:26 +0100 Subject: [PATCH 7/7] #581 - s/parentComment/parent --- tests/Doctrine/Tests/ODM/PHPCR/Functional/UnitOfWorkTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Doctrine/Tests/ODM/PHPCR/Functional/UnitOfWorkTest.php b/tests/Doctrine/Tests/ODM/PHPCR/Functional/UnitOfWorkTest.php index fd7531c8f..5aa7220dd 100644 --- a/tests/Doctrine/Tests/ODM/PHPCR/Functional/UnitOfWorkTest.php +++ b/tests/Doctrine/Tests/ODM/PHPCR/Functional/UnitOfWorkTest.php @@ -183,6 +183,6 @@ public function testFetchingMultipleHierarchicalObjectsWithChildIdFirst() $parent = $documents->last(); $this->assertSame($child->parent, $parent); - $this->assertSame('parentComment', $parent->nodename); + $this->assertSame('parent', $parent->nodename); } }