You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@logging.apache.org by rp...@apache.org on 2016/09/12 15:13:39 UTC

logging-log4j2 git commit: LOG4J2-1313 added support for Property configurations with "" (empty string) as value. Show status logger error if both attribute and element value are specified.

Repository: logging-log4j2
Updated Branches:
  refs/heads/master c78e50691 -> 235955ab4


LOG4J2-1313 added support for Property configurations with "" (empty string) as value. Show status logger error if both attribute and element value are specified.


Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo
Commit: http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit/235955ab
Tree: http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/235955ab
Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/235955ab

Branch: refs/heads/master
Commit: 235955ab4ed2bd0a9a5256b1e0e3017eb7a90c4f
Parents: c78e506
Author: rpopma <rp...@apache.org>
Authored: Tue Sep 13 00:13:38 2016 +0900
Committer: rpopma <rp...@apache.org>
Committed: Tue Sep 13 00:13:38 2016 +0900

----------------------------------------------------------------------
 .../logging/log4j/core/config/Property.java     |  4 ++--
 .../plugins/visitors/PluginValueVisitor.java    | 17 ++++++++++++---
 .../logging/log4j/core/config/PropertyTest.java | 22 +++++++++++++-------
 .../src/test/resources/configPropertyTest.xml   |  6 ++++--
 src/changes/changes.xml                         |  3 +++
 5 files changed, 38 insertions(+), 14 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/235955ab/log4j-core/src/main/java/org/apache/logging/log4j/core/config/Property.java
----------------------------------------------------------------------
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/Property.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/Property.java
index 4dca05a..06a8a43 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/Property.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/Property.java
@@ -54,7 +54,7 @@ public final class Property {
      * @return the value of the property.
      */
     public String getValue() {
-        return value;
+        return value == null ? "" : value; // LOG4J2-1313 null would be same as Property not existing
     }
 
     /**
@@ -84,6 +84,6 @@ public final class Property {
 
     @Override
     public String toString() {
-        return name + '=' + value;
+        return name + '=' + getValue();
     }
 }

http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/235955ab/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/visitors/PluginValueVisitor.java
----------------------------------------------------------------------
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/visitors/PluginValueVisitor.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/visitors/PluginValueVisitor.java
index 82dc20d..8544570 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/visitors/PluginValueVisitor.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/visitors/PluginValueVisitor.java
@@ -34,10 +34,21 @@ public class PluginValueVisitor extends AbstractPluginVisitor<PluginValue> {
 
     @Override
     public Object visit(final Configuration configuration, final Node node, final LogEvent event,
-                        final StringBuilder log) {
+            final StringBuilder log) {
         final String name = this.annotation.value();
-        final String rawValue = Strings.isNotEmpty(node.getValue()) ? node.getValue() :
-                removeAttributeValue(node.getAttributes(), "value");
+        final String elementValue = node.getValue();
+        final String attributeValue = node.getAttributes().get("value");
+        String rawValue = null; // if neither is specified, return null (LOG4J2-1313)
+        if (Strings.isNotEmpty(elementValue)) {
+            if (Strings.isNotEmpty(attributeValue)) {
+                LOGGER.error("Configuration contains {} with both attribute value ({}) AND element" +
+                                " value ({}). Please specify only one value. Using the element value.",
+                        node.getName(), attributeValue, elementValue);
+            }
+            rawValue = elementValue;
+        } else {
+            rawValue = removeAttributeValue(node.getAttributes(), "value");
+        }
         final String value = this.substitutor.replace(event, rawValue);
         StringBuilders.appendKeyDqValue(log, name, value);
         return value;

http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/235955ab/log4j-core/src/test/java/org/apache/logging/log4j/core/config/PropertyTest.java
----------------------------------------------------------------------
diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/core/config/PropertyTest.java b/log4j-core/src/test/java/org/apache/logging/log4j/core/config/PropertyTest.java
index 4c087e5..e1ec7d0 100644
--- a/log4j-core/src/test/java/org/apache/logging/log4j/core/config/PropertyTest.java
+++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/config/PropertyTest.java
@@ -36,7 +36,6 @@ public class PropertyTest {
     @ClassRule
     public static LoggerContextRule context = new LoggerContextRule(CONFIG);
 
-    @Ignore("TODO fix LOG4J2-1313")
     @Test
     public void testEmptyAttribute() throws Exception {
         final org.apache.logging.log4j.Logger logger = LogManager.getLogger();
@@ -49,21 +48,30 @@ public class PropertyTest {
         assertNotNull("No Messages", messages);
         assertEquals("message count" + messages, 1, messages.size());
 
-//        <Property name="elementKey">elementValue</Property>
-//        <Property name="emptyElementKey"></Property>
-//        <Property name="attributeKey" value="attributeValue" />
-//        <Property name="attributeWithEmptyElementKey" value="attributeValue2"></Property>
-//        <Property name="bothElementAndAttributeKey" value="attributeValue"3>elementValue</Property>
+        //<Property name="emptyElementKey" />
+        //<Property name="emptyAttributeKey" value="" />
+        //<Property name="emptyAttributeKey2" value=""></Property>
+        //<Property name="elementKey">elementValue</Property>
+        //<Property name="attributeKey" value="attributeValue" />
+        //<Property name="attributeWithEmptyElementKey" value="attributeValue2"></Property>
+        //<Property name="bothElementAndAttributeKey" value="attributeValue3">elementValue3</Property>
         final String expect = "1=elementValue" + // ${sys:elementKey}
                 ",2=" + // ${sys:emptyElementKey}
+                ",a=" + // ${sys:emptyAttributeKey}
+                ",b=" + // ${sys:emptyAttributeKey2}
                 ",3=attributeValue" + // ${sys:attributeKey}
                 ",4=attributeValue2" + // ${sys:attributeWithEmptyElementKey}
-                ",5=attributeValue3,m=msg"; // ${sys:bothElementAndAttributeKey}
+                ",5=elementValue3,m=msg"; // ${sys:bothElementAndAttributeKey}
         assertEquals(expect, messages.get(0));
         app.clear();
     }
 
     @Test
+    public void testNullValueIsConvertedToEmptyString() { // LOG4J2-1313 <Property name="x" /> support
+        assertEquals("", Property.createProperty("name", null).getValue());
+    }
+
+    @Test
     public void testIsValueNeedsLookup() {
         assertTrue("with ${ as value", Property.createProperty("", "${").isValueNeedsLookup());
         assertTrue("with ${ in value", Property.createProperty("", "blah${blah").isValueNeedsLookup());

http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/235955ab/log4j-core/src/test/resources/configPropertyTest.xml
----------------------------------------------------------------------
diff --git a/log4j-core/src/test/resources/configPropertyTest.xml b/log4j-core/src/test/resources/configPropertyTest.xml
index 04154d8..261f7cf 100644
--- a/log4j-core/src/test/resources/configPropertyTest.xml
+++ b/log4j-core/src/test/resources/configPropertyTest.xml
@@ -17,15 +17,17 @@
   -->
 <Configuration status="ERROR">
   <Properties>
+    <Property name="emptyElementKey" />
+    <Property name="emptyAttributeKey" value="" />
+    <Property name="emptyAttributeKey2" value=""></Property>
     <Property name="elementKey">elementValue</Property>
-    <Property name="emptyElementKey"></Property>
     <Property name="attributeKey" value="attributeValue" />
     <Property name="attributeWithEmptyElementKey" value="attributeValue2"></Property>
     <Property name="bothElementAndAttributeKey" value="attributeValue3">elementValue3</Property>
   </Properties>
   <Appenders>
     <List name="List">
-      <PatternLayout pattern="1=${sys:elementKey},2=${sys:emptyElementKey},3=${sys:attributeKey},4=${sys:attributeWithEmptyElementKey},5=${sys:bothElementAndAttributeKey},m=%m" />
+      <PatternLayout pattern="1=${sys:elementKey},2=${sys:emptyElementKey},a=${sys:emptyAttributeKey},b=${sys:emptyAttributeKey2},3=${sys:attributeKey},4=${sys:attributeWithEmptyElementKey},5=${sys:bothElementAndAttributeKey},m=%m" />
     </List>
   </Appenders>
   <Loggers>

http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/235955ab/src/changes/changes.xml
----------------------------------------------------------------------
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index e0fa64d..f0b28d5 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -24,6 +24,9 @@
   </properties>
   <body>
     <release version="2.7" date="2016-MM-DD" description="GA Release 2.7">
+      <action issue="LOG4J2-1313" dev="rpopma" type="fix" due-to="Philipp Knobel, Leon Finker">
+        Support Property values to be specified in configuration as a value attribute as well as an element.
+      </action>
       <action issue="LOG4J2-1575" dev="rpopma" type="fix">
         (GC) LoggerConfig now stores configuration properties in a List, not a Map to prevent creating temporary Iterator objects. Added method LoggerConfig#getPropertyList(), deprecated method #getProperties().
       </action>