Assume you declare a configuration properties class like this:
@ConstructorBinding
@ConfigurationProperties("sample")
class SampleProperties {
SampleProperties(@DefaultValue("${user.home}/foo") String location) { … }
}
If the application now configures the property sample.location to ${user.home}/bar, the constructor is called with /Users/…/bar, i.e. the dynamic reference to the user home folder has been replaced with the actual value. If the application does not configure an explicit value, the value handed into constructor is literally ${user.home}/foo, i.e. the reference to the user's home directory is not resolved.
Comment From: wilkinsona
This is a duplicate of https://github.com/spring-projects/spring-boot/issues/23164. With hindsight, I shouldn't have closed that one but used it to update the documentation instead. We can use this issue to at least do that. We can also use this as an opportunity to confirm that we want to keep the current behaviour. As things stand, the behaviour of @DefaultValue aligns with how things work with a mutable property when the field's declared with a default value.
Comment From: dsyer
Maybe that's a bug too then? I mean, this seems like a reasonable use case to me. OTOH I suppose you can always add sample.location=${user.home}/foo to application.properties.
Comment From: odrotbohm
That's a bit inconvenient if the property is part of a library. Isn't the point of a default value, that the user doesn't have to provide it in the first place?
Comment From: wilkinsona
Maybe that's a bug too then?
I don't think so. In addition to the recommendation that default values are constant, if we honoured placeholders in a field's default value, it would have a huge impact on binding performance. For every property on a @ConfigurationProperties class, irrespective of whether there's a property being bound to it, we'd have to find its backing field, retrieve its value, perform placeholder resolution, and then set the value again.
Comment From: odrotbohm
I am not sure that I follow. @DefaultValue can only be used on constructor parameters of configuration properties classes using constructor binding or setters, right? Only if the annotation is present and no value is configured explicitly, that processing has to happen. Which it also would if the user provided the value via application.properties. Am I missing something?
Comment From: dsyer
Agree - to me the equivalence between @DefaultValue and a field with an initializer seems false.
Comment From: wilkinsona
I thought Dave's suggestion that "maybe that's a bug too then" was in response to "the behaviour of @DefaultValue aligns with how things work with a mutable property when the field's declared with a default value". What I was trying to say is that if the field's initializer uses a placeholder (for example private String credentials = "${username}:${password}") it'll be unchanged by property binding unless that value's being overwritten.
Comment From: odrotbohm
I see. Didn't even think that far. I was solely referring to handling the String values declared in the annotation.
Comment From: wilkinsona
As things stand, the desired behaviour can be achieved like this with setter-based binding:
@ConfigurationProperties("sample")
public class SampleMutableProperties {
/**
* Location to use. Defaults to a directory named example in the user's home directory.
*/
private String location = System.getProperty("user.home") + "/example";
public String getLocation() {
return location;
}
public void setLocation(String location) {
this.location = location;
}
}
The same can be achieved with immutable configuration properties like this:
@ConstructorBinding
@ConfigurationProperties("sample")
public class SampleImmutableProperties {
/**
* Location to use. Defaults to a directory named example in the user's home directory.
*/
private final String location;
public SampleImmutableProperties(String location) {
this.location = (location != null) ? location : System.getProperty("user.home") + "/example";
}
public String getLocation() {
return location;
}
}
The javadoc for @DefaultValue states that it "can be used to specify the default value when binding to an immutable property". That's not really accurate when you're trying to turn SampleMutableProperties into something immutable. You have to perform the defaulting manually in the constructor as shown above instead.
Knowing that you've only got a string to work with, trying to use @DefaultValue("${user.home}/example") (particularly if you're already familiar with Spring) seems quite logical to me but it won't work as hoped as ${user.home} will be left as-is. If it worked as @odrotbohm expected and performed property placeholder resolution it would be more concise and also have the benefit over the field initializer of being able to document a sensible default value for the property automatically.
Comment From: philwebb
One issue we'd have if we changed this would be what if a user wanted to use "${...} without having placeholder resolution occur. I think we'd a way to do that, and we'd also need to consider back-compatibility.
I personally lean towards keeping this as they are and improving the javadoc. I quite like the alignment with the way that fields currently work. I also worry a bit about the overlap with @Value if we resolve placeholders.
It's also work considering that in this example, I think we're really wanting the user home folder, which property resolution may or may not give you depending on the way that property sources have been configured. It's possible (but unlikely) that the user has removed the systemProperties source. It's also possible that they have a USER_HOME environment variable set, or a user.home value in their application.properties. That might be fine, but it's something to consider.
Comment From: philwebb
Having said that, the name @DefaultValue does make one think that the behavior would be similar to @Value. 🤔
Comment From: odrotbohm
One issue we'd have if we changed this would be what if a user wanted to use
"${...}without having placeholder resolution occur. I think we'd a way to do that, and we'd also need to consider back-compatibility.
Can someone achieve this when declaring the property in a properties file (maybe by escaping the $)? Can / should it work the same way in the annotation, then? Ultimately, I was assuming symmetry between an explicit configuration of the value in a properties file and the annotation attribute definition. Wouldn't the challenge you outline regarding the environments apply to both cases, too?
Comment From: philwebb
I pretty sure we don't offer $ escaping in property files. I thought we had an open issue about that but I can't find it.
Yes, the challenge of environments would apply in properties files as well.
Comment From: dsyer
Escaping is kind of important. I’m surprised we have got this far without explicit support for it. But @odrotbohm is right: it’s not specifically a problem with @DefaultValue.
Comment From: wilkinsona
We talked about this and concluded that we’d like to align the behaviour of @DefaultValue with what would happen if you had written the same value in application .properties or application.YAML. You get conversion at the moment (so, for example, "3w" for a Duration would become a Duration of 3 weeks), but you don’t get placeholder replacement. For consistency, we’d like to get both.
We have some concerns about backward-compatibility so it’ll have to wait for 3.x. We’ve reopened #23164 to improve the docs in the meantime.