DDC-1680: Id is null in postRemove events #2110

@dkarlovi commented on GitHub (Jan 26, 2018):

@Ocramius thanks, makes sense.

The id being null is currently by design, so we'd break BC for a scenario like this if we reverted it:
if we revert the change, this one would fail due to duplicate identifier

I know, that was my point with

mark the entity as "deleted" or "unmergeable" or something.

so, when you try to merge it back, it just throws. This allows for correct behavior in both cases (entity which is deleted is a orphaned entity you still have a reference to, you still don't get to reuse it just like that).

@dkarlovi commented on GitHub (Jan 26, 2018): @Ocramius thanks, makes sense. > The id being null is currently by design, so we'd break BC for a scenario like this if we reverted it: > if we revert the change, this one would fail due to duplicate identifier I know, that was my point with > mark the entity as "deleted" or "unmergeable" or something. so, when you try to merge it back, it just throws. This allows for correct behavior in both cases (entity which is deleted is a orphaned entity you still have a reference to, you still don't get to reuse it just like that).

admin commented

@Ocramius commented on GitHub (Jan 26, 2018):

->merge() will no longer be improved, and is deprecated (and removed in 3.x

@Ocramius commented on GitHub (Jan 26, 2018): `->merge()` will no longer be improved, and is deprecated (and removed in `3.x`

admin commented

@dkarlovi commented on GitHub (Jan 26, 2018):

Sorry, "merge" is a lapsus, I meant "when you try to treat it as a regular entity in any way" (persist, update, etc).

@dkarlovi commented on GitHub (Jan 26, 2018): Sorry, "merge" is a lapsus, I meant "when you try to treat it as a regular entity in any way" (persist, update, etc).

admin commented

2026-01-22 13:41:22 +01:00

@Ocramius commented on GitHub (Jan 26, 2018):

Ah, sorry. No, there is no way to do that. After flush(), memory usage must stay constant in the ORM, and adding a registry that marks removed entities would be a big issue.

@Ocramius commented on GitHub (Jan 26, 2018): Ah, sorry. No, there is no way to do that. After `flush()`, memory usage must stay constant in the ORM, and adding a registry that marks removed entities would be a big issue.

admin commented

@dkarlovi commented on GitHub (Jan 26, 2018):

Fair point.

Would a better workaround be to introduce a new event type, PostRemoveEventArgs which would have getIdentifier()? Then Doctrine can do this for you and I wouldn't be too bad.

@dkarlovi commented on GitHub (Jan 26, 2018): Fair point. Would a better workaround be to introduce a new event type, `PostRemoveEventArgs` which would have `getIdentifier()`? Then Doctrine can do this for you and I wouldn't be too bad.

admin commented

@Ocramius commented on GitHub (Jan 26, 2018):

@dkarlovi we can improve this in 3.x by adding the identifier to PostRemoveEventArgs or by breaking BC as I described in https://github.com/doctrine/doctrine2/issues/2326#issuecomment-360719911

@Ocramius commented on GitHub (Jan 26, 2018): @dkarlovi we can improve this in 3.x by adding the identifier to `PostRemoveEventArgs` or by breaking BC as I described in https://github.com/doctrine/doctrine2/issues/2326#issuecomment-360719911

admin commented

@ismail1432 commented on GitHub (Jan 23, 2019):

PostRemoveEventArgs which would have getIdentifier()

One year later, I think it's a good idea 👍

@ismail1432 commented on GitHub (Jan 23, 2019): > PostRemoveEventArgs which would have getIdentifier() One year later, I think it's a good idea :+1:

admin commented

@Nathanael-Shermett commented on GitHub (Jan 5, 2021):

PostRemoveEventArgs which would have getIdentifier()

One year later, I think it's a good idea 👍

Another two years later, and I also think it's a good idea.

It doesn't make sense to modify an entity class (or perhaps every entity class), complete with getters and setters, just to store a historical ID. It's hacky, and it subverts the entity's integrity. With that said, I have a hacky solution that is at least self-contained:

public function preRemove(LifecycleEventArgs $args): void {
    $entity = $args->getObject();
    $entity->historicalId = $entity->getId(); // $historicalId is not defined or referenced anywhere but here
}

public function postRemove(LifecycleEventArgs $args): void {
    $entity = $args->getObject(); // the old ID is accessible via $entity->historicalId
}

I'm sure some purists on here will say this is gross. I agree. However, this limitation shouldn't exist in the first place, and a man's gotta eat somehow.

@Nathanael-Shermett commented on GitHub (Jan 5, 2021): > > PostRemoveEventArgs which would have getIdentifier() > > One year later, I think it's a good idea 👍 Another two years later, and I also think it's a good idea. It doesn't make sense to modify an entity class (or perhaps every entity class), complete with getters and setters, just to store a historical ID. It's hacky, and it subverts the entity's integrity. With that said, I have a hacky solution that is at least self-contained: public function preRemove(LifecycleEventArgs $args): void { $entity = $args->getObject(); $entity->historicalId = $entity->getId(); // $historicalId is not defined or referenced anywhere but here } public function postRemove(LifecycleEventArgs $args): void { $entity = $args->getObject(); // the old ID is accessible via $entity->historicalId } I'm sure some purists on here will say this is gross. I agree. However, this limitation shouldn't exist in the first place, and a man's gotta eat somehow.

admin commented

@Warxcell commented on GitHub (May 10, 2021):

I suggest just to move
https://github.com/doctrine/orm/blob/2.8.x/lib/Doctrine/ORM/UnitOfWork.php#L1244-L1246
after
https://github.com/doctrine/orm/blob/2.8.x/lib/Doctrine/ORM/UnitOfWork.php#L1248-L1250.

That will bring the ID in postRemove events. I think it's not convenient to have getIdentifier in event, for examples on places like that: https://github.com/Warxcell/files/blob/master/src/NamingStrategy/IdToPathStrategy.php

+1 for BC-break, It would be nice to have the ID even after postRemove. The use case is, when/if https://github.com/doctrine/dbal/pull/4622 is accepted, I will be able to refactor my listener on postRemove it will collect entities for removal, and on onTransactionCommit I can loop over pending entities for removal and call $fileManager->remove($entity) which could use ID to determine the path to the file. That will be impossible if IDs are gone after postRemove.

@Warxcell commented on GitHub (May 10, 2021): I suggest just to move https://github.com/doctrine/orm/blob/2.8.x/lib/Doctrine/ORM/UnitOfWork.php#L1244-L1246 after https://github.com/doctrine/orm/blob/2.8.x/lib/Doctrine/ORM/UnitOfWork.php#L1248-L1250. That will bring the ID in `postRemove` events. I think it's not convenient to have `getIdentifier` in event, for examples on places like that: https://github.com/Warxcell/files/blob/master/src/NamingStrategy/IdToPathStrategy.php +1 for BC-break, It would be nice to have the ID even after `postRemove`. The use case is, when/if https://github.com/doctrine/dbal/pull/4622 is accepted, I will be able to refactor my listener on `postRemove` it will collect entities for removal, and on `onTransactionCommit` I can loop over pending entities for removal and call `$fileManager->remove($entity)` which could use `ID` to determine the path to the file. That will be impossible if IDs are gone after postRemove.

admin commented

@Nathanael-Shermett commented on GitHub (Nov 15, 2021):

FYI, my above fix (using a dynamic property to store the entity ID before it is lost during the delete process) will be deprecated in PHP 8.2 and will throw an error in PHP 9.0.

You can read more here.

@Nathanael-Shermett commented on GitHub (Nov 15, 2021): FYI, my above fix (using a dynamic property to store the entity ID before it is lost during the delete process) will be deprecated in PHP 8.2 and will throw an error in PHP 9.0. You can read more [here](https://wiki.php.net/rfc/deprecate_dynamic_properties).

admin commented

@justin-oh commented on GitHub (May 16, 2024):

For anyone searching in the future, I had to do something like the following to maintain an ID-based value (which was a directory path).

Before:

public function getDirectory(): string
{
    return __DIR__.'/../../entity-uploads/'.$this->id;
}

After:

private string $directory;

public function getDirectory(): string
{
    return $this->directory ?? __DIR__.'/../../entity-uploads/'.$this->id;
}

/**
 * For preRemove lifecycle callback because $this->id is NULL in postRemove.
 */
public function setDirectory(): static
{
    $this->directory = $this->getDirectory();

    return $this;
}

@justin-oh commented on GitHub (May 16, 2024): For anyone searching in the future, I had to do something like the following to maintain an ID-based value (which was a directory path). Before: ```php public function getDirectory(): string { return __DIR__.'/../../entity-uploads/'.$this->id; } ``` After: ```php private string $directory; public function getDirectory(): string { return $this->directory ?? __DIR__.'/../../entity-uploads/'.$this->id; } /** * For preRemove lifecycle callback because $this->id is NULL in postRemove. */ public function setDirectory(): static { $this->directory = $this->getDirectory(); return $this; } ```

admin commented

@orangevinz commented on GitHub (May 28, 2024):

Using a trait can be convenient as well

trait StoreIdAfterDeletionEntity
{
    private ?int $storedId;

    public function getStoredId(): ?int
    {
        return $this->storedId;
    }

    #[ORM\PreRemove]
    public function setStoredId(): static
    {
        $this->storedId = $this->getId();

        return $this;
    }
}

#[ORM\Entity]
#[ORM\HasLifecycleCallbacks]
class MyEntity
{
    use StoreIdAfterDeletionEntity;
}

@orangevinz commented on GitHub (May 28, 2024): Using a trait can be convenient as well ```php trait StoreIdAfterDeletionEntity { private ?int $storedId; public function getStoredId(): ?int { return $this->storedId; } #[ORM\PreRemove] public function setStoredId(): static { $this->storedId = $this->getId(); return $this; } } ``` ```php #[ORM\Entity] #[ORM\HasLifecycleCallbacks] class MyEntity { use StoreIdAfterDeletionEntity; } ```

admin commented