Declared defaults do not get property references replace

[odrotbohm]

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.

[wilkinsona]

This is a duplicate of #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.

[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.

[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?

[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.

[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?

[dsyer]

Agree - to me the equivalence between @DefaultValue and a field with an initializer seems false.

[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.

[odrotbohm]

I see. Didn't even think that far. I was solely referring to handling the String values declared in the annotation.

[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.

[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.

[philwebb]

Having said that, the name @DefaultValue does make one think that the behavior would be similar to @Value. 🤔

[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?

[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.