From d78d82c516e08c9c74f9ad6260e7a54c8798a342 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Tue, 18 Apr 2017 22:58:56 -0400 Subject: [PATCH] Use Conventions for reactive model attribute names Issue: SPR-14915 --- .../ErrorsMethodArgumentResolver.java | 7 +--- .../ModelAttributeMethodArgumentResolver.java | 42 +++++++++++++++---- .../method/annotation/ModelInitializer.java | 9 ++-- .../view/ViewResolutionResultHandler.java | 9 ++-- ...lAttributeMethodArgumentResolverTests.java | 36 +++++++++------- 5 files changed, 66 insertions(+), 37 deletions(-) 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 d8bc9ab1509..218aed9c06a 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 @@ -18,18 +18,17 @@ package org.springframework.web.reactive.result.method.annotation; import reactor.core.publisher.Mono; +import org.springframework.core.Conventions; import org.springframework.core.MethodParameter; import org.springframework.core.ReactiveAdapter; import org.springframework.core.ReactiveAdapterRegistry; import org.springframework.core.ResolvableType; import org.springframework.util.Assert; -import org.springframework.util.ClassUtils; import org.springframework.util.StringUtils; import org.springframework.validation.BindingResult; import org.springframework.validation.Errors; import org.springframework.web.bind.annotation.ModelAttribute; import org.springframework.web.reactive.BindingContext; -import org.springframework.web.reactive.result.method.HandlerMethodArgumentResolver; import org.springframework.web.reactive.result.method.HandlerMethodArgumentResolverSupport; import org.springframework.web.server.ServerWebExchange; @@ -82,7 +81,6 @@ public class ErrorsMethodArgumentResolver extends HandlerMethodArgumentResolverS int index = parameter.getParameterIndex() - 1; MethodParameter attributeParam = new MethodParameter(parameter.getMethod(), index); - Class attributeType = attributeParam.getParameterType(); ResolvableType type = ResolvableType.forMethodParameter(attributeParam); ReactiveAdapter adapter = getAdapterRegistry().getAdapter(type.resolve()); @@ -95,8 +93,7 @@ public class ErrorsMethodArgumentResolver extends HandlerMethodArgumentResolverS if (annot != null && StringUtils.hasText(annot.value())) { return annot.value(); } - // TODO: Conventions does not deal with async wrappers - return ClassUtils.getShortNameAsProperty(attributeType); + return Conventions.getVariableNameForParameter(attributeParam); } } 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 be7a70cfeb5..4dfe13ee718 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.MonoProcessor; import org.springframework.beans.BeanUtils; +import org.springframework.core.Conventions; import org.springframework.core.DefaultParameterNameDiscoverer; import org.springframework.core.MethodParameter; import org.springframework.core.ParameterNameDiscoverer; @@ -33,6 +34,7 @@ import org.springframework.core.ReactiveAdapter; import org.springframework.core.ReactiveAdapterRegistry; import org.springframework.core.ResolvableType; import org.springframework.core.annotation.AnnotationUtils; +import org.springframework.ui.Model; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; import org.springframework.util.StringUtils; @@ -110,8 +112,8 @@ public class ModelAttributeMethodArgumentResolver extends HandlerMethodArgumentR () -> getClass().getSimpleName() + " doesn't support multi-value reactive type wrapper: " + parameter.getGenericParameterType()); - String name = getAttributeName(valueType, parameter); - Mono valueMono = getAttributeMono(name, valueType, context, exchange); + String name = getAttributeName(parameter); + Mono valueMono = prepareAttributeMono(name, valueType, context, exchange); Map model = context.getModel().asMap(); MonoProcessor bindingResultMono = MonoProcessor.create(); @@ -145,19 +147,23 @@ public class ModelAttributeMethodArgumentResolver extends HandlerMethodArgumentR }); } - private String getAttributeName(ResolvableType valueType, MethodParameter parameter) { + private String getAttributeName(MethodParameter parameter) { ModelAttribute ann = parameter.getParameterAnnotation(ModelAttribute.class); if (ann != null && StringUtils.hasText(ann.value())) { return ann.value(); } - // TODO: Conventions does not deal with async wrappers - return ClassUtils.getShortNameAsProperty(valueType.getRawClass()); + return Conventions.getVariableNameForParameter(parameter); } - private Mono getAttributeMono( - String attributeName, ResolvableType attributeType, BindingContext context, ServerWebExchange exchange) { + private Mono prepareAttributeMono(String attributeName, ResolvableType attributeType, + BindingContext context, ServerWebExchange exchange) { Object attribute = context.getModel().asMap().get(attributeName); + + if (attribute == null) { + attribute = findAndRemoveReactiveAttribute(context.getModel(), attributeName); + } + if (attribute == null) { return createAttribute(attributeName, attributeType.getRawClass(), context, exchange); } @@ -172,6 +178,28 @@ public class ModelAttributeMethodArgumentResolver extends HandlerMethodArgumentR } } + private Object findAndRemoveReactiveAttribute(Model model, String attributeName) { + return model.asMap().entrySet().stream() + .filter(entry -> { + if (!entry.getKey().startsWith(attributeName)) { + return false; + } + ReactiveAdapter adapter = getAdapterRegistry().getAdapter(null, entry.getValue()); + if (adapter == null) { + return false; + } + String name = attributeName + ClassUtils.getShortName(adapter.getReactiveType()); + return entry.getKey().equals(name); + }) + .findFirst() + .map(entry -> { + // Remove since we will be re-inserting the resolved attribute value + model.asMap().remove(entry.getKey()); + return entry.getValue(); + }) + .orElse(null); + } + private Mono createAttribute( String attributeName, Class attributeType, BindingContext context, ServerWebExchange exchange) { diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/ModelInitializer.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/ModelInitializer.java index 7308170f382..2940e892359 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/ModelInitializer.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/ModelInitializer.java @@ -23,12 +23,12 @@ import java.util.stream.Collectors; import reactor.core.publisher.Mono; +import org.springframework.core.Conventions; import org.springframework.core.MethodParameter; import org.springframework.core.ReactiveAdapter; import org.springframework.core.ReactiveAdapterRegistry; import org.springframework.core.ResolvableType; import org.springframework.core.annotation.AnnotatedElementUtils; -import org.springframework.util.ClassUtils; import org.springframework.util.StringUtils; import org.springframework.web.bind.annotation.ModelAttribute; import org.springframework.web.reactive.BindingContext; @@ -107,21 +107,20 @@ class ModelInitializer { attributeType = type.resolve(); } - String name = getAttributeName(attributeType, handlerResult.getReturnTypeSource()); + String name = getAttributeName(handlerResult.getReturnTypeSource()); bindingContext.getModel().asMap().putIfAbsent(name, value); return Mono.empty(); }) .orElse(Mono.empty()); } - private String getAttributeName(Class valueType, MethodParameter parameter) { + private String getAttributeName(MethodParameter parameter) { Method method = parameter.getMethod(); ModelAttribute annot = AnnotatedElementUtils.findMergedAnnotation(method, ModelAttribute.class); if (annot != null && StringUtils.hasText(annot.value())) { return annot.value(); } - // TODO: Conventions does not deal with async wrappers - return ClassUtils.getShortNameAsProperty(valueType); + return Conventions.getVariableNameForParameter(parameter); } } 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 b1502b065f6..5bcf52a2447 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 @@ -28,6 +28,7 @@ import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; import org.springframework.beans.BeanUtils; +import org.springframework.core.Conventions; import org.springframework.core.MethodParameter; import org.springframework.core.Ordered; import org.springframework.core.ReactiveAdapter; @@ -37,7 +38,6 @@ import org.springframework.core.annotation.AnnotationAwareOrderComparator; import org.springframework.http.MediaType; import org.springframework.ui.Model; import org.springframework.util.Assert; -import org.springframework.util.ClassUtils; import org.springframework.util.StringUtils; import org.springframework.validation.BindingResult; import org.springframework.web.bind.annotation.ModelAttribute; @@ -235,7 +235,7 @@ public class ViewResolutionResultHandler extends HandlerResultHandlerSupport viewsMono = Mono.just(Collections.singletonList((View) returnValue)); } else { - String name = getNameForReturnValue(clazz, parameter); + String name = getNameForReturnValue(parameter); model.addAttribute(name, returnValue); viewsMono = resolveViews(getDefaultViewName(exchange), locale); } @@ -275,13 +275,12 @@ public class ViewResolutionResultHandler extends HandlerResultHandlerSupport }); } - private String getNameForReturnValue(Class returnValueType, MethodParameter returnType) { + private String getNameForReturnValue(MethodParameter returnType) { ModelAttribute annotation = returnType.getMethodAnnotation(ModelAttribute.class); if (annotation != null && StringUtils.hasText(annotation.value())) { return annotation.value(); } - // TODO: Conventions does not deal with async wrappers - return ClassUtils.getShortNameAsProperty(returnValueType); + return Conventions.getVariableNameForParameter(returnType); } private void updateBindingContext(BindingContext context, ServerWebExchange exchange) { 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 6a1998b3089..381f13a16d2 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 @@ -42,7 +42,11 @@ import org.springframework.web.method.ResolvableMethod; import org.springframework.web.reactive.BindingContext; import org.springframework.web.server.ServerWebExchange; -import static org.junit.Assert.*; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertTrue; /** * Unit tests for {@link ModelAttributeMethodArgumentResolver}. @@ -105,7 +109,7 @@ public class ModelAttributeMethodArgumentResolverTests { @Test public void createAndBind() throws Exception { - testBindFoo(this.testMethod.annotPresent(ModelAttribute.class).arg(Foo.class), value -> { + testBindFoo("foo", this.testMethod.annotPresent(ModelAttribute.class).arg(Foo.class), value -> { assertEquals(Foo.class, value.getClass()); return (Foo) value; }); @@ -116,7 +120,7 @@ public class ModelAttributeMethodArgumentResolverTests { MethodParameter parameter = this.testMethod .annotNotPresent(ModelAttribute.class).arg(Mono.class, Foo.class); - testBindFoo(parameter, mono -> { + testBindFoo("fooMono", parameter, mono -> { assertTrue(mono.getClass().getName(), mono instanceof Mono); Object value = ((Mono) mono).block(Duration.ofSeconds(5)); assertEquals(Foo.class, value.getClass()); @@ -129,7 +133,7 @@ public class ModelAttributeMethodArgumentResolverTests { MethodParameter parameter = this.testMethod .annotPresent(ModelAttribute.class).arg(Single.class, Foo.class); - testBindFoo(parameter, single -> { + testBindFoo("fooSingle", parameter, single -> { assertTrue(single.getClass().getName(), single instanceof Single); Object value = ((Single) single).toBlocking().value(); assertEquals(Foo.class, value.getClass()); @@ -144,7 +148,7 @@ public class ModelAttributeMethodArgumentResolverTests { this.bindContext.getModel().addAttribute(foo); MethodParameter parameter = this.testMethod.annotNotPresent(ModelAttribute.class).arg(Foo.class); - testBindFoo(parameter, value -> { + testBindFoo("foo", parameter, value -> { assertEquals(Foo.class, value.getClass()); return (Foo) value; }); @@ -156,10 +160,10 @@ public class ModelAttributeMethodArgumentResolverTests { public void bindExistingMono() throws Exception { Foo foo = new Foo(); foo.setName("Jim"); - this.bindContext.getModel().addAttribute("foo", Mono.just(foo)); + this.bindContext.getModel().addAttribute("fooMono", Mono.just(foo)); MethodParameter parameter = this.testMethod.annotNotPresent(ModelAttribute.class).arg(Foo.class); - testBindFoo(parameter, value -> { + testBindFoo("foo", parameter, value -> { assertEquals(Foo.class, value.getClass()); return (Foo) value; }); @@ -171,10 +175,10 @@ public class ModelAttributeMethodArgumentResolverTests { public void bindExistingSingle() throws Exception { Foo foo = new Foo(); foo.setName("Jim"); - this.bindContext.getModel().addAttribute("foo", Single.just(foo)); + this.bindContext.getModel().addAttribute("fooSingle", Single.just(foo)); MethodParameter parameter = this.testMethod.annotNotPresent(ModelAttribute.class).arg(Foo.class); - testBindFoo(parameter, value -> { + testBindFoo("foo", parameter, value -> { assertEquals(Foo.class, value.getClass()); return (Foo) value; }); @@ -186,12 +190,13 @@ public class ModelAttributeMethodArgumentResolverTests { public void bindExistingMonoToMono() throws Exception { Foo foo = new Foo(); foo.setName("Jim"); - this.bindContext.getModel().addAttribute("foo", Mono.just(foo)); + String modelKey = "fooMono"; + this.bindContext.getModel().addAttribute(modelKey, Mono.just(foo)); MethodParameter parameter = this.testMethod .annotNotPresent(ModelAttribute.class).arg(Mono.class, Foo.class); - testBindFoo(parameter, mono -> { + testBindFoo(modelKey, parameter, mono -> { assertTrue(mono.getClass().getName(), mono instanceof Mono); Object value = ((Mono) mono).block(Duration.ofSeconds(5)); assertEquals(Foo.class, value.getClass()); @@ -199,7 +204,9 @@ public class ModelAttributeMethodArgumentResolverTests { }); } - private void testBindFoo(MethodParameter param, Function valueExtractor) throws Exception { + private void testBindFoo(String modelKey, MethodParameter param, Function valueExtractor) + throws Exception { + Object value = createResolver() .resolveArgument(param, this.bindContext, postForm("name=Robert&age=25")) .block(Duration.ZERO); @@ -208,12 +215,11 @@ public class ModelAttributeMethodArgumentResolverTests { assertEquals("Robert", foo.getName()); assertEquals(25, foo.getAge()); - String key = "foo"; - String bindingResultKey = BindingResult.MODEL_KEY_PREFIX + key; + String bindingResultKey = BindingResult.MODEL_KEY_PREFIX + modelKey; Map map = bindContext.getModel().asMap(); assertEquals(map.toString(), 2, map.size()); - assertSame(foo, map.get(key)); + assertSame(foo, map.get(modelKey)); assertNotNull(map.get(bindingResultKey)); assertTrue(map.get(bindingResultKey) instanceof BindingResult); }