From a5103307c6f0b124ca778fd23a4f55f3382a4f2d Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Fri, 10 Nov 2017 10:15:46 -0500 Subject: [PATCH] Polish ErrorArgumentResolver --- .../ErrorsMethodArgumentResolver.java | 13 +-- .../ErrorsMethodArgumentResolver.java | 8 +- ...=> ErrorsMethodArgumentResolverTests.java} | 85 ++++++++++--------- 3 files changed, 56 insertions(+), 50 deletions(-) rename spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/{ErrorsArgumentResolverTests.java => ErrorsMethodArgumentResolverTests.java} (63%) diff --git a/spring-web/src/main/java/org/springframework/web/method/annotation/ErrorsMethodArgumentResolver.java b/spring-web/src/main/java/org/springframework/web/method/annotation/ErrorsMethodArgumentResolver.java index 7928ab8d7ba..cdf13c70ac6 100644 --- a/spring-web/src/main/java/org/springframework/web/method/annotation/ErrorsMethodArgumentResolver.java +++ b/spring-web/src/main/java/org/springframework/web/method/annotation/ErrorsMethodArgumentResolver.java @@ -50,10 +50,12 @@ public class ErrorsMethodArgumentResolver implements HandlerMethodArgumentResolv @Override @Nullable - public Object resolveArgument(MethodParameter parameter, @Nullable ModelAndViewContainer mavContainer, - NativeWebRequest webRequest, @Nullable WebDataBinderFactory binderFactory) throws Exception { + public Object resolveArgument(MethodParameter parameter, + @Nullable ModelAndViewContainer mavContainer, NativeWebRequest webRequest, + @Nullable WebDataBinderFactory binderFactory) throws Exception { - Assert.state(mavContainer != null, "Errors/BindingResult argument only supported on regular handler methods"); + Assert.state(mavContainer != null, + "Errors/BindingResult argument only supported on regular handler methods"); ModelMap model = mavContainer.getModel(); if (model.size() > 0) { @@ -65,8 +67,9 @@ public class ErrorsMethodArgumentResolver implements HandlerMethodArgumentResolv } throw new IllegalStateException( - "An Errors/BindingResult argument is expected to be declared immediately after the model attribute, " + - "the @RequestBody or the @RequestPart arguments to which they apply: " + parameter.getMethod()); + "An Errors/BindingResult argument is expected to be declared immediately after " + + "the model attribute, the @RequestBody or the @RequestPart arguments " + + "to which they apply: " + parameter.getMethod()); } } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/ErrorsMethodArgumentResolver.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/ErrorsMethodArgumentResolver.java index 4409f1d929a..356da46e85a 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/ErrorsMethodArgumentResolver.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/ErrorsMethodArgumentResolver.java @@ -75,6 +75,7 @@ public class ErrorsMethodArgumentResolver extends HandlerMethodArgumentResolverS } private String getModelAttributeName(MethodParameter parameter) { + Assert.isTrue(parameter.getParameterIndex() > 0, "Errors argument must be immediately after a model attribute argument"); @@ -82,9 +83,10 @@ public class ErrorsMethodArgumentResolver extends HandlerMethodArgumentResolverS MethodParameter attributeParam = MethodParameter.forExecutable(parameter.getExecutable(), index); ReactiveAdapter adapter = getAdapterRegistry().getAdapter(attributeParam.getParameterType()); - Assert.isNull(adapter, "Errors/BindingResult cannot be used with an async model attribute. " + - "Either declare the model attribute without the async wrapper type " + - "or handle WebExchangeBindException through the async type."); + Assert.isNull(adapter, "An @ModelAttribute and an Errors/BindingResult) arguments " + + "cannot both be declared with an async type wrapper. " + + "Either declare the @ModelAttribute without an async wrapper type or " + + "handle a WebExchangeBindException error signal through the async type."); ModelAttribute ann = parameter.getParameterAnnotation(ModelAttribute.class); if (ann != null && StringUtils.hasText(ann.value())) { diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/ErrorsArgumentResolverTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/ErrorsMethodArgumentResolverTests.java similarity index 63% rename from spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/ErrorsArgumentResolverTests.java rename to spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/ErrorsMethodArgumentResolverTests.java index 3c8338f10fa..5493079c880 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/ErrorsArgumentResolverTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/ErrorsMethodArgumentResolverTests.java @@ -19,7 +19,9 @@ package org.springframework.web.reactive.result.method.annotation; import java.time.Duration; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; import reactor.core.publisher.Mono; import reactor.core.publisher.MonoProcessor; @@ -29,9 +31,9 @@ import org.springframework.core.ResolvableType; import org.springframework.mock.http.server.reactive.test.MockServerHttpRequest; import org.springframework.mock.web.test.server.MockServerWebExchange; import org.springframework.validation.BindingResult; +import org.springframework.validation.DataBinder; import org.springframework.validation.Errors; import org.springframework.web.bind.annotation.ModelAttribute; -import org.springframework.web.bind.support.WebExchangeDataBinder; import org.springframework.web.method.ResolvableMethod; import org.springframework.web.reactive.BindingContext; @@ -42,29 +44,22 @@ import static org.junit.Assert.fail; /** * Unit tests for {@link ErrorsMethodArgumentResolver}. - * * @author Rossen Stoyanchev */ -public class ErrorsArgumentResolverTests { +public class ErrorsMethodArgumentResolverTests { - private ErrorsMethodArgumentResolver resolver ; + private final ErrorsMethodArgumentResolver resolver = + new ErrorsMethodArgumentResolver(new ReactiveAdapterRegistry()); private final BindingContext bindingContext = new BindingContext(); - private BindingResult bindingResult; - - private MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.post("/path")); + private final MockServerWebExchange exchange = + MockServerWebExchange.from(MockServerHttpRequest.post("/path")); private final ResolvableMethod testMethod = ResolvableMethod.on(getClass()).named("handle").build(); - - @Before - public void setup() throws Exception { - this.resolver = new ErrorsMethodArgumentResolver(new ReactiveAdapterRegistry()); - Foo foo = new Foo(); - WebExchangeDataBinder binder = this.bindingContext.createDataBinder(this.exchange, foo, "foo"); - this.bindingResult = binder.getBindingResult(); - } + @Rule + public final ExpectedException expectedException = ExpectedException.none(); @Test @@ -82,48 +77,53 @@ public class ErrorsArgumentResolverTests { MethodParameter parameter = this.testMethod.arg(String.class); assertFalse(this.resolver.supportsParameter(parameter)); - try { - parameter = this.testMethod.arg(ResolvableType.forClassWithGenerics(Mono.class, Errors.class)); - assertFalse(this.resolver.supportsParameter(parameter)); - fail(); - } - catch (IllegalStateException ex) { - assertTrue("Unexpected error message:\n" + ex.getMessage(), - ex.getMessage().startsWith( - "ErrorsMethodArgumentResolver doesn't support reactive type wrapper")); - } + this.expectedException.expectMessage("ErrorsMethodArgumentResolver doesn't support reactive type wrapper"); + parameter = this.testMethod.arg(ResolvableType.forClassWithGenerics(Mono.class, Errors.class)); + this.resolver.supportsParameter(parameter); } @Test public void resolveErrors() throws Exception { - testResolve(this.bindingResult); - } - @Test - public void resolveErrorsMono() throws Exception { - MonoProcessor monoProcessor = MonoProcessor.create(); - monoProcessor.onNext(this.bindingResult); - testResolve(monoProcessor); - } + BindingResult bindingResult = createBindingResult(new Foo(), "foo"); + this.bindingContext.getModel().asMap().put(BindingResult.MODEL_KEY_PREFIX + "foo", bindingResult); - @Test(expected = IllegalArgumentException.class) - public void resolveErrorsAfterMonoModelAttribute() throws Exception { - MethodParameter parameter = this.testMethod.arg(BindingResult.class); - this.resolver.resolveArgument(parameter, this.bindingContext, this.exchange).block(Duration.ofMillis(5000)); + MethodParameter parameter = this.testMethod.arg(Errors.class); + Object actual = this.resolver.resolveArgument(parameter, this.bindingContext, this.exchange) + .block(Duration.ofMillis(5000)); + + assertSame(bindingResult, actual); } + private BindingResult createBindingResult(Foo target, String name) { + DataBinder binder = this.bindingContext.createDataBinder(this.exchange, target, name); + return binder.getBindingResult(); + } - private void testResolve(Object bindingResult) { + @Test + public void resolveErrorsWithMono() throws Exception { - String key = BindingResult.MODEL_KEY_PREFIX + "foo"; - this.bindingContext.getModel().asMap().put(key, bindingResult); + BindingResult bindingResult = createBindingResult(new Foo(), "foo"); + MonoProcessor monoProcessor = MonoProcessor.create(); + monoProcessor.onNext(bindingResult); + this.bindingContext.getModel().asMap().put(BindingResult.MODEL_KEY_PREFIX + "foo", monoProcessor); MethodParameter parameter = this.testMethod.arg(Errors.class); - Object actual = this.resolver.resolveArgument(parameter, this.bindingContext, this.exchange) .block(Duration.ofMillis(5000)); - assertSame(this.bindingResult, actual); + assertSame(bindingResult, actual); + } + + @Test + public void resolveErrorsAfterMonoModelAttribute() throws Exception { + + this.expectedException.expectMessage("An @ModelAttribute and an Errors/BindingResult) arguments " + + "cannot both be declared with an async type wrapper."); + + MethodParameter parameter = this.testMethod.arg(BindingResult.class); + this.resolver.resolveArgument(parameter, this.bindingContext, this.exchange) + .block(Duration.ofMillis(5000)); } @@ -148,6 +148,7 @@ public class ErrorsArgumentResolverTests { } } + @SuppressWarnings("unused") void handle( @ModelAttribute Foo foo, Errors errors,