brooklyncentral/brooklyn — pull request #930 by aledsage (2013-09-12 10:18:15+00:00)
Title: Fix: NPE from ConfigKeys fields

Deletes ConfigKeys.BROOKLYN_DATA_DIR etc to avoid a horrible NPE problem (introduced when these keys were moved to BrooklynConfigKey.BROOKLYN_DATA_DIR and the deprecated field in ConfigKeys was changed to reference that).

By deleting these values, I'm breaking backwards compatibility, but I don't think we have much choice: can't leave it broken like this.

All seems to work inside brooklyn, but from a dependent project I see NPEs like this:
Caused by: java.lang.NullPointerException
at brooklyn.location.jclouds.JcloudsPropertiesFromBrooklynProperties.getJcloudsProperties(JcloudsPropertiesFromBrooklynProperties.java:77)
at brooklyn.location.jclouds.JcloudsResolver.newLocationFromString(JcloudsResolver.java:183)

I think the problem is down to the order of resolving statics in ConfigKeys and BrooklynConfigKeys. Resolving the fields of BrooklynConfigKeys involves calling ConfigKeys.newStringConfigKey(), which thus requires that ConfigKeys be resolved. If it pauses in initializing the fields of BrooklynConfigKeys to first do those of ConfigKeys, then looking up BrooklynConfigKeys.BROOKLYN_DATA_DIR will still be null. So ConfigKeys.BROOKLYN_DATA_DIR gets a null value! Yuck!

The other option would be to move the "real" field declarations back to ConfigKeys and have BrooklynConfigKeys just refer to those (and then move them permanently in the next brooklyn release). But there's a comment in ConfigKeys fields saying `@deprecated since 0.6.0; use {@link BrooklynConfigKeys} to prevent classload ordering problems`, and also `Key definitions were deprecated here in 0.6.0 because they introduce nasty circular dependencies on the methods in this class, causing some final fields to be null when they are accessed.` So that sounds very bad as well!