Author: oheger
Date: Sun Sep 29 20:12:54 2013
New Revision: 1527396

URL: http://svn.apache.org/r1527396
Log:
[CONFIGURATION-555] Fixed a problem with the handling of the xml:space 
attribute.

The attribute was only evaluated for sub elements, but not for the current
element. However, now there is a corner case of an element which only
contains sub elements and has the attribute set to "preserve". In this case,
a trim is done because it does not make sense to assign a value consisting
only of whitespace to this element.

Modified:
    commons/proper/configuration/trunk/src/changes/changes.xml
    
commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration/XMLConfiguration.java
    
commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration/TestXMLConfiguration.java
    commons/proper/configuration/trunk/src/test/resources/test.xml

Modified: commons/proper/configuration/trunk/src/changes/changes.xml
URL: 
http://svn.apache.org/viewvc/commons/proper/configuration/trunk/src/changes/changes.xml?rev=1527396&r1=1527395&r2=1527396&view=diff
==============================================================================
--- commons/proper/configuration/trunk/src/changes/changes.xml (original)
+++ commons/proper/configuration/trunk/src/changes/changes.xml Sun Sep 29 
20:12:54 2013
@@ -27,6 +27,11 @@
   <body>
     <release version="2.0" date="in SVN"
       description="TBD">
+      <action dev="oheger" type="update" issue="CONFIGURATION-555">
+        Fixed a bug in the handling of the xml:space attribute in
+        XMLConfiguration. The attribute is now also applied to the current
+        element, not only to sub elements.
+      </action>
       <action dev="oheger" type="update" issue="CONFIGURATION-554">
         BeanHelper is no longer a static utility class. Instances can be
         created with a specific configuration of bean factories. There is still

Modified: 
commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration/XMLConfiguration.java
URL: 
http://svn.apache.org/viewvc/commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration/XMLConfiguration.java?rev=1527396&r1=1527395&r2=1527396&view=diff
==============================================================================
--- 
commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration/XMLConfiguration.java
 (original)
+++ 
commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration/XMLConfiguration.java
 Sun Sep 29 20:12:54 2013
@@ -26,7 +26,6 @@ import java.io.Writer;
 import java.net.URL;
 import java.util.ArrayList;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
@@ -53,6 +52,7 @@ import org.apache.commons.configuration.
 import org.apache.commons.configuration.resolver.EntityRegistry;
 import org.apache.commons.configuration.tree.ConfigurationNode;
 import org.apache.commons.configuration.tree.DefaultConfigurationNode;
+import org.apache.commons.lang3.StringUtils;
 import org.apache.commons.logging.LogFactory;
 import org.w3c.dom.Attr;
 import org.w3c.dom.CDATASection;
@@ -544,10 +544,13 @@ public class XMLConfiguration extends Ba
      *
      * @param node the actual node
      * @param element the actual XML element
-     * @param elemRefs a flag whether references to the XML elements should be 
set
-     * @param trim a flag whether the text content of elements should be 
trimmed;
-     * this controls the whitespace handling
-     * @return a map with all attribute values extracted for the current node
+     * @param elemRefs a flag whether references to the XML elements should be
+     *        set
+     * @param trim a flag whether the text content of elements should be
+     *        trimmed; this controls the whitespace handling
+     * @return a map with all attribute values extracted for the current node;
+     *         this map also contains the value of the trim flag for this node
+     *         under the key {@value #ATTR_SPACE}
      */
     private Map<String, String> constructHierarchy(ConfigurationNode node,
             Element element, boolean elemRefs, boolean trim)
@@ -555,6 +558,7 @@ public class XMLConfiguration extends Ba
         boolean trimFlag = shouldTrim(element, trim);
         Map<String, String> attributes =
                 processAttributes(node, element, elemRefs);
+        attributes.put(ATTR_SPACE, String.valueOf(trimFlag));
         StringBuilder buffer = new StringBuilder();
         NodeList list = element.getChildNodes();
         for (int i = 0; i < list.getLength(); i++)
@@ -568,7 +572,8 @@ public class XMLConfiguration extends Ba
                 Map<String, String> attrmap =
                         constructHierarchy(childNode, child, elemRefs, 
trimFlag);
                 node.addChild(childNode);
-                handleDelimiters(node, childNode, trimFlag, attrmap);
+                Boolean childTrim = 
Boolean.valueOf(attrmap.remove(ATTR_SPACE));
+                handleDelimiters(node, childNode, childTrim.booleanValue(), 
attrmap);
             }
             else if (w3cNode instanceof Text)
             {
@@ -577,11 +582,7 @@ public class XMLConfiguration extends Ba
             }
         }
 
