Skip to content

Loading…

DDC-2996: UnitOfWork::recomputeSingleEntityChangeSet() will not add a new change set #3760

Closed
doctrinebot opened this Issue · 12 comments

2 participants

@doctrinebot

Jira issue originally created by user mnapoli:

I have noticed something weird in the UnitOfWork::recomputeSingleEntityChangeSet() and I suspect it is a bug.

If I write a Listener that, on the onFlush() event, changes an entity: the documentation says to call "recomputeSingleEntityChangeSet()". However, in that method, if a change set DOESN'T EXIST, then no changeset seems to be set at all: https://github.com/doctrine/doctrine2/blob/master/lib/Doctrine/ORM/UnitOfWork.php#L940

        if ($changeSet) {
            if (isset($this->entityChangeSets[$oid])) {
                $this->entityChangeSets[$oid] = array_merge($this->entityChangeSets[$oid], $changeSet);
            }

            $this->originalEntityData[$oid] = $actualData;
        }

As you can see, if a changeset exist, we merge it, but there is no "else".

That "bug" would be consistent with a problem I'm facing: calling recomputeSingleEntityChangeSet() in onFlush() on an entity that previously (at the time of the flush) had no changes, then that entity's changes are not persisted.

Related: people saying recomputeSingleEntityChangeSet() doesn't work (just like me):

So it is a bug? Or is it intended?

@doctrinebot

Comment created by @ocramius:

{quote}That "bug" would be consistent with a problem I'm facing{quote}

[~matthieu] can you provide an example of what your listener is doing? And yes, the assumption is that a changeset for that entity already exists (otherwise it is an insert or delete operation)

@doctrinebot

Comment created by mnapoli:

I have User which has a UserPreference. When UserPreference is updated in any way, I need to set "User::updatedAt" with the current time. (FYI this is because Timestampable doesn't monitor changes on a "sub-entity").

So indeed, when User is not changed but UserPreference is, the listener will introduce changes on User.

@doctrinebot

Comment created by @ocramius:

[~matthieu] tricky. What if the suggested else would just trigger UnitOfWork#computeSingleEntityChangeSet()? I'm not sure if that's going to be a BC break though.

You can workaround that by doing it manually to see if your problem is fixed by this:

if ($uow->getEntityChangeSet()) {
$uow->recomputeSingleEntityChangeSet(...);
} else {
$uow->computeSingleEntityChangeSet(...);
}

@doctrinebot

Comment created by mnapoli:

:( I was going to try that but computeSingleEntityChangeSet() is private anyway...

What if the suggested else would just trigger UnitOfWork#computeSingleEntityChangeSet()? I'm not sure if that's going to be a BC break though.

That would be the best solution I think, but I have no idea about BC.

@doctrinebot

Comment created by juanmaia:

Is anyone taking a look at this?

I also had a problem that maybe is related to this one.
I was calling (computeChangeSet) inside the same listener, 3 different times, for 3 different entities, but the first one doesnt change the entity (the other 2 was just fine). Then I tried recomputeSingleEntityChangeSet and nothing happened to the entities (the entities are already persisted and I'm just trying to modifying it).

@doctrinebot

Comment created by @beberlei:

Fixed

@doctrinebot

Issue was closed with resolution "Fixed"

@doctrinebot

Comment created by mnapoli:

Awesome news thanks

@doctrinebot

Comment created by zhukv:

I have a problem after update with this commit.
My application control entity insertions/updated and change another field in entity.

Logic example:

Check each entity in insertions
Check each entity in updated

And i call recompute change set in entity in check insertion, this entity added to updated, and the second step not good working, because entity INSERTIONS!

Subscriber, for example:

<?php

namespace Acme\DemoBundle\EventListener\Doctrine;

use Acme\DemoBundle\Entity\Top;
use Doctrine\Common\EventSubscriber;
use Doctrine\ORM\Event\OnFlushEventArgs;
use Doctrine\ORM\Events;

class TopChangedSubscriber implements EventSubscriber
{
    /****
     * On flush event
     *
     * @param OnFlushEventArgs $event
     */
    public function onFlush(OnFlushEventArgs $event)
    {
        $em = $event->getEntityManager();
        $uow = $em->getUnitOfWork();

        $topMetadata = $em->getClassMetadata('DemoBundle:Top');

        foreach ($uow->getScheduledEntityInsertions() as $top) {
            if ($top instanceof Top) {
                $top->setChanged(301 -  $top->getPosition());

                // So, i call to recompute, because entity changed.
                // BUG #1? This entity added to entity updates, and getScheduledEntityUpdates returned incorrect data
                // BUG #2? This entity has been insert (1 SQL query) and update (2 SQL query)!
                $uow->recomputeSingleEntityChangeSet($topMetadata, $top);
            }
        }

        foreach ($uow->getScheduledEntityUpdates() as $top) {
            if ($top instanceof Top) {
                $changes = $uow->getEntityChangeSet($top);

                if (isset($changes['position'])) {
                    list ($oldPosition, $newPosition) = $changes['position'];

                    $top->setChanged($oldPosition - $newPosition);

                    $uow->recomputeSingleEntityChangeSet($topMetadata, $top);
                }
            }
        }
    }

    /****
     * {@inheritDoc}
     */
    public function getSubscribedEvents()
    {
        return array(Events::onFlush);
    }
}

Thank.

@doctrinebot

Comment created by @ocramius:

[~zhukv] as I've already explained in d473824#commitcomment-5796295, you are not supposed to re-compute changes for insertions.

@doctrinebot

Comment created by zhukv:

Ok, i can change entity field in insert action?

@beberlei beberlei was assigned by doctrinebot
@doctrinebot doctrinebot added this to the 2.4.3 milestone
@doctrinebot doctrinebot closed this
@doctrinebot doctrinebot added the Bug label
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.