Author: ceki Date: Tue Aug 26 23:14:37 2008 New Revision: 1778 Added: logback/trunk/logback-classic/src/test/java/ch/qos/logback/classic/net/DilutedSMTPAppenderTest.java - copied, changed from r1772, /logback/trunk/logback-classic/src/test/java/ch/qos/logback/classic/net/SMTPAppenderTest.java logback/trunk/logback-classic/src/test/java/ch/qos/logback/classic/net/SMTPAppender_GreenTest.java Removed: logback/trunk/logback-classic/src/test/java/ch/qos/logback/classic/net/SMTPAppenderTest.java Modified: logback/trunk/logback-access/src/main/java/ch/qos/logback/access/PatternLayout.java logback/trunk/logback-classic/pom.xml logback/trunk/logback-classic/src/main/java/ch/qos/logback/classic/PatternLayout.java logback/trunk/logback-classic/src/test/java/ch/qos/logback/classic/net/PackageTest.java logback/trunk/logback-core/src/main/java/ch/qos/logback/core/net/SMTPAppenderBase.java logback/trunk/logback-core/src/main/java/ch/qos/logback/core/pattern/ConverterUtil.java logback/trunk/logback-core/src/main/java/ch/qos/logback/core/pattern/PatternLayoutBase.java Log: LBCLASSIC-67 Not not all PatternLayout instances need to have a ThrowableInformationConverter added at the end of the converter chain by the postCompileProcessing() method. With this commit, post compile processing code has been moved to an interface. By default, PatternLayouts that ship with logback have a default post compile processor. However, it is now very easy to set processor to null. We now test SMTPAppender using GreenMail. Problem described in LBCLASSIC-67 is reproduced in tests but not yet fixed. Modified: logback/trunk/logback-access/src/main/java/ch/qos/logback/access/PatternLayout.java ============================================================================== --- logback/trunk/logback-access/src/main/java/ch/qos/logback/access/PatternLayout.java (original) +++ logback/trunk/logback-access/src/main/java/ch/qos/logback/access/PatternLayout.java Tue Aug 26 23:14:37 2008 @@ -14,6 +14,7 @@ import ch.qos.logback.access.pattern.ContentLengthConverter; import ch.qos.logback.access.pattern.DateConverter; +import ch.qos.logback.access.pattern.EnsureLineSeparation; import ch.qos.logback.access.pattern.FullRequestConverter; import ch.qos.logback.access.pattern.FullResponseConverter; import ch.qos.logback.access.pattern.LineSeparatorConverter; @@ -37,7 +38,6 @@ import ch.qos.logback.access.pattern.ServerNameConverter; import ch.qos.logback.access.pattern.StatusCodeConverter; import ch.qos.logback.access.spi.AccessEvent; -import ch.qos.logback.core.pattern.Converter; import ch.qos.logback.core.pattern.PatternLayoutBase; /** @@ -146,9 +146,12 @@ defaultConverterMap.put("n", LineSeparatorConverter.class.getName()); } + public PatternLayout() { // set a default value for pattern setPattern(CLF_PATTERN); + // by default postCompileProcessor the is an EnsureLineSeparation instance + this.postCompileProcessor = new EnsureLineSeparation(); } /** @@ -158,24 +161,6 @@ return defaultConverterMap; } - /** - * Add a line separator so that each line is on a separate line. - */ - @SuppressWarnings("unchecked") - @Override - protected void postCompileProcessing(Converter<AccessEvent> head) { - Converter<AccessEvent> tail = findTail(head); - Converter<AccessEvent> newLineConverter = new LineSeparatorConverter(); - if (tail == null) { - head = newLineConverter; - } else { - if (!(tail instanceof LineSeparatorConverter)) { - tail.setNext(newLineConverter); - } - } - setContextForConverters(head); - } - public String doLayout(AccessEvent event) { if (!isStarted()) { return null; Modified: logback/trunk/logback-classic/pom.xml ============================================================================== --- logback/trunk/logback-classic/pom.xml (original) +++ logback/trunk/logback-classic/pom.xml Tue Aug 26 23:14:37 2008 @@ -107,6 +107,14 @@ <optional>true</optional> </dependency> + <dependency> + <groupId>com.icegreen</groupId> + <artifactId>greenmail</artifactId> + <version>1.3</version> + <scope>test</scope> + </dependency> + + </dependencies> <build> Modified: logback/trunk/logback-classic/src/main/java/ch/qos/logback/classic/PatternLayout.java ============================================================================== --- logback/trunk/logback-classic/src/main/java/ch/qos/logback/classic/PatternLayout.java (original) +++ logback/trunk/logback-classic/src/main/java/ch/qos/logback/classic/PatternLayout.java Tue Aug 26 23:14:37 2008 @@ -13,6 +13,7 @@ import java.util.Map; import ch.qos.logback.classic.pattern.CallerDataConverter; +import ch.qos.logback.classic.pattern.EnsureExceptionHandling; import ch.qos.logback.classic.pattern.ClassOfCallerConverter; import ch.qos.logback.classic.pattern.DateConverter; import ch.qos.logback.classic.pattern.FileOfCallerConverter; @@ -27,11 +28,9 @@ import ch.qos.logback.classic.pattern.NopThrowableInformationConverter; import ch.qos.logback.classic.pattern.RelativeTimeConverter; import ch.qos.logback.classic.pattern.ThreadConverter; -import ch.qos.logback.classic.pattern.ThrowableHandlingConverter; import ch.qos.logback.classic.pattern.ThrowableInformationConverter; import ch.qos.logback.classic.spi.LoggingEvent; import ch.qos.logback.core.CoreGlobal; -import ch.qos.logback.core.pattern.Converter; import ch.qos.logback.core.pattern.PatternLayoutBase; /** @@ -109,57 +108,14 @@ defaultConverterMap.put("n", LineSeparatorConverter.class.getName()); } - /** - * This implementation checks if any of the converters in the chain handles - * exceptions. If not, then this method adds a ThrowableInformationConverter - * instance to the end of the chain. - * <p> - * This allows appenders using this layout to output exception information - * event if the user forgets to add %ex to the pattern. Note that the - * appenders defined in the Core package are not aware of exceptions nor - * LoggingEvents. - * <p> - * If for some reason the user wishes to NOT print exceptions, then she can - * add %nopex to the pattern. - * - * - */ - protected void postCompileProcessing(Converter<LoggingEvent> head) { - if (!chainHandlesThrowable(head)) { - Converter<LoggingEvent> tail = findTail(head); - Converter<LoggingEvent> exConverter = new ThrowableInformationConverter(); - if (tail == null) { - head = exConverter; - } else { - tail.setNext(exConverter); - } - } - setContextForConverters(head); + public PatternLayout() { + this.postCompileProcessor = new EnsureExceptionHandling(); } - + public Map<String, String> getDefaultConverterMap() { return defaultConverterMap; } - - /** - * This method computes whether a chain of converters handles exceptions or - * not. - * - * @param head - * The first element of the chain - * @return true if can handle throwables contained in logging events - */ - public static boolean chainHandlesThrowable(Converter head) { - Converter c = head; - while (c != null) { - if (c instanceof ThrowableHandlingConverter) { - return true; - } - c = c.getNext(); - } - return false; - } - + public String doLayout(LoggingEvent event) { if (!isStarted()) { return CoreGlobal.EMPTY_STRING; Copied: logback/trunk/logback-classic/src/test/java/ch/qos/logback/classic/net/DilutedSMTPAppenderTest.java (from r1772, /logback/trunk/logback-classic/src/test/java/ch/qos/logback/classic/net/SMTPAppenderTest.java) ============================================================================== --- /logback/trunk/logback-classic/src/test/java/ch/qos/logback/classic/net/SMTPAppenderTest.java (original) +++ logback/trunk/logback-classic/src/test/java/ch/qos/logback/classic/net/DilutedSMTPAppenderTest.java Tue Aug 26 23:14:37 2008 @@ -1,21 +1,28 @@ package ch.qos.logback.classic.net; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + import javax.mail.Address; import javax.mail.MessagingException; -import junit.framework.TestCase; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + import ch.qos.logback.classic.Level; import ch.qos.logback.classic.LoggerContext; import ch.qos.logback.classic.PatternLayout; import ch.qos.logback.classic.spi.LoggingEvent; import ch.qos.logback.core.Layout; -public class SMTPAppenderTest extends TestCase { +public class DilutedSMTPAppenderTest { SMTPAppender appender; + @Before public void setUp() throws Exception { - super.setUp(); LoggerContext lc = new LoggerContext(); appender = new SMTPAppender(); appender.setContext(lc); @@ -28,11 +35,22 @@ appender.start(); } + private static Layout<LoggingEvent> buildLayout(LoggerContext lc) { + PatternLayout layout = new PatternLayout(); + layout.setContext(lc); + layout.setFileHeader("Some header\n"); + layout.setPattern("%-4relative [%thread] %-5level %class - %msg%n"); + layout.setFileFooter("Some footer"); + layout.start(); + return layout; + } + + @After public void tearDown() throws Exception { - super.tearDown(); appender = null; } + @Test public void testStart() { try { Address[] addressArray = appender.getMessage().getFrom(); @@ -55,6 +73,7 @@ } } + @Test public void testAppendNonTriggeringEvent() { LoggingEvent event = new LoggingEvent(); event.setThreadName("thead name"); @@ -63,36 +82,34 @@ assertEquals(1, appender.cb.length()); } + @Test public void testEntryConditionsCheck() { appender.checkEntryConditions(); assertEquals(0, appender.getContext().getStatusManager().getCount()); } + @Test public void testEntryConditionsCheckNoMessage() { appender.setMessage(null); appender.checkEntryConditions(); assertEquals(1, appender.getContext().getStatusManager().getCount()); } - public void setTriggeringPolicy() { + @Test + public void testTriggeringPolicy() { appender.setEvaluator(null); appender.checkEntryConditions(); assertEquals(1, appender.getContext().getStatusManager().getCount()); } - + + @Test public void testEntryConditionsCheckNoLayout() { appender.setLayout(null); appender.checkEntryConditions(); assertEquals(1, appender.getContext().getStatusManager().getCount()); } + + - private static Layout<LoggingEvent> buildLayout(LoggerContext lc) { - PatternLayout layout = new PatternLayout(); - layout.setContext(lc); - layout.setFileHeader("Some header\n"); - layout.setPattern("%-4relative [%thread] %-5level %class - %msg%n"); - layout.setFileFooter("Some footer"); - layout.start(); - return layout; - } + } Modified: logback/trunk/logback-classic/src/test/java/ch/qos/logback/classic/net/PackageTest.java ============================================================================== --- logback/trunk/logback-classic/src/test/java/ch/qos/logback/classic/net/PackageTest.java (original) +++ logback/trunk/logback-classic/src/test/java/ch/qos/logback/classic/net/PackageTest.java Tue Aug 26 23:14:37 2008 @@ -20,7 +20,7 @@ public static Test suite() { TestSuite suite = new TestSuite(); suite.addTest(new JUnit4TestAdapter(SyslogAppenderTest.class)); - suite.addTestSuite(SMTPAppenderTest.class); + suite.addTest(new JUnit4TestAdapter(DilutedSMTPAppenderTest.class)); suite.addTest(new JUnit4TestAdapter(SocketAppenderTest.class)); suite.addTestSuite(JMSTopicAppenderTest.class); return suite; Added: logback/trunk/logback-classic/src/test/java/ch/qos/logback/classic/net/SMTPAppender_GreenTest.java ============================================================================== --- (empty file) +++ logback/trunk/logback-classic/src/test/java/ch/qos/logback/classic/net/SMTPAppender_GreenTest.java Tue Aug 26 23:14:37 2008 @@ -0,0 +1,86 @@ +package ch.qos.logback.classic.net; + +import static org.junit.Assert.*; + +import java.util.Random; + +import javax.mail.internet.MimeMessage; + +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +import ch.qos.logback.classic.Logger; +import ch.qos.logback.classic.LoggerContext; +import ch.qos.logback.classic.PatternLayout; +import ch.qos.logback.classic.spi.LoggingEvent; +import ch.qos.logback.core.Layout; + +import com.icegreen.greenmail.util.GreenMail; +import com.icegreen.greenmail.util.ServerSetup; + +public class SMTPAppender_GreenTest { + + int diff = 1024 + new Random().nextInt(10000); + GreenMail greenMail; + SMTPAppender smtpAppender; + LoggerContext lc = new LoggerContext(); + + static final String TEST_SUBJECT = "test subject"; + + + @Before + public void setUp() throws Exception { + ServerSetup serverSetup = new ServerSetup(diff, "localhost", + ServerSetup.PROTOCOL_SMTP); + greenMail = new GreenMail(serverSetup); + greenMail.start(); + buildSMTPAppender(); + } + + void buildSMTPAppender() throws Exception { + smtpAppender = new SMTPAppender(); + smtpAppender.setContext(lc); + smtpAppender.setName("smtp"); + smtpAppender.setFrom("user@host.dom"); + + smtpAppender.setLayout(buildLayout(lc)); + smtpAppender.setSMTPHost("localhost"); + smtpAppender.setSMTPPort(diff); + smtpAppender.setSubject(TEST_SUBJECT); + smtpAppender.addTo("nospam@qos.ch"); +// smtpAppender.start(); + } + + private Layout<LoggingEvent> buildLayout(LoggerContext lc) { + PatternLayout layout = new PatternLayout(); + layout.setContext(lc); + layout.setFileHeader("Some header\n"); + layout.setPattern("%-4relative [%thread] %-5level %class - %msg%n"); + layout.setFileFooter("Some footer"); + layout.start(); + return layout; + } + + @After + public void tearDown() throws Exception { + } + + @Test + public void smoke() throws Exception { + smtpAppender.start(); + Logger logger = lc.getLogger("test"); + logger.addAppender(smtpAppender); + logger.debug("hello"); + logger.error("en error", new Exception("an exception")); + MimeMessage[] mma = greenMail.getReceivedMessages(); + assertNotNull(mma); + assertEquals(1, mma.length); + MimeMessage mm = mma[0]; + + assertEquals(TEST_SUBJECT, mm.getSubject()); + //System.out.println(mm.getContent().toString()); + + } + +} Modified: logback/trunk/logback-core/src/main/java/ch/qos/logback/core/net/SMTPAppenderBase.java ============================================================================== --- logback/trunk/logback-core/src/main/java/ch/qos/logback/core/net/SMTPAppenderBase.java (original) +++ logback/trunk/logback-core/src/main/java/ch/qos/logback/core/net/SMTPAppenderBase.java Tue Aug 26 23:14:37 2008 @@ -48,7 +48,8 @@ private String from; private String subjectStr = null; private String smtpHost; - + private int smtpPort = 25; + protected Message msg; protected EventEvaluator eventEvaluator; @@ -72,7 +73,9 @@ if (smtpHost != null) { props.put("mail.smtp.host", smtpHost); } - + props.put("mail.smtp.port", Integer.toString(smtpPort)); + + Session session = Session.getInstance(props, null); // session.setDebug(true); msg = new MimeMessage(session); @@ -284,6 +287,23 @@ } /** + * The port where the SMTP server is running. Default value is 25. + * + * @param port + */ + public void setSMTPPort(int port) { + this.smtpPort = port; + } + + /** + * @see #setSMTPPort(int) + * @return + */ + public int getSMTPPort() { + return smtpPort; + } + + /** * The <b>To</b> option takes a string value which should be * an e-mail address of one of the recipients. */ Modified: logback/trunk/logback-core/src/main/java/ch/qos/logback/core/pattern/ConverterUtil.java ============================================================================== --- logback/trunk/logback-core/src/main/java/ch/qos/logback/core/pattern/ConverterUtil.java (original) +++ logback/trunk/logback-core/src/main/java/ch/qos/logback/core/pattern/ConverterUtil.java Tue Aug 26 23:14:37 2008 @@ -31,4 +31,17 @@ } } + + public static<E> Converter<E> findTail(Converter<E> head) { + Converter<E> c = head; + while (c != null) { + Converter<E> next = c.getNext(); + if (next == null) { + break; + } else { + c = next; + } + } + return c; + } } Modified: logback/trunk/logback-core/src/main/java/ch/qos/logback/core/pattern/PatternLayoutBase.java ============================================================================== --- logback/trunk/logback-core/src/main/java/ch/qos/logback/core/pattern/PatternLayoutBase.java (original) +++ logback/trunk/logback-core/src/main/java/ch/qos/logback/core/pattern/PatternLayoutBase.java Tue Aug 26 23:14:37 2008 @@ -27,7 +27,8 @@ Converter<E> head; String pattern; - + protected PostCompileProcessor<E> postCompileProcessor; + Map<String, String> instanceConverterMap = new HashMap<String, String>(); /** @@ -77,7 +78,10 @@ } Node t = p.parse(); this.head = p.compile(t, getEffectiveConverterMap()); - postCompileProcessing(head); + if(postCompileProcessor != null) { + postCompileProcessor.process(head); + } + setContextForConverters(head); ConverterUtil.startConverters(this.head); super.start(); } catch (ScanException sce) { @@ -87,18 +91,12 @@ } } - /** - * Let derived classes perform postCompile processing. However, PatternLayout - * found in the classic module needs to add a converter for exception handling - * if there isn't one already. - * - * @param head - */ - protected void postCompileProcessing(Converter<E> head) { + public void setPostCompileProcessor( + PostCompileProcessor<E> postCompileProcessor) { + this.postCompileProcessor = postCompileProcessor; } protected void setContextForConverters(Converter<E> head) { - Context context = getContext(); Converter c = head; while (c != null) { @@ -131,19 +129,6 @@ return this.getClass().getName() + "(" + getPattern() + ")"; } - protected Converter<E> findTail(Converter<E> head) { - Converter<E> c = head; - while (c != null) { - Converter<E> next = c.getNext(); - if (next == null) { - break; - } else { - c = next; - } - } - return c; - } - public Map<String, String> getInstanceConverterMap() { return instanceConverterMap; }