Modified: commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration/tree/xpath/ConfigurationNodePointerFactory.java URL: http://svn.apache.org/viewvc/commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration/tree/xpath/ConfigurationNodePointerFactory.java?rev=1588831&r1=1588830&r2=1588831&view=diff ============================================================================== --- commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration/tree/xpath/ConfigurationNodePointerFactory.java (original) +++ commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration/tree/xpath/ConfigurationNodePointerFactory.java Sun Apr 20 19:32:08 2014 @@ -18,19 +18,29 @@ package org.apache.commons.configuration import java.util.Locale; -import org.apache.commons.configuration.tree.ConfigurationNode; +import org.apache.commons.configuration.tree.NodeHandler; import org.apache.commons.jxpath.ri.QName; import org.apache.commons.jxpath.ri.model.NodePointer; import org.apache.commons.jxpath.ri.model.NodePointerFactory; /** - * Implementation of the {@code NodePointerFactory} interface for - * configuration nodes. + * <p> + * Implementation of the {@code NodePointerFactory} interface for configuration + * nodes. + * </p> + * <p> + * This class is able to create {@code NodePointer}s for the nodes of + * hierarchical configurations. Because there is no common base class for + * configuration nodes (any specific configuration implementation can use its + * own node class) a trick is needed for activating this factory for a concrete + * JXPath query: The {@code wrapNode()} method has to be called with the node + * object and its corresponding {@code NodeHandler}. This creates a wrapper + * object containing all information required by the factory for processing a + * query. Then this wrapper object has to be passed to the query methods of the + * JXPath context. + * </p> * * @since 1.3 - * @author <a - * href="http://commons.apache.org/configuration/team-list.html";>Commons - * Configuration team</a> * @version $Id$ */ public class ConfigurationNodePointerFactory implements NodePointerFactory @@ -51,7 +61,8 @@ public class ConfigurationNodePointerFac /** * Creates a node pointer for the specified bean. If the bean is a - * configuration node, a corresponding pointer is returned. + * configuration node (indicated by a wrapper object), a corresponding + * pointer is returned. * * @param name the name of the node * @param bean the bean @@ -59,12 +70,17 @@ public class ConfigurationNodePointerFac * @return a pointer for a configuration node if the bean is such a node */ @Override + @SuppressWarnings("unchecked") + /* Type casts are safe here; because of the way the NodeWrapper was + constructed the node handler must be compatible with the node. + */ public NodePointer createNodePointer(QName name, Object bean, Locale locale) { - if (bean instanceof ConfigurationNode) + if (bean instanceof NodeWrapper) { - return new ConfigurationNodePointer((ConfigurationNode) bean, - locale); + NodeWrapper<?> wrapper = (NodeWrapper<?>) bean; + return new ConfigurationNodePointer(wrapper.getNode(), + locale, wrapper.getNodeHandler()); } return null; } @@ -79,14 +95,81 @@ public class ConfigurationNodePointerFac * @return a pointer for a configuration node if the bean is such a node */ @Override + @SuppressWarnings("unchecked") + /* Type casts are safe here, see above. Also, the hierarchy of node + pointers is consistent, so a parent is compatible to a child. + */ public NodePointer createNodePointer(NodePointer parent, QName name, Object bean) { - if (bean instanceof ConfigurationNode) + if (bean instanceof NodeWrapper) { - return new ConfigurationNodePointer(parent, - (ConfigurationNode) bean); + NodeWrapper<?> wrapper = (NodeWrapper<?>) bean; + return new ConfigurationNodePointer((ConfigurationNodePointer) parent, + wrapper.getNode(), wrapper.getNodeHandler()); } return null; } + + /** + * Creates a node wrapper for the specified node and its handler. This + * wrapper has to be passed to the JXPath context instead of the original + * node. + * + * @param <T> the type of the node + * @param node the node + * @param handler the corresponding node handler + * @return a wrapper for this node + */ + public static <T> Object wrapNode(T node, NodeHandler<T> handler) + { + return new NodeWrapper<T>(node, handler); + } + + /** + * An internally used wrapper class that holds all information for + * processing a query for a specific node. + * + * @param <T> the type of the nodes this class deals with + */ + static class NodeWrapper<T> + { + /** Stores the node. */ + private final T node; + + /** Stores the corresponding node handler. */ + private final NodeHandler<T> nodeHandler; + + /** + * Creates a new instance of {@code NodeWrapper} and initializes it. + * + * @param nd the node + * @param handler the node handler + */ + public NodeWrapper(T nd, NodeHandler<T> handler) + { + node = nd; + nodeHandler = handler; + } + + /** + * Returns the wrapped node. + * + * @return the node + */ + public T getNode() + { + return node; + } + + /** + * Returns the node handler for the wrapped node. + * + * @return the node handler + */ + public NodeHandler<T> getNodeHandler() + { + return nodeHandler; + } + } }
Modified: commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration/tree/xpath/XPathExpressionEngine.java URL: http://svn.apache.org/viewvc/commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration/tree/xpath/XPathExpressionEngine.java?rev=1588831&r1=1588830&r2=1588831&view=diff ============================================================================== --- commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration/tree/xpath/XPathExpressionEngine.java (original) +++ commons/proper/configuration/trunk/src/main/java/org/apache/commons/configuration/tree/xpath/XPathExpressionEngine.java Sun Apr 20 19:32:08 2014 @@ -16,21 +16,24 @@ */ package org.apache.commons.configuration.tree.xpath; +import java.util.ArrayList; import java.util.Collections; +import java.util.LinkedList; import java.util.List; import java.util.StringTokenizer; -import org.apache.commons.configuration.tree.ConfigurationNode; import org.apache.commons.configuration.tree.ExpressionEngine; import org.apache.commons.configuration.tree.NodeAddData; +import org.apache.commons.configuration.tree.NodeHandler; +import org.apache.commons.configuration.tree.QueryResult; import org.apache.commons.jxpath.JXPathContext; import org.apache.commons.jxpath.ri.JXPathContextReferenceImpl; import org.apache.commons.lang3.StringUtils; /** * <p> - * A specialized implementation of the {@code ExpressionEngine} interface - * that is able to evaluate XPATH expressions. + * A specialized implementation of the {@code ExpressionEngine} interface that + * is able to evaluate XPATH expressions. * </p> * <p> * This class makes use of <a href="http://commons.apache.org/jxpath/";> Commons @@ -41,9 +44,9 @@ import org.apache.commons.lang3.StringUt * <p> * For selecting properties arbitrary XPATH expressions can be used, which * select single or multiple configuration nodes. The associated - * {@code Configuration} instance will directly pass the specified property - * keys into this engine. If a key is not syntactically correct, an exception - * will be thrown. + * {@code Configuration} instance will directly pass the specified property keys + * into this engine. If a key is not syntactically correct, an exception will be + * thrown. * </p> * <p> * For adding new properties, this expression engine uses a specific syntax: the @@ -68,8 +71,8 @@ import org.apache.commons.lang3.StringUt * * </p> * <p> - * This will add a new {@code type} node as a child of the first - * {@code table} element. + * This will add a new {@code type} node as a child of the first {@code table} + * element. * </p> * <p> * @@ -92,8 +95,7 @@ import org.apache.commons.lang3.StringUt * <p> * This example shows how a complex path can be added. Parent node is the * {@code tables} element. Here a new branch consisting of the nodes - * {@code table}, {@code fields}, {@code field}, and - * {@code name} will be added. + * {@code table}, {@code fields}, {@code field}, and {@code name} will be added. * </p> * <p> * @@ -108,15 +110,15 @@ import org.apache.commons.lang3.StringUt * </p> * <p> * <strong>Note:</strong> This extended syntax for adding properties only works - * with the {@code addProperty()} method. {@code setProperty()} does - * not support creating new nodes this way. + * with the {@code addProperty()} method. {@code setProperty()} does not support + * creating new nodes this way. * </p> * <p> * From version 1.7 on, it is possible to use regular keys in calls to - * {@code addProperty()} (i.e. keys that do not have to contain a - * whitespace as delimiter). In this case the key is evaluated, and the biggest - * part pointing to an existing node is determined. The remaining part is then - * added as new path. As an example consider the key + * {@code addProperty()} (i.e. keys that do not have to contain a whitespace as + * delimiter). In this case the key is evaluated, and the biggest part pointing + * to an existing node is determined. The remaining part is then added as new + * path. As an example consider the key * * <pre> * "tables/table[last()]/fields/field/name" @@ -124,22 +126,19 @@ import org.apache.commons.lang3.StringUt * * If the key does not point to an existing node, the engine will check the * paths {@code "tables/table[last()]/fields/field"}, - * {@code "tables/table[last()]/fields"}, - * {@code "tables/table[last()]"}, and so on, until a key is - * found which points to a node. Let's assume that the last key listed above can - * be resolved in this way. Then from this key the following key is derived: - * {@code "tables/table[last()] fields/field/name"} by appending - * the remaining part after a whitespace. This key can now be processed using - * the original algorithm. Keys of this form can also be used with the - * {@code setProperty()} method. However, it is still recommended to use - * the old format because it makes explicit at which position new nodes should - * be added. For keys without a whitespace delimiter there may be ambiguities. + * {@code "tables/table[last()]/fields"}, {@code "tables/table[last()]"}, and so + * on, until a key is found which points to a node. Let's assume that the last + * key listed above can be resolved in this way. Then from this key the + * following key is derived: {@code "tables/table[last()] fields/field/name"} by + * appending the remaining part after a whitespace. This key can now be + * processed using the original algorithm. Keys of this form can also be used + * with the {@code setProperty()} method. However, it is still recommended to + * use the old format because it makes explicit at which position new nodes + * should be added. For keys without a whitespace delimiter there may be + * ambiguities. * </p> * * @since 1.3 - * @author <a - * href="http://commons.apache.org/configuration/team-list.html";>Commons - * Configuration team</a> * @version $Id$ */ public class XPathExpressionEngine implements ExpressionEngine @@ -160,6 +159,38 @@ public class XPathExpressionEngine imple */ private static final String SPACE = " "; + /** Constant for a default size of a key buffer. */ + private static final int BUF_SIZE = 128; + + /** Constant for the start of an index expression. */ + private static final char START_INDEX = '['; + + /** Constant for the end of an index expression. */ + private static final char END_INDEX = ']'; + + /** The internally used context factory. */ + private final XPathContextFactory contextFactory; + + /** + * Creates a new instance of {@code XPathExpressionEngine} with default + * settings. + */ + public XPathExpressionEngine() + { + this(new XPathContextFactory()); + } + + /** + * Creates a new instance of {@code XPathExpressionEngine} and sets the + * context factory. This constructor is mainly used for testing purposes. + * + * @param factory the {@code XPathContextFactory} + */ + XPathExpressionEngine(XPathContextFactory factory) + { + contextFactory = factory; + } + /** * Executes a query. The passed in property key is directly passed to a * JXPath context. @@ -168,50 +199,43 @@ public class XPathExpressionEngine imple * @param key the query to be executed * @return a list with the nodes that are selected by the query */ - @Override - public List<ConfigurationNode> query(ConfigurationNode root, String key) + public <T> List<QueryResult<T>> query(T root, String key, + NodeHandler<T> handler) { if (StringUtils.isEmpty(key)) { - return Collections.singletonList(root); + QueryResult<T> result = createResult(root); + return Collections.singletonList(result); } else { - JXPathContext context = createContext(root, key); - // This is safe because our node pointer implementations will return - // a list of configuration nodes. - @SuppressWarnings("unchecked") - List<ConfigurationNode> result = context.selectNodes(key); - if (result == null) + JXPathContext context = createContext(root, handler); + List<?> results = context.selectNodes(key); + if (results == null) { - result = Collections.emptyList(); + results = Collections.emptyList(); } - return result; + return convertResults(results); } } /** - * Returns a (canonical) key for the given node based on the parent's key. - * This implementation will create an XPATH expression that selects the - * given node (under the assumption that the passed in parent key is valid). - * As the {@code nodeKey()} implementation of - * {@link org.apache.commons.configuration.tree.DefaultExpressionEngine DefaultExpressionEngine} - * this method will not return indices for nodes. So all child nodes of a - * given parent with the same name will have the same key. - * - * @param node the node for which a key is to be constructed - * @param parentKey the key of the parent node - * @return the key for the given node + * {@inheritDoc} This implementation creates an XPATH expression that + * selects the given node (under the assumption that the passed in parent + * key is valid). As the {@code nodeKey()} implementation of + * {@link org.apache.commons.configuration.tree.DefaultExpressionEngine + * DefaultExpressionEngine} this method does not return indices for nodes. + * So all child nodes of a given parent with the same name have the same + * key. */ - @Override - public String nodeKey(ConfigurationNode node, String parentKey) + public <T> String nodeKey(T node, String parentKey, NodeHandler<T> handler) { if (parentKey == null) { // name of the root node return StringUtils.EMPTY; } - else if (node.getName() == null) + else if (handler.nodeName(node) == null) { // paranoia check for undefined node names return parentKey; @@ -219,33 +243,66 @@ public class XPathExpressionEngine imple else { - StringBuilder buf = new StringBuilder(parentKey.length() - + node.getName().length() + PATH_DELIMITER.length()); + StringBuilder buf = + new StringBuilder(parentKey.length() + + handler.nodeName(node).length() + + PATH_DELIMITER.length()); if (parentKey.length() > 0) { buf.append(parentKey); buf.append(PATH_DELIMITER); } - if (node.isAttribute()) - { - buf.append(ATTR_DELIMITER); - } - buf.append(node.getName()); + buf.append(handler.nodeName(node)); return buf.toString(); } } + public String attributeKey(String parentKey, String attributeName) + { + StringBuilder buf = + new StringBuilder(StringUtils.length(parentKey) + + StringUtils.length(attributeName) + + PATH_DELIMITER.length() + ATTR_DELIMITER.length()); + if (StringUtils.isNotEmpty(parentKey)) + { + buf.append(parentKey).append(PATH_DELIMITER); + } + buf.append(ATTR_DELIMITER).append(attributeName); + return buf.toString(); + } + /** - * Prepares an add operation for a configuration property. The expected - * format of the passed in key is explained in the class comment. - * - * @param root the configuration's root node - * @param key the key describing the target of the add operation and the - * path of the new node - * @return a data object to be evaluated by the calling configuration object + * {@inheritDoc} This implementation works similar to {@code nodeKey()}, but + * always adds an index expression to the resulting key. + */ + public <T> String canonicalKey(T node, String parentKey, + NodeHandler<T> handler) + { + T parent = handler.getParent(node); + if (parent == null) + { + // this is the root node + return StringUtils.defaultString(parentKey); + } + + StringBuilder buf = new StringBuilder(BUF_SIZE); + if (StringUtils.isNotEmpty(parentKey)) + { + buf.append(parentKey).append(PATH_DELIMITER); + } + buf.append(handler.nodeName(node)); + buf.append(START_INDEX); + buf.append(determineIndex(parent, node, handler)); + buf.append(END_INDEX); + return buf.toString(); + } + + /** + * {@inheritDoc} The expected format of the passed in key is explained in + * the class comment. */ - @Override - public NodeAddData prepareAdd(ConfigurationNode root, String key) + public <T> NodeAddData<T> prepareAdd(T root, String key, + NodeHandler<T> handler) { if (key == null) { @@ -257,55 +314,61 @@ public class XPathExpressionEngine imple int index = findKeySeparator(addKey); if (index < 0) { - addKey = generateKeyForAdd(root, addKey); + addKey = generateKeyForAdd(root, addKey, handler); index = findKeySeparator(addKey); } + else if (index >= addKey.length() - 1) + { + invalidPath(addKey, " new node path must not be empty."); + } - List<ConfigurationNode> nodes = query(root, addKey.substring(0, index).trim()); + List<QueryResult<T>> nodes = + query(root, addKey.substring(0, index).trim(), handler); if (nodes.size() != 1) { - throw new IllegalArgumentException( - "prepareAdd: key must select exactly one target node!"); + throw new IllegalArgumentException("prepareAdd: key '" + key + + "' must select exactly one target node!"); } - NodeAddData data = new NodeAddData(); - data.setParent(nodes.get(0)); - initNodeAddData(data, addKey.substring(index).trim()); - return data; + return createNodeAddData(addKey.substring(index).trim(), nodes.get(0)); } /** - * Creates the {@code JXPathContext} used for executing a query. This - * method will create a new context and ensure that it is correctly - * initialized. + * Creates the {@code JXPathContext} to be used for executing a query. This + * method delegates to the context factory. * * @param root the configuration root node - * @param key the key to be queried + * @param handler the node handler * @return the new context */ - protected JXPathContext createContext(ConfigurationNode root, String key) + private <T> JXPathContext createContext(T root, NodeHandler<T> handler) { - JXPathContext context = JXPathContext.newContext(root); - context.setLenient(true); - return context; + return getContextFactory().createContext(root, handler); } /** - * Initializes most properties of a {@code NodeAddData} object. This - * method is called by {@code prepareAdd()} after the parent node has - * been found. Its task is to interpret the passed in path of the new node. + * Creates a {@code NodeAddData} object as a result of a + * {@code prepareAdd()} operation. This method interprets the passed in path + * of the new node. * - * @param data the data object to initialize * @param path the path of the new node + * @param parentNodeResult the parent node + * @param <T> the type of the nodes involved */ - protected void initNodeAddData(NodeAddData data, String path) + <T> NodeAddData<T> createNodeAddData(String path, + QueryResult<T> parentNodeResult) { + if (parentNodeResult.isAttributeResult()) + { + invalidPath(path, " cannot add properties to an attribute."); + } + List<String> pathNodes = new LinkedList<String>(); String lastComponent = null; boolean attr = false; boolean first = true; - StringTokenizer tok = new StringTokenizer(path, NODE_PATH_DELIMITERS, - true); + StringTokenizer tok = + new StringTokenizer(path, NODE_PATH_DELIMITERS, true); while (tok.hasMoreTokens()) { String token = tok.nextToken(); @@ -314,14 +377,14 @@ public class XPathExpressionEngine imple if (attr) { invalidPath(path, " contains an attribute" - + " delimiter at an unallowed position."); + + " delimiter at a disallowed position."); } if (lastComponent == null) { invalidPath(path, - " contains a '/' at an unallowed position."); + " contains a '/' at a disallowed position."); } - data.addPathNode(lastComponent); + pathNodes.add(lastComponent); lastComponent = null; } @@ -335,11 +398,11 @@ public class XPathExpressionEngine imple if (lastComponent == null && !first) { invalidPath(path, - " contains an attribute delimiter at an unallowed position."); + " contains an attribute delimiter at a disallowed position."); } if (lastComponent != null) { - data.addPathNode(lastComponent); + pathNodes.add(lastComponent); } attr = true; lastComponent = null; @@ -356,29 +419,42 @@ public class XPathExpressionEngine imple { invalidPath(path, "contains no components."); } - data.setNewNodeName(lastComponent); - data.setAttribute(attr); + + return new NodeAddData<T>(parentNodeResult.getNode(), lastComponent, + attr, pathNodes); + } + + /** + * Returns the {@code XPathContextFactory} used by this instance. + * + * @return the {@code XPathContextFactory} + */ + XPathContextFactory getContextFactory() + { + return contextFactory; } /** * Tries to generate a key for adding a property. This method is called if a * key was used for adding properties which does not contain a space * character. It splits the key at its single components and searches for - * the last existing component. Then a key compatible for adding properties - * is generated. + * the last existing component. Then a key compatible key for adding + * properties is generated. * * @param root the root node of the configuration * @param key the key in question + * @param handler the node handler * @return the key to be used for adding the property */ - private String generateKeyForAdd(ConfigurationNode root, String key) + private <T> String generateKeyForAdd(T root, String key, + NodeHandler<T> handler) { int pos = key.lastIndexOf(PATH_DELIMITER, key.length()); while (pos >= 0) { String keyExisting = key.substring(0, pos); - if (!query(root, keyExisting).isEmpty()) + if (!query(root, keyExisting, handler).isEmpty()) { StringBuilder buf = new StringBuilder(key.length() + 1); buf.append(keyExisting).append(SPACE); @@ -392,12 +468,29 @@ public class XPathExpressionEngine imple } /** + * Determines the index of the given child node in the node list of its + * parent. + * + * @param parent the parent node + * @param child the child node + * @param handler the node handler + * @param <T> the type of the nodes involved + * @return the index of this child node + */ + private static <T> int determineIndex(T parent, T child, + NodeHandler<T> handler) + { + return handler.getChildren(parent, handler.nodeName(child)).indexOf( + child) + 1; + } + + /** * Helper method for throwing an exception about an invalid path. * * @param path the invalid path * @param msg the exception message */ - private void invalidPath(String path, String msg) + private static void invalidPath(String path, String msg) { throw new IllegalArgumentException("Invalid node path: \"" + path + "\" " + msg); @@ -420,6 +513,54 @@ public class XPathExpressionEngine imple return index; } + /** + * Converts the objects returned as query result from the JXPathContext to + * query result objects. + * + * @param results the list with results from the context + * @param <T> the type of results to be produced + * @return the result list + */ + private static <T> List<QueryResult<T>> convertResults(List<?> results) + { + List<QueryResult<T>> queryResults = + new ArrayList<QueryResult<T>>(results.size()); + for (Object res : results) + { + QueryResult<T> queryResult = createResult(res); + queryResults.add(queryResult); + } + return queryResults; + } + + /** + * Creates a {@code QueryResult} object from the given result object of a + * query. Because of the node pointers involved result objects can only be + * of two types: + * <ul> + * <li>nodes of type T</li> + * <li>attribute results already wrapped in {@code QueryResult} objects</li> + * </ul> + * This method performs a corresponding cast. Warnings can be suppressed + * because of the implementation of the query functionality. + * + * @param resObj the query result object + * @param <T> the type of the result to be produced + * @return the {@code QueryResult} + */ + @SuppressWarnings("unchecked") + private static <T> QueryResult<T> createResult(Object resObj) + { + if (resObj instanceof QueryResult) + { + return (QueryResult<T>) resObj; + } + else + { + return QueryResult.createNodeResult((T) resObj); + } + } + // static initializer: registers the configuration node pointer factory static { Modified: commons/proper/configuration/trunk/src/main/javacc/PropertyListParser.jj URL: http://svn.apache.org/viewvc/commons/proper/configuration/trunk/src/main/javacc/PropertyListParser.jj?rev=1588831&r1=1588830&r2=1588831&view=diff ============================================================================== --- commons/proper/configuration/trunk/src/main/javacc/PropertyListParser.jj (original) +++ commons/proper/configuration/trunk/src/main/javacc/PropertyListParser.jj Sun Apr 20 19:32:08 2014 @@ -29,8 +29,7 @@ import java.util.List; import java.util.ArrayList; import org.apache.commons.configuration.HierarchicalConfiguration; -import org.apache.commons.configuration.tree.ConfigurationNode; -import org.apache.commons.configuration.tree.DefaultConfigurationNode; +import org.apache.commons.configuration.tree.ImmutableNode; import org.apache.commons.codec.binary.Hex; @@ -38,7 +37,7 @@ import org.apache.commons.codec.binary.H * JavaCC based parser for the PropertyList format. * * @author Emmanuel Bourg - * @version $Revision$, $Date$ + * @version $Id$ */ class PropertyListParser { @@ -119,12 +118,12 @@ SKIP : { " " | "\t" | "\n" | "\r" } // Handle comments MORE : { "/*": IN_COMMENT } < IN_COMMENT > MORE : { < ~[] > } -< IN_COMMENT > SKIP : { "*/": DEFAULT } +< IN_COMMENT > SKIP : { "*/": DEFAULT } MORE : { "//": IN_SINGLE_LINE_COMMENT } < IN_SINGLE_LINE_COMMENT > SPECIAL_TOKEN : { < SINGLE_LINE_COMMENT: "\n"|"\r"|"\r\n" > : DEFAULT } -< IN_SINGLE_LINE_COMMENT > MORE : { < ~[] > } +< IN_SINGLE_LINE_COMMENT > MORE : { < ~[] > } TOKEN : { <ARRAY_BEGIN : "(" > } TOKEN : { <ARRAY_END : ")" > } @@ -166,9 +165,8 @@ PropertyListConfiguration parse() : PropertyListConfiguration Dictionary() : { - PropertyListConfiguration configuration = new PropertyListConfiguration(); - List<ConfigurationNode> children = new ArrayList<ConfigurationNode>(); - ConfigurationNode child = null; + ImmutableNode.Builder builder = new ImmutableNode.Builder(); + ImmutableNode child = null; } { <DICT_BEGIN> @@ -178,43 +176,41 @@ PropertyListConfiguration Dictionary() : if (child.getValue() instanceof HierarchicalConfiguration) { // prune & graft the nested configuration to the parent configuration - HierarchicalConfiguration conf = (HierarchicalConfiguration) child.getValue(); - ConfigurationNode root = conf.getRootNode(); - root.setName(child.getName()); - children.add(root); + @SuppressWarnings("unchecked") // we created this configuration + HierarchicalConfiguration<ImmutableNode> conf = + (HierarchicalConfiguration<ImmutableNode>) child.getValue(); + ImmutableNode root = conf.getNodeModel().getNodeHandler().getRootNode(); + ImmutableNode.Builder childBuilder = new ImmutableNode.Builder(); + childBuilder.name(child.getNodeName()).value(root.getValue()) + .addChildren(root.getChildren()); + builder.addChild(childBuilder.create()); } else { - children.add(child); + builder.addChild(child); } } )* <DICT_END> { - for (int i = 0; i < children.size(); i++) - { - child = children.get(i); - configuration.getRootNode().addChild(child); - } - - return configuration; + return new PropertyListConfiguration(builder.create()); } } -ConfigurationNode Property() : +ImmutableNode Property() : { String key = null; Object value = null; - ConfigurationNode node = new DefaultConfigurationNode(); + ImmutableNode.Builder node = new ImmutableNode.Builder(); } { key = String() - { node.setName(key); } + { node.name(key); } <EQUAL> value = Element() - { node.setValue(value); } + { node.value(value); } (<DICT_SEPARATOR>)? - { return node; } + { return node.create(); } } Object Element() : @@ -223,7 +219,7 @@ Object Element() : } { LOOKAHEAD(2) - + value = Array() { return value; } | Propchange: commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration/ ------------------------------------------------------------------------------ Merged /commons/proper/configuration/branches/immutableNodes/src/test/java/org/apache/commons/configuration:r1561338-1588830 Modified: commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration/TestBaseHierarchicalConfigurationSynchronization.java URL: http://svn.apache.org/viewvc/commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration/TestBaseHierarchicalConfigurationSynchronization.java?rev=1588831&r1=1588830&r2=1588831&view=diff ============================================================================== --- commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration/TestBaseHierarchicalConfigurationSynchronization.java (original) +++ commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration/TestBaseHierarchicalConfigurationSynchronization.java Sun Apr 20 19:32:08 2014 @@ -18,9 +18,7 @@ package org.apache.commons.configuration import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNotSame; -import static org.junit.Assert.assertNull; import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -29,7 +27,6 @@ import java.io.File; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; -import java.util.LinkedList; import java.util.List; import java.util.concurrent.CountDownLatch; @@ -38,8 +35,9 @@ import org.apache.commons.configuration. import org.apache.commons.configuration.builder.fluent.Parameters; import org.apache.commons.configuration.ex.ConfigurationException; import org.apache.commons.configuration.io.FileHandler; -import org.apache.commons.configuration.tree.ConfigurationNode; -import org.apache.commons.configuration.tree.DefaultConfigurationNode; +import org.apache.commons.configuration.tree.ImmutableNode; +import org.apache.commons.configuration.tree.InMemoryNodeModel; +import org.apache.commons.configuration.tree.NodeStructureHelper; import org.junit.Before; import org.junit.Test; @@ -112,8 +110,7 @@ public class TestBaseHierarchicalConfigu @Test public void testAddNodesSynchronized() { - DefaultConfigurationNode node = - new DefaultConfigurationNode("newNode", "true"); + ImmutableNode node = NodeStructureHelper.createNode("newNode", "true"); config.addNodes("test.addNodes", Collections.singleton(node)); sync.verify(Methods.BEGIN_WRITE, Methods.END_WRITE); } @@ -134,7 +131,7 @@ public class TestBaseHierarchicalConfigu @Test public void testSetRootNodeSynchronized() { - config.setRootNode(new DefaultConfigurationNode("testRoot")); + config.setRootNode(NodeStructureHelper.createNode("testRoot", null)); sync.verify(Methods.BEGIN_WRITE, Methods.END_WRITE); } @@ -157,10 +154,10 @@ public class TestBaseHierarchicalConfigu @Test public void testConfigurationAtSynchronized() { - SubnodeConfiguration sub = config.configurationAt("element2"); + HierarchicalConfiguration<ImmutableNode> sub = config.configurationAt("element2"); assertEquals("Wrong property", "I'm complex!", sub.getString("subelement.subsubelement")); - sync.verify(Methods.BEGIN_WRITE, Methods.END_WRITE, Methods.BEGIN_READ, + sync.verify(Methods.BEGIN_READ, Methods.END_READ, Methods.BEGIN_READ, Methods.END_READ); } @@ -171,9 +168,10 @@ public class TestBaseHierarchicalConfigu @Test public void testConfigurationsAtSynchronized() { - List<SubnodeConfiguration> subs = config.configurationsAt("list.item"); + List<HierarchicalConfiguration<ImmutableNode>> subs = + config.configurationsAt("list.item"); assertFalse("No subnode configurations", subs.isEmpty()); - sync.verify(Methods.BEGIN_WRITE, Methods.END_WRITE); + sync.verify(Methods.BEGIN_READ, Methods.END_READ); } /** @@ -182,38 +180,24 @@ public class TestBaseHierarchicalConfigu @Test public void testChildConfigurationsAtSynchronized() { - List<SubnodeConfiguration> subs = config.childConfigurationsAt("clear"); + List<HierarchicalConfiguration<ImmutableNode>> subs = + config.childConfigurationsAt("clear"); assertFalse("No subnode configurations", subs.isEmpty()); - sync.verify(Methods.BEGIN_WRITE, Methods.END_WRITE); - } - - /** - * Tests whether synchronization is performed when setting the key of a - * SubnodeConfiguration. - */ - @Test - public void testSetSubnodeKeySynchronized() - { - SubnodeConfiguration sub = config.configurationAt("element2"); - assertNull("Got a subnode key", sub.getSubnodeKey()); - sub.setSubnodeKey("element2"); - // 1 x configurationAt(), 1 x getSubnodeKey(), 1 x setSubnodeKey() - sync.verify(Methods.BEGIN_WRITE, Methods.END_WRITE, Methods.BEGIN_READ, - Methods.END_READ, Methods.BEGIN_WRITE, Methods.END_WRITE); + sync.verify(Methods.BEGIN_READ, Methods.END_READ); } /** - * Tests whether synchronization is performed when querying the key of a - * SubnodeConfiguration. - */ - @Test - public void testGetSubnodeKeySynchronized() - { - SubnodeConfiguration sub = config.configurationAt("element2", true); - assertEquals("Wrong subnode key", "element2", sub.getSubnodeKey()); - // 1 x configurationAt(), 1 x getSubnodeKey() - sync.verify(Methods.BEGIN_WRITE, Methods.END_WRITE, - Methods.BEGIN_READ, Methods.END_READ); + * Tests whether the specified configuration is detached. + * + * @param c the configuration to test + * @return a flag whether the root node of this configuration is detached + */ + private static boolean isDetached(HierarchicalConfiguration<ImmutableNode> c) + { + assertTrue("Not a sub configuration", c instanceof SubnodeConfiguration); + return ((InMemoryNodeModel) c.getNodeModel()) + .isTrackedNodeDetached(((SubnodeConfiguration) c) + .getRootSelector()); } /** @@ -224,11 +208,13 @@ public class TestBaseHierarchicalConfigu public void testSubnodeUpdate() { config.addProperty("element2.test", Boolean.TRUE); - SubnodeConfiguration sub = config.configurationAt("element2", true); - SubnodeConfiguration subsub = sub.configurationAt("subelement", true); + HierarchicalConfiguration<ImmutableNode> sub = + config.configurationAt("element2", true); + HierarchicalConfiguration<ImmutableNode> subsub = + sub.configurationAt("subelement", true); config.clearTree("element2.subelement"); - assertNotNull("Sub1 detached", sub.getSubnodeKey()); - assertNull("Sub2 still attached", subsub.getSubnodeKey()); + assertFalse("Sub1 detached", isDetached(sub)); + assertTrue("Sub2 still attached", isDetached(subsub)); } /** @@ -238,17 +224,15 @@ public class TestBaseHierarchicalConfigu @Test public void testSubnodeUpdateBySubnode() { - SubnodeConfiguration sub = config.configurationAt("element2", true); - SubnodeConfiguration subsub = sub.configurationAt("subelement", true); - SubnodeConfiguration sub2 = + HierarchicalConfiguration<ImmutableNode> sub = + config.configurationAt("element2", true); + HierarchicalConfiguration<ImmutableNode> subsub = + sub.configurationAt("subelement", true); + HierarchicalConfiguration<ImmutableNode> sub2 = config.configurationAt("element2.subelement", true); sub.clearTree("subelement"); - // 3 x configurationAt(), 1 x clearTree() - sync.verify(Methods.BEGIN_WRITE, Methods.END_WRITE, - Methods.BEGIN_WRITE, Methods.END_WRITE, Methods.BEGIN_WRITE, - Methods.END_WRITE, Methods.BEGIN_WRITE, Methods.END_WRITE); - assertNull("Sub2 still attached", sub2.getSubnodeKey()); - assertNull("Subsub still attached", subsub.getSubnodeKey()); + assertTrue("Sub2 still attached", isDetached(sub2)); + assertTrue("Subsub still attached", isDetached(subsub)); } /** @@ -258,64 +242,20 @@ public class TestBaseHierarchicalConfigu @Test public void testCloneCopySubnodeData() { - final Collection<SubnodeConfiguration> validatedConfigs = - new LinkedList<SubnodeConfiguration>(); - - // A special configuration class which creates SubConfigurations that - // record validation operations BaseHierarchicalConfiguration conf2 = - new BaseHierarchicalConfiguration(config) - { - private static final long serialVersionUID = 1L; - - @Override - protected SubnodeConfiguration createSubnodeConfiguration( - ConfigurationNode node, String subnodeKey) - { - return new SubnodeConfiguration(this, node, subnodeKey) - { - private static final long serialVersionUID = 1L; - - @Override - void validateRootNode() - { - super.validateRootNode(); - validatedConfigs.add(this); - } - }; - } - }; + new BaseHierarchicalConfiguration(config); - SubnodeConfiguration sub = + HierarchicalConfiguration<ImmutableNode> sub = conf2.configurationAt("element2.subelement", true); - HierarchicalConfiguration copy = - (HierarchicalConfiguration) conf2.clone(); - SubnodeConfiguration sub2 = + @SuppressWarnings("unchecked") // clone retains the type + HierarchicalConfiguration<ImmutableNode> copy = + (HierarchicalConfiguration<ImmutableNode>) conf2.clone(); + HierarchicalConfiguration<ImmutableNode> sub2 = copy.configurationAt("element2.subelement", true); // This must not cause a validate operation on sub1, but on sub2 copy.clearTree("element2"); - assertNull("Sub2 not detached", sub2.getSubnodeKey()); - assertNotNull("Sub 1 was detached", sub.getSubnodeKey()); - assertEquals("Wrong number of validated configs", 1, - validatedConfigs.size()); - assertSame("Wrong validated config", sub2, validatedConfigs.iterator() - .next()); - } - - /** - * Tests whether a SubnodeConfiguration's clearAndDetachFromParent() method - * is correctly synchronized. - */ - @Test - public void testSubnodeClearAndDetachFromParentSynchronized() - { - SubnodeConfiguration sub = config.configurationAt("element2", true); - sub.clearAndDetachFromParent(); - assertFalse("Node not removed", config.containsKey("element2")); - // configurationAt() + clearTree() + containsKey() - sync.verify(Methods.BEGIN_WRITE, Methods.END_WRITE, - Methods.BEGIN_WRITE, Methods.END_WRITE, Methods.BEGIN_READ, - Methods.END_READ); + assertTrue("Sub2 not detached", isDetached(sub2)); + assertFalse("Sub 1 was detached", isDetached(sub)); } /** @@ -376,7 +316,7 @@ public class TestBaseHierarchicalConfigu private static class SubNodeAccessThread extends Thread { /** The test configuration. */ - private final HierarchicalConfiguration config; + private final HierarchicalConfiguration<ImmutableNode> config; /** The latch for synchronizing thread start. */ private final CountDownLatch latch; @@ -398,7 +338,7 @@ public class TestBaseHierarchicalConfigu * @param keySubConfig the key for the sub configuration * @param keyProperty the key for the property */ - public SubNodeAccessThread(HierarchicalConfiguration c, + public SubNodeAccessThread(HierarchicalConfiguration<ImmutableNode> c, CountDownLatch startLatch, String keySubConfig, String keyProperty) { @@ -414,7 +354,8 @@ public class TestBaseHierarchicalConfigu try { latch.await(); - SubnodeConfiguration subConfig = config.configurationAt(keySub); + HierarchicalConfiguration<ImmutableNode> subConfig = + config.configurationAt(keySub, true); value = subConfig.getString(keyProp); } catch (InterruptedException iex) Modified: commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration/TestCombinedConfiguration.java URL: http://svn.apache.org/viewvc/commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration/TestCombinedConfiguration.java?rev=1588831&r1=1588830&r2=1588831&view=diff ============================================================================== --- commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration/TestCombinedConfiguration.java (original) +++ commons/proper/configuration/trunk/src/test/java/org/apache/commons/configuration/TestCombinedConfiguration.java Sun Apr 20 19:32:08 2014 @@ -45,10 +45,11 @@ import org.apache.commons.configuration. import org.apache.commons.configuration.sync.LockMode; import org.apache.commons.configuration.sync.ReadWriteSynchronizer; import org.apache.commons.configuration.sync.Synchronizer; -import org.apache.commons.configuration.tree.ConfigurationNode; import org.apache.commons.configuration.tree.DefaultExpressionEngine; import org.apache.commons.configuration.tree.DefaultExpressionEngineSymbols; +import org.apache.commons.configuration.tree.ImmutableNode; import org.apache.commons.configuration.tree.NodeCombiner; +import org.apache.commons.configuration.tree.NodeModel; import org.apache.commons.configuration.tree.OverrideCombiner; import org.apache.commons.configuration.tree.UnionCombiner; import org.junit.Before; @@ -78,6 +79,9 @@ public class TestCombinedConfiguration /** Constant for the name of the second child configuration.*/ private static final String CHILD2 = TEST_NAME + "2"; + /** Constant for the key for a sub configuration. */ + private static final String SUB_KEY = "test.sub.config"; + /** Helper object for managing temporary files. */ @Rule public TemporaryFolder folder = new TemporaryFolder(); @@ -387,7 +391,6 @@ public class TestCombinedConfiguration CombinedConfiguration cc2 = (CombinedConfiguration) config.clone(); assertNotNull("No root node", cc2.getRootNode()); - assertNotSame("Root node not copied", config.getRootNode(), cc2.getRootNode()); assertEquals("Wrong number of contained configurations", config .getNumberOfConfigurations(), cc2.getNumberOfConfigurations()); assertSame("Wrong node combiner", config.getNodeCombiner(), cc2 @@ -547,6 +550,47 @@ public class TestCombinedConfiguration } /** + * Tests getSource() if a child configuration is again a combined configuration. + */ + @Test + public void testGetSourceWithCombinedChildConfiguration() + { + setUpSourceTest(); + CombinedConfiguration cc = new CombinedConfiguration(); + cc.addConfiguration(config); + assertEquals("Wrong source", config, cc.getSource(TEST_KEY)); + } + + /** + * Tests whether multiple sources of a key can be retrieved. + */ + @Test + public void testGetSourcesMultiSources() + { + setUpSourceTest(); + final String key = "list.key"; + config.getConfiguration(CHILD1).addProperty(key, "1,2,3"); + config.getConfiguration(CHILD2).addProperty(key, "a,b,c"); + Set<Configuration> sources = config.getSources(key); + assertEquals("Wrong number of sources", 2, sources.size()); + assertTrue("Source 1 not found", + sources.contains(config.getConfiguration(CHILD1))); + assertTrue("Source 2 not found", + sources.contains(config.getConfiguration(CHILD2))); + } + + /** + * Tests getSources() for a non existing key. + */ + @Test + public void testGetSourcesUnknownKey() + { + setUpSourceTest(); + assertTrue("Got sources", config.getSources("non.existing,key") + .isEmpty()); + } + + /** * Tests whether escaped list delimiters are treated correctly. */ @Test @@ -689,7 +733,7 @@ public class TestCombinedConfiguration SynchronizerTestImpl sync = setUpSynchronizerTest(); config.addConfiguration(new BaseHierarchicalConfiguration()); sync.verify(Methods.BEGIN_WRITE, Methods.END_WRITE); - assertNull("Root node not reset", config.getRootNode()); + checkCombinedRootNotConstructed(); } /** @@ -701,7 +745,7 @@ public class TestCombinedConfiguration SynchronizerTestImpl sync = setUpSynchronizerTest(); config.setNodeCombiner(new UnionCombiner()); sync.verify(Methods.BEGIN_WRITE, Methods.END_WRITE); - assertNull("Root node not reset", config.getRootNode()); + checkCombinedRootNotConstructed(); } /** @@ -713,7 +757,7 @@ public class TestCombinedConfiguration SynchronizerTestImpl sync = setUpSynchronizerTest(); assertNotNull("No node combiner", config.getNodeCombiner()); sync.verify(Methods.BEGIN_READ, Methods.END_READ); - assertNull("Root node was constructed", config.getRootNode()); + checkCombinedRootNotConstructed(); } /** @@ -726,7 +770,7 @@ public class TestCombinedConfiguration SynchronizerTestImpl sync = setUpSynchronizerTest(); assertNotNull("No configuration", config.getConfiguration(0)); sync.verify(Methods.BEGIN_READ, Methods.END_READ); - assertNull("Root node was constructed", config.getRootNode()); + checkCombinedRootNotConstructed(); } /** @@ -739,7 +783,7 @@ public class TestCombinedConfiguration SynchronizerTestImpl sync = setUpSynchronizerTest(); assertNotNull("No configuration", config.getConfiguration(CHILD1)); sync.verify(Methods.BEGIN_READ, Methods.END_READ); - assertNull("Root node was constructed", config.getRootNode()); + checkCombinedRootNotConstructed(); } /** @@ -752,7 +796,7 @@ public class TestCombinedConfiguration SynchronizerTestImpl sync = setUpSynchronizerTest(); assertFalse("No child names", config.getConfigurationNames().isEmpty()); sync.verify(Methods.BEGIN_READ, Methods.END_READ); - assertNull("Root node was constructed", config.getRootNode()); + checkCombinedRootNotConstructed(); } /** @@ -766,7 +810,17 @@ public class TestCombinedConfiguration assertFalse("No child names", config.getConfigurationNameList() .isEmpty()); sync.verify(Methods.BEGIN_READ, Methods.END_READ); - assertNull("Root node was constructed", config.getRootNode()); + checkCombinedRootNotConstructed(); + } + + /** + * Helper method for testing that the combined root node has not yet been + * constructed. + */ + private void checkCombinedRootNotConstructed() + { + assertTrue("Root node was constructed", config.getRootNode() + .getChildren().isEmpty()); } /** @@ -779,7 +833,7 @@ public class TestCombinedConfiguration assertFalse("No child configurations", config.getConfigurations() .isEmpty()); sync.verify(Methods.BEGIN_READ, Methods.END_READ); - assertNull("Root node was constructed", config.getRootNode()); + checkCombinedRootNotConstructed(); } /** @@ -793,7 +847,7 @@ public class TestCombinedConfiguration assertNull("Got a conversion engine", config.getConversionExpressionEngine()); sync.verify(Methods.BEGIN_READ, Methods.END_READ); - assertNull("Root node was constructed", config.getRootNode()); + checkCombinedRootNotConstructed(); } /** @@ -807,7 +861,7 @@ public class TestCombinedConfiguration config.setConversionExpressionEngine(new DefaultExpressionEngine( DefaultExpressionEngineSymbols.DEFAULT_SYMBOLS)); sync.verify(Methods.BEGIN_WRITE, Methods.END_WRITE); - assertNull("Root node was constructed", config.getRootNode()); + checkCombinedRootNotConstructed(); } /** @@ -844,7 +898,7 @@ public class TestCombinedConfiguration assertEquals("Wrong number of configurations", 2, config.getNumberOfConfigurations()); sync.verify(Methods.BEGIN_READ, Methods.END_READ); - assertNull("Root node was constructed", config.getRootNode()); + checkCombinedRootNotConstructed(); } /** @@ -880,10 +934,9 @@ public class TestCombinedConfiguration private static final long serialVersionUID = 1L; @Override - public ConfigurationNode getRootNode() - { + public NodeModel<ImmutableNode> getModel() { throw testEx; - }; + } }; config.addConfiguration(childEx); try @@ -961,12 +1014,76 @@ public class TestCombinedConfiguration } /** + * Prepares the test configuration for a test for sub configurations. Some + * child configurations are added. + * + * @return the sub configuration at the test sub key + */ + private AbstractConfiguration setUpSubConfigTest() + { + AbstractConfiguration srcConfig = setUpTestConfiguration(); + config.addConfiguration(srcConfig, "source", SUB_KEY); + config.addConfiguration(setUpTestConfiguration()); + config.addConfiguration(setUpTestConfiguration(), "otherTest", + "other.prefix"); + return srcConfig; + } + + /** + * Tests whether a sub configuration survives updates of its parent. + */ + @Test + public void testSubConfigurationWithUpdates() + { + AbstractConfiguration srcConfig = setUpSubConfigTest(); + HierarchicalConfiguration<ImmutableNode> sub = + config.configurationAt(SUB_KEY, true); + assertTrue("Wrong value before update", sub.getBoolean(TEST_KEY)); + srcConfig.setProperty(TEST_KEY, Boolean.FALSE); + assertFalse("Wrong value after update", sub.getBoolean(TEST_KEY)); + assertFalse("Wrong value from combined configuration", + config.getBoolean(SUB_KEY + '.' + TEST_KEY)); + } + + /** + * Checks the configurationsAt() method. + * @param withUpdates flag whether updates are supported + */ + private void checkConfigurationsAt(boolean withUpdates) + { + setUpSubConfigTest(); + List<HierarchicalConfiguration<ImmutableNode>> subs = + config.configurationsAt(SUB_KEY, withUpdates); + assertEquals("Wrong number of sub configurations", 1, subs.size()); + assertTrue("Wrong value in sub configuration", + subs.get(0).getBoolean(TEST_KEY)); + } + + /** + * Tests whether sub configurations can be created from a key. + */ + @Test + public void testConfigurationsAt() + { + checkConfigurationsAt(false); + } + + /** + * Tests whether sub configurations can be created which are attached. + */ + @Test + public void testConfigurationsAtWithUpdates() + { + checkConfigurationsAt(true); + } + + /** * Helper method for creating a test configuration to be added to the * combined configuration. * * @return the test configuration */ - private AbstractConfiguration setUpTestConfiguration() + private static AbstractConfiguration setUpTestConfiguration() { BaseHierarchicalConfiguration config = new BaseHierarchicalConfiguration(); config.addProperty(TEST_KEY, Boolean.TRUE);
