DDC-1775: Notify strategy listener is not attached for new entities #2233

Closed
opened 2026-01-22 13:45:44 +01:00 by admin · 5 comments
Owner

Originally created by @doctrinebot on GitHub (Apr 11, 2012).

Originally assigned to: @beberlei on GitHub.

Jira issue originally created by user deatheriam:

New entities with Notify strategy for changes and with GeneratedValue(strategy="AUTO") never get the onPropertyChanged listener attached to them. That happens because of the logic in the UOW::scheduleForInsert($entity) method. The last condition in it "isset($this->entityIdentifiers[$oid])" is never true because the identifier is not set and therefore the code that attaches the PropertyChangedListener in addToIdentityMap is never run.

Originally created by @doctrinebot on GitHub (Apr 11, 2012). Originally assigned to: @beberlei on GitHub. Jira issue originally created by user deatheriam: New entities with Notify strategy for changes and with GeneratedValue(strategy="AUTO") never get the onPropertyChanged listener attached to them. That happens because of the logic in the UOW::scheduleForInsert($entity) method. The last condition in it "isset($this->entityIdentifiers[$oid])" is never true because the identifier is not set and therefore the code that attaches the PropertyChangedListener in addToIdentityMap is never run.
admin added the Bug label 2026-01-22 13:45:44 +01:00
admin closed this issue 2026-01-22 13:45:45 +01:00
Author
Owner

@doctrinebot commented on GitHub (May 23, 2012):

Comment created by @guilhermeblanco:

This is intentional to me.
According to code and documentation, PropertyChanged is supposed to operate over updates, never over inserts.

I experimented changing the code to also notify about changed during new and the consequences were very drastic. Internally, propertyChanged creates entityChangesets that implies an UPDATE.

Marking ticket as won't fix.

@doctrinebot commented on GitHub (May 23, 2012): Comment created by @guilhermeblanco: This is intentional to me. According to code and documentation, PropertyChanged is supposed to operate over updates, never over inserts. I experimented changing the code to also notify about changed during new and the consequences were very drastic. Internally, propertyChanged creates entityChangesets that implies an UPDATE. Marking ticket as won't fix.
Author
Owner

@doctrinebot commented on GitHub (May 24, 2012):

Comment created by deatheriam:

In that case the Notify strategy is partially broken:
Category entity:

..............

  *\Entity\Category
 *
 * @ORM\Table(name="category")
 * @ORM\Entity
 */
class Category implements TimestampableInterface

    /****
     * Caption
     *
     * @var string $caption
     * @ORM\Column(name="caption", type="text", nullable=true)
     */
    protected $caption;

    /****
     * Added At
     *
     * @var \DateTime $addedAt
     *
     * @ORM\Column(name="added_at", type="datetime", nullable=true)
     */
    protected $addedAt;

    public function setCaption($caption)
    {
        if ($this->caption != $caption)  {
            $this->_onPropertyChanged('caption', $this->caption, $caption);
            $this->caption = $caption;
        }
    }

    public function setAddedAt($addedAt)
    {
        $oldValue = $this->addedAt;
        $this->addedAt = $addedAt;
        if ((is_null($oldValue) || ($oldValue->getTimestamp() != $this->addedAt->getTimestamp()))) {
            $this->_onPropertyChanged('addedAt', $oldValue, $this->addedAt);
        }
    }

    protected function _onPropertyChanged($propName, $oldValue, $newValue)
    {
        if ($this->_listeners) {
            /*** @var $listener \Doctrine\Common\PropertyChangedListener **/
            foreach ($this->_listeners as $listener) {
                $listener->propertyChanged($this, $propName, $oldValue, $newValue);
            }
        }
    }

    public function addPropertyChangedListener(PropertyChangedListener $listener)
    {
        $this->_listeners[] = $listener;
    }

..............  

OnFlush event handler:

..............

        foreach ($uow->getScheduledEntityInsertions() as $entity) {
            if ($entity instanceof TimestampableInterface) {
                $entity->setUpdatedAt(new \DateTime('now'));
                $entity->setAddedAt(new \DateTime('now'));
                $uow->recomputeSingleEntityChangeSet($em->getClassMetadata(get_class($entity)), $entity);
            }
        }

...............     

Code that uses the entity:

        $cat = new \Entity\Category();
        // @todo this is a workaround until the http://www.doctrine-project.org/jira/browse/[DDC-1775](http://www.doctrine-project.org/jira/browse/DDC-1775) is resolved
        $cat->addPropertyChangedListener($this->_em->getUnitOfWork());
        $cat->setCaption('Please explain');
        $this->_em->persist($cat);
        $this->_em->flush();

If there is no explicit call to the addPropertyChangedListener method, the caption field gets saved but the $addedAt remains null after flush. The entity does not have the attached listener so UnitOfWork does not know anything about the update that happened in the OnFlush event, and the recomputeSingleEntityChangeSet method skips entities with Notify strategy.

@doctrinebot commented on GitHub (May 24, 2012): Comment created by deatheriam: In that case the Notify strategy is partially broken: Category entity: ``` .............. *\Entity\Category * * @ORM\Table(name="category") * @ORM\Entity */ class Category implements TimestampableInterface /**** * Caption * * @var string $caption * @ORM\Column(name="caption", type="text", nullable=true) */ protected $caption; /**** * Added At * * @var \DateTime $addedAt * * @ORM\Column(name="added_at", type="datetime", nullable=true) */ protected $addedAt; public function setCaption($caption) { if ($this->caption != $caption) { $this->_onPropertyChanged('caption', $this->caption, $caption); $this->caption = $caption; } } public function setAddedAt($addedAt) { $oldValue = $this->addedAt; $this->addedAt = $addedAt; if ((is_null($oldValue) || ($oldValue->getTimestamp() != $this->addedAt->getTimestamp()))) { $this->_onPropertyChanged('addedAt', $oldValue, $this->addedAt); } } protected function _onPropertyChanged($propName, $oldValue, $newValue) { if ($this->_listeners) { /*** @var $listener \Doctrine\Common\PropertyChangedListener **/ foreach ($this->_listeners as $listener) { $listener->propertyChanged($this, $propName, $oldValue, $newValue); } } } public function addPropertyChangedListener(PropertyChangedListener $listener) { $this->_listeners[] = $listener; } .............. ``` OnFlush event handler: ``` .............. foreach ($uow->getScheduledEntityInsertions() as $entity) { if ($entity instanceof TimestampableInterface) { $entity->setUpdatedAt(new \DateTime('now')); $entity->setAddedAt(new \DateTime('now')); $uow->recomputeSingleEntityChangeSet($em->getClassMetadata(get_class($entity)), $entity); } } ............... ``` Code that uses the entity: ``` $cat = new \Entity\Category(); // @todo this is a workaround until the http://www.doctrine-project.org/jira/browse/[DDC-1775](http://www.doctrine-project.org/jira/browse/DDC-1775) is resolved $cat->addPropertyChangedListener($this->_em->getUnitOfWork()); $cat->setCaption('Please explain'); $this->_em->persist($cat); $this->_em->flush(); ``` If there is no explicit call to the addPropertyChangedListener method, the caption field gets saved but the $addedAt remains null after flush. The entity does not have the attached listener so UnitOfWork does not know anything about the update that happened in the OnFlush event, and the recomputeSingleEntityChangeSet method skips entities with Notify strategy.
Author
Owner

@doctrinebot commented on GitHub (May 27, 2012):

Comment created by @beberlei:

Changing computeScheduleInsertsChangeSets() would solve this, but it would also mean that the notifier gets injected more than once.

    private function computeScheduleInsertsChangeSets()
    {
        foreach ($this->entityInsertions as $entity) {
            $class = $this->em->getClassMetadata(get_class($entity));

            $this->computeChangeSet($class, $entity);

            if ($entity instanceof NotifyPropertyChanged) {
                $entity->addPropertyChangedListener($this);
            }
        }
    }

I think injecting in scheduleForInsert() is ok, but we have to look at potential problems also

@doctrinebot commented on GitHub (May 27, 2012): Comment created by @beberlei: Changing computeScheduleInsertsChangeSets() would solve this, but it would also mean that the notifier gets injected more than once. ``` private function computeScheduleInsertsChangeSets() { foreach ($this->entityInsertions as $entity) { $class = $this->em->getClassMetadata(get_class($entity)); $this->computeChangeSet($class, $entity); if ($entity instanceof NotifyPropertyChanged) { $entity->addPropertyChangedListener($this); } } } ``` I think injecting in scheduleForInsert() is ok, but we have to look at potential problems also
Author
Owner

@doctrinebot commented on GitHub (Jul 7, 2012):

Comment created by @beberlei:

Fixed

@doctrinebot commented on GitHub (Jul 7, 2012): Comment created by @beberlei: Fixed
Author
Owner

@doctrinebot commented on GitHub (Jul 7, 2012):

Issue was closed with resolution "Fixed"

@doctrinebot commented on GitHub (Jul 7, 2012): Issue was closed with resolution "Fixed"
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: doctrine/archived-orm#2233