From 3780d040ee351e718492ac81fba66f4ef00bc8ab Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Fri, 7 Apr 2017 16:42:59 -0400 Subject: [PATCH] Polish and minor fixes in ViewResolutionResultHandler --- .../web/method/ResolvableMethod.java | 14 +- .../view/ViewResolutionResultHandler.java | 27 +-- .../ViewResolutionResultHandlerTests.java | 179 ++++++++---------- 3 files changed, 101 insertions(+), 119 deletions(-) diff --git a/spring-web/src/test/java/org/springframework/web/method/ResolvableMethod.java b/spring-web/src/test/java/org/springframework/web/method/ResolvableMethod.java index a04650b71d0..4362e112cd5 100644 --- a/spring-web/src/test/java/org/springframework/web/method/ResolvableMethod.java +++ b/spring-web/src/test/java/org/springframework/web/method/ResolvableMethod.java @@ -282,7 +282,7 @@ public class ResolvableMethod { /** * Filter on methods with the given name. */ - public Builder named(String methodName) { + public Builder named(String methodName) { addFilter("methodName=" + methodName, m -> m.getName().equals(methodName)); return this; } @@ -292,7 +292,7 @@ public class ResolvableMethod { * See {@link MvcAnnotationPredicates}. */ @SafeVarargs - public final Builder annot(Predicate... filters) { + public final Builder annot(Predicate... filters) { this.filters.addAll(Arrays.asList(filters)); return this; } @@ -303,7 +303,7 @@ public class ResolvableMethod { * @see MvcAnnotationPredicates */ @SafeVarargs - public final Builder annotPresent(Class... annotationTypes) { + public final Builder annotPresent(Class... annotationTypes) { String message = "annotationPresent=" + Arrays.toString(annotationTypes); addFilter(message, method -> Arrays.stream(annotationTypes).allMatch(annotType -> @@ -315,7 +315,7 @@ public class ResolvableMethod { * Filter on methods not annotated with the given annotation type. */ @SafeVarargs - public final Builder annotNotPresent(Class... annotationTypes) { + public final Builder annotNotPresent(Class... annotationTypes) { String message = "annotationNotPresent=" + Arrays.toString(annotationTypes); addFilter(message, method -> { if (annotationTypes.length != 0) { @@ -334,7 +334,7 @@ public class ResolvableMethod { * @param returnType the return type * @param generics optional array of generic types */ - public Builder returning(Class returnType, Class... generics) { + public Builder returning(Class returnType, Class... generics) { return returning(toResolvableType(returnType, generics)); } @@ -344,7 +344,7 @@ public class ResolvableMethod { * @param generic at least one generic type * @param generics optional extra generic types */ - public Builder returning(Class returnType, ResolvableType generic, ResolvableType... generics) { + public Builder returning(Class returnType, ResolvableType generic, ResolvableType... generics) { return returning(toResolvableType(returnType, generic, generics)); } @@ -352,7 +352,7 @@ public class ResolvableMethod { * Filter on methods returning the given type. * @param returnType the return type */ - public Builder returning(ResolvableType returnType) { + public Builder returning(ResolvableType returnType) { String expected = returnType.toString(); String message = "returnType=" + expected; addFilter(message, m -> expected.equals(ResolvableType.forMethodReturnType(m).toString())); diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandler.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandler.java index 1f59bd55cf0..a58e4f96897 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandler.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandler.java @@ -157,9 +157,10 @@ public class ViewResolutionResultHandler extends HandlerResultHandlerSupport } type = result.getReturnType().getGeneric(0).getRawClass(); } - return (CharSequence.class.isAssignableFrom(type) || View.class.isAssignableFrom(type) || + return (CharSequence.class.isAssignableFrom(type) || Rendering.class.isAssignableFrom(type) || Model.class.isAssignableFrom(type) || Map.class.isAssignableFrom(type) || - Rendering.class.isAssignableFrom(type) || !BeanUtils.isSimpleProperty(type)); + void.class.equals(type) || View.class.isAssignableFrom(type) || + !BeanUtils.isSimpleProperty(type)); } private boolean hasModelAnnotation(MethodParameter parameter) { @@ -210,17 +211,6 @@ public class ViewResolutionResultHandler extends HandlerResultHandlerSupport if (returnValue == NO_VALUE || Void.class.equals(clazz) || void.class.equals(clazz)) { viewsMono = resolveViews(getDefaultViewName(exchange), locale); } - else if (Model.class.isAssignableFrom(clazz)) { - model.addAllAttributes(((Model) returnValue).asMap()); - viewsMono = resolveViews(getDefaultViewName(exchange), locale); - } - else if (Map.class.isAssignableFrom(clazz)) { - model.addAllAttributes((Map) returnValue); - viewsMono = resolveViews(getDefaultViewName(exchange), locale); - } - else if (View.class.isAssignableFrom(clazz)) { - viewsMono = Mono.just(Collections.singletonList((View) returnValue)); - } else if (CharSequence.class.isAssignableFrom(clazz) && !hasModelAnnotation(parameter)) { viewsMono = resolveViews(returnValue.toString(), locale); } @@ -233,6 +223,17 @@ public class ViewResolutionResultHandler extends HandlerResultHandlerSupport viewsMono = (view instanceof String ? resolveViews((String) view, locale) : Mono.just(Collections.singletonList((View) view))); } + else if (Model.class.isAssignableFrom(clazz)) { + model.addAllAttributes(((Model) returnValue).asMap()); + viewsMono = resolveViews(getDefaultViewName(exchange), locale); + } + else if (Map.class.isAssignableFrom(clazz) && !hasModelAnnotation(parameter)) { + model.addAllAttributes((Map) returnValue); + viewsMono = resolveViews(getDefaultViewName(exchange), locale); + } + else if (View.class.isAssignableFrom(clazz)) { + viewsMono = Mono.just(Collections.singletonList((View) returnValue)); + } else { String name = getNameForReturnValue(clazz, parameter); model.addAttribute(name, returnValue); diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandlerTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandlerTests.java index 934096866f7..8b693d5663f 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandlerTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandlerTests.java @@ -32,10 +32,10 @@ import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; import reactor.test.StepVerifier; import rx.Completable; -import rx.Single; import org.springframework.core.MethodParameter; import org.springframework.core.Ordered; +import org.springframework.core.ResolvableType; import org.springframework.core.io.buffer.DataBuffer; import org.springframework.core.io.buffer.DefaultDataBufferFactory; import org.springframework.core.io.buffer.support.DataBufferTestUtils; @@ -55,8 +55,6 @@ import org.springframework.web.server.ServerWebExchange; import static java.nio.charset.StandardCharsets.UTF_8; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.mock; import static org.springframework.http.MediaType.APPLICATION_JSON; import static org.springframework.mock.http.server.reactive.test.MockServerHttpRequest.get; @@ -76,35 +74,43 @@ public class ViewResolutionResultHandlerTests { @Test public void supports() throws Exception { - testSupports(on(TestController.class).resolveReturnType(String.class)); - testSupports(on(TestController.class).resolveReturnType(View.class)); - testSupports(on(TestController.class).resolveReturnType(Mono.class, String.class)); - testSupports(on(TestController.class).resolveReturnType(Mono.class, View.class)); - testSupports(on(TestController.class).resolveReturnType(Single.class, String.class)); - testSupports(on(TestController.class).resolveReturnType(Single.class, View.class)); - testSupports(on(TestController.class).resolveReturnType(Mono.class, Void.class)); - testSupports(on(TestController.class).resolveReturnType(Completable.class)); - testSupports(on(TestController.class).resolveReturnType(Model.class)); - testSupports(on(TestController.class).resolveReturnType(Map.class)); - testSupports(on(TestController.class).resolveReturnType(TestBean.class)); - testSupports(on(TestController.class).resolveReturnType(Rendering.class)); - testSupports(on(TestController.class).resolveReturnType(Mono.class, Rendering.class)); - - testSupports(on(TestController.class).annotPresent(ModelAttribute.class).resolveReturnType()); + testSupports(on(Handler.class).annotPresent(ModelAttribute.class).resolveReturnType(String.class)); + testSupports(on(Handler.class).annotNotPresent(ModelAttribute.class).resolveReturnType(String.class)); + testSupports(on(Handler.class).resolveReturnType(Mono.class, String.class)); + + testSupports(on(Handler.class).resolveReturnType(Rendering.class)); + testSupports(on(Handler.class).resolveReturnType(Mono.class, Rendering.class)); + + testSupports(on(Handler.class).resolveReturnType(View.class)); + testSupports(on(Handler.class).resolveReturnType(Mono.class, View.class)); + + testSupports(on(Handler.class).resolveReturnType(void.class)); + testSupports(on(Handler.class).resolveReturnType(Mono.class, Void.class)); + testSupports(on(Handler.class).resolveReturnType(Completable.class)); + + testSupports(on(Handler.class).resolveReturnType(Model.class)); + + testSupports(on(Handler.class).annotPresent(ModelAttribute.class).resolveReturnType(Map.class)); + testSupports(on(Handler.class).annotNotPresent(ModelAttribute.class).resolveReturnType(Map.class)); + + testSupports(on(Handler.class).resolveReturnType(TestBean.class)); + + testSupports(on(Handler.class).annotPresent(ModelAttribute.class).resolveReturnType(Long.class)); + testDoesNotSupport(on(Handler.class).annotNotPresent(ModelAttribute.class).resolveReturnType(Long.class)); } private void testSupports(MethodParameter returnType) { - ViewResolutionResultHandler resultHandler = resultHandler(mock(ViewResolver.class)); - HandlerResult handlerResult = new HandlerResult(new Object(), null, returnType, this.bindingContext); - assertTrue(resultHandler.supports(handlerResult)); + testSupports(returnType, true); } - @Test - public void doesNotSupport() throws Exception { - MethodParameter returnType = on(TestController.class).resolveReturnType(Integer.class); + private void testDoesNotSupport(MethodParameter returnType) { + testSupports(returnType, false); + } + + private void testSupports(MethodParameter returnType, boolean supports) { ViewResolutionResultHandler resultHandler = resultHandler(mock(ViewResolver.class)); HandlerResult handlerResult = new HandlerResult(new Object(), null, returnType, this.bindingContext); - assertFalse(resultHandler.supports(handlerResult)); + assertEquals(supports, resultHandler.supports(handlerResult)); } @Test @@ -125,31 +131,45 @@ public class ViewResolutionResultHandlerTests { MethodParameter returnType; ViewResolver resolver = new TestViewResolver("account"); - returnType = on(TestController.class).resolveReturnType(View.class); + returnType = on(Handler.class).resolveReturnType(View.class); returnValue = new TestView("account"); testHandle("/path", returnType, returnValue, "account: {id=123}"); - returnType = on(TestController.class).resolveReturnType(Mono.class, View.class); + returnType = on(Handler.class).resolveReturnType(Mono.class, View.class); returnValue = Mono.just(new TestView("account")); testHandle("/path", returnType, returnValue, "account: {id=123}"); - returnType = on(TestController.class).resolveReturnType(String.class); + returnType = on(Handler.class).annotNotPresent(ModelAttribute.class).resolveReturnType(String.class); returnValue = "account"; testHandle("/path", returnType, returnValue, "account: {id=123}", resolver); - returnType = on(TestController.class).resolveReturnType(Mono.class, String.class); + returnType = on(Handler.class).annotPresent(ModelAttribute.class).resolveReturnType(String.class); + returnValue = "123"; + testHandle("/account", returnType, returnValue, "account: {id=123, myString=123}", resolver); + + returnType = on(Handler.class).resolveReturnType(Mono.class, String.class); returnValue = Mono.just("account"); testHandle("/path", returnType, returnValue, "account: {id=123}", resolver); - returnType = on(TestController.class).resolveReturnType(Model.class); + returnType = on(Handler.class).resolveReturnType(Model.class); returnValue = new ConcurrentModel().addAttribute("name", "Joe"); testHandle("/account", returnType, returnValue, "account: {id=123, name=Joe}", resolver); - returnType = on(TestController.class).resolveReturnType(Map.class); + // Work around caching issue... + ResolvableType.clearCache(); + + returnType = on(Handler.class).annotNotPresent(ModelAttribute.class).resolveReturnType(Map.class); returnValue = Collections.singletonMap("name", "Joe"); testHandle("/account", returnType, returnValue, "account: {id=123, name=Joe}", resolver); - returnType = on(TestController.class).resolveReturnType(TestBean.class); + // Work around caching issue... + ResolvableType.clearCache(); + + returnType = on(Handler.class).annotPresent(ModelAttribute.class).resolveReturnType(Map.class); + returnValue = Collections.singletonMap("name", "Joe"); + testHandle("/account", returnType, returnValue, "account: {id=123, myMap={name=Joe}}", resolver); + + returnType = on(Handler.class).resolveReturnType(TestBean.class); returnValue = new TestBean("Joe"); String responseBody = "account: {id=123, " + "org.springframework.validation.BindingResult.testBean=" + @@ -157,10 +177,10 @@ public class ViewResolutionResultHandlerTests { "testBean=TestBean[name=Joe]}"; testHandle("/account", returnType, returnValue, responseBody, resolver); - returnType = on(TestController.class).annotPresent(ModelAttribute.class).resolveReturnType(); - testHandle("/account", returnType, 99L, "account: {id=123, num=99}", resolver); + returnType = on(Handler.class).annotPresent(ModelAttribute.class).resolveReturnType(Long.class); + testHandle("/account", returnType, 99L, "account: {id=123, myLong=99}", resolver); - returnType = on(TestController.class).resolveReturnType(Rendering.class); + returnType = on(Handler.class).resolveReturnType(Rendering.class); HttpStatus status = HttpStatus.UNPROCESSABLE_ENTITY; returnValue = Rendering.view("account").modelAttribute("a", "a1").status(status).header("h", "h1").build(); String expected = "account: {a=a1, id=123}"; @@ -172,7 +192,7 @@ public class ViewResolutionResultHandlerTests { @Test public void handleWithMultipleResolvers() throws Exception { Object returnValue = "profile"; - MethodParameter returnType = on(TestController.class).resolveReturnType(String.class); + MethodParameter returnType = on(Handler.class).annotNotPresent(ModelAttribute.class).resolveReturnType(String.class); ViewResolver[] resolvers = {new TestViewResolver("account"), new TestViewResolver("profile")}; testHandle("/account", returnType, returnValue, "profile: {id=123}", resolvers); @@ -180,10 +200,10 @@ public class ViewResolutionResultHandlerTests { @Test public void defaultViewName() throws Exception { - testDefaultViewName(null, on(TestController.class).resolveReturnType(String.class)); - testDefaultViewName(Mono.empty(), on(TestController.class).resolveReturnType(Mono.class, String.class)); - testDefaultViewName(Mono.empty(), on(TestController.class).resolveReturnType(Mono.class, Void.class)); - testDefaultViewName(Completable.complete(), on(TestController.class).resolveReturnType(Completable.class)); + testDefaultViewName(null, on(Handler.class).annotPresent(ModelAttribute.class).resolveReturnType(String.class)); + testDefaultViewName(Mono.empty(), on(Handler.class).resolveReturnType(Mono.class, String.class)); + testDefaultViewName(Mono.empty(), on(Handler.class).resolveReturnType(Mono.class, Void.class)); + testDefaultViewName(Completable.complete(), on(Handler.class).resolveReturnType(Completable.class)); } private void testDefaultViewName(Object returnValue, MethodParameter returnType) throws URISyntaxException { @@ -207,7 +227,7 @@ public class ViewResolutionResultHandlerTests { @Test public void unresolvedViewName() throws Exception { String returnValue = "account"; - MethodParameter returnType = on(TestController.class).resolveReturnType(String.class); + MethodParameter returnType = on(Handler.class).annotPresent(ModelAttribute.class).resolveReturnType(String.class); HandlerResult result = new HandlerResult(new Object(), returnValue, returnType, this.bindingContext); MockServerWebExchange exchange = get("/path").toExchange(); @@ -222,7 +242,7 @@ public class ViewResolutionResultHandlerTests { @Test public void contentNegotiation() throws Exception { TestBean value = new TestBean("Joe"); - MethodParameter returnType = on(TestController.class).resolveReturnType(TestBean.class); + MethodParameter returnType = on(Handler.class).resolveReturnType(TestBean.class); HandlerResult handlerResult = new HandlerResult(new Object(), value, returnType, this.bindingContext); MockServerWebExchange exchange = get("/account").accept(APPLICATION_JSON).toExchange(); @@ -244,7 +264,7 @@ public class ViewResolutionResultHandlerTests { @Test public void contentNegotiationWith406() throws Exception { TestBean value = new TestBean("Joe"); - MethodParameter returnType = on(TestController.class).resolveReturnType(TestBean.class); + MethodParameter returnType = on(Handler.class).resolveReturnType(TestBean.class); HandlerResult handlerResult = new HandlerResult(new Object(), value, returnType, this.bindingContext); MockServerWebExchange exchange = get("/account").accept(APPLICATION_JSON).toExchange(); @@ -381,71 +401,32 @@ public class ViewResolutionResultHandlerTests { @SuppressWarnings("unused") - private static class TestController { - - String string() { - return null; - } - - View view() { - return null; - } - - Mono monoString() { - return null; - } - - Mono monoView() { - return null; - } - - Mono monoVoid() { - return null; - } - - void voidMethod() { - } - - Single singleString() { - return null; - } + private static class Handler { - Single singleView() { - return null; - } + String string() { return null; } + Mono monoString() { return null; } + @ModelAttribute("myString") String stringWithAnnotation() { return null; } - Completable completable() { - return null; - } + Rendering rendering() { return null; } + Mono monoRendering() { return null; } - Model model() { - return null; - } + View view() { return null; } + Mono monoView() { return null; } - Map map() { - return null; - } + void voidMethod() { } + Mono monoVoid() { return null; } + Completable completable() { return null; } - TestBean testBean() { - return null; - } + Model model() { return null; } - Integer integer() { - return null; - } + Map map() { return null; } + @ModelAttribute("myMap") Map mapWithAnnotation() { return null; } - @ModelAttribute("num") - Long longAttribute() { - return null; - } + TestBean testBean() { return null; } - Rendering rendering() { - return null; - } + Long longValue() { return null; } + @ModelAttribute("myLong") Long longModelAttribute() { return null; } - Mono monoRendering() { - return null; - } } }