dropwizard-guicier icon indicating copy to clipboard operation
dropwizard-guicier copied to clipboard

Not able to find method enableAutoConfig in GuiceBundle in version >= 1.0.0.0

Open harmishlakhani opened this issue 7 years ago • 5 comments

In dropwizard-guice we have enableAutoCongig method present in which we can give base package and Dropwizard and Guice will take for resource classes. In dropwizard-guicer we are not able to find such method and because of that we have to tell jersey for base package. Because of this Guice is not able to inject dependencies of services into resources. So I think if somehow we are enabling this package scan phase in guicer then we should be able to inject service as well with use of @Inject annotation to resources.

harmishlakhani avatar Sep 10 '18 12:09 harmishlakhani

We ultimately found the AutoConfig feature in dropwizard-guice to be harmful for a few reasons. First, we found the reflections library to be unreliable; it wouldn't always discover everything that it should, and this caused people to not trust the feature. Second, it was too magical and hard to reason about. Seemingly harmless code reorganization would break applications because AutoConfig was relying on a specific package structure. Third, it was hard to override. If something gets picked up by AutoConfig that you don't want, there is no natural way to stop it from happening.

In dropwizard-guicier, you would bind these classes in Guice and they would get added to the Dropwizard environment. There is an example of this here: https://github.com/jhaber/dropwizard-guicier-example/blob/7d5e5161b4dddfe99acbcf99f72bfd8a16540e5f/src/main/java/com/hubspot/dropwizard/example/ExampleModule.java#L20-L38

This may require a little more code, but we have found that this extra step makes things much more explicit, reliable, and easy to reason about.

We don't have plans to re-introduce functionality similar to AutoConfig, but you could probably recreate it using something like this: https://github.com/manzke/guice-automatic-injection which I think you could use to create Guice bindings via classpath scanning. I'd also be curious if you have any use-cases that require AutoConfig, or if it's just a matter of convenience.

jhaber avatar Sep 13 '18 14:09 jhaber

Thanks for the explanation, I can see why some teams don't like classpath scanning. But, :-) ... there are teams that are used to it, especially those coming from the Spring framework. Since it was an optional configuration with a default set to false I don't see why wouldn't the library support it and let the programmer decide if he likes auto config or not. You're right it's easy to add, I did it, but I also noticed there were at least 3 issues about this lost functionality. So I guess people use it. To simplify the migration from the previous version I think it would be nice to add it. Are you open for a pull-request to address this?

liorhar avatar Sep 13 '18 18:09 liorhar

I don't think we should add native support in dropwizard-guicier, and instead rely on users who want that functionality to add it themselves. I don't imagine this being particularly onerous, and it also seems like this functionality could be bundled up into a separate library.

jhaber avatar Sep 14 '18 17:09 jhaber

I could use some guidance on registering an implementation of ConfiguredBundle while also performing dependency injection. What is the correct way to do it given AutoConfig would have picked it up previously?

jbridger avatar Sep 17 '19 18:09 jbridger

Do you have a code sample? In particular, what does your ConfiguredBundle do?

jhaber avatar Sep 17 '19 18:09 jhaber