Skip to content

Loading…

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

Closed
doctrinebot opened this Issue · 6 comments

2 participants

@doctrinebot

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.

@doctrinebot

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

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

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

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

Issue was closed with resolution "Invalid"

@doctrinebot

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.

@beberlei beberlei was assigned by doctrinebot
@doctrinebot doctrinebot closed this
@doctrinebot doctrinebot added the Bug label
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.