-        String text = buffer.toString();
-        if (trimFlag)
-        {
-            text = text.trim();
-        }
+        String text = determineValue(node, buffer.toString(), trimFlag);
         if (text.length() > 0 || (!hasChildren(node) && node != getRootNode()))
         {
             node.setValue(text);
@@ -601,6 +602,28 @@ public class XMLConfiguration extends Ba
     }
 
     /**
+     * Determines the value of a configuration node. This method mainly checks
+     * whether the text value is to be trimmed or not. This is normally defined
+     * by the trim flag. However, if the node has children and its content is
+     * only whitespace, then it makes no sense to store any value; this would
+     * only scramble layout when the configuration is saved again.
+     *
+     * @param node the current {@code ConfigurationNode}
+     * @param content the text content of this node
+     * @param trimFlag the trim flag
+     * @return the value to be stored for this node
+     */
+    private static String determineValue(ConfigurationNode node,
+            String content, boolean trimFlag)
+    {
+        boolean shouldTrim =
+                trimFlag
+                        || (StringUtils.isBlank(content) && node
+                                .getChildrenCount() > 0);
+        return shouldTrim ? content.trim() : content;
+    }
+
+    /**
      * Helper method for constructing node objects for the attributes of the
      * given XML element.
      *
@@ -613,15 +636,7 @@ public class XMLConfiguration extends Ba
             Element element, boolean elemRefs)
     {
         NamedNodeMap attributes = element.getAttributes();
-        Map<String, String> attrmap;
-        if (attributes.getLength() > 0)
-        {
-            attrmap = new HashMap<String, String>();
-        }
-        else
-        {
-            attrmap = Collections.emptyMap();
-        }
+        Map<String, String> attrmap = new HashMap<String, String>();
 
         for (int i = 0; i < attributes.getLength(); ++i)
         {

Modified: 
commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration/TestXMLConfiguration.java
URL: 
http://svn.apache.org/viewvc/commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration/TestXMLConfiguration.java?rev=1527396&r1=1527395&r2=1527396&view=diff
==============================================================================
--- 
commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration/TestXMLConfiguration.java
 (original)
+++ 
commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration/TestXMLConfiguration.java
 Sun Sep 29 20:12:54 2013
@@ -1432,6 +1432,16 @@ public class TestXMLConfiguration
     }
 
     /**
+     * Tests whether the xml:space attribute works directly on the current
+     * element. This test is related to CONFIGURATION-555.
+     */
+    @Test
+    public void testPreserveSpaceOnElement()
+    {
+        assertEquals("Wrong value", " preserved ", 
conf.getString("spaceElement"));
+    }
+
+    /**
      * Tests whether the xml:space attribute can be overridden in nested
      * elements.
      */

Modified: commons/proper/configuration/trunk/src/test/resources/test.xml
URL: 
http://svn.apache.org/viewvc/commons/proper/configuration/trunk/src/test/resources/test.xml?rev=1527396&r1=1527395&r2=1527396&view=diff
==============================================================================
--- commons/proper/configuration/trunk/src/test/resources/test.xml (original)
+++ commons/proper/configuration/trunk/src/test/resources/test.xml Sun Sep 29 
20:12:54 2013
@@ -115,4 +115,5 @@ And even longer.
       <description xml:space="default">     Some text      </description>
       <testInvalid xml:space="invalid">     Some other text </testInvalid>
     </space>
+    <spaceElement xml:space="preserve"> preserved </spaceElement>
 </testconfig>


Reply via email to