From 7c47b470ff53f4e1950ea5a3bb4a6d0aa9654d07 Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Mon, 6 Jun 2022 17:04:07 +0100 Subject: [PATCH 1/2] MockMvc allows HttpMethod input for multipart request Closes gh-28545 --- .../servlet/client/MockMvcHttpConnector.java | 4 +- ...ockMultipartHttpServletRequestBuilder.java | 20 +++++---- .../request/MockMvcRequestBuilders.java | 15 ++++++- .../standalone/MultipartControllerTests.java | 42 ++++++++++++++----- 4 files changed, 59 insertions(+), 22 deletions(-) diff --git a/spring-test/src/main/java/org/springframework/test/web/servlet/client/MockMvcHttpConnector.java b/spring-test/src/main/java/org/springframework/test/web/servlet/client/MockMvcHttpConnector.java index ba6b040b10e..b4d07e814c7 100644 --- a/spring-test/src/main/java/org/springframework/test/web/servlet/client/MockMvcHttpConnector.java +++ b/spring-test/src/main/java/org/springframework/test/web/servlet/client/MockMvcHttpConnector.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2002-2022 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. @@ -152,7 +152,7 @@ public class MockMvcHttpConnector implements ClientHttpConnector { } // Parse the multipart request in order to adapt to Servlet Part's - MockMultipartHttpServletRequestBuilder requestBuilder = MockMvcRequestBuilders.multipart(uri); + MockMultipartHttpServletRequestBuilder requestBuilder = MockMvcRequestBuilders.multipart(httpMethod, uri); Assert.notNull(bytes, "No multipart content"); ReactiveHttpInputMessage inputMessage = MockServerHttpRequest.post(uri.toString()) 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 73a0a3bd29d..3f2b59909b8 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2022 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. @@ -69,12 +69,8 @@ public class MockMultipartHttpServletRequestBuilder extends MockHttpServletReque } /** - * Package-private constructor. Use static factory methods in - * {@link MockMvcRequestBuilders}. - *

