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>