Doctrine 2 - ORM

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

Details

  • Type: Bug Bug
  • Status: Closed 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

Ascending order - Click to sort in descending order
Hide
Permalink
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
Permalink
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
Permalink
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.
Hide
Permalink
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!

People

  • Assignee:
    Roman S. Borschel
    Reporter:
    Nico Kaiser
Vote (1)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved: