spring-projects / spring-framework

Spring Framework
https://spring.io/projects/spring-framework
Apache License 2.0
56.14k stars 37.96k forks source link

Support method validation for an interface only proxy like an HTTP interface client #29782

Closed DanielLiu1123 closed 1 year ago

DanielLiu1123 commented 1 year ago

I want to validate the params on client side, it works fine when using spring-cloud-openfeign, but it doesn't work when using HttpExchange.

@Validated
public interface EchoClient {
    @GetExchange("/echo")
    String echo(@RequestParam("message") @NotNull String message);
}
sbrannen commented 1 year ago

Where does that @Validate annotation come from?

Do you perhaps mean Spring's @org.springframework.validation.annotation.Validated?

If so, have you registered an org.springframework.validation.beanvalidation.MethodValidationPostProcessor bean?

DanielLiu1123 commented 1 year ago

Do you perhaps mean Spring's @org.springframework.validation.annotation.Validated?

yep~

If so, have you registered an org.springframework.validation.beanvalidation.MethodValidationPostProcessor bean?

I have spring-boot-starter-validation on classpath, so there is a MethodValidationPostProcessor bean.

I've looked into the proxy bean, there are two advisors, I think the second one is used to method validation, but it doesn't work.

image
sbrannen commented 1 year ago

Thanks for the feedback. We'll look into it.

rstoyanchev commented 1 year ago

The proxy advisors are not in the right order indeed. The one for @HttpExchange doesn't delegate and is meant to be last in such a chain, but is always added first when the proxy is created. I was able to use an AdvisedSupportListener that updates the order whenever there is an advice change, although I'm not sure if there is a better way.

Now MethodValidationInterceptor is getting called but raises an ISE with "Target must not be null". From what I can see it needs the target in order 1) get the @Validated from the class to check for validation groups, and 2) invoke bean validation.

So it looks like method validation via @Validated doesn't work for interface-only proxies, unless I'm missing something, but I'm wondering if it can be made to work? Perhaps by passing the type-level @Validated into the the interceptor so it doesn't have to look it up. For Hibernate Validator, I see it asserts the target is not null, but I'm not sure if it needs much else from it, or whether passing the proxy itself would work. /cc @jhoeller

It's also interesting that it works with spring-cloud-openfeign. @DanielLiu1123, do you get an actual ConstraintViolation with spring-cloud-openfeign since with the above snippet you would also get an IAE from the HttpServiceProxyFactory code for such a required @RequestParameter.

DanielLiu1123 commented 1 year ago

Here's my test example(Spring Boot 3.0.1, Spring Cloud 2022.0.0):

dependencies {
    implementation 'org.springframework.boot:spring-boot-starter-web'
    implementation 'org.springframework.boot:spring-boot-starter-validation'
    implementation 'org.springframework.cloud:spring-cloud-starter-openfeign'
    compileOnly 'org.projectlombok:lombok'
    annotationProcessor 'org.projectlombok:lombok'
}
@SpringBootApplication
@EnableFeignClients
public class App implements ApplicationRunner {
    public static void main(String[] args) {
        SpringApplication.run(App.class, args);
    }

    @FeignClient(name = "post", url = "https://my-json-server.typicode.com")
    @Validated
    interface FeignPostApi {
        @GetMapping("/typicode/demo/posts/{id}")
        Post get(@PathVariable("id") @Min(2) int id);
    }

    @HttpExchange("https://my-json-server.typicode.com")
    @Validated
    interface HttpExchangePostApi {
        @GetExchange("/typicode/demo/posts/{id}")
        Post get(@PathVariable("id") @Min(2) int id);
    }

    @Data
    static class Post {
        public String id;
        public String title;
    }

    @Autowired
    HttpExchangePostApi httpExchangePostApi;
    @Autowired
    FeignPostApi feignPostApi;

    @Override
    public void run(ApplicationArguments args) throws Exception {

        // @Validated not work, can get the response
        System.out.println(httpExchangePostApi.get(1));

        // @Validated works, throw ConstraintViolationException
        System.out.println(feignPostApi.get(1));
    }

    @Bean
    public HttpExchangePostApi postApi(WebClient.Builder builder) {
        return HttpServiceProxyFactory.builder(WebClientAdapter.forClient(builder.build()))
                .build()
                .createClient(HttpExchangePostApi.class);
    }
}

feignPostApi.get(1) throws ConstraintViolationException:

jakarta.validation.ConstraintViolationException: get.id: must be greater than or equal to 2
DanielLiu1123 commented 1 year ago

I personally think the ability to support @Validated on interface (feign, httpExchange) is a very useful feature. It can unify client-side and server-side constraints (server-side implements the interface).

rstoyanchev commented 1 year ago

This has been addressed by improving Spring AOP support for interface-only proxies where there is no target class, and in which case the last AOP advisor does the actual work and should always remain last in the order. So this change should help not only method validation, but also any other AOP advice applied to an HTTP interface client.