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

[Question] Is there a reason why the bundle doesn't register repositories as services? #660

Closed
dkarlovi opened this Issue May 16, 2017 · 19 comments

Comments

Projects
None yet
@dkarlovi

dkarlovi commented May 16, 2017

I've looked through the issues and documentation, couldn't find any reference to it, but seeing people are looking for it or doing it in a hackish kind of way: is there a reason why the bundle doesn't register repositories as services?

I see it's quite common and handy to auto-register configured / local resources (for example, GaufretteBundle or Symfony Workflow component).

@stof

This comment has been minimized.

Member

stof commented May 16, 2017

Defining repositories as services is a very bad idea due to the way Doctrine ORM works.
When resetting the entity manager, all repositories must be recreated (to have repositories linked to the new entity manager instance). If you define repositories as services, the service will keep referencing the old instance, leading to very hard to debug bugs in your project.

So I definitely won't implement this in DoctrineBundle itself

@dkarlovi

This comment has been minimized.

dkarlovi commented May 16, 2017

@stof great, that's the info I was looking for.

So the preferred way to get a repository into your service is by injecting the entity manager and asking for it?

@vatson

This comment has been minimized.

vatson commented May 16, 2017

@stof maybe it's better not to reset entity manager?

@jvasseur

This comment has been minimized.

jvasseur commented May 16, 2017

@vatson if you get an database error during a flush your entity manager is in an unusable state and doctrine prevents you from using it, the only solution in this case is to reset it.

@vatson

This comment has been minimized.

vatson commented May 17, 2017

@jvasseur if it's normal to continue working after you get an error. Generally this is an unpleasant situation that entails a lot of tricky solutions. All this leads to more confusion. It is necessary to choose the lesser of two evils

@xabbuh

This comment has been minimized.

Member

xabbuh commented May 17, 2017

@vatson But it's probably a good idea to let the developer make this decision based on their project and not force them into one direction.

@xabbuh xabbuh referenced this issue Jun 2, 2017

Closed

Custom repository service compiler pass #658

1 of 1 task complete
@craigh

This comment has been minimized.

@alcaeus

This comment has been minimized.

Member

alcaeus commented Jul 10, 2017

@craigh That post is 3 years old at this time. Here, have a newer one: https://blog.fervo.se/blog/2017/07/06/doctrine-repositories-autowiring/

@Hanmac

This comment has been minimized.

Hanmac commented Jul 10, 2017

about that fervo thing, couldn't that UserRepository from the blog implement "ObjectRepository, Selectable" and you wouldn't notice a difference?

@alcaeus

This comment has been minimized.

Member

alcaeus commented Jul 10, 2017

Maybe @magnusnordlander can answer your question, but in theory I'd say you could do that.

@stof

This comment has been minimized.

Member

stof commented Jul 10, 2017

@vatson a very good use case to reset the entity manager to keep working is when writing a queue consumer. If there is an issue processing of one the message, you want to be able to recover the entity manager to be able to process next ones. Having to kill your consumer process on each failure is not good.
Removing the need to re-instantiate the entity manager on failure requires changing the architecture of the ORM, and this will not happen anytime soon (rewriting the ORM would be a huge work, and the ORM maintainers don't have resources to do that currently)

@craigh

This comment has been minimized.

craigh commented Jul 10, 2017

@alcaeus - 3 years isn't that long. And "old" doesn't mean wrong or bad. Doctrine 2 hasn't changed too dramatically since that time. I've found @matthiasnoback 's work to be really good.

@magnusnordlander

This comment has been minimized.

Contributor

magnusnordlander commented Jul 10, 2017

@alcaeus: He certainly can :)

@Hanmac: You could, yes. That would probably make for a smoother transition if you're already using custom repositories. What you shouldn't do is extend EntityRepository, and what you can't do is have them mapped as the repositoryClass in the entity mapping.

This also means that if your code is doing something like $container->get('doctrine')->getRepositoryForClass('App:SomeEntity') you're going to have to change that.

@craigh: @matthiasnoback is great, and yes, the article is still factually correct, in fact he mentions this very issue under Something to be aware of: resetting closed entity managers. We just disagree in whether this issue means that you should always inject the ManagerRegistry rather than the EntityManager (I think so, he seems to not think so).

@stof

This comment has been minimized.

Member

stof commented Jul 10, 2017

@magnusnordlander as of Symfony 3.3, you can reset the entity manager while injecting it directly, as it know uses a lazy service and resets the internal object of the entity manager proxy.
However, you still have the same limitation than before for any other object instantiated by the EM (repositories, the UnitOfWork, and other internal Doctrine objects). If you store them in a property of your object graph instead of getting them from the EntityManager each time, you will keep using the old object.

@dkarlovi

This comment has been minimized.

dkarlovi commented Jul 10, 2017

@stof so to repeat the question:

the preferred way to get a repository into your service is by injecting the entity manager and asking for it?

@stof

This comment has been minimized.

Member

stof commented Jul 10, 2017

if you actually need the doctrine repository (the one returned by $em->getRepository()), yes. This is even the only way which is officially supported (defining repositories as services is doable on your side, but you are on your own for issues you face due to this)

@matthiasnoback

This comment has been minimized.

matthiasnoback commented Jul 10, 2017

@mablae

This comment has been minimized.

mablae commented Oct 22, 2017

I just wanted to share a recent blogpost on that topic that follows @matthiasnoback 's approach:

https://www.tomasvotruba.cz/blog/2017/10/16/how-to-use-repository-with-doctrine-as-service-in-symfony/

@weaverryan

This comment has been minimized.

Contributor

weaverryan commented Jan 27, 2018

This can be closed. As of DoctrineBundle 1.8, you can use a new base class + Symfony's auto-registration to wire your repositories automatically as services.

@alcaeus alcaeus closed this Jan 29, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment