ConcurrentModel.addAttribute(String, Object) ignores null value

Issue: SPR-17141

spring-context/src/main/java/org/springframework/ui/ConcurrentModel.java

@@ -68,13 +68,17 @@ public class ConcurrentModel extends ConcurrentHashMap<String, Object> implement
 	/**
 	 * Add the supplied attribute under the supplied name.
 	 * @param attributeName the name of the model attribute (never {@code null})
-	 * @param attributeValue the model attribute value (never {@code null} for {@code ConcurrentModel},
-	 * with the {@code Nullable} declaration inherited from {@link Model#addAttribute(String, Object)})
+	 * @param attributeValue the model attribute value (ignored if {@code null},
+	 * just removing an existing entry if any)
 	 */
 	public ConcurrentModel addAttribute(String attributeName, @Nullable Object attributeValue) {
 		Assert.notNull(attributeName, "Model attribute name must not be null");
-		Assert.notNull(attributeValue, "ConcurrentModel does not support null attribute value");
-		put(attributeName, attributeValue);
+		if (attributeValue != null) {
+			put(attributeName, attributeValue);
+		}
+		else {
+			remove(attributeName);
+		}
 		return this;
 	}
 

spring-webflux/src/test/java/org/springframework/web/reactive/result/view/ViewResolutionResultHandlerTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2018 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.reactive.result.view;
 
-import java.net.URISyntaxException;
 import java.nio.ByteBuffer;
 import java.time.Duration;
 import java.util.Arrays;
@@ -56,12 +55,12 @@ import org.springframework.web.reactive.accept.RequestedContentTypeResolver;
 import org.springframework.web.server.NotAcceptableStatusException;
 import org.springframework.web.server.ServerWebExchange;
 
-import static java.nio.charset.StandardCharsets.UTF_8;
-import static org.junit.Assert.assertEquals;
-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;
-import static org.springframework.web.method.ResolvableMethod.on;
+import static java.nio.charset.StandardCharsets.*;
+import static org.junit.Assert.*;
+import static org.mockito.Mockito.*;
+import static org.springframework.http.MediaType.*;
+import static org.springframework.mock.http.server.reactive.test.MockServerHttpRequest.*;
+import static org.springframework.web.method.ResolvableMethod.*;
 
 /**
  * ViewResolutionResultHandler relying on a canned {@link TestViewResolver}
@@ -75,8 +74,7 @@ public class ViewResolutionResultHandlerTests {
 
 
 	@Test
-	public void supports() throws Exception {
-
+	public void supports() {
 		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));
@@ -120,7 +118,7 @@ public class ViewResolutionResultHandlerTests {
 	}
 
 	@Test
-	public void viewResolverOrder() throws Exception {
+	public void viewResolverOrder() {
 		TestViewResolver resolver1 = new TestViewResolver("account");
 		TestViewResolver resolver2 = new TestViewResolver("profile");
 		resolver1.setOrder(2);
@@ -131,8 +129,7 @@ public class ViewResolutionResultHandlerTests {
 	}
 
 	@Test
-	public void handleReturnValueTypes() throws Exception {
-
+	public void handleReturnValueTypes() {
 		Object returnValue;
 		MethodParameter returnType;
 		ViewResolver resolver = new TestViewResolver("account");
@@ -158,7 +155,7 @@ public class ViewResolutionResultHandlerTests {
 		testHandle("/path", returnType, returnValue, "account: {id=123}", resolver);
 
 		returnType = on(Handler.class).resolveReturnType(Model.class);
-		returnValue = new ConcurrentModel().addAttribute("name", "Joe");
+		returnValue = new ConcurrentModel().addAttribute("name", "Joe").addAttribute("ignore", null);
 		testHandle("/account", returnType, returnValue, "account: {id=123, name=Joe}", resolver);
 
 		// Work around  caching issue...
@@ -196,7 +193,7 @@ public class ViewResolutionResultHandlerTests {
 	}
 
 	@Test
-	public void handleWithMultipleResolvers() throws Exception {
+	public void handleWithMultipleResolvers() {
 		testHandle("/account",
 				on(Handler.class).annotNotPresent(ModelAttribute.class).resolveReturnType(String.class),
 				"profile", "profile: {id=123}",
@@ -204,14 +201,14 @@ public class ViewResolutionResultHandlerTests {
 	}
 
 	@Test
-	public void defaultViewName() throws Exception {
+	public void defaultViewName() {
 		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 {
+	private void testDefaultViewName(Object returnValue, MethodParameter returnType) {
 		this.bindingContext.getModel().addAttribute("id", "123");
 		HandlerResult result = new HandlerResult(new Object(), returnValue, returnType, this.bindingContext);
 		ViewResolutionResultHandler handler = resultHandler(new TestViewResolver("account"));
@@ -230,7 +227,7 @@ public class ViewResolutionResultHandlerTests {
 	}
 
 	@Test
-	public void unresolvedViewName() throws Exception {
+	public void unresolvedViewName() {
 		String returnValue = "account";
 		MethodParameter returnType = on(Handler.class).annotPresent(ModelAttribute.class).resolveReturnType(String.class);
 		HandlerResult result = new HandlerResult(new Object(), returnValue, returnType, this.bindingContext);
@@ -245,7 +242,7 @@ public class ViewResolutionResultHandlerTests {
 	}
 
 	@Test
-	public void contentNegotiation() throws Exception {
+	public void contentNegotiation() {
 		TestBean value = new TestBean("Joe");
 		MethodParameter returnType = on(Handler.class).resolveReturnType(TestBean.class);
 		HandlerResult handlerResult = new HandlerResult(new Object(), value, returnType, this.bindingContext);
@@ -267,7 +264,7 @@ public class ViewResolutionResultHandlerTests {
 	}
 
 	@Test
-	public void contentNegotiationWith406() throws Exception {
+	public void contentNegotiationWith406() {
 		TestBean value = new TestBean("Joe");
 		MethodParameter returnType = on(Handler.class).resolveReturnType(TestBean.class);
 		HandlerResult handlerResult = new HandlerResult(new Object(), value, returnType, this.bindingContext);
@@ -282,9 +279,8 @@ public class ViewResolutionResultHandlerTests {
 				.verify();
 	}
 
-	@Test // SPR-15291
-	public void contentNegotiationWithRedirect() throws Exception {
-
+	@Test  // SPR-15291
+	public void contentNegotiationWithRedirect() {
 		HandlerResult handlerResult = new HandlerResult(new Object(), "redirect:/",
 				on(Handler.class).annotNotPresent(ModelAttribute.class).resolveReturnType(String.class),
 				this.bindingContext);
@@ -315,7 +311,7 @@ public class ViewResolutionResultHandlerTests {
 	}
 
 	private ServerWebExchange testHandle(String path, MethodParameter returnType, Object returnValue,
-			String responseBody, ViewResolver... resolvers) throws URISyntaxException {
+			String responseBody, ViewResolver... resolvers) {
 
 		Model model = this.bindingContext.getModel();
 		model.asMap().clear();