For other ways to initialize a {@code MockMultipartHttpServletRequest}, - * see {@link #with(RequestPostProcessor)} and the - * {@link RequestPostProcessor} extension point. - * @param uri the URL + * Variant of {@link #MockMultipartHttpServletRequestBuilder(String, Object...)} + * with a {@link URI}. * @since 4.0.3 */ MockMultipartHttpServletRequestBuilder(URI uri) { @@ -82,6 +78,16 @@ public class MockMultipartHttpServletRequestBuilder extends MockHttpServletReque super.contentType(MediaType.MULTIPART_FORM_DATA); } + /** + * Variant of {@link #MockMultipartHttpServletRequestBuilder(String, Object...)} + * with a {@link URI} and an {@link HttpMethod}. + * @since 5.3.21 + */ + MockMultipartHttpServletRequestBuilder(HttpMethod httpMethod, URI uri) { + super(httpMethod, uri); + super.contentType(MediaType.MULTIPART_FORM_DATA); + } + /** * Create a new MockMultipartFile with the given content. 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 068320216f6..7334028daa7 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2022 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. @@ -215,7 +215,7 @@ public abstract class MockMvcRequestBuilders { } /** - * Create a {@link MockMultipartHttpServletRequestBuilder} for a multipart request. + * Variant of {@link #multipart(String, Object...)} with a {@link URI}. * @param uri the URL * @since 5.0 */ @@ -223,6 +223,17 @@ public abstract class MockMvcRequestBuilders { return new MockMultipartHttpServletRequestBuilder(uri); } + /** + * Variant of {@link #multipart(String, Object...)} with a {@link URI} and + * an {@link HttpMethod}. + * @param httpMethod the HTTP method to use + * @param uri the URL + * @since 5.3.21 + */ + public static MockMultipartHttpServletRequestBuilder multipart(HttpMethod httpMethod, URI uri) { + return new MockMultipartHttpServletRequestBuilder(httpMethod, uri); + } + /** * Create a {@link MockMultipartHttpServletRequestBuilder} for a multipart request. * @param urlTemplate a URL template; the resulting URL will be encoded diff --git a/spring-test/src/test/java/org/springframework/test/web/servlet/samples/client/standalone/MultipartControllerTests.java b/spring-test/src/test/java/org/springframework/test/web/servlet/samples/client/standalone/MultipartControllerTests.java index b60a0e6f6e3..495ff3e17ab 100644 --- a/spring-test/src/test/java/org/springframework/test/web/servlet/samples/client/standalone/MultipartControllerTests.java +++ b/spring-test/src/test/java/org/springframework/test/web/servlet/samples/client/standalone/MultipartControllerTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2002-2022 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. @@ -39,8 +39,8 @@ import org.springframework.test.web.reactive.server.EntityExchangeResult; import org.springframework.test.web.reactive.server.WebTestClient; import org.springframework.test.web.servlet.client.MockMvcWebTestClient; import org.springframework.ui.Model; -import org.springframework.web.bind.annotation.RequestMapping; -import org.springframework.web.bind.annotation.RequestMethod; +import org.springframework.web.bind.annotation.PostMapping; +import org.springframework.web.bind.annotation.PutMapping; import org.springframework.web.bind.annotation.RequestParam; import org.springframework.web.bind.annotation.RequestPart; import org.springframework.web.filter.OncePerRequestFilter; @@ -80,6 +80,18 @@ public class MultipartControllerTests { MockMvcWebTestClient.resultActionsFor(exchangeResult) .andExpect(model().attribute("fileContent", fileContent)) .andExpect(model().attribute("jsonContent", json)); + + // Now try the same with HTTP PUT + exchangeResult = testClient.put().uri("/multipartfile-via-put") + .bodyValue(bodyBuilder.build()) + .exchange() + .expectStatus().isFound() + .expectBody().isEmpty(); + + // Further assertions on the server response + MockMvcWebTestClient.resultActionsFor(exchangeResult) + .andExpect(model().attribute("fileContent", fileContent)) + .andExpect(model().attribute("jsonContent", json)); } @Test @@ -284,10 +296,11 @@ public class MultipartControllerTests { } + @SuppressWarnings("OptionalUsedAsFieldOrParameterType") @Controller private static class MultipartController { - @RequestMapping(value = "/multipartfile", method = RequestMethod.POST) + @PostMapping("/multipartfile") public String processMultipartFile(@RequestParam(required = false) MultipartFile file, @RequestPart(required = false) Map json, Model model) throws IOException { @@ -301,7 +314,14 @@ public class MultipartControllerTests { return "redirect:/index"; } - @RequestMapping(value = "/multipartfilearray", method = RequestMethod.POST) + @PutMapping("/multipartfile-via-put") + public String processMultipartFileViaHttpPut(@RequestParam(required = false) MultipartFile file, + @RequestPart(required = false) Map json, Model model) throws IOException { + + return processMultipartFile(file, json, model); + } + + @PostMapping("/multipartfilearray") public String processMultipartFileArray(@RequestParam(required = false) MultipartFile[] file, @RequestPart(required = false) Map json, Model model) throws IOException { @@ -317,7 +337,7 @@ public class MultipartControllerTests { return "redirect:/index"; } - @RequestMapping(value = "/multipartfilelist", method = RequestMethod.POST) + @PostMapping("/multipartfilelist") public String processMultipartFileList(@RequestParam(required = false) List file, @RequestPart(required = false) Map json, Model model) throws IOException { @@ -333,7 +353,7 @@ public class MultipartControllerTests { return "redirect:/index"; } - @RequestMapping(value = "/optionalfile", method = RequestMethod.POST) + @PostMapping("/optionalfile") public String processOptionalFile(@RequestParam Optional file, @RequestPart Map json, Model model) throws IOException { @@ -345,7 +365,7 @@ public class MultipartControllerTests { return "redirect:/index"; } - @RequestMapping(value = "/optionalfilearray", method = RequestMethod.POST) + @PostMapping("/optionalfilearray") public String processOptionalFileArray(@RequestParam Optional file, @RequestPart Map json, Model model) throws IOException { @@ -359,7 +379,7 @@ public class MultipartControllerTests { return "redirect:/index"; } - @RequestMapping(value = "/optionalfilelist", method = RequestMethod.POST) + @PostMapping("/optionalfilelist") public String processOptionalFileList(@RequestParam Optional> file, @RequestPart Map json, Model model) throws IOException { @@ -373,7 +393,7 @@ public class MultipartControllerTests { return "redirect:/index"; } - @RequestMapping(value = "/part", method = RequestMethod.POST) + @PostMapping("/part") public String processPart(@RequestParam Part part, @RequestPart Map json, Model model) throws IOException { @@ -383,7 +403,7 @@ public class MultipartControllerTests { return "redirect:/index"; } - @RequestMapping(value = "/json", method = RequestMethod.POST) + @PostMapping("/json") public String processMultipart(@RequestPart Map json, Model model) { model.addAttribute("json", json); return "redirect:/index"; From 8fcc7ab9d1916533cdd0e9ef68eb5eadd8f9da59 Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Wed, 8 Jun 2022 10:06:59 +0100 Subject: [PATCH 2/2] CompositeLog respects log level changes at runtime Closes gh-28477 --- .../core/log/CompositeLog.java | 85 ++++++++---------- .../core/log/CompositeLogTests.java | 86 +++++++++++++++++++ 2 files changed, 123 insertions(+), 48 deletions(-) create mode 100644 spring-core/src/test/java/org/springframework/core/log/CompositeLogTests.java diff --git a/spring-core/src/main/java/org/springframework/core/log/CompositeLog.java b/spring-core/src/main/java/org/springframework/core/log/CompositeLog.java index 9279180b1ef..39a5f4c59b7 100644 --- a/spring-core/src/main/java/org/springframework/core/log/CompositeLog.java +++ b/spring-core/src/main/java/org/springframework/core/log/CompositeLog.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2022 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. @@ -22,6 +22,7 @@ import java.util.function.Predicate; import org.apache.commons.logging.Log; import org.apache.commons.logging.impl.NoOpLog; + /** * Implementation of {@link Log} that wraps a list of loggers and delegates * to the first one for which logging is enabled at the given level. @@ -35,131 +36,119 @@ final class CompositeLog implements Log { private static final Log NO_OP_LOG = new NoOpLog(); - private final Log fatalLogger; - - private final Log errorLogger; - - private final Log warnLogger; - - private final Log infoLogger; - - private final Log debugLogger; - - private final Log traceLogger; + private final List loggers; /** - * Constructor with list of loggers. For optimal performance, the constructor - * checks and remembers which logger is on for each log category. + * Package-private constructor with list of loggers. * @param loggers the loggers to use */ - public CompositeLog(List loggers) { - this.fatalLogger = initLogger(loggers, Log::isFatalEnabled); - this.errorLogger = initLogger(loggers, Log::isErrorEnabled); - this.warnLogger = initLogger(loggers, Log::isWarnEnabled); - this.infoLogger = initLogger(loggers, Log::isInfoEnabled); - this.debugLogger = initLogger(loggers, Log::isDebugEnabled); - this.traceLogger = initLogger(loggers, Log::isTraceEnabled); - } - - private static Log initLogger(List loggers, Predicate predicate) { - for (Log logger : loggers) { - if (predicate.test(logger)) { - return logger; - } - } - return NO_OP_LOG; + CompositeLog(List loggers) { + this.loggers = loggers; } @Override public boolean isFatalEnabled() { - return (this.fatalLogger != NO_OP_LOG); + return isEnabled(Log::isFatalEnabled); } @Override public boolean isErrorEnabled() { - return (this.errorLogger != NO_OP_LOG); + return isEnabled(Log::isErrorEnabled); } @Override public boolean isWarnEnabled() { - return (this.warnLogger != NO_OP_LOG); + return isEnabled(Log::isWarnEnabled); } @Override public boolean isInfoEnabled() { - return (this.infoLogger != NO_OP_LOG); + return isEnabled(Log::isInfoEnabled); } @Override public boolean isDebugEnabled() { - return (this.debugLogger != NO_OP_LOG); + return isEnabled(Log::isDebugEnabled); } @Override public boolean isTraceEnabled() { - return (this.traceLogger != NO_OP_LOG); + return isEnabled(Log::isTraceEnabled); + } + + private boolean isEnabled(Predicate predicate) { + return (getLogger(predicate) != NO_OP_LOG); } @Override public void fatal(Object message) { - this.fatalLogger.fatal(message); + getLogger(Log::isFatalEnabled).fatal(message); } @Override public void fatal(Object message, Throwable ex) { - this.fatalLogger.fatal(message, ex); + getLogger(Log::isFatalEnabled).fatal(message, ex); } @Override public void error(Object message) { - this.errorLogger.error(message); + getLogger(Log::isErrorEnabled).error(message); } @Override public void error(Object message, Throwable ex) { - this.errorLogger.error(message, ex); + getLogger(Log::isErrorEnabled).error(message, ex); } @Override public void warn(Object message) { - this.warnLogger.warn(message); + getLogger(Log::isWarnEnabled).warn(message); } @Override public void warn(Object message, Throwable ex) { - this.warnLogger.warn(message, ex); + getLogger(Log::isWarnEnabled).warn(message, ex); } @Override public void info(Object message) { - this.infoLogger.info(message); + getLogger(Log::isInfoEnabled).info(message); } @Override public void info(Object message, Throwable ex) { - this.infoLogger.info(message, ex); + getLogger(Log::isInfoEnabled).info(message, ex); } @Override public void debug(Object message) { - this.debugLogger.debug(message); + getLogger(Log::isDebugEnabled).debug(message); } @Override public void debug(Object message, Throwable ex) { - this.debugLogger.debug(message, ex); + getLogger(Log::isDebugEnabled).debug(message, ex); } @Override public void trace(Object message) { - this.traceLogger.trace(message); + getLogger(Log::isTraceEnabled).trace(message); } @Override public void trace(Object message, Throwable ex) { - this.traceLogger.trace(message, ex); + getLogger(Log::isTraceEnabled).trace(message, ex); + } + + private Log getLogger(Predicate predicate) { + for (Log logger : this.loggers) { + if (predicate.test(logger)) { + return logger; + } + } + return NO_OP_LOG; } } diff --git a/spring-core/src/test/java/org/springframework/core/log/CompositeLogTests.java b/spring-core/src/test/java/org/springframework/core/log/CompositeLogTests.java new file mode 100644 index 00000000000..a15bfd38447 --- /dev/null +++ b/spring-core/src/test/java/org/springframework/core/log/CompositeLogTests.java @@ -0,0 +1,86 @@ +/* + * Copyright 2002-2022 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.core.log; + +import java.util.Arrays; + +import org.apache.commons.logging.Log; +import org.junit.jupiter.api.Test; + +import static org.mockito.BDDMockito.when; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; + + +/** + * Unit tests for {@link CompositeLog}. + * @author Rossen Stoyanchev + */ +public class CompositeLogTests { + + private final Log logger1 = mock(Log.class); + + private final Log logger2 = mock(Log.class); + + private final CompositeLog compositeLog = new CompositeLog(Arrays.asList(logger1, logger2)); + + + @Test + void useFirstLogger() { + when(logger1.isInfoEnabled()).thenReturn(true); + when(logger2.isInfoEnabled()).thenReturn(true); + + this.compositeLog.info("info message"); + + verify(this.logger1).isInfoEnabled(); + verify(this.logger1).info("info message"); + + verifyNoMoreInteractions(this.logger1); + verifyNoMoreInteractions(this.logger2); + } + + @Test + void useSecondLogger() { + when(logger1.isInfoEnabled()).thenReturn(false); + when(logger2.isInfoEnabled()).thenReturn(true); + + this.compositeLog.info("info message"); + + verify(this.logger1).isInfoEnabled(); + verify(this.logger2).isInfoEnabled(); + verify(this.logger2).info("info message"); + + verifyNoMoreInteractions(this.logger1); + verifyNoMoreInteractions(this.logger2); + } + + @Test + void useNeitherLogger() { + when(logger1.isInfoEnabled()).thenReturn(false); + when(logger2.isInfoEnabled()).thenReturn(false); + + this.compositeLog.info("info message"); + + verify(this.logger1).isInfoEnabled(); + verify(this.logger2).isInfoEnabled(); + + verifyNoMoreInteractions(this.logger1); + verifyNoMoreInteractions(this.logger2); + } + +}