From db8e9eafb208860f53911c5f352074142b7f74ea Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Fri, 14 Sep 2018 14:38:11 -0400 Subject: [PATCH] Add LogFormatUtils 1. Helper method to eliminate duplication in formatting (de-)serialized values for logging introduced with prior commit #e62298. 2. Helper method for TRACE vs DEBUG logging with different details. Issue: SPR-17254 --- .../core/codec/CharSequenceEncoder.java | 23 ++--- .../core/codec/StringDecoder.java | 22 ++--- .../core/log/LogFormatUtils.java | 87 +++++++++++++++++++ .../http/codec/FormHttpMessageReader.java | 17 ++-- .../http/codec/FormHttpMessageWriter.java | 21 ++--- .../http/codec/LoggingCodecSupport.java | 17 ---- .../codec/json/AbstractJackson2Decoder.java | 15 ++-- .../codec/json/AbstractJackson2Encoder.java | 15 ++-- .../http/codec/json/Jackson2CodecSupport.java | 8 -- .../multipart/MultipartHttpMessageReader.java | 17 ++-- .../multipart/MultipartHttpMessageWriter.java | 17 ++-- .../SynchronossPartHttpMessageReader.java | 15 ++-- .../http/codec/xml/Jaxb2XmlDecoder.java | 23 ++--- .../http/codec/xml/Jaxb2XmlEncoder.java | 23 ++--- .../AbstractListenerWriteFlushProcessor.java | 3 +- .../commons/CommonsFileUploadSupport.java | 30 +++---- .../commons/CommonsMultipartFile.java | 16 ++-- .../server/adapter/HttpWebHandlerAdapter.java | 36 +++----- .../function/client/ExchangeFunctions.java | 27 ++---- .../web/servlet/DispatcherServlet.java | 11 +-- .../web/servlet/FrameworkServlet.java | 4 +- ...essageConverterMethodArgumentResolver.java | 18 ++-- ...stractMessageConverterMethodProcessor.java | 14 +-- .../RequestMappingHandlerAdapter.java | 22 ++--- 24 files changed, 212 insertions(+), 289 deletions(-) create mode 100644 spring-core/src/main/java/org/springframework/core/log/LogFormatUtils.java diff --git a/spring-core/src/main/java/org/springframework/core/codec/CharSequenceEncoder.java b/spring-core/src/main/java/org/springframework/core/codec/CharSequenceEncoder.java index e9d9269047..d6bf4e96db 100644 --- a/spring-core/src/main/java/org/springframework/core/codec/CharSequenceEncoder.java +++ b/spring-core/src/main/java/org/springframework/core/codec/CharSequenceEncoder.java @@ -28,6 +28,7 @@ import reactor.core.publisher.Flux; import org.springframework.core.ResolvableType; import org.springframework.core.io.buffer.DataBuffer; import org.springframework.core.io.buffer.DataBufferFactory; +import org.springframework.core.log.LogFormatUtils; import org.springframework.lang.Nullable; import org.springframework.util.MimeType; import org.springframework.util.MimeTypeUtils; @@ -68,15 +69,11 @@ public final class CharSequenceEncoder extends AbstractEncoder { Charset charset = getCharset(mimeType); return Flux.from(inputStream).map(charSequence -> { - if (logger.isDebugEnabled() && !Hints.isLoggingSuppressed(hints)) { - String logPrefix = Hints.getLogPrefix(hints); - String s = logPrefix + "Writing " + formatValue(charSequence, logger.isTraceEnabled()); - if (logger.isTraceEnabled()) { - logger.trace(s); - } - else { - logger.debug(s); - } + if (!Hints.isLoggingSuppressed(hints)) { + LogFormatUtils.traceDebug(logger, traceOn -> { + String formatted = LogFormatUtils.formatValue(charSequence, !traceOn); + return Hints.getLogPrefix(hints) + "Writing " + formatted; + }); } CharBuffer charBuffer = CharBuffer.wrap(charSequence); ByteBuffer byteBuffer = charset.encode(charBuffer); @@ -95,14 +92,6 @@ public final class CharSequenceEncoder extends AbstractEncoder { return charset; } - private String formatValue(@Nullable Object value, boolean logFullValue) { - if (value == null) { - return ""; - } - String s = value instanceof CharSequence ? "\"" + value + "\"" : value.toString(); - return logFullValue || s.length() < 100 ? s : s.substring(0, 100) + " (truncated)..."; - } - /** * Create a {@code CharSequenceEncoder} that supports only "text/plain". diff --git a/spring-core/src/main/java/org/springframework/core/codec/StringDecoder.java b/spring-core/src/main/java/org/springframework/core/codec/StringDecoder.java index ceac18bf90..0a167d22ae 100644 --- a/spring-core/src/main/java/org/springframework/core/codec/StringDecoder.java +++ b/spring-core/src/main/java/org/springframework/core/codec/StringDecoder.java @@ -33,6 +33,7 @@ import org.springframework.core.ResolvableType; import org.springframework.core.io.buffer.DataBuffer; import org.springframework.core.io.buffer.DataBufferUtils; import org.springframework.core.io.buffer.DefaultDataBufferFactory; +import org.springframework.core.log.LogFormatUtils; import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.util.MimeType; @@ -206,15 +207,10 @@ public final class StringDecoder extends AbstractDataBufferDecoder { CharBuffer charBuffer = charset.decode(dataBuffer.asByteBuffer()); DataBufferUtils.release(dataBuffer); String value = charBuffer.toString(); - if (logger.isDebugEnabled()) { - String s = Hints.getLogPrefix(hints) + "Decoded " + formatValue(value, logger.isTraceEnabled()); - if (logger.isTraceEnabled()) { - logger.trace(s); - } - else { - logger.debug(s); - } - } + LogFormatUtils.traceDebug(logger, traceOn -> { + String formatted = LogFormatUtils.formatValue(value, !traceOn); + return Hints.getLogPrefix(hints) + "Decoded " + formatted; + }); return value; } @@ -227,14 +223,6 @@ public final class StringDecoder extends AbstractDataBufferDecoder { } } - private String formatValue(@Nullable Object value, boolean logFullValue) { - if (value == null) { - return ""; - } - String s = value instanceof CharSequence ? "\"" + value + "\"" : value.toString(); - return logFullValue || s.length() < 100 ? s : s.substring(0, 100) + " (truncated)..."; - } - /** * Create a {@code StringDecoder} for {@code "text/plain"}. diff --git a/spring-core/src/main/java/org/springframework/core/log/LogFormatUtils.java b/spring-core/src/main/java/org/springframework/core/log/LogFormatUtils.java new file mode 100644 index 0000000000..a079a5f76c --- /dev/null +++ b/spring-core/src/main/java/org/springframework/core/log/LogFormatUtils.java @@ -0,0 +1,87 @@ +/* + * 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. + * You may obtain a copy of the License at + * + * http://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.function.Function; + +import org.apache.commons.logging.Log; + +import org.springframework.lang.Nullable; + +/** + * Utility methods for formatting and logging messages. + * + *

Mainly for internal use within the framework with Apache Commons Logging, + * typically in the form of the {@code spring-jcl} bridge but also compatible + * with other Commons Logging bridges. + * + * @author Rossen Stoyanchev + * @since 5.1 + */ +public final class LogFormatUtils { + + private LogFormatUtils() { + } + + + /** + * Format the given value via {@code toString()}, quoting it if it is a + * {@link CharSequence}, and possibly truncating at 100 if limitLength is + * set to true. + * @param value the value to format + * @param limitLength whether to truncate large formatted values (over 100). + * @return the formatted value + * @since 5.1 + */ + public static String formatValue(@Nullable Object value, boolean limitLength) { + if (value == null) { + return ""; + } + String s = value instanceof CharSequence ? "\"" + value + "\"" : value.toString(); + return limitLength && s.length() > 100 ? s.substring(0, 100) + " (truncated)..." : s; + } + + /** + * Use this to log a message with different levels of detail (or different + * messages) at TRACE vs DEBUG log levels. Effectively, a substitute for: + * 

+	 * if (logger.isDebugEnabled()) {
+	 *	String s = logger.isTraceEnabled() ? "..." : "...";
+	 *	if (logger.isTraceEnabled()) {
+	 *		logger.trace(s);
+	 *	}
+	 *	else {
+	 *		logger.debug(s);
+	 *	}
+	 * }
+	 *
+ * @param logger the logger to use to log the message + * @param messageFactory function that accepts a boolean set to the value + * of {@link Log#isTraceEnabled()}. + */ + public static void traceDebug(Log logger, Function messageFactory) { + if (logger.isDebugEnabled()) { + String logMessage = messageFactory.apply(logger.isTraceEnabled()); + if (logger.isTraceEnabled()) { + logger.trace(logMessage); + } + else { + logger.debug(logMessage); + } + } + } + +} diff --git a/spring-web/src/main/java/org/springframework/http/codec/FormHttpMessageReader.java b/spring-web/src/main/java/org/springframework/http/codec/FormHttpMessageReader.java index a893c69798..9fe22e8e6d 100644 --- a/spring-web/src/main/java/org/springframework/http/codec/FormHttpMessageReader.java +++ b/spring-web/src/main/java/org/springframework/http/codec/FormHttpMessageReader.java @@ -31,6 +31,7 @@ import reactor.core.publisher.Mono; import org.springframework.core.ResolvableType; import org.springframework.core.codec.Hints; import org.springframework.core.io.buffer.DataBufferUtils; +import org.springframework.core.log.LogFormatUtils; import org.springframework.http.MediaType; import org.springframework.http.ReactiveHttpInputMessage; import org.springframework.lang.Nullable; @@ -114,18 +115,10 @@ public class FormHttpMessageReader extends LoggingCodecSupport } private void logFormData(MultiValueMap formData, Map hints) { - if (logger.isDebugEnabled()) { - String s = Hints.getLogPrefix(hints) + "Read " + - (isEnableLoggingRequestDetails() ? - formatValue(formData, logger.isTraceEnabled()) : - "form fields " + formData.keySet() + " (content masked)"); - if (logger.isTraceEnabled()) { - logger.trace(s); - } - else { - logger.debug(s); - } - } + LogFormatUtils.traceDebug(logger, traceOn -> Hints.getLogPrefix(hints) + "Read " + + (isEnableLoggingRequestDetails() ? + LogFormatUtils.formatValue(formData, !traceOn) : + "form fields " + formData.keySet() + " (content masked)")); } private Charset getMediaTypeCharset(@Nullable MediaType mediaType) { diff --git a/spring-web/src/main/java/org/springframework/http/codec/FormHttpMessageWriter.java b/spring-web/src/main/java/org/springframework/http/codec/FormHttpMessageWriter.java index ebe0b01f10..63655b06a3 100644 --- a/spring-web/src/main/java/org/springframework/http/codec/FormHttpMessageWriter.java +++ b/spring-web/src/main/java/org/springframework/http/codec/FormHttpMessageWriter.java @@ -31,6 +31,7 @@ import reactor.core.publisher.Mono; import org.springframework.core.ResolvableType; import org.springframework.core.codec.Hints; import org.springframework.core.io.buffer.DataBuffer; +import org.springframework.core.log.LogFormatUtils; import org.springframework.http.MediaType; import org.springframework.http.ReactiveHttpOutputMessage; import org.springframework.lang.Nullable; @@ -131,18 +132,7 @@ public class FormHttpMessageWriter extends LoggingCodecSupport Assert.notNull(charset, "No charset"); // should never occur return Mono.from(inputStream).flatMap(form -> { - if (logger.isDebugEnabled()) { - String s = Hints.getLogPrefix(hints) + "Writing " + - (isEnableLoggingRequestDetails() ? - formatValue(form, logger.isTraceEnabled()) : - "form fields " + form.keySet() + " (content masked)"); - if (logger.isTraceEnabled()) { - logger.trace(s); - } - else { - logger.debug(s); - } - } + logFormData(form, hints); String value = serializeForm(form, charset); ByteBuffer byteBuffer = charset.encode(value); DataBuffer buffer = message.bufferFactory().wrap(byteBuffer); @@ -163,6 +153,13 @@ public class FormHttpMessageWriter extends LoggingCodecSupport } } + private void logFormData(MultiValueMap form, Map hints) { + LogFormatUtils.traceDebug(logger, traceOn -> Hints.getLogPrefix(hints) + "Writing " + + (isEnableLoggingRequestDetails() ? + LogFormatUtils.formatValue(form, !traceOn) : + "form fields " + form.keySet() + " (content masked)")); + } + protected String serializeForm(MultiValueMap formData, Charset charset) { StringBuilder builder = new StringBuilder(); formData.forEach((name, values) -> diff --git a/spring-web/src/main/java/org/springframework/http/codec/LoggingCodecSupport.java b/spring-web/src/main/java/org/springframework/http/codec/LoggingCodecSupport.java index 5191c990b5..3502adc0e6 100644 --- a/spring-web/src/main/java/org/springframework/http/codec/LoggingCodecSupport.java +++ b/spring-web/src/main/java/org/springframework/http/codec/LoggingCodecSupport.java @@ -19,7 +19,6 @@ package org.springframework.http.codec; import org.apache.commons.logging.Log; import org.springframework.http.HttpLogging; -import org.springframework.lang.Nullable; /** * Base class for {@link org.springframework.core.codec.Encoder}, @@ -56,20 +55,4 @@ public class LoggingCodecSupport { return this.enableLoggingRequestDetails; } - /** - * Format the given value via {{toString()}}, either in full or truncated - * if it has 100 or more characters. - * @param value the value to format - * @param logFullValue whether to log in full or truncate if necessary - * @return the formatted value - * @since 5.1 - */ - protected String formatValue(@Nullable Object value, boolean logFullValue) { - if (value == null) { - return ""; - } - String s = value instanceof CharSequence ? "\"" + value + "\"" : value.toString(); - return logFullValue || s.length() < 100 ? s : s.substring(0, 100) + " (truncated)..."; - } - } diff --git a/spring-web/src/main/java/org/springframework/http/codec/json/AbstractJackson2Decoder.java b/spring-web/src/main/java/org/springframework/http/codec/json/AbstractJackson2Decoder.java index cd222927c5..f3c2433f2e 100644 --- a/spring-web/src/main/java/org/springframework/http/codec/json/AbstractJackson2Decoder.java +++ b/spring-web/src/main/java/org/springframework/http/codec/json/AbstractJackson2Decoder.java @@ -38,6 +38,7 @@ import org.springframework.core.codec.CodecException; import org.springframework.core.codec.DecodingException; import org.springframework.core.codec.Hints; import org.springframework.core.io.buffer.DataBuffer; +import org.springframework.core.log.LogFormatUtils; import org.springframework.http.codec.HttpMessageDecoder; import org.springframework.http.server.reactive.ServerHttpRequest; import org.springframework.http.server.reactive.ServerHttpResponse; @@ -115,15 +116,11 @@ public abstract class AbstractJackson2Decoder extends Jackson2CodecSupport imple return tokens.map(tokenBuffer -> { try { Object value = reader.readValue(tokenBuffer.asParser(getObjectMapper())); - if (logger.isDebugEnabled() && !Hints.isLoggingSuppressed(hints)) { - boolean traceOn = logger.isTraceEnabled(); - String s = Hints.getLogPrefix(hints) + "Decoded [" + formatValue(value, traceOn) + "]"; - if (traceOn) { - logger.trace(s); - } - else { - logger.debug(s); - } + if (!Hints.isLoggingSuppressed(hints)) { + LogFormatUtils.traceDebug(logger, traceOn -> { + String formatted = LogFormatUtils.formatValue(value, !traceOn); + return Hints.getLogPrefix(hints) + "Decoded [" + formatted + "]"; + }); } return value; } diff --git a/spring-web/src/main/java/org/springframework/http/codec/json/AbstractJackson2Encoder.java b/spring-web/src/main/java/org/springframework/http/codec/json/AbstractJackson2Encoder.java index 1ff5b413a6..ffeb685568 100644 --- a/spring-web/src/main/java/org/springframework/http/codec/json/AbstractJackson2Encoder.java +++ b/spring-web/src/main/java/org/springframework/http/codec/json/AbstractJackson2Encoder.java @@ -44,6 +44,7 @@ import org.springframework.core.codec.EncodingException; import org.springframework.core.codec.Hints; import org.springframework.core.io.buffer.DataBuffer; import org.springframework.core.io.buffer.DataBufferFactory; +import org.springframework.core.log.LogFormatUtils; import org.springframework.http.MediaType; import org.springframework.http.codec.HttpMessageEncoder; import org.springframework.http.server.reactive.ServerHttpRequest; @@ -141,15 +142,11 @@ public abstract class AbstractJackson2Encoder extends Jackson2CodecSupport imple private DataBuffer encodeValue(Object value, @Nullable MimeType mimeType, DataBufferFactory bufferFactory, ResolvableType elementType, @Nullable Map hints, JsonEncoding encoding) { - if (logger.isDebugEnabled() && !Hints.isLoggingSuppressed(hints)) { - boolean traceOn = logger.isTraceEnabled(); - String s = Hints.getLogPrefix(hints) + "Encoding [" + formatValue(value, traceOn) + "]"; - if (traceOn) { - logger.trace(s); - } - else { - logger.debug(s); - } + if (!Hints.isLoggingSuppressed(hints)) { + LogFormatUtils.traceDebug(logger, traceOn -> { + String formatted = LogFormatUtils.formatValue(value, !traceOn); + return Hints.getLogPrefix(hints) + "Encoding [" + formatted + "]"; + }); } JavaType javaType = getJavaType(elementType.getType(), null); diff --git a/spring-web/src/main/java/org/springframework/http/codec/json/Jackson2CodecSupport.java b/spring-web/src/main/java/org/springframework/http/codec/json/Jackson2CodecSupport.java index 1b2439f41a..ff3efa860c 100644 --- a/spring-web/src/main/java/org/springframework/http/codec/json/Jackson2CodecSupport.java +++ b/spring-web/src/main/java/org/springframework/http/codec/json/Jackson2CodecSupport.java @@ -125,12 +125,4 @@ public abstract class Jackson2CodecSupport { @Nullable protected abstract A getAnnotation(MethodParameter parameter, Class annotType); - String formatValue(@Nullable Object value, boolean logFullValue) { - if (value == null) { - return ""; - } - String s = value instanceof CharSequence ? "\"" + value + "\"" : value.toString(); - return logFullValue || s.length() < 100 ? s : s.substring(0, 100) + " (truncated)..."; - } - } diff --git a/spring-web/src/main/java/org/springframework/http/codec/multipart/MultipartHttpMessageReader.java b/spring-web/src/main/java/org/springframework/http/codec/multipart/MultipartHttpMessageReader.java index 79fe01e2ca..0e8ca328b5 100644 --- a/spring-web/src/main/java/org/springframework/http/codec/multipart/MultipartHttpMessageReader.java +++ b/spring-web/src/main/java/org/springframework/http/codec/multipart/MultipartHttpMessageReader.java @@ -28,6 +28,7 @@ import reactor.core.publisher.Mono; import org.springframework.core.ResolvableType; import org.springframework.core.codec.Hints; +import org.springframework.core.log.LogFormatUtils; import org.springframework.http.MediaType; import org.springframework.http.ReactiveHttpInputMessage; import org.springframework.http.codec.HttpMessageReader; @@ -94,18 +95,10 @@ public class MultipartHttpMessageReader extends LoggingCodecSupport return this.partReader.read(elementType, inputMessage, allHints) .collectMultimap(Part::name) .doOnNext(map -> { - if (logger.isDebugEnabled()) { - String s = Hints.getLogPrefix(hints) + "Parsed " + - (isEnableLoggingRequestDetails() ? - formatValue(map, logger.isTraceEnabled()) : - "parts " + map.keySet() + " (content masked)"); - if (logger.isTraceEnabled()) { - logger.trace(s); - } - else { - logger.debug(s); - } - } + LogFormatUtils.traceDebug(logger, traceOn -> Hints.getLogPrefix(hints) + "Parsed " + + (isEnableLoggingRequestDetails() ? + LogFormatUtils.formatValue(map, !traceOn) : + "parts " + map.keySet() + " (content masked)")); }) .map(this::toMultiValueMap); } diff --git a/spring-web/src/main/java/org/springframework/http/codec/multipart/MultipartHttpMessageWriter.java b/spring-web/src/main/java/org/springframework/http/codec/multipart/MultipartHttpMessageWriter.java index c1e35f3459..26b8ab9c7a 100644 --- a/spring-web/src/main/java/org/springframework/http/codec/multipart/MultipartHttpMessageWriter.java +++ b/spring-web/src/main/java/org/springframework/http/codec/multipart/MultipartHttpMessageWriter.java @@ -41,6 +41,7 @@ import org.springframework.core.io.Resource; import org.springframework.core.io.buffer.DataBuffer; import org.springframework.core.io.buffer.DataBufferFactory; import org.springframework.core.io.buffer.DefaultDataBufferFactory; +import org.springframework.core.log.LogFormatUtils; import org.springframework.http.HttpEntity; import org.springframework.http.HttpHeaders; import org.springframework.http.MediaType; @@ -225,18 +226,10 @@ public class MultipartHttpMessageWriter extends LoggingCodecSupport outputMessage.getHeaders().setContentType(new MediaType(MediaType.MULTIPART_FORM_DATA, params)); - if (logger.isDebugEnabled()) { - String s = Hints.getLogPrefix(hints) + "Encoding " + - (isEnableLoggingRequestDetails() ? - formatValue(map, logger.isTraceEnabled()) : - "parts " + map.keySet() + " (content masked)"); - if (logger.isTraceEnabled()) { - logger.trace(s); - } - else { - logger.debug(s); - } - } + LogFormatUtils.traceDebug(logger, traceOn -> Hints.getLogPrefix(hints) + "Encoding " + + (isEnableLoggingRequestDetails() ? + LogFormatUtils.formatValue(map, !traceOn) : + "parts " + map.keySet() + " (content masked)")); Flux body = Flux.fromIterable(map.entrySet()) .concatMap(entry -> encodePartValues(boundary, entry.getKey(), entry.getValue())) diff --git a/spring-web/src/main/java/org/springframework/http/codec/multipart/SynchronossPartHttpMessageReader.java b/spring-web/src/main/java/org/springframework/http/codec/multipart/SynchronossPartHttpMessageReader.java index 4168ed02a8..a41db131a0 100644 --- a/spring-web/src/main/java/org/springframework/http/codec/multipart/SynchronossPartHttpMessageReader.java +++ b/spring-web/src/main/java/org/springframework/http/codec/multipart/SynchronossPartHttpMessageReader.java @@ -50,6 +50,7 @@ import org.springframework.core.io.buffer.DataBuffer; import org.springframework.core.io.buffer.DataBufferFactory; import org.springframework.core.io.buffer.DataBufferUtils; import org.springframework.core.io.buffer.DefaultDataBufferFactory; +import org.springframework.core.log.LogFormatUtils; import org.springframework.http.HttpHeaders; import org.springframework.http.MediaType; import org.springframework.http.ReactiveHttpInputMessage; @@ -95,17 +96,11 @@ public class SynchronossPartHttpMessageReader extends LoggingCodecSupport implem public Flux read(ResolvableType elementType, ReactiveHttpInputMessage message, Map hints) { return Flux.create(new SynchronossPartGenerator(message, this.bufferFactory, this.streamStorageFactory)) .doOnNext(part -> { - if (logger.isDebugEnabled() && !Hints.isLoggingSuppressed(hints)) { - String s = Hints.getLogPrefix(hints) + "Parsed " + + if (!Hints.isLoggingSuppressed(hints)) { + LogFormatUtils.traceDebug(logger, traceOn -> Hints.getLogPrefix(hints) + "Parsed " + (isEnableLoggingRequestDetails() ? - formatValue(part, logger.isTraceEnabled()) : - "parts '" + part.name() + "' (content masked)"); - if (logger.isTraceEnabled()) { - logger.trace(s); - } - else { - logger.debug(s); - } + LogFormatUtils.formatValue(part, !traceOn) : + "parts '" + part.name() + "' (content masked)")); } }); } diff --git a/spring-web/src/main/java/org/springframework/http/codec/xml/Jaxb2XmlDecoder.java b/spring-web/src/main/java/org/springframework/http/codec/xml/Jaxb2XmlDecoder.java index ec02c715f2..8e9745b4b8 100644 --- a/spring-web/src/main/java/org/springframework/http/codec/xml/Jaxb2XmlDecoder.java +++ b/spring-web/src/main/java/org/springframework/http/codec/xml/Jaxb2XmlDecoder.java @@ -42,6 +42,7 @@ import org.springframework.core.codec.CodecException; import org.springframework.core.codec.DecodingException; import org.springframework.core.codec.Hints; import org.springframework.core.io.buffer.DataBuffer; +import org.springframework.core.log.LogFormatUtils; import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; @@ -107,16 +108,10 @@ public class Jaxb2XmlDecoder extends AbstractDecoder { return splitEvents.map(events -> { Object value = unmarshal(events, outputClass); - if (logger.isDebugEnabled()) { - boolean traceOn = logger.isTraceEnabled(); - String s = Hints.getLogPrefix(hints) + "Decoded [" + formatValue(value, traceOn) + "]"; - if (traceOn) { - logger.trace(s); - } - else { - logger.debug(s); - } - } + LogFormatUtils.traceDebug(logger, traceOn -> { + String formatted = LogFormatUtils.formatValue(value, !traceOn); + return Hints.getLogPrefix(hints) + "Decoded [" + formatted + "]"; + }); return value; }); } @@ -213,14 +208,6 @@ public class Jaxb2XmlDecoder extends AbstractDecoder { return xmlEventFlux.flatMap(new SplitFunction(desiredName)); } - String formatValue(@Nullable Object value, boolean logFullValue) { - if (value == null) { - return ""; - } - String s = value instanceof CharSequence ? "\"" + value + "\"" : value.toString(); - return logFullValue || s.length() < 100 ? s : s.substring(0, 100) + " (truncated)..."; - } - private static class SplitFunction implements Function>> { diff --git a/spring-web/src/main/java/org/springframework/http/codec/xml/Jaxb2XmlEncoder.java b/spring-web/src/main/java/org/springframework/http/codec/xml/Jaxb2XmlEncoder.java index 5d991268a4..e1b71e08c0 100644 --- a/spring-web/src/main/java/org/springframework/http/codec/xml/Jaxb2XmlEncoder.java +++ b/spring-web/src/main/java/org/springframework/http/codec/xml/Jaxb2XmlEncoder.java @@ -34,6 +34,7 @@ import org.springframework.core.codec.EncodingException; import org.springframework.core.codec.Hints; import org.springframework.core.io.buffer.DataBuffer; import org.springframework.core.io.buffer.DataBufferFactory; +import org.springframework.core.log.LogFormatUtils; import org.springframework.lang.Nullable; import org.springframework.util.ClassUtils; import org.springframework.util.MimeType; @@ -78,15 +79,11 @@ public class Jaxb2XmlEncoder extends AbstractSingleValueEncoder { protected Flux encode(Object value, DataBufferFactory dataBufferFactory, ResolvableType type, @Nullable MimeType mimeType, @Nullable Map hints) { try { - if (logger.isDebugEnabled() && !Hints.isLoggingSuppressed(hints)) { - boolean traceOn = logger.isTraceEnabled(); - String s = Hints.getLogPrefix(hints) + "Encoding [" + formatValue(value, traceOn) + "]"; - if (traceOn) { - logger.trace(s); - } - else { - logger.debug(s); - } + if (!Hints.isLoggingSuppressed(hints)) { + LogFormatUtils.traceDebug(logger, traceOn -> { + String formatted = LogFormatUtils.formatValue(value, !traceOn); + return Hints.getLogPrefix(hints) + "Encoding [" + formatted + "]"; + }); } DataBuffer buffer = dataBufferFactory.allocateBuffer(1024); OutputStream outputStream = buffer.asOutputStream(); @@ -104,12 +101,4 @@ public class Jaxb2XmlEncoder extends AbstractSingleValueEncoder { } } - String formatValue(@Nullable Object value, boolean logFullValue) { - if (value == null) { - return ""; - } - String s = value instanceof CharSequence ? "\"" + value + "\"" : value.toString(); - return logFullValue || s.length() < 100 ? s : s.substring(0, 100) + " (truncated)..."; - } - } diff --git a/spring-web/src/main/java/org/springframework/http/server/reactive/AbstractListenerWriteFlushProcessor.java b/spring-web/src/main/java/org/springframework/http/server/reactive/AbstractListenerWriteFlushProcessor.java index 361db0a889..0ae3115d09 100644 --- a/spring-web/src/main/java/org/springframework/http/server/reactive/AbstractListenerWriteFlushProcessor.java +++ b/spring-web/src/main/java/org/springframework/http/server/reactive/AbstractListenerWriteFlushProcessor.java @@ -49,7 +49,8 @@ public abstract class AbstractListenerWriteFlushProcessor implements Processo * @see AbstractListenerWriteProcessor#rsWriteLogger * @see WriteResultPublisher#rsWriteResultLogger */ - protected static final Log rsWriteFlushLogger = LogDelegateFactory.getHiddenLog(AbstractListenerWriteFlushProcessor.class); + protected static final Log rsWriteFlushLogger = + LogDelegateFactory.getHiddenLog(AbstractListenerWriteFlushProcessor.class); private final AtomicReference state = new AtomicReference<>(State.UNSUBSCRIBED); diff --git a/spring-web/src/main/java/org/springframework/web/multipart/commons/CommonsFileUploadSupport.java b/spring-web/src/main/java/org/springframework/web/multipart/commons/CommonsFileUploadSupport.java index 317f38ab63..8bf2e75dbc 100644 --- a/spring-web/src/main/java/org/springframework/web/multipart/commons/CommonsFileUploadSupport.java +++ b/spring-web/src/main/java/org/springframework/web/multipart/commons/CommonsFileUploadSupport.java @@ -31,6 +31,7 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.core.io.Resource; +import org.springframework.core.log.LogFormatUtils; import org.springframework.http.MediaType; import org.springframework.lang.Nullable; import org.springframework.util.LinkedMultiValueMap; @@ -281,16 +282,11 @@ public abstract class CommonsFileUploadSupport { // multipart file field CommonsMultipartFile file = createMultipartFile(fileItem); multipartFiles.add(file.getName(), file); - if (logger.isDebugEnabled()) { - String message = "Part '" + file.getName() + "', " + - "size " + file.getSize() + " bytes, filename='" + file.getOriginalFilename() + "'"; - if (logger.isTraceEnabled()) { - logger.trace(message + ", storage=" + file.getStorageDescription()); - } - else { - logger.debug(message); - } - } + LogFormatUtils.traceDebug(logger, traceOn -> + "Part '" + file.getName() + "', size " + file.getSize() + + " bytes, filename='" + file.getOriginalFilename() + "'" + + (traceOn ? ", storage=" + file.getStorageDescription() : "") + ); } } return new MultipartParsingResult(multipartFiles, multipartParameters, multipartParameterContentTypes); @@ -323,16 +319,10 @@ public abstract class CommonsFileUploadSupport { if (file instanceof CommonsMultipartFile) { CommonsMultipartFile cmf = (CommonsMultipartFile) file; cmf.getFileItem().delete(); - if (logger.isDebugEnabled()) { - String filename = cmf.getOriginalFilename(); - String message = "Cleaning up part '" + cmf.getName() + "', filename '" + filename + "'"; - if (logger.isTraceEnabled()) { - logger.trace(message + ", stored " + cmf.getStorageDescription()); - } - else { - logger.debug(message); - } - } + LogFormatUtils.traceDebug(logger, traceOn -> + "Cleaning up part '" + cmf.getName() + + "', filename '" + cmf.getOriginalFilename() + "'" + + (traceOn ? ", stored " + cmf.getStorageDescription() : "")); } } } diff --git a/spring-web/src/main/java/org/springframework/web/multipart/commons/CommonsMultipartFile.java b/spring-web/src/main/java/org/springframework/web/multipart/commons/CommonsMultipartFile.java index 727cc1056f..69a1923832 100644 --- a/spring-web/src/main/java/org/springframework/web/multipart/commons/CommonsMultipartFile.java +++ b/spring-web/src/main/java/org/springframework/web/multipart/commons/CommonsMultipartFile.java @@ -29,6 +29,7 @@ import org.apache.commons.fileupload.disk.DiskFileItem; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.springframework.core.log.LogFormatUtils; import org.springframework.util.FileCopyUtils; import org.springframework.util.StreamUtils; import org.springframework.web.multipart.MultipartFile; @@ -165,20 +166,15 @@ public class CommonsMultipartFile implements MultipartFile, Serializable { try { this.fileItem.write(dest); - if (logger.isDebugEnabled()) { + LogFormatUtils.traceDebug(logger, traceOn -> { String action = "transferred"; if (!this.fileItem.isInMemory()) { action = (isAvailable() ? "copied" : "moved"); } - String message = "Part '" + getName() + "', filename '" + getOriginalFilename() + "'"; - if (logger.isTraceEnabled()) { - logger.trace(message + ", stored " + getStorageDescription() + ": " + action + - " to [" + dest.getAbsolutePath() + "]"); - } - else { - logger.debug(message + ": " + action + " to [" + dest.getAbsolutePath() + "]"); - } - } + return "Part '" + getName() + "', filename '" + getOriginalFilename() + "'" + + (traceOn ? ", stored " + getStorageDescription() : "") + + ": " + action + " to [" + dest.getAbsolutePath() + "]"; + }); } catch (FileUploadException ex) { throw new IllegalStateException(ex.getMessage(), ex); diff --git a/spring-web/src/main/java/org/springframework/web/server/adapter/HttpWebHandlerAdapter.java b/spring-web/src/main/java/org/springframework/web/server/adapter/HttpWebHandlerAdapter.java index 8495bb3e1c..4a9a682605 100644 --- a/spring-web/src/main/java/org/springframework/web/server/adapter/HttpWebHandlerAdapter.java +++ b/spring-web/src/main/java/org/springframework/web/server/adapter/HttpWebHandlerAdapter.java @@ -26,6 +26,7 @@ import reactor.core.publisher.Mono; import org.springframework.context.ApplicationContext; import org.springframework.core.NestedExceptionUtils; +import org.springframework.core.log.LogFormatUtils; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatus; import org.springframework.http.codec.LoggingCodecSupport; @@ -237,7 +238,10 @@ public class HttpWebHandlerAdapter extends WebHandlerDecorator implements HttpHa } ServerWebExchange exchange = createExchange(request, response); - logExchange(exchange); + + LogFormatUtils.traceDebug(logger, traceOn -> + exchange.getLogPrefix() + formatRequest(exchange.getRequest()) + + (traceOn ? ", headers=" + formatHeaders(exchange.getRequest().getHeaders()) : "")); return getDelegate().handle(exchange) .doOnSuccess(aVoid -> logResponse(exchange)) @@ -250,19 +254,6 @@ public class HttpWebHandlerAdapter extends WebHandlerDecorator implements HttpHa getCodecConfigurer(), getLocaleContextResolver(), this.applicationContext); } - private void logExchange(ServerWebExchange exchange) { - if (logger.isDebugEnabled()) { - String logPrefix = exchange.getLogPrefix(); - ServerHttpRequest request = exchange.getRequest(); - if (logger.isTraceEnabled()) { - logger.trace(logPrefix + formatRequest(request) + ", headers=" + formatHeaders(request.getHeaders())); - } - else { - logger.debug(logPrefix + formatRequest(request)); - } - } - } - private String formatRequest(ServerHttpRequest request) { String rawQuery = request.getURI().getRawQuery(); String query = StringUtils.hasText(rawQuery) ? "?" + rawQuery : ""; @@ -270,18 +261,11 @@ public class HttpWebHandlerAdapter extends WebHandlerDecorator implements HttpHa } private void logResponse(ServerWebExchange exchange) { - if (logger.isDebugEnabled()) { - String logPrefix = exchange.getLogPrefix(); - ServerHttpResponse response = exchange.getResponse(); - HttpStatus status = response.getStatusCode(); - String message = "Completed " + (status != null ? status : "200 OK"); - if (logger.isTraceEnabled()) { - logger.trace(logPrefix + message + ", headers=" + formatHeaders(response.getHeaders())); - } - else { - logger.debug(logPrefix + message); - } - } + LogFormatUtils.traceDebug(logger, traceOn -> { + HttpStatus status = exchange.getResponse().getStatusCode(); + return exchange.getLogPrefix() + "Completed " + (status != null ? status : "200 OK") + + (traceOn ? ", headers=" + formatHeaders(exchange.getResponse().getHeaders()) : ""); + }); } private String formatHeaders(HttpHeaders responseHeaders) { diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/ExchangeFunctions.java b/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/ExchangeFunctions.java index 5377ea5197..a141869e6f 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/ExchangeFunctions.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/ExchangeFunctions.java @@ -22,6 +22,7 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import reactor.core.publisher.Mono; +import org.springframework.core.log.LogFormatUtils; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpMethod; import org.springframework.http.HttpStatus; @@ -109,29 +110,19 @@ public abstract class ExchangeFunctions { } private void logRequest(ClientRequest request) { - if (logger.isDebugEnabled()) { - String message = request.logPrefix() + "HTTP " + request.method() + " " + request.url(); - if (logger.isTraceEnabled()) { - logger.trace(message + ", headers=" + formatHeaders(request.headers())); - } - else { - logger.debug(message); - } - } + LogFormatUtils.traceDebug(logger, traceOn -> + request.logPrefix() + "HTTP " + request.method() + " " + request.url() + + (traceOn ? ", headers=" + formatHeaders(request.headers()) : "") + ); } private void logResponse(ClientHttpResponse response, String logPrefix) { - if (logger.isDebugEnabled()) { + LogFormatUtils.traceDebug(logger, traceOn -> { int code = response.getRawStatusCode(); HttpStatus status = HttpStatus.resolve(code); - String message = logPrefix + "Response " + (status != null ? status : code); - if (logger.isTraceEnabled()) { - logger.trace(message + ", headers=" + formatHeaders(response.getHeaders())); - } - else { - logger.debug(message); - } - } + return logPrefix + "Response " + (status != null ? status : code) + + (traceOn ? ", headers=" + formatHeaders(response.getHeaders()) : ""); + }); } private String formatHeaders(HttpHeaders headers) { diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/DispatcherServlet.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/DispatcherServlet.java index 1e3945819b..d1a1bda6af 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/DispatcherServlet.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/DispatcherServlet.java @@ -48,6 +48,7 @@ import org.springframework.context.i18n.LocaleContext; import org.springframework.core.annotation.AnnotationAwareOrderComparator; import org.springframework.core.io.ClassPathResource; import org.springframework.core.io.support.PropertiesLoaderUtils; +import org.springframework.core.log.LogFormatUtils; import org.springframework.http.server.ServletServerHttpRequest; import org.springframework.lang.Nullable; import org.springframework.ui.context.ThemeSource; @@ -951,7 +952,7 @@ public class DispatcherServlet extends FrameworkServlet { } private void logRequest(HttpServletRequest request) { - if (logger.isDebugEnabled()) { + LogFormatUtils.traceDebug(logger, traceOn -> { String params; if (isEnableLoggingRequestDetails()) { params = request.getParameterMap().entrySet().stream() @@ -968,19 +969,19 @@ public class DispatcherServlet extends FrameworkServlet { String message = (dispatchType + request.getMethod() + " \"" + getRequestUri(request) + query + "\", parameters={" + params + "}"); - if (logger.isTraceEnabled()) { + if (traceOn) { List values = Collections.list(request.getHeaderNames()); String headers = values.size() > 0 ? "masked" : ""; if (isEnableLoggingRequestDetails()) { headers = values.stream().map(name -> name + ":" + Collections.list(request.getHeaders(name))) .collect(Collectors.joining(", ")); } - logger.trace(message + ", headers={" + headers + "} in DispatcherServlet '" + getServletName() + "'"); + return message + ", headers={" + headers + "} in DispatcherServlet '" + getServletName() + "'"; } else { - logger.debug(message); + return message; } - } + }); } /** diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/FrameworkServlet.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/FrameworkServlet.java index 59217870d9..95449f2715 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/FrameworkServlet.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/FrameworkServlet.java @@ -1082,7 +1082,9 @@ public abstract class FrameworkServlet extends HttpServletBean implements Applic if (failureCause != null) { if (!initialDispatch) { // FORWARD/ERROR/ASYNC: minimal message (there should be enough context already) - logger.debug("Unresolved failure from \"" + dispatchType + "\" dispatch: " + failureCause); + if (logger.isDebugEnabled()) { + logger.debug("Unresolved failure from \"" + dispatchType + "\" dispatch: " + failureCause); + } } else if (logger.isTraceEnabled()) { logger.trace("Failed to complete request", failureCause); diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AbstractMessageConverterMethodArgumentResolver.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AbstractMessageConverterMethodArgumentResolver.java index acb4c41977..bb68fcf44b 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AbstractMessageConverterMethodArgumentResolver.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AbstractMessageConverterMethodArgumentResolver.java @@ -37,6 +37,7 @@ import org.apache.commons.logging.LogFactory; import org.springframework.core.MethodParameter; import org.springframework.core.ResolvableType; import org.springframework.core.annotation.AnnotationUtils; +import org.springframework.core.log.LogFormatUtils; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpInputMessage; import org.springframework.http.HttpMethod; @@ -223,17 +224,12 @@ public abstract class AbstractMessageConverterMethodArgumentResolver implements throw new HttpMediaTypeNotSupportedException(contentType, this.allSupportedMediaTypes); } - if (logger.isDebugEnabled()) { - boolean traceOn = logger.isTraceEnabled(); - String s = "Read \"" + contentType + "\" to [" + - RequestMappingHandlerAdapter.formatValue(body, traceOn) + "]"; - if (traceOn) { - logger.trace(s); - } - else { - logger.debug(s); - } - } + MediaType selectedContentType = contentType; + Object theBody = body; + LogFormatUtils.traceDebug(logger, traceOn -> { + String formatted = LogFormatUtils.formatValue(theBody, !traceOn); + return "Read \"" + selectedContentType + "\" to [" + formatted + "]"; + }); return body; } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AbstractMessageConverterMethodProcessor.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AbstractMessageConverterMethodProcessor.java index 58df0c4f32..9b3d43071b 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AbstractMessageConverterMethodProcessor.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AbstractMessageConverterMethodProcessor.java @@ -35,6 +35,7 @@ import org.springframework.core.ResolvableType; import org.springframework.core.io.InputStreamResource; import org.springframework.core.io.Resource; import org.springframework.core.io.support.ResourceRegion; +import org.springframework.core.log.LogFormatUtils; import org.springframework.http.HttpEntity; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpOutputMessage; @@ -280,16 +281,9 @@ public abstract class AbstractMessageConverterMethodProcessor extends AbstractMe (Class>) converter.getClass(), inputMessage, outputMessage); if (body != null) { - if (logger.isDebugEnabled()) { - boolean traceOn = logger.isTraceEnabled(); - String s = "Writing [" + RequestMappingHandlerAdapter.formatValue(body, traceOn) + "]"; - if (traceOn) { - logger.trace(s); - } - else { - logger.debug(s); - } - } + Object theBody = body; + LogFormatUtils.traceDebug(logger, traceOn -> + "Writing [" + LogFormatUtils.formatValue(theBody, traceOn) + "]"); addContentDispositionHeader(inputMessage, outputMessage); if (genericConverter != null) { genericConverter.write(body, targetType, selectedMediaType, outputMessage); diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapter.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapter.java index f859b730bf..02bc6e801e 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapter.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapter.java @@ -38,6 +38,7 @@ import org.springframework.core.ParameterNameDiscoverer; import org.springframework.core.ReactiveAdapterRegistry; import org.springframework.core.annotation.AnnotatedElementUtils; import org.springframework.core.annotation.AnnotationAwareOrderComparator; +import org.springframework.core.log.LogFormatUtils; import org.springframework.core.task.AsyncTaskExecutor; import org.springframework.core.task.SimpleAsyncTaskExecutor; import org.springframework.http.converter.ByteArrayHttpMessageConverter; @@ -884,15 +885,10 @@ public class RequestMappingHandlerAdapter extends AbstractHandlerMethodAdapter Object result = asyncManager.getConcurrentResult(); mavContainer = (ModelAndViewContainer) asyncManager.getConcurrentResultContext()[0]; asyncManager.clearConcurrentResult(); - if (logger.isDebugEnabled()) { - String s = "Resume with async result [" + formatValue(result, logger.isTraceEnabled()) + "]"; - if (logger.isTraceEnabled()) { - logger.trace(s); - } - else { - logger.debug(s); - } - } + LogFormatUtils.traceDebug(logger, traceOn -> { + String formatted = LogFormatUtils.formatValue(result, !traceOn); + return "Resume with async result [" + formatted + "]"; + }); invocableMethod = invocableMethod.wrapConcurrentResult(result); } @@ -1024,12 +1020,4 @@ public class RequestMappingHandlerAdapter extends AbstractHandlerMethodAdapter return mav; } - static String formatValue(@Nullable Object body, boolean logFullBody) { - if (body == null) { - return ""; - } - String s = body instanceof CharSequence ? "\"" + body + "\"" : body.toString(); - return logFullBody || s.length() < 100 ? s : s.substring(0, 100) + " (truncated)..."; - } - }