Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix remove listener if property namer is used #1292

Merged
merged 1 commit into from
Jun 29, 2022

Conversation

1ed
Copy link
Contributor

@1ed 1ed commented Jun 24, 2022

When the PropertyDirectoryNamer is used with the id of the entity
the remove listener throws an exception because it runs after the removal
and the id is no longer available at that time. This PR fixes the issue.

@@ -54,5 +54,6 @@ public function postFlush(): void
$this->handler->remove($object, $field);
}
}
$this->entities = [];
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the purpose of this line?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It resets the internal state. It removes the references to the entities and frees memory, and if the listener runs multiple times the handler will not be called again for the same entities.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you provide an example of a case where the listener runs multiple times?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Of course, for example if the entity manager flushed multiple times, e.g. in a batch process or at context/transaction boundaries (and apart from that I think making services as stateless as possible is a best practice).

$photos = $entityManager->getRepository(FeaturedPhoto::class)->findAll();

$entityManager->remove(array_shift($photos));
$entityManager->flush();

$entityManager->remove(array_shift($photos));
$entityManager->flush();

image

@garak garak merged commit 6b8ac89 into dustin10:master Jun 29, 2022
@1ed 1ed deleted the fix-remove-listener branch June 29, 2022 17:19
@1ed
Copy link
Contributor Author

1ed commented Jun 29, 2022

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants