From 1cf5264163a64b5cee67ad60cff259bc070dfbd5 Mon Sep 17 00:00:00 2001 From: Sam Brannen <104798+sbrannen@users.noreply.github.com> Date: Thu, 27 Jun 2024 10:46:00 +0200 Subject: [PATCH 1/2] Polishing --- .../pages/web/webflux/controller/ann-methods/arguments.adoc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/framework-docs/modules/ROOT/pages/web/webflux/controller/ann-methods/arguments.adoc b/framework-docs/modules/ROOT/pages/web/webflux/controller/ann-methods/arguments.adoc index 4c0215ad40..c342e96963 100644 --- a/framework-docs/modules/ROOT/pages/web/webflux/controller/ann-methods/arguments.adoc +++ b/framework-docs/modules/ROOT/pages/web/webflux/controller/ann-methods/arguments.adoc @@ -89,9 +89,9 @@ and others) and is equivalent to `required=false`. Note that use of `@ModelAttribute` is optional -- for example, to set its attributes. See "`Any other argument`" later in this table. -| `Errors`, `BindingResult` +| `Errors` or `BindingResult` | For access to errors from validation and data binding for a command object, i.e. a - `@ModelAttribute` argument. An `Errors`, or `BindingResult` argument must be declared + `@ModelAttribute` argument. An `Errors` or `BindingResult` argument must be declared immediately after the validated method argument. | `SessionStatus` + class-level `@SessionAttributes` From 8b11ee9ee2f49f0014215ff60117be82585addc2 Mon Sep 17 00:00:00 2001 From: Sam Brannen <104798+sbrannen@users.noreply.github.com> Date: Thu, 27 Jun 2024 11:33:50 +0200 Subject: [PATCH 2/2] Document that ModelMap is not a supported argument type in WebFlux Prior to this commit, the "Method Arguments" documentation for WebFlux in the reference manual stated that WebFlux controller methods can accept arguments of type Map, Model, or ModelMap to access the model. However, ModelMap is actually not supported and results in exception due to a type mismatch. This commit updates the documentation to reflect this. In addition, this commit updates related Javadoc and tests to avoid mentioning or using ModelMap in WebFlux. Closes gh-33107 --- .../controller/ann-methods/arguments.adoc | 2 +- .../springframework/ui/ConcurrentModel.java | 8 +-- .../org/springframework/ui/package-info.java | 2 +- .../SessionAttributesHandlerTests.java | 15 +++--- .../result/view/DummyMacroRequestContext.java | 7 ++- .../view/HttpMessageWriterViewTests.java | 49 +++++++++---------- .../view/freemarker/FreeMarkerMacroTests.java | 4 +- .../view/freemarker/FreeMarkerViewTests.java | 11 ++--- 8 files changed, 45 insertions(+), 53 deletions(-) diff --git a/framework-docs/modules/ROOT/pages/web/webflux/controller/ann-methods/arguments.adoc b/framework-docs/modules/ROOT/pages/web/webflux/controller/ann-methods/arguments.adoc index c342e96963..37d297afe3 100644 --- a/framework-docs/modules/ROOT/pages/web/webflux/controller/ann-methods/arguments.adoc +++ b/framework-docs/modules/ROOT/pages/web/webflux/controller/ann-methods/arguments.adoc @@ -77,7 +77,7 @@ and others) and is equivalent to `required=false`. | For access to a part in a `multipart/form-data` request. Supports reactive types. See xref:web/webflux/controller/ann-methods/multipart-forms.adoc[Multipart Content] and xref:web/webflux/reactive-spring.adoc#webflux-multipart[Multipart Data]. -| `java.util.Map`, `org.springframework.ui.Model`, and `org.springframework.ui.ModelMap`. +| `java.util.Map` or `org.springframework.ui.Model` | For access to the model that is used in HTML controllers and is exposed to templates as part of view rendering. diff --git a/spring-context/src/main/java/org/springframework/ui/ConcurrentModel.java b/spring-context/src/main/java/org/springframework/ui/ConcurrentModel.java index 0d2b6afaef..25e3d4d7b0 100644 --- a/spring-context/src/main/java/org/springframework/ui/ConcurrentModel.java +++ b/spring-context/src/main/java/org/springframework/ui/ConcurrentModel.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. @@ -46,7 +46,7 @@ public class ConcurrentModel extends ConcurrentHashMap implement } /** - * Construct a new {@code ModelMap} containing the supplied attribute + * Construct a new {@code ConcurrentModel} containing the supplied attribute * under the supplied name. * @see #addAttribute(String, Object) */ @@ -55,8 +55,8 @@ public class ConcurrentModel extends ConcurrentHashMap implement } /** - * Construct a new {@code ModelMap} containing the supplied attribute. - * Uses attribute name generation to generate the key for the supplied model + * Construct a new {@code ConcurrentModel} containing the supplied attribute. + *

Uses attribute name generation to generate the key for the supplied model * object. * @see #addAttribute(Object) */ diff --git a/spring-context/src/main/java/org/springframework/ui/package-info.java b/spring-context/src/main/java/org/springframework/ui/package-info.java index 988880e3b9..96269d532a 100644 --- a/spring-context/src/main/java/org/springframework/ui/package-info.java +++ b/spring-context/src/main/java/org/springframework/ui/package-info.java @@ -1,6 +1,6 @@ /** * Generic support for UI layer concepts. - * Provides a generic ModelMap for model holding. + *

Provides generic {@code Model} and {@code ModelMap} holders for model attributes. */ @NonNullApi @NonNullFields diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/SessionAttributesHandlerTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/SessionAttributesHandlerTests.java index ca5e52d659..eaec0ff6b2 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/SessionAttributesHandlerTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/SessionAttributesHandlerTests.java @@ -18,11 +18,11 @@ package org.springframework.web.reactive.result.method.annotation; import java.util.HashSet; +import java.util.Map; import org.junit.jupiter.api.Test; import org.springframework.beans.testfixture.beans.TestBean; -import org.springframework.ui.ModelMap; import org.springframework.web.bind.annotation.SessionAttributes; import org.springframework.web.server.WebSession; import org.springframework.web.testfixture.server.MockWebSession; @@ -31,7 +31,8 @@ import static java.util.Arrays.asList; import static org.assertj.core.api.Assertions.assertThat; /** - * Test fixture with {@link SessionAttributesHandler}. + * Tests for {@link SessionAttributesHandler}. + * * @author Rossen Stoyanchev */ class SessionAttributesHandlerTests { @@ -86,11 +87,11 @@ class SessionAttributesHandlerTests { @Test void storeAttributes() { - - ModelMap model = new ModelMap(); - model.put("attr1", "value1"); - model.put("attr2", "value2"); - model.put("attr3", new TestBean()); + Map model = Map.of( + "attr1", "value1", + "attr2", "value2", + "attr3", new TestBean() + ); WebSession session = new MockWebSession(); sessionAttributesHandler.storeAttributes(session, model); diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/result/view/DummyMacroRequestContext.java b/spring-webflux/src/test/java/org/springframework/web/reactive/result/view/DummyMacroRequestContext.java index d7e91cdbcc..fc9a2edab7 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/result/view/DummyMacroRequestContext.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/result/view/DummyMacroRequestContext.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. @@ -20,7 +20,6 @@ import java.util.List; import java.util.Map; import org.springframework.context.support.GenericApplicationContext; -import org.springframework.ui.ModelMap; import org.springframework.web.server.ServerWebExchange; import org.springframework.web.util.UriComponents; import org.springframework.web.util.UriComponentsBuilder; @@ -38,7 +37,7 @@ public class DummyMacroRequestContext { private final ServerWebExchange exchange; - private final ModelMap model; + private final Map model; private final GenericApplicationContext context; @@ -46,7 +45,7 @@ public class DummyMacroRequestContext { private String contextPath; - public DummyMacroRequestContext(ServerWebExchange exchange, ModelMap model, GenericApplicationContext context) { + public DummyMacroRequestContext(ServerWebExchange exchange, Map model, GenericApplicationContext context) { this.exchange = exchange; this.model = model; this.context = context; diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/result/view/HttpMessageWriterViewTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/result/view/HttpMessageWriterViewTests.java index 415ec7a6f9..18222a1aa8 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/result/view/HttpMessageWriterViewTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/result/view/HttpMessageWriterViewTests.java @@ -17,11 +17,10 @@ package org.springframework.web.reactive.result.view; import java.time.Duration; -import java.util.Arrays; -import java.util.Collections; -import java.util.HashSet; +import java.util.HashMap; import java.util.LinkedHashMap; import java.util.Map; +import java.util.Set; import org.junit.jupiter.api.Test; import reactor.test.StepVerifier; @@ -30,8 +29,6 @@ import org.springframework.core.codec.CharSequenceEncoder; import org.springframework.http.MediaType; import org.springframework.http.codec.json.Jackson2JsonEncoder; import org.springframework.http.codec.xml.Jaxb2XmlEncoder; -import org.springframework.ui.ExtendedModelMap; -import org.springframework.ui.ModelMap; import org.springframework.web.testfixture.http.server.reactive.MockServerHttpRequest; import org.springframework.web.testfixture.server.MockServerWebExchange; @@ -48,7 +45,7 @@ class HttpMessageWriterViewTests { private HttpMessageWriterView view = new HttpMessageWriterView(new Jackson2JsonEncoder()); - private final ModelMap model = new ExtendedModelMap(); + private final Map model = new HashMap<>(); private final MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/")); @@ -63,18 +60,18 @@ class HttpMessageWriterViewTests { @Test void singleMatch() throws Exception { - this.view.setModelKeys(Collections.singleton("foo2")); - this.model.addAttribute("foo1", Collections.singleton("bar1")); - this.model.addAttribute("foo2", Collections.singleton("bar2")); - this.model.addAttribute("foo3", Collections.singleton("bar3")); + this.view.setModelKeys(Set.of("foo2")); + this.model.put("foo1", Set.of("bar1")); + this.model.put("foo2", Set.of("bar2")); + this.model.put("foo3", Set.of("bar3")); assertThat(doRender()).isEqualTo("[\"bar2\"]"); } @Test void noMatch() throws Exception { - this.view.setModelKeys(Collections.singleton("foo2")); - this.model.addAttribute("foo1", "bar1"); + this.view.setModelKeys(Set.of("foo2")); + this.model.put("foo1", "bar1"); assertThat(doRender()).isEmpty(); } @@ -82,18 +79,18 @@ class HttpMessageWriterViewTests { @Test void noMatchBecauseNotSupported() throws Exception { this.view = new HttpMessageWriterView(new Jaxb2XmlEncoder()); - this.view.setModelKeys(new HashSet<>(Collections.singletonList("foo1"))); - this.model.addAttribute("foo1", "bar1"); + this.view.setModelKeys(Set.of("foo1")); + this.model.put("foo1", "bar1"); assertThat(doRender()).isEmpty(); } @Test void multipleMatches() throws Exception { - this.view.setModelKeys(new HashSet<>(Arrays.asList("foo1", "foo2"))); - this.model.addAttribute("foo1", Collections.singleton("bar1")); - this.model.addAttribute("foo2", Collections.singleton("bar2")); - this.model.addAttribute("foo3", Collections.singleton("bar3")); + this.view.setModelKeys(Set.of("foo1", "foo2")); + this.model.put("foo1", Set.of("bar1")); + this.model.put("foo2", Set.of("bar2")); + this.model.put("foo3", Set.of("bar3")); assertThat(doRender()).isEqualTo("{\"foo1\":[\"bar1\"],\"foo2\":[\"bar2\"]}"); } @@ -101,13 +98,13 @@ class HttpMessageWriterViewTests { @Test void multipleMatchesNotSupported() throws Exception { this.view = new HttpMessageWriterView(CharSequenceEncoder.allMimeTypes()); - this.view.setModelKeys(new HashSet<>(Arrays.asList("foo1", "foo2"))); - this.model.addAttribute("foo1", "bar1"); - this.model.addAttribute("foo2", "bar2"); + this.view.setModelKeys(Set.of("foo1", "foo2")); + this.model.put("foo1", "bar1"); + this.model.put("foo2", "bar2"); - assertThatIllegalStateException().isThrownBy( - this::doRender) - .withMessageContaining("Map rendering is not supported"); + assertThatIllegalStateException() + .isThrownBy(this::doRender) + .withMessageContaining("Map rendering is not supported"); } @Test @@ -115,8 +112,8 @@ class HttpMessageWriterViewTests { Map pojoData = new LinkedHashMap<>(); pojoData.put("foo", "f"); pojoData.put("bar", "b"); - this.model.addAttribute("pojoData", pojoData); - this.view.setModelKeys(Collections.singleton("pojoData")); + this.model.put("pojoData", pojoData); + this.view.setModelKeys(Set.of("pojoData")); this.view.render(this.model, MediaType.APPLICATION_JSON, exchange).block(Duration.ZERO); diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/result/view/freemarker/FreeMarkerMacroTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/result/view/freemarker/FreeMarkerMacroTests.java index bceadaf45c..8a647bdb1e 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/result/view/freemarker/FreeMarkerMacroTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/result/view/freemarker/FreeMarkerMacroTests.java @@ -37,8 +37,6 @@ import org.springframework.beans.testfixture.beans.TestBean; import org.springframework.context.support.GenericApplicationContext; import org.springframework.core.io.ClassPathResource; import org.springframework.http.MediaType; -import org.springframework.ui.ExtendedModelMap; -import org.springframework.ui.ModelMap; import org.springframework.util.FileCopyUtils; import org.springframework.util.StringUtils; import org.springframework.web.reactive.result.view.BindStatus; @@ -322,7 +320,7 @@ class FreeMarkerMacroTests { names.put("Fred", "Fred Bloggs"); names.put("Rob&Harrop", "Rob Harrop"); - ModelMap model = new ExtendedModelMap(); + Map model = new HashMap<>(); DummyMacroRequestContext rc = new DummyMacroRequestContext(this.exchange, model, this.applicationContext); rc.setMessageMap(msgMap); diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/result/view/freemarker/FreeMarkerViewTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/result/view/freemarker/FreeMarkerViewTests.java index d57a7769cb..32926929cc 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/result/view/freemarker/FreeMarkerViewTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/result/view/freemarker/FreeMarkerViewTests.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. @@ -19,6 +19,7 @@ package org.springframework.web.reactive.result.view.freemarker; import java.nio.charset.StandardCharsets; import java.time.Duration; import java.util.Locale; +import java.util.Map; import freemarker.template.Configuration; import org.junit.jupiter.api.BeforeEach; @@ -28,8 +29,6 @@ import reactor.test.StepVerifier; import org.springframework.context.ApplicationContextException; import org.springframework.context.support.GenericApplicationContext; import org.springframework.http.codec.ServerCodecConfigurer; -import org.springframework.ui.ExtendedModelMap; -import org.springframework.ui.ModelMap; import org.springframework.web.reactive.result.view.ZeroDemandResponse; import org.springframework.web.server.ServerWebExchange; import org.springframework.web.server.adapter.DefaultServerWebExchange; @@ -125,8 +124,7 @@ class FreeMarkerViewTests { freeMarkerView.setConfiguration(this.freeMarkerConfig); freeMarkerView.setUrl("test.ftl"); - ModelMap model = new ExtendedModelMap(); - model.addAttribute("hello", "hi FreeMarker"); + Map model = Map.of("hello", "hi FreeMarker"); freeMarkerView.render(model, null, this.exchange).block(Duration.ofMillis(5000)); StepVerifier.create(this.exchange.getResponse().getBody()) @@ -148,8 +146,7 @@ class FreeMarkerViewTests { freeMarkerView.setConfiguration(this.freeMarkerConfig); freeMarkerView.setUrl("test.ftl"); - ModelMap model = new ExtendedModelMap(); - model.addAttribute("hello", "hi FreeMarker"); + Map model = Map.of("hello", "hi FreeMarker"); freeMarkerView.render(model, null, exchange).subscribe(); response.cancelWrite();