Map arg resolver backs out if annotations present

Closes gh-21874

Author: rstoyanchev

spring-web/src/main/java/org/springframework/web/method/annotation/MapMethodProcessor.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2018 the original author or authors.
+ * Copyright 2002-2019 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.
@@ -32,8 +32,8 @@
  *
  * <p>A Map return value can be interpreted in more than one ways depending
  * on the presence of annotations like {@code @ModelAttribute} or
- * {@code @ResponseBody}. Therefore this handler should be configured after
- * the handlers that support these annotations.
+ * {@code @ResponseBody}. As of 5.2 this resolver returns false if the
+ * parameter is annotated.
  *
  * @author Rossen Stoyanchev
  * @since 3.1
@@ -42,7 +42,8 @@ public class MapMethodProcessor implements HandlerMethodArgumentResolver, Handle
 
 	@Override
 	public boolean supportsParameter(MethodParameter parameter) {
-		return Map.class.isAssignableFrom(parameter.getParameterType());
+		return Map.class.isAssignableFrom(parameter.getParameterType()) &&
+				parameter.getParameterAnnotations().length == 0;
 	}
 
 	@Override

spring-web/src/test/java/org/springframework/web/method/annotation/MapMethodProcessorTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2018 the original author or authors.
+ * Copyright 2002-2019 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.method.annotation;
 
-import java.lang.reflect.Method;
 import java.util.Map;
 
 import org.junit.Before;
@@ -25,14 +24,18 @@
 import org.springframework.core.MethodParameter;
 import org.springframework.mock.web.test.MockHttpServletRequest;
 import org.springframework.ui.ModelMap;
+import org.springframework.web.bind.annotation.RequestBody;
+import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.context.request.NativeWebRequest;
 import org.springframework.web.context.request.ServletWebRequest;
+import org.springframework.web.method.ResolvableMethod;
 import org.springframework.web.method.support.ModelAndViewContainer;
 
 import static org.junit.Assert.*;
 
 /**
- * Test fixture with {@link org.springframework.web.method.annotation.MapMethodProcessor}.
+ * Test fixture with
+ * {@link org.springframework.web.method.annotation.MapMethodProcessor}.
  *
  * @author Rossen Stoyanchev
  */
@@ -42,52 +45,59 @@ public class MapMethodProcessorTests {
 
 	private ModelAndViewContainer mavContainer;
 
-	private MethodParameter paramMap;
+	private NativeWebRequest webRequest;
 
-	private MethodParameter returnParamMap;
+	private final ResolvableMethod resolvable =
+			ResolvableMethod.on(getClass()).annotPresent(RequestMapping.class).build();
 
-	private NativeWebRequest webRequest;
 
 	@Before
 	public void setUp() throws Exception {
-		processor = new MapMethodProcessor();
-		mavContainer = new ModelAndViewContainer();
-
-		Method method = getClass().getDeclaredMethod("map", Map.class);
-		paramMap = new MethodParameter(method, 0);
-		returnParamMap = new MethodParameter(method, 0);
-
-		webRequest = new ServletWebRequest(new MockHttpServletRequest());
+		this.processor = new MapMethodProcessor();
+		this.mavContainer = new ModelAndViewContainer();
+		this.webRequest = new ServletWebRequest(new MockHttpServletRequest());
 	}
 
+
 	@Test
 	public void supportsParameter() {
-		assertTrue(processor.supportsParameter(paramMap));
+		assertTrue(this.processor.supportsParameter(
+				this.resolvable.annotNotPresent().arg(Map.class, String.class, Object.class)));
+		assertFalse(this.processor.supportsParameter(
+				this.resolvable.annotPresent(RequestBody.class).arg(Map.class, String.class, Object.class)));
 	}
 
 	@Test
 	public void supportsReturnType() {
-		assertTrue(processor.supportsReturnType(returnParamMap));
+		assertTrue(this.processor.supportsReturnType(this.resolvable.returnType()));
 	}
 
 	@Test
 	public void resolveArgumentValue() throws Exception {
-		assertSame(mavContainer.getModel(), processor.resolveArgument(paramMap, mavContainer, webRequest, null));
+		MethodParameter param = this.resolvable.annotNotPresent().arg(Map.class, String.class, Object.class);
+		assertSame(this.mavContainer.getModel(),
+				this.processor.resolveArgument(param, this.mavContainer, this.webRequest, null));
 	}
 
 	@Test
 	public void handleMapReturnValue() throws Exception {
-		mavContainer.addAttribute("attr1", "value1");
+		this.mavContainer.addAttribute("attr1", "value1");
 		Map<String, Object> returnValue = new ModelMap("attr2", "value2");
 
-		processor.handleReturnValue(returnValue , returnParamMap, mavContainer, webRequest);
+		this.processor.handleReturnValue(
+				returnValue , this.resolvable.returnType(), this.mavContainer, this.webRequest);
 
 		assertEquals("value1", mavContainer.getModel().get("attr1"));
 		assertEquals("value2", mavContainer.getModel().get("attr2"));
 	}
 
+
 	@SuppressWarnings("unused")
-	private Map<String, Object> map(Map<String, Object> map) {
+	@RequestMapping
+	private Map<String, Object> handle(
+			Map<String, Object> map,
+			@RequestBody Map<String, Object> annotMap) {
+
 		return null;
 	}
 

spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/ModelArgumentResolver.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2018 the original author or authors.
+ * Copyright 2002-2019 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.
@@ -30,6 +30,11 @@
  * Resolver for a controller method argument of type {@link Model} that can
  * also be resolved as a {@link java.util.Map}.
  *
+ * <p>A Map return value can be interpreted in more than one ways depending
+ * on the presence of annotations like {@code @ModelAttribute} or
+ * {@code @ResponseBody}. As of 5.2 this resolver returns false if a
+ * parameter of type {@code Map} is also annotated.
+ *
  * @author Rossen Stoyanchev
  * @since 5.0
  */
@@ -42,9 +47,10 @@ public ModelArgumentResolver(ReactiveAdapterRegistry adapterRegistry) {
 
 
 	@Override
-	public boolean supportsParameter(MethodParameter parameter) {
-		return checkParameterTypeNoReactiveWrapper(parameter,
-				type -> Model.class.isAssignableFrom(type) || Map.class.isAssignableFrom(type));
+	public boolean supportsParameter(MethodParameter param) {
+		return checkParameterTypeNoReactiveWrapper(param, type ->
+				Model.class.isAssignableFrom(type) ||
+						(Map.class.isAssignableFrom(type) && param.getParameterAnnotations().length == 0));
 	}
 
 	@Override

spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/ModelArgumentResolverTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2019 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.
@@ -25,14 +25,13 @@
 import org.springframework.mock.web.test.server.MockServerWebExchange;
 import org.springframework.ui.Model;
 import org.springframework.ui.ModelMap;
+import org.springframework.web.bind.annotation.RequestBody;
 import org.springframework.web.method.ResolvableMethod;
 import org.springframework.web.reactive.BindingContext;
 import org.springframework.web.server.ServerWebExchange;
 
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertSame;
-import static org.junit.Assert.assertTrue;
-import static org.springframework.mock.http.server.reactive.test.MockServerHttpRequest.get;
+import static org.junit.Assert.*;
+import static org.springframework.mock.http.server.reactive.test.MockServerHttpRequest.*;
 
 /**
  * Unit tests for {@link ModelArgumentResolver}.
@@ -45,22 +44,27 @@ public class ModelArgumentResolverTests {
 
 	private final ServerWebExchange exchange = MockServerWebExchange.from(get("/"));
 
-	private final ResolvableMethod testMethod = ResolvableMethod.on(getClass()).named("handle").build();
+	private final ResolvableMethod resolvable = ResolvableMethod.on(getClass()).named("handle").build();
 
 
 	@Test
-	public void supportsParameter() throws Exception {
-		assertTrue(this.resolver.supportsParameter(this.testMethod.arg(Model.class)));
-		assertTrue(this.resolver.supportsParameter(this.testMethod.arg(Map.class, String.class, Object.class)));
-		assertTrue(this.resolver.supportsParameter(this.testMethod.arg(ModelMap.class)));
-		assertFalse(this.resolver.supportsParameter(this.testMethod.arg(Object.class)));
+	public void supportsParameter() {
+
+		assertTrue(this.resolver.supportsParameter(this.resolvable.arg(Model.class)));
+		assertTrue(this.resolver.supportsParameter(this.resolvable.arg(ModelMap.class)));
+		assertTrue(this.resolver.supportsParameter(
+				this.resolvable.annotNotPresent().arg(Map.class, String.class, Object.class)));
+
+		assertFalse(this.resolver.supportsParameter(this.resolvable.arg(Object.class)));
+		assertFalse(this.resolver.supportsParameter(
+				this.resolvable.annotPresent(RequestBody.class).arg(Map.class, String.class, Object.class)));
 	}
 
 	@Test
-	public void resolveArgument() throws Exception {
-		testResolveArgument(this.testMethod.arg(Model.class));
-		testResolveArgument(this.testMethod.arg(Map.class, String.class, Object.class));
-		testResolveArgument(this.testMethod.arg(ModelMap.class));
+	public void resolveArgument() {
+		testResolveArgument(this.resolvable.arg(Model.class));
+		testResolveArgument(this.resolvable.annotNotPresent().arg(Map.class, String.class, Object.class));
+		testResolveArgument(this.resolvable.arg(ModelMap.class));
 	}
 
 	private void testResolveArgument(MethodParameter parameter) {
@@ -69,7 +73,13 @@ private void testResolveArgument(MethodParameter parameter) {
 		assertSame(context.getModel(), result);
 	}
 
+
 	@SuppressWarnings("unused")
-	void handle(Model model, Map<String, Object> map, ModelMap modelMap, Object object) {}
+	void handle(
+			Model model,
+			Map<String, Object> map,
+			@RequestBody Map<String, Object> annotatedMap,
+			ModelMap modelMap,
+			Object object) {}
 
 }