Reject ModelMap argument types in WebFlux

Prior to this commit, if ModelMap was used as an argument type in a
WebFlux controller method, the user encountered an exception similar to
the following.

java.lang.IllegalStateException: argument type mismatch
  Controller [example.SampleController]
  Method [java.lang.String example.SampleController.index(org.springframework.ui.ModelMap)] with argument values:
  [0] [type=org.springframework.validation.support.BindingAwareConcurrentModel] [value={}]

However, the above error message is a bit cryptic since the error
occurs while attempting to invoke the controller method with an
instance of BindingAwareConcurrentModel which is not compatible with
ModelMap. More importantly, this error message does not explicitly
convey to the user that a ModelMap is not supported.

This commit improve the diagnostics for the user in such scenarios by
rejecting the use of ModelMap upfront in WebFlux.

Consequently, for the same use case as above, the user now encounters
an exception similar to the following.

java.lang.IllegalStateException:
  Could not resolve parameter [0] in
  java.lang.String example.SampleController.index(org.springframework.ui.ModelMap):
  No suitable resolver

Closes gh-33109

Author: sbrannen

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2023 the original author or authors.
+ * Copyright 2002-2024 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.
@@ -31,6 +31,7 @@
 import org.springframework.core.ResolvableType;
 import org.springframework.lang.Nullable;
 import org.springframework.ui.Model;
+import org.springframework.ui.ModelMap;
 import org.springframework.util.Assert;
 import org.springframework.util.ClassUtils;
 import org.springframework.validation.BindingResult;
@@ -53,9 +54,10 @@
  * {@code @jakarta.validation.Valid} or Spring's own
  * {@code @org.springframework.validation.annotation.Validated}.
  *
- * <p>When this handler is created with {@code useDefaultResolution=true}
- * any non-simple type argument and return value is regarded as a model
- * attribute with or without the presence of an {@code @ModelAttribute}.
+ * <p>When this handler is created with {@code useDefaultResolution=true} any
+ * non-simple type argument or return value (excluding those of type
+ * {@link ModelMap}) is regarded as a model attribute with or without the
+ * presence of {@code @ModelAttribute}.
  *
  * @author Rossen Stoyanchev
  * @author Juergen Hoeller
@@ -88,7 +90,8 @@ public boolean supportsParameter(MethodParameter parameter) {
 			return true;
 		}
 		else if (this.useDefaultResolution) {
-			return checkParameterType(parameter, type -> !BeanUtils.isSimpleProperty(type));
+			return checkParameterType(parameter, type ->
+					!(ModelMap.class.isAssignableFrom(type) || BeanUtils.isSimpleProperty(type)));
 		}
 		return false;
 	}

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2019 the original author or authors.
+ * Copyright 2002-2024 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.
@@ -21,6 +21,7 @@
 import org.springframework.core.MethodParameter;
 import org.springframework.core.ReactiveAdapterRegistry;
 import org.springframework.ui.Model;
+import org.springframework.ui.ModelMap;
 import org.springframework.web.reactive.BindingContext;
 import org.springframework.web.reactive.result.method.HandlerMethodArgumentResolverSupport;
 import org.springframework.web.reactive.result.method.SyncHandlerMethodArgumentResolver;
@@ -30,12 +31,16 @@
  * 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 way depending
+ * <p>A {@code Map} return value can be interpreted in more than one way 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.
+ * {@code @ResponseBody}.
+ *
+ * <p>As of 5.2 this resolver returns {@code false} if a parameter of type
+ * {@code Map} is also annotated. As of 6.2 this resolver returns {@code false}
+ * for a parameter of type {@link ModelMap}.
  *
  * @author Rossen Stoyanchev
+ * @author Sam Brannen
  * @since 5.2
  */
 public class ModelMethodArgumentResolver extends HandlerMethodArgumentResolverSupport
@@ -48,9 +53,9 @@ public ModelMethodArgumentResolver(ReactiveAdapterRegistry adapterRegistry) {
 
 	@Override
 	public boolean supportsParameter(MethodParameter param) {
-		return checkParameterTypeNoReactiveWrapper(param, type ->
-				Model.class.isAssignableFrom(type) ||
-						(Map.class.isAssignableFrom(type) && param.getParameterAnnotations().length == 0));
+		return checkParameterTypeNoReactiveWrapper(param, type -> Model.class.isAssignableFrom(type) ||
+				(Map.class.isAssignableFrom(type) && !ModelMap.class.isAssignableFrom(type) &&
+					!param.hasParameterAnnotations()));
 	}
 
 	@Override
@@ -61,12 +66,12 @@ public Object resolveArgumentValue(
 		if (Model.class.isAssignableFrom(type)) {
 			return context.getModel();
 		}
-		else if (Map.class.isAssignableFrom(type)) {
+		else if (Map.class.isAssignableFrom(type) && !ModelMap.class.isAssignableFrom(type)) {
 			return context.getModel().asMap();
 		}
 		else {
 			// Should never happen..
-			throw new IllegalStateException("Unexpected method parameter type: " + type);
+			throw new IllegalStateException("Unexpected method parameter type: " + type.getName());
 		}
 	}
 

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

@@ -32,12 +32,14 @@
 import org.springframework.web.testfixture.server.MockServerWebExchange;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
 import static org.springframework.web.testfixture.http.server.reactive.MockServerHttpRequest.get;
 
 /**
  * Tests for {@link ModelMethodArgumentResolver}.
  *
  * @author Rossen Stoyanchev
+ * @author Sam Brannen
  */
 class ModelMethodArgumentResolverTests {
 
@@ -52,11 +54,11 @@ class ModelMethodArgumentResolverTests {
 	@Test
 	void supportsParameter() {
 		assertThat(this.resolver.supportsParameter(this.resolvable.arg(Model.class))).isTrue();
-		assertThat(this.resolver.supportsParameter(this.resolvable.arg(ModelMap.class))).isTrue();
 		assertThat(this.resolver.supportsParameter(
 				this.resolvable.annotNotPresent().arg(Map.class, String.class, Object.class))).isTrue();
 
 		assertThat(this.resolver.supportsParameter(this.resolvable.arg(Object.class))).isFalse();
+		assertThat(this.resolver.supportsParameter(this.resolvable.arg(ModelMap.class))).isFalse();
 		assertThat(this.resolver.supportsParameter(
 				this.resolvable.annotPresent(RequestBody.class).arg(Map.class, String.class, Object.class))).isFalse();
 	}
@@ -65,7 +67,13 @@ void supportsParameter() {
 	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));
+
+		assertThatIllegalStateException()
+				.isThrownBy(() -> testResolveArgument(this.resolvable.arg(Object.class)))
+				.withMessage("Unexpected method parameter type: " + Object.class.getName());
+		assertThatIllegalStateException()
+				.isThrownBy(() -> testResolveArgument(this.resolvable.arg(ModelMap.class)))
+				.withMessage("Unexpected method parameter type: " + ModelMap.class.getName());
 	}
 
 	private void testResolveArgument(MethodParameter parameter) {