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

Getting an Entity from an injected Repository in a Service returns a detached Entity from versions >= 1.6.4 #841

Closed
sebblanc opened this issue Aug 22, 2018 · 8 comments
Assignees
Labels
⭐️ EUFOSSA Hackathon https://symfony.com/blog/the-symfony-and-api-platform-hackathon-is-coming

Comments

@sebblanc
Copy link

Getting an Entity from an injected Repository in a Service returns a detached Entity for versions of DoctrineBundle >= 1.6.4, due to the following merge : Set doctrine.orm.entity_manager.abstract as lazy #559

This bug occurs with the following conditions:

  • Version of doctrine/doctrine-bundle >= 1.6.4

  • the Entity has an attached EntityListener

  • The Service is injected into the EntityListener

I'm not sure if it should be considered as a bug or not, maybe this is a wanted behavior but I don't think so.

I made a small project in order to test it easily : unexpected-doctrine-entity-state-in-service

I've tested with Symfony 3.4, not with other versions yet.

Note : Before finding the origin of the bug, I posted on StackOverflow, in case of you are interested : https://stackoverflow.com/questions/51426933/symfony-3-4-unexpected-doctrine-entity-state-in-service

@iwan-wijaya
Copy link

I'm having the exact same problem (Doctrine Bundle 1.9.1, ORM 2.6), This bug also occurs if an entity doesn't have an attached entity listener and has its repository injected into an entity listener either directly or as a sub dependency..

@supersmile2009
Copy link

You're dealing with circular dependency issue here.

  1. While EntityManager is being initialized it instantiates listeners
  2. Listeners, which require a repository as a service, try to instantiate that repository.
  3. Repository needs an EntityManager, so it instantiates one (a different one from the one being instantiated currently because it's not ready yet and not registered in the container).
  4. You end up with 2 managers: the main one which started it's instantiation process in step 1, and another one from step 3 which is referenced from your repository.

Solution - declare your listeners and repositories as lazy.
Since Symfony 4.2 entity listeners are always lazy, so those using 4.2 shouldn't face this issue.

@alcaeus
Copy link
Member

alcaeus commented Apr 6, 2019

@supersmile2009 is correct: entity listeners should be declared as lazy services. With 1.11, we'll always lazy load listeners, so you don't have to update your configuration anymore (see #899).

@alcaeus alcaeus closed this as completed Apr 6, 2019
@sebblanc
Copy link
Author

sebblanc commented Apr 6, 2019

@supersmile2009 @alcaeus I just tried to turn both the listener and the repository as lazy in the debug project I made for this issue : unexpected-doctrine-entity-state-in-service and it still not work. Did I missed something ?

@alcaeus
Copy link
Member

alcaeus commented Apr 7, 2019

That's not the reply I was hoping for. I'll take a look at the project and try to reproduce it in a test case. Thanks for following up!

@alcaeus alcaeus reopened this Apr 7, 2019
@alcaeus alcaeus self-assigned this Apr 7, 2019
@alcaeus alcaeus added the Bug label Apr 7, 2019
@sebblanc
Copy link
Author

sebblanc commented Apr 7, 2019

Thank you too ! Just a little note about the project : in the services.yml, SuperListener & SuperService are loaded by the AppBundle\ default section. You have to alias them or comment this section/exclude them to be sure they are loaded via their own declaration (present below) and then be able to lazy load. (I had let this file as is to be able to test many configurations)

@alcaeus
Copy link
Member

alcaeus commented Apr 7, 2019

So, after taking another look I figured out what's going on. First of all, please don't take the first point personally: you are using a 2.5 year old version of the bundle; at the time you created the issue it was already 2 years old. The first thing you should always do is try to reproduce it in a current version, as that makes my job significantly easier.

Either way, I took the time to debug this to figure out what's going on. The first load cycle is triggered by the ParamConverter: since the controller is being called, the DoctrineParamConverter is asked if it supports the SuperService argument. This requires instantiating the entity manager. This causes instantiation of the EntityListenerResolver, which instantiates the SuperService class which again requires an entity manager because of the factory involved. This essentially creates an entity manager for the repository, then dumps it once it goes back to creating the original manager instance and overwrites it in the container.

So now, your repository has a different entity manager instance than the one stored in the container, which is why your entities are going to be detached after fetching them using the repository. There are a few ways around this:

  • Fetching the repository from the entity manager will yield correct results, as you now have a repository that is tied to the entity manager in the container. In general, using repository services before 1.8.0 (where we introduced support or repositories as services) was always prone to such issues.
  • In general, avoid injecting services that depend on the entity manager stack into event subscribers and listeners, including entity listeners. Instead, use the appropriate <event>Args instance in the method to fetch the entity manager and any related services from there. This helps avoid these circular references.
  • Marking the entity listener service as lazy in the tag would work; however, this was only fixed in 1.6.5. Note that this requires the service to be public. This was fixed in 1.6.11 where we always mark listeners as public.

TL;DR: please update to a supported version of DoctrineBundle (or at least one with support for lazy entity listeners) and make the listener lazy.

@alcaeus alcaeus closed this as completed Apr 7, 2019
@alcaeus alcaeus added ⭐️ EUFOSSA Hackathon https://symfony.com/blog/the-symfony-and-api-platform-hackathon-is-coming and removed Bug labels Apr 7, 2019
@sebblanc
Copy link
Author

sebblanc commented Apr 8, 2019

@alcaeus Ok I understand. I effectively used an old version of the Bundle (1.6.4) in this debug project, because I've chosen the first version in which I detected that it wasn't working as expected. I had no idea about the fact that tagging the entity listener as lazy would work only from 1.6.5, sorry for that. Thanks again for time and your detailed explanations !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⭐️ EUFOSSA Hackathon https://symfony.com/blog/the-symfony-and-api-platform-hackathon-is-coming
Projects
None yet
Development

No branches or pull requests

4 participants