From 56992e1d9ebb130b8fd912d12e553b765bee98b9 Mon Sep 17 00:00:00 2001 From: David Roussel Date: Thu, 18 Apr 2013 10:00:15 +0100 Subject: [PATCH 1/4] LOGBACK-835 - make timestamp properties default to local scope. Can be overriden by setting scope attribute --- .../ch/qos/logback/core/joran/action/TimestampAction.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/logback-core/src/main/java/ch/qos/logback/core/joran/action/TimestampAction.java b/logback-core/src/main/java/ch/qos/logback/core/joran/action/TimestampAction.java index 8121fbae4e..3085132387 100644 --- a/logback-core/src/main/java/ch/qos/logback/core/joran/action/TimestampAction.java +++ b/logback-core/src/main/java/ch/qos/logback/core/joran/action/TimestampAction.java @@ -16,6 +16,7 @@ import ch.qos.logback.core.util.CachingDateFormatter; import org.xml.sax.Attributes; +import ch.qos.logback.core.joran.action.ActionUtil.Scope; import ch.qos.logback.core.joran.spi.ActionException; import ch.qos.logback.core.joran.spi.InterpretationContext; import ch.qos.logback.core.util.OptionHelper; @@ -64,12 +65,16 @@ public void begin(InterpretationContext ec, String name, Attributes attributes) if (inError) return; + String scopeStr = attributes.getValue(SCOPE_ATTRIBUTE); + + Scope scope = ActionUtil.stringToScope(scopeStr); + CachingDateFormatter sdf = new CachingDateFormatter(datePatternStr); String val = sdf.format(timeReference); addInfo("Adding property to the context with key=\"" + keyStr + "\" and value=\"" + val + "\" to the context"); - context.putProperty(keyStr, val); + ActionUtil.setProperty(ec, keyStr, val, scope); } @Override From 2ac2ea954058480bd62ccd715f5f8c078995efe4 Mon Sep 17 00:00:00 2001 From: David Roussel Date: Thu, 18 Apr 2013 15:19:14 +0100 Subject: [PATCH 2/4] LOGBACK-833 - allow previously defined properties (in local scope) to be seen by SiftingJoranConfigurator * implemented part two of Ceki's suggestion * add getCopyOfPropertyMap() to the PropertyContainer and thus InterpretationContext * use local properties that are ineffect at time of creation of SiftingAppender to seed the properties map used when creting the child appenders and properties * removed some generics warnings * in AppenderFactoryBase use List.subList() rather than "copy whole array then delete and re-shuffle elements" --- .../ch/qos/logback/classic/sift/AppenderFactory.java | 7 +++++-- .../java/ch/qos/logback/classic/sift/SiftAction.java | 4 +++- .../classic/sift/SiftingJoranConfigurator.java | 5 ++++- .../src/test/input/joran/timestamp-local.xml | 4 ++++ logback-classic/src/test/input/joran/timestamp.xml | 2 +- .../logback/classic/joran/JoranConfiguratorTest.java | 12 ++++++++++++ .../core/joran/spi/InterpretationContext.java | 9 +++++---- .../qos/logback/core/sift/AppenderFactoryBase.java | 10 ++++------ .../qos/logback/core/sift/SiftingAppenderBase.java | 2 +- .../core/sift/SiftingJoranConfiguratorBase.java | 2 +- .../ch/qos/logback/core/spi/PropertyContainer.java | 4 ++++ 11 files changed, 44 insertions(+), 17 deletions(-) create mode 100644 logback-classic/src/test/input/joran/timestamp-local.xml diff --git a/logback-classic/src/main/java/ch/qos/logback/classic/sift/AppenderFactory.java b/logback-classic/src/main/java/ch/qos/logback/classic/sift/AppenderFactory.java index 554632417c..ec9536e6eb 100644 --- a/logback-classic/src/main/java/ch/qos/logback/classic/sift/AppenderFactory.java +++ b/logback-classic/src/main/java/ch/qos/logback/classic/sift/AppenderFactory.java @@ -14,6 +14,7 @@ package ch.qos.logback.classic.sift; import java.util.List; +import java.util.Map; import ch.qos.logback.classic.spi.ILoggingEvent; import ch.qos.logback.core.joran.event.SaxEvent; @@ -23,14 +24,16 @@ public class AppenderFactory extends AppenderFactoryBase{ String key; + Map parentpropertyMap; - AppenderFactory(List eventList, String key) { + AppenderFactory(List eventList, String key, Map parentpropertyMap) { super(eventList); this.key = key; + this.parentpropertyMap = parentpropertyMap; } public SiftingJoranConfiguratorBase getSiftingJoranConfigurator(String discriminatingValue) { - return new SiftingJoranConfigurator(key, discriminatingValue); + return new SiftingJoranConfigurator(key, discriminatingValue, parentpropertyMap); } } diff --git a/logback-classic/src/main/java/ch/qos/logback/classic/sift/SiftAction.java b/logback-classic/src/main/java/ch/qos/logback/classic/sift/SiftAction.java index f6f0a5f3e4..a09bea220b 100644 --- a/logback-classic/src/main/java/ch/qos/logback/classic/sift/SiftAction.java +++ b/logback-classic/src/main/java/ch/qos/logback/classic/sift/SiftAction.java @@ -15,6 +15,7 @@ import java.util.ArrayList; import java.util.List; +import java.util.Map; import org.xml.sax.Attributes; @@ -40,8 +41,9 @@ public void end(InterpretationContext ic, String name) throws ActionException { Object o = ic.peekObject(); if (o instanceof SiftingAppender) { SiftingAppender sa = (SiftingAppender) o; + Map propertyMap = ic.getCopyOfPropertyMap(); AppenderFactory appenderFactory = new AppenderFactory(seList, sa - .getDiscriminatorKey()); + .getDiscriminatorKey(), propertyMap); sa.setAppenderFactory(appenderFactory); } } diff --git a/logback-classic/src/main/java/ch/qos/logback/classic/sift/SiftingJoranConfigurator.java b/logback-classic/src/main/java/ch/qos/logback/classic/sift/SiftingJoranConfigurator.java index 7bd8a4f9b3..9c06abab21 100644 --- a/logback-classic/src/main/java/ch/qos/logback/classic/sift/SiftingJoranConfigurator.java +++ b/logback-classic/src/main/java/ch/qos/logback/classic/sift/SiftingJoranConfigurator.java @@ -31,10 +31,12 @@ public class SiftingJoranConfigurator extends SiftingJoranConfiguratorBase parentpropertyMap; - SiftingJoranConfigurator(String key, String value) { + SiftingJoranConfigurator(String key, String value, Map parentpropertyMap) { this.key = key; this.value = value; + this.parentpropertyMap = parentpropertyMap; } @Override @@ -61,6 +63,7 @@ protected void buildInterpreter() { omap.put(ActionConst.APPENDER_BAG, new HashMap()); omap.put(ActionConst.FILTER_CHAIN_BAG, new HashMap()); Map propertiesMap = new HashMap(); + propertiesMap.putAll(parentpropertyMap); propertiesMap.put(key, value); interpreter.setInterpretationContextPropertiesMap(propertiesMap); } diff --git a/logback-classic/src/test/input/joran/timestamp-local.xml b/logback-classic/src/test/input/joran/timestamp-local.xml new file mode 100644 index 0000000000..e36da68144 --- /dev/null +++ b/logback-classic/src/test/input/joran/timestamp-local.xml @@ -0,0 +1,4 @@ + + + + \ No newline at end of file diff --git a/logback-classic/src/test/input/joran/timestamp.xml b/logback-classic/src/test/input/joran/timestamp.xml index e36da68144..f184f5ac4e 100644 --- a/logback-classic/src/test/input/joran/timestamp.xml +++ b/logback-classic/src/test/input/joran/timestamp.xml @@ -1,4 +1,4 @@ - + \ No newline at end of file diff --git a/logback-classic/src/test/java/ch/qos/logback/classic/joran/JoranConfiguratorTest.java b/logback-classic/src/test/java/ch/qos/logback/classic/joran/JoranConfiguratorTest.java index d9886e4a21..b200b747f0 100755 --- a/logback-classic/src/test/java/ch/qos/logback/classic/joran/JoranConfiguratorTest.java +++ b/logback-classic/src/test/java/ch/qos/logback/classic/joran/JoranConfiguratorTest.java @@ -323,6 +323,18 @@ public void timestamp() throws JoranException, IOException, assertEquals("expected \"" + expected + "\" but got " + r, expected, r); } + @Test + public void timestampLocal() throws JoranException, IOException, + InterruptedException { + + String configFileAsStr = ClassicTestConstants.JORAN_INPUT_PREFIX + + "timestamp-local.xml"; + configure(configFileAsStr); + + String r = loggerContext.getProperty("testTimestamp"); + assertNull(r); + } + @Test public void encoderCharset() throws JoranException, IOException, InterruptedException { diff --git a/logback-core/src/main/java/ch/qos/logback/core/joran/spi/InterpretationContext.java b/logback-core/src/main/java/ch/qos/logback/core/joran/spi/InterpretationContext.java index ba663b4a57..e9e6d95483 100644 --- a/logback-core/src/main/java/ch/qos/logback/core/joran/spi/InterpretationContext.java +++ b/logback-core/src/main/java/ch/qos/logback/core/joran/spi/InterpretationContext.java @@ -15,7 +15,6 @@ import java.util.ArrayList; import java.util.HashMap; -import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Properties; @@ -62,6 +61,10 @@ public DefaultNestedComponentRegistry getDefaultNestedComponentRegistry() { return defaultNestedComponentRegistry; } + public Map getCopyOfPropertyMap() { + return new HashMap(propertiesMap); + } + void setPropertiesMap(Map propertiesMap) { this.propertiesMap = propertiesMap; } @@ -129,9 +132,7 @@ public void addSubstitutionProperties(Properties props) { if (props == null) { return; } - Iterator i = props.keySet().iterator(); - while (i.hasNext()) { - String key = (String) i.next(); + for (String key : props.stringPropertyNames()) { addSubstitutionProperty(key, props.getProperty(key)); } } diff --git a/logback-core/src/main/java/ch/qos/logback/core/sift/AppenderFactoryBase.java b/logback-core/src/main/java/ch/qos/logback/core/sift/AppenderFactoryBase.java index 0033a790bd..f72880c4ab 100644 --- a/logback-core/src/main/java/ch/qos/logback/core/sift/AppenderFactoryBase.java +++ b/logback-core/src/main/java/ch/qos/logback/core/sift/AppenderFactoryBase.java @@ -13,7 +13,6 @@ */ package ch.qos.logback.core.sift; -import java.util.ArrayList; import java.util.List; import ch.qos.logback.core.Appender; @@ -26,13 +25,12 @@ public abstract class AppenderFactoryBase { final List eventList; protected AppenderFactoryBase(List eventList) { - this.eventList = new ArrayList(eventList); - removeSiftElement(); + this.eventList = removeSiftElement(eventList); + } - void removeSiftElement() { - eventList.remove(0); - eventList.remove(eventList.size() - 1); + List removeSiftElement(List eventList) { + return eventList.subList(1, eventList.size() - 1); } public abstract SiftingJoranConfiguratorBase getSiftingJoranConfigurator(String k); diff --git a/logback-core/src/main/java/ch/qos/logback/core/sift/SiftingAppenderBase.java b/logback-core/src/main/java/ch/qos/logback/core/sift/SiftingAppenderBase.java index 0a60805220..0fe9769649 100644 --- a/logback-core/src/main/java/ch/qos/logback/core/sift/SiftingAppenderBase.java +++ b/logback-core/src/main/java/ch/qos/logback/core/sift/SiftingAppenderBase.java @@ -124,7 +124,7 @@ NOPAppender buildNOPAppender(String discriminatingValue) { /** * @since 0.9.19 */ - public AppenderTracker getAppenderTracker() { + public AppenderTracker getAppenderTracker() { return appenderTracker; } diff --git a/logback-core/src/main/java/ch/qos/logback/core/sift/SiftingJoranConfiguratorBase.java b/logback-core/src/main/java/ch/qos/logback/core/sift/SiftingJoranConfiguratorBase.java index e6e4d3e199..3bf0e2633c 100644 --- a/logback-core/src/main/java/ch/qos/logback/core/sift/SiftingJoranConfiguratorBase.java +++ b/logback-core/src/main/java/ch/qos/logback/core/sift/SiftingJoranConfiguratorBase.java @@ -46,7 +46,7 @@ protected void addImplicitRules(Interpreter interpreter) { int errorEmmissionCount = 0; - protected void oneAndOnlyOneCheck(Map appenderMap) { + protected void oneAndOnlyOneCheck(Map appenderMap) { String errMsg = null; if (appenderMap.size() == 0) { errorEmmissionCount++; diff --git a/logback-core/src/main/java/ch/qos/logback/core/spi/PropertyContainer.java b/logback-core/src/main/java/ch/qos/logback/core/spi/PropertyContainer.java index ead032bb23..7907962e05 100644 --- a/logback-core/src/main/java/ch/qos/logback/core/spi/PropertyContainer.java +++ b/logback-core/src/main/java/ch/qos/logback/core/spi/PropertyContainer.java @@ -13,7 +13,11 @@ */ package ch.qos.logback.core.spi; +import java.util.Map; + public interface PropertyContainer { String getProperty(String key); + + Map getCopyOfPropertyMap(); } From e8a239415a5b2cbb249be046f5e09eb04085ed24 Mon Sep 17 00:00:00 2001 From: David Roussel Date: Thu, 18 Apr 2013 15:20:06 +0100 Subject: [PATCH 3/4] LOGBACK-835 - corrected status message in timestamp --- .../java/ch/qos/logback/core/joran/action/TimestampAction.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/logback-core/src/main/java/ch/qos/logback/core/joran/action/TimestampAction.java b/logback-core/src/main/java/ch/qos/logback/core/joran/action/TimestampAction.java index 3085132387..3a5e90148d 100644 --- a/logback-core/src/main/java/ch/qos/logback/core/joran/action/TimestampAction.java +++ b/logback-core/src/main/java/ch/qos/logback/core/joran/action/TimestampAction.java @@ -73,7 +73,7 @@ public void begin(InterpretationContext ec, String name, Attributes attributes) String val = sdf.format(timeReference); addInfo("Adding property to the context with key=\"" + keyStr - + "\" and value=\"" + val + "\" to the context"); + + "\" and value=\"" + val + "\" to the " + scope + " scope"); ActionUtil.setProperty(ec, keyStr, val, scope); } From af2b445fe6ce1378efe4812b92882c40135e7de0 Mon Sep 17 00:00:00 2001 From: David Roussel Date: Fri, 19 Apr 2013 13:38:57 +0100 Subject: [PATCH 4/4] LOGBACK-833 - unit test improvements to check that a local timestamp property really was set --- .../src/test/input/joran/timestamp-local.xml | 3 +++ .../classic/joran/JoranConfiguratorTest.java | 16 ++++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/logback-classic/src/test/input/joran/timestamp-local.xml b/logback-classic/src/test/input/joran/timestamp-local.xml index e36da68144..dd20ba01da 100644 --- a/logback-classic/src/test/input/joran/timestamp-local.xml +++ b/logback-classic/src/test/input/joran/timestamp-local.xml @@ -1,4 +1,7 @@ + \ No newline at end of file diff --git a/logback-classic/src/test/java/ch/qos/logback/classic/joran/JoranConfiguratorTest.java b/logback-classic/src/test/java/ch/qos/logback/classic/joran/JoranConfiguratorTest.java index b200b747f0..92344bc7e0 100755 --- a/logback-classic/src/test/java/ch/qos/logback/classic/joran/JoranConfiguratorTest.java +++ b/logback-classic/src/test/java/ch/qos/logback/classic/joran/JoranConfiguratorTest.java @@ -15,6 +15,8 @@ import java.io.File; import java.io.IOException; +import java.text.SimpleDateFormat; +import java.util.Date; import java.util.concurrent.TimeUnit; import ch.qos.logback.classic.jul.JULHelper; @@ -326,13 +328,27 @@ public void timestamp() throws JoranException, IOException, @Test public void timestampLocal() throws JoranException, IOException, InterruptedException { + + String sysProp = "ch.qos.logback.classic.joran.JoranConfiguratorTest.timestampLocal"; + System.setProperty(sysProp, ""); String configFileAsStr = ClassicTestConstants.JORAN_INPUT_PREFIX + "timestamp-local.xml"; configure(configFileAsStr); + // It's hard to test the local variable has been set, as it's not + // visible from here. But instead we test that it's not set in the + // context. And check that a system property has been replaced with the + // contents of the local variable + String r = loggerContext.getProperty("testTimestamp"); assertNull(r); + + String exprected = "today is " + new SimpleDateFormat("yyyy-MM").format(new Date()); + String sysPropValue = System.getProperty(sysProp); + assertEquals(exprected, sysPropValue); + + } @Test