From 265299c790fcec55d2f3de12b91e43e2361523e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Nicoll?= Date: Thu, 18 Jul 2024 11:21:35 +0200 Subject: [PATCH] Restore source and binary compatibility in MockMvc Previous to this commit, MockHttpServletRequestBuilder was not binary compatible as its methods had moved to a parent class with a generic argument on the return type. MockMultipartHttpServletRequestBuilder was also not source compatible as it no longed extended from MockHttpServletRequestBuilder. Both these changes were introduced to allow the AssertJ support to expose builders that implement an extra AssertJ interface, without copying the features the builders provide. This commit restore compatibility. For MockHttpServletRequestBuilder we simply override all methods that returns the generic type into the resolved type. MockMultipartHttpServletRequestBuilder is more involved. Because we need to extend from MockHttpServletRequestBuilder, we have no other choice than duplicating the code. For now, the abstract builder for multipart is only used by the AssertJ support, but we can revisit this again in a major release. Closes gh-33229 --- .../MockHttpServletRequestBuilder.java | 186 ++++++++++++++++++ ...ockMultipartHttpServletRequestBuilder.java | 123 +++++++++++- .../request/MockMvcRequestBuilders.java | 16 +- ...ltipartHttpServletRequestBuilderTests.java | 12 +- 4 files changed, 328 insertions(+), 9 deletions(-) diff --git a/spring-test/src/main/java/org/springframework/test/web/servlet/request/MockHttpServletRequestBuilder.java b/spring-test/src/main/java/org/springframework/test/web/servlet/request/MockHttpServletRequestBuilder.java index f659b0e8ea..4ec50f72c0 100644 --- a/spring-test/src/main/java/org/springframework/test/web/servlet/request/MockHttpServletRequestBuilder.java +++ b/spring-test/src/main/java/org/springframework/test/web/servlet/request/MockHttpServletRequestBuilder.java @@ -16,9 +16,22 @@ package org.springframework.test.web.servlet.request; +import java.net.URI; +import java.nio.charset.Charset; +import java.security.Principal; +import java.util.Locale; +import java.util.Map; + +import jakarta.servlet.http.Cookie; + +import org.springframework.http.HttpHeaders; import org.springframework.http.HttpMethod; +import org.springframework.http.MediaType; +import org.springframework.lang.Nullable; import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.mock.web.MockHttpSession; import org.springframework.test.web.servlet.MockMvc; +import org.springframework.util.MultiValueMap; /** * Default builder for {@link MockHttpServletRequest} required as input to @@ -53,4 +66,177 @@ public class MockHttpServletRequestBuilder super(httpMethod); } + + // Override to keep binary compatibility. + + @Override + public MockHttpServletRequestBuilder uri(URI uri) { + return super.uri(uri); + } + + @Override + public MockHttpServletRequestBuilder uri(String uriTemplate, Object... uriVariables) { + return super.uri(uriTemplate, uriVariables); + } + + @Override + public MockHttpServletRequestBuilder contextPath(String contextPath) { + return super.contextPath(contextPath); + } + + @Override + public MockHttpServletRequestBuilder servletPath(String servletPath) { + return super.servletPath(servletPath); + } + + @Override + public MockHttpServletRequestBuilder pathInfo(@Nullable String pathInfo) { + return super.pathInfo(pathInfo); + } + + @Override + public MockHttpServletRequestBuilder secure(boolean secure) { + return super.secure(secure); + } + + @Override + public MockHttpServletRequestBuilder characterEncoding(Charset encoding) { + return super.characterEncoding(encoding); + } + + @Override + public MockHttpServletRequestBuilder characterEncoding(String encoding) { + return super.characterEncoding(encoding); + } + + @Override + public MockHttpServletRequestBuilder content(byte[] content) { + return super.content(content); + } + + @Override + public MockHttpServletRequestBuilder content(String content) { + return super.content(content); + } + + @Override + public MockHttpServletRequestBuilder contentType(MediaType contentType) { + return super.contentType(contentType); + } + + @Override + public MockHttpServletRequestBuilder contentType(String contentType) { + return super.contentType(contentType); + } + + @Override + public MockHttpServletRequestBuilder accept(MediaType... mediaTypes) { + return super.accept(mediaTypes); + } + + @Override + public MockHttpServletRequestBuilder accept(String... mediaTypes) { + return super.accept(mediaTypes); + } + + @Override + public MockHttpServletRequestBuilder header(String name, Object... values) { + return super.header(name, values); + } + + @Override + public MockHttpServletRequestBuilder headers(HttpHeaders httpHeaders) { + return super.headers(httpHeaders); + } + + @Override + public MockHttpServletRequestBuilder param(String name, String... values) { + return super.param(name, values); + } + + @Override + public MockHttpServletRequestBuilder params(MultiValueMap params) { + return super.params(params); + } + + @Override + public MockHttpServletRequestBuilder queryParam(String name, String... values) { + return super.queryParam(name, values); + } + + @Override + public MockHttpServletRequestBuilder queryParams(MultiValueMap params) { + return super.queryParams(params); + } + + @Override + public MockHttpServletRequestBuilder formField(String name, String... values) { + return super.formField(name, values); + } + + @Override + public MockHttpServletRequestBuilder formFields(MultiValueMap formFields) { + return super.formFields(formFields); + } + + @Override + public MockHttpServletRequestBuilder cookie(Cookie... cookies) { + return super.cookie(cookies); + } + + @Override + public MockHttpServletRequestBuilder locale(Locale... locales) { + return super.locale(locales); + } + + @Override + public MockHttpServletRequestBuilder locale(@Nullable Locale locale) { + return super.locale(locale); + } + + @Override + public MockHttpServletRequestBuilder requestAttr(String name, Object value) { + return super.requestAttr(name, value); + } + + @Override + public MockHttpServletRequestBuilder sessionAttr(String name, Object value) { + return super.sessionAttr(name, value); + } + + @Override + public MockHttpServletRequestBuilder sessionAttrs(Map sessionAttributes) { + return super.sessionAttrs(sessionAttributes); + } + + @Override + public MockHttpServletRequestBuilder flashAttr(String name, Object value) { + return super.flashAttr(name, value); + } + + @Override + public MockHttpServletRequestBuilder flashAttrs(Map flashAttributes) { + return super.flashAttrs(flashAttributes); + } + + @Override + public MockHttpServletRequestBuilder session(MockHttpSession session) { + return super.session(session); + } + + @Override + public MockHttpServletRequestBuilder principal(Principal principal) { + return super.principal(principal); + } + + @Override + public MockHttpServletRequestBuilder remoteAddress(String remoteAddress) { + return super.remoteAddress(remoteAddress); + } + + @Override + public MockHttpServletRequestBuilder with(RequestPostProcessor postProcessor) { + return super.with(postProcessor); + } + } diff --git a/spring-test/src/main/java/org/springframework/test/web/servlet/request/MockMultipartHttpServletRequestBuilder.java b/spring-test/src/main/java/org/springframework/test/web/servlet/request/MockMultipartHttpServletRequestBuilder.java index 2dd98f5e48..4817e7fb49 100644 --- a/spring-test/src/main/java/org/springframework/test/web/servlet/request/MockMultipartHttpServletRequestBuilder.java +++ b/spring-test/src/main/java/org/springframework/test/web/servlet/request/MockMultipartHttpServletRequestBuilder.java @@ -16,9 +16,28 @@ package org.springframework.test.web.servlet.request; +import java.io.IOException; +import java.io.InputStream; +import java.io.InputStreamReader; +import java.nio.charset.Charset; +import java.nio.charset.StandardCharsets; +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; + +import jakarta.servlet.ServletContext; +import jakarta.servlet.http.Part; + import org.springframework.http.HttpMethod; import org.springframework.http.MediaType; +import org.springframework.lang.Nullable; +import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.mock.web.MockMultipartFile; import org.springframework.mock.web.MockMultipartHttpServletRequest; +import org.springframework.util.Assert; +import org.springframework.util.FileCopyUtils; +import org.springframework.util.LinkedMultiValueMap; +import org.springframework.util.MultiValueMap; /** * Default builder for {@link MockMultipartHttpServletRequest}. @@ -28,8 +47,12 @@ import org.springframework.mock.web.MockMultipartHttpServletRequest; * @author Stephane Nicoll * @since 3.2 */ -public class MockMultipartHttpServletRequestBuilder - extends AbstractMockMultipartHttpServletRequestBuilder { +public class MockMultipartHttpServletRequestBuilder extends MockHttpServletRequestBuilder { + + private final List files = new ArrayList<>(); + + private final MultiValueMap parts = new LinkedMultiValueMap<>(); + /** * Package-private constructor. Use static factory methods in @@ -53,4 +76,100 @@ public class MockMultipartHttpServletRequestBuilder } + /** + * Add a new {@link MockMultipartFile} with the given content. + * @param name the name of the file + * @param content the content of the file + */ + public MockMultipartHttpServletRequestBuilder file(String name, byte[] content) { + this.files.add(new MockMultipartFile(name, content)); + return this; + } + + /** + * Add the given {@link MockMultipartFile}. + * @param file the multipart file + */ + public MockMultipartHttpServletRequestBuilder file(MockMultipartFile file) { + this.files.add(file); + return this; + } + + /** + * Add {@link Part} components to the request. + * @param parts one or more parts to add + * @since 5.0 + */ + public MockMultipartHttpServletRequestBuilder part(Part... parts) { + Assert.notEmpty(parts, "'parts' must not be empty"); + for (Part part : parts) { + this.parts.add(part.getName(), part); + } + return this; + } + + @Override + public Object merge(@Nullable Object parent) { + if (parent == null) { + return this; + } + if (parent instanceof MockHttpServletRequestBuilder) { + super.merge(parent); + if (parent instanceof MockMultipartHttpServletRequestBuilder parentBuilder) { + this.files.addAll(parentBuilder.files); + parentBuilder.parts.keySet().forEach(name -> + this.parts.putIfAbsent(name, parentBuilder.parts.get(name))); + } + } + else { + throw new IllegalArgumentException("Cannot merge with [" + parent.getClass().getName() + "]"); + } + return this; + } + + /** + * Create a new {@link MockMultipartHttpServletRequest} based on the + * supplied {@code ServletContext} and the {@code MockMultipartFiles} + * added to this builder. + */ + @Override + protected final MockHttpServletRequest createServletRequest(ServletContext servletContext) { + MockMultipartHttpServletRequest request = new MockMultipartHttpServletRequest(servletContext); + Charset defaultCharset = (request.getCharacterEncoding() != null ? + Charset.forName(request.getCharacterEncoding()) : StandardCharsets.UTF_8); + + this.files.forEach(request::addFile); + this.parts.values().stream().flatMap(Collection::stream).forEach(part -> { + request.addPart(part); + try { + String name = part.getName(); + String filename = part.getSubmittedFileName(); + InputStream is = part.getInputStream(); + if (filename != null) { + request.addFile(new MockMultipartFile(name, filename, part.getContentType(), is)); + } + else { + InputStreamReader reader = new InputStreamReader(is, getCharsetOrDefault(part, defaultCharset)); + String value = FileCopyUtils.copyToString(reader); + request.addParameter(part.getName(), value); + } + } + catch (IOException ex) { + throw new IllegalStateException("Failed to read content for part " + part.getName(), ex); + } + }); + + return request; + } + + private Charset getCharsetOrDefault(Part part, Charset defaultCharset) { + if (part.getContentType() != null) { + MediaType mediaType = MediaType.parseMediaType(part.getContentType()); + if (mediaType.getCharset() != null) { + return mediaType.getCharset(); + } + } + return defaultCharset; + } + } diff --git a/spring-test/src/main/java/org/springframework/test/web/servlet/request/MockMvcRequestBuilders.java b/spring-test/src/main/java/org/springframework/test/web/servlet/request/MockMvcRequestBuilders.java index 139fd8dcf7..fa73f00795 100644 --- a/spring-test/src/main/java/org/springframework/test/web/servlet/request/MockMvcRequestBuilders.java +++ b/spring-test/src/main/java/org/springframework/test/web/servlet/request/MockMvcRequestBuilders.java @@ -214,7 +214,9 @@ public abstract class MockMvcRequestBuilders { * @since 5.0 */ public static MockMultipartHttpServletRequestBuilder multipart(String uriTemplate, Object... uriVariables) { - return new MockMultipartHttpServletRequestBuilder().uri(uriTemplate, uriVariables); + MockMultipartHttpServletRequestBuilder builder = new MockMultipartHttpServletRequestBuilder(); + builder.uri(uriTemplate, uriVariables); + return builder; } /** @@ -226,7 +228,9 @@ public abstract class MockMvcRequestBuilders { * @since 5.3.22 */ public static MockMultipartHttpServletRequestBuilder multipart(HttpMethod httpMethod, String uriTemplate, Object... uriVariables) { - return new MockMultipartHttpServletRequestBuilder(httpMethod).uri(uriTemplate, uriVariables); + MockMultipartHttpServletRequestBuilder builder = new MockMultipartHttpServletRequestBuilder(httpMethod); + builder.uri(uriTemplate, uriVariables); + return builder; } /** @@ -235,7 +239,9 @@ public abstract class MockMvcRequestBuilders { * @since 5.0 */ public static MockMultipartHttpServletRequestBuilder multipart(URI uri) { - return new MockMultipartHttpServletRequestBuilder().uri(uri); + MockMultipartHttpServletRequestBuilder builder = new MockMultipartHttpServletRequestBuilder(); + builder.uri(uri); + return builder; } /** @@ -246,7 +252,9 @@ public abstract class MockMvcRequestBuilders { * @since 5.3.21 */ public static MockMultipartHttpServletRequestBuilder multipart(HttpMethod httpMethod, URI uri) { - return new MockMultipartHttpServletRequestBuilder(httpMethod).uri(uri); + MockMultipartHttpServletRequestBuilder builder = new MockMultipartHttpServletRequestBuilder(httpMethod); + builder.uri(uri); + return builder; } /** diff --git a/spring-test/src/test/java/org/springframework/test/web/servlet/request/MockMultipartHttpServletRequestBuilderTests.java b/spring-test/src/test/java/org/springframework/test/web/servlet/request/MockMultipartHttpServletRequestBuilderTests.java index 5d559a777a..e8c3cfdadd 100644 --- a/spring-test/src/test/java/org/springframework/test/web/servlet/request/MockMultipartHttpServletRequestBuilderTests.java +++ b/spring-test/src/test/java/org/springframework/test/web/servlet/request/MockMultipartHttpServletRequestBuilderTests.java @@ -39,7 +39,7 @@ public class MockMultipartHttpServletRequestBuilderTests { @Test // gh-26166 void addFileAndParts() throws Exception { MockMultipartHttpServletRequest mockRequest = - (MockMultipartHttpServletRequest) new MockMultipartHttpServletRequestBuilder().uri("/upload") + (MockMultipartHttpServletRequest) createBuilder("/upload") .file(new MockMultipartFile("file", "test.txt", "text/plain", "Test".getBytes(UTF_8))) .part(new MockPart("name", "value".getBytes(UTF_8))) .buildRequest(new MockServletContext()); @@ -55,7 +55,7 @@ public class MockMultipartHttpServletRequestBuilderTests { jsonPart.getHeaders().setContentType(MediaType.APPLICATION_JSON); MockMultipartHttpServletRequest mockRequest = - (MockMultipartHttpServletRequest) new MockMultipartHttpServletRequestBuilder().uri("/upload") + (MockMultipartHttpServletRequest) createBuilder("/upload") .file(new MockMultipartFile("file", "Test".getBytes(UTF_8))) .part(jsonPart) .buildRequest(new MockServletContext()); @@ -70,7 +70,7 @@ public class MockMultipartHttpServletRequestBuilderTests { void mergeAndBuild() { MockHttpServletRequestBuilder parent = new MockHttpServletRequestBuilder(HttpMethod.GET).uri("/"); parent.characterEncoding("UTF-8"); - Object result = new MockMultipartHttpServletRequestBuilder().uri("/fileUpload").merge(parent); + Object result = createBuilder("/fileUpload").merge(parent); assertThat(result).isNotNull(); assertThat(result.getClass()).isEqualTo(MockMultipartHttpServletRequestBuilder.class); @@ -80,4 +80,10 @@ public class MockMultipartHttpServletRequestBuilderTests { assertThat(request.getCharacterEncoding()).isEqualTo("UTF-8"); } + private MockMultipartHttpServletRequestBuilder createBuilder(String uri) { + MockMultipartHttpServletRequestBuilder builder = new MockMultipartHttpServletRequestBuilder(); + builder.uri(uri); + return builder; + } + }