Mon Jul 14 21:32:00 CEST 2008

Author: ceki
Date: Mon Jul 14 21:31:59 2008
New Revision: 1706

Added:
   logback/trunk/logback-core/src/main/java/ch/qos/logback/core/pattern/util/AlmostAsIsEscapeUtil.java
   logback/trunk/logback-core/src/main/java/ch/qos/logback/core/pattern/util/IEscapeUtil.java
   logback/trunk/logback-core/src/main/java/ch/qos/logback/core/pattern/util/RegularEscapeUtil.java
      - copied, changed from r1704, /logback/trunk/logback-core/src/main/java/ch/qos/logback/core/pattern/util/EscapeUtil.java
Removed:
   logback/trunk/logback-core/src/main/java/ch/qos/logback/core/pattern/util/EscapeUtil.java
Modified:
   logback/trunk/logback-classic/src/main/java/org/slf4j/impl/StaticLoggerBinder.java
   logback/trunk/logback-core/src/main/java/ch/qos/logback/core/CoreGlobal.java
   logback/trunk/logback-core/src/main/java/ch/qos/logback/core/joran/action/PropertyAction.java
   logback/trunk/logback-core/src/main/java/ch/qos/logback/core/pattern/parser/OptionTokenizer.java
   logback/trunk/logback-core/src/main/java/ch/qos/logback/core/pattern/parser/Parser.java
   logback/trunk/logback-core/src/main/java/ch/qos/logback/core/pattern/parser/TokenStream.java
   logback/trunk/logback-core/src/main/java/ch/qos/logback/core/rolling/helper/FileNamePattern.java
   logback/trunk/logback-core/src/main/java/ch/qos/logback/core/util/PropertySetter.java
   logback/trunk/logback-core/src/test/java/ch/qos/logback/core/pattern/parser/TokenStreamTest.java
   logback/trunk/logback-core/src/test/java/ch/qos/logback/core/rolling/helper/FileNamePatternTest.java

Log:
- fixes LBCLASSIC-56

Escaping is implemented selectively. Cerated a new interface called EscapeUtil.
In particular, FileNamePattern now uses AlmostAsIsEscapeUtil.

See also http://jira.qos.ch/browse/LBCLASSIC-56

Modified: logback/trunk/logback-classic/src/main/java/org/slf4j/impl/StaticLoggerBinder.java
==============================================================================

--- logback/trunk/logback-classic/src/main/java/org/slf4j/impl/StaticLoggerBinder.java	(original)
+++ logback/trunk/logback-classic/src/main/java/org/slf4j/impl/StaticLoggerBinder.java	Mon Jul 14 21:31:59 2008
@@ -86,7 +86,10 @@
   }
 
   static ContextSelector dynamicalContextSelector(
-      LoggerContext defaultLoggerContext, String contextSelectorStr) throws ClassNotFoundException, SecurityException, NoSuchMethodException, IllegalArgumentException, InstantiationException, IllegalAccessException, InvocationTargetException {
+      LoggerContext defaultLoggerContext, String contextSelectorStr)
+      throws ClassNotFoundException, SecurityException, NoSuchMethodException,
+      IllegalArgumentException, InstantiationException, IllegalAccessException,
+      InvocationTargetException {
     Class contextSelectorClass = Loader.loadClass(contextSelectorStr);
     Constructor cons = contextSelectorClass
         .getConstructor(new Class[] { LoggerContext.class });

Modified: logback/trunk/logback-core/src/main/java/ch/qos/logback/core/CoreGlobal.java
==============================================================================
--- logback/trunk/logback-core/src/main/java/ch/qos/logback/core/CoreGlobal.java	(original)
+++ logback/trunk/logback-core/src/main/java/ch/qos/logback/core/CoreGlobal.java	Mon Jul 14 21:31:59 2008
@@ -44,5 +44,8 @@
    * An empty Class array.
    */
   public final static Class[] EMPTY_CLASS_ARRAY = new Class[] {};
-  static public final String CAUSED_BY = "Caused by: ";
+  public final static String CAUSED_BY = "Caused by: ";
+  
+  
+  public final static char PERCENT_CHAR = '%';
 }

Modified: logback/trunk/logback-core/src/main/java/ch/qos/logback/core/joran/action/PropertyAction.java
==============================================================================
--- logback/trunk/logback-core/src/main/java/ch/qos/logback/core/joran/action/PropertyAction.java	(original)
+++ logback/trunk/logback-core/src/main/java/ch/qos/logback/core/joran/action/PropertyAction.java	Mon Jul 14 21:31:59 2008
@@ -12,7 +12,7 @@
 import org.xml.sax.Attributes;
 
 import ch.qos.logback.core.joran.spi.InterpretationContext;
-import ch.qos.logback.core.pattern.util.EscapeUtil;
+import ch.qos.logback.core.pattern.util.RegularEscapeUtil;
 import ch.qos.logback.core.util.OptionHelper;
 
 
@@ -65,7 +65,7 @@
     } else if (
       !(OptionHelper.isEmpty(name) || OptionHelper.isEmpty(value))
         && OptionHelper.isEmpty(fileName)) {
-      value = EscapeUtil.basicEscape(value);
+      value = RegularEscapeUtil.basicEscape(value);
       // now remove both leading and trailing spaces
       value = value.trim();
       setProperty(ec, name, value);

Modified: logback/trunk/logback-core/src/main/java/ch/qos/logback/core/pattern/parser/OptionTokenizer.java
==============================================================================
--- logback/trunk/logback-core/src/main/java/ch/qos/logback/core/pattern/parser/OptionTokenizer.java	(original)
+++ logback/trunk/logback-core/src/main/java/ch/qos/logback/core/pattern/parser/OptionTokenizer.java	Mon Jul 14 21:31:59 2008
@@ -3,7 +3,9 @@
 import java.util.ArrayList;
 import java.util.List;
 
-import ch.qos.logback.core.pattern.util.EscapeUtil;
+import ch.qos.logback.core.CoreGlobal;
+import ch.qos.logback.core.pattern.util.IEscapeUtil;
+import ch.qos.logback.core.pattern.util.RegularEscapeUtil;
 
 public class OptionTokenizer {
 
@@ -16,15 +18,26 @@
   private static final char DOUBLE_QUOTE_CHAR = '"';
   private static final char SINGLE_QUOTE_CHAR = '\'';
 
-  String pattern;
-  int patternLength;
+  final String pattern;
+  final int patternLength;
+  final IEscapeUtil escapeUtil;
+  
   char quoteChar;
   int pointer = 0;
   int state = EXPECTING_STATE;
 
+  /**
+   * This variant is used in tests
+   * @param pattern
+   */
   OptionTokenizer(String pattern) {
+    this(pattern, new RegularEscapeUtil());
+  }
+  
+  OptionTokenizer(String pattern, IEscapeUtil escapeUtil) {
     this.pattern = pattern;
     patternLength = pattern.length();
+    this.escapeUtil = escapeUtil;
   }
 
   List tokenize() throws ScanException {
@@ -96,7 +109,7 @@
   void escape(String escapeChars, StringBuffer buf) {
     if ((pointer < patternLength)) {
       char next = pattern.charAt(pointer++);
-      EscapeUtil.escape(escapeChars, buf, next, pointer);
+      escapeUtil.escape(escapeChars, buf, next, pointer);
     }
   }
 }
\ No newline at end of file

Modified: logback/trunk/logback-core/src/main/java/ch/qos/logback/core/pattern/parser/Parser.java
==============================================================================
--- logback/trunk/logback-core/src/main/java/ch/qos/logback/core/pattern/parser/Parser.java	(original)
+++ logback/trunk/logback-core/src/main/java/ch/qos/logback/core/pattern/parser/Parser.java	Mon Jul 14 21:31:59 2008
@@ -5,6 +5,8 @@
 
 import ch.qos.logback.core.pattern.Converter;
 import ch.qos.logback.core.pattern.FormatInfo;
+import ch.qos.logback.core.pattern.util.IEscapeUtil;
+import ch.qos.logback.core.pattern.util.RegularEscapeUtil;
 import ch.qos.logback.core.spi.ContextAwareBase;
 
 
@@ -12,14 +14,21 @@
 
   final List tokenList;
   int pointer = 0;
+  IEscapeUtil escapeUtil; 
   
   Parser(TokenStream ts) throws ScanException {
     this.tokenList = ts.tokenize();
   }
 
+  // this variant should be used for testing purposes only
   public Parser(String pattern) throws ScanException {
+    this(pattern, new RegularEscapeUtil());
+  }
+  
+  public Parser(String pattern, IEscapeUtil escapeUtil) throws ScanException {
+    this.escapeUtil = escapeUtil;
     try {
-      TokenStream ts = new TokenStream(pattern);
+      TokenStream ts = new TokenStream(pattern, escapeUtil);
       this.tokenList = ts.tokenize();
     } catch (NullPointerException npe) {
       throw new ScanException("Failed to initialize Parser", npe);

Modified: logback/trunk/logback-core/src/main/java/ch/qos/logback/core/pattern/parser/TokenStream.java
==============================================================================
--- logback/trunk/logback-core/src/main/java/ch/qos/logback/core/pattern/parser/TokenStream.java	(original)
+++ logback/trunk/logback-core/src/main/java/ch/qos/logback/core/pattern/parser/TokenStream.java	Mon Jul 14 21:31:59 2008
@@ -12,7 +12,11 @@
 import java.util.List;
 import java.util.ArrayList;
 
-import ch.qos.logback.core.pattern.util.EscapeUtil;
+import ch.qos.logback.core.CoreGlobal;
+import ch.qos.logback.core.pattern.util.IEscapeUtil;
+import ch.qos.logback.core.pattern.util.RegularEscapeUtil;
+
+
 
 /**
  * <p>Return a steady stream of tokens. <p/>
@@ -29,7 +33,7 @@
 class TokenStream {
 
   private static final char ESCAPE_CHAR = '\\';
-  private static final char PERCENT_CHAR = '%';
+  private static final char PERCENT_CHAR = CoreGlobal.PERCENT_CHAR; 
   private static final char LEFT_PARENTHESIS = '(';
   private static final char RIGHT_PARENTHESIS = ')';
   private static final char CURLY_LEFT = '{';
@@ -42,15 +46,23 @@
 
   final String pattern;
   final int patternLength;
+  final IEscapeUtil escapeUtil;
+  
   int state = LITERAL_STATE;
   int pointer = 0;
 
+  // this variant should be used for testing purposes only
   TokenStream(String pattern) {
+    this(pattern, new RegularEscapeUtil());
+  }
+  
+  TokenStream(String pattern, IEscapeUtil escapeUtil) {
     if(pattern == null) {
       throw new NullPointerException("null pattern string not allowed");
     }
     this.pattern = pattern;
     patternLength = pattern.length();
+    this.escapeUtil = escapeUtil;
   }
 
   List tokenize() throws ScanException {
@@ -133,7 +145,7 @@
           } else if (c == ESCAPE_CHAR) {
             if ((pointer < patternLength)) {
               char next = pattern.charAt(pointer++);
-              EscapeUtil.escape("%()", buf, next, pointer);
+              escapeUtil.escape("%()", buf, next, pointer);
             }
           } else {
             buf.append(c);
@@ -167,7 +179,7 @@
   void escape(String escapeChars, StringBuffer buf) {
     if ((pointer < patternLength)) {
       char next = pattern.charAt(pointer++);
-      EscapeUtil.escape(escapeChars, buf, next, pointer);
+      escapeUtil.escape(escapeChars, buf, next, pointer);
     }
   }
 

Added: logback/trunk/logback-core/src/main/java/ch/qos/logback/core/pattern/util/AlmostAsIsEscapeUtil.java
==============================================================================
--- (empty file)
+++ logback/trunk/logback-core/src/main/java/ch/qos/logback/core/pattern/util/AlmostAsIsEscapeUtil.java	Mon Jul 14 21:31:59 2008
@@ -0,0 +1,40 @@
+package ch.qos.logback.core.pattern.util;
+
+import ch.qos.logback.core.CoreGlobal;
+import ch.qos.logback.core.rolling.helper.FileNamePattern;
+
+/**
+ * This implementation is intended for use in {@link FileNamePattern}.
+ * 
+ * @author Ceki G&uuml;lc&uuml;
+ */
+public class AlmostAsIsEscapeUtil implements IEscapeUtil {
+
+  /**
+   * Do not perform any character escaping, except for '%'.
+   * 
+   * <p>
+   * Here is the rationale. First, filename patterns do not include escape
+   * combinations such as \r or \n. Moreover, characters which have special
+   * meaning in logback parsers, such as '(', ')', '{', or '}' cannot be part of
+   * file names (so me thinks). Thus, the only character that needs escaping is
+   * '%'.
+   * 
+   * <p>
+   * Note that this method assumes that it is called after the escape character
+   * has been consumed.
+   */
+  public void escape(String escapeChars, StringBuffer buf, char next,
+      int pointer) {
+
+    if (next == CoreGlobal.PERCENT_CHAR) {
+      buf.append(CoreGlobal.PERCENT_CHAR);
+    } else {
+      // restitute the escape char (because it was consumed 
+      // before this method was called).
+      buf.append("\\");
+      // restitute the next character
+      buf.append(next);
+    }
+  }
+}

Added: logback/trunk/logback-core/src/main/java/ch/qos/logback/core/pattern/util/IEscapeUtil.java
==============================================================================
--- (empty file)
+++ logback/trunk/logback-core/src/main/java/ch/qos/logback/core/pattern/util/IEscapeUtil.java	Mon Jul 14 21:31:59 2008
@@ -0,0 +1,6 @@
+package ch.qos.logback.core.pattern.util;
+
+public interface IEscapeUtil {
+
+  void escape(String additionalEscapeChars, StringBuffer buf, char next, int pointer);
+}
\ No newline at end of file

Copied: logback/trunk/logback-core/src/main/java/ch/qos/logback/core/pattern/util/RegularEscapeUtil.java (from r1704, /logback/trunk/logback-core/src/main/java/ch/qos/logback/core/pattern/util/EscapeUtil.java)
==============================================================================
--- /logback/trunk/logback-core/src/main/java/ch/qos/logback/core/pattern/util/EscapeUtil.java	(original)
+++ logback/trunk/logback-core/src/main/java/ch/qos/logback/core/pattern/util/RegularEscapeUtil.java	Mon Jul 14 21:31:59 2008
@@ -1,8 +1,13 @@
 package ch.qos.logback.core.pattern.util;
 
-public class EscapeUtil {
+/**
+ * This implementation is intended for use in PatternLayout.
+ * 
+ * @author Ceki G&uuml;lc&uuml;
+ */
+public class RegularEscapeUtil implements IEscapeUtil {
 
-  public static void escape(String escapeChars, StringBuffer buf, char next,
+  public void escape(String escapeChars, StringBuffer buf, char next,
       int pointer) {
     if (escapeChars.indexOf(next) >= 0) {
       buf.append(next);
@@ -24,16 +29,21 @@
         buf.append('\n');
         break;
       default:
-        String echars = "";
-        for (int i = 0; i < escapeChars.length(); i++) {
-          echars += ", \\" + escapeChars.charAt(i);
-        }
+        String commaSeperatedEscapeChars = formatEscapeCharsForListing(escapeChars);
         new IllegalArgumentException("Illegal char '" + next + " at column "
-            + pointer + ". Only \\\\, \\_" + echars
+            + pointer + ". Only \\\\, \\_" + commaSeperatedEscapeChars
             + ", \\t, \\n, \\r combinations are allowed as escape characters.");
       }
   }
 
+  String formatEscapeCharsForListing(String escapeChars) {
+    String commaSeperatedEscapeChars = "";
+    for (int i = 0; i < escapeChars.length(); i++) {
+      commaSeperatedEscapeChars += ", \\" + escapeChars.charAt(i);
+    }
+    return commaSeperatedEscapeChars;
+  }
+
   public static String basicEscape(String s) {
     char c;
     int len = s.length();

Modified: logback/trunk/logback-core/src/main/java/ch/qos/logback/core/rolling/helper/FileNamePattern.java
==============================================================================
--- logback/trunk/logback-core/src/main/java/ch/qos/logback/core/rolling/helper/FileNamePattern.java	(original)
+++ logback/trunk/logback-core/src/main/java/ch/qos/logback/core/rolling/helper/FileNamePattern.java	Mon Jul 14 21:31:59 2008
@@ -20,6 +20,7 @@
 import ch.qos.logback.core.pattern.parser.Node;
 import ch.qos.logback.core.pattern.parser.Parser;
 import ch.qos.logback.core.pattern.parser.ScanException;
+import ch.qos.logback.core.pattern.util.AlmostAsIsEscapeUtil;
 import ch.qos.logback.core.spi.ContextAwareBase;
 
 
@@ -52,7 +53,7 @@
 
   void parse() {
     try {
-      Parser<Object> p = new Parser<Object>(pattern);
+      Parser<Object> p = new Parser<Object>(pattern, new AlmostAsIsEscapeUtil());
       p.setContext(context);
       Node t = p.parse();
       this.headTokenConverter = p.compile(t, CONVERTER_MAP);

Modified: logback/trunk/logback-core/src/main/java/ch/qos/logback/core/util/PropertySetter.java
==============================================================================
--- logback/trunk/logback-core/src/main/java/ch/qos/logback/core/util/PropertySetter.java	(original)
+++ logback/trunk/logback-core/src/main/java/ch/qos/logback/core/util/PropertySetter.java	Mon Jul 14 21:31:59 2008
@@ -22,6 +22,7 @@
 import java.beans.Introspector;
 import java.beans.MethodDescriptor;
 import java.beans.PropertyDescriptor;
+import java.lang.reflect.Constructor;
 import java.lang.reflect.Method;
 
 import ch.qos.logback.core.spi.ContextAwareBase;
@@ -247,6 +248,20 @@
       }
     }
   }
+  
+  <T> boolean isUnequivocallyInstantiable(Class<T> clazz) {
+    if(clazz.isInterface()) {
+      return false;
+    }
+    try {
+      Constructor<T> pubConstructor = clazz.getConstructor();
+      return true;
+    } catch (SecurityException e) {
+      return false;
+    } catch (NoSuchMethodException e) {
+      return false;
+    }
+  }
 
   public Class getObjClass() {
     return objClass;

Modified: logback/trunk/logback-core/src/test/java/ch/qos/logback/core/pattern/parser/TokenStreamTest.java
==============================================================================
--- logback/trunk/logback-core/src/test/java/ch/qos/logback/core/pattern/parser/TokenStreamTest.java	(original)
+++ logback/trunk/logback-core/src/test/java/ch/qos/logback/core/pattern/parser/TokenStreamTest.java	Mon Jul 14 21:31:59 2008
@@ -10,14 +10,12 @@
 
 package ch.qos.logback.core.pattern.parser;
 
-import junit.framework.*;
-
-import java.util.List;
 import java.util.ArrayList;
+import java.util.List;
 
-import ch.qos.logback.core.pattern.parser.ScanException;
-import ch.qos.logback.core.pattern.parser.Token;
-import ch.qos.logback.core.pattern.parser.TokenStream;
+import ch.qos.logback.core.pattern.util.AlmostAsIsEscapeUtil;
+
+import junit.framework.TestCase;
 
 public class TokenStreamTest extends TestCase {
 
@@ -317,6 +315,18 @@
       witness.add(new Token(Token.LITERAL, ")"));
       assertEquals(witness, tl);
     }
+  }
 
+  public void testWindowsLikeBackSlashes() throws ScanException {
+    {
+      List tl = new TokenStream("c:\\hello\\world.%i", new AlmostAsIsEscapeUtil())
+          .tokenize();
+
+      List<Token> witness = new ArrayList<Token>();
+      witness.add(new Token(Token.LITERAL, "c:\\hello\\world."));
+      witness.add(Token.PERCENT_TOKEN);
+      witness.add(new Token(Token.KEYWORD, "i"));
+      assertEquals(witness, tl);
+    }
   }
 }
\ No newline at end of file

Modified: logback/trunk/logback-core/src/test/java/ch/qos/logback/core/rolling/helper/FileNamePatternTest.java
==============================================================================
--- logback/trunk/logback-core/src/test/java/ch/qos/logback/core/rolling/helper/FileNamePatternTest.java	(original)
+++ logback/trunk/logback-core/src/test/java/ch/qos/logback/core/rolling/helper/FileNamePatternTest.java	Mon Jul 14 21:31:59 2008
@@ -20,35 +20,33 @@
 import ch.qos.logback.core.rolling.helper.FileNamePattern;
 import ch.qos.logback.core.util.StatusPrinter;
 
-
-
 /**
  * @author Ceki
- *
+ * 
  */
 public class FileNamePatternTest extends TestCase {
-  
+
   Context context = new ContextBase();
-  
+
   public FileNamePatternTest(String arg) {
     super(arg);
   }
-  
-  public void test1() {
-    //System.out.println("Testing [t]");
+
+
+  public void testSmoke() {
+    // System.out.println("Testing [t]");
     FileNamePattern pp = new FileNamePattern("t", context);
     assertEquals("t", pp.convertInt(3));
 
-    //System.out.println("Testing [foo]");
+    // System.out.println("Testing [foo]");
     pp = new FileNamePattern("foo", context);
     assertEquals("foo", pp.convertInt(3));
 
-    //System.out.println("Testing [foo%]");
-    //pp = new FileNamePattern("foo%", context);
-    //StatusPrinter.print(context.getStatusManager());
-    //assertEquals("foo%", pp.convertInt(3));
-  
-    
+    // System.out.println("Testing [foo%]");
+    // pp = new FileNamePattern("foo%", context);
+    // StatusPrinter.print(context.getStatusManager());
+    // assertEquals("foo%", pp.convertInt(3));
+
     pp = new FileNamePattern("%i foo", context);
     StatusPrinter.print(context.getStatusManager());
     assertEquals("3 foo", pp.convertInt(3));
@@ -62,7 +60,6 @@
     pp = new FileNamePattern("foo.%i.log", context);
     assertEquals("foo.3.log", pp.convertInt(3));
 
-
     pp = new FileNamePattern("%i.foo\\%", context);
     assertEquals("3.foo%", pp.convertInt(3));
 
@@ -70,15 +67,21 @@
     assertEquals("%foo", pp.convertInt(3));
   }
 
-  public void test2() {
-    //System.out.println("Testing [foo%ibar%i]");
-
-    FileNamePattern pp = new FileNamePattern("foo%i\\_bar%i", context);
-    assertEquals("foo3bar3", pp.convertInt(3));
-
+  // test ways for dealing with flowing i converter, as in "foo%ix"
+  public void testFlowingI() {
+    // System.out.println("Testing [foo%ibar%i]");
+
+    {
+      FileNamePattern pp = new FileNamePattern("foo%i{}bar%i", context);
+      assertEquals("foo3bar3", pp.convertInt(3));
+    }
+    {
+      FileNamePattern pp = new FileNamePattern("foo%i{}bar%i", context);
+      assertEquals("foo3bar3", pp.convertInt(3));
+    }
   }
 
-  public void test3() {
+  public void testDate() {
     Calendar cal = Calendar.getInstance();
     cal.set(2003, 4, 20, 17, 55);
 
@@ -90,15 +93,20 @@
     assertEquals("foo2003.05.20 17:55", pp.convertDate(cal.getTime()));
 
     pp = new FileNamePattern("%d{yyyy.MM.dd HH:mm} foo", context);
-     assertEquals("2003.05.20 17:55 foo", pp.convertDate(cal.getTime()));
+    assertEquals("2003.05.20 17:55 foo", pp.convertDate(cal.getTime()));
 
   }
+  
+  public void testWithBackslash() {
+    FileNamePattern pp = new FileNamePattern("c:\\foo\\bar.%i", context);
+    assertEquals("c:\\foo\\bar.3", pp.convertInt(3));
+  }
 
   public static Test xsuite() {
     TestSuite suite = new TestSuite();
     suite.addTest(new FileNamePatternTest("test1"));
     suite.addTest(new FileNamePatternTest("test2"));
-    //suite.addTest(new FileNamePatternTestCase("test3"));
+    // suite.addTest(new FileNamePatternTestCase("test3"));
 
     return suite;
   }

