Improve the com.google.inject.name.Names properties resolution

[GoogleCodeExporter]

Loading Properties/Map works like a charm but I'm sure it could be improved 
(and users would appreciate) with few effort: what I'm proposing, and 
submitting in the attached patch, is enabling the Apache Ant style properties 
resolution without modifying the existing Guice APIs.

The already known scenario is: given a dynamic property

    JDBC.url=jdbc:mysql://${JDBC.host}:${JDBC.port}/${JDBC.schema}

and given the following set of fixed properties

    JDBC.host=localhost
    JDBC.port=3306
    JDBC.schema=guice

when requesting the injection of JDBC.url:

    @Inject
    @Named("JDBC.url")
    String url;

the set value will be

    System.out.println(url) ---> jdbc:mysql://localhost:3306/guice

That will be available just invoking

    com.google.inject.name.Names#bindProperties(Binder,Map<String, String>)
    com.google.inject.name.Names#bindProperties(Binder,Properties)

moreover it would be useful adding 2 shortcuts to bind java system properties :

    com.google.inject.name.Names#bindSystemProperties(Binder)

so users could reuse them as variables:

    os=${os.name}-${os.version} (${os.arch})

same thing for environment variables:

    com.google.inject.name.Names#bindEnvironmentProperties(Binder)

so

    username={USER}

Implementation notes: no external dependencies and no APIs modifications are 
needed, only 4 package private classes have been added

Original issue reported on code.google.com by simone.t...@gmail.com on 24 Sep 2010 at 2:31

Attachments:

• NamesImprovements.patch

[GoogleCodeExporter]

Forgot to mention that this Guice-approach is more powerful than the Spring 
one: I had a not so good experience with the Spring's 
[http://static.springsource.org/spring/docs/2.0.2/api/org/springframework/beans/
factory/config/PropertyPlaceholderConfigurer.html 
PropertyPlaceholderConfigurer] because it forces users to centralize the 
Properties loading; I mean, to make things working, in the way that ${} 
expressions were correctly resolved, I had to declare just one 
PropertyPlaceholderConfigurer for the whole context, list all the *.properties 
files for the all modules.

I didn't like that approach at all, it doesn't allow users to split 
configurations in different modules even if some keys are shared by two or more 
modules. With this approach users won't have these problems at all, they can 
load Properties in different modules and obtain the same result.

Original comment by simone.t...@gmail.com on 24 Sep 2010 at 2:37

[GoogleCodeExporter]

Forgot also to mention that no additional data structures have been used, the 
variable resolution is totally delegated to the Injector.

Thanks to this delegation, when creating an Injector instance, users could let 
some variable undefined, to define them in a child Injector.

Like in Apache Ant, if a variable is not resolved, won't be replaced at all and 
will stay in its declaration form.

Original comment by simone.t...@gmail.com on 25 Sep 2010 at 11:06

[GoogleCodeExporter]

This sounds like it would make an excellent extension, but not something that 
needs to be in core Guice.  Is there something Guice doesn't provide right now 
that would prevent this from being built as an extension?

Original comment by sberlin on 26 Sep 2010 at 8:49

[GoogleCodeExporter]

there are no blocking issues that prevent this feature built as an extension, 
I'm already using it as external module (you can see it on 
http://www.99soft.org/projects/rocoto/3.2/simple-configuration.html); that 
module has also the feature of loading properties files from classpath, URLs 
and filesystem.

I'm open on submitting another patch, if you're interested, just let me know.

Original comment by simone.t...@gmail.com on 26 Sep 2010 at 9:39

[GoogleCodeExporter]

If you'd like to build it out as an extension, we can link to it @ 
http://code.google.com/p/google-guice/wiki/3rdPartyModules .

Original comment by sberlin on 26 Sep 2010 at 10:09

[GoogleCodeExporter]

given the simplicity and lightweight of the code included in the submitted 
patch, it still would be very nice IMHO including it in the core module, since 
public APIs won't be changed at all, users can still use the Names class in the 
traditional way and the use of ${} variables is totally under the hood, it is 
completely backward compatibile.

BTW just let me know what you prefer.

Moreover, being the original author of other Guice integrations, I'm interested 
in participating 3rdPartyModules program, it would be nice adding:
 * Apache Bean Validation [http://incubator.apache.org/bval/cwiki/obtaining-a-validator.html] see _Using Google Guice_ section;
 * MyBatis (formerly Apache iBATIS) integration module with Guice (http://code.google.com/p/mybatis)

Original comment by simone.t...@gmail.com on 27 Sep 2010 at 7:32

[GoogleCodeExporter]

One possible issue with replacing the current behavior of bindProperties() is 
that some users could be using Ant-style placeholders in properties they load 
via bindProperties() and it would break their code to have those placeholders 
filled in automatically. Unlikely perhaps, but it suggests that at the very 
least the existing behavior needs to be retained for the existing methods. I do 
like the idea of being able to do this, though.

Original comment by cgdec...@gmail.com on 27 Sep 2010 at 4:42

[GoogleCodeExporter]

This needs to go in a third-party module. It's a fantastic idea, but there's so 
much room for design decisions and features that it doesn't make sense to lock 
it to the core platform.

Guice is intended to have a small core and to be easily extensible. If we could 
do it again I'm not even sure that Names would be in the core API. 

If you can post this as a 3rd party module, we'll make sure to make it linked 
and accessible to other Guice users!

Original comment by limpbizkit on 28 Sep 2010 at 4:14

• Changed state: WontFix

[GoogleCodeExporter]

Ok, here it comes the link to add in the third-party modules page: 
http://www.99soft.org/projects/rocoto/3.2/

thanks in advance :)

Original comment by simone.t...@gmail.com on 28 Sep 2010 at 8:05