HubSpot/dropwizard-guice

@BeanParam + @QueryParam don't work with Dropwizard-guice

Opened this issue ยท 6 comments

Dropwizard version: 1.0.0, dropwizard-guice version: 1.0.0.1

Given:
Resource method containing "@BeanParam"-annotated parameter, each field of the parameter's class is annotated with "@QueryParam(...)"

When server is running, values passed as query parameters to request are not passed to the resource method. The bean arrives with all fields' default (null) values.

Simple demo app (requires gradle)
Try to run main methods of GuiceApp and NoGuiceApp from your IDE and send get request to "localhost:8080/?foo=foo&bar=bar" to observe the issue.

This is an interesting problem. It looks like w/ Guice the org.vmd408.DemoParams class is being instantiated by Guice and taking a different code path then w/ just HK2. Check out org.glassfish.jersey.server.internal.inject.BeanParamValueFactoryProvider.BeanParamValueFactory#provide (snip of that method):

        @Override
        public Object provide() {

            final Class<?> rawType = parameter.getRawType();

            final Object fromHk2 = locator.getService(rawType);
            if (fromHk2 != null) { // the bean parameter type is already bound in HK2, let's just take it from there
                return fromHk2;
            }
            ActiveDescriptor<?> reifiedDescriptor = descriptorCache.compute(rawType);
            return locator.getServiceHandle(reifiedDescriptor).getService();
        }

Using Guice the locator is the Guice implementation of org.glassfish.hk2.api.ServiceLocator , com.squarespace.jersey2.guice.GuiceServiceLocator, which asks Guice first to provide objects and causes this method to return the fromHk2 object, which is a new DemoParams with nothing else done to it.

Using HK2 the object is returned from the last line of that method which does some more complicated stuff to create DemoParams and set the fields on it. Guice doesn't know about these extra steps.

Updating DemoParams like below at least sets the two strings foo and bar to empty "" but I don't know if that's actually getting closer or not. If I have more time later I'll take another stab at it.

public class DemoParams {

    @Inject
    public DemoParams(@QueryParam("foo") String foo, @QueryParam("bar") String bar) {
        this.foo = foo;
        this.bar = bar;
    }

    private String foo;
    private String bar;
    //omitted getter/setter
}

This issue is observed when using @FormParam with @BeanParam as well , seems this is related to HK2/Jersey issue, please refer - https://java.net/jira/browse/JERSEY-3119 . Is there a ETA for the next release of DropWizard with Jersey 2.23.2 & HK2 2.5.0-b06 , which claimed to have the fix?

FYI. Just tested with jersey 2.25
@QueryParam annotation works fine, however @BeanParam is still returning "empty" bean, and the flow is the same as described by @heldeen above.

I tested with HK2 version 2.5.0-b06 and it seemed to resolve the issue. So I guess as @sachin-r mentioned we need a new release of DW for that. For now I have to do something like:

        <dependency>
            <groupId>org.glassfish.hk2</groupId>
            <artifactId>guice-bridge</artifactId>
            <version>2.5.0-b06</version>
            <exclusions>
                <exclusion>
                    <groupId>com.google.inject</groupId>
                    <artifactId>guice</artifactId>
                </exclusion>
            </exclusions>
        </dependency>
        <dependency>
            <groupId>ru.vyarus</groupId>
            <artifactId>dropwizard-guicey</artifactId>
            <version>4.0.1</version>
            <exclusions>
                <exclusion>
                    <groupId>org.glassfish.hk2</groupId>
                    <artifactId>guice-bridge</artifactId>
                </exclusion>
            </exclusions>
        </dependency>

You can try to downgrade hk2 (as my comment in HubSpot/dropwizard-guicier#13 suggests). When I'm forcing the version with:

	<dependencyManagement>
		<dependencies>
			<dependency>
				<groupId>org.glassfish.hk2</groupId>
				<artifactId>hk2-locator</artifactId>
				<version>2.4.0-b33</version>
			</dependency>
			<dependency>
				<groupId>org.glassfish.hk2</groupId>
				<artifactId>hk2-api</artifactId>
				<version>2.4.0-b33</version>
			</dependency>
		</dependencies>
	</dependencyManagement>

it works.

As downgrading hk2 caused us some other issues, I investigated https://github.com/xvik/dropwizard-guicey and it seems to work better for this use case. So looks like we'll switch to that project for now :)

Before I switched I discovered that https://github.com/Squarespace/jersey2-guice/blob/master/jersey2-guice-impl/src/main/java/com/squarespace/jersey2/guice/GuiceJustInTimeResolver.java is quite different from https://github.com/javaee/hk2/blob/master/guice-bridge/src/main/java/org/jvnet/hk2/guice/bridge/internal/GuiceToHk2JITResolver.java

In that it tries:

            try {
                return guiceInjector.getBinding(key);
            } catch (ConfigurationException ce) {
                return null;
            } 

and catches exceptions in order to try creating instances with hk2 instead of with guice.