Align cascade behavior of @Validated @ConfigurationProperties with the bean validation spec

[LeMikaelF]

When a JSR 303 implementation is on the classpath, Spring Boot validates all Configuration Properties classes that are annotated with @Validated (see ConfigurationPropertiesBinder::getValidators). But the validation applies to all nested objects, even if they're not annotated with @Valid. This doesn't comply with the JSR 303 spec (see the notion of traversable properties) and can lead to some surprising results. For example, the bean below will fail validation given a single property props.nested.myprop1=value, even though the field private Nested nested isn't annotated with @Valid. Likewise, nested collections and maps will fail validation if they contain objects that are themselves annotated with jsr 303 annotations (ex: @NotEmpty), even if validation isn't cascaded with an @Valid. See this reproducer.

@Data
@Validated
@ConfigurationProperties(prefix = "props")
static class ConfigPropsClass {

    private Nested nested;

    @Data
    static class Nested {

        String myprop1;

        @NotEmpty String myprop2;

    }

}

The documentation is also somewhat confusing for this. It states:

To ensure that validation is always triggered for nested properties, even when no properties are found, the associated field must be annotated with @Valid.

But nested objects will not be validated if there are no properties to bind. For example, adding @Valid to the private Nested nested field above will not result in a failure if there is no props.nested.myprop1 property.

I can think of two solutions:

1. Propagate validation to @Valid fields only.
2. Modify the documentation to explicitly say that all nested objects will be validated if the configuration properties class is @Validated.

I personally prefer option 1, because it corresponds to behaviour that is expected from a JSR 303 validator.

[mhalbritter]

I think it's a bug that we propagate validation to fields which aren't annotated with @Valid.

[LeMikaelF]

@mhalbritter I'd be happy to contribute a fix for this, if you accept contributions and it's not too time-sensitive (in the next month).

[mhalbritter]

Thanks for the offer @LeMikaelF, but we first want to understand all the different ways how this works. Configuration property binding is a beast, with multiple ways how fields are bound to configuration properties (setters, constructor injection, etc.). We're also not sure in which version we should fix this, as the current behavior is there for a long time and this may fail users in a subtle way.

[wilkinsona]

But nested objects will not be validated if there are no properties to bind. For example, adding @Valid to the private Nested nested field above will not result in a failure if there is no props.nested.myprop1 property.

This is to be expected. With no properties to bind to a Nested instance, one isn't created. As a result the nested field is null so there's nothing to validate. It will fail if the nested field is annotated with @NotNull. The same goes for plain JSR 303 validation where if nested is annotated with just @Valid and its value is null, no validation failure will occur.

Where a difference can be observed is when a property is bound to nested but it isn't annotated with @Valid. In that case, configuration property binding fails due to a constraint violation but plain JSR 303 validation of a similar object graph does not.

I've verified the current behavior back to 2.0 and things appear to have behaved as they do now since then. Given this, I think it's quite hard to justify changing this. I'm leaning towards documenting the current behavior more precisely.

[wilkinsona]

I'm now not so sure about just documenting this as I've noticed another problem with how we use @Valid combined with the binder's bottom-up approach and JSR 303's top-down approach to validation.

When the binder encounters a constraint violation, it stops validating. This means that if you have constraints at different levels of the hierarchy, only the lowest down violations will be reported. Consider this example:

package com.example.gh_40345;

import org.springframework.boot.SpringApplication;
import org.springframework.boot.autoconfigure.SpringBootApplication;
import org.springframework.boot.context.properties.ConfigurationProperties;
import org.springframework.boot.context.properties.EnableConfigurationProperties;
import org.springframework.validation.annotation.Validated;

import com.example.gh_40345.Gh40345Application.SomeProperties;

import jakarta.validation.Valid;
import jakarta.validation.constraints.NotNull;

@EnableConfigurationProperties(SomeProperties.class)
@SpringBootApplication
public class Gh40345Application {

	public static void main(String[] args) {
		SpringApplication.run(Gh40345Application.class, args);
	}
	
	@Validated
	@ConfigurationProperties("some")
	static class SomeProperties {
		
		@Valid
		private final Group group = new Group();
		
		@NotNull
		private String propertyOne;
		
		public Group getGroup() {
			return group;
		}

		public String getPropertyOne() {
			return propertyOne;
		}

		public void setPropertyOne(String propertyOne) {
			this.propertyOne = propertyOne;
		}

		static class Group {
			
			@NotNull
			private String propertyTwo;
			
			private String propertyThree;

			public String getPropertyTwo() {
				return propertyTwo;
			}

			public void setPropertyTwo(String propertyTwo) {
				this.propertyTwo = propertyTwo;
			}

			public String getPropertyThree() {
				return propertyThree;
			}

			public void setPropertyThree(String propertyThree) {
				this.propertyThree = propertyThree;
			}

		}
		
	}

}

If we run it with some.group.property-three=value, it's reasonable to expect violations for some.propertyOne and some.group.propertyTwo as both should not be null. However, only a single violation is reported:

***************************
APPLICATION FAILED TO START
***************************

Description:

Binding to target com.example.gh_40345.Gh40345Application$SomeProperties failed:

    Property: some.group.propertyTwo
    Value: "null"
    Reason: must not be null


Action:

Update your application's configuration

A constraint is missing as the binder validates the Group instance, a constraint violation is detected, and it then creates and stores the exception that will report the problem:

spring-boot/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/validation/ValidationBindHandler.java

Lines 140 to 142 in 84956ad

	if (result != null && result.hasErrors()) {
	this.exception = new BindValidationException(result.getValidationErrors());
	}

This prevents any further validation and the exception is thrown once we've reached the top of the hierarchy:

spring-boot/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/validation/ValidationBindHandler.java

Lines 109 to 120 in 84956ad

	private void validate(ConfigurationPropertyName name, Bindable<?> target, BindContext context, Object result) {
	if (this.exception == null) {
	Object validationTarget = getValidationTarget(target, context, result);
	Class<?> validationType = target.getBoxedType().resolve();
	if (validationTarget != null) {
	validateAndPush(name, validationTarget, validationType);
	}
	}
	if (context.getDepth() == 0 && this.exception != null) {
	throw this.exception;
	}
	}

If we run it without any properties, we get both of the violations that we expected:

***************************
APPLICATION FAILED TO START
***************************

Description:

Binding to target com.example.gh_40345.Gh40345Application$SomeProperties failed:

    Property: some.propertyOne
    Value: "null"
    Reason: must not be null

    Property: some.group.propertyTwo
    Value: "null"
    Reason: must not be null


Action:

Update your application's configuration

In this case, we've received both violations as they've been identified when validating the SomeProperties instance where the problem with some.propertyOne is detected. Due to @Valid on the group field, the problem with some.group.propertyTwo is also detected as JSR 303 propagates the validation downwards.

If we changed ValidationBindHandler to continue validating and combine all of the errors that it encounters, we would no longer miss any constraints violations. Unfortunately, this would also result in some double-validation as the Group instance would be validated once by the binder on the way up and again on the way back down due to JSR 303's propagation and the presence of @Valid on the group field of SomeProperties. In fact, in situations where there are no constraint violations, this double validation is already occurring which, while benign in terms of functionality, is inefficient in terms of performance.

[philwebb]

We discussed this today and we consider this a bug, but one that is too risky to attempt to fix in a patch release. We're going to try and take a look for the next minor.