Support mixing constructor and setter binding for ConfigurationProperties

[quaff]

It's common to divide configuration properties into required and optional, the required properties are defined as final and should be bound via constructor, and the optional properties should be bound via setter method but currently ignored. For example:

@ConfigurationProperties(prefix = "my")
public class MyConfigurationProperties {

    private final String requiredParam1;
    private final String requiredParam2;

    private String optionalParam1;
    private String optionalParam2;

    public MyConfigurationProperties(String requiredParam1, String requiredParam2) {
        this.requiredParam1 = requiredParam1;
        this.requiredParam2 = requiredParam2;
    }

    public String getOptionalParam1() {
        return optionalParam1;
    }

    public void setOptionalParam1(String optionalParam1) {
        this.optionalParam1 = optionalParam1;
    }

    public String getOptionalParam2() {
        return optionalParam2;
    }

    public void setOptionalParam2(String optionalParam2) {
        this.optionalParam2 = optionalParam2;
    }

    public String getRequiredParam1() {
        return requiredParam1;
    }

    public String getRequiredParam2() {
        return requiredParam2;
    }

}

and

my:
  required-param1: required-param1
  required-param2: required-param2
  optional-param2: optional-param2

required-param1 and required-param2 are bound as expected, we should bind optional-param2 also.

[wilkinsona]

Thanks for the suggestion. This would add a fairly large amount of complexity to the binder, either requiring a mixing of the Java Bean and constructor binding logic, or requiring two passes. Optional parameters can be constructor bound, and personally I think that's better as it allows the class to remain immutable. Why do you want to mix the two?

[quaff]

or requiring two passes

I think this approach is better.

Optional parameters can be constructor bound, and personally I think that's better as it allows the class to remain immutable. Why do you want to mix the two?

That would make the constructor bloat, normally configuration properties are consist of a few required and more optional properties. I still suggest to implement it if the cost is not expensive to not surprise people who thought it will works, I can live with that, but we should document that it's not supported, maybe already done?

[wilkinsona]

I still suggest to implement it if the cost is not expensive

If it were to happen by default, it would roughly double the time take to perform configuration property binding. That could be avoided by adding a mechanism that allows you to opt in but my feeling is that it isn't worth it.

That would make the constructor bloat,

While it makes the constructor larger, the class would be more concise overall, even more so if you use a record.

we should document that it's not supported, maybe already done?

I don't think we explicitly say that the two cannot be mixed, but the documentation talks about the two forms of binding separately. This, as far as I can remember, is the first time I've seen someone expecting both to work on the same target.

Let's see what the rest of the team thinks.

[bclozel]

I don't think we should pursue this, it's going to add complexity to our codebase, probably introduce regressions and degrade performance. I think the two options we're offering are quite nice and I don't see why we should document all other possible options that we don't support.

[quaff]

Another concern, default value of optional property need extra @DefaultValue which is not elegant and typesafe since Java doesn’t support default method parameter like Kotlin, correct me if I’m wrong.

[philwebb]

I'm afraid I don't think we should pursue the either. IMO one of the best benefits of constructor binding is that the resulting object is immutable. I agree that @DefaultValue can be a little clunky, but I still think it's better than a half mutable class.

I'm also of the opinion that we should think very carefully before we add more complexity to the binder. Especially to the existing code paths. We've already had a lot of back-compatibilty issues and I'm not keen to introduce more. It's also worth noting that constructor bloat is offset quite significantly when a java record is used as you then no longer need to declare all the getters.

Thanks anyway for the suggestion.

[quaff]

For people who are interesting in this, I've created prototype, It's a tiny change, see https://github.com/quaff/spring-boot/tree/patch-71.