Doctrine 2 - ORM
  1. Doctrine 2 - ORM
  2. DDC-2156

[regression] EntityManager::find() doesn't call custom repository anymore

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Invalid
    • Affects Version/s: 2.3
    • Fix Version/s: None
    • Component/s: ORM
    • Security Level: All
    • Labels:
      None

      Description

      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.

        Activity

        Stan Imbt created issue -
        Hide
        Benjamin Eberlei added a comment -

        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.

        Guilherme Blanco Whats your opinion on this BC break?

        Show
        Benjamin Eberlei added a comment - 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. Guilherme Blanco Whats your opinion on this BC break?
        Hide
        Stan Imbt added a comment -

        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()"...

        Show
        Stan Imbt added a comment - 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()"...
        Hide
        Marco Pivetta added a comment -

        Stan Imbt 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.

        Show
        Marco Pivetta added a comment - Stan Imbt 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.
        Hide
        Benjamin Eberlei added a comment -

        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
        Show
        Benjamin Eberlei added a comment - 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
        Benjamin Eberlei made changes -
        Field Original Value New Value
        Status Open [ 1 ] Resolved [ 5 ]
        Resolution Invalid [ 6 ]
        Hide
        Guilherme Blanco added a comment -

        Stan Imbt 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.

        Show
        Guilherme Blanco added a comment - Stan Imbt 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.

        This list may be incomplete, as errors occurred whilst retrieving source from linked applications:

        • Request to http://www.doctrine-project.org/fisheye/ failed: Error in remote call to 'FishEye 0 (http://www.doctrine-project.org/fisheye/)' (http://www.doctrine-project.org/fisheye) [AbstractRestCommand{path='/rest-service-fe/search-v1/crossRepositoryQuery', params={query=DDC-2156, expand=changesets[0:20].revisions[0:29],reviews}, methodType=GET}] : Received status code 503 (Service Temporarily Unavailable)

          People

          • Assignee:
            Benjamin Eberlei
            Reporter:
            Stan Imbt
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: