search

spring-projects / spring-boot

Spring Boot helps you to create Spring-powered, production-grade applications and services with absolute minimum fuss.
https://spring.io/projects/spring-boot
Apache License 2.0
75.12k stars 40.67k forks source link

Document that validation of @ConfigurationProperties in a native image requires @Valid to cascade validation #37101

Open KimSeongIl opened 1 year ago

KimSeongIl commented 1 year ago

spring boot version: 3.1.0 graalvm plugin version: id("org.graalvm.buildtools.native") version "0.9.20" graalvm version: 22.3.r17

DemoContext.kt

@ConfigurationProperties(prefix = "demo.context")
@Validated
class DemoContext {

    var exclude: List<Exclude> = mutableListOf()

    data class Exclude(
        var url: List<String>? = null,

        var httpMethod: List<HttpMethod>? = null,

        var httpStatus: List<@Pattern(regexp="^([1-5][x|X]{2}|[1-5][0-9]{2})\$") String>? = null,

        var elapsedTime: Long? = null
    )

}

application.yml

demo:
  context:
    exclude:
      - httpStatus: 2xx

nativeRun result

Binding to target [Bindable@517a5ddb type = java.util.List<DemoContext$Exclude>, value = 'provided', annotations = array<Annotation>[[empty]]] failed:

    Property: demo.context.exclude[0].httpstatus
    Value: "2xx"
    Origin: class path resource [application.yml] - 31:21
    Reason: The elements [demo.context.exclude[0].httpstatus] were left unbound.

Action:

Update your application's configuration

It worked fine before, but when I run it on the native image I get an error. However, if I remove the Pattern annotation, it works fine.

Why is this problem occurring? What should I do to resolve this?

sdeleuze commented 1 year ago

I can reproduce, but:

sdeleuze commented 1 year ago

See the related stacktrace with the refined application.yml here.

KimSeongIl commented 1 year ago

@sdeleuze

Thank you for your review.

But that code works fine with jvm not graalvm.

Result when "7xx" is value in jvm.

Binding to target org.springframework.boot.context.properties.bind.BindException: Failed to bind properties under 'demo.context' to DemoContext failed:

    Property: demo.context.exclude[0].httpStatus[0]
    Value: "7xx"
    Reason: "^([1-5][x|X]{2}|[1-5][0-9]{2})$"와 일치해야 합니다

Action:

Update your application's configuration

It doesn't work only in graalvm native image.

Why is this problem occurring?

wilkinsona commented 1 year ago

The validation doesn't work for me on the JVM either:

> Task :bootRun

  .   ____          _            __ _ _
 /\\ / ___'_ __ _ _(_)_ __  __ _ \ \ \ \
( ( )\___ | '_ | '_| | '_ \/ _` | \ \ \ \
 \\/  ___)| |_)| | | | | || (_| |  ) ) ) )
  '  |____| .__|_| |_|_| |_\__, | / / / /
 =========|_|==============|___/=/_/_/_/
 :: Spring Boot ::                (v3.1.0)

2023-06-22T10:54:42.200+01:00  INFO 79694 --- [           main] c.example.gh36006.Gh36006ApplicationKt   : Starting Gh36006ApplicationKt using Java 17.0.5 with PID 79694 (/Users/awilkinson/Downloads/gh-36006/build/classes/kotlin/main started by awilkinson in /Users/awilkinson/Downloads/gh-36006)
2023-06-22T10:54:42.203+01:00  INFO 79694 --- [           main] c.example.gh36006.Gh36006ApplicationKt   : No active profile set, falling back to 1 default profile: "default"
2023-06-22T10:54:42.864+01:00  INFO 79694 --- [           main] c.example.gh36006.Gh36006ApplicationKt   : Started Gh36006ApplicationKt in 1.008 seconds (process running for 1.247)
[Exclude(url=null, httpMethod=null, httpStatus=[boom], elapsedTime=null)]

@KimSeongIl, so that we can be certain that we're looking at exactly the same thing, please provide a complete yet minimal sample that reproduces the behaviour you have described. Guessing dependencies and copy-pasting code from issue comments leaves to much room for unwanted differences.

KimSeongIl commented 1 year ago

@wilkinsona @sdeleuze

https://github.com/KimSeongIl/demo

Created a demo project.

The problem is reproduced in this project.

Could you please check again?

KimSeongIl commented 1 year ago

@wilkinsona

https://github.com/KimSeongIl/demo

in the demo project

    tasks.withType<KotlinCompile> {
        kotlinOptions {
            freeCompilerArgs = listOf("-Xjsr305=strict", "-Xemit-jvm-type-annotations")
            jvmTarget = "11"
        }

-Xemit-jvm-type-annotations

option works fine in jvm environment.

Do I need to use other options in a graalvm environment?

wilkinsona commented 1 year ago

Thanks, @KimSeongIl. I've been able to explore the problem now.

The problem occurs because Spring Boot's configuration property binding cannot identify the constructor to use to create an instance of Exclude. This only happens when running in a native image. It's this code that's behaving differently:

https://github.com/spring-projects/spring-boot/blob/f0e1fdfd11d651f2129023f027d92734570f842f/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/DefaultBindConstructorProvider.java#L202-L208

On the JVM, BeanUtils.findPrimaryConstructor returns public com.demo.search.middle.demo.DemoContext$Exclude(java.util.List,java.util.List,java.util.List,java.lang.Long). In a native image it returns null. BeanUtils is part of Spring Framework but I'm not sure if this is a Spring Framework bug, a Kotlin bug, or even a Graal bug. We'll transfer this issue to the Framework team so that they can continue the investigation.

wilkinsona commented 1 year ago

Digging a little more before transferring the issue shows that the primary constructor is null due to an UnsupportedOperationException that's thrown by Kotlin:

java.lang.UnsupportedOperationException: Type not found: class jakarta.validation.constraints.Pattern$Flag
        at kotlin.reflect.jvm.internal.impl.descriptors.runtime.structure.ReflectJavaClassifierType.getClassifierQualifiedName(ReflectJavaClassifierType.kt:41)
        at kotlin.reflect.jvm.internal.impl.load.java.lazy.types.JavaTypeResolver.createNotFoundClass(JavaTypeResolver.kt:161)
        at kotlin.reflect.jvm.internal.impl.load.java.lazy.types.JavaTypeResolver.computeTypeConstructor(JavaTypeResolver.kt:147)
        at kotlin.reflect.jvm.internal.impl.load.java.lazy.types.JavaTypeResolver.computeSimpleJavaClassifierType(JavaTypeResolver.kt:128)
        at kotlin.reflect.jvm.internal.impl.load.java.lazy.types.JavaTypeResolver.transformJavaClassifierType(JavaTypeResolver.kt:104)
        at kotlin.reflect.jvm.internal.impl.load.java.lazy.types.JavaTypeResolver.transformJavaType(JavaTypeResolver.kt:58)
        at kotlin.reflect.jvm.internal.impl.load.java.lazy.types.JavaTypeResolver.transformArrayType(JavaTypeResolver.kt:82)
        at kotlin.reflect.jvm.internal.impl.load.java.lazy.types.JavaTypeResolver.transformArrayType$default(JavaTypeResolver.kt:67)
        at kotlin.reflect.jvm.internal.impl.load.java.lazy.types.JavaTypeResolver.transformJavaType(JavaTypeResolver.kt:59)
        at kotlin.reflect.jvm.internal.impl.load.java.lazy.descriptors.LazyJavaClassMemberScope.createAnnotationConstructorParameters(LazyJavaClassMemberScope.kt:769)
        at kotlin.reflect.jvm.internal.impl.load.java.lazy.descriptors.LazyJavaClassMemberScope.createDefaultConstructor(LazyJavaClassMemberScope.kt:724)
        at kotlin.reflect.jvm.internal.impl.load.java.lazy.descriptors.LazyJavaClassMemberScope.access$createDefaultConstructor(LazyJavaClassMemberScope.kt:63)
        at kotlin.reflect.jvm.internal.impl.load.java.lazy.descriptors.LazyJavaClassMemberScope$constructors$1.invoke(LazyJavaClassMemberScope.kt:105)
        at kotlin.reflect.jvm.internal.impl.load.java.lazy.descriptors.LazyJavaClassMemberScope$constructors$1.invoke(LazyJavaClassMemberScope.kt:83)
        at kotlin.reflect.jvm.internal.impl.storage.LockBasedStorageManager$LockBasedLazyValue.invoke(LockBasedStorageManager.java:408)
        at kotlin.reflect.jvm.internal.impl.storage.LockBasedStorageManager$LockBasedNotNullLazyValue.invoke(LockBasedStorageManager.java:527)
        at kotlin.reflect.jvm.internal.impl.load.java.lazy.descriptors.LazyJavaClassDescriptor.getConstructors(LazyJavaClassDescriptor.kt:146)
        at kotlin.reflect.jvm.internal.impl.load.java.lazy.descriptors.LazyJavaClassDescriptor.getConstructors(LazyJavaClassDescriptor.kt:42)
        at kotlin.reflect.jvm.internal.impl.serialization.deserialization.AnnotationDeserializer.deserializeAnnotation(AnnotationDeserializer.kt:45)
        at kotlin.reflect.jvm.internal.impl.load.kotlin.BinaryClassAnnotationAndConstantLoaderImpl.loadTypeAnnotation(BinaryClassAnnotationAndConstantLoaderImpl.kt:40)
        at kotlin.reflect.jvm.internal.impl.load.kotlin.BinaryClassAnnotationAndConstantLoaderImpl.loadTypeAnnotation(BinaryClassAnnotationAndConstantLoaderImpl.kt:27)
        at kotlin.reflect.jvm.internal.impl.load.kotlin.AbstractBinaryClassAnnotationLoader.loadTypeAnnotations(AbstractBinaryClassAnnotationLoader.kt:196)
        at kotlin.reflect.jvm.internal.impl.serialization.deserialization.TypeDeserializer$simpleType$annotations$1.invoke(TypeDeserializer.kt:97)
        at kotlin.reflect.jvm.internal.impl.serialization.deserialization.TypeDeserializer$simpleType$annotations$1.invoke(TypeDeserializer.kt:96)
        at kotlin.reflect.jvm.internal.impl.storage.LockBasedStorageManager$LockBasedLazyValue.invoke(LockBasedStorageManager.java:408)
        at kotlin.reflect.jvm.internal.impl.storage.LockBasedStorageManager$LockBasedNotNullLazyValue.invoke(LockBasedStorageManager.java:527)
        at kotlin.reflect.jvm.internal.impl.storage.StorageKt.getValue(storage.kt:42)
        at kotlin.reflect.jvm.internal.impl.serialization.deserialization.descriptors.DeserializedAnnotations.getAnnotations(DeserializedAnnotations.kt:28)
        at kotlin.reflect.jvm.internal.impl.serialization.deserialization.descriptors.DeserializedAnnotations.isEmpty(DeserializedAnnotations.kt:30)
        at kotlin.reflect.jvm.internal.impl.types.DefaultTypeAttributeTranslator.toAttributes(TypeAttributeTranslator.kt:28)
        at kotlin.reflect.jvm.internal.impl.serialization.deserialization.TypeDeserializer.toAttributes(TypeDeserializer.kt:77)
        at kotlin.reflect.jvm.internal.impl.serialization.deserialization.TypeDeserializer.simpleType(TypeDeserializer.kt:100)
        at kotlin.reflect.jvm.internal.impl.serialization.deserialization.TypeDeserializer.type(TypeDeserializer.kt:68)
        at kotlin.reflect.jvm.internal.impl.serialization.deserialization.TypeDeserializer.typeArgument(TypeDeserializer.kt:300)
        at kotlin.reflect.jvm.internal.impl.serialization.deserialization.TypeDeserializer.simpleType(TypeDeserializer.kt:106)
        at kotlin.reflect.jvm.internal.impl.serialization.deserialization.TypeDeserializer.type(TypeDeserializer.kt:68)
        at kotlin.reflect.jvm.internal.impl.serialization.deserialization.MemberDeserializer.valueParameters(MemberDeserializer.kt:340)
        at kotlin.reflect.jvm.internal.impl.serialization.deserialization.MemberDeserializer.loadConstructor(MemberDeserializer.kt:276)
        at kotlin.reflect.jvm.internal.impl.serialization.deserialization.descriptors.DeserializedClassDescriptor.computePrimaryConstructor(DeserializedClassDescriptor.kt:137)
        at kotlin.reflect.jvm.internal.impl.serialization.deserialization.descriptors.DeserializedClassDescriptor.access$computePrimaryConstructor(DeserializedClassDescriptor.kt:34)
        at kotlin.reflect.jvm.internal.impl.serialization.deserialization.descriptors.DeserializedClassDescriptor$primaryConstructor$1.invoke(DeserializedClassDescriptor.kt:75)
        at kotlin.reflect.jvm.internal.impl.serialization.deserialization.descriptors.DeserializedClassDescriptor$primaryConstructor$1.invoke(DeserializedClassDescriptor.kt:75)
        at kotlin.reflect.jvm.internal.impl.storage.LockBasedStorageManager$LockBasedLazyValue.invoke(LockBasedStorageManager.java:408)
        at kotlin.reflect.jvm.internal.impl.serialization.deserialization.descriptors.DeserializedClassDescriptor.getUnsubstitutedPrimaryConstructor(DeserializedClassDescriptor.kt:141)
        at kotlin.reflect.jvm.internal.impl.serialization.deserialization.descriptors.DeserializedClassDescriptor.computeConstructors(DeserializedClassDescriptor.kt:144)
        at kotlin.reflect.jvm.internal.impl.serialization.deserialization.descriptors.DeserializedClassDescriptor.access$computeConstructors(DeserializedClassDescriptor.kt:34)
        at kotlin.reflect.jvm.internal.impl.serialization.deserialization.descriptors.DeserializedClassDescriptor$constructors$1.invoke(DeserializedClassDescriptor.kt:76)
        at kotlin.reflect.jvm.internal.impl.serialization.deserialization.descriptors.DeserializedClassDescriptor$constructors$1.invoke(DeserializedClassDescriptor.kt:76)
        at kotlin.reflect.jvm.internal.impl.storage.LockBasedStorageManager$LockBasedLazyValue.invoke(LockBasedStorageManager.java:408)
        at kotlin.reflect.jvm.internal.impl.storage.LockBasedStorageManager$LockBasedNotNullLazyValue.invoke(LockBasedStorageManager.java:527)
        at kotlin.reflect.jvm.internal.impl.serialization.deserialization.descriptors.DeserializedClassDescriptor.getConstructors(DeserializedClassDescriptor.kt:152)
        at kotlin.reflect.jvm.internal.KClassImpl.getConstructorDescriptors(KClassImpl.kt:203)
        at kotlin.reflect.jvm.internal.KClassImpl$Data$constructors$2.invoke(KClassImpl.kt:94)
        at kotlin.reflect.jvm.internal.KClassImpl$Data$constructors$2.invoke(KClassImpl.kt:93)
        at kotlin.reflect.jvm.internal.ReflectProperties$LazySoftVal.invoke(ReflectProperties.java:93)
        at kotlin.reflect.jvm.internal.ReflectProperties$Val.getValue(ReflectProperties.java:32)
        at kotlin.reflect.jvm.internal.KClassImpl$Data.getConstructors(KClassImpl.kt:93)
        at kotlin.reflect.jvm.internal.KClassImpl.getConstructors(KClassImpl.kt:238)
        at kotlin.reflect.full.KClasses.getPrimaryConstructor(KClasses.kt:36)
        at org.springframework.beans.BeanUtils$KotlinDelegate.findPrimaryConstructor(BeanUtils.java:853)

This is looking more like a GraalVM bug to me as it appears to have failed to include jakarta.validation.constraints.Pattern$Flag in the native image, despite the use of @Pattern and Pattern$Flag being the type of its flags() attribute.

wilkinsona commented 1 year ago

it appears to have failed to include jakarta.validation.constraints.Pattern$Flag in the native image

That's not the problem. Changing the main function to reference Pattern$Flag does not help:

fun main(args: Array<String>) {
    println(jakarta.validation.constraints.Pattern.Flag::class)
    runApplication<DemooApplication>(*args)
}

This outputs class jakarta.validation.constraints.Pattern$Flag on startup and still fails with the same UnsupportedOperationException.

Perhaps BeanValidationBeanRegistrationAotProcessor needs to have generated some reflection hints for Bean Validation annotations so that KClasses.getPrimaryConstructor still works for constructors that use those annotations on their parameters?

We'll transfer this to Framework to get some of @sdeleuze's Kotlin expertise.

sdeleuze commented 1 year ago

After a deeper look, it looks like a GraalVM bug to me. I am not sure println(jakarta.validation.constraints.Pattern.Flag::class) is supposed to change something from a GraalVM reflection POV, but adding the hint fixes the issue. We could workaround in BeanValidationBeanRegistrationAotProcessor but that would be much more involved that current algorythm and would basically implement the logic that GraalVM should implement OOTB.

GraalVM is supposed to add the related annotation reflection config transitively based on com.demo.search.middle.demo.DemoContext hints, so @KimSeongIl please create an issue on https://github.com/oracle/graal/issues with:

We will see the feedback from GraalVM team.

sdeleuze commented 1 year ago

@snicoll @bclozel @mhalbritter @wilkinsona It looks like we have been going full circle on this one. GraalVM confirmed this need to be fixed on Spring side, and after a another look I agree.

I had a deeper look on Spring Boot side while debugging, and I think I now understand why BeanValidationBeanRegistrationAotProcessor detects no relevant hint. It does not because it has no knowledge of org.springframework.boot.context.properties.bind.AggregateBinder logic which is able to analyze DemoContext List<Exclude> property, to invoke Hibernate validator on individual Exclude element.

As a consequence, I think this issue should be bring back to Spring Boot side where advanced validation handling like AggregateBinder should have related AOT processor to anticipate what will be needed by Hibernate validator.

Notice that a logic similar to BeanValidationBeanRegistrationAotProcessor will probably be needed, the key point being to process for the repro use case Exclude, not DemoContext.

If some refinements are needed on Spring Framework side like extracting BeanValidationBeanRegistrationAotProcessor logic to be able to apply it on any Class<?> not only RegisteredBean, feel free to create dedicated issue and I will take care of it.

wilkinsona commented 13 hours ago

It does not because it has no knowledge of org.springframework.boot.context.properties.bind.AggregateBinder logic which is able to analyze DemoContext List<Exclude> property, to invoke Hibernate validator on individual Exclude element

This validation of Exclude without @Valid is a mistake in the binder that we've corrected in 3.4.

From Spring Boot's perspective, this should work in a native image if @Valid is added to the declaration of the exclude field as this should be enough for AOT processing to cascade the search for constraints down into Exclude as Hibernate Validator itself does. However, it doesn't work as BeanValidationBeanRegistrationAotProcessor doesn't consider cascaded constraints or container element constraints. I've opened https://github.com/spring-projects/spring-framework/issues/33842 so that this can be addressed in Framework.

Once a fix has been made in Framework, we should be able to fix this by documenting in this section that @Valid is also needed in a native image.