Nested object is not initialized if no matching property is defined with constructor binding

[snicoll]

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.

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

[OLPMO]

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

[snicoll]

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

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

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

[philwebb]

🤷‍♀ I can't remember either

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

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

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

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

[zeratul021]

@mbhave any update on this?

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

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