DropwizardEnvironmentModule - Class<? extends Configuration> support

[btiernay]

Is there any reason why DropwizardEnvironmentModule uses a Configuration @Provider instead of an application derived class? This mean that all dependent classes must downcast to use this dependency which is less than ideal. For value objects, it really makes sense to used the most derived type.

Perhaps also binding to an instance with a derived class object would help, by using configuration.getClass():

    modules.add(new AbstractModule() {
        @Override
        protected void configure() {
            bind(configuration.getClass()).toInstance(configuration);
            bind(Configuration.class).toInstance(configuration);
            bind(Environment.class).toInstance(environment);
        }
    });

[eliast]

@btiernay I absolutely agree and would love to change it. The reason it was done this way was because we are creating the injector before the run call so we don't have access to the environment or configuration. Would you like to take a stab at it and send us a pull request?

If you notice we create the module during the initialize phase then set the environment and configuration in the run method. This doesn't let us do anything during the configure method.

[btiernay]

The version of GuiceBundle.java lines 71-97 in 0.6.1.1 seems to have access to the configuration object in the run method:

@Override
public void run(final Configuration configuration, final Environment environment) {
    @SuppressWarnings("serial")
    GuiceContainer container = new GuiceContainer() {
        protected ResourceConfig getDefaultResourceConfig(
                Map<String, Object> props, WebConfig webConfig)
                throws javax.servlet.ServletException {
            return environment.getJerseyResourceConfig();
        };
    };
    environment.setJerseyServletContainer(container);
    environment.addFilter(GuiceFilter.class, configuration.getHttpConfiguration().getRootPath());

    modules.add(Modules.override(new JerseyServletModule()).with(new JerseyContainerModule(container)));
    modules.add(new AbstractModule() {

        @Override
        protected void configure() {
            // TODO: bind(configuration.getClass()).toInstance(configuration);
            bind(Configuration.class).toInstance(configuration);
            bind(Environment.class).toInstance(environment);
        }
    });
    Injector injector = Guice.createInjector(modules);
    if (autoConfig != null) {
        autoConfig.run(environment, injector);
    }
}

[eliast]

I guess you haven't seen the latest iteration. :( Let me ask @axiak about it.

[axiak]

We need to create the injector early in order to use guice for database configuration during the bootstrap phase. The only thing I can think of is asking for Class explicitly in the builder, and binding based off of that.

[btiernay]

I understand how you have 2 phases to building the injector and the constraints it imposes.

What about https://github.com/jhalterman/typetools
or, if that seems like overkill, call this in the init method to get the type:

Class parameterizedType = (Class) ((ParameterizedType) getClass()
.getGenericSuperclass()).getActualTypeArguments()[0];

This technique works where the type parameter is defined on the immediate superclass, but it fails if the type parameter is defined elsewhere in the type hierarchy.

[btiernay]

One more thing, Guice provides TypeLiteral for this type of thing:

http://google-guice.googlecode.com/git/javadoc/com/google/inject/TypeLiteral.html

This illustrates what you can do:

http://google-guice.googlecode.com/git/javadoc/com/google/inject/Binder.html

(see bind(new TypeLiteral<PaymentService>() {}).to(CreditCardPaymentService.class); )

Essentially in GuiceBundle's init, call new TypeLiteral(){} (notice the braces!) and pass that to the DropwizardEnvironmentModule constructor.

[btiernay]

Another option is to use a child injector in the run method. Not sure if this would break other dependencies.

Out of curiosity, what type of database access has to be done so early? And how are you configuring the connections without a Configuration instance?

[axiak]

I think the TypeLiteral is a bit overkill here. We would use the Key for this anyway, but I think it's easier just to ask for Class in the builder (and typesafe too).

I don't think the typetools or getGenericSuperclass() will work since we're not asking people to create concrete subclasses of GuiceBundle. Again, it's be simpler just to add a .setConfigurationClass(Class clazz) in the builder.

Essentially any time we need to use guice for creating a bundle, it needs to be during the bootstrap phase. At our company we have lots of existing configuration stuff based off of guice that we want to leverage here.

[btiernay]

The TypeLiteral is just a convenient way to access the generic type without requiring anything of the client (see http://stackoverflow.com/questions/3370641/how-does-guices-typeliteral-work for an explanation). However, I don't think GuiceBundle's type parameter is bound by the caller. This would actually be typesafe since the user can't accidentally pass the wrong class instance dynamically as you are recommending. However, I think your solution is fine if this doesn't work out.

Just curious, are you creating your own Configuration instance in initialize some where to configure your bundles? I might need to do this at some point and it would be good to know :)

[axiak]

I understand what TypeLiteral does and how it works (incidentally, the fact that it requires subclassing is why we can't access T in our GuiceBundle without subclassing). I'm just saying that if we ask for a TypeLiteral, we might as well ask for the more generic Key (http://google-guice.googlecode.com/svn/trunk/javadoc/com/google/inject/Key.html) class, since one can build a Key from a typeliteral or anything else that can identify an instance.
Let me create a pull request for the simple method.

As for the Configuration creation: right now we're leaving it up to the clients. We have a separate database configuration system (see https://github.com/HubSpot/Mothra) and we're going to ship a library to create a DatabaseConfiguration object given some zookeeper key.

[eliast]

This was resolved in the PR3.