-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Persist Extension: PersistService.start() cannot be called multiple times #598
Comments
From dhanji@google.com on February 07, 2011 20:13:18 We can add a simple isActive() method, would that solve the issue for you? |
From michal.galet on February 07, 2011 23:50:14 Well that would be nice. However this will not solve the problem when the PersistService is already started in some ServletContextListener. What about adding this check to the PersistFilter: public void init(FilterConfig filterConfig) throws ServletException { |
From sberlin on February 18, 2011 06:48:41 Issue 605 has been merged into this issue. |
From sberlin on February 21, 2011 17:45:51 (No comment was entered for this change.) Labels: Extension-Persist |
From psalmi on April 18, 2011 02:00:59 There is another related problem: Sequence start() -> stop() -> start() throws the same exception even if the restart should be possible. The reason is that start checks for the emFactory to be null but it is never reset on stop(). public synchronized void start() { public synchronized void stop() { |
From terciofilho on January 21, 2013 06:15:46 Same here. I need to restart and when I call the second start it just throw an exception. emFactory should be set null when stop. |
From guillaume.polet on June 04, 2013 08:26:24 Any chance this issues gets solved at some point? This would really be helpful in the context of tests where (in-memory) databases needs to be created and then dropped (which can easily be achieved by closing the EMF). |
From xavier.dury on July 16, 2013 02:28:26 In the meantime, what I do is bind an interceptor around PersistService.class which swallows IllegalStateExceptions... |
From terciofilho on July 16, 2013 04:48:10
Do you accept patches? Seriously, more than 2 years to set a variable to null. |
From sberlin on December 20, 2013 06:19:58 (No comment was entered for this change.) Labels: -Extension-Persist Component-Persist |
From dhanji on May 20, 2014 20:59:32 I don't think we should allow restarting a PersistService. Other options are:
We can add the isActive() method. However I am loathe to assume arbitrary ordering of service starts. If you install the PersistFilter it should give you a strong guarantee of having initialized the service when the filter is ready to receive requests. |
From terciofilho on May 27, 2014 07:18:04 I just don't get the point not to allow restarting. |
From guillaume.polet on May 27, 2014 12:00:24 I don't get it either. In the context of unit tests, it is very useful to be able to create an EMF pointing to an in-memory database which gets created upon the service start and deleted when the service stops. Allowing to restart, actually allows to execute several unit-tests. |
From dhanji on May 27, 2014 13:52:25 It's pretty simple to just create a new injector for a new unit test isn't it? I don't think it's the right pattern to share the Injector + PersistService across unit tests... |
From terciofilho on May 27, 2014 14:14:58 My situation is kind of different. I start the persistence service, get some info from the database, re-configure the application and start the service again. I cannot create a new injector, all the dependencies in my application have already been binded. |
I want to connect to the database straight after I have configured the JpaPersistModule instance, but this is not possible until servlet container initializes the PersistFilter, this means there is no option to load some data from DB and use it in @Provider methods (for startup initialization). I think that there should be at least an option to configure the filter to initialize itself before servlet initialization stage of servlet container. Or option to start the JpaPersistModule explicitly, so that PersistFilter would not fail on init. Loading data from DB on application startup is a basic scenario which is currently not working out of the box with current version of PersistFilter (without applying ugly workarounds). |
@dhanji It is simple to create a new injector, but when unit testing and the injector takes a few seconds to create, the cost adds up quick. |
@jhalterman I agree, but what is the purpose of restarting the PersistService? If it's just clearing out the session data, you can do that by shutting down the sessions and clearing L2 caches. Or clearing out the DB tables underneath. |
I'm not working on this now, but some tests I was looking at generally spent a lot of time setting up a PersistService/EntityManager - a few seconds for each test case, with a whole lot of test cases. I'm not sure how much of that could actually be saved by reusing an EntityManager/PersistService without having state from one test case interfere with another, but that's basically what I was aiming to do, which led to wanting PersistService to be more reusable. I'm guessing there's another way to approach that problem, but somewhere I need some more reuse. |
FYI this seems to cause issues with guice-persist usage in some cases |
also http://stackoverflow.com/questions/20337037/guice-creationexception-with-persistfilter... and I'm experiencing something similar right now... |
From michal.galet on February 02, 2011 04:06:48
The PersistService.start() method claims that it is possible to call this method multiple times. The implementation however throws "java.lang.IllegalStateException: Persistence service was already initialized." on second call.
This is a problem since it is not possible to determine if the service was already started. Moreover the PersistFilter starts the PersistService in initialization. Therefore it is not possible to use the persistence in any ServletContextListener!
Original issue: http://code.google.com/p/google-guice/issues/detail?id=598
The text was updated successfully, but these errors were encountered: