xiaodududu / google-guice

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

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

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
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 reported on code.google.com by michal.g...@gmail.com on 2 Feb 2011 at 9:06

GoogleCodeExporter commented 9 years ago
We can add a simple isActive() method, would that solve the issue for you?

Original comment by dha...@google.com on 8 Feb 2011 at 4:13

GoogleCodeExporter commented 9 years ago
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();
  }
}

Original comment by michal.g...@gmail.com on 8 Feb 2011 at 7:50

GoogleCodeExporter commented 9 years ago
Issue 605 has been merged into this issue.

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

GoogleCodeExporter commented 9 years ago

Original comment by sberlin on 22 Feb 2011 at 1:45

GoogleCodeExporter commented 9 years ago
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();
  }

Original comment by psa...@gmail.com on 18 Apr 2011 at 9:00

GoogleCodeExporter commented 9 years ago
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.

Original comment by terciofi...@gmail.com on 21 Jan 2013 at 2:15

GoogleCodeExporter commented 9 years ago
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).

Original comment by guillaum...@gmail.com on 4 Jun 2013 at 3:26

GoogleCodeExporter commented 9 years ago
In the meantime, what I do is bind an interceptor around PersistService.class 
which swallows IllegalStateExceptions...

Original comment by xavier.d...@gmail.com on 16 Jul 2013 at 9:28

GoogleCodeExporter commented 9 years ago
@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.

Original comment by terciofi...@gmail.com on 16 Jul 2013 at 11:48

GoogleCodeExporter commented 9 years ago

Original comment by sberlin on 20 Dec 2013 at 2:19

GoogleCodeExporter commented 9 years ago
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.

Original comment by dha...@gmail.com on 21 May 2014 at 3:59

GoogleCodeExporter commented 9 years ago
I just don't get the point not to allow restarting.

Original comment by terciofi...@gmail.com on 27 May 2014 at 2:18

GoogleCodeExporter commented 9 years ago
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.

Original comment by guillaum...@gmail.com on 27 May 2014 at 7:00

GoogleCodeExporter commented 9 years ago
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...

Original comment by dha...@gmail.com on 27 May 2014 at 8:52

GoogleCodeExporter commented 9 years ago
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.

Original comment by terciofi...@gmail.com on 27 May 2014 at 9:14

GoogleCodeExporter commented 9 years ago
Hi all,
I have the same problem.

I must have some code executed and using persistence on application startup.
The first request to my system then tries to configure the PersistFilter, and 
this one starts the PersistService I already started on application start.

How can I achieve this? Or avoid a new PersistService start?

Can someone paste a piece of code, to do this?
> #8 xavier.d...@gmail.com
>
>In the meantime, what I do is bind an interceptor around PersistService.class 
which >swallows IllegalStateExceptions...

kind regards
Patrick

Original comment by ethan.hu...@web.de on 14 Sep 2014 at 4:08

GoogleCodeExporter commented 9 years ago
I solved the problem by creating my own class extended
JpaPersistService with the overloaded method:
@Override
public synchronized void start() {
    try {
        super.start();
    } catch (IllegalStateException e) {
        ; // no worries if it's already running.
    }
}

I then added bindings for my class to the injector:

bind(PersistService.class).to(MyJpaPersistService.class);
bind(JpaPersistService.class).to(MyJpaPersistService.class);
bind(MyJpaPersistService.class).in(Singleton.class);

It sounds like Xavier did the equivalent using Guice's AOP features:
https://github.com/google/guice/wiki/AOP

Our application encounters the problem in a similar way as #15
describes: We attach to the database with one configuration to do
things like database migrations, and then reconfigure for normal use
with more limited permissions.

Original comment by michael....@gmail.com on 15 Sep 2014 at 4:42

GoogleCodeExporter commented 9 years ago
Hi,
overloading might work if I trick package visibility of JpaPersistService by 
faking package structure com.google.inject.persist.jpa in my project to be able 
to extend JpaPersistService, but JpaPersistModule also binds JpaPersistService 
and guice complains about "A binding to 
com.google.inject.persist.PersistService was already configured at 
com.google.inject.persist.jpa.JpaPersistModule.configurePersistence(...)"

And overriding configureServlets() of JpaPersistModule() does not work, because 
this class is final.

Any other ideas?

regards
Patrick

Original comment by ethan.hu...@web.de on 16 Sep 2014 at 6:50

GoogleCodeExporter commented 9 years ago
I mean...
"And overriding configurePersistence() of JpaPersistModule()..."

Original comment by ethan.hu...@web.de on 16 Sep 2014 at 6:52

GoogleCodeExporter commented 9 years ago
install(Modules.override(new
JpaPersistModule("yourjpaunitstring")).with(new
YourCustomizedPersistModule()));

where YourCustomizedPersistModule does the bindings I showed previously:
@Override
protected void configure() {
    bindConstant().annotatedWith(Jpa.class).to("yourjpaunitstring");
    bind(Properties.class).annotatedWith(Jpa.class).toInstance(new
Properties());
    bind(PersistService.class).to(YourCustomizedJpaPersistService.class);
    bind(JpaPersistService.class).to(YourCustomizedJpaPersistService.class);
    bind(YourCustomizedJpaPersistService.class).in(Singleton.class);
}

Original comment by michael....@gmail.com on 16 Sep 2014 at 9:44

GoogleCodeExporter commented 9 years ago
Does the YourCustomizedPersistModule class just extends AbstractModule or also 
PersistModule like the default JpaPersistModule?

Original comment by ethan.hu...@web.de on 18 Sep 2014 at 5:39

GoogleCodeExporter commented 9 years ago
Just AbstractModule. In the end it's the JpaPersistModule that gets
installed; it just gets some bindings overridden by
YourCustomizedPersistModule.

Original comment by michael....@gmail.com on 18 Sep 2014 at 5:50