Spring/spring-integration
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

INT-4144: Prevent NPE in the LoggingHandler

Browse Source 
JIRA: https://jira.spring.io/browse/INT-4144
Fixes GH-1936 (https://github.com/spring-projects/spring-integration/issues/1936)

Previously `LoggingHandler` could be used as a standalone object, without any Spring Container initialization.
Even if that doesn't sound reasonable, we should reinstate the logic to avoid breaking changes

* Initialize `expression` and `evaluationContext` during object `<init>` phase
* Some code reformatting to avoid duplicate blocks and cyclomatic complexity

* Simple code formatting
* Additional JavaDocs for `LoggingHandler`

**Cherry-pick to 4.3.x**
This commit is contained in:
Andriy Kryvtsun
2016-10-21 17:13:37 -04:00
committed by Artem Bilan
parent a1a3c2a1ac
commit ee848e9ecf
2 changed files with 95 additions and 63 deletions
Download Patch File Download Diff File Expand all files Collapse all files

136
spring-integration-core/src/main/java/org/springframework/integration/handler/LoggingHandler.java
View File

@@ -18,6 +18,7 @@ package org.springframework.integration.handler;
import java.io.PrintWriter;
import java.io.StringWriter;
import java.io.Writer;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
@@ -32,33 +33,36 @@ import org.springframework.util.StringUtils;
/**
* MessageHandler implementation that simply logs the Message or its payload depending on the value of the
* 'shouldLogFullMessage' property. If logging the payload, and it is assignable to Throwable, it will log the stack
* 'shouldLogFullMessage' or SpEL 'logExpression' property.
* If logging the payload, and it is assignable to Throwable, it will log the stack
* trace. By default, it will log the payload only.
*
* @author Mark Fisher
* @author Gary Russell
* @author Artem Bilan
* @author Andriy Kryvtsun
* @since 1.0.1
*/
public class LoggingHandler extends AbstractMessageHandler {
public enum Level {
FATAL, ERROR, WARN, INFO, DEBUG, TRACE
}
private volatile Expression expression;
private volatile boolean expressionSet;
private volatile boolean shouldLogFullMessageSet;
private volatile Level level;
private volatile EvaluationContext evaluationContext;
private volatile boolean expressionSet;
private volatile Expression expression = EXPRESSION_PARSER.parseExpression("payload");
private volatile EvaluationContext evaluationContext = ExpressionUtils.createStandardEvaluationContext();
private volatile boolean shouldLogFullMessageSet;
private volatile Log messageLogger = this.logger;
/**
* Create a LoggingHandler with the given log level (case-insensitive).
* <p>
@@ -68,9 +72,13 @@ public class LoggingHandler extends AbstractMessageHandler {
* @see #LoggingHandler(Level)
*/
public LoggingHandler(String level) {
this(convertLevel(level));
}
private static Level convertLevel(String level) {
Assert.hasText(level, "'level' cannot be empty");
try {
this.level = Level.valueOf(level.toUpperCase());
return Level.valueOf(level.toUpperCase());
}
catch (IllegalArgumentException e) {
throw new IllegalArgumentException("Invalid log level '" + level
@@ -85,8 +93,7 @@ public class LoggingHandler extends AbstractMessageHandler {
* @since 4.3
*/
public LoggingHandler(Level level) {
Assert.notNull(level, "'level' cannot be null");
this.level = level;
doSetLevel(level);
}
/**
@@ -125,6 +132,10 @@ public class LoggingHandler extends AbstractMessageHandler {
* @param level the level.
*/
public void setLevel(Level level) {
doSetLevel(level);
}
private void doSetLevel(Level level) {
Assert.notNull(level, "'level' cannot be null");
this.level = level;
}
@@ -142,7 +153,7 @@ public class LoggingHandler extends AbstractMessageHandler {
public void setShouldLogFullMessage(boolean shouldLogFullMessage) {
Assert.isTrue(!(this.expressionSet), "Cannot set both 'expression' AND 'shouldLogFullMessage' properties");
this.shouldLogFullMessageSet = true;
this.expression = (shouldLogFullMessage)
this.expression = shouldLogFullMessage
? EXPRESSION_PARSER.parseExpression("#root")
: EXPRESSION_PARSER.parseExpression("payload");
}
@@ -156,66 +167,69 @@ public class LoggingHandler extends AbstractMessageHandler {
protected void onInit() throws Exception {
super.onInit();
this.evaluationContext = ExpressionUtils.createStandardEvaluationContext(getBeanFactory());
if (this.expression == null) {
this.expression = EXPRESSION_PARSER.parseExpression("payload");
}
}
@Override
protected void handleMessageInternal(Message<?> message) throws Exception {
switch (this.level) {
case FATAL:
if (this.messageLogger.isFatalEnabled()) {
this.messageLogger.fatal(createLogMessage(message));
}
break;
case ERROR:
if (this.messageLogger.isErrorEnabled()) {
this.messageLogger.error(createLogMessage(message));
}
break;
case WARN:
if (this.messageLogger.isWarnEnabled()) {
this.messageLogger.warn(createLogMessage(message));
}
break;
case INFO:
if (this.messageLogger.isInfoEnabled()) {
this.messageLogger.info(createLogMessage(message));
}
break;
case DEBUG:
if (this.messageLogger.isDebugEnabled()) {
this.messageLogger.debug(createLogMessage(message));
}
break;
case TRACE:
if (this.messageLogger.isTraceEnabled()) {
this.messageLogger.trace(createLogMessage(message));
}
break;
default:
throw new IllegalStateException("Level '" + this.level + "' is not supported");
case FATAL:
if (this.messageLogger.isFatalEnabled()) {
this.messageLogger.fatal(createLogMessage(message));
}
break;
case ERROR:
if (this.messageLogger.isErrorEnabled()) {
this.messageLogger.error(createLogMessage(message));
}
break;
case WARN:
if (this.messageLogger.isWarnEnabled()) {
this.messageLogger.warn(createLogMessage(message));
}
break;
case INFO:
if (this.messageLogger.isInfoEnabled()) {
this.messageLogger.info(createLogMessage(message));
}
break;
case DEBUG:
if (this.messageLogger.isDebugEnabled()) {
this.messageLogger.debug(createLogMessage(message));
}
break;
case TRACE:
if (this.messageLogger.isTraceEnabled()) {
this.messageLogger.trace(createLogMessage(message));
}
break;
default:
throw new IllegalStateException("Level '" + this.level + "' is not supported");
}
}
private Object createLogMessage(Message<?> message) {
Object logMessage = this.expression.getValue(this.evaluationContext, message);
if (logMessage instanceof Throwable) {
StringWriter stringWriter = new StringWriter();
if (logMessage instanceof AggregateMessageDeliveryException) {
stringWriter.append(((Throwable) logMessage).getMessage());
for (Exception exception : ((AggregateMessageDeliveryException) logMessage).getAggregatedExceptions()) {
exception.printStackTrace(new PrintWriter(stringWriter, true));
}
return logMessage instanceof Throwable
? createLogMessage((Throwable) logMessage)
: logMessage;
}
private String createLogMessage(Throwable throwable) {
StringWriter stringWriter = new StringWriter();
if (throwable instanceof AggregateMessageDeliveryException) {
stringWriter.append(throwable.getMessage());
for (Exception exception : ((AggregateMessageDeliveryException) throwable).getAggregatedExceptions()) {
printStackTrace(exception, stringWriter);
}
else {
((Throwable) logMessage).printStackTrace(new PrintWriter(stringWriter, true));
}
logMessage = stringWriter.toString();
}
return logMessage;
else {
printStackTrace(throwable, stringWriter);
}
return stringWriter.toString();
}
private void printStackTrace(Throwable throwable, Writer writer) {
throwable.printStackTrace(new PrintWriter(writer, true));
}
}

22
spring-integration-core/src/test/java/org/springframework/integration/handler/LoggingHandlerTests.java
View File

@@ -29,22 +29,24 @@ import org.apache.commons.logging.Log;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.Mockito;
import org.springframework.beans.DirectFieldAccessor;
import org.springframework.beans.factory.BeanFactory;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.expression.EvaluationContext;
import org.springframework.expression.Expression;
import org.springframework.integration.handler.LoggingHandler.Level;
import org.springframework.integration.support.MessageBuilder;
import org.springframework.messaging.Message;
import org.springframework.messaging.MessageChannel;
import org.springframework.integration.handler.LoggingHandler.Level;
import org.springframework.messaging.support.GenericMessage;
import org.springframework.integration.support.MessageBuilder;
import org.springframework.test.context.ContextConfiguration;
import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
/**
* @author Mark Fisher
* @author Artem Bilan
* @author Andriy Kryvtsun
* @since 2.0
*/
@ContextConfiguration
@@ -126,6 +128,22 @@ public class LoggingHandlerTests {
verify(log, times(1)).warn(Mockito.anyString());
}
@Test
public void testUsageWithoutSpringInitialization() {
LoggingHandler loggingHandler = new LoggingHandler("ERROR");
DirectFieldAccessor accessor = new DirectFieldAccessor(loggingHandler);
Log log = (Log) accessor.getPropertyValue("messageLogger");
log = spy(log);
accessor.setPropertyValue("messageLogger", log);
String testPayload = "TEST_PAYLOAD";
Message<String> message = MessageBuilder.withPayload(testPayload).build();
loggingHandler.handleMessage(message);
verify(log).error(testPayload);
}
public static class TestBean {
private final String name;