Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Added fix for collection->contains with many-to-many extra lazy fetchMode #259

Merged
merged 2 commits into from over 2 years ago

3 participants

Daniel Holmes Guilherme Blanco Christophe Coevoet
Daniel Holmes

That case previously triggered a PHP error along the lines of:

Notice: Undefined index: 0000000062a3a7690000000033c91b26 in doctrine/lib/Doctrine/ORM/UnitOfWork.php line 2202
#0 doctrine/lib/Doctrine/ORM/Persisters/ManyToManyPersister.php(267): Doctrine\ORM\UnitOfWork->getEntityIdentifier(Object(Item))
#1 doctrine/lib/Doctrine/ORM/PersistentCollection.php(411): Doctrine\ORM\Persisters\ManyToManyPersister->contains(Object(Doctrine\ORM\PersistentCollection), Object(Item))
#2 Test.php(71): Doctrine\ORM\PersistentCollection->contains(Object(Item))

lib/Doctrine/ORM/Persisters/ManyToManyPersister.php
@@ -254,7 +254,9 @@ public function contains(PersistentCollection $coll, $element)
254 254
         $uow = $this->_em->getUnitOfWork();
255 255
 
256 256
         // shortcut for new entities
257  
-        if ($uow->getEntityState($element, UnitOfWork::STATE_NEW) == UnitOfWork::STATE_NEW) {
  257
+        $entityState = $uow->getEntityState($element, UnitOfWork::STATE_NEW);
  258
+        if ($entityState == UnitOfWork::STATE_NEW || 
2
Christophe Coevoet
stof added a note January 15, 2012

couldn't it be a strict comparison here ?

Yea I feel like it could, but the comparison already there was "==". There are other places all over the code base using that comparison when it could be strict so i figured it was probably for a reason, whether that be coding style or a micro optimisation or something else. I've changed it and commited

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Guilherme Blanco guilhermeblanco merged commit c1012f7 into from January 16, 2012
Guilherme Blanco guilhermeblanco closed this January 16, 2012
Guilherme Blanco

@beberlei please merge into 2.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
6  lib/Doctrine/ORM/Persisters/ManyToManyPersister.php
@@ -254,7 +254,9 @@ public function contains(PersistentCollection $coll, $element)
254 254
         $uow = $this->_em->getUnitOfWork();
255 255
 
256 256
         // shortcut for new entities
257  
-        if ($uow->getEntityState($element, UnitOfWork::STATE_NEW) == UnitOfWork::STATE_NEW) {
  257
+        $entityState = $uow->getEntityState($element, UnitOfWork::STATE_NEW);
  258
+        if ($entityState === UnitOfWork::STATE_NEW || 
  259
+            ($entityState === UnitOfWork::STATE_MANAGED && $uow->isScheduledForInsert($element))) {
258 260
             return false;
259 261
         }
260 262
 
@@ -275,7 +277,7 @@ public function removeElement(PersistentCollection $coll, $element)
275 277
         $uow = $this->_em->getUnitOfWork();
276 278
 
277 279
         // shortcut for new entities
278  
-        if ($uow->getEntityState($element, UnitOfWork::STATE_NEW) == UnitOfWork::STATE_NEW) {
  280
+        if ($uow->getEntityState($element, UnitOfWork::STATE_NEW) === UnitOfWork::STATE_NEW) {
279 281
             return false;
280 282
         }
281 283
 
55  tests/Doctrine/Tests/ORM/Functional/ManyToManyExtraLazyContainsTest.php
... ...
@@ -0,0 +1,55 @@
  1
+<?php
  2
+
  3
+namespace Doctrine\Tests\ORM\Functional;
  4
+
  5
+use Doctrine\Common\Collections\ArrayCollection;
  6
+require_once __DIR__ . '/../../TestInit.php';
  7
+
  8
+/**
  9
+ * 
  10
+ */
  11
+class ManyToManyExtraLazyContainsTest extends \Doctrine\Tests\OrmFunctionalTestCase
  12
+{
  13
+    public function setUp()
  14
+    {
  15
+        $this->useModelSet('company');
  16
+        parent::setUp();
  17
+    }
  18
+
  19
+    public function testManyToManyExtraLazyContainsAddedPendingInsertEntityIsTrue()
  20
+    {
  21
+        $contract = new \Doctrine\Tests\Models\Company\CompanyFlexContract();
  22
+
  23
+        $this->_em->persist($contract);
  24
+        $this->_em->flush();
  25
+        
  26
+        $this->_em->clear();
  27
+        $contract = $this->_em->find('Doctrine\Tests\Models\Company\CompanyFlexContract', $contract->getId());
  28
+        
  29
+        $pendingInsertManager = new \Doctrine\Tests\Models\Company\CompanyManager();
  30
+        $this->_em->persist($pendingInsertManager);
  31
+        $contract->getManagers()->add($pendingInsertManager);
  32
+        
  33
+        $result = $contract->getManagers()->contains($pendingInsertManager);
  34
+
  35
+        $this->assertTrue($result);
  36
+    }
  37
+
  38
+    public function testManyToManyExtraLazyContainsNonAddedPendingInsertEntityIsFalse()
  39
+    {
  40
+        $contract = new \Doctrine\Tests\Models\Company\CompanyFlexContract();
  41
+
  42
+        $this->_em->persist($contract);
  43
+        $this->_em->flush();
  44
+        
  45
+        $this->_em->clear();
  46
+        $contract = $this->_em->find('Doctrine\Tests\Models\Company\CompanyFlexContract', $contract->getId());
  47
+        
  48
+        $pendingInsertManager = new \Doctrine\Tests\Models\Company\CompanyManager();
  49
+        $this->_em->persist($pendingInsertManager);
  50
+        
  51
+        $result = $contract->getManagers()->contains($pendingInsertManager);
  52
+
  53
+        $this->assertFalse($result);
  54
+    }
  55
+}
Commit_comment_tip

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.