dropwizard / dropwizard

A damn simple library for building production-ready RESTful web services.
https://www.dropwizard.io
Apache License 2.0
8.51k stars 3.44k forks source link

DBIHealthCheck depends on DBI implementation rather than its abstraction #2124

Closed smifun closed 7 years ago

smifun commented 7 years ago

Hi, When using DBIHealthCheck it requires DBI as constructor param which makes it possible to init only in place where DBI is created. When trying to move that to separate method like that

    @Nonnull
    @EagerLoad
    public static HealthCheckRegistry buildHealthChecks(Environment environment, IDBI idbi) throws ClassNotFoundException {
        environment.healthChecks().register("database", new DBIHealthCheck((DBI) idbi, VALIDATION_QUERY));
        return environment.healthChecks();
    }

its producing error like this

Caused by: java.lang.ClassCastException: $IDBI_a3a4209300b4 cannot be cast to org.skife.jdbi.v2.DBI
    at FrontendModule.buildHealthChecks(FrontendModule.java:145)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:498)
    at org.apache.tapestry5.ioc.internal.util.MethodInvoker.invoke(MethodInvoker.java:52)
    ... 35 more

when I changed param type in buildHealthChecks to DBI I get

 No service implements the interface org.skife.jdbi.v2.DBI

IMO solution for this would be to use IDBI in DBIHealthCheck. DBIHealthCheck is only calling open method on DBI which is actually part of IDBI interface. Apart from that it good practice to depend on abstraction rather than implementation.

arteam commented 7 years ago

Do you have a specific reason to use IDBI in your factory method? $IDBI_a3a4209300b4 seems to me as a proxy class generated by a byte-code code generation library. Could you make sure that a DBI instance registered in the IoC framework you use?

Generally I agree with you that programming with interfaces is a good practice, but I doubt in usefulness of the IDBI interface. If I'm not mistaken, its origin comes from the times when JDBI was trying to integrate with the Spring Framework 2 and it required registered beans to have interfaces. Outside of Spring, this interface was never adopted and consequently removed in JDBI3.

smifun commented 7 years ago

I changed build method which creates DBI to return DBI instead of IDBI and it works. Didn't expect that it would be that simple. Thanks.