Doctrine 2 - ORM
  1. Doctrine 2 - ORM
  2. DDC-74

Updates get lost when Lifecycle Events (@PreUpdate) are invoked

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.0-ALPHA4
    • Component/s: Mapping Drivers
    • Security Level: All
    • Labels:
      None

      Description

      When Lifecycle Events are invoked on entity update (@PreUpdate), the entity is not updated properly in the database.

      This code creates a User object, sets its name to "Bob" and its value to empty, then updates the object and updates the name to "Alice".

      $user = new User;
      $user->setName('Bob');
      $user->setValue('');
      $em->persist($user);
      $em->flush();
      $user->setName('Alice');
      $em->flush();

      However, when the User class has a @PreUpdate event that e.g. sets the value to "Hello World", the name change gets lost and only the value is updated by the second flush() call.

      This is a critical bug which prevents creation of entities that simulate the "Timestampable" behaviour of Doctrine 1.x...

        Activity

        Nico Kaiser created issue -
        Roman S. Borschel made changes -
        Field Original Value New Value
        Priority Major [ 3 ] Critical [ 2 ]
        Hide
        Benjamin Eberlei added a comment -

        I think this might be a hen-egg problem.

        @PreUpdate is only invoked after it is calculated that a change occured on all those entities that have updates. After a change in @PreUpdate events there would have to be another calculation of changes on all those entities, which would probably mean a significant performance hit.

        Show
        Benjamin Eberlei added a comment - I think this might be a hen-egg problem. @PreUpdate is only invoked after it is calculated that a change occured on all those entities that have updates. After a change in @PreUpdate events there would have to be another calculation of changes on all those entities, which would probably mean a significant performance hit.
        Hide
        Eric Durand-Tremblay added a comment - - edited

        I may not know all the consequences of this, but I think I have a fix for this bug.

        In the class ORM\UnitOfWork

        As I understand it, computeSingleEntityChangeSet() is called to update the changeset and _originalEntityData is set to the current values.

        But, I see that the old changes are lost.

        If i modify the function computeSingleEntityChangeSet to merge the changeset, it works.

        At line 656 replace

        $this->_entityChangeSets[$oid] = $changeSet;
        

        By :

        if($this->_entityChangeSets[$oid]){
            $this->_entityChangeSets[$oid] += $changeSet;
        }
        else {
            $this->_entityChangeSets[$oid] = $changeSet;
        }
        

        Here is my test case :

        $qb = new \Doctrine\ORM\QueryBuilder($em);
        $qb->select('fna')
        			->from('Entity\FNA', 'fna')
        			->andwhere($qb->expr()->eq('fna.id', ':fna_id'));
        $qb->setParameter('fna_id', 1);
        $query = $qb->getQuery();
        
        $fna = $query->getSingleResult();
        		
        $fna->setStatus('COMPLETED');
        $em->persist($fna);
        $em->flush();
        

        AND The preUdate :

        /** 
         * @PreUpdate
        */
        public function onPreUpdate($args=false)
         {
                $this->modified_at = new \DateTime();
        }
        
        Show
        Eric Durand-Tremblay added a comment - - edited I may not know all the consequences of this, but I think I have a fix for this bug. In the class ORM\UnitOfWork As I understand it, computeSingleEntityChangeSet() is called to update the changeset and _originalEntityData is set to the current values. But, I see that the old changes are lost. If i modify the function computeSingleEntityChangeSet to merge the changeset, it works. At line 656 replace $ this ->_entityChangeSets[$oid] = $changeSet; By : if ($ this ->_entityChangeSets[$oid]){ $ this ->_entityChangeSets[$oid] += $changeSet; } else { $ this ->_entityChangeSets[$oid] = $changeSet; } Here is my test case : $qb = new \Doctrine\ORM\QueryBuilder($em); $qb->select('fna') ->from('Entity\FNA', 'fna') ->andwhere($qb->expr()->eq('fna.id', ':fna_id')); $qb->setParameter('fna_id', 1); $query = $qb->getQuery(); $fna = $query->getSingleResult(); $fna->setStatus('COMPLETED'); $em->persist($fna); $em->flush(); AND The preUdate : /** * @PreUpdate */ public function onPreUpdate($args= false ) { $ this ->modified_at = new \DateTime(); }
        Hide
        Roman S. Borschel added a comment -

        Indeed this looks like a good fix except that the addition has to be the other way around so that when the same field is changed twice, first before the flush and then in a lifecycle callback/event the change from the callback prevails.

        I will work on this and write a test for it.

        Thanks Eric.

        Show
        Roman S. Borschel added a comment - Indeed this looks like a good fix except that the addition has to be the other way around so that when the same field is changed twice, first before the flush and then in a lifecycle callback/event the change from the callback prevails. I will work on this and write a test for it. Thanks Eric.
        Roman S. Borschel made changes -
        Status Open [ 1 ] In Progress [ 3 ]
        Hide
        Roman S. Borschel added a comment -

        Fixed now.

        Thanks Nico for reporting and thanks Eric for the suggestion!

        Show
        Roman S. Borschel added a comment - Fixed now. Thanks Nico for reporting and thanks Eric for the suggestion!
        Roman S. Borschel made changes -
        Status In Progress [ 3 ] Closed [ 6 ]
        Fix Version/s 2.0-ALPHA4 [ 10036 ]
        Resolution Fixed [ 1 ]
        Benjamin Eberlei made changes -
        Workflow jira [ 10267 ] jira-feedback [ 15452 ]
        Benjamin Eberlei made changes -
        Workflow jira-feedback [ 15452 ] jira-feedback2 [ 17316 ]
        Benjamin Eberlei made changes -
        Workflow jira-feedback2 [ 17316 ] jira-feedback3 [ 19573 ]

        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-74, expand=changesets[0:20].revisions[0:29],reviews}, methodType=GET}] : Received status code 503 (Service Temporarily Unavailable)

          People

          • Assignee:
            Roman S. Borschel
            Reporter:
            Nico Kaiser
          • Votes:
            1 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: