From 8af9648c434ac99d60a7fdde4fc2c5e0aff6e4bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Deleuze?= Date: Fri, 18 Aug 2023 11:40:25 +0200 Subject: [PATCH 1/2] Polish ModelAttributeMethodArgumentResolverTests --- ...lAttributeMethodArgumentResolverTests.java | 52 ++++++++----------- 1 file changed, 23 insertions(+), 29 deletions(-) diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/ModelAttributeMethodArgumentResolverTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/ModelAttributeMethodArgumentResolverTests.java index 0d6f771942..d54b842a4f 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/ModelAttributeMethodArgumentResolverTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/ModelAttributeMethodArgumentResolverTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,7 +16,6 @@ package org.springframework.web.reactive.result.method.annotation; -import java.net.URISyntaxException; import java.time.Duration; import java.util.Map; import java.util.function.Function; @@ -112,7 +111,7 @@ class ModelAttributeMethodArgumentResolverTests { } @Test - void createAndBind() throws Exception { + void createAndBind() { testBindPojo("pojo", this.testMethod.annotPresent(ModelAttribute.class).arg(Pojo.class), value -> { assertThat(value.getClass()).isEqualTo(Pojo.class); return (Pojo) value; @@ -120,7 +119,7 @@ class ModelAttributeMethodArgumentResolverTests { } @Test - void createAndBindToMono() throws Exception { + void createAndBindToMono() { MethodParameter parameter = this.testMethod .annotNotPresent(ModelAttribute.class).arg(Mono.class, Pojo.class); @@ -133,7 +132,7 @@ class ModelAttributeMethodArgumentResolverTests { } @Test - void createAndBindToSingle() throws Exception { + void createAndBindToSingle() { MethodParameter parameter = this.testMethod .annotPresent(ModelAttribute.class).arg(Single.class, Pojo.class); @@ -146,7 +145,7 @@ class ModelAttributeMethodArgumentResolverTests { } @Test - void createButDoNotBind() throws Exception { + void createButDoNotBind() { MethodParameter parameter = this.testMethod.annotPresent(ModelAttribute.class).arg(NonBindingPojo.class); @@ -157,7 +156,7 @@ class ModelAttributeMethodArgumentResolverTests { } @Test - void createButDoNotBindToMono() throws Exception { + void createButDoNotBindToMono() { MethodParameter parameter = this.testMethod.annotPresent(ModelAttribute.class).arg(Mono.class, NonBindingPojo.class); @@ -170,7 +169,7 @@ class ModelAttributeMethodArgumentResolverTests { } @Test - void createButDoNotBindToSingle() throws Exception { + void createButDoNotBindToSingle() { MethodParameter parameter = this.testMethod.annotPresent(ModelAttribute.class).arg(Single.class, NonBindingPojo.class); @@ -183,7 +182,7 @@ class ModelAttributeMethodArgumentResolverTests { } private void createButDoNotBindToPojo(String modelKey, MethodParameter methodParameter, - Function valueExtractor) throws Exception { + Function valueExtractor) { Object value = createResolver() .resolveArgument(methodParameter, this.bindContext, postForm("name=Enigma")) @@ -202,7 +201,7 @@ class ModelAttributeMethodArgumentResolverTests { } @Test - void bindExisting() throws Exception { + void bindExisting() { Pojo pojo = new Pojo(); pojo.setName("Jim"); this.bindContext.getModel().addAttribute(pojo); @@ -217,7 +216,7 @@ class ModelAttributeMethodArgumentResolverTests { } @Test - void bindExistingMono() throws Exception { + void bindExistingMono() { Pojo pojo = new Pojo(); pojo.setName("Jim"); this.bindContext.getModel().addAttribute("pojoMono", Mono.just(pojo)); @@ -232,7 +231,7 @@ class ModelAttributeMethodArgumentResolverTests { } @Test - void bindExistingSingle() throws Exception { + void bindExistingSingle() { Pojo pojo = new Pojo(); pojo.setName("Jim"); this.bindContext.getModel().addAttribute("pojoSingle", Single.just(pojo)); @@ -247,7 +246,7 @@ class ModelAttributeMethodArgumentResolverTests { } @Test - void bindExistingMonoToMono() throws Exception { + void bindExistingMonoToMono() { Pojo pojo = new Pojo(); pojo.setName("Jim"); String modelKey = "pojoMono"; @@ -264,8 +263,7 @@ class ModelAttributeMethodArgumentResolverTests { }); } - private void testBindPojo(String modelKey, MethodParameter param, Function valueExtractor) - throws Exception { + private void testBindPojo(String modelKey, MethodParameter param, Function valueExtractor) { Object value = createResolver() .resolveArgument(param, this.bindContext, postForm("name=Robert&age=25")) @@ -284,13 +282,13 @@ class ModelAttributeMethodArgumentResolverTests { } @Test - void validationErrorForPojo() throws Exception { + void validationErrorForPojo() { MethodParameter parameter = this.testMethod.annotNotPresent(ModelAttribute.class).arg(Pojo.class); testValidationError(parameter, Function.identity()); } @Test - void validationErrorForMono() throws Exception { + void validationErrorForMono() { MethodParameter parameter = this.testMethod .annotNotPresent(ModelAttribute.class).arg(Mono.class, Pojo.class); @@ -303,7 +301,7 @@ class ModelAttributeMethodArgumentResolverTests { } @Test - void validationErrorForSingle() throws Exception { + void validationErrorForSingle() { MethodParameter parameter = this.testMethod .annotPresent(ModelAttribute.class).arg(Single.class, Pojo.class); @@ -316,13 +314,13 @@ class ModelAttributeMethodArgumentResolverTests { } @Test - void validationErrorWithoutBindingForPojo() throws Exception { + void validationErrorWithoutBindingForPojo() { MethodParameter parameter = this.testMethod.annotPresent(ModelAttribute.class).arg(ValidatedPojo.class); testValidationErrorWithoutBinding(parameter, Function.identity()); } @Test - void validationErrorWithoutBindingForMono() throws Exception { + void validationErrorWithoutBindingForMono() { MethodParameter parameter = this.testMethod.annotPresent(ModelAttribute.class).arg(Mono.class, ValidatedPojo.class); testValidationErrorWithoutBinding(parameter, resolvedArgumentMono -> { @@ -343,20 +341,16 @@ class ModelAttributeMethodArgumentResolverTests { }); } - private void testValidationError(MethodParameter parameter, Function, Mono> valueMonoExtractor) - throws URISyntaxException { - + private void testValidationError(MethodParameter parameter, Function, Mono> valueMonoExtractor) { testValidationError(parameter, valueMonoExtractor, "age=invalid", "age", "invalid"); } - private void testValidationErrorWithoutBinding(MethodParameter parameter, Function, Mono> valueMonoExtractor) - throws URISyntaxException { - + private void testValidationErrorWithoutBinding(MethodParameter parameter, Function, Mono> valueMonoExtractor) { testValidationError(parameter, valueMonoExtractor, "name=Enigma", "name", null); } private void testValidationError(MethodParameter param, Function, Mono> valueMonoExtractor, - String formData, String field, String rejectedValue) throws URISyntaxException { + String formData, String field, String rejectedValue) { Mono mono = createResolver().resolveArgument(param, this.bindContext, postForm(formData)); mono = valueMonoExtractor.apply(mono); @@ -373,7 +367,7 @@ class ModelAttributeMethodArgumentResolverTests { } @Test - void bindDataClass() throws Exception { + void bindDataClass() { MethodParameter parameter = this.testMethod.annotNotPresent(ModelAttribute.class).arg(DataClass.class); Object value = createResolver() @@ -401,7 +395,7 @@ class ModelAttributeMethodArgumentResolverTests { return new ModelAttributeMethodArgumentResolver(ReactiveAdapterRegistry.getSharedInstance(), false); } - private ServerWebExchange postForm(String formData) throws URISyntaxException { + private ServerWebExchange postForm(String formData) { return MockServerWebExchange.from(MockServerHttpRequest.post("/") .contentType(MediaType.APPLICATION_FORM_URLENCODED) .body(formData)); From e6565c600a642af2ebafb459c738cda06c58f493 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Deleuze?= Date: Fri, 18 Aug 2023 11:46:00 +0200 Subject: [PATCH 2/2] Refine type conversion errors in ModelAttributeMethodProcessor This commit turns TypeMismatchException thrown in ModelAttributeMethodArgumentResolver#createAttribute into proper ServerWebInputException in order get HTTP response with 400 Bad Request status code instead of 500 Internal error. Closes gh-31045 --- .../ModelAttributeMethodArgumentResolver.java | 10 ++- ...lAttributeMethodArgumentResolverTests.java | 12 +++ ...ributeMethodArgumentResolverKotlinTests.kt | 85 +++++++++++++++++++ 3 files changed, 106 insertions(+), 1 deletion(-) create mode 100644 spring-webflux/src/test/kotlin/org/springframework/web/reactive/result/method/annotation/ModelAttributeMethodArgumentResolverKotlinTests.kt diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/ModelAttributeMethodArgumentResolver.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/ModelAttributeMethodArgumentResolver.java index 25ae88e54f..c59d835f0d 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/ModelAttributeMethodArgumentResolver.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/ModelAttributeMethodArgumentResolver.java @@ -26,6 +26,7 @@ import reactor.core.publisher.Mono; import reactor.core.publisher.Sinks; import org.springframework.beans.BeanUtils; +import org.springframework.beans.TypeMismatchException; import org.springframework.context.i18n.LocaleContext; import org.springframework.context.i18n.LocaleContextHolder; import org.springframework.core.MethodParameter; @@ -45,6 +46,7 @@ import org.springframework.web.bind.support.WebExchangeDataBinder; import org.springframework.web.reactive.BindingContext; import org.springframework.web.reactive.result.method.HandlerMethodArgumentResolverSupport; import org.springframework.web.server.ServerWebExchange; +import org.springframework.web.server.ServerWebInputException; /** * Resolve {@code @ModelAttribute} annotated method arguments. @@ -63,6 +65,7 @@ import org.springframework.web.server.ServerWebExchange; * @author Rossen Stoyanchev * @author Juergen Hoeller * @author Sam Brannen + * @author Sebastien Deleuze * @since 5.0 */ public class ModelAttributeMethodArgumentResolver extends HandlerMethodArgumentResolverSupport { @@ -255,7 +258,12 @@ public class ModelAttributeMethodArgumentResolver extends HandlerMethodArgumentR args[i] = (methodParam.getParameterType() == Optional.class ? Optional.empty() : null); } else { - args[i] = binder.convertIfNecessary(value, paramTypes[i], methodParam); + try { + args[i] = binder.convertIfNecessary(value, paramTypes[i], methodParam); + } + catch (TypeMismatchException ex) { + throw new ServerWebInputException("Type mismatch.", methodParam, ex); + } } } return BeanUtils.instantiateClass(ctor, args); diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/ModelAttributeMethodArgumentResolverTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/ModelAttributeMethodArgumentResolverTests.java index d54b842a4f..c3c10578b5 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/ModelAttributeMethodArgumentResolverTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/ModelAttributeMethodArgumentResolverTests.java @@ -38,6 +38,7 @@ import org.springframework.web.bind.support.ConfigurableWebBindingInitializer; import org.springframework.web.bind.support.WebExchangeBindException; import org.springframework.web.reactive.BindingContext; import org.springframework.web.server.ServerWebExchange; +import org.springframework.web.server.ServerWebInputException; import org.springframework.web.testfixture.http.server.reactive.MockServerHttpRequest; import org.springframework.web.testfixture.method.ResolvableMethod; import org.springframework.web.testfixture.server.MockServerWebExchange; @@ -50,6 +51,7 @@ import static org.assertj.core.api.Assertions.assertThat; * @author Rossen Stoyanchev * @author Juergen Hoeller * @author Sam Brannen + * @author Sebastien Deleuze */ class ModelAttributeMethodArgumentResolverTests { @@ -388,6 +390,16 @@ class ModelAttributeMethodArgumentResolverTests { assertThat(model.get(bindingResultKey)).isInstanceOf(BindingResult.class); } + @Test + void bindDataClassError() { + MethodParameter parameter = this.testMethod.annotNotPresent(ModelAttribute.class).arg(DataClass.class); + Mono mono = createResolver().resolveArgument(parameter, this.bindContext, postForm("name=Robert&age=invalid&count=1")); + StepVerifier.create(mono) + .expectNextCount(0) + .expectError(ServerWebInputException.class) + .verify(); + } + // TODO: SPR-15871, SPR-15542 diff --git a/spring-webflux/src/test/kotlin/org/springframework/web/reactive/result/method/annotation/ModelAttributeMethodArgumentResolverKotlinTests.kt b/spring-webflux/src/test/kotlin/org/springframework/web/reactive/result/method/annotation/ModelAttributeMethodArgumentResolverKotlinTests.kt new file mode 100644 index 0000000000..4e98ac85e5 --- /dev/null +++ b/spring-webflux/src/test/kotlin/org/springframework/web/reactive/result/method/annotation/ModelAttributeMethodArgumentResolverKotlinTests.kt @@ -0,0 +1,85 @@ +/* + * Copyright 2002-2023 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.web.reactive.result.method.annotation + +import org.junit.jupiter.api.BeforeEach +import org.junit.jupiter.api.Test +import org.springframework.core.MethodParameter +import org.springframework.core.ReactiveAdapterRegistry +import org.springframework.http.MediaType +import org.springframework.validation.beanvalidation.LocalValidatorFactoryBean +import org.springframework.web.bind.annotation.ModelAttribute +import org.springframework.web.bind.support.ConfigurableWebBindingInitializer +import org.springframework.web.reactive.BindingContext +import org.springframework.web.server.ServerWebExchange +import org.springframework.web.server.ServerWebInputException +import org.springframework.web.testfixture.http.server.reactive.MockServerHttpRequest +import org.springframework.web.testfixture.method.ResolvableMethod +import org.springframework.web.testfixture.server.MockServerWebExchange +import reactor.core.publisher.Mono +import reactor.test.StepVerifier + +/** + * Kotlin test fixture for [ModelAttributeMethodArgumentResolver]. + * + * @author Sebastien Deleuze + */ +class ModelAttributeMethodArgumentResolverKotlinTests { + + private val testMethod = ResolvableMethod.on(javaClass).named("handle").build() + + private lateinit var bindContext: BindingContext + + @BeforeEach + fun setup() { + val validator = LocalValidatorFactoryBean() + validator.afterPropertiesSet() + val initializer = ConfigurableWebBindingInitializer() + initializer.validator = validator + this.bindContext = BindingContext(initializer) + } + + @Test + fun bindDataClassError() { + val parameter: MethodParameter = this.testMethod.annotNotPresent(ModelAttribute::class.java).arg(DataClass::class.java) + val mono: Mono = + createResolver().resolveArgument(parameter, this.bindContext, postForm("name=Robert&age=invalid&count=1")) + StepVerifier.create(mono) + .expectNextCount(0) + .expectError(ServerWebInputException::class.java) + .verify() + } + + private fun createResolver(): ModelAttributeMethodArgumentResolver { + return ModelAttributeMethodArgumentResolver(ReactiveAdapterRegistry.getSharedInstance(), false) + } + + private fun postForm(formData: String): ServerWebExchange { + return MockServerWebExchange.from( + MockServerHttpRequest.post("/") + .contentType(MediaType.APPLICATION_FORM_URLENCODED) + .body(formData) + ) + } + + @Suppress("UNUSED_PARAMETER") + private fun handle(dataClassNotAnnotated: DataClass) { + } + + private class DataClass(val name: String, val age: Int, val count: Int) + +}