Author: ceki Date: Tue Mar 4 17:38:27 2008 New Revision: 1628 Added: logback/trunk/logback-core/src/test/input/joran/inclusion/multiIncludeByFile.xml logback/trunk/logback-core/src/test/input/joran/inclusion/second.xml logback/trunk/logback-core/src/test/input/joran/inclusion/subByFile.xml logback/trunk/logback-core/src/test/input/joran/inclusion/topByFile.xml - copied, changed from r1622, /logback/trunk/logback-core/src/test/input/joran/inclusion/includeByFile.xml logback/trunk/logback-core/src/test/input/joran/inclusion/topByResource.xml - copied, changed from r1622, /logback/trunk/logback-core/src/test/input/joran/inclusion/includeByResource.xml logback/trunk/logback-core/src/test/input/joran/inclusion/topByUrl.xml - copied, changed from r1622, /logback/trunk/logback-core/src/test/input/joran/inclusion/includeByUrl.xml Removed: logback/trunk/logback-core/src/test/input/joran/inclusion/includeByFile.xml logback/trunk/logback-core/src/test/input/joran/inclusion/includeByResource.xml logback/trunk/logback-core/src/test/input/joran/inclusion/includeByUrl.xml Modified: logback/trunk/logback-core/src/main/java/ch/qos/logback/core/joran/action/IncludeAction.java logback/trunk/logback-core/src/main/java/ch/qos/logback/core/util/OptionHelper.java logback/trunk/logback-core/src/test/java/ch/qos/logback/core/joran/action/IncAction.java logback/trunk/logback-core/src/test/java/ch/qos/logback/core/joran/action/IncludeActionTest.java Log: - fixing bug #129 see also http://bugzilla.qos.ch/show_bug.cgi?id=129\ - refactored the IncludeAction for clarity - IncludeActionTest has been enhanced to check for bug 129 Modified: logback/trunk/logback-core/src/main/java/ch/qos/logback/core/joran/action/IncludeAction.java ============================================================================== --- logback/trunk/logback-core/src/main/java/ch/qos/logback/core/joran/action/IncludeAction.java (original) +++ logback/trunk/logback-core/src/main/java/ch/qos/logback/core/joran/action/IncludeAction.java Tue Mar 4 17:38:27 2008 @@ -14,10 +14,10 @@ import java.io.InputStream; import java.net.MalformedURLException; import java.net.URL; +import java.util.List; import org.xml.sax.Attributes; -import ch.qos.logback.core.joran.action.Action; import ch.qos.logback.core.joran.event.SaxEvent; import ch.qos.logback.core.joran.event.SaxEventRecorder; import ch.qos.logback.core.joran.spi.ActionException; @@ -32,41 +32,27 @@ private static final String FILE_ATTR = "file"; private static final String URL_ATTR = "url"; private static final String RESOURCE_ATTR = "resource"; - private SaxEventRecorder recorder = new SaxEventRecorder();; + + + private String attributeInUse; @Override public void begin(InterpretationContext ec, String name, Attributes attributes) throws ActionException { - String fileAttribute = attributes.getValue(FILE_ATTR); - String urlAttribute = attributes.getValue(URL_ATTR); - String resourceAttribute = attributes.getValue(RESOURCE_ATTR); - String attributeInUse = null; + SaxEventRecorder recorder = new SaxEventRecorder(); + + this.attributeInUse = null; - if(!checkAttributes(fileAttribute, urlAttribute, resourceAttribute)) { + if (!checkAttributes(attributes)) { return; } - InputStream in = null; - - if (!OptionHelper.isEmpty(fileAttribute)) { - attributeInUse = ec.subst(fileAttribute); - in = getInputStreamByFilePath(attributeInUse); - } - - if (!OptionHelper.isEmpty(urlAttribute)) { - attributeInUse = ec.subst(urlAttribute); - in = getInputStreamByUrl(attributeInUse); - } - - if (!OptionHelper.isEmpty(resourceAttribute)) { - attributeInUse = ec.subst(resourceAttribute); - in = getInputStreamByResource(attributeInUse); - } + InputStream in = getInputStream(ec, attributes); try { if (in != null) { - parseAndRecord(in); + parseAndRecord(in, recorder); in.close(); } } catch (JoranException e) { @@ -75,27 +61,17 @@ // called if in.close did not work } - if (recorder.saxEventList.size() == 0) { - return; - } - - // Let's remove the two <included> events before - // adding the events to the player. - SaxEvent first = recorder.saxEventList.get(0); - if (first != null && first.qName.equalsIgnoreCase(INCLUDED_TAG)) { - recorder.saxEventList.remove(0); - } - - SaxEvent last = recorder.saxEventList.get(recorder.saxEventList.size() - 1); - if (last != null && last.qName.equalsIgnoreCase(INCLUDED_TAG)) { - recorder.saxEventList.remove(recorder.saxEventList.size() - 1); - } - + // remove the <included> tag from the beginning and </included> from the end + trimHeadAndTail(recorder); + ec.getJoranInterpreter().addEvents(recorder.saxEventList); } - private boolean checkAttributes(String fileAttribute, - String urlAttribute, String resourceAttribute) { + private boolean checkAttributes(Attributes attributes) { + String fileAttribute = attributes.getValue(FILE_ATTR); + String urlAttribute = attributes.getValue(URL_ATTR); + String resourceAttribute = attributes.getValue(RESOURCE_ATTR); + int count = 0; if (!OptionHelper.isEmpty(fileAttribute)) { @@ -117,7 +93,8 @@ } else if (count == 1) { return true; } - throw new IllegalStateException("Count value ["+count+"] is not expected"); + throw new IllegalStateException("Count value [" + count + + "] is not expected"); } private InputStream getInputStreamByFilePath(String pathToFile) { @@ -163,7 +140,51 @@ return openURL(url); } - private void parseAndRecord(InputStream inputSource) throws JoranException { + InputStream getInputStream(InterpretationContext ec, Attributes attributes) { + String fileAttribute = attributes.getValue(FILE_ATTR); + String urlAttribute = attributes.getValue(URL_ATTR); + String resourceAttribute = attributes.getValue(RESOURCE_ATTR); + + if (!OptionHelper.isEmpty(fileAttribute)) { + attributeInUse = ec.subst(fileAttribute); + return getInputStreamByFilePath(attributeInUse); + } + + if (!OptionHelper.isEmpty(urlAttribute)) { + attributeInUse = ec.subst(urlAttribute); + return getInputStreamByUrl(attributeInUse); + } + + if (!OptionHelper.isEmpty(resourceAttribute)) { + attributeInUse = ec.subst(resourceAttribute); + return getInputStreamByResource(attributeInUse); + } + // given previous checkAttributes() check we cannot reach this line + throw new IllegalStateException("A input stream should have been returned"); + } + + private void trimHeadAndTail(SaxEventRecorder recorder) { + // Let's remove the two <included> events before + // adding the events to the player. + + List<SaxEvent> saxEventList = recorder.saxEventList; + + if (saxEventList.size() == 0) { + return; + } + + SaxEvent first = saxEventList.get(0); + if (first != null && first.qName.equalsIgnoreCase(INCLUDED_TAG)) { + saxEventList.remove(0); + } + + SaxEvent last = saxEventList.get(recorder.saxEventList.size() - 1); + if (last != null && last.qName.equalsIgnoreCase(INCLUDED_TAG)) { + saxEventList.remove(recorder.saxEventList.size() - 1); + } + } + + private void parseAndRecord(InputStream inputSource, SaxEventRecorder recorder) throws JoranException { recorder.setContext(context); recorder.recordEvents(inputSource); } Modified: logback/trunk/logback-core/src/main/java/ch/qos/logback/core/util/OptionHelper.java ============================================================================== --- logback/trunk/logback-core/src/main/java/ch/qos/logback/core/util/OptionHelper.java (original) +++ logback/trunk/logback-core/src/main/java/ch/qos/logback/core/util/OptionHelper.java Tue Mar 4 17:38:27 2008 @@ -90,16 +90,16 @@ * <p> * If no value could be found for the specified key in the context map, then * the system properties are searched, if that fails, then substitution defaults - * to the empty string. + * to appending "_IS_UNDEFINED" to the key name. * * <p> - * For example, if system properties contains no value for the key + * For example, if not the context not the system properties contains no value for the key * "inexistentKey", then the call * * <pre> * String s = OptionConverter.subsVars( * "Value of inexistentKey is [${inexistentKey}]", context);</pre> - * will set <code>s</code> to "Value of inexistentKey is []". + * will set <code>s</code> to "Value of inexistentKey is [inexistentKey_IS_UNDEFINED]". * * <p> * Nevertheless, it is possible to specify a default substitution value using @@ -108,13 +108,14 @@ * <pre> * String s = OptionConverter.subsVars("Value of key is [${key2:-val2}]", context);</pre> * will set <code>s</code> to "Value of key is [val2]" even if the "key2" - * property is unset. + * property is not set. * * <p> * An {@link java.lang.IllegalArgumentException} is thrown if <code>val</code> * contains a start delimeter "${" which is not balanced by a stop delimeter * "}". * </p> + * * @param val * The string on which variable substitution is performed. Added: logback/trunk/logback-core/src/test/input/joran/inclusion/multiIncludeByFile.xml ============================================================================== --- (empty file) +++ logback/trunk/logback-core/src/test/input/joran/inclusion/multiIncludeByFile.xml Tue Mar 4 17:38:27 2008 @@ -0,0 +1,7 @@ +<?xml version="1.0" encoding="UTF-8" ?> +<!DOCTYPE x> + +<x> + <include file="${includeKey}" /> + <include file="${secondFileKey}" /> +</x> Added: logback/trunk/logback-core/src/test/input/joran/inclusion/second.xml ============================================================================== --- (empty file) +++ logback/trunk/logback-core/src/test/input/joran/inclusion/second.xml Tue Mar 4 17:38:27 2008 @@ -0,0 +1,8 @@ +<?xml version="1.0" encoding="UTF-8" ?> +<!DOCTYPE included> + +<included> + + <inc increment="1"/> + +</included> \ No newline at end of file Added: logback/trunk/logback-core/src/test/input/joran/inclusion/subByFile.xml ============================================================================== --- (empty file) +++ logback/trunk/logback-core/src/test/input/joran/inclusion/subByFile.xml Tue Mar 4 17:38:27 2008 @@ -0,0 +1,6 @@ +<?xml version="1.0" encoding="UTF-8" ?> +<!DOCTYPE included> + +<included> + <include file="${includeKey}" /> +</<included> Copied: logback/trunk/logback-core/src/test/input/joran/inclusion/topByFile.xml (from r1622, /logback/trunk/logback-core/src/test/input/joran/inclusion/includeByFile.xml) ============================================================================== --- /logback/trunk/logback-core/src/test/input/joran/inclusion/includeByFile.xml (original) +++ logback/trunk/logback-core/src/test/input/joran/inclusion/topByFile.xml Tue Mar 4 17:38:27 2008 @@ -2,5 +2,5 @@ <!DOCTYPE x> <x> - <include file="${testing}" /> + <include file="${includeKey}" /> </x> Copied: logback/trunk/logback-core/src/test/input/joran/inclusion/topByResource.xml (from r1622, /logback/trunk/logback-core/src/test/input/joran/inclusion/includeByResource.xml) ============================================================================== --- /logback/trunk/logback-core/src/test/input/joran/inclusion/includeByResource.xml (original) +++ logback/trunk/logback-core/src/test/input/joran/inclusion/topByResource.xml Tue Mar 4 17:38:27 2008 @@ -3,6 +3,6 @@ <x> - <include resource="${testing}" /> + <include resource="${includeKey}" /> </x> Copied: logback/trunk/logback-core/src/test/input/joran/inclusion/topByUrl.xml (from r1622, /logback/trunk/logback-core/src/test/input/joran/inclusion/includeByUrl.xml) ============================================================================== --- /logback/trunk/logback-core/src/test/input/joran/inclusion/includeByUrl.xml (original) +++ logback/trunk/logback-core/src/test/input/joran/inclusion/topByUrl.xml Tue Mar 4 17:38:27 2008 @@ -2,5 +2,5 @@ <!DOCTYPE x> <x> - <include url="${testing}" /> + <include url="${includeKey}" /> </x> Modified: logback/trunk/logback-core/src/test/java/ch/qos/logback/core/joran/action/IncAction.java ============================================================================== --- logback/trunk/logback-core/src/test/java/ch/qos/logback/core/joran/action/IncAction.java (original) +++ logback/trunk/logback-core/src/test/java/ch/qos/logback/core/joran/action/IncAction.java Tue Mar 4 17:38:27 2008 @@ -24,6 +24,11 @@ static public int endCount; static public int errorCount; + static public void reset() { + beginCount = 0; + endCount = 0; + errorCount = 0; + } /** * Instantiates an layout of the given class and sets its name. * Modified: logback/trunk/logback-core/src/test/java/ch/qos/logback/core/joran/action/IncludeActionTest.java ============================================================================== --- logback/trunk/logback-core/src/test/java/ch/qos/logback/core/joran/action/IncludeActionTest.java (original) +++ logback/trunk/logback-core/src/test/java/ch/qos/logback/core/joran/action/IncludeActionTest.java Tue Mar 4 17:38:27 2008 @@ -24,20 +24,34 @@ public class IncludeActionTest { - final static String FILE_KEY = "testing"; - + final static String INCLUDE_KEY = "includeKey"; + final static String SUB_FILE_KEY = "subFileKey"; + final static String SECOND_FILE_KEY = "secondFileKey"; + + Context context = new ContextBase(); TrivialConfigurator tc; static final String INCLUSION_DIR_PREFIX = "src/test/input/joran/inclusion/"; - static final String INCLUDE_BY_FILE = INCLUSION_DIR_PREFIX - + "includeByFile.xml"; - static final String INCLUDE_BY_URL = INCLUSION_DIR_PREFIX - + "includeByUrl.xml"; + static final String TOP_BY_FILE = INCLUSION_DIR_PREFIX + + "topByFile.xml"; + + static final String SUB_FILE = INCLUSION_DIR_PREFIX + + "subByFile.xml"; + + static final String MULTI_INCLUDE_BY_FILE = INCLUSION_DIR_PREFIX + + "multiIncludeByFile.xml"; + + static final String SECOND_FILE = INCLUSION_DIR_PREFIX + + "second.xml"; + + + static final String TOP_BY_URL = INCLUSION_DIR_PREFIX + + "topByUrl.xml"; static final String INCLUDE_BY_RESOURCE = INCLUSION_DIR_PREFIX - + "includeByResource.xml"; + + "topByResource.xml"; static final String INCLUDED_FILE = INCLUSION_DIR_PREFIX + "included.xml"; static final String URL_TO_INCLUDE = "file:./" + INCLUDED_FILE; @@ -66,19 +80,20 @@ @After public void tearDown() throws Exception { context = null; - System.clearProperty(FILE_KEY); + System.clearProperty(INCLUDE_KEY); + IncAction.reset(); } @Test public void basicFile() throws JoranException { - System.setProperty(FILE_KEY, INCLUDED_FILE); - tc.doConfigure(INCLUDE_BY_FILE); + System.setProperty(INCLUDE_KEY, INCLUDED_FILE); + tc.doConfigure(TOP_BY_FILE); verifyConfig(2); } @Test public void basicResource() throws JoranException { - System.setProperty(FILE_KEY, INCLUDED_AS_RESOURCE); + System.setProperty(INCLUDE_KEY, INCLUDED_AS_RESOURCE); tc.doConfigure(INCLUDE_BY_RESOURCE); StatusPrinter.print(context); verifyConfig(2); @@ -86,16 +101,16 @@ @Test public void testBasicURL() throws JoranException { - System.setProperty(FILE_KEY, URL_TO_INCLUDE); - tc.doConfigure(INCLUDE_BY_URL); + System.setProperty(INCLUDE_KEY, URL_TO_INCLUDE); + tc.doConfigure(TOP_BY_URL); StatusPrinter.print(context); verifyConfig(2); } @Test public void noFileFound() throws JoranException { - System.setProperty(FILE_KEY, "toto"); - tc.doConfigure(INCLUDE_BY_FILE); + System.setProperty(INCLUDE_KEY, "toto"); + tc.doConfigure(TOP_BY_FILE); assertEquals(Status.ERROR, context.getStatusManager().getLevel()); StatusChecker sc = new StatusChecker(context.getStatusManager()); assertTrue(sc.containsException(FileNotFoundException.class)); @@ -103,8 +118,8 @@ @Test public void withCorruptFile() throws JoranException { - System.setProperty(FILE_KEY, INVALID); - tc.doConfigure(INCLUDE_BY_FILE); + System.setProperty(INCLUDE_KEY, INVALID); + tc.doConfigure(TOP_BY_FILE); assertEquals(Status.ERROR, context.getStatusManager().getLevel()); StatusChecker sc = new StatusChecker(context.getStatusManager()); assertTrue(sc.containsException(SAXParseException.class)); @@ -112,22 +127,42 @@ @Test public void malformedURL() throws JoranException { - System.setProperty(FILE_KEY, "htp://logback.qos.ch"); - tc.doConfigure(INCLUDE_BY_URL); + System.setProperty(INCLUDE_KEY, "htp://logback.qos.ch"); + tc.doConfigure(TOP_BY_URL); assertEquals(Status.ERROR, context.getStatusManager().getLevel()); StatusChecker sc = new StatusChecker(context.getStatusManager()); assertTrue(sc.containsException(MalformedURLException.class)); } @Test - public void testUnknownURL() throws JoranException { - System.setProperty(FILE_KEY, "http://logback2345.qos.ch"); - tc.doConfigure(INCLUDE_BY_URL); + public void unknownURL() throws JoranException { + System.setProperty(INCLUDE_KEY, "http://logback2345.qos.ch"); + tc.doConfigure(TOP_BY_URL); assertEquals(Status.ERROR, context.getStatusManager().getLevel()); StatusChecker sc = new StatusChecker(context.getStatusManager()); assertTrue(sc.containsException(UnknownHostException.class)); } + @Test + public void nestedInclude() throws JoranException { + System.setProperty(SUB_FILE_KEY, INCLUDED_FILE); + System.setProperty(INCLUDE_KEY, SECOND_FILE); + tc.doConfigure(TOP_BY_FILE); + StatusPrinter.print(context); + verifyConfig(1); + + } + + @Test + public void multiInclude() throws JoranException { + System.setProperty(INCLUDE_KEY, INCLUDED_FILE); + System.setProperty(SECOND_FILE_KEY, SECOND_FILE); + tc.doConfigure(MULTI_INCLUDE_BY_FILE); + verifyConfig(3); + } + + + void verifyConfig(int expected) { assertEquals(expected, IncAction.beginCount); assertEquals(expected, IncAction.endCount);