Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix for PreUpdate listeners dispatching #126

Closed
wants to merge 1 commit into from
Closed

fix for PreUpdate listeners dispatching #126

wants to merge 1 commit into from

Conversation

vgarvardt
Copy link

Catchable Fatal Error: Argument 3 passed to Doctrine\ORM\Event\PreUpdateEventArgs::__construct() must be an array, null given

Catchable Fatal Error: Argument 3 passed to Doctrine\ORM\Event\PreUpdateEventArgs::__construct() must be an array, null given
@beberlei
Copy link
Member

Why empty()? Can this just be if ($this->changes...[$oid]) as before?

@vgarvardt
Copy link
Author

When $this->entityChangeSets[$oid] is not set PHP Notice is thrown.
Maybe isset(...) may be used instead of !empty(...), but not the original code IMO.

dcousineau added a commit to dcousineau/doctrine2 that referenced this pull request Feb 7, 2012
@asm89
Copy link
Member

asm89 commented Mar 11, 2012

We've been trying to come up with ways of how this can happen and were unable to reproduce it. Can you re-open a PR with a failing testcase? Or explain exactly what your code does?

@asm89 asm89 closed this Mar 11, 2012
@dcousineau
Copy link

Closing this ticket was premature, our application also encountered this edge case however it is understandably difficult to reproduce. The code simply does sanity checks on keys to ensure they exist before accessing them, which is a good habit to have when working with PHP.

@jdewit
Copy link

jdewit commented May 14, 2012

I just came across this error today using a postUpdate listener in Symfony2 using Doctrine 2.2.x.

My listener is similar to the one in this post

This patch fixed the error.

@stof
Copy link
Member

stof commented May 14, 2012

You should never flush from inside a lifecycle listener. This means that you are nesting a flush inside another one, leading to unexpected behaviors.

@jdewit
Copy link

jdewit commented May 14, 2012

Thanks for the tip stof. Would firing a separate event after flush to update another entity be the way to go?

@calumbrodie
Copy link
Contributor

I would also be interested in what the correct thing to do in this instance is. I'm using a lifecycle listener in order to log changes on my entities (I create a new 'Log' entity detailing the nature of the change and then persist/flush it).

I perform this after the initial flush has happened (postUpdate/postPersist).

I've not ran into any issue with doing this yet, but it did feel like the wrong thing to do (a flush inside a flush).

@ostretsov
Copy link

Also interested in best practice

@stephpy
Copy link
Contributor

stephpy commented Nov 7, 2012

I've same issue atm. I would like to update a counter from an other one entity on postUpdate/persist/remove, I guess i have to call flush method during theses lifecycle. Any other solution ? ping @stof

@guilhermeblanco
Copy link
Member

When interested to update an entity during the postInsert or postUpdate, as everyone already said, you can't call flush.
The alternative, since the graph was already calculated (so you cannot just update the entity), is to schedule an extra update. Here is an example:

class EntryListener extends ContainerAware
{
    /**
     * @var array
     */
    protected $postInsertScheduleList = array();

    /**
     * Handle Doctrine onFlush event
     *
     * @param \Doctrine\ORM\Event\OnFlushEventArgs $eventArgs "On Flush" event arguments
     */
    public function onFlush(OnFlushEventArgs $eventArgs)
    {
        $entityManager = $eventArgs->getEntityManager();
        $unitOfWork    = $entityManager->getUnitOfWork();

        foreach ($unitOfWork->getScheduledEntityInsertions() as $entity) {
            $entry = $this->scheduleEntryForInsert(EntryType::TYPE_CREATE, $entity, $entityManager);

            $this->postInsertScheduleList[spl_object_hash($entity)] = $entry;
        }

        foreach ($unitOfWork->getScheduledEntityUpdates() as $entity) {
            $this->scheduleEntryForInsert(EntryType::TYPE_UPDATE, $entity, $entityManager);
        }

        foreach ($unitOfWork->getScheduledEntityDeletions() as $entity) {
            $this->scheduleEntryForInsert(EntryType::TYPE_DELETE, $entity, $entityManager);
        }
    }

    /**
     * Post-persist event hook
     *
     * @param LifecycleEventArgs $args Life cycle event arguments
     */
    public function postPersist(LifecycleEventArgs $args)
    {
        $entity     = $args->getEntity();
        $objectHash = spl_object_hash($entity);

        if ( ! isset($this->postInsertScheduleList[$objectHash])) {
            return;
        }

        $entityManager = $args->getEntityManager();
        $unitOfWork    = $entityManager->getUnitOfWork();
        $classMetadata = $entityManager->getClassMetadata(get_class($entity));

        $entityIdentifierList = $unitOfWork->getEntityIdentifier($entity);

        $unitOfWork->scheduleExtraUpdate(
            $this->postInsertScheduleList[$objectHash],
            array(
                'objectId' => array(null, $entityIdentifierList[$classMetadata->identifier[0]])
            )
        );

        unset($this->postInsertScheduleList[$objectHash]);
    }

    /**
     * Schedule a Journal Entry for insertion
     *
     * @param string                                        $entryType     Type of entry
     * @param Entity $entity        Entity
     * @param \Doctrine\ORM\EntityManager                   $entityManager Entity manager
     *
     * @return Entry
     */
    protected function scheduleEntryForInsert($entryType, $entity, $entityManager)
    {
        $service = $this->container->get('entry_builder');
        $entry   = $service->buildEntry($entryType, $entity, $entityManager);

        $unitOfWork    = $entityManager->getUnitOfWork();
        $classMetadata = $entityManager->getClassMetadata(get_class($entry));

        $entityManager->persist($entry);
        $unitOfWork->computeChangeSet($classMetadata, $entry);

        return $entry;
    }
}

@stephpy
Copy link
Contributor

stephpy commented Nov 7, 2012

I tried this solution, it fixes my issue, thanks ;)

@jmather
Copy link

jmather commented Jan 9, 2013

I got popped over here by someone -- I wanted to say, this is an awesome solution @guilhermeblanco. Amazingly elegant.

guilhermeblanco added a commit that referenced this pull request Jan 23, 2013
Fix some typos in annotations reference
@pedrofs
Copy link

pedrofs commented Feb 25, 2013

@guilhermeblanco and if I want to update another entity inside the postUpdate. I tried something similar to what you said:

public function postUpdate($eventArgs)
    {
        $this->em = $eventArgs->getEntityManager();
        $this->uow = $this->em->getUnitOfWork();

        $entity = $eventArgs->getEntity();

        if ($entity instanceof Team) {
            $teams = $this->getTeamsWithSameName($entity);

                foreach ($teams as $t) {
                    $t->setLabel($entity->getLabel());

                    $this->em->persist($t);
                    $classMetadata = $this->em->getClassMetadata(get_class($t));
                    $this->uow->computeChangeSet($classMetadata, $t);
                }
            }
        }
    }

It is not working as expected. Does not update the entities that I need. If I just use the flush method it throws that error. (I have the preUpdate event set by the FOS users)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet