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

Persist Extension: PersistService.start() cannot be called multiple times #598

Closed
gissuebot opened this issue Jul 7, 2014 · 21 comments · Fixed by #1694
Closed

Persist Extension: PersistService.start() cannot be called multiple times #598

gissuebot opened this issue Jul 7, 2014 · 21 comments · Fixed by #1694

Comments

@gissuebot
Copy link

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

@gissuebot
Copy link
Author

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?

@gissuebot
Copy link
Author

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 {
  if (! persistService.isActive()) {         // <--
    persistService.start();
  }
}

@gissuebot
Copy link
Author

From sberlin on February 18, 2011 06:48:41

Issue 605 has been merged into this issue.

@gissuebot
Copy link
Author

From sberlin on February 21, 2011 17:45:51

(No comment was entered for this change.)

Labels: Extension-Persist

@gissuebot
Copy link
Author

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() {
    Preconditions.checkState(null == emFactory, "Persistence service was already initialized.");
     ...
  }

  public synchronized void stop() {
    Preconditions.checkState(emFactory.isOpen(), "Persistence service was already shut down.");
    emFactory.close();
  }

@gissuebot
Copy link
Author

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.

@gissuebot
Copy link
Author

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

@gissuebot
Copy link
Author

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

@gissuebot
Copy link
Author

From terciofilho on July 16, 2013 04:48:10

@xavier, the problem is not the exception itself, but the fact that we cannot re-start the PersistService. Sometimes I need to start, do same job, stop and start again after some modifications to the configuration in runtime.

Do you accept patches? Seriously, more than 2 years to set a variable to null.

@gissuebot
Copy link
Author

From sberlin on December 20, 2013 06:19:58

(No comment was entered for this change.)

Labels: -Extension-Persist Component-Persist

@gissuebot
Copy link
Author

From dhanji on May 20, 2014 20:59:32

I don't think we should allow restarting a PersistService. Other options are:

  • instead you can create a new Injector/PersistService combination
  • user a child-injector (or PrivateModule) to isolate different persist services

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.

@gissuebot
Copy link
Author

From terciofilho on May 27, 2014 07:18:04

I just don't get the point not to allow restarting.

@gissuebot
Copy link
Author

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.

@gissuebot
Copy link
Author

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

@gissuebot
Copy link
Author

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.

@lextiz
Copy link

lextiz commented Apr 28, 2015

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

@jhalterman
Copy link

@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.

@dhanji
Copy link
Contributor

dhanji commented Aug 19, 2015

@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.

@jhalterman
Copy link

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.

@fsparv
Copy link

fsparv commented Jul 10, 2016

FYI this seems to cause issues with guice-persist usage in some cases
http://stackoverflow.com/questions/17402081/how-to-start-jpa-in-a-guice-quartz-web-application

@fsparv
Copy link

fsparv commented Jul 10, 2016

also http://stackoverflow.com/questions/20337037/guice-creationexception-with-persistfilter... and I'm experiencing something similar right now...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants