eclipse-ee4j / glassfish-hk2

Dynamic dependency injection framework
https://eclipse-ee4j.github.io/glassfish-hk2
Other
84 stars 56 forks source link

Guice-servlet integration #166

Open glassfishrobot opened 11 years ago

glassfishrobot commented 11 years ago

org.jvnet.hk2.guice.bridge.test.bidirectional.BiDirectionalBridgeTest tests basic Guice injection but it fails to take Guice-Servlet into consideration.

Please add a unit test for a bi-directional HK2 <--> Guice Servlet bridge.

Affected Versions

[2.2.0]

glassfishrobot commented 6 years ago
glassfishrobot commented 11 years ago

@glassfishrobot Commented Reported by cowwoc

glassfishrobot commented 11 years ago

@glassfishrobot Commented cowwoc said: Justification: Guice can't be used Jersey 2.0 without a guice-servlet bridge. Basic Guice support is insufficient because it prevents the use of @RequestScoped.

glassfishrobot commented 11 years ago

@glassfishrobot Commented piersyp said: Hi, I'm looking for some clarification.

I would like to use Guice with Jersey 2.0 I would like to use Guice exclusively and I would like to use the automatic package based registration of resources similar to that shown by the following snippet.

Map<String, String> params = new HashMap<String, String>(); params.put(PackagesResourceConfig.PROPERTY_PACKAGES, "unbound"); serve("/*").with(GuiceContainer.class, params);

Can this be done currently?

I have looked at https://github.com/aluedeke/jersey2-guice-example linked from https://java.net/jira/browse/HK2-39 but I cannot get it to work it seems as if the resources are never registered. Is there some documentation that covers the registration of resources am I missing something?

Thanks

Piers

glassfishrobot commented 11 years ago

@glassfishrobot Commented cowwoc said: piersyp,

I don't think it is possible to use Guice with Jersey 2.0 until https://java.net/jira/browse/JERSEY-1950 is fixed.

glassfishrobot commented 11 years ago

@glassfishrobot Commented piersyp said: Thanks for getting back to me cowwoc I've actually just constructed an example project where I am using guice with jersey 2.0 see here -

https://github.com/piersy/jersey2-guice-example-with-test

but I am not using @RequestScope yet. Could you elaborate on why request scope will not work at present, I'm a bit hazy on how all this webapp/DI/bootstrap plumbing is achieved? Is there a good resource or book you know of which will explain to me the initialization sequence of a webapp?

Thanks

Piers

glassfishrobot commented 11 years ago

@glassfishrobot Commented cowwoc said: piersyp,

You must use GuiceFilter. See https://code.google.com/p/google-guice/wiki/Servlets

glassfishrobot commented 11 years ago

@glassfishrobot Commented piersyp said: Hi cowwoc

Thanks, I am in fact already using GuiceFilter, but I am confused. Have you changed your mind then, regarding your earlier comment?

"I don't think it is possible to use Guice with Jersey 2.0 until https://java.net/jira/browse/JERSEY-1950 is fixed."

I also have one more, hopefully last question.

I am receiving the following error

Jul 25, 2013 6:20:41 PM com.google.inject.servlet.InternalServletModule$BackwardsCompatibleServletContextProvider get
WARNING: You are attempting to use a deprecated API (specifically, attempting to @Inject ServletContext inside an eagerly created singleton.
While we allow this for backwards compatibility, be warned that this MAY have unexpected behavior if you have more than
one injector (with ServletModule) running in the same JVM. . .

I see that this is because I have not used GuiceServletContextListener is there some way that I can avoid this, maybe by getting a hold of the ServletContext inside the javax.ws.rs.Application class and doing the setup that would have been done by GuiceServletContextListener there?

Thanks

glassfishrobot commented 11 years ago

@glassfishrobot Commented cowwoc said: I don't think you can get a hold of ServletContext from inside Application, which is why you need GuiceServletContextListener in the first place.

But then you run into the circular dependency issue mentioned in https://java.net/jira/browse/JERSEY-1950

glassfishrobot commented 11 years ago

@glassfishrobot Commented piersyp said: Thanks for the clarification cowwoc. As you stated

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

Could you point out then what is wrong with the following approach, is there some feature of guice or jersey that would not funcntion correctly?

1. Configure GuiceFilter via the web.xml 2. Initialize the injector via GuiceServletContextListener 3. Initialize the guice HK2 bridge using the already created injector instance in the class specified by javax.ws.rs.Application parameter.

You can see below I store the injector instance as a static member of Main and then access it in Mypplication

public class Main extends GuiceServletContextListener {

    public static Injector injector;

    @Override
    protected Injector getInjector() {
        injector = Guice.createInjector(new ServletModule() {
            @Override
            protected void configureServlets() {
bind(Service.class);
            }
        });

        return injector;
    }
}

public class MyApplication extends ResourceConfig {

    @Inject
    public MyApplication(ServiceLocator serviceLocator) {
        // Set package to look for resources in
        packages("example.jersey");
        GuiceBridge.getGuiceBridge().initializeGuiceBridge(serviceLocator);
        GuiceIntoHK2Bridge guiceBridge = serviceLocator.getService(GuiceIntoHK2Bridge.class);
        guiceBridge.bridgeGuiceInjector(Main.injector);
    }
}
glassfishrobot commented 11 years ago

@glassfishrobot Commented cowwoc said: piersyp,

The easiest way to know whether your approach works or not is by building an application and checking if @RequestScoped works properly.

I believe the above will not work because your ServletModule configuration must:

filter("/*").through(ServletContainer.class);

where ServletContainer is Jersey's class for handling incoming requests. If your configuration is missing this, incoming HTTP requests will get picked up by Guice but never end up reaching Jersey.

glassfishrobot commented 10 years ago

@glassfishrobot Commented cowwoc said: John,

I think we need to revisit this issue.

Because we cannot construct a ServiceLocator before javax.ws.rs.core.Application we cannot inject ServletContextListener. Because we cannot inject ServletContextListener, we lose out on the ability to shut down scheduled tasks (that use injected values).

We need the ability to construct a Guice Injector earlier than Application's constructor. What do you think?

glassfishrobot commented 10 years ago

@glassfishrobot Commented jontro said: I managed to do guice servlets integration after a small hack. Basically what I did in my servlet module is this:

protected void configureServlets() {
    ResourceConfig jerseyConfig = new ResourceConfig();
    jerseyConfig.packages("mypackages");
    bind(ResourceConfig.class).toInstance(jerseyConfig);
    serve("/_dsf/services/*").with(MyJerseyServletContainer.class);
}
**MyJerseyServletContainer.java**@Singleton
public class MyJerseyServletContainer extends ServletContainer {

    private Injector injector;

    @Inject
    MyJerseyServletContainer(Injector injector, ResourceConfig configuration) {
        super(configuration);
        this.injector = injector;
    }

    @Override
    protected void init(WebConfig webConfig) throws ServletException {

        super.init(webConfig);

        ServiceLocator locator;
        try {
            Field webComponentField = getClass().getSuperclass()
    .getDeclaredField("webComponent");

            webComponentField.setAccessible(true);
            WebComponent webComponent = (WebComponent) webComponentField.get(this);

            Field appHandlerField = webComponent.getClass().getDeclaredField("appHandler");
            appHandlerField.setAccessible(true);
            ApplicationHandler appHandler  = (ApplicationHandler) appHandlerField.get(webComponent);
            locator = appHandler.getServiceLocator();

        } catch (NoSuchFieldException | SecurityException | IllegalArgumentException | IllegalAccessException e) {
            throw new RuntimeException(e);
        }

        GuiceBridge.getGuiceBridge().initializeGuiceBridge(
locator);

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

    }
}

As you can see it's a hackish way to retreive the locator. If we wrote a clone of ServletContainer setting up the locator ourselves we would not need to do this.

Alternativley we could expose the locator through a protected method in ServletContainer

glassfishrobot commented 10 years ago

@glassfishrobot Commented jontro said: To overcome the problem of multiple ServletContainers that is present in jersey 1 you could use the following approach with the "MyJerseyServletContainer" mentioned above.

protected void configureServlets() {
    ResourceConfig jerseyConfig = new ResourceConfig();
    jerseyConfig.packages("my.package.api.v1");
    serve("/api/v1/*").with(getJerseyServlet(jerseyConfig));

    jerseyConfig = new ResourceConfig();
    jerseyConfig.packages("my.package.api.v2");
    serve("/api/v2/*").with(getJerseyServlet(jerseyConfig));
}

private Key<MyJerseyServletContainer> getJerseyServlet(
        final ResourceConfig jerseyConfig) {
    final Key<MyJerseyServletContainer> key = Key.get(
            MyJerseyServletContainer.class, UniqueAnnotations.create());
    install(new PrivateModule() {

        @Override
        protected void configure() {
            bind(ResourceConfig.class).toInstance(jerseyConfig);
            bind(key).to(MyJerseyServletContainer.class).in(
    Singleton.class);
            expose(key);
        }
    });
    return key;
}
glassfishrobot commented 10 years ago

@glassfishrobot Commented cowwoc said: jontro,

This approach:

  1. Is hackish (as you mentioned).
  2. Only initializes one end of the two-way bridge. You need to pass new HK2IntoGuiceBridge(serviceLocator) into the Injector during initialization.
  3. Fails to solve some of the other shortcomings of the HK2 bridge, such as the inability to use constructor injection: #183
glassfishrobot commented 10 years ago

@glassfishrobot Commented jontro said: Thanks for your feedback!

1. This one is possible to solve if this is the right approach.

2. Care to explain what you mean in more detail? We have both the guice injector and the service locator here. No need to keep the guice injector in a static variable as in the code earlier in this thread.

3. I did not try to resolve any short comings of the bridge itself

My focus at this was mainly to get jersey 2.4 up and running with guice servlets, which this thread helped with. What's nice with this approach is that programatic configuration is easy, no need to go through web.xml.

glassfishrobot commented 10 years ago

@glassfishrobot Commented cowwoc said:

  1. It's easier to build the bridge inside the Application subclass. You can inject ServiceLocator into the subclass constructor. No hacks needed.
  2. If you read https://hk2.java.net/guice-bridge/#Bi-Directional_HK2_Guice_Bridge you will notice it requires you to use a HK2-specific module into Guice.createInjector(). Your solution is missing this step.
  3. Okay, but if that's all you want we're already there: https://java.net/jira/browse/JERSEY-1950?focusedCommentId=369064&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#action_369064 ... The real elephant in the room is fixing all these edge-cases. I don't consider the bridge to be reasonably usable until these are fixed.

Out of curiosity, who is invoking configureServlets() if web.xml is removed?

glassfishrobot commented 10 years ago

@glassfishrobot Commented jontro said: 1 & 2: Very good points, if bi-directional support is needed maybe the solution I listed is impossible (unless createChildInjector works). I will still explain why I think initializing jersey from a ServletModule is important: 3. Well the big difference between the two approaches is that in your approach the guice injector created in the Application subclass and in mine Jersey is created within a guice servlet module.

We are creating the guice injector in a servlet context listener (Subclass of GuiceServletContextListener) with a lot of different modules. The listener is either listed in web.xml or annotated with @WebListener

Jersey is then created in a ServletModule along with a bunch of regular j2ee servlets and filters and bound to a path using guice servlets api.

Maybe it would work just as good with creating it from the Application, but if Jersey is just a small part of your application you would need to rewrite your whole application initialization to make it work like this.

Creating it from the servletmodule makes it more similar to how you added Jersey in the 1.x branch which felt natural, as is done with other guice services.

https://code.google.com/p/google-guice/wiki/ServletModule

glassfishrobot commented 10 years ago

@glassfishrobot Commented cowwoc said: 1. createChildInjector() works. My latest solution uses this (though I don't think I've posted this anywhere). 3. I won't comment much on this except to say that if you want to avoid web.xml I recommend using Jetty instead of guice-servlet (it is better supported).

glassfishrobot commented 10 years ago

@glassfishrobot Commented jontro said: 1. Will try this out tomorrow. 3. How would one create multiple jersey containers sharing the same guice injector using the first approach?

glassfishrobot commented 10 years ago

@glassfishrobot Commented cowwoc said: I'm not sure what "the first approach" refers to but you'd create one top-level Injector and then multiple child injectors per Jersey container.

Why do you need multiple containers?

glassfishrobot commented 10 years ago

@glassfishrobot Commented realrunner said: @cowwoc

Could you post your latest solution? What pieces aren't working? I've tried all permutations of examples in this thread and JERSEY-1950 but I cannot get Guice services to inject in my Jersey resources. Were you able to get constructor injection working?

glassfishrobot commented 10 years ago

@glassfishrobot Commented cowwoc said: No. Constructor injection does not work, which is why I'm stuck

Off the top of my head, what I did was construct a main Injector before Jersey (say in a ServletContextListener) and fed it all my application-specific Modules. Then, inside the Application subclass, I created a child injector using the HK2-Guice bridge. I fed the child injector a Module that had manually-defined providers for UriInfo and other built-in types so that when the type was requested, I'd redirect the injection to ServiceLocator (HK2).

HK2's Guice bridge tries to handle all possible injections dynamically. I found it much easier to simply define whatever bindings I needed by hand. This way I didn't need to use @HK2Inject (normal @Inject would work).

I remember that constructor injection still failed, but I don't remember why. I plan to dedicate more time to this later on next week. I was just totally drained working on this last week so I decided to take a break.

glassfishrobot commented 10 years ago

@glassfishrobot Commented aythus said: I am not sure how the team here handles issues. But this issue has been marked Major (in fact it is critical) and 6 months later is still not resolved. I can see there were many attempts by community members to assist (thanks @cowwoc), it would be even better if core team members would join in as well. Jersey 2 is pretty much useless without this for major projects using Guice.

Can somebody from the team at least let us know if it is anywhere in the pipeline?

Thanks

glassfishrobot commented 10 years ago

@glassfishrobot Commented dreamershl said: Today I try out another way to integrate Guice with Jersey. It may solve the issue. I can get the session/request/Singleton object. But I haven't fully tested them yet. Just list down the approach, may help the discussion Jersey: 2.4.1 , Guice-Bridge: 2.2.0-b26, Guice-Servlet: 4.0 beta

The key is to use "ApplicationEventListener" in order to wait for Jersey fully initialised. After that, install the "GuiceBridge". Following is the code. The "NullableGuiceScopeContext" is to avoid the exception. If Guice injector return NULL, Guice-Bridge default implementation will report error.

1. How to get the ServiceLocator Through the HK2 injection directly.

2. How to get the Guice injector instance. Guice servlet package will use the GuiceServletContextListener to get the instance. So need install that object and return the application's injector instance. After that, in Jersey module, need configure HK2 how to inject the Injector.class instance. This can be just like other bind() code, (see below, how ResourceConfig configure "GuiceProvider"). Guice servlet will store the injector instance into the ServletContext attribute. So just directly retrieve the instance and return it.

Then how to get the ServletContext? Through HK2 injection. Because GuiceProvider.class is just a normal HK2 provider, it follows HK2 way. The tricky part is we MUST ENSURE when GuiceProvider.class is called, Jersey HK2 environment is already up. This is guaranteed by the ApplicationEvent.INITIALIZATION_FINISHED.

3. How to use Guice in HK2. GuiceBridge need 2 parameters, Guice injector + HK2 ServiceLocator, in order to configure JIT resolver. After application is fully initalized by Jersey, these 2 can be easily get through injection. This can be achieved through the ApplicationEvent event listener as showed below.

@WebListener("Init Guice for Servlets")
public class GuiceContextListener extends GuiceServletContextListener
{
    @Override
    protected Injector getInjector()
    {
        return injector;
    }

    public final Injector injector = Guice.createInjector(new ServletModule()
    {
        @Override
        protected void configureServlets()
        {
            bind(UserPool.class).in(Singleton.class);
            bind(User.class).in(SessionScoped.class);
        }
    });
}

public class JerseyEventListener implements ApplicationEventListener
{
    private ServiceLocator serviceLocator;

    @Inject
    public void setServiceLocator(ServiceLocator serviceLocator)
    {
        this.serviceLocator = serviceLocator;
    }

    @Override
    public void onEvent(ApplicationEvent event)
    {
        switch (event.getType())
        {
            case INITIALIZATION_FINISHED:

Injector injector = serviceLocator.getService(Injector.class);

// Instantiate Guice Bridge
GuiceBridge.getGuiceBridge().initializeGuiceBridge(serviceLocator);
GuiceIntoHK2Bridge guiceBridge = serviceLocator.getService(GuiceIntoHK2Bridge.class);

guiceBridge.bridgeGuiceInjector(injector);
break;
        }
    }

    @Override
    public RequestEventListener onRequest(RequestEvent requestEvent)
    {
        return null;
    }
}

public class JerseyResourceConfig extends ResourceConfig
{
    private ServiceLocator serviceLocator;

    @Inject
    public void setServiceLocator(ServiceLocator serviceLocator)
    {
        this.serviceLocator = serviceLocator;
    }

    @Context
    ServletContext servletContext;

    public JerseyResourceConfig()
    {
        super(JacksonFeature.class);

        registerClasses(JerseyEventListener.class);
        register(new JerseyDIBinder());
    }
}

public class JerseyDIBinder extends AbstractBinder
{
    @Override
    protected void configure()
    {
        bind(NullableGuiceScopeContext.class).to(NullableGuiceScopeContext.class)
.to(GuiceScopeContext.class)
.to(new TypeLiteral<Context<GuiceScope>>() {})
.in(Singleton.class);
        bindFactory(GuiceProvider.class).to(Injector.class).in(Singleton.class);
    }

    @Singleton
    private static class NullableGuiceScopeContext extends GuiceScopeContext
    {
        @Override
        public boolean supportsNullCreation()
        {
            return true;
        }
    }

    private static class GuiceProvider implements Factory<Injector>
    {
        @Inject
        public void setServletContext(ServletContext servletContext)
        {
            this.servletContext = servletContext;
        }

        ServletContext servletContext;

        @Override
        public Injector provide()
        {
            return (Injector) servletContext.getAttribute(Injector.class.getName());
        }

        @Override
        public void dispose(Injector instance)
        {

        }
    }
}
glassfishrobot commented 10 years ago

@glassfishrobot Commented @jwells131313 said: I like this methodology. If you are ok with it I may put this into the documentation for the Guice bridge (even though it is a Jersey example).

glassfishrobot commented 10 years ago

@glassfishrobot Commented cowwoc said: Guys, the problem is no longer when/how to initialize the GuiceBridge. The problem is that it is technically impossible for GuiceBridge to work unless Guice adds new functionality (more on this below).

There are only two possible solutions I am aware of:

  1. Guice needs to add functionality to make GuiceBridge work: https://code.google.com/p/google-guice/issues/detail?id=778 (or...)
  2. Jersey needs to become DI agnostic (allowing us to replace HK2 with Guice): JERSEY-1950

If you want this issue resolved, please voice your support on the Guice and Jersey mailing list and star the relevant issues.

glassfishrobot commented 10 years ago

@glassfishrobot Commented dreamershl said: to Jwells, it is my pleasure. Please go ahead.

glassfishrobot commented 10 years ago

@glassfishrobot Commented jontro said: Cowwoc, agreed. Im running into new issues all the time. Binding issues should be fail fast, not runtime aware. Custom type listeners do not seem to work etc.

glassfishrobot commented 10 years ago

@glassfishrobot Commented cowwoc said: (I'm posting this discussion on the HK2 issue tracker first. If it makes sense, we'll move discussion to the Jersey issue tracker)

I've recently moved away from Dependency Injection towards https://bitbucket.org/cowwoc/servicelocator/wiki/Home

I'm not necessarily advocating you do the same (although...) but this change of perspective gave me some ideas:

Guice cannot delegate to another framework for failed bindings, and the Jersey team refuses to provide the building blocks for creating such bindings. But... what about implementing a Guice Provider on top of HK2's ServiceLocator for each Jersey type (e.g. UriInfo) and bind it like Guice expects?

Meaning:

  1. Configure HK2 to inject Jersey types. Don't try making it inject Guice types.
  2. Create one Provider per Jersey-type, built on top of a HK2 ServiceLocator which is passed into the Module.
  3. At runtime, get your hands on an HK2 ServiceLocator, use it to create a Guice Injector then add a binding to HK2 which delegates to this injector for unknown types.

The beauty of this approach is that it would work for any DI framework, including https://bitbucket.org/cowwoc/servicelocator/wiki/Home

This story isn't complete for two reasons:

  1. Can we add an HK2 binding for unknown types after the HK2 ServiceLocator is already instantiated? I think so, but we'd need to double check.
  2. We need to figure out whether GuiceFilter is still needed injecting resources (I suspect it is), or whether we can find a replacement. If we need GuiceFilter then the Jersey team needs to let us pass in a ServiceLocator into the ResourceConfig constructor instead of having ResourceConfig construct its own instance.
glassfishrobot commented 10 years ago

@glassfishrobot Commented cowwoc said: It took me a while, but I can finally provide you with some answers.

  1. Yes, you can add bindings to ServiceLocator after its already instantiated.
  2. I haven't tried implementing this with GuiceFilter but I don't think you need it. If you use ServletContainer, injection will pass from Jersey to HK2 to Guice, which is just fine.

Here is how I got this to work for my own IoC framework: https://bitbucket.org/cowwoc/pouch-jersey2/wiki/Home

I believe the following steps will work for Guice:

  1. Inject ServiceLocator into the Jersey application. Notice that HK2's ServiceLocator must be instantiated before Guice's Injector.
  2. Create a new Guice injector, passing in a Module that takes the ServiceLocator as argument.
  3. The Module should bind each Jersey type to a Provider that delegates to ServiceLocator for injection. So for example, when you invoke Injector.getInstance(UriInfo.class) this should bind to a Provider that invokes serviceLocator.getService(UriInfo.class).
  4. GuiceBridge.getGuiceBridge().initializeGuiceBridge(serviceLocator) will take care of the opposite direction, instructing HK2 to delegate to Guice on unknown types.

The main reason I haven't translated these instructions into code is that I have moved from from Guice to Pouch: https://bitbucket.org/cowwoc/pouch/

Please let me know if anyone picks this up and converts it into a library.