vipx / google-guice

Automatically exported from code.google.com/p/google-guice
Apache License 2.0
0 stars 0 forks source link

Persist: Automatically starting a UnitOfWork in JpaPersistService.get() #604

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
JpaPersistService.get() starts a UnitOfWork if one has not already been 
started. It then does a checkState to ensure that a UnitOfWork has been 
started, with a message indicating that the user should ensure that they start 
a UnitOfWork before trying to get() an EntityManager. Since a UnitOfWork is 
always started before this check, it'll never throw an exception.

However, I think that the correct behavior here would be to throw the exception 
rather than starting a UnitOfWork for the user. The problem is that an 
EntityManager will be set in the ThreadLocal, but it probably will not be 
removed because if the user didn't call UnitOfWork.begin() before getting the 
EntityManager, they probably won't call UnitOfWork.end() after either. This 
seems like a problem to me.

Is the point to allow users to choose to not manually start a UnitOfWork and 
have it only start when (and if) it's needed? And then to have them call end() 
at some point in case one was started? I can see the advantage in that, but I 
still somewhat worry that it makes it too easy to leave an unclosed 
EntityManager in the ThreadLocal. I also think in many cases, the EntityManager 
would likely be retrieved even if it weren't used, either due to a high level 
@Transactional method or due to a class that has the EntityManager injected 
into it being instantiated.

I think my preference would be to require that either a UnitOfWork be manually 
started before a class that has an EntityManager injected into it is 
instantiated (already the case for any webapp using PersistFilter) or that a 
Provider<EntityManager> be injected and called in each transactional method 
when a UnitOfWork is guaranteed to be active.

Original issue reported on code.google.com by cgdec...@gmail.com on 16 Feb 2011 at 4:37

GoogleCodeExporter commented 9 years ago

Original comment by sberlin on 18 Feb 2011 at 2:49