From d54a88f52837f381add5b43e8c74ccc93fe4693f Mon Sep 17 00:00:00 2001 From: Gary Russell Date: Tue, 26 Feb 2013 18:35:19 -0500 Subject: [PATCH 1/2] INT-2939 LoggingHandler Improvements - Don't evaluate expression if the log level is not enabled - Add mutually exclusive assertions (already protected by the parser but not for programmatic configuration). INT-2944 Document Add reference sub-chapter. INT-2939 Polishing - PR Comments - remove unnecessary cast. - add setter for level changes. Polishing * Change setter to use enum. * Add getter for current level. * Rename method to 'createLogMessage'. --- .../integration/handler/LoggingHandler.java | 118 +++++++++++------- .../xml/LoggingChannelAdapterParserTests.java | 3 +- .../handler/LoggingHandlerTests.java | 77 +++++++++++- src/reference/docbook/logging-adapter.xml | 70 +++++++++++ src/reference/docbook/messaging-endpoints.xml | 1 + 5 files changed, 222 insertions(+), 47 deletions(-) create mode 100644 src/reference/docbook/logging-adapter.xml diff --git a/spring-integration-core/src/main/java/org/springframework/integration/handler/LoggingHandler.java b/spring-integration-core/src/main/java/org/springframework/integration/handler/LoggingHandler.java index 9df1922376..2af36212c8 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/handler/LoggingHandler.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/handler/LoggingHandler.java @@ -1,11 +1,11 @@ /* - * Copyright 2002-2011 the original author or authors. - * + * Copyright 2002-2013 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. @@ -15,11 +15,9 @@ package org.springframework.integration.handler; import java.io.PrintWriter; import java.io.StringWriter; -import java.util.List; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; - import org.springframework.context.expression.MapAccessor; import org.springframework.expression.EvaluationContext; import org.springframework.expression.Expression; @@ -34,13 +32,14 @@ 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 * trace. By default, it will log the payload only. - * + * * @author Mark Fisher + * @author Gary Russell * @since 1.0.1 */ public class LoggingHandler extends AbstractMessageHandler { - private static enum Level { + public static enum Level { FATAL, ERROR, WARN, INFO, DEBUG, TRACE } @@ -49,7 +48,11 @@ public class LoggingHandler extends AbstractMessageHandler { private volatile Expression expression; - private final Level level; + private volatile boolean expressionSet; + + private volatile boolean shouldLogFullMessageSet; + + private volatile Level level; private final EvaluationContext evaluationContext; @@ -62,6 +65,7 @@ public class LoggingHandler extends AbstractMessageHandler { * The valid levels are: FATAL, ERROR, WARN, INFO, DEBUG, or TRACE */ public LoggingHandler(String level) { + Assert.notNull(level, "'level' cannot be null"); try { this.level = Level.valueOf(level.toUpperCase()); } @@ -76,11 +80,27 @@ public class LoggingHandler extends AbstractMessageHandler { this.expression = EXPRESSION_PARSER.parseExpression("payload"); } - public void setExpression(String expressionString) { + Assert.isTrue(!(this.shouldLogFullMessageSet), "Cannot set both 'expression' AND 'shouldLogFullMessage' properties"); + this.expressionSet = true; this.expression = EXPRESSION_PARSER.parseExpression(expressionString); } + /** + * @return The current logging {@link Level}. + */ + public Level getLevel() { + return level; + } + + /** + * Set the logging {@link Level}. + * @param level the level. + */ + public void setLevel(Level level) { + this.level = level; + } + public void setLoggerName(String loggerName) { Assert.hasText(loggerName, "loggerName must not be empty"); this.messageLogger = LogFactory.getLog(loggerName); @@ -91,6 +111,8 @@ public class LoggingHandler extends AbstractMessageHandler { * false by default. */ public void setShouldLogFullMessage(boolean shouldLogFullMessage) { + Assert.isTrue(!(this.expressionSet), "Cannot set both 'expression' AND 'shouldLogFullMessage' properties"); + this.shouldLogFullMessageSet = true; this.expression = (shouldLogFullMessage) ? EXPRESSION_PARSER.parseExpression("#root") : EXPRESSION_PARSER .parseExpression("payload"); } @@ -102,51 +124,59 @@ public class LoggingHandler extends AbstractMessageHandler { @Override protected void handleMessageInternal(Message message) throws Exception { + switch (this.level) { + case FATAL: + if (messageLogger.isFatalEnabled()) { + messageLogger.fatal(createLogMessage(message)); + } + break; + case ERROR: + if (messageLogger.isErrorEnabled()) { + messageLogger.error(createLogMessage(message)); + } + break; + case WARN: + if (messageLogger.isWarnEnabled()) { + messageLogger.warn(createLogMessage(message)); + } + break; + case INFO: + if (messageLogger.isInfoEnabled()) { + messageLogger.info(createLogMessage(message)); + } + break; + case DEBUG: + if (messageLogger.isDebugEnabled()) { + messageLogger.debug(createLogMessage(message)); + } + break; + case TRACE: + if (messageLogger.isTraceEnabled()) { + 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 : (List) ((AggregateMessageDeliveryException)logMessage).getAggregatedExceptions()) { + for (Exception exception : ((AggregateMessageDeliveryException)logMessage).getAggregatedExceptions()) { exception.printStackTrace(new PrintWriter(stringWriter, true)); } - } else { + } + else { ((Throwable) logMessage).printStackTrace(new PrintWriter(stringWriter, true)); } logMessage = stringWriter.toString(); } - switch (this.level) { - case FATAL: - if (messageLogger.isFatalEnabled()) { - messageLogger.fatal(logMessage); - } - break; - case ERROR: - if (messageLogger.isErrorEnabled()) { - messageLogger.error(logMessage); - } - break; - case WARN: - if (messageLogger.isWarnEnabled()) { - messageLogger.warn(logMessage); - } - break; - case INFO: - if (messageLogger.isInfoEnabled()) { - messageLogger.info(logMessage); - } - break; - case DEBUG: - if (messageLogger.isDebugEnabled()) { - messageLogger.debug(logMessage); - } - break; - case TRACE: - if (messageLogger.isTraceEnabled()) { - messageLogger.trace(logMessage); - } - break; - } + return logMessage; } } diff --git a/spring-integration-core/src/test/java/org/springframework/integration/config/xml/LoggingChannelAdapterParserTests.java b/spring-integration-core/src/test/java/org/springframework/integration/config/xml/LoggingChannelAdapterParserTests.java index 3ffc54f5b7..8f15c933aa 100644 --- a/spring-integration-core/src/test/java/org/springframework/integration/config/xml/LoggingChannelAdapterParserTests.java +++ b/spring-integration-core/src/test/java/org/springframework/integration/config/xml/LoggingChannelAdapterParserTests.java @@ -22,14 +22,12 @@ import static org.junit.Assert.fail; import org.junit.Test; import org.junit.runner.RunWith; - import org.springframework.beans.BeansException; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.beans.factory.parsing.BeanDefinitionParsingException; import org.springframework.context.support.ClassPathXmlApplicationContext; import org.springframework.core.Ordered; -import org.springframework.expression.spel.SpelEvaluationException; import org.springframework.integration.endpoint.EventDrivenConsumer; import org.springframework.integration.handler.LoggingHandler; import org.springframework.integration.test.util.TestUtils; @@ -39,6 +37,7 @@ import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; /** * @author Mark Fisher * @author Artem Bilan + * @author Gary Russell * @since 2.1 */ @ContextConfiguration diff --git a/spring-integration-core/src/test/java/org/springframework/integration/handler/LoggingHandlerTests.java b/spring-integration-core/src/test/java/org/springframework/integration/handler/LoggingHandlerTests.java index 0b92f9c571..50dc6b97cd 100644 --- a/spring-integration-core/src/test/java/org/springframework/integration/handler/LoggingHandlerTests.java +++ b/spring-integration-core/src/test/java/org/springframework/integration/handler/LoggingHandlerTests.java @@ -16,10 +16,25 @@ package org.springframework.integration.handler; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +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.annotation.Autowired; +import org.springframework.expression.EvaluationContext; +import org.springframework.expression.Expression; import org.springframework.integration.MessageChannel; +import org.springframework.integration.handler.LoggingHandler.Level; +import org.springframework.integration.message.GenericMessage; import org.springframework.integration.support.MessageBuilder; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; @@ -41,11 +56,71 @@ public class LoggingHandlerTests { input.send(MessageBuilder.withPayload(bean).setHeader("foo", "bar").build()); } + @Test + public void assertMutuallyExclusive() { + LoggingHandler loggingHandler = new LoggingHandler("INFO"); + loggingHandler.setExpression("'foo'"); + try { + loggingHandler.setShouldLogFullMessage(true); + fail("Expected IllegalArgumentException"); + } + catch (IllegalArgumentException e) { + assertEquals("Cannot set both 'expression' AND 'shouldLogFullMessage' properties", e.getMessage()); + } + + loggingHandler = new LoggingHandler("INFO"); + loggingHandler.setShouldLogFullMessage(true); + try { + loggingHandler.setExpression("'foo'"); + fail("Expected IllegalArgumentException"); + } + catch (IllegalArgumentException e) { + assertEquals("Cannot set both 'expression' AND 'shouldLogFullMessage' properties", e.getMessage()); + } + } + + @Test + public void testDontEvaluateIfNotEnabled() { + LoggingHandler loggingHandler = new LoggingHandler("INFO"); + DirectFieldAccessor accessor = new DirectFieldAccessor(loggingHandler); + Log log = (Log) accessor.getPropertyValue("messageLogger"); + log = spy(log); + accessor.setPropertyValue("messageLogger", log); + Expression expression = (Expression) accessor.getPropertyValue("expression"); + expression = spy(expression); + accessor.setPropertyValue("expression", expression); + when(log.isInfoEnabled()).thenReturn(false); + loggingHandler.handleMessage(new GenericMessage("foo")); + verify(expression, never()).getValue(Mockito.any(EvaluationContext.class), Mockito.any()); + + when(log.isInfoEnabled()).thenReturn(true); + loggingHandler.handleMessage(new GenericMessage("foo")); + verify(expression, times(1)).getValue(Mockito.any(EvaluationContext.class), Mockito.any()); + } + + @Test + public void testChangeLevel() { + LoggingHandler loggingHandler = new LoggingHandler("INFO"); + DirectFieldAccessor accessor = new DirectFieldAccessor(loggingHandler); + Log log = (Log) accessor.getPropertyValue("messageLogger"); + log = spy(log); + accessor.setPropertyValue("messageLogger", log); + when(log.isInfoEnabled()).thenReturn(true); + loggingHandler.handleMessage(new GenericMessage("foo")); + verify(log, times(1)).info(Mockito.anyString()); + verify(log, never()).warn(Mockito.anyString()); + + loggingHandler.setLevel(Level.WARN); + loggingHandler.handleMessage(new GenericMessage("foo")); + verify(log, times(1)).info(Mockito.anyString()); + verify(log, times(1)).warn(Mockito.anyString()); + } + public static class TestBean { private final String name; - private int age; + private final int age; public TestBean(String name, int age) { this.name = name; diff --git a/src/reference/docbook/logging-adapter.xml b/src/reference/docbook/logging-adapter.xml new file mode 100644 index 0000000000..eca5b5f44d --- /dev/null +++ b/src/reference/docbook/logging-adapter.xml @@ -0,0 +1,70 @@ + +
+ Logging Channel Adapter + + + The <logging-channel-adapter/> is often used in conjunction with a + Wire Tap, as discussed in + . However, it can also be used as the ultimate consumer + of any flow. For example, consider a flow that ends with a <service-activator/> + that returns a result, but you wish to discard that result. To do that, you could send the + result to NullChannel. Alternatively, you can route it to an INFO + level <logging-channel-adapter/>; that way, you can see + the discarded message when logging at INFO level, but not see it when logging at, + say, WARN level. With a NullChannel, you would only see the + discarded message when logging at DEBUG level. + + + +]]> + + + + The channel connecting the logging adapter to an upstream component. + + + + + The logging level at which messages sent to this adapter will be logged. + Default: INFO. + + + + + A SpEL expression representing exactly what part(s) of the message will be + logged. Default: payload - just the payload will be logged. + This attribute cannot be specified if log-full-message is + specified. + + + + + When true, the entire message will be logged (including headers). + Default: false - just the payload will be logged. + This attribute cannot be specified if expression is + specified. + + + + + Specifies the name of the logger (known as + category in log4j) + used for log messages created by this adapter. This enables setting the + log name (in the logging subsystem) for individual adapters. By default, all + adapters will log under the name + org.springframework.integration.handler.LoggingHandler. + + + +
diff --git a/src/reference/docbook/messaging-endpoints.xml b/src/reference/docbook/messaging-endpoints.xml index 77c1bd7065..d1aa0f89de 100644 --- a/src/reference/docbook/messaging-endpoints.xml +++ b/src/reference/docbook/messaging-endpoints.xml @@ -11,5 +11,6 @@ + From 4818d9a722e5bca9d94b52f647b0a28b0fb682c4 Mon Sep 17 00:00:00 2001 From: Gary Russell Date: Mon, 1 Apr 2013 16:45:42 -0400 Subject: [PATCH 2/2] INT-2939 Polishing Add assert for non-null Level. --- .../org/springframework/integration/handler/LoggingHandler.java | 1 + 1 file changed, 1 insertion(+) diff --git a/spring-integration-core/src/main/java/org/springframework/integration/handler/LoggingHandler.java b/spring-integration-core/src/main/java/org/springframework/integration/handler/LoggingHandler.java index 2af36212c8..ac44578072 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/handler/LoggingHandler.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/handler/LoggingHandler.java @@ -98,6 +98,7 @@ public class LoggingHandler extends AbstractMessageHandler { * @param level the level. */ public void setLevel(Level level) { + Assert.notNull(level, "'level' cannot be null"); this.level = level; }