Author: oheger
Date: Sat Jan 21 20:21:41 2017
New Revision: 1779753
URL: http://svn.apache.org/viewvc?rev=1779753&view=rev
Log:
[CONFIGURATION-649] Improved list handling in XMLConfiguration.
List properties defined as single string with a delimiter character
now retain their format when the configuration is saved.
Note that the implemented solution is not perfect, it cannot handle
all possible constellations. For instance, for lists defined in a
mixed form (multiple XML elements each of wich defines multiple
list values) cannot be handled and are reformatted on saving.
Added:
commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration2/XMLListReference.java
commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration2/TestXMLListHandling.java
Modified:
commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration2/XMLConfiguration.java
commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration2/TestXMLConfiguration.java
Modified:
commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration2/XMLConfiguration.java
URL:
http://svn.apache.org/viewvc/commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration2/XMLConfiguration.java?rev=1779753&r1=1779752&r2=1779753&view=diff
==============================================================================
---
commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration2/XMLConfiguration.java
(original)
+++
commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration2/XMLConfiguration.java
Sat Jan 21 20:21:41 2017
@@ -614,10 +614,10 @@ public class XMLConfiguration extends Ba
Boolean childTrim =
Boolean.valueOf(attrmap.remove(ATTR_SPACE_INTERNAL));
childNode.addAttributes(attrmap);
ImmutableNode newChild =
- createChildNodeWithValue(node, childNode,
+ createChildNodeWithValue(node, childNode, child,
refChildValue.getValue(),
- childTrim.booleanValue(), attrmap);
- if (elemRefs != null)
+ childTrim.booleanValue(), attrmap, elemRefs);
+ if (elemRefs != null && !elemRefs.containsKey(newChild))
{
elemRefs.put(newChild, child);
}
@@ -692,14 +692,18 @@ public class XMLConfiguration extends Ba
*
* @param parent the builder for the parent element
* @param child the builder for the child element
+ * @param elem the associated XML element
* @param value the value of the child element
* @param trim flag whether texts of elements should be trimmed
* @param attrmap a map with the attributes of the current node
+ * @param elemRefs a map for assigning references objects to nodes; can be
+ * <b>null</b>, then reference objects are irrelevant
* @return the first child node added to the parent
*/
- private ImmutableNode createChildNodeWithValue(
- ImmutableNode.Builder parent, ImmutableNode.Builder child,
- String value, boolean trim, Map<String, String> attrmap)
+ private ImmutableNode createChildNodeWithValue(ImmutableNode.Builder
parent,
+ ImmutableNode.Builder child, Element elem, String value,
+ boolean trim, Map<String, String> attrmap,
+ Map<ImmutableNode, Object> elemRefs)
{
ImmutableNode addedChildNode;
Collection<String> values;
@@ -715,11 +719,13 @@ public class XMLConfiguration extends Ba
if (values.size() > 1)
{
+ Map<ImmutableNode, Object> refs = isSingleElementList(elem) ?
elemRefs : null;
Iterator<String> it = values.iterator();
// Create new node for the original child's first value
child.value(it.next());
addedChildNode = child.create();
parent.addChild(addedChildNode);
+ XMLListReference.assignListReference(refs, addedChildNode, elem);
// add multiple new children
while (it.hasNext())
@@ -728,7 +734,9 @@ public class XMLConfiguration extends Ba
c.name(addedChildNode.getNodeName());
c.value(it.next());
c.addAttributes(attrmap);
- parent.addChild(c.create());
+ ImmutableNode newChild = c.create();
+ parent.addChild(newChild);
+ XMLListReference.assignListReference(refs, newChild, null);
}
}
else if (values.size() == 1)
@@ -749,6 +757,45 @@ public class XMLConfiguration extends Ba
}
/**
+ * Checks whether an element defines a complete list. If this is the case,
+ * extended list handling can be applied.
+ *
+ * @param element the element to be checked
+ * @return a flag whether this is the only element defining the list
+ */
+ private static boolean isSingleElementList(Element element)
+ {
+ Node parentNode = element.getParentNode();
+ return countChildElements(parentNode, element.getTagName()) == 1;
+ }
+
+ /**
+ * Determines the number of child elements of this given node with the
+ * specified node name.
+ *
+ * @param parent the parent node
+ * @param name the name in question
+ * @return the number of child elements with this name
+ */
+ private static int countChildElements(Node parent, String name)
+ {
+ NodeList childNodes = parent.getChildNodes();
+ int count = 0;
+ for (int i = 0; i < childNodes.getLength(); i++)
+ {
+ Node item = childNodes.item(i);
+ if (item instanceof Element)
+ {
+ if (name.equals(((Element) item).getTagName()))
+ {
+ count++;
+ }
+ }
+ }
+ return count;
+ }
+
+ /**
* Checks whether the content of the current XML element should be trimmed.
* This method checks whether a {@code xml:space} attribute is
* present and evaluates its value. See <a
@@ -1102,8 +1149,11 @@ public class XMLConfiguration extends Ba
{
for (Object ref : refHandler.removedReferences())
{
- Node removedElem = (Node) ref;
- removeReference((Element) elementMapping.get(removedElem));
+ if (ref instanceof Node)
+ {
+ Node removedElem = (Node) ref;
+ removeReference((Element) elementMapping.get(removedElem));
+ }
}
}
@@ -1116,6 +1166,11 @@ public class XMLConfiguration extends Ba
ImmutableNode sibling1, ImmutableNode sibling2,
ReferenceNodeHandler refHandler)
{
+ if(XMLListReference.isListNode(newNode, refHandler))
+ {
+ return;
+ }
+
Element elem = document.createElement(newNode.getNodeName());
newElements.put(newNode, elem);
updateAttributes(newNode, elem);
@@ -1152,8 +1207,27 @@ public class XMLConfiguration extends Ba
protected void update(ImmutableNode node, Object reference,
ReferenceNodeHandler refHandler)
{
+ if(XMLListReference.isListNode(node, refHandler))
+ {
+ if(XMLListReference.isFirstListItem(node, refHandler))
+ {
+ String value = XMLListReference.listValue(node,
refHandler, listDelimiterHandler);
+ updateElement(node, refHandler, value);
+ }
+ }
+ else
+ {
+ Object value =
listDelimiterHandler.escape(refHandler.getValue(node),
+ ListDelimiterHandler.NOOP_TRANSFORMER);
+ updateElement(node, refHandler, value);
+ }
+ }
+
+ private void updateElement(ImmutableNode node, ReferenceNodeHandler
refHandler,
+ Object value)
+ {
Element element = getElement(node, refHandler);
- updateElement(element, refHandler.getValue(node));
+ updateElement(element, value);
updateAttributes(node, element);
}
@@ -1176,9 +1250,7 @@ public class XMLConfiguration extends Ba
}
else
{
- String newValue =
- String.valueOf(listDelimiterHandler.escape(value,
- ListDelimiterHandler.NOOP_TRANSFORMER));
+ String newValue = String.valueOf(value);
if (txtNode == null)
{
txtNode = document.createTextNode(newValue);
@@ -1236,6 +1308,10 @@ public class XMLConfiguration extends Ba
((XMLDocumentHelper) reference).getDocument()
.getDocumentElement();
}
+ else if(reference instanceof XMLListReference)
+ {
+ element = ((XMLListReference) reference).getElement();
+ }
else
{
element = (Node) reference;
Added:
commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration2/XMLListReference.java
URL:
http://svn.apache.org/viewvc/commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration2/XMLListReference.java?rev=1779753&view=auto
==============================================================================
---
commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration2/XMLListReference.java
(added)
+++
commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration2/XMLListReference.java
Sat Jan 21 20:21:41 2017
@@ -0,0 +1,220 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.configuration2;
+
+import org.apache.commons.configuration2.convert.DefaultListDelimiterHandler;
+import org.apache.commons.configuration2.convert.ListDelimiterHandler;
+import org.apache.commons.configuration2.ex.ConfigurationRuntimeException;
+import org.apache.commons.configuration2.tree.ImmutableNode;
+import org.apache.commons.configuration2.tree.ReferenceNodeHandler;
+import org.apache.commons.lang3.StringUtils;
+import org.w3c.dom.Element;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * <p>
+ * An internal class implementing list handling functionality for
+ * {@link XMLConfiguration}.
+ * </p>
+ * <p>
+ * When an XML document is loaded list properties defined as a string with
+ * multiple values separated by the list delimiter are split into multiple
+ * configuration nodes. When the configuration is saved the original format
+ * should be kept if possible. This class implements functionality to achieve
+ * this. Instances are used as references associated with configuration nodes
so
+ * that the original format can be restored when the configuration is saved.
+ * </p>
+ */
+class XMLListReference
+{
+ /** The wrapped XML element. */
+ private final Element element;
+
+ /**
+ * Private constructor. No instances can be created from other classes.
+ *
+ * @param e the associated element
+ */
+ private XMLListReference(Element e)
+ {
+ element = e;
+ }
+
+ /**
+ * Returns the associated element.
+ *
+ * @return the associated XML element
+ */
+ public Element getElement()
+ {
+ return element;
+ }
+
+ /**
+ * Assigns an instance of this class as reference to the specified
+ * configuration node. This reference acts as a marker indicating that this
+ * node is subject to extended list handling.
+ *
+ * @param refs the mapping for node references
+ * @param node the affected configuration node
+ * @param elem the current XML element
+ */
+ public static void assignListReference(Map<ImmutableNode, Object> refs,
+ ImmutableNode node, Element elem)
+ {
+ if (refs != null)
+ {
+ refs.put(node, new XMLListReference(elem));
+ }
+ }
+
+ /**
+ * Checks whether the specified configuration node has to be taken into
+ * account for list handling. This is the case if the node's parent has at
+ * least one child node with the same name which has a special list
+ * reference assigned. (Note that the passed in node does not necessarily
+ * have such a reference; if it has been added at a later point in time, it
+ * also has to become an item of the list.)
+ *
+ * @param node the configuration node
+ * @param handler the reference node handler
+ * @return a flag whether this node is relevant for list handling
+ */
+ public static boolean isListNode(ImmutableNode node,
+ ReferenceNodeHandler handler)
+ {
+ if (hasListReference(node, handler))
+ {
+ return true;
+ }
+
+ ImmutableNode parent = handler.getParent(node);
+ if (parent != null)
+ {
+ for (int i = 0; i < handler.getChildrenCount(parent, null); i++)
+ {
+ ImmutableNode child = handler.getChild(parent, i);
+ if (hasListReference(child, handler) && nameEquals(node,
child))
+ {
+ return true;
+ }
+ }
+ }
+ return false;
+ }
+
+ /**
+ * Checks whether the specified node is the first node of a list. This is
+ * needed because all items of the list are collected and stored as value
of
+ * the first list node. Note: This method requires that the passed in node
+ * is a list node, so
+ * {@link #isListNode(ImmutableNode, ReferenceNodeHandler)} must have
+ * returned <strong>true</strong> for it.
+ *
+ * @param node the configuration node
+ * @param handler the reference node handler
+ * @return a flag whether this is the first node of a list
+ */
+ public static boolean isFirstListItem(ImmutableNode node,
+ ReferenceNodeHandler handler)
+ {
+ ImmutableNode parent = handler.getParent(node);
+ ImmutableNode firstItem = null;
+ int idx = 0;
+ while (firstItem == null)
+ {
+ ImmutableNode child = handler.getChild(parent, idx);
+ if (nameEquals(node, child))
+ {
+ firstItem = child;
+ }
+ idx++;
+ }
+ return firstItem == node;
+ }
+
+ /**
+ * Constructs the concatenated string value of all items comprising the
list
+ * the specified node belongs to. This method is called when saving an
+ * {@link XMLConfiguration}. Then configuration nodes created for list
items
+ * have to be collected again and transformed into a string defining all
+ * list elements.
+ *
+ * @param node the configuration node
+ * @param nodeHandler the reference node handler
+ * @param delimiterHandler the list delimiter handler of the configuration
+ * @return a string with all values of the current list
+ * @throws ConfigurationRuntimeException if the list delimiter handler does
+ * not support the transformation of list items to a string
+ */
+ public static String listValue(ImmutableNode node,
+ ReferenceNodeHandler nodeHandler,
+ ListDelimiterHandler delimiterHandler)
+ {
+ // cannot be null if the current node is a list node
+ ImmutableNode parent = nodeHandler.getParent(node);
+ List<ImmutableNode> items =
+ nodeHandler.getChildren(parent, node.getNodeName());
+ List<Object> values = new ArrayList<Object>(items.size());
+ for (ImmutableNode n : items)
+ {
+ values.add(n.getValue());
+ }
+ try
+ {
+ return String.valueOf(delimiterHandler.escapeList(values,
+ DefaultListDelimiterHandler.NOOP_TRANSFORMER));
+ }
+ catch (UnsupportedOperationException e)
+ {
+ throw new ConfigurationRuntimeException(
+ "List handling not supported by "
+ + "the current ListDelimiterHandler! Make sure
that the same delimiter "
+ + "handler is used for loading and saving the
configuration.",
+ e);
+ }
+ }
+
+ /**
+ * Checks whether the specified node has an associated list reference. This
+ * marks the node as part of a list.
+ *
+ * @param node the node to be checked
+ * @param handler the reference handler
+ * @return a flag whether this node has a list reference
+ */
+ private static boolean hasListReference(ImmutableNode node,
+ ReferenceNodeHandler handler)
+ {
+ return handler.getReference(node) instanceof XMLListReference;
+ }
+
+ /**
+ * Helper method for comparing the names of two nodes.
+ *
+ * @param n1 node 1
+ * @param n2 node 2
+ * @return a flag whether these nodes have equal names
+ */
+ private static boolean nameEquals(ImmutableNode n1, ImmutableNode n2)
+ {
+ return StringUtils.equals(n2.getNodeName(), n1.getNodeName());
+ }
+}
Modified:
commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration2/TestXMLConfiguration.java
URL:
http://svn.apache.org/viewvc/commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration2/TestXMLConfiguration.java?rev=1779753&r1=1779752&r2=1779753&view=diff
==============================================================================
---
commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration2/TestXMLConfiguration.java
(original)
+++
commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration2/TestXMLConfiguration.java
Sat Jan 21 20:21:41 2017
@@ -1583,6 +1583,7 @@ public class TestXMLConfiguration
public void testAddPropertyListWithDelimiterParsingDisabled()
throws ConfigurationException
{
+ conf.clear();
String prop = "delimiterListProp";
conf.setListDelimiterHandler(DisabledListDelimiterHandler.INSTANCE);
List<String> list = Arrays.asList("val", "val2", "val3");
Added:
commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration2/TestXMLListHandling.java
URL:
http://svn.apache.org/viewvc/commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration2/TestXMLListHandling.java?rev=1779753&view=auto
==============================================================================
---
commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration2/TestXMLListHandling.java
(added)
+++
commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration2/TestXMLListHandling.java
Sat Jan 21 20:21:41 2017
@@ -0,0 +1,208 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.configuration2;
+
+import org.apache.commons.configuration2.convert.DefaultListDelimiterHandler;
+import org.apache.commons.configuration2.convert.DisabledListDelimiterHandler;
+import org.apache.commons.configuration2.ex.ConfigurationException;
+import org.apache.commons.configuration2.ex.ConfigurationRuntimeException;
+import org.apache.commons.configuration2.io.FileHandler;
+import org.apache.commons.lang3.StringUtils;
+import org.junit.Before;
+import org.junit.Test;
+
+import java.io.StringReader;
+import java.io.StringWriter;
+import java.util.Arrays;
+import java.util.List;
+
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThat;
+
+/**
+ * Test class to test the handling of list structures in XMLConfiguration.
+ */
+public class TestXMLListHandling
+{
+ /** XML to be loaded by the configuration. */
+ private static final String SOURCE = "<config>" + "<values>a,b,c</values>"
+ + "<split><value>1</value><value>2</value></split>"
+ +
"<mixed><values>foo,blah</values><values>bar,baz</values></mixed>"
+ + "</config>";
+
+ /** Key for the string property with multiple values. */
+ private static final String KEY_VALUES = "values";
+
+ /** Key for the split list property. */
+ private static final String KEY_SPLIT = "split.value";
+
+ /** The XML element name for the single values of the split list. */
+ private static final String ELEM_SPLIT = "value";
+
+ /** Configuration to be tested. */
+ private XMLConfiguration config;
+
+ @Before
+ public void setUp() throws Exception
+ {
+ config = readFromString(SOURCE);
+ }
+
+ /**
+ * Parses the specified string into an XML configuration.
+ *
+ * @param xml the XML to be parsed
+ * @return the resulting configuration
+ */
+ private static XMLConfiguration readFromString(String xml)
+ throws ConfigurationException
+ {
+ XMLConfiguration config = new XMLConfiguration();
+ config.setListDelimiterHandler(new DefaultListDelimiterHandler(','));
+ FileHandler handler = new FileHandler(config);
+ handler.load(new StringReader(xml));
+ return config;
+ }
+
+ /**
+ * Saves the test configuration into a string.
+ *
+ * @return the resulting string
+ */
+ private String saveToString() throws ConfigurationException
+ {
+ StringWriter writer = new StringWriter(4096);
+ FileHandler handler = new FileHandler(config);
+ handler.save(writer);
+ return writer.toString();
+ }
+
+ /**
+ * Generates an XML element with the specified value as body.
+ *
+ * @param key the key
+ * @param value the value
+ * @return the string representation of this element
+ */
+ private static String element(String key, String value)
+ {
+ return "<" + key + '>' + value + "</" + key + '>';
+ }
+
+ /**
+ * Checks whether the specified XML contains a list of values as a single,
+ * comma-separated string.
+ *
+ * @param xml the XML
+ * @param key the key
+ * @param values the expected values
+ */
+ private static void checkCommaSeparated(String xml, String key,
+ String... values)
+ {
+ String strValues = StringUtils.join(values, ',');
+ String element = element(key, strValues);
+ assertThat(xml, containsString(element));
+ }
+
+ /**
+ * Checks whether the specified XML contains a list of values as multiple
+ * XML elements.
+ *
+ * @param xml the XML
+ * @param key the key
+ * @param values the expected values
+ */
+ private static void checkSplit(String xml, String key, String... values)
+ {
+ for (String v : values)
+ {
+ assertThat(xml, containsString(element(key, v)));
+ }
+ }
+
+ /**
+ * Tests that the list format is kept if properties are not touched,
+ */
+ @Test
+ public void testSaveNoChanges() throws ConfigurationException
+ {
+ String xml = saveToString();
+
+ checkSplit(xml, ELEM_SPLIT, "1", "2");
+ checkCommaSeparated(xml, KEY_VALUES, "a", "b", "c");
+ }
+
+ /**
+ * Tests that a list item can be added without affecting the format.
+ */
+ @Test
+ public void testAddListItem() throws ConfigurationException
+ {
+ config.addProperty(KEY_VALUES, "d");
+ config.addProperty(KEY_SPLIT, "3");
+ String xml = saveToString();
+
+ checkSplit(xml, ELEM_SPLIT, "1", "2", "3");
+ checkCommaSeparated(xml, KEY_VALUES, "a", "b", "c", "d");
+ }
+
+ /**
+ * Tests that a list item can be removed without affecting the format.
+ */
+ @Test
+ public void testRemoveListItem() throws ConfigurationException
+ {
+ config.clearProperty(KEY_VALUES + "(2)");
+ config.clearProperty(KEY_SPLIT + "(1)");
+ String xml = saveToString();
+
+ checkSplit(xml, ELEM_SPLIT, "1");
+ checkCommaSeparated(xml, KEY_VALUES, "a", "b");
+ }
+
+ /**
+ * Tests whether a list consisting of multiple elements where some elements
+ * define multiple values is handled correctly.
+ */
+ @Test
+ public void testMixedList() throws ConfigurationException
+ {
+ List<String> expected = Arrays.asList("foo", "blah", "bar", "baz");
+ assertEquals("Wrong list value (1)", expected,
+ config.getList("mixed.values"));
+ String xml = saveToString();
+
+ XMLConfiguration c2 = readFromString(xml);
+ assertEquals("Wrong list value (2)", expected,
+ c2.getList("mixed.values"));
+ }
+
+ /**
+ * Tries to save the configuration with a different list delimiter handler
+ * which does not support escaping of lists. This should fail with a
+ * meaningful exception message.
+ */
+ @Test(expected = ConfigurationRuntimeException.class)
+ public void testIncompatibleListDelimiterOnSaving()
+ throws ConfigurationException
+ {
+ config.setListDelimiterHandler(DisabledListDelimiterHandler.INSTANCE);
+ saveToString();
+ }
+}
