javaee / jersey

This is no longer the active Jersey repository. Please see the README.md
http://jersey.github.io
Other
2.86k stars 2.35k forks source link

Enable Jersey/Guice integration via HK2 Guice Bridge #2222

Open glassfishrobot opened 11 years ago

glassfishrobot commented 11 years ago

The HK2 Guice Bridge is available, but there may be more work needed in Jersey in order to enable it.

Affected Versions

[2.0]

glassfishrobot commented 11 years ago

Reported by @jwells131313

glassfishrobot commented 11 years ago

cowwoc said: This issue is related to https://java.net/jira/browse/HK2-39 and https://java.net/jira/browse/HK2-121

To reiterate, here is what we had in Jersey 1.0:

Container
\-> GuiceFilter (Guice: initialize request scope)
  \-> GuiceServletContextListener (Guice: initialize the Guice injector)
    \-> JerseyServletModule (Jersey: bind Jersey types to Guice module)
      \-> GuiceContainer (Jersey: redirect incoming requests to resource classes)

In other words, we created a Guice injector first and initialized Jersey second.

In Jersey 2.0 we have a circular reference. We need to initialize Injector before ServletContainer (otherwise @RequestScoped isn't initialized), but we need to initialize ServletContainer before Injector (otherwise we can't get a reference to ServiceLocator).

glassfishrobot commented 11 years ago

vokiel said: I've been reading the various issues on Guice with Jersey 2.0 & HK2 and I have to say I'm a bit confused as to what the bridge allows currently and what it doesn't allow. I'm trying to get Guice-Persist to work with Jersey, but yes it just won't inject what the module defines (factories and such).

If I just use the bridge as is and install a JpaPersistModule into the HK2 bridge, shouldn't it be able to inject an EntityManager from the Guice definition in my resources afterward? All that without having to use GuiceFilter & the Servlet plumbing of Guice? Isn't this just adaptation?

What sm I missing or what's the status?

glassfishrobot commented 11 years ago

cowwoc said: Jersey 2.0 doesn't support Guice, period.

Development is being held up on the Jersey end of things, not HK2.

Anyone wishing to help should take a look at how Spring integration was added in #2229 and try to replicate the technique for Guice.

glassfishrobot commented 11 years ago

cowwoc said: I finally figured out how to wedge Guice support into Jersey 2 (mostly through trial and error). Can someone from the Jersey team please review and publish the following documentation as part of the official user guide?


1. Add GuiceFilter and ServletContainer to web.xml:

<filter>
        <filter-name>GuiceFilter</filter-name>
        <filter-class>com.google.inject.servlet.GuiceFilter</filter-class>
    </filter>
    <filter-mapping>
        <filter-name>GuiceFilter</filter-name>
        <url-pattern>/*</url-pattern>
    </filter-mapping>

    <filter>
        <filter-name>JerseyApplication</filter-name>
        <filter-class>org.glassfish.jersey.servlet.ServletContainer</filter-class>
        <init-param>
            <param-name>javax.ws.rs.Application</param-name>
            <param-value>com.mycompany.MyApplication</param-value>
        </init-param>
    </filter>
    <filter-mapping>
        <filter-name>JerseyApplication</filter-name>
        <url-pattern>/*</url-pattern>
    </filter-mapping>

2. Note that GuiceFilter must show up before ServletContainer. 3. Define an application as follows:

public class MyApplication extends ResourceConfig {

    @Inject
    public MyApplication(ServiceLocator serviceLocator) {
        // Register root resources, then...

        // Instantiate Guice Bridge
        GuiceBridge.getGuiceBridge().initializeGuiceBridge(serviceLocator);

        GuiceIntoHK2Bridge guiceBridge = serviceLocator.getService(GuiceIntoHK2Bridge.class);
        Injector injector = Guice.createInjector(new MyModule());
        guiceBridge.bridgeGuiceInjector(injector);
    }
}

4. Define a Guice module as follows:

public class MyModule extends AbstractModule {

    @Override
    protected void configure() {
        // Configure Guice as you normally would, then... 
        install(new ServletModule());
    }
}

5. ServletModule defines the "request" scope. GuiceFilter creates/destroys the scope on each incoming request.


Let's begin by amending the documentation, and then we can follow up by adding a sample application and unit tests that mirror the Spring module.

glassfishrobot commented 11 years ago

@jwells131313 said: That's a gr8 solution. I'd still like to see a more integrated solution though...

glassfishrobot commented 11 years ago

reinert said: It is not possible to just substitute H2K to Guice?

I don't like the idea of two DI containers working together. I would rather prefer just using Guice for managing every DI issue in my application.

glassfishrobot commented 11 years ago

cowwoc said: @jwells, what did you have in mind? @reinert, I don't think Jersey gives us an option. I believe it forces the use of HK2.

glassfishrobot commented 11 years ago

cowwoc said: Bad news. The above solution won't work. It only initializes one direction of HK2's bi-directional bridge.

I am beyond frustrated with this, having spent days trying to get it to work.

@jwells, please contact me by email (cowwoc at bbs.darktech.org). I'd like to resolve this issue once and for all over chat. Playing tag over the issue tracker is not working out.

glassfishrobot commented 11 years ago

cowwoc said: I found a way to beat the circular dependency for Injector. You can create the main Injector before the Application. Once you're inside Application, you create a child injector and pass in Hk2Module(serviceLocator) as follows: injector.createChildInjector(new Hk2Module(serviceLocator))

Now both Guice and HK2 are happy. This moves us in the right direction but it's still not a working solution:

1. There is no clean way to pass the root Injector into the Application class due to http://stackoverflow.com/q/19596592/14731. Currently I'm passing the variable using a public singleton, which is ugly. 2. There are problems mixing Guice and HK2 types. See HK2-139.

I will continue working to resolve this.

glassfishrobot commented 11 years ago

jontro said: @cowwoc

Would you mind trying out the solution outlined in https://java.net/jira/browse/HK2-121 ?

glassfishrobot commented 11 years ago

cowwoc said: @jontro

I replied to your solution, in HK2-121. Let's keep the discussion there to reduce noise for everyone else watching this issue.

glassfishrobot commented 10 years ago

cowwoc said: Proposal:

  1. Isolate all HK2-specific bindings into reusable "factories".
  2. Extract all HK2-specific code into its own module. jersey-core would only use JSR 330 to @Inject but it wouldn't do the actual binding or construction of the ServiceLocator. Instead of Application creating ServiceLocator, ServiceLocator would create the Application (more on this below).
  3. The user add the HK2, Guice or Spring module to the classpath at built-time.
  4. At startup these implementation would bind against the factories from step 1, inject the Application class, answer all subsequent @Inject requests.

The beauty of this is that Jersey would depend on JSR 330 instead of HK2 directly, and wouldn't need to manage any DI frameworks (as it did in 1.0).

Jersey committers: what do you think of this proposal?

glassfishrobot commented 10 years ago

cowwoc said: Can someone please change the title of this issue from "Enable Jersey/Guice integration via HK2 Guice Bridge" to "Guice support"? I don't think we'll be able to use the HK2 bridge for that.

glassfishrobot commented 10 years ago

jontro said: I would just like to add that I support @cowwoc completely. After a couple of months now since switching to jersey 2.x from the 1.x version it really feels cumbersome developing with both hk2 and guice.

Is it possible to reach a consensus on how this should move forward?

glassfishrobot commented 10 years ago

bowenwr said: I'm also voicing my support for moving forward on this issue.

glassfishrobot commented 10 years ago

djxak said: I think Jersey need to be DI framework agnostic.

For example I have a maven module in my application with JPA persistence classes. Entities and DAOs. DAOs implemented with Guice. They receive request-scoped EntityManager by Guice DI, have methods with @Transactional annotation (by guice-persist) etc.

Now I want to use this DAOs in my new REST module. For this I need to inject them to appropriate Jersey Resources. I don't want to rewrite all my DAOs with HK2. They already used in another UI module of my application. And HK2 doesn't provide functionality similar to guice-persist (request scoped UnitOfWork, @Transactional interceptor).

glassfishrobot commented 10 years ago

jitterted said: I've voted for this, but I also wanted to comment that because of this issue alone, I may be forced to switch to another JAX-RS provider (namely RESTEasy, which works quite well with Guice, thank you very much). Obviously I'd rather not do that, but HK2 is really getting in my way.

glassfishrobot commented 10 years ago

aldenquimby said: @jitterted i was able to get things working by following this HK2 comment. It's not great, but should work until the Jersey guys finally get around to tackling this issue

glassfishrobot commented 10 years ago

sugis said: I too, after many happy years of Jersey use, had to switch to RESTeasy over this issue

glassfishrobot commented 10 years ago

natros said: I start to feel you pain over this issue. I have a large code base that dependes on jersey 1.x and can't simple switch to Resteasy.

glassfishrobot commented 9 years ago

jkidd said: @cowwoc +1 As long as the support for Guice in Jersey 2.x is so cumbersome, I myself will not be willing to switch to Jersey 2. Using/handling one DI framework is enough, using two DI frameworks is pointing towards spaghetti.

glassfishrobot commented 9 years ago

vshankri said: We wanted to use "asycn servlet" so want to upgrade to Jersey 2.x but this is such a huge draw back that GUICE is not supported. Currently we use GUICE and JERSEY 1.x; so it has been really tough so far. I hope Jersey starts supporting GUICE

glassfishrobot commented 9 years ago

gmussi said: Hello,

Why is Jersey so dependent of HK2? Shouldnt DI and Rest be two different things? Do I miss something here? I use jersey 1.x and Guice heavily accross my applications, and would like to switch to 2.x, but like this, it is impossible at the moment.

My vote is for an official and well-maintained Guice support, as in the previous version

glassfishrobot commented 8 years ago

superbull said: Hello, Is there any one in the jersey team care about this issue? It seems it is the most voted issue.

glassfishrobot commented 8 years ago

@mpotociar said: Guice support is not the priority for the core Jersey team at the moment.

I can see two options how to get this feature in: 1. Get Guice owners to contribute a support module 2. Community contribution. Jersey is an open-source project and since the issue has 52 votes, chances are that there is someone out there in the community who cares enough about this feature to implement it and contribute it back to the project as a Jersey extension pull request.

glassfishrobot commented 8 years ago

cowwoc said: Marek,

This issue cannot be solved without design-level changes in Jersey. Even if someone from the community were to contribute such a pull request, you wouldn't accept it (you implied as much in past mailing list discussion). There is no magical way to provide a Guice implementation that is completely abstracted away by HK2 without Jersey needing to be aware of it (the HK2 and Guice designs do not match and it causes the problems mentioned above). I already tried and gave up for this reason.

glassfishrobot commented 8 years ago

sugis said: As a long-time voter on this issue, we also tried to contribute a fix to this issue. We used Jersey 1 + Guice with great success, but wanted JAX-RS 2.0. We ran into the same issues cowwoc mentioned (complex implementation, unclear path to contributing such a complex change upstream) and eventually decided that migrating to RESTEasy was a simpler task than fixing Jersey, and that is what we ended up doing. We were sad to leave as Jersey had served us well, but were really forced to because of this issue.

glassfishrobot commented 8 years ago

aythus said: I remember that excitement. How much did we expect Jersey 2. We were so excited. but there was this nasty surprise when we tried to migrate over.

I can argue, that with HK2 it is not Jersey anymore. This is a different product, and it is altogether misleading to call it Jersey 2. It would better be called HKersey.

Marek, you need to admit to us and to yourself Jersey is dead, so that we can move one and you can continue developing HKersey.

It is easier to migrate from Jersey to RestEasy than to HKersey.

I am not being sarcastic, but your two points above are offensive to all the people on this thread.

Lot's of people tried to find a way to integrate Guice with HKersey, and all failed due to tight coupling of HK2. So rather than admitting, that the way HK2 was built into Jersey was shambles you are coming with inadequate excuses.

I don't even mention you suggesting Guice owners to contribute. Come on... Really, so it's Guice is guilty now. BTw, Is there any other DI framework HKersey can work with? Maybe Dagger or Spring?

glassfishrobot commented 8 years ago

@jwells131313 said: I guess I'm not fully understanding what exactly does not work. Many people have used the guice/jersey bridge to interoperate as much as guice will allow. One good example is here:

http://stackoverflow.com/questions/32449706/guice-jersey-integration-injects-null-objects/32458999#32458999

Perhaps this is just a documentation issue? If there are specific features that do not work they should probably be broken out into separate bugs.

glassfishrobot commented 8 years ago

sugis said: It's hard to pin down exactly which things are broken, because of the futility of the task.

Say you were trying to port a small application from a Jersey 1.x+Guice codebase to Jersey2.x+Guice – the very first thing you'll find, a number of previously working constructor injections fail: https://java.net/jira/browse/HK2-139

Fails because HK2 and Guice don't have exactly the same bindings. (In fact, Guice expects the set of bindings to be immutable, whereas HK2 changes it dynamically.)

Opened in 2013. Blocked on a Guice RFE, no real hope of getting the Guice team to change this, and even if they do it wouldn't be in a release until who-knows-when. Guice is infamously slow to evolve, or even release desperately-needed bugfix versions.

Even if it does get fixed – you're still in the bizarre position of running two competing DI frameworks. There's going to be more problems. It's bizarre to me that Jersey even depends on HK2 so strongly.

The whole point of JSR330 is to be DI-framework independent until the end consumer selects a product to use. But instead Jersey integrates HK2 to tightly that we suddenly can't use our existing Guice-based code – which we have a lot of time invested in and expertise with – we must instead start looking at replacing Guice, or fixing the subtly-broken bridge.

Another option, that I (and sounds like cowwoc) personally think is better, would be if Jersey was separated from HK2 somewhat and could run inside of Guice. The OSGI features and whatnot that require dynamism are great, but a lot of people (very intentionally) don't run e.g. OSGI and prefer immutable service configurations. But making that separation is a lot of work, and we have day-jobs...

RESTEasy offered us a path out, and we took it. I remember after spending a number of hours on this, I started to wonder how easy switching was. Turned out to be a relatively easy upgrade path. So we switched and stopped looking. Sounds like cowwoc did too (by moving to Pouch instead of Guice).

Just reading through the tickets there's a ton of specialized Guice and HK2 knowledge both required to tackle the problem, so the number of people who could possibly contribute it is very small. Any work I did at least is now somewhere on the dusty shelf.

Anyway sorry for the long-winded comment, was just hoping to better explain how I saw the situation.

glassfishrobot commented 8 years ago

@jwells131313 said: So JSR-330 did not achieve any sort of interoperability except for a very limited set of use cases. Very limited. I'm told this was because of bickering between the various DI players (Spring, Guice, JBoss, etc).

Because of the essential failure of JSR-330 to be comprehensive enough to support any complex use cases JSR-299 (CDI) was created. It is a lot better and a lot more complete but has other issues such as only truly working in a container environment (which is being fixed). A lot (but certainly not all) of hk2 is based on the CDI model.

So I would say that the whole point of JSR-330 was not really to allow for DI framework independence, or if it was, that it failed at that. The only purpose JSR-330 serves now is to provide a few (a very few!) common annotations and API that other specs can use (such as CDI).

glassfishrobot commented 8 years ago

cowwoc said: sugis summarized my position quite well.

I have a lot of respect for jwells and his work, but there is absolutely no technical way to get Jersey's current design to integrate perfectly against any other engine but HK2, and that is simply not okay.

There is no technical reason for Jersey to bind itself so tightly against HK2. The arrangement we had in Jersey1 worked perfectly well. We aren't asking Oracle to support multiple DI engines. We are asking for Jersey to operate against an interface and allow the community to contribute different implementations of that interface.

Final point: it is unreasonable for you to expect other DI engines (such as Guice) to add features whose main purpose is to help users to migrate away (use other engines). No one in their right mind would waste their time implementing this... Every DI engine I have ever encountered operates under two basic assumptions:

1. It is the only DI engine being used. 2. The DI engine injects values into the application (Jersey), not the other way around.

Jersey's design needs to internalize these points and provide a solution that works accordingly.

glassfishrobot commented 8 years ago

jhesse said: I've recently ported some non-trivial jersey 1.x + guice code to jersey 2, and found what seems like a workable solution. Apart from a couple exceptions, I don't have to think about HK2 at all and can just use all the existing guice based code.

I define a JerseyModule for my apps:

public class JerseyModule extends AbstractModule {
  private final Supplier<Injector> injectorSupplier;

  public JerseyModule(Supplier<Injector> injectorSupplier) {
    this.injectorSupplier = Preconditions.checkNotNull(injectorSupplier);
  }

  @Override
  protected void configure() {
    bind(InjectorContainer.class).toInstance(new InjectorContainer(injectorSupplier));
    bind(ServiceLocator.class).toProvider(ServiceLocatorContainer.class).in(Singleton.class);
  }

  @Provides
  public SecurityContext securityContext(ServiceLocator locator) {
    return locator.getService(SecurityContext.class);
  }

  @Provides
  public UriInfo uriInfo(ServiceLocator locator) {
    return locator.getService(UriInfo.class);
  }
}

This module defines the custom components and provides an easy way to add Jersey bindings to guice that are resolvable at injector creation time (by delegating to the service locator). The SecurityContext and UriInfo bindings are the only ones that I needed to port our existing code.

The ServiceLocatorContainer is simple:

@Singleton
public class ServiceLocatorContainer implements Provider<ServiceLocator> {

  private volatile ServiceLocator locator;

  @Override
  public ServiceLocator get() {
    if (locator == null) {
      throw new IllegalStateException("ServiceLocator not set");
    }
    return locator;
  }

  public void setServiceLocator(ServiceLocator locator) {
    this.locator = locator;
  }
}

As is the InjectorContainer:

public class InjectorContainer {

  private final Supplier<Injector> injectorSupplier;

  public InjectorContainer(Supplier<Injector> injectorSupplier) {
    this.injectorSupplier = injectorSupplier;
  }

  public Injector getInjector() {
    return Preconditions.checkNotNull(injectorSupplier.get(), "Null injector");
  }
}

Then, finally, there is a JerseyServletContainer that binds the guice bridge and pieces everything together:

@Singleton
@SuppressWarnings("serial")
public class JerseyServletContainer extends ServletContainer {

  private final InjectorContainer injectorContainer;
  private final ServiceLocatorContainer serviceLocatorContainer;

  @Inject
  public JerseyServletContainer(ResourceConfig resourceConfig, InjectorContainer injectorContainer,
      ServiceLocatorContainer serviceLocatorContainer) {
    super(resourceConfig);
    this.injectorContainer = checkNotNull(injectorContainer);
    this.serviceLocatorContainer = checkNotNull(serviceLocatorContainer);
  }

  @Override
  public void init() throws ServletException {
    super.init();
    setupGuiceBridge();
  }

  @Override
  public void reload(ResourceConfig configuration) {
    throw new IllegalStateException("Reload not supported!");
  }

  void setupGuiceBridge() {
    Injector injector = injectorContainer.getInjector();
    GuiceBridge gb = GuiceBridge.getGuiceBridge();
    ServiceLocator locator = getApplicationHandler().getServiceLocator();
    // set the service locator so it's available to the guice provider
    serviceLocatorContainer.setServiceLocator(locator);
    gb.initializeGuiceBridge(locator);

    GuiceIntoHK2Bridge guiceBridge = locator.getService(GuiceIntoHK2Bridge.class);
    guiceBridge.bridgeGuiceInjector(injector);
  }
}

Then you just have to pass an injector supplier to the JerseyModule, which is pretty straight forward, eg:

...
AtomicReference<Injector> inj = new AtomicReference();
List<Module> modules = new ArrayList<>();
modules.add(new JerseyModule(() -> inj.get()));
...
inj.set(Guice.createInjector(modules));
...

I chose not to support the reload() method in the JerseyServletContainer, but it seems possible to do so.

Everything should just work for an existing Jersey app that relies on guice if you follow a few rules:

1) Use javax.inject. (instead of com.google.) for all injection annotations in resource classes 2) For non-singleton resources, you have to add the @PerLookup annotation to the resource since HK2 uses singleton by default 3) Don't bind resource classes in guice modules, they'll be created by HK2 (however with the above code, you are expected to bind a ResourceConfig, which is where you register your resource classes for HK2)

That's pretty much it. If you need additional jersey bindings to be available to guice at module creation time, just create new ones that delegate to the ServiceLocator again.

glassfishrobot commented 8 years ago

cowwoc said: jhesse,

Check if constructor injection works, especially if the constructor parameters contain some types that are injected by HK2 and others injected by Guice.

glassfishrobot commented 8 years ago

jhesse said: cowwoc,

This code base uses only constructor injection. Pretty much all of the objects injected in resources are regular, pojos defined in guice modules. The HK2 -> guice delegation works fine. The guice -> HK2 (ServiceLocator) delegation seems to work fine too as a means to provide bindings to guice for Jersey based services.

glassfishrobot commented 8 years ago

cplummer said: @jhesse - how are you integrating this into the bootstrap? Are you using the GuiceFilter to drive it with your JerseyServletContainer in a ServletModule? Or is Jersey driving it from a ResourceConfig that you've set up? It'd be interesting to see (relevant portions of) your web.xml, ResourceConfig/Application class, and ResourceConfig binding if you wouldn't mind showing them.

glassfishrobot commented 8 years ago

jhesse said: @cplummer: yeah, I'm using GuiceFilter. Here's an example server module:

public class ExampleServerModule extends AbstractModule {

  private final Supplier<Injector> injectorSupplier;

  public ExampleServerModule(Supplier<Injector> injectorSupplier) {
    this.injectorSupplier = checkNotNull(injectorSupplier);
  }

  @Override
  protected void configure() {
    install(new JerseyModule(injectorSupplier));
    install(new ServletModule() {
      @Override
      protected void configureServlets() {
        serve("/*").with(JerseyServletContainer.class);
      }
    });
  }

  @Provides
  @Singleton
  public ObjectMapper objectMapper() {
    return ObjectMapperContextResolver.mapper;
  }

  @Provides
  public ResourceConfig resourceConfig() {
    ResourceConfig rc = new ResourceConfig();
    rc.registerClasses(MyResource.class,
        JacksonJsonProvider.class,
        ObjectMapperContextResolver.class);
    return rc;
  }

  @javax.ws.rs.ext.Provider
  public static class ObjectMapperContextResolver implements ContextResolver<ObjectMapper> {
    static final ObjectMapper mapper;
    static {
      mapper = new ObjectMapper();
      mapper.configure(SerializationFeature.INDENT_OUTPUT, true);
      mapper.setSerializationInclusion(Include.NON_NULL);
    }

    @Override
    public ObjectMapper getContext(Class<?> arg0) {
      return mapper;
    }
  }
}

Then I configure it in a Jetty server like:

ServletContextHandler app = new ServletContextHandler(options);
app.setContextPath(contextPath);
app.addFilter(GuiceFilter.class, "/*", EnumSet.of(DispatcherType.REQUEST,
    DispatcherType.FORWARD, DispatcherType.ASYNC, DispatcherType.INCLUDE,
    DispatcherType.ERROR));
app.addEventListener(new GuiceServletContextListener() {
    @Override
    protected Injector getInjector() {
        return webInjector;
    }
});

ServletHolder ds = app.addServlet(DefaultServlet.class, "/");
ds.setInitParameter("dirAllowed", "false");
,,,

The webInjector is passed to the GuiceServletContextListener in my case. But, you should be able to create it from within a guice context listener implementation, as well.

glassfishrobot commented 11 years ago

Issue-Links: blocks HK2-121 is related to JERSEY-2184 JERSEY-1957 is related to HK2-139

glassfishrobot commented 7 years ago

This issue was imported from java.net JIRA JERSEY-1950