spring-boot icon indicating copy to clipboard operation
spring-boot copied to clipboard

Validation is not applied for ConfigurationProperties that implement Validator and use @ConstructorBinding

Open fstaudt opened this issue 2 years ago • 3 comments

In this article (https://reflectoring.io/validate-spring-boot-configuration-parameters-at-startup/ ), it is advised to implement Validator interface in class annotated with @ConfigurationProperties for complex checks.

This mechanism works well when class is not annotated with @ConstructorBinding.
It doesn't work if class is annotated with @ConstructorBinding or if class is implemented as an immutable class in Spring Boot 3.0.0.

I've reproduced the issue in a Java project on this github repository for Spring Boot 2.7.7 (branch sb27) and for Spring Boot 3.0.0 (branch master).

In test SimpleDocumentationPropertiesTest, validate method of class SimpleDocumentationProperties is well called.
This class is a simple POJO with @ConfigurationProperties annotation.

In test ConstructorBindingDocumentationPropertiesTest, test fails because validate method of class ConstructorBindingDocumentationProperties is not called. This class is implemented as an immutable class with same @ConfigurationProperties annotation and additional @ConstructorBinding annotation (for Spring Boot 2).

Issue can also be reproduced in Kotlin projects when ConfigurationProperties classes are implemented with data classes.

I'd like to use immutable classes for all my configuration properties classes and still be able to use Validator interface for complex checks.

fstaudt avatar Jan 03 '23 08:01 fstaudt

First analysis points out to these lines: https://github.com/spring-projects/spring-boot/blob/main/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBinder.java#L146-L148

I believe following change would fix the issue (to be tested):

  if (Validator.class.isAssignableFrom(target.type.getRawClass())) {
    if (target.getValue() != null) {
      validators.add((Validator) target.getValue().get());
    } else {
      validators.add((Validator) getBinder().bindOrCreate("validator", target));
    }
  }

fstaudt avatar Jan 03 '23 11:01 fstaudt

I think this is a bug, but for what it's worth I don't think it makes as much sense to use Validator with constructor binding.

For setter based binding, it's useful to have the validate callback that is called once all the setters have been called. For constructor binding, you should already have all the data required for validation in the constructor parameters. I would do validation in the constructor and throw an exception there to prevent the object from being created in an invalid state.

philwebb avatar Jan 18 '23 20:01 philwebb

Validate callback also eases aggregation of multiple validation errors in one response for a given properties class.
The same benefit can be achieved with BeanPropertyBindingResult and BindException with some additional boiler plate.

Validate callback also provides out-of-the-box a clear error message in application logs:

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

Description:

Binding to target org.springframework.boot.context.properties.bind.BindException: Failed to bind properties under 'documentation' to com.example.cbcpvalidator.SimpleDocumentationProperties failed:

    Property: documentation.url
    Value: "http://docs.spring.io/spring-boot/docs/current/reference/htmlsingle/"
    Origin: class path resource [application.yml] - 2:8
    Reason: Documentation URL must start with https://.


Action:

Update your application's configuration

I was not able to get this clear error message when throwing explicitly a BindException.

In this context, it would still make sense to use Validator with constructor binding.
Would it be OK if I provide a PR to fix this bug ?

fstaudt avatar Jan 23 '23 11:01 fstaudt