"preRemove" given wrong entity #6288

Open
opened 2026-01-22 15:30:10 +01:00 by admin · 3 comments
Owner

Originally created by @justin-nl on GitHub (Aug 19, 2019).

Originally assigned to: @justin-nl on GitHub.

Bug Report

Q A
BC Break no?
Version 2.4.8 (and 2.5)

Summary

I'm working in Symfony 2.8 (and 3.4), with an Invoice entity that has a OneToOne with a File entity (for a PDF upload). I'm cascading "all" from Invoice to File.

The File entity is implementing the PostRemove callback for deleting the physical file after the DB is updated. Sometimes, this is called on the wrong entity, resulting in the wrong file being deleted.

Current behavior

The PostRemove callback is called on the wrong File entity. This results in the wrong physical file being removed. The correct parent Invoice and child File entities are removed from the DB.

It seems related to recent/subsequent entity creation. I would create a few invoices. Delete one. Load another, and find that the file could not be found.

Oddly enough, once I implemented the "PreRemove" (simply for debugging which entity I was working with), the "PostRemove" suddenly worked as expected. Basically, an empty "PreRemove" function "magically" made things work.

My ultimate work-around was to store the filename in a class variable in the "PreRemove" callback, and use that variable to determine which file is deleted in the "PostRemove" callback.

How to reproduce

In the Invoice entity:

/**
 * @ORM\OneToOne(targetEntity="File", cascade={"all"})
 * @ORM\JoinColumn(name="pdf_id", referencedColumnName="id")
 */
protected $pdf;

In the File entity:

/**
 * @ORM\PostRemove()
 */
public function postRemove()
{
    // "getAbsolutePath()" uses the "path" variable from the file object
    // and prepends the local system path
    $file = $this->getAbsolutePath();
    if (null !== $file) {
        // i did a `var_dump($file); exit;` to show the wrong filepath

        // "fs()" returns a new `Symfony\Component\Filesystem\Filesystem`
        $this->fs()->remove($file);
    }
}

Simple deletion using the Entity Manager:

// i verified here that the associations were correct
$em->remove($invoice);
// i verified here that the associations were still correct
$em->flush();
// i never got here because of the `exit;` I had implemented above

Expected behavior

I expect if I'm cascading from a parent object, the resulting PostRemove should be called on the correct child object.

Originally created by @justin-nl on GitHub (Aug 19, 2019). Originally assigned to: @justin-nl on GitHub. ### Bug Report | Q | A |------------ | ------ | BC Break | no? | Version | 2.4.8 (and 2.5) #### Summary I'm working in Symfony 2.8 (and 3.4), with an Invoice entity that has a OneToOne with a File entity (for a PDF upload). I'm cascading "all" from Invoice to File. The File entity is implementing the PostRemove callback for deleting the physical file after the DB is updated. Sometimes, this is called on the wrong entity, resulting in the wrong file being deleted. #### Current behavior The PostRemove callback is called on the wrong File entity. This results in the wrong physical file being removed. The correct parent Invoice and child File entities are removed from the DB. It seems related to recent/subsequent entity creation. I would create a few invoices. Delete one. Load another, and find that the file could not be found. Oddly enough, once I implemented the "PreRemove" (simply for debugging which entity I was working with), the "PostRemove" suddenly worked as expected. Basically, an empty "PreRemove" function "magically" made things work. My ultimate work-around was to store the filename in a class variable in the "PreRemove" callback, and use that variable to determine which file is deleted in the "PostRemove" callback. #### How to reproduce In the Invoice entity: ``` /** * @ORM\OneToOne(targetEntity="File", cascade={"all"}) * @ORM\JoinColumn(name="pdf_id", referencedColumnName="id") */ protected $pdf; ``` In the File entity: ``` /** * @ORM\PostRemove() */ public function postRemove() { // "getAbsolutePath()" uses the "path" variable from the file object // and prepends the local system path $file = $this->getAbsolutePath(); if (null !== $file) { // i did a `var_dump($file); exit;` to show the wrong filepath // "fs()" returns a new `Symfony\Component\Filesystem\Filesystem` $this->fs()->remove($file); } } ``` Simple deletion using the Entity Manager: ``` // i verified here that the associations were correct $em->remove($invoice); // i verified here that the associations were still correct $em->flush(); // i never got here because of the `exit;` I had implemented above ``` #### Expected behavior I expect if I'm cascading from a parent object, the resulting PostRemove should be called on the correct child object.
admin added the BugMissing Tests labels 2026-01-22 15:30:10 +01:00
Author
Owner

@lcobucci commented on GitHub (Aug 20, 2019):

@justin-nl unfortunately, v2.4 and v2.5 aren't maintained anymore (only security fixes are backported). Can you check if that still happens in v2.6.3?

@lcobucci commented on GitHub (Aug 20, 2019): @justin-nl unfortunately, v2.4 and v2.5 aren't maintained anymore (only security fixes are backported). Can you check if that still happens in v2.6.3?
Author
Owner

@RobinHoutevelts commented on GitHub (Mar 23, 2021):

@lcobucci We seem to be hitting this as well in latest v2.8.2 with a cascading delete.

The title of this issue is wrong though, it happens in the "PostRemove" callback.


The File entity is a Proxy whose values aren’t hydrated ( uninitialised ). After persisting the deletion, when the entity passes Doctrine/ORM/UnitOfWork.php#L1248 it loses its identifier.

3358ccde39/lib/Doctrine/ORM/UnitOfWork.php (L1244-L1249)

A few lines further the PostRemove event is invoked.
Anyone that listens for that event and tries to get a value from the File proxy will trigger a hydration. However because the identifier is gone, the loadById call thats being made in the ProxyFactory is passing an empty array as the $criteria.

// The entity's identifier has been unset in UnitOfWork::executeDeletions() after the deletion has been persisted
$identifier = $classMetadata->getIdentifierValues($proxy);

// $identifier now contains an empty array

3358ccde39/lib/Doctrine/ORM/Proxy/ProxyFactory.php (L147-L152)

This causes a SELECT * FROM file query to be run in

3358ccde39/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php (L725-L735)

Which loads all(!) File entities. The first one is being returned..

Which causes the $proxy to be hydrated with a random entity.


Just like @justin-nl noticed we can fix this by making sure the entity is hydrated in a “PreRemove” event.

@RobinHoutevelts commented on GitHub (Mar 23, 2021): @lcobucci We seem to be hitting this as well in latest v2.8.2 with a cascading delete. The title of this issue is wrong though, it happens in the "PostRemove" callback. --- The File entity is a Proxy whose values aren’t hydrated ( uninitialised ). After persisting the deletion, when the entity passes [Doctrine/ORM/UnitOfWork.php#L1248](https://github.com/doctrine/orm/blob/3358ccde399e6c75e738c0da5cd0e06ad6cc2395/lib/Doctrine/ORM/UnitOfWork.php#L1248) it loses its identifier. https://github.com/doctrine/orm/blob/3358ccde399e6c75e738c0da5cd0e06ad6cc2395/lib/Doctrine/ORM/UnitOfWork.php#L1244-L1249 A few lines further the PostRemove event is invoked. Anyone that listens for that event and tries to get a value from the File proxy will trigger a hydration. However because the identifier is gone, the [loadById call thats being made in the ProxyFactory](https://github.com/doctrine/orm/blob/3358ccde399e6c75e738c0da5cd0e06ad6cc2395/lib/Doctrine/ORM/Proxy/ProxyFactory.php#L149) is passing an empty array as the $criteria. ```php // The entity's identifier has been unset in UnitOfWork::executeDeletions() after the deletion has been persisted $identifier = $classMetadata->getIdentifierValues($proxy); // $identifier now contains an empty array ``` https://github.com/doctrine/orm/blob/3358ccde399e6c75e738c0da5cd0e06ad6cc2395/lib/Doctrine/ORM/Proxy/ProxyFactory.php#L147-L152 This causes a `SELECT * FROM file` query to be run in https://github.com/doctrine/orm/blob/3358ccde399e6c75e738c0da5cd0e06ad6cc2395/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php#L725-L735 Which loads **all**(!) File entities. The first one is being returned.. Which causes the `$proxy` to be hydrated with a random entity. --- Just like @justin-nl noticed we can fix this by making sure the entity is hydrated in a “PreRemove” event.
Author
Owner

@akasake commented on GitHub (Mar 23, 2021):

I can confirm that @RobinHoutevelts solution works.
By adding a PreRemoveSubscriber that hydrates the entity, I can access the correct values of the removed entity in the postRemove event.

Important to note is that you can still not access the previous id in postRemove (cause the entity does not exist anymore).

<?php

namespace App\EventListener;

use Doctrine\Common\EventSubscriber;
use Doctrine\ORM\Events;
use Doctrine\Persistence\Event\LifecycleEventArgs;
use Doctrine\Common\Proxy\Proxy;

class PreRemoveHydratorSubscriber implements EventSubscriber
{
    public function getSubscribedEvents(): array
    {
        return [
            Events::preRemove,
        ];
    }

    public function preRemove(LifecycleEventArgs $args): void
    {
        // TODO remove when issue https://github.com/doctrine/orm/issues/7806 is fixed
        $entity = $args->getObject();
        if (!$entity instanceof Proxy) {
            // is not recommended to do this
            // this would mean always initializing every entity that you are about to remove
            // could also make a HasPostRemoveInterface and decide this way which entity you want to do this for
            return;
        }

        // hydrate values of entity proxy,
        // so we can access the values in postRemove
        // is no-op if entity is already initialized
        $entity->__load();
    }
}
@akasake commented on GitHub (Mar 23, 2021): I can confirm that @RobinHoutevelts solution works. By adding a `PreRemoveSubscriber` that hydrates the entity, I can access the correct values of the removed entity in the postRemove event. Important to note is that you can still not access the previous `id` in postRemove (cause the entity does not exist anymore). ```php <?php namespace App\EventListener; use Doctrine\Common\EventSubscriber; use Doctrine\ORM\Events; use Doctrine\Persistence\Event\LifecycleEventArgs; use Doctrine\Common\Proxy\Proxy; class PreRemoveHydratorSubscriber implements EventSubscriber { public function getSubscribedEvents(): array { return [ Events::preRemove, ]; } public function preRemove(LifecycleEventArgs $args): void { // TODO remove when issue https://github.com/doctrine/orm/issues/7806 is fixed $entity = $args->getObject(); if (!$entity instanceof Proxy) { // is not recommended to do this // this would mean always initializing every entity that you are about to remove // could also make a HasPostRemoveInterface and decide this way which entity you want to do this for return; } // hydrate values of entity proxy, // so we can access the values in postRemove // is no-op if entity is already initialized $entity->__load(); } } ```
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: doctrine/archived-orm#6288