DDC-2156: [regression] EntityManager::find() doesn't call custom repository anymore #2714

Closed
opened 2026-01-22 14:01:18 +01:00 by admin · 6 comments
Owner

Originally created by @doctrinebot on GitHub (Nov 19, 2012).

Originally assigned to: @beberlei on GitHub.

Jira issue originally created by user literal:

Up to version 2.2 the entity manager would forward calls to find() to the respective repositories. In 2.3 the entity manager handles these calls directly.

This interferes with custom repositories that have an overridden find() method.

It's inconsistent, because $em->getRepository('Foo')->find($id) may do something different to $em ->find('Foo', $id) now.

Not sure if this is intentional. If it is, it's missing in the change log and the docs (7.8.1. "Essentially, EntityManager#find() is just a shortcut for the following...").

In any case it's a big BC issue for us:

  • We have (generated) repositories for all aggregate roots which override find() to throw (also generated) custom exceptions when an entity isn't found. We just don't want all those null checks in the client code, especially as we'd sooner or later forget one somewhere.
  • We are using the $em->find('Foo', $id) style everywhere because it's more concise - it doesn't seem to make much sense to explicitly fetch the repository when this could be done implicitly.
Originally created by @doctrinebot on GitHub (Nov 19, 2012). Originally assigned to: @beberlei on GitHub. Jira issue originally created by user literal: Up to version 2.2 the entity manager would forward calls to `find()` to the respective repositories. In 2.3 the entity manager handles these calls directly. This interferes with custom repositories that have an overridden `find()` method. It's inconsistent, because `$em->getRepository('Foo')->find($id)` may do something different to `$em ->find('Foo', $id)` now. Not sure if this is intentional. If it is, it's missing in the change log and the docs (7.8.1. "Essentially, EntityManager#find() is just a shortcut for the following..."). In any case it's a big BC issue for us: - We have (generated) repositories for all aggregate roots which override `find()` to throw (also generated) custom exceptions when an entity isn't found. We just don't want all those `null` checks in the client code, especially as we'd sooner or later forget one somewhere. - We are using the `$em->find('Foo', $id)` style everywhere because it's more concise - it doesn't seem to make much sense to explicitly fetch the repository when this could be done implicitly.
admin added the Bug label 2026-01-22 14:01:18 +01:00
admin closed this issue 2026-01-22 14:01:23 +01:00
Author
Owner

@doctrinebot commented on GitHub (Nov 25, 2012):

Comment created by @beberlei:

The problem is, that users changing EntityRepository#find() is actually why we moved the code to EntityManager#find(), because the behavior is required to work this way internally.

[~guilhermeblanco] Whats your opinion on this BC break?

@doctrinebot commented on GitHub (Nov 25, 2012): Comment created by @beberlei: The problem is, that users changing EntityRepository#find() is actually why we moved the code to EntityManager#find(), because the behavior is required to work this way internally. [~guilhermeblanco] Whats your opinion on this BC break?
Author
Owner

@doctrinebot commented on GitHub (Nov 26, 2012):

Comment created by literal:

I don't quite get it. You have moved the code to protect people from themselves? Like people overriding "find()" with a modified copy of the original "find()"-code which would fail with a new version of Doctrine? Did that really happen a lot?

I mean, what could be the problem be with an overridden "find()" doing "$entity = parent::find(...); if (!$entity) throw ...; return $entity;". And this seems like the typical example of why someone would want to override "find()"...

@doctrinebot commented on GitHub (Nov 26, 2012): Comment created by literal: I don't quite get it. You have moved the code to protect people from themselves? Like people overriding "find()" with a modified copy of the original "find()"-code which would fail with a new version of Doctrine? Did that really happen a lot? I mean, what could be the problem be with an overridden "find()" doing "$entity = parent::find(...); if (!$entity) throw ...; return $entity;". And this seems like the typical example of why someone would want to override "find()"...
Author
Owner

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

Comment created by @ocramius:

[~literal] why were you assuming that the find method on the ObjectManager is just a shortcut to the ObjectRepository's find? I think that assumption is wrong.

@doctrinebot commented on GitHub (Nov 27, 2012): Comment created by @ocramius: [~literal] why were you assuming that the `find` method on the ObjectManager is just a shortcut to the ObjectRepository's `find`? I think that assumption is wrong.
Author
Owner

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

Comment created by @beberlei:

I close this here for the following reasons:

  • EntityManager#find() behavior is required to be the one defined by Doctrine
  • Extension Point is EntityRepository#find().
  • EntityManager#find() is not a shortcut for EntityRepository#find().

You may work around this in your application by:

  • Decorating the EntityManager
  • Using the repository instead of the EntityManager
@doctrinebot commented on GitHub (Nov 27, 2012): Comment created by @beberlei: I close this here for the following reasons: - EntityManager#find() behavior is required to be the one defined by Doctrine - Extension Point is EntityRepository#find(). - EntityManager#find() is not a shortcut for EntityRepository#find(). You may work around this in your application by: - Decorating the EntityManager - Using the repository instead of the EntityManager
Author
Owner

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

Issue was closed with resolution "Invalid"

@doctrinebot commented on GitHub (Nov 27, 2012): Issue was closed with resolution "Invalid"
Author
Owner

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

Comment created by @guilhermeblanco:

[~literal] Our point of extension is EntityRepository, not EntityManager.
So, if you override the EntityRepository, you should also access using EntityRepository. We received many reports of people complaining that Locking was not working, while in fact they have overwritten find() and forgot to add the lock call.

To fix your problem, I'd suggest you to use composition around EntityManager and create a delegate to EntityRepository on your method find. That would fix your problem.

@doctrinebot commented on GitHub (Nov 27, 2012): Comment created by @guilhermeblanco: [~literal] Our point of extension is EntityRepository, not EntityManager. So, if you override the EntityRepository, you should also access using EntityRepository. We received many reports of people complaining that Locking was not working, while in fact they have overwritten find() and forgot to add the lock call. To fix your problem, I'd suggest you to use composition around EntityManager and create a delegate to EntityRepository on your method find. That would fix your problem.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: doctrine/archived-orm#2714