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

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

Open
vmd408 opened this issue Sep 20, 2016 · 6 comments
Open

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

vmd408 opened this issue Sep 20, 2016 · 6 comments

Comments

@vmd408
Copy link

vmd408 commented Sep 20, 2016

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.

@heldeen
Copy link

heldeen commented Oct 5, 2016

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
}

@sachin-r
Copy link

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?

@vmd408
Copy link
Author

vmd408 commented Dec 27, 2016

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.

@xargsgrep
Copy link

xargsgrep commented Dec 28, 2016

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>

@jontejj
Copy link

jontejj commented Aug 4, 2017

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.

@jontejj
Copy link

jontejj commented Aug 7, 2017

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.

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

No branches or pull requests

5 participants