Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

added failing test-case for triggering updates from listener #441

Closed
wants to merge 1 commit into from

3 participants

@schmittjoh

My guess is that it is caused by the self-referencing association as I had to add it before I saw the bug.

@stof stof commented on the diff
.../ORM/Functional/AdditionalUpdatesFromListenerTest.php
((62 lines not shown))
+
+ public function postUpdate(\Doctrine\ORM\Event\LifecycleEventArgs $event)
+ {
+ $entity = $event->getEntity();
+
+ switch (true) {
+ case $entity instanceof AUFL_TriggeringEntity:
+ if ('failed' === $entity->state) {
+ $em = $event->getEntityManager();
+ $otherEntity = $em->createQuery("SELECT e FROM Doctrine\Tests\ORM\Functional\AUFL_OtherEntity e WHERE e.id = :id")
+ ->setParameter('id', $entity->relatedId)
+ ->getOneOrNullResult();
+ if ($otherEntity !== null) {
+ $otherEntity->state = 'failed';
+ $em->persist($otherEntity);
+ $em->flush();
@stof
stof added a note

you should never flush from a listener as the listeners are called during the flush.

I think it is quite common to do that, and it works in most cases.

There are however events where you should not do that.

@stof
stof added a note

Quite common ? in bug reports yes :)

Can you point me to these bug reports?

I briefly talked with @beberlei about this yesterday, and he thought it was a bug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@guilhermeblanco

It doesn't seem like a bug to me.
The problem happens because postUpdate is executed after the commit order calculator is executed. If you are attempting to run a code like that, you need to rely on onFlush.

@schmittjoh

The problem here is that onFlush does not work because of post-insertion ID generation.

I had taken a look at the UoW, but unfortunately the state during a flush is not really isolated, so indeed calling flush while in a flush causes weird things.

There were two solutions I found:

  1. use a second entity manager which was fortunately possible
  2. use the connection to directly execute queries (possibly causing state errors in loaded entities)

I'm not sure, but it would be cool if Doctrine would provide a better solution here. One option might be encapsulating the flush state in a separate object which is created when "->commit" is called, and passing this object along (but that would be a major change). Another option would be to allow the user to call a method like "UoW->reflush()" which if called during a flush initiates another flush operation after that.

@guilhermeblanco

@schmittjoh that is the situation where you need to schedule an extra update. That's how you it correctly.

I know this is odd, I want to refactor how and where postUpdate is called, but it breaks BC. We're going to have to wait for ORM 3.0, where we need to refactor not only the UnitOfWork, but also Persisters and Hydrators.

@schmittjoh

Ok, let's close this for now then.

@schmittjoh schmittjoh closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
125 tests/Doctrine/Tests/ORM/Functional/AdditionalUpdatesFromListenerTest.php
@@ -0,0 +1,125 @@
+<?php
+
+namespace Doctrine\Tests\ORM\Functional;
+
+class AdditionalUpdatesFromListenerTest extends \Doctrine\Tests\OrmFunctionalTestCase
+{
+ public function testUoWDoesNotThrowErrorWhenUpdatingSecondEntity()
+ {
+ $otherEntity = new AUFL_OtherEntity();
+ $this->_em->persist($otherEntity);
+ $this->_em->flush($otherEntity);
+ $this->_em->clear();
+
+ $triggeringEntity = new AUFL_TriggeringEntity();
+ $triggeringEntity->relatedId = $otherEntity->id;
+ $this->_em->persist($triggeringEntity);
+
+ $depTriggeringEntity = new AUFL_TriggeringEntity();
+ $depTriggeringEntity->deps->add($triggeringEntity);
+ $this->_em->persist($depTriggeringEntity);
+ $this->_em->flush();
+ $this->_em->clear();
+
+ $this->_em->getConnection()->beginTransaction();
+ $triggeringEntity = $this->_em->find('Doctrine\Tests\ORM\Functional\AUFL_TriggeringEntity', $triggeringEntity->id);
+ foreach ($this->_em->createQuery("SELECT e FROM Doctrine\Tests\ORM\Functional\AUFL_TriggeringEntity e WHERE :e MEMBER OF e.deps")
+ ->setParameter('e', $triggeringEntity)->getResult() as $depEntity) {
+ $depEntity->state = 'failed';
+ $this->_em->persist($depEntity);
+ }
+ $triggeringEntity->state = 'failed';
+ $this->_em->persist($triggeringEntity);
+ $this->_em->flush();
+ $this->_em->getConnection()->commit();
+
+ $this->_em->clear();
+
+ $this->assertEquals('failed', $this->_em->find('Doctrine\Tests\ORM\Functional\AUFL_TriggeringEntity', $triggeringEntity->id)->state);
+ $this->assertEquals('failed', $this->_em->find('Doctrine\Tests\ORM\Functional\AUFL_OtherEntity', $otherEntity->id)->state);
+ }
+
+ protected function setUp()
+ {
+ parent::setUp();
+
+ $this->_schemaTool->createSchema(array(
+ $this->_em->getClassMetadata('Doctrine\Tests\ORM\Functional\AUFL_TriggeringEntity'),
+ $this->_em->getClassMetadata('Doctrine\Tests\ORM\Functional\AUFL_OtherEntity'),
+ ));
+
+ $this->_em->getEventManager()->addEventListener(array('preUpdate', 'postUpdate'), new AUFL_Listener());
+ }
+}
+
+class AUFL_Listener
+{
+ public function preUpdate(\Doctrine\ORM\Event\PreUpdateEventArgs $event)
+ {
+ // This method is just here to make sure the PreUpdate event is being
+ // triggered by the UoW.
+ }
+
+ public function postUpdate(\Doctrine\ORM\Event\LifecycleEventArgs $event)
+ {
+ $entity = $event->getEntity();
+
+ switch (true) {
+ case $entity instanceof AUFL_TriggeringEntity:
+ if ('failed' === $entity->state) {
+ $em = $event->getEntityManager();
+ $otherEntity = $em->createQuery("SELECT e FROM Doctrine\Tests\ORM\Functional\AUFL_OtherEntity e WHERE e.id = :id")
+ ->setParameter('id', $entity->relatedId)
+ ->getOneOrNullResult();
+ if ($otherEntity !== null) {
+ $otherEntity->state = 'failed';
+ $em->persist($otherEntity);
+ $em->flush();
@stof
stof added a note

you should never flush from a listener as the listeners are called during the flush.

I think it is quite common to do that, and it works in most cases.

There are however events where you should not do that.

@stof
stof added a note

Quite common ? in bug reports yes :)

Can you point me to these bug reports?

I briefly talked with @beberlei about this yesterday, and he thought it was a bug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ }
+
+ }
+ break;
+ }
+ }
+}
+
+/**
+ * @Entity
+ */
+class AUFL_TriggeringEntity
+{
+ /** @Id @GeneratedValue(strategy = "AUTO") @Column(type = "integer") */
+ public $id;
+
+ /** @Column(type = "integer", nullable = true) */
+ public $relatedId;
+
+ /** @Column(type = "string") */
+ public $state = 'new';
+
+ /**
+ * @ManyToMany(targetEntity = "AUFL_TriggeringEntity", fetch = "EAGER")
+ * @JoinTable(name="deps",
+ * joinColumns = { @JoinColumn(name = "source_id", referencedColumnName = "id") },
+ * inverseJoinColumns = { @JoinColumn(name = "dest_id", referencedColumnName = "id")}
+ * )
+ */
+ public $deps;
+
+ public function __construct()
+ {
+ $this->deps = new \Doctrine\Common\Collections\ArrayCollection();
+ }
+}
+
+/**
+ * @Entity
+ */
+class AUFL_OtherEntity
+{
+ /** @Id @GeneratedValue(strategy = "AUTO") @Column(type = "integer") */
+ public $id;
+
+ /** @Column(type = "string") */
+ public $state = 'new';
+}
Something went wrong with that request. Please try again.