If you read some of my posts dealing with dependency injection in Java you have probably figured it out (and I even stated it in clear) that I use Netflix Governator’s a lot. Especially as I revealed in one of my previous posts because Google Guice doesn’t support out of the box the likes of @PostConstruct and @PreDestroy — whereas Governator provides all the lifecycle management you might need occasionally in the land of DI.
And since I use it a lot I also make sense of some of its other features — in particular the @Configuration annotation I find useful: simply annotate a member variable in your class with it and it will be assigned auto-magically the value of the given property to your variable! Saves all the hassle from dealing with the Properties class yourself in Java.
For instance if your application.properties looks like this:
my.string.value=some value i want
some.int.value=123 |
then you can just do this:
public class MyClass { @Configuration("my.string.value") private String one; @Configuration("some.int.value") private int two; } |
And you get the values from your properties file assigned — you must admit that’s quite neat!
The problem with this magic is that it’s so transparent to the user then occasionally I see techies getting this terribly wrong.
The mistake that some devs would make about this is that they assume once you annotate a member field with @Configuration the value gets assigned at construction time. And based on that assumption they would write code such as:
In the above example we are trying to ensure that all file names we build in that class are prefixed by a certain directory — let’s say to ensure all files we create in that class are created inside that directory (ok, not the best example but roll with it as it does help proving the point). To do this, the coder decided to store the directory name as a configuration in their application properties under the key path.prefix. Since this is constant throughout the app why not declare that member as static final right? This way we make sure no one messes it up for us. And then when we create a new instance of our class simply prepend this value to the path given in the constructor.
To the untrained eye this will even pass a code review probably — and make its way into production causing havoc 🙂
There are a few problems with this code:
First of all the variable is final — which means once the value is set it cannot be changed. No compiler will signal though the issue with using @Configuration since the compiler doesn’t “know” that annotation involves runtime code to change the value. As such it compiles fine and when run, Governator will fail to update that value — so it will stay null.
Secondly, Governator will not process that annotation when the class is being loaded — which is often the assumption — but when Governator kicks in. By that point the class is already loaded and the field set to its final value.
Which means that when we create an instance of the class the field is null so we concatenate null with the given value … and get some rubbish 🙂
Another slightly different approach is to declare the field only as final but not static. On the assumption that ok, static fields get initialized when the class is loaded by the classloader, but when the DI mechanism will create a new instance then the field will be set.
Again, DI will first create an instance of the object — using the constructor — and only after the instance is created all the @Configuration annotations will be processed. Which means that still at the time our constructor kicks in our member variable will be null. And again, because the field is null, even after Governator traverses the annotations it will still not be able to update that field.
So what is the right away to have @Configuration fields and use them in initializing our object you ask? The answer lies in the @PostConstruct annotation: this kicks in after the instance (the object) has been constructed but before the object is “released in the wild”. So we can do anything like that in a method annotated with @PostConstruct:
As you can see this requires a 2 step operation: we store the value passed in our constructor first then we concatenate it only in the @PostConstruct section, when our @Configuration values have been filled in by Governator.
However, this turns out to be an easy mistake to make — especially by techies new to the Guice/Governator framework. So what I thought is that it would be nice if we could have an automated code check to detect that, right? And with that in mind I turned my attention to Checkstyle.
Checktyle already has built-in checks which verify various aspects around annotations. So I set off to write my own. I didn’t want to limit it to just this particular case — there might be other frameworks and other annotations out there that don’t work with final modifier. And in fact, I thought, why limit this to just the final modifier? Let’s allow just a combination of any annotation name and any modifier to be signalled by this check!
And this is how this pull request came to life: https://github.com/checkstyle/checkstyle/pull/4118
It adds a new Checkstyle check that takes an annotation and a modifier and triggers a notification when this combination is met. In our case we would simply do:
<module name="AvoidAnnotationCombination"> <property name="annotation" value="Configuration"/> <property name="modifier" value="FINAL"/> </module> |
This configures exactly the scenario we described above: signals any field that is final and annotated with @Configuration.
The pull request is work in progress — the Checkstyle peeps have their own code checks and style in place and I am still learning about it. But hopefully soon enough this open source contribution will make its way into their code base — and I will follow up here with more details on how to use that check when it does.
Meanwhile just be aware of this subtlety when using @Configuration since the compiler won’t signal anything — and Governator will just fail quietly.
