While I was creating a simpler test case for the bug report, it appears that .setDefault and .setBinding behave consistently wrt each other, but violate rule/step #5 in BindingResolution https://github.com/google/guice/wiki/BindingResolution .
I filed an issue for my own personal curiosity here: https://github.com/google/guice/issues/901 Will be interesting to see the official opinion. Thanks for your help! On Monday, February 2, 2015 at 1:25:51 AM UTC-7, Laszlo Ferenczi wrote: > > Josh, > > Well, let's see some official opinion on this matter, i'm just a user of > guice. > > -- > L > > On Sun, Feb 1, 2015 at 8:20 PM, Joshua Moore-Oliva <[email protected] > <javascript:>> wrote: > >> Thanks, that helped me ramp up on the guice ecosystem and configuration. >> >> However - the fact that setDefault has different binding coercion >> semantics to setBinding smells like a bug to me. Is there any reason that >> its current behaviour is the correct one? If not I might file a bug report. >> >> >> >> >> On Saturday, January 31, 2015 at 4:42:38 AM UTC-7, Laszlo Ferenczi wrote: >>> >>> Hi Josh, >>> >>> Using the binding mechanism in guice might be a bit of an overkill for >>> simple property bindings, however you have a number of alternatives: >>> >>> - Properties itself allows for defaults handling, create a property file >>> with the defaults and create one for the application and merge them in the >>> properties object. Binding the resulting properties will have the defaults. >>> - Bind the properties object itself and manually fetch the values from it >>> - Specify defaults in the code, only override when you get a meaningful >>> value injected >>> >>> A more complex way would be to roll your own configuration management. >>> In our projects we use an in-house developed ConfigManager object which >>> is similar to the Properties object but it provides typesafe getters for >>> various primitive types. A generic TypeListener allows the injections of >>> config values into the object. (conceptually very similar of what netflix's >>> governator does) >>> Obviously you can use the governator library also, it'll do the trick >>> (and many others). I believe apache onami has a solution for configuration >>> too. >>> >>> Hope this helps. >>> >>> >>> -- >>> L >>> >>> On Sat, Jan 31, 2015 at 5:25 AM, Joshua Moore-Oliva <[email protected]> >>> wrote: >>> >>>> First off - I am only a few days into guice, so I apologize if my >>>> questions are dumb - I have tried to compensate with what I hope are clear >>>> examples. >>>> >>>> I am attempting to make liberal use of OptionalBinder in my code to >>>> allow some default values to be specified and then potentially overriden >>>> by >>>> configuration. I have run into a cascading series of issues. First, lets >>>> start with what works in a simple case that currently works (but does not >>>> have the default value functionality I hope to achieve with >>>> OptionalBinder): >>>> >>>> public class Simple { >>>> private final int simpleInt; >>>> private final String simpleStr; >>>> >>>> @Inject >>>> public Simple(@Named("simpleInt") int simpleInt, >>>> @Named("simpleStr") String simpleStr) { >>>> this.simpleInt = simpleInt; >>>> this.simpleStr = simpleStr; >>>> } >>>> >>>> public int getSimpleInt() { >>>> return simpleInt; >>>> } >>>> >>>> public String getSimpleStr() { >>>> return simpleStr; >>>> } >>>> } >>>> >>>> >>>> If I wish to wire up the value of simpleVal from configuration, I can >>>> use Names.bindProperties (which was recommended in the FAQ) >>>> >>>> public class SimpleTest { >>>> >>>> @Test >>>> public void testGetSimpleVal() throws Exception { >>>> Injector inj = Guice.createInjector(getTestModule()); >>>> Simple s = inj.getInstance(Simple.class); >>>> >>>> assertEquals(1, s.getSimpleInt()); >>>> assertEquals("default", s.getSimpleStr()); >>>> } >>>> >>>> private Module getTestModule() { >>>> return new AbstractModule() { >>>> @Override protected void configure() { >>>> bind(Simple.class); >>>> >>>> Properties p = new Properties(); >>>> p.setProperty("simpleInt", "1"); >>>> p.setProperty("simpleStr", "default"); >>>> >>>> Names.bindProperties(binder(), p); >>>> } >>>> }; >>>> } >>>> } >>>> >>>> So far, so good - Names.bindProperties is great, it even converts my >>>> strings to ints ("1" -> 1 for simpleInt) >>>> >>>> Now I attempt to set up default values for simpleInt and simpleStr >>>> using OptionalBinder. I do this by returning another Module with default >>>> bindings. (I assume I should do this on a package not class level, but it >>>> keeps the example simple). >>>> >>>> public class Simple { >>>> private final int simpleInt; >>>> private final String simpleStr; >>>> >>>> @Inject >>>> public Simple(@Named("simpleInt") int simpleInt, >>>> @Named("simpleStr") String simpleStr) { >>>> this.simpleInt = simpleInt; >>>> this.simpleStr = simpleStr; >>>> } >>>> >>>> public int getSimpleInt() { >>>> return simpleInt; >>>> } >>>> >>>> public String getSimpleStr() { >>>> return simpleStr; >>>> } >>>> >>>> public static AbstractModule getDefaultsModule() { >>>> return new AbstractModule() { >>>> @Override protected void configure() { >>>> >>>> OptionalBinder.newOptionalBinder(binder(), >>>> Key.get(Integer.class, Names.named("simpleInt" ))) >>>> .setDefault().toInstance(12345); >>>> >>>> //OptionalBinder.newOptionalBinder(binder(), >>>> Key.get(String.class, Names.named("simpleInt" ))) >>>> // .setDefault().toInstance("12345"); >>>> >>>> OptionalBinder.newOptionalBinder(binder(), >>>> Key.get(String.class, Names.named("simpleStr"))) >>>> .setDefault().toInstance("Simple class >>>> default"); >>>> } >>>> }; >>>> } >>>> } >>>> >>>> public class SimpleTest { >>>> >>>> @Test >>>> public void testGetSimpleVal() throws Exception { >>>> Injector inj = Guice.createInjector(Simple.getDefaultsModule(), >>>> getTestModule()); >>>> Simple s = inj.getInstance(Simple.class); >>>> >>>> assertEquals(1, s.getSimpleInt()); >>>> assertEquals("default", s.getSimpleStr()); >>>> } >>>> >>>> private Module getTestModule() { >>>> return new AbstractModule() { >>>> @Override protected void configure() { >>>> bind(Simple.class); >>>> >>>> Properties p = new Properties(); >>>> p.setProperty("simpleInt", "1"); >>>> p.setProperty("simpleStr", "default"); >>>> >>>> Names.bindProperties(binder(), p); >>>> } >>>> }; >>>> } >>>> } >>>> >>>> At this point, I get the following error >>>> >>>> 1) A binding to java.lang.String annotated with >>>> @com.google.inject.name.Named(value=simpleStr) was already configured >>>> at com.baselayer.device.scratch.Simple$1.configure(Simple.java:42). >>>> at com.baselayer.device.scratch.SimpleTest$1.configure( >>>> SimpleTest.java:33) >>>> >>>> Looking into the source code for Names.bindProperties, I see this is >>>> because it does not use setBinding, but rather normal non-optional >>>> .toInstance. >>>> >>>> binder.bind(Key.get(String.class, new NamedImpl(propertyName))).toIn >>>> stance(value); >>>> >>>> At this point, I assume there is some good reason that you must use >>>> setBindings, though I will make the suggestion that as a new user of >>>> guice, >>>> I would expect a normal non-optional .toInstance to override a .setDefault. >>>> >>>> So, shamelessly copying the source code of Names.bindProperties, I >>>> created my own replacement test module as follows: >>>> >>>> private Module getTestModule() { >>>> return new AbstractModule() { >>>> @Override protected void configure() { >>>> bind(Simple.class); >>>> >>>> Properties p = new Properties(); >>>> p.setProperty("simpleInt", "1"); >>>> p.setProperty("simpleStr", "default"); >>>> >>>> //Names.bindProperties(binder(), p); >>>> //The following code replaces Names.bindProperties >>>> Binder skippedBinder = binder().skipSources(Names. >>>> class); >>>> >>>> for (Enumeration<?> e = p.propertyNames(); >>>> e.hasMoreElements(); ) { >>>> String propertyName = (String) e.nextElement(); >>>> String value = p.getProperty(propertyName); >>>> OptionalBinder.newOptionalBinder(skippedBinder, >>>> Key.get(String.class, Names.named(propertyName))). >>>> setBinding().toInstance(value); >>>> } >>>> } >>>> }; >>>> } >>>> >>>> However - unlike Names.bindProperties, the String binding will not >>>> stick to simpleInt. The very useful auto-mutation of String to Int that >>>> occurs with non-optional .toInstance does not appear with .setBinding. >>>> Either I get >>>> >>>> 1) No implementation for java.lang.Integer annotated with >>>> @com.google.inject.name.Named(value=simpleInt) was bound. >>>> while locating java.lang.Integer annotated with >>>> @com.google.inject.name.Named(value=simpleInt) >>>> for parameter 0 at com.baselayer.device.scratch. >>>> Simple.<init>(Simple.java:20) >>>> at com.baselayer.device.scratch.SimpleTest$1.configure( >>>> SimpleTest.java:27) >>>> >>>> >>>> If I do not have any .setDefault bound for simpleInt in >>>> Simple.getDefaultsModule (neither Integer.class or String.class), or I get >>>> >>>> Failed tests: testGetSimpleVal(com.baselayer.device.scratch.SimpleTest): >>>> expected:<1> but was:<12345> >>>> >>>> If I had bound an Integer in Simple.getDefaultsModule with >>>> >>>> OptionalBinder.newOptionalBinder(binder(), Key.get(Integer.class, >>>> Names.named("simpleInt" ))) >>>> .setDefault().toInstance(12345); >>>> >>>> Finally - If I use >>>> >>>> OptionalBinder.newOptionalBinder(binder(), Key.get(String.class, >>>> Names.named("simpleInt" ))) >>>> .setDefault().toInstance("12345"); >>>> >>>> But do NOT overwrite it with setBinding(), then I get the following >>>> error >>>> >>>> 1) No implementation for java.lang.Integer annotated with >>>> @com.google.inject.name.Named(value=simpleInt) was bound. >>>> while locating java.lang.Integer annotated with >>>> @com.google.inject.name.Named(value=simpleInt) >>>> for parameter 0 at com.baselayer.device.scratch. >>>> Simple.<init>(Simple.java:20) >>>> at >>>> com.baselayer.device.scratch.SimpleTest$1.configure(SimpleTest.java:30) >>>> (via modules: com.google.inject.util.Modules$OverrideModule -> >>>> com.baselayer.device.scratch.SimpleTest$1) >>>> >>>> >>>> It appears that guice will bind a String to an int with the normal, >>>> non-optional .toInstance, but .setDefault .toInstance refuses to bind a >>>> String to an int with the OptionalBinder. (but .setBinding .toInstance >>>> will >>>> set a String to an Integer, as long as a separate specific Integer binding >>>> does not already exist). >>>> >>>> If .setDefault and .setBinding .toInstance had the same semantics as >>>> normal, non-optional .toInstance, I could make my own Names.bindProperties >>>> and use that everywhere instead (with the intent being that if any default >>>> was already set, I would overwrite it) >>>> >>>> However, the only thing I have tried that works is to attempt to bind >>>> to every single instance (ensuring that the exact type has been bound), >>>> something like this for getTestModule() >>>> >>>> private Module getTestModule() { >>>> return new AbstractModule() { >>>> @Override protected void configure() { >>>> bind(Simple.class); >>>> >>>> Properties p = new Properties(); >>>> p.setProperty("simpleInt", "1"); >>>> p.setProperty("simpleStr", "default"); >>>> >>>> //Names.bindProperties(binder(), p); >>>> Binder skippedBinder = binder().skipSources(Names. >>>> class); >>>> >>>> for (Enumeration<?> e = p.propertyNames(); >>>> e.hasMoreElements(); ) { >>>> String propertyName = (String) e.nextElement(); >>>> String value = p.getProperty(propertyName); >>>> OptionalBinder.newOptionalBinder(skippedBinder, >>>> Key.get(String.class, Names.named(propertyName))). >>>> setBinding().toInstance(value); >>>> >>>> try { >>>> int i = Integer.parseInt(value); >>>> OptionalBinder.newOptionalBinder(skippedBinder, >>>> Key.get(Integer.class, Names.named(propertyName))). >>>> setBinding().toInstance(i); >>>> } catch (NumberFormatException asdf) { >>>> } >>>> } >>>> } >>>> }; >>>> } >>>> >>>> In order to bind a configuration file's properties to @Named instances, >>>> I need to attempt to convert the String to each possible type if I wish to >>>> load dynamically without some kind of type metadata in the configuration >>>> file as well to know which type I should target the binding to. >>>> >>>> Having outlined all this, a few questions >>>> >>>> 1) Is it feasible for the behaviour of OptionalBinder to allow a >>>> .toInstance to override a .setDefault? If so then things like >>>> Names.bindProperties could (maybe) just work as is. If not, why? (I assume >>>> there is a good reason since .to was explicitly disallowed as an override >>>> for .setDefault in the documentation) >>>> >>>> 2) What is the intended behaviour of Names.bindProperties. Are strings >>>> supposed to be able to inject onto a Integer? Or only inject if a specific >>>> Integer injection doesn't already exist? (Are users supposed to be able to >>>> inject different String and Integer toInstance values for a Named >>>> annotation?) Is this behaviour I am experiencing via .setDefault >>>> something >>>> that hasn't been fully thought through, or is there a ruleset for when >>>> some >>>> types are supposed to ovverride other types when a more specific mapping >>>> doesn't already exist for the annotation? >>>> Experimenting with Modules.override it seems that normal >>>> non-optional .toInstance starts behaving similarly if the defaults module >>>> binds both an Integer and a String, and the overriding module just binds a >>>> string (As Names.bindProperties). >>>> >>>> 3) Are there any downsides to just OptionalBinding any configuration >>>> file property to a variety of common primitive types to ensure that all >>>> default injection parameters gets overridden by configuration? >>>> >>>> 4) If I just want default values (without the Optional<Foo> support), >>>> should I be using Modules.override instead? >>>> >>>> 5) Is setDefault working as intended, given that is has different >>>> semantics to .setBinding (.setDefault .toInstance String.class will not >>>> inject a String to an Integer in the absence of a Integer injection, but >>>> .setBinding .toInstance will just like normal non-optional .toInstance) >>>> >>>> Thanks for reading if you made it this far in my rocky start to DI in >>>> java :) >>>> >>>> Josh >>>> >>>> -- >>>> You received this message because you are subscribed to the Google >>>> Groups "google-guice" group. >>>> To unsubscribe from this group and stop receiving emails from it, send >>>> an email to [email protected]. >>>> To post to this group, send email to [email protected]. >>>> Visit this group at http://groups.google.com/group/google-guice. >>>> To view this discussion on the web visit https://groups.google.com/d/ >>>> msgid/google-guice/0c3bb455-d65d-4ed4-bf7d-e84d5f84f7af% >>>> 40googlegroups.com >>>> <https://groups.google.com/d/msgid/google-guice/0c3bb455-d65d-4ed4-bf7d-e84d5f84f7af%40googlegroups.com?utm_medium=email&utm_source=footer> >>>> . >>>> For more options, visit https://groups.google.com/d/optout. >>>> >>> >>> -- >> You received this message because you are subscribed to the Google Groups >> "google-guice" group. >> To unsubscribe from this group and stop receiving emails from it, send an >> email to [email protected] <javascript:>. >> To post to this group, send email to [email protected] >> <javascript:>. >> Visit this group at http://groups.google.com/group/google-guice. >> To view this discussion on the web visit >> https://groups.google.com/d/msgid/google-guice/a88525ad-a952-4364-9e30-d6bce7ea173a%40googlegroups.com >> >> <https://groups.google.com/d/msgid/google-guice/a88525ad-a952-4364-9e30-d6bce7ea173a%40googlegroups.com?utm_medium=email&utm_source=footer> >> . >> >> For more options, visit https://groups.google.com/d/optout. >> > > -- You received this message because you are subscribed to the Google Groups "google-guice" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected]. Visit this group at http://groups.google.com/group/google-guice. To view this discussion on the web visit https://groups.google.com/d/msgid/google-guice/9d5ad5ad-5b9f-4dc8-acd7-bb7123141950%40googlegroups.com. For more options, visit https://groups.google.com/d/optout.
