Doctrine 2 - ORM
  1. Doctrine 2 - ORM
  2. DDC-2409

Merge operation tries to add new detached entities to indentity map and load them as proxies

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.3.1
    • Fix Version/s: 2.4, 2.3.4
    • Component/s: ORM
    • Security Level: All
    • Labels:
      None

      Description

      class A
          {
             /**
              *  @ORM\ManyToOne(targetEntity= "B"...
              *  @ORM\JoinColumn(name=" ...
              */
              protected $b;
              
              public function getB()
              {
                  return $this->b;
              }
              
              public function setB($b)
              {
                  $this->b = $b;
              }
      }
          
          class B
      	{
             /**
              * As
              *
              * @var \Doctrine\Common\Collections\Collection
              *
              * @ORM\OneToMany(targetEntity="A", mappedBy="B")
              */
      		protected $As;
          }
      	
      	$b = new \B();
      	
      	$a = $em->find('A', 123);
      	
      	$a->setB($b);
      	
      	$em->detach($a);
      	
      	$em->detach($b);
      	
      	$b = $em->merge($b); // notice that $b now is merged
      	
      	$a = $em->merge($a); //  hangs as it creates the proxy for $b and tries to load a though __load even though $b is already managed
      

      Couple of possible issues in the following code from doMerge:

      if ($assoc2['type'] & ClassMetadata::TO_ONE) {
                              $other = $prop->getValue($entity);
                              if ($other === null) {
                                  $prop->setValue($managedCopy, null);
                              } else if ($other instanceof Proxy && !$other->__isInitialized__) {
                                  // do not merge fields marked lazy that have not been fetched.
                                  continue;
                              } else if ( ! $assoc2['isCascadeMerge']) {
                                  if ($this->getEntityState($other, self::STATE_DETACHED) !== self::STATE_MANAGED) {
                                      $targetClass = $this->em->getClassMetadata($assoc2['targetEntity']);
                                      $relatedId = $targetClass->getIdentifierValues($other);
      
                                      if ($targetClass->subClasses) {
                                          $other = $this->em->find($targetClass->name, $relatedId);
                                      } else {
                                          $other = $this->em->getProxyFactory()->getProxy($assoc2['targetEntity'], $relatedId);
                                          $this->registerManaged($other, $relatedId, array());
                                      }
                                  }
                                  $prop->setValue($managedCopy, $other);
                              }
      
           $relatedId = $targetClass->getIdentifierValues($other);
      

      $relatedId is emply as the detached $other was never flushed. It should never be used to add this entity to the identityMap ($this->registerManaged($other, $relatedId, array())

      $other = $this->em->getProxyFactory()->getProxy($assoc2['targetEntity'], $relatedId);
      

      This should never use the proxy factory for new detached entities as they are already merged back ($b). This method seems to not have any means to find managed $b.

      If $b = $em->merge($b); was not called, the method would probably have worked but I think it is not right to rely on that calling or not calling certain methods or their order.

        Activity

        Oleg Namaka created issue -
        Oleg Namaka made changes -
        Field Original Value New Value
        Status Open [ 1 ] Resolved [ 5 ]
        Resolution Incomplete [ 4 ]
        Oleg Namaka made changes -
        Resolution Incomplete [ 4 ]
        Status Resolved [ 5 ] Reopened [ 4 ]
        Oleg Namaka made changes -
        Description {code}
        class A
            {
               /**
                * @ORM\ManyToOne(targetEntity= "B"...
                * @ORM\JoinColumn(name=" ...
                */
                protected $b;
                
                public function getB()
                {
                    return $this->b;
                }
                
                public function setB($b)
                {
                    $this->b = $b;
                }
        }
            
            class B
        {
               /**
                * As
                *
                * @var \Doctrine\Common\Collections\Collection
                *
                * @ORM\OneToMany(targetEntity="A", mappedBy="B")
                */
        protected $As;
            }

        $b = new \B();

        $a = $em->find('A', 123);

        $a->setB($b);

        $em->detach($a);

        $em->detach($b);

        $b = $em->merge($b); // notice that $b now is merged

        $a = $em->merge($a); // hangs as it creates the proxy for $b and tries to load a though __load even though $b is already managed
        {code}

        Couple of possible issues in the following code from doMerge:
        {code}
        if ($assoc2['type'] & ClassMetadata::TO_ONE) {
                                $other = $prop->getValue($entity);
                                if ($other === null) {
                                    $prop->setValue($managedCopy, null);
                                } else if ($other instanceof Proxy && !$other->__isInitialized__) {
                                    // do not merge fields marked lazy that have not been fetched.
                                    continue;
                                } else if ( ! $assoc2['isCascadeMerge']) {
                                    if ($this->getEntityState($other, self::STATE_DETACHED) !== self::STATE_MANAGED) {
                                        $targetClass = $this->em->getClassMetadata($assoc2['targetEntity']);
                                        $relatedId = $targetClass->getIdentifierValues($other);

                                        if ($targetClass->subClasses) {
                                            $other = $this->em->find($targetClass->name, $relatedId);
                                        } else {
                                            $other = $this->em->getProxyFactory()->getProxy($assoc2['targetEntity'], $relatedId);
                                            $this->registerManaged($other, $relatedId, array());
                                        }
                                    }
                                    $prop->setValue($managedCopy, $other);
                                }
        {code}

        {code}
             $relatedId = $targetClass->getIdentifierValues($other);
        {code}
        $relatedId is emply as the detached $other was never flushed. It should never be used to add this entity to the identityMap ($this->registerManaged($other, $relatedId, array());)

        {code}
        $other = $this->em->getProxyFactory()->getProxy($assoc2['targetEntity'], $relatedId);
        {code}
        This should never use the proxy factory for new detached entities as they are already merged back ($b). This method seems to not have any means to find managed $b.

        If $b = $em->merge($b); was not done, the method would probably worked but I think it is not right to rely on that.


        Oleg Namaka made changes -
        Description {code}
        class A
            {
               /**
                * @ORM\ManyToOne(targetEntity= "B"...
                * @ORM\JoinColumn(name=" ...
                */
                protected $b;
                
                public function getB()
                {
                    return $this->b;
                }
                
                public function setB($b)
                {
                    $this->b = $b;
                }
        }
            
            class B
        {
               /**
                * As
                *
                * @var \Doctrine\Common\Collections\Collection
                *
                * @ORM\OneToMany(targetEntity="A", mappedBy="B")
                */
        protected $As;
            }

        $b = new \B();

        $a = $em->find('A', 123);

        $a->setB($b);

        $em->detach($a);

        $em->detach($b);

        $b = $em->merge($b); // notice that $b now is merged

        $a = $em->merge($a); // hangs as it creates the proxy for $b and tries to load a though __load even though $b is already managed
        {code}

        Couple of possible issues in the following code from doMerge:
        {code}
        if ($assoc2['type'] & ClassMetadata::TO_ONE) {
                                $other = $prop->getValue($entity);
                                if ($other === null) {
                                    $prop->setValue($managedCopy, null);
                                } else if ($other instanceof Proxy && !$other->__isInitialized__) {
                                    // do not merge fields marked lazy that have not been fetched.
                                    continue;
                                } else if ( ! $assoc2['isCascadeMerge']) {
                                    if ($this->getEntityState($other, self::STATE_DETACHED) !== self::STATE_MANAGED) {
                                        $targetClass = $this->em->getClassMetadata($assoc2['targetEntity']);
                                        $relatedId = $targetClass->getIdentifierValues($other);

                                        if ($targetClass->subClasses) {
                                            $other = $this->em->find($targetClass->name, $relatedId);
                                        } else {
                                            $other = $this->em->getProxyFactory()->getProxy($assoc2['targetEntity'], $relatedId);
                                            $this->registerManaged($other, $relatedId, array());
                                        }
                                    }
                                    $prop->setValue($managedCopy, $other);
                                }
        {code}

        {code}
             $relatedId = $targetClass->getIdentifierValues($other);
        {code}
        $relatedId is emply as the detached $other was never flushed. It should never be used to add this entity to the identityMap ($this->registerManaged($other, $relatedId, array());)

        {code}
        $other = $this->em->getProxyFactory()->getProxy($assoc2['targetEntity'], $relatedId);
        {code}
        This should never use the proxy factory for new detached entities as they are already merged back ($b). This method seems to not have any means to find managed $b.

        If $b = $em->merge($b); was not done, the method would probably worked but I think it is not right to rely on that.


        {code}
        class A
            {
               /**
                * @ORM\ManyToOne(targetEntity= "B"...
                * @ORM\JoinColumn(name=" ...
                */
                protected $b;
                
                public function getB()
                {
                    return $this->b;
                }
                
                public function setB($b)
                {
                    $this->b = $b;
                }
        }
            
            class B
        {
               /**
                * As
                *
                * @var \Doctrine\Common\Collections\Collection
                *
                * @ORM\OneToMany(targetEntity="A", mappedBy="B")
                */
        protected $As;
            }

        $b = new \B();

        $a = $em->find('A', 123);

        $a->setB($b);

        $em->detach($a);

        $em->detach($b);

        $b = $em->merge($b); // notice that $b now is merged

        $a = $em->merge($a); // hangs as it creates the proxy for $b and tries to load a though __load even though $b is already managed
        {code}

        Couple of possible issues in the following code from doMerge:
        {code}
        if ($assoc2['type'] & ClassMetadata::TO_ONE) {
                                $other = $prop->getValue($entity);
                                if ($other === null) {
                                    $prop->setValue($managedCopy, null);
                                } else if ($other instanceof Proxy && !$other->__isInitialized__) {
                                    // do not merge fields marked lazy that have not been fetched.
                                    continue;
                                } else if ( ! $assoc2['isCascadeMerge']) {
                                    if ($this->getEntityState($other, self::STATE_DETACHED) !== self::STATE_MANAGED) {
                                        $targetClass = $this->em->getClassMetadata($assoc2['targetEntity']);
                                        $relatedId = $targetClass->getIdentifierValues($other);

                                        if ($targetClass->subClasses) {
                                            $other = $this->em->find($targetClass->name, $relatedId);
                                        } else {
                                            $other = $this->em->getProxyFactory()->getProxy($assoc2['targetEntity'], $relatedId);
                                            $this->registerManaged($other, $relatedId, array());
                                        }
                                    }
                                    $prop->setValue($managedCopy, $other);
                                }
        {code}

        {code}
             $relatedId = $targetClass->getIdentifierValues($other);
        {code}
        $relatedId is emply as the detached $other was never flushed. It should never be used to add this entity to the identityMap ($this->registerManaged($other, $relatedId, array());)

        {code}
        $other = $this->em->getProxyFactory()->getProxy($assoc2['targetEntity'], $relatedId);
        {code}
        This should never use the proxy factory for new detached entities as they are already merged back ($b). This method seems to not have any means to find managed $b.

        If $b = $em->merge($b); was not called, the method would probably have worked but I think it is not right to rely on that calling or not calling certain methods or their order.


        Fabio B. Silva made changes -
        Assignee Benjamin Eberlei [ beberlei ] Fabio B. Silva [ fabio.bat.silva ]
        Hide
        Benjamin Eberlei added a comment -

        Merged Fabios Pull Request

        Show
        Benjamin Eberlei added a comment - Merged Fabios Pull Request
        Benjamin Eberlei made changes -
        Status Reopened [ 4 ] Resolved [ 5 ]
        Fix Version/s 2.4 [ 10321 ]
        Fix Version/s 2.3.4 [ 10420 ]
        Resolution Fixed [ 1 ]
        Hide
        Oleg Namaka added a comment - - edited
        Show
        Oleg Namaka added a comment - - edited https://github.com/doctrine/doctrine2/pull/655

        This list may be incomplete, as errors occurred whilst retrieving source from linked applications:

        • Request to http://www.doctrine-project.org/fisheye/ failed: Error in remote call to 'FishEye 0 (http://www.doctrine-project.org/fisheye/)' (http://www.doctrine-project.org/fisheye) [AbstractRestCommand{path='/rest-service-fe/search-v1/crossRepositoryQuery', params={query=DDC-2409, expand=changesets[0:20].revisions[0:29],reviews}, methodType=GET}] : Received status code 503 (Service Temporarily Unavailable)

          People

          • Assignee:
            Fabio B. Silva
            Reporter:
            Oleg Namaka
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: