Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bean validation doesn't work on Kotlin coroutines controller parameters #23499

Closed
yuki-teraoka opened this issue Aug 22, 2019 · 27 comments
Closed
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) theme: kotlin An issue related to Kotlin support type: bug A general bug
Milestone

Comments

@yuki-teraoka
Copy link

Affects: 5.2.0.RC1

when using Kotlin suspend function and Validated annotation, like this.

package example

import org.springframework.http.HttpStatus
import org.springframework.http.ResponseEntity
import org.springframework.validation.annotation.Validated
import org.springframework.web.bind.annotation.RequestMapping
import org.springframework.web.bind.annotation.RestController

@Validated
@RestController
class Foo {
    @RequestMapping("/foo")
    suspend fun foo() = ResponseEntity("foo", HttpStatus.OK)
}

occured exception.

java.lang.ArrayIndexOutOfBoundsException: 0
        at java.util.Arrays$ArrayList.get(Arrays.java:3841) ~[na:1.8.0_102]
        Suppressed: reactor.core.publisher.FluxOnAssembly$OnAssemblyException:
Error has been observed at the following site(s):
        |_ checkpoint ⇢ Handler example.Foo#foo(Continuation) [DispatcherHandler]
        |_ checkpoint ⇢ org.springframework.security.web.server.authorization.AuthorizationWebFilter [DefaultWebFilterChain]
        |_ checkpoint ⇢ org.springframework.security.web.server.authorization.ExceptionTranslationWebFilter [DefaultWebFilterChain]
        |_ checkpoint ⇢ org.springframework.security.web.server.authentication.logout.LogoutWebFilter [DefaultWebFilterChain]
        |_ checkpoint ⇢ org.springframework.security.web.server.savedrequest.ServerRequestCacheWebFilter [DefaultWebFilterChain]
        |_ checkpoint ⇢ org.springframework.security.web.server.context.SecurityContextServerWebExchangeWebFilter [DefaultWebFilterChain]
        |_ checkpoint ⇢ org.springframework.security.web.server.context.ReactorContextWebFilter [DefaultWebFilterChain]
        |_ checkpoint ⇢ org.springframework.security.web.server.header.HttpHeaderWriterWebFilter [DefaultWebFilterChain]
        |_ checkpoint ⇢ org.springframework.security.config.web.server.ServerHttpSecurity$ServerWebExchangeReactorContextWebFilter [DefaultWebFilterChain]
        |_ checkpoint ⇢ org.springframework.security.web.server.WebFilterChainProxy [DefaultWebFilterChain]
        |_ checkpoint ⇢ org.springframework.cloud.sleuth.instrument.web.TraceWebFilter [DefaultWebFilterChain]
        |_ checkpoint ⇢ org.springframework.boot.actuate.metrics.web.reactive.server.MetricsWebFilter [DefaultWebFilterChain]
        |_ checkpoint ⇢ HTTP GET "/foo" [ExceptionHandlingWebHandler]
Stack trace:
                at java.util.Arrays$ArrayList.get(Arrays.java:3841) ~[na:1.8.0_102]
                at org.hibernate.validator.internal.metadata.aggregated.ParameterMetaData$Builder.build(ParameterMetaData.java:169) ~[hibernate-validator-6.0.17.Final.jar:6.0.17.Final]
                at org.hibern ate.validator.internal.metadata.aggregated.ExecutableMetaData$Builder.findParameterMetaData(ExecutableMetaData.java:435) ~[hibernate-validator-6.0.17.Final.jar:6.0.17.Final]
                at org.hibernate.validator.internal.metadata.aggregated.ExecutableMetaData$Builder.build(ExecutableMetaData.java:388) ~[hibernate-validator-6.0.17.Final.jar:6.0.17.Final]
                at org.hibernate.validator.internal.metadata.aggregated.BeanMetaDataImpl$BuilderDelegate.build(BeanMetaDataImpl.java:788) ~[hibernate-validator-6.0.17.Final.jar:6.0.17.Final]
                at org.hibernate.validator.internal.metadata.aggregated.BeanMetaDataImpl$BeanMetaDataBuilder.build(BeanMetaDataImpl.java:648) ~[hibernate-validator-6.0.17.Final.jar:6.0.17.Final]
                at org.hibernate.validator.internal.metadata.BeanMetaDataManager.createBeanMetaData(BeanMetaDataManager.java:204) ~[hibernate-validator-6.0.17.Final.jar:6.0.17.Final]
                at org.hibernate.validator.internal.metadata.BeanMetaDataManager.getBeanMetaData(BeanMetaDataManager.java:166) ~[hibernate-validator-6.0.17.Final.jar:6.0.17.Final]
                at org.hibernate.validator.internal.engine.ValidatorImpl.validateParameters(ValidatorImpl.java:265) ~[hibernate-validator-6.0.17.Final.jar:6.0.17.Final]
                at org.hibernate.validator.internal.engine.ValidatorImpl.validateParameters(ValidatorImpl.java:233) ~[hibernate-validator-6.0.17.Final.jar:6.0.17.Final]
                at org.springframework.validation.beanvalidation.MethodValidationInterceptor.invoke(MethodValidationInterceptor.java:104) ~[spring-context-5.2.0.RC1.jar:5.2.0.RC1]
                at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:186) ~[spring-aop-5.2.0.RC1.jar:5.2.0.RC1]
                at org.springframework.aop.framework.CglibAopProxy$DynamicAdvisedInterceptor.intercept(CglibAopProxy.java:689) ~[spring-aop-5.2.0.RC1.jar:5.2.0.RC1]
...
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Aug 22, 2019
@sdeleuze sdeleuze self-assigned this Aug 22, 2019
@sdeleuze sdeleuze added in: web Issues in web modules (web, webmvc, webflux, websocket) type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Aug 22, 2019
@sdeleuze sdeleuze added this to the 5.2 RC2 milestone Aug 22, 2019
@sdeleuze
Copy link
Contributor

@Validated is indeed not yet Coroutines compliant, we need to fix that by using Coroutines aware methods to discover method parameters.

@sdeleuze sdeleuze modified the milestones: 5.2 RC2, 5.2 GA Sep 3, 2019
@sdeleuze
Copy link
Contributor

I am not entirely sure what we should do here. The issue comes from MethodValidationInterceptor which is obviously not Coroutines aware, but JSR-303 is designed to deal with Java reflection API not Kotlin one. We could maybe pass a fake parameter value for the Continuation one, but there are other issues like how to support annotation on return values, the fact that WebMvc is not supported yet, etc.

Given those uncertainties, it seems more reasonable to postpone this issue for 5.3.

@kostya05983
Copy link

Hello, @sdeleuze , any updates? Today, I think about another annotation something like KValidated as temporary solution, For example you can add new annotation KValidated which should be used only in kotlin. And after add KMethodValidationInterceptor with similary behaviour as MethodValidationInterceptor, what do you think about this? And can you describe the problem with solution please?. Why we can't just use CoroutinesUtils.invokeSuspendingFunction(method, getBean(), args); as it is used in InvocableHandlerMethod?

@sdeleuze
Copy link
Contributor

sdeleuze commented Oct 26, 2020

@kostya05983 Even after adding support for Coroutines via CoroutinesUtils.invokeSuspendingFunction there is an error at Hibernate Validator level, so I suggest you or somebody raises an issue on Hibernate Validator side for such support. See the related stack trace bellow:

at java.base/java.util.Arrays$ArrayList.get(Arrays.java:4351)
at org.hibernate.validator.internal.properties.javabean.JavaBeanExecutable.getParameterName(JavaBeanExecutable.java:86)
at org.hibernate.validator.internal.metadata.aggregated.ParameterMetaData$Builder.build(ParameterMetaData.java:165)
at org.hibernate.validator.internal.metadata.aggregated.ExecutableMetaData$Builder.findParameterMetaData(ExecutableMetaData.java:436)
at org.hibernate.validator.internal.metadata.aggregated.ExecutableMetaData$Builder.build(ExecutableMetaData.java:391)
at org.hibernate.validator.internal.metadata.aggregated.BeanMetaDataBuilder$BuilderDelegate.build(BeanMetaDataBuilder.java:260)
at org.hibernate.validator.internal.metadata.aggregated.BeanMetaDataBuilder.build(BeanMetaDataBuilder.java:133)
at org.hibernate.validator.internal.metadata.BeanMetaDataManagerImpl.createBeanMetaData(BeanMetaDataManagerImpl.java:206)
at org.hibernate.validator.internal.metadata.BeanMetaDataManagerImpl.getBeanMetaData(BeanMetaDataManagerImpl.java:165)
at org.hibernate.validator.internal.engine.ValidatorImpl.validateParameters(ValidatorImpl.java:267)
at org.hibernate.validator.internal.engine.ValidatorImpl.validateParameters(ValidatorImpl.java:235)
at org.springframework.validation.beanvalidation.MethodValidationInterceptor.invoke(MethodValidationInterceptor.java:110)
at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:186)
at org.springframework.aop.framework.CglibAopProxy$CglibMethodInvocation.proceed(CglibAopProxy.java:749)
at org.springframework.aop.framework.CglibAopProxy$DynamicAdvisedInterceptor.intercept(CglibAopProxy.java:691)
at com.example.FooCoroutinesController$$EnhancerBySpringCGLIB$$1b76b143.find(<generated>)

#22462 fix should be all what is needed on Spring side.

@sdeleuze sdeleuze modified the milestones: 5.3 GA, 5.x Backlog Oct 26, 2020
@sdeleuze sdeleuze added the status: blocked An issue that's blocked on an external project change label Oct 26, 2020
@sdeleuze sdeleuze changed the title ArrayIndexOutOfBoundsException occurred when using Kotlin suspend function and Validated annotation Bean validation doesn't work with Kotlin coroutines controller Apr 7, 2021
@pschichtel
Copy link

For reference, this is the hibernate issue: https://hibernate.atlassian.net/browse/HV-1638

sdeleuze added a commit to sdeleuze/spring-framework that referenced this issue Jan 18, 2023
Removing the usage of KotlinReflectionParameterNameDiscoverer in
LocalValidatorFactoryBean and just delegate to Hibernate Validator
fixed the use case with suspending functions.

Closes spring-projectsgh-23499
@sdeleuze
Copy link
Contributor

Based on a quick test, it seems just removing the usage of KotlinReflectionParameterNameDiscoverer in LocalValidatorFactoryBean combined with the changes done in #29566 fix parameter validation with Coroutines.

Could people interested in getting that fix test my changes on sdeleuze@gh-23499 and provide a feedback here?

@sdeleuze sdeleuze added the status: waiting-for-feedback We need additional information before we can continue label Jan 18, 2023
@cjdxhjj
Copy link

cjdxhjj commented Jan 18, 2023

i don't think hibernate validation will compatible with the kotlin suspend function.
i have read some replay from hibernate validation

https://hibernate.atlassian.net/browse/HV-1638
https://hibernate.atlassian.net/browse/HV-1796

@cjdxhjj
Copy link

cjdxhjj commented Jan 18, 2023

@sdeleuze image
it may be solved by jetbrain remove that parameter

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Jan 18, 2023
@sdeleuze
Copy link
Contributor

On Spring side, I have the feeling that KotlinReflectionParameterNameDiscoverer in LocalValidatorFactoryBean does more harm than good. Coroutines use case seems totally broken with it, works for typical use case (unless I get different feedback but that's what my quick test shown).

Some Kotlin use cases may still be broken until Kotlin team move forward on https://youtrack.jetbrains.com/issue/KT-40857, but I could ask them to move forward on that issue.

Could you please test my branch and let me know how it goes for typical use case?

@cjdxhjj
Copy link

cjdxhjj commented Jan 18, 2023

i will have a test

@sdeleuze sdeleuze added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Jan 19, 2023
@sdeleuze
Copy link
Contributor

@cjdxhjj Any chance you could test and provide a feedback?

@cjdxhjj
Copy link

cjdxhjj commented Jan 24, 2023

@sdeleuze i'm sorry for the slow response, i'm on holidy, i would try it as soon as possible.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Jan 24, 2023
@cjdxhjj
Copy link

cjdxhjj commented Jan 25, 2023

@sdeleuze i have just do a simple test with
image
image
image
that works.

@sdeleuze sdeleuze removed the status: feedback-provided Feedback has been provided label Jan 31, 2023
@sdeleuze
Copy link
Contributor

Bean validation on suspending function parameters should be fixed as of Spring Framework 6.0.5, I don't think the fix is doable easily on 5.3.x so I won't backport it. Validation of suspending function return values remains unsupported as Hibernate Validator is not Coroutines aware, but I think parameter validation was the most critical need.

@sdeleuze sdeleuze changed the title Bean validation doesn't work with Kotlin coroutines controller Bean validation doesn't work on Kotlin coroutines controller parameters Jan 31, 2023
@cjdxhjj
Copy link

cjdxhjj commented Jan 31, 2023

@sdeleuze thanks a lot

@Michal-Kucera
Copy link

Hello! I'm not sure this is a related issue, though, let's see if we should open a new ticket.

After upgrading from Spring Boot 3.0.2 to 3.0.4 validations on GraphQL controller parameters work as expected, but I'm now having a problem with custom ConstraintValidators which are not triggered at all.

Please see a sample code:

@Controller
@Validated
class CompanyController {

    @PreAuthorize("hasRole('ROLE_COMPANY_EDIT')")
    @MutationMapping("createCompany")
    suspend fun createCompany(
        @Argument("input")
        @Valid
        companyInput: CompanyInput
    ): CompanyResponse {
        // TODO..
    }
}

@CompanyNameUniquePerWhitelabelId
data class CompanyInput(
    val companyType: CompanyType,
    val name: String,
    @IsIso2Country val registeredAddressCountryCode: Int
)

@MustBeDocumented
@Constraint(validatedBy = [CompanyNameUniquePerWhitelabelIdValidator::class])
@Target(AnnotationTarget.CLASS)
@Retention(AnnotationRetention.RUNTIME)
annotation class CompanyNameUniquePerWhitelabelId(
    val message: String = "This company name already exists for Provided whitelabel.",
    val groups: Array<KClass<*>> = [],
    val payload: Array<KClass<out Any>> = []
)

class CompanyNameUniquePerWhitelabelIdValidator : ConstraintValidator<CompanyNameUniquePerWhitelabelId, CompanyInput> {

    override fun isValid(companyInput: CompanyInput?, context: ConstraintValidatorContext?): Boolean {
        // TODO
    }
}

@MustBeDocumented
@Constraint(validatedBy = [Iso2CountryValidator::class])
@Target(AnnotationTarget.FIELD)
@Retention(AnnotationRetention.RUNTIME)
annotation class IsIso2Country(
    val message: String = "Wrong country code",
    val groups: Array<KClass<*>> = [],
    val payload: Array<KClass<out Any>> = []
)

class Iso2CountryValidator : ConstraintValidator<IsIso2Country, Int?> {
    override fun isValid(value: Int?, context: ConstraintValidatorContext?): Boolean {
        // TODO
    }
}

In my case, none of Iso2CountryValidator#isValid and CompanyNameUniquePerWhitelabelIdValidator#isValid is triggered.

Any thoughts?

Thanks!
Michal

@koenpunt
Copy link
Contributor

koenpunt commented Mar 8, 2023

@Michal-Kucera this might be related to this issue: spring-projects/spring-graphql#624 which was fixed in this commit spring-projects/spring-graphql@581b110, and will be released with Spring GraphQL 1.1.3.

@Michal-Kucera
Copy link

Thanks for your fast response @koenpunt! Next time I should perhaps first look into graphql issues :)

@rishiraj88
Copy link

Nice issue request. Thanks, @Michal-Kucera

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) theme: kotlin An issue related to Kotlin support type: bug A general bug
Projects
None yet
Development

No branches or pull requests