Consider the following example:

@ConfigurationProperties("acme")
@ConstructorBinding
public class AcmeProperties {

    private final Security security;

    public AcmeProperties(Security security) {
        this.security = security;
    }

    public Security getSecurity() {
        return this.security;
    }

    public static class Security {

        private final String username;

        private final String password;

        public Security(@DefaultValue("user") String username, String password) {
            this.username = username;
            this.password = password;
        }

        public String getUsername() {
            return this.username;
        }

        public String getPassword() {
            return this.password;
        }
    }
}

If this object is bound and no acme.security.* property is available in the environment, null is provided to the top-level constructor. If a matching property is found, the Security type is initialized and provided to the constructor.

This is consistent with what we do with JavaBean binding. The main difference here is that the object may have default values assigned to the constructor (here a default user). One solution would be to provide an instance no matter what. The alternative is for the user to check if the constructor provided a null instance and then create a default Security.

There are pros and cons for each:

  • If we create a default instance no matter what, we no longer have the signal that the user didn't provide any key for that nested Security object
  • If we don't care about that, we have to check for null and then create an instance with a copy paste of the @DefaultValue we already provided.

A middle-ground would be to make this configurable, perhaps using @Nullable as we do for actuator endpoints.

Comment From: snicoll

@Nullable won't work as that would flip the responsibility. As for always creating the object, it is going to break use cases where the inner class is flagged with @Valid. Users are relying on the fact the instance is set to trigger validation conditionally.

Comment From: OLPMO

Is it a good idea to solve this problem by throwing an exception? @snicoll

Comment From: snicoll

We've decided to always create the object no matter what to promote the fact browsing through the hierarchy is guaranteed.

Comment From: OLPMO

If you do so,how would you solve the problem that lose the signal that the user didn't provide any key for that nested Security object.

Comment From: snicoll

We should probably extend the scope of this issue to other use cases that are similar, i.e:

  • Collection-based types is null rather than empty
  • Optional is null, rather than Optional#empty

Given the team decision, it looks like we should do this as well for consistency.

@philwebb I can see this issue is on hold and I don't remember why that is.

Comment From: philwebb

🤷‍♀ I can't remember either

Comment From: zeratul021

Hello team,

I've just found this problem in my project. I see on-hold tag removed, does this mean this bug will we fixed in next release?

Thanks!

Comment From: snicoll

does this mean this bug will we fixed in next release?

That's not what the label means. On hold means we can't make progress at the moment. We don't quite remember why we added it so now it's back to the "issues to fix in 2.2.x" bucket. There is no guarantee that it will be fixed for the next release though.

Comment From: zeratul021

@snicoll thanks for letting me know. Would you be interested in co-op, like a PR? I'd just need a few pointers, where to start looking.

Comment From: mbhave

@zeratul021 Thanks for the offer but I think it's better if we handle this one. I'd started looking into but we ran into a few issues which is why the issue was put on hold. I'll pick this up again when time permits.

Comment From: zeratul021

@mbhave any update on this?

Comment From: snicoll

There is a @DefaultValue copy in the annotation processor tests that hasn't been reflected. Given that the value is optional now, I suspect something should be done there too. At the very least we should have a test to validate the scenario.

Comment From: snicoll

Turns out everything was ok. The only downside with that approach is that you can write something like this now:

@ConfigurationProperties("test")
public class EmptyDefaultValueProperties {

    @ConstructorBinding
    public EmptyDefaultValueProperties(@DefaultValue String name) {
        this.name = name;
    }
        ....
}

This won't generate a default value at all (no-op).