Orm: DDC-1680: Id is null in postRemove events

[doctrinebot]

Jira issue originally created by user redemption:

When using the postRemove event, $this->id (where id is the annotated @Id of the Entity) is null.

All other fields have the correct data (eg if you have an Entity with name, email, address, and id as fields, the id field is the only one that is null).

[doctrinebot]

Comment created by @beberlei:

This is expected behavior, otherwise when you merge that entity back it will be detected as "known", although its actually deleted. You can save the id in preRemove and then reuse it in postRemove if you need it.

[doctrinebot]

Comment created by redemption:

Is there any chance of providing a native premade value (or method) to store the deleted id, for instance "deletedId"? It seems logical to me that postRemove events will need to know the Id that was removed, and hacking it in with preRemove events all the time is less elegant.

[doctrinebot]

Comment created by umed:

yeah I also think that we need something native. This hack with preremove looks really ugly.

[doctrinebot]

Comment created by umed:

Here is https://groups.google.com/forum/#!topic/doctrine-user/bTukzq0QrSE a very good comment left by Pavel Horal

[doctrinebot]

Comment created by @ocramius:

[~umed] I'm fine with reverting this, but it needs a new and well exposed issue.

[doctrinebot]

Comment created by ahmadnazir:

There there another issue created for this problem? If not, then I would like to help.

What do you mean by a well exposed issue?

[doctrinebot]

Comment created by @ocramius:

[~ahmadnazir] I mean that we could need a functional test case

[doctrinebot]

Comment created by pawelbaranski:

Hi,

I'd also see this issue solved. I have a case where I'm storing an entity both in MySQL and part of it in Elasticsearch. When I delete an entity from MySQL I'd like it to be deleted from ES as well. I would not want to remove it from ES too soon so postRemove event would be what I need.

I will use the hack Benjamin presented, but it is like an unwanted child

[doctrinebot]

Issue was closed with resolution "Invalid"

[ebuildy]

Why this is closed?

[lcobucci]

@ebuildy apparently nobody bothered to create a "new and well exposed issue" (which means it has a clear explanation and a functional test) as @Ocramius asked. Would you be wiling to do that?

You can find examples of functional tests on https://github.com/doctrine/doctrine2/tree/388afb46d0cb3ed0c51332e8df0de9e942c2690b/tests/Doctrine/Tests/ORM/Functional/Ticket

[dkarlovi]

@lcobucci if I'm creating a test case, it's a PR against master or where?

[lcobucci]

@dkarlovi you can use 2.6 as base.

[dkarlovi]

Until I create the test, it get's reviewed, accepted/rejected/fixed/released, I need the described workaround.

Where exactly am I supposed to store the identifier in the preRemove handler? On the event? But it's not the same object in preRemove and postRemove. I imagine we can abuse PHP's synchronous nature and store as a property inside my handler and count on things not getting out of order, but that's just too messy for comfort, TBH.

IMO if this state is the solution for

when you merge that entity back it will be detected as "known", although its actually deleted

then this exact problem needed a better solution. For example, mark the entity as "deleted" or "unmergeable" or something.

[Ocramius]

Where exactly am I supposed to store the identifier in the preRemove handler?

I usually record everything I need to "keep" in onFlush, which right after the changes in the UnitOfWork were computed.

The id being null is currently by design, so we'd break BC for a scenario like this if we reverted it:

$entity = new Foo(); // has generated id
$em->persist($entity);
$em->flush();
$em->remove($entity);
$em->flush();
$em->persist($entity); // if we revert the change, this one would fail due to duplicate identifier
$em->flush();

[dkarlovi]

@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).

[Ocramius]

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

[dkarlovi]

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

[Ocramius]

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.

[dkarlovi]

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.

[Ocramius]

@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

[ismail1432]

PostRemoveEventArgs which would have getIdentifier()

One year later, I think it's a good idea :+1: