[logback-dev] svn commit: r1778 - in logback/trunk: logback-access/src/main/java/ch/qos/logback/access logback-classic logback-classic/src/main/java/ch/qos/logback/classic logback-classic/src/test/java/ch/qos/logback/classic/net logback-core/src/main/java/ch/qos/logback/core/net logback-core/src/main/java/ch/qos/logback/core/pattern

noreply.ceki at qos.ch noreply.ceki at qos.ch
Tue Aug 26 23:14:37 CEST 2008


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 at host.dom");
+
+    smtpAppender.setLayout(buildLayout(lc));
+    smtpAppender.setSMTPHost("localhost");
+    smtpAppender.setSMTPPort(diff);
+    smtpAppender.setSubject(TEST_SUBJECT);
+    smtpAppender.addTo("nospam at 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;
   }


More information about the logback-dev mailing list