mpoeschl 2002/12/03 06:03:54
Modified: configuration build.xml
configuration/src/java/org/apache/commons/configuration
BaseConfiguration.java
configuration/src/test/org/apache/commons/configuration
TestPropertiesConfiguration.java test.properties
Added: configuration/src/test/org/apache/commons/configuration
include.properties
Log:
I stumped over the following problem:
foo.bar = aaa
foo.bar = bbb, ccc
I expected a Configuration object to return for getVector("foo.bar") :
[ "aaa", "bbb", "ccc" ]
but I got
[ "aaa", "bbb, ccc" ]
Which basically sucks and is not the expected behaviour. Then I took
a look into BaseConfiguration (and recoiled in horror).
The attached patch(es) fix up the mess surrounding the internal store,
implement a Container wrapper for Vectors (as suggested in the comments)
and return the correct values for the scenario described above.
patch by Henning P. Schmiedehausen <[EMAIL PROTECTED]>
i also added a unit test for the problem described
Revision Changes Path
1.4 +2 -0 jakarta-commons-sandbox/configuration/build.xml
Index: build.xml
===================================================================
RCS file: /home/cvs/jakarta-commons-sandbox/configuration/build.xml,v
retrieving revision 1.3
retrieving revision 1.4
diff -u -r1.3 -r1.4
--- build.xml 28 Aug 2002 19:45:27 -0000 1.3
+++ build.xml 3 Dec 2002 14:03:53 -0000 1.4
@@ -130,6 +130,8 @@
<get dest="lib/commons-collections-2.0.jar" usetimestamp="true"
ignoreerrors="true"
src="http://www.ibiblio.org/maven/commons-collections/jars/commons-collections-2.0.jar";></get>
<get dest="lib/commons-lang-1.0-b1.jar" usetimestamp="true" ignoreerrors="true"
src="http://www.ibiblio.org/maven/commons-lang/jars/commons-lang-1.0-b1.jar";></get>
<get dest="lib/junit-3.7.jar" usetimestamp="true" ignoreerrors="true"
src="http://www.ibiblio.org/maven/junit/jars/junit-3.7.jar";></get>
+ <get dest="lib/xercesImpl-2.0.2.jar" usetimestamp="true" ignoreerrors="true"
src="http://www.ibiblio.org/maven/xerces/jars/xercesImpl-2.0.2.jar";></get>
+ <get dest="lib/xmlParserAPIs-2.0.2.jar" usetimestamp="true" ignoreerrors="true"
src="http://www.ibiblio.org/maven/xml-apis/jars/xmlParserAPIs-2.0.2.jar";></get>
</target>
1.4 +218 -135
jakarta-commons-sandbox/configuration/src/java/org/apache/commons/configuration/BaseConfiguration.java
Index: BaseConfiguration.java
===================================================================
RCS file:
/home/cvs/jakarta-commons-sandbox/configuration/src/java/org/apache/commons/configuration/BaseConfiguration.java,v
retrieving revision 1.3
retrieving revision 1.4
diff -u -r1.3 -r1.4
--- BaseConfiguration.java 28 Aug 2002 18:16:02 -0000 1.3
+++ BaseConfiguration.java 3 Dec 2002 14:03:54 -0000 1.4
@@ -55,8 +55,10 @@
*/
import java.util.ArrayList;
+import java.util.Collection;
import java.util.Hashtable;
import java.util.Iterator;
+import java.util.List;
import java.util.NoSuchElementException;
import java.util.Properties;
import java.util.StringTokenizer;
@@ -81,6 +83,7 @@
* @author <a href="mailto:[EMAIL PROTECTED]";>Ilkka Priha</a>
* @author <a href="mailto:[EMAIL PROTECTED]";>Jason van Zyl</a>
* @author <a href="mailto:[EMAIL PROTECTED]";>Martin Poeschl</a>
+ * @author <a href="mailto:[EMAIL PROTECTED]";>Henning P. Schmiedehausen</a>
* @version $Id$
*/
public class BaseConfiguration implements Configuration
@@ -115,81 +118,118 @@
*
* ["file", "classpath"]
*
- * @param key
- * @param token
+ * @param key The Key to add the property to.
+ * @param token The Value to add.
*/
public void addProperty(String key, Object token)
{
- Object o = store.get(key);
+ List tokenAdd = null;
- /*
- * $$$ GMJ
- * FIXME : post 1.0 release, we need to not assume
- * that a scalar is a String - it can be an Object
- * so we should make a little vector-like class
- * say, Foo that wraps (not extends Vector),
- * so we can do things like
- * if ( !( o instanceof Foo) )
- * so we know it's our 'vector' container
- *
- * This applies throughout
- */
- if (o instanceof String)
+ if (token instanceof String)
{
- Vector v = new Vector(2);
- v.addElement(o);
- v.addElement(token);
- store.put(key, v);
+ tokenAdd = processString((String) token);
}
- else if (o instanceof Vector)
+ else if (token instanceof Collection)
{
- ((Vector) o).addElement(token);
+ for (Iterator it = ((Collection) token).iterator(); it.hasNext(); )
+ {
+ addProperty(key, it.next());
+ }
+ return;
}
else
{
- /*
- * This is the first time that we have seen request to place an
- * object in the configuration with the key 'key'. So we just want
- * to place it directly into the configuration ... but we are going
- * to make a special exception for String objects that contain ","
- * characters. We will take CSV lists and turn the list into a
- * vector of Strings before placing it in the configuration.
- * This is a concession for Properties and the like that cannot
- * parse multiple same key values.
- */
- if (token instanceof String &&
- ((String) token).indexOf(PropertiesTokenizer.DELIMITER) > 0)
+ tokenAdd = new Vector(1);
+ tokenAdd.add(token);
+ }
+
+ Object o = store.get(key);
+
+ if (o instanceof Container)
+ {
+ // There is already a container for our key in the config
+ // Simply add the new tokens
+ for (Iterator it = tokenAdd.iterator(); it.hasNext(); )
+ {
+ ((Container) o).add(it.next());
+ }
+ }
+ else
+ {
+ // No Key at all or the token key is not a container.
+ Container c = new Container();
+
+ if (o != null)
{
- PropertiesTokenizer tokenizer =
- new PropertiesTokenizer((String) token);
+ // There is an element. Put it into the container
+ // at the first position
+ c.add(o);
+ }
- while (tokenizer.hasMoreTokens())
- {
- String value = tokenizer.nextToken();
+ // Now gobble up the supplied objects
+ for (Iterator it = tokenAdd.iterator(); it.hasNext(); )
+ {
+ c.add(it.next());
+ }
- /*
- * we know this is a string, so make sure it just goes in
- * rather than risking vectorization if it contains an
- * escaped comma
- */
- addStringProperty(key, value);
- }
+ // Do we have a key? If not, we simply add either the container
+ // (If the element was a CSV) or the first element of the
+ // Container (if its size is 1)
+
+ if (o == null && c.size() == 1)
+ {
+ // No Key existed and only one got put into the container. Then
+ // add the key direct. Do not mess with the container
+ addPropertyDirect(key, c.get(0));
}
else
{
- /*
- * We want to keep track of the order the keys are parsed, or
- * dynamically entered into the configuration. So when we see a
- * key for the first time we will place it in an ArrayList so
- * that if a client class needs to perform operations with
- * configuration in a definite order it will be possible.
- */
- addPropertyDirect(key, token);
+ // Either a key already existed or there was a CSV supplied
+ // Add the Container to the Store
+ addPropertyDirect(key, c);
}
}
}
/**
+ * Returns a Vector of Strings built from the supplied
+ * String. Splits up CSV lists. If no commas are in the
+ * String, simply returns a Vector with the String as its
+ * first element
+ *
+ * @param token The String to tokenize
+ *
+ * @return A List of Strings
+ */
+ protected List processString(String token)
+ {
+ List retList = new ArrayList(2);
+
+ if (token.indexOf(PropertiesTokenizer.DELIMITER) > 0)
+ {
+ PropertiesTokenizer tokenizer =
+ new PropertiesTokenizer((String) token);
+
+ while (tokenizer.hasMoreTokens())
+ {
+ String value = tokenizer.nextToken();
+ retList.add(value);
+ }
+ }
+ else
+ {
+ retList.add(token);
+ }
+
+ //
+ // We keep the sequence of the keys here and
+ // we also keep it in the Container. So the
+ // Keys are added to the store in the sequence that
+ // is given in the properties
+ return retList;
+ }
+
+ /**
* Adds a key/value pair to the map. This routine does no magic morphing.
* It ensures the keylist is maintained
*
@@ -209,49 +249,6 @@
}
/**
- * Sets a string property w/o checking for commas - used internally when a
- * property has been broken up into strings that could contain escaped
- * commas to prevent the inadvertant vectorization.
- */
- private void addStringProperty(String key, String token)
- {
- Object o = store.get(key);
-
- /*
- * $$$ GMJ
- * FIXME : post 1.0 release, we need to not assume
- * that a scalar is a String - it can be an Object
- * so we should make a little vector-like class
- * say, Foo that wraps (not extends Vector),
- * so we can do things like
- * if ( !( o instanceof Foo) )
- * so we know it's our 'vector' container
- *
- * This applies throughout
- */
-
- /*
- * do the usual thing - if we have a value and
- * it's scalar, make a vector, otherwise add to the vector
- */
- if (o instanceof String)
- {
- Vector v = new Vector(2);
- v.addElement(o);
- v.addElement(token);
- store.put(key, v);
- }
- else if (o instanceof Vector)
- {
- ((Vector) o).addElement(token);
- }
- else
- {
- addPropertyDirect(key, token);
- }
- }
-
- /**
* interpolate key names to handle ${key} stuff
*/
protected String interpolate(String base)
@@ -502,7 +499,7 @@
* @param key The configuration key.
* @return The associated properties if key is found.
* @exception ClassCastException is thrown if the key maps to an
- * object that is not a String/Vector.
+ * object that is not a String/Vector of Strings.
* @exception IllegalArgumentException if one of the tokens is
* malformed (does not contain an equals sign).
*/
@@ -557,6 +554,16 @@
o = defaults.getProperty(key);
}
}
+
+ //
+ // We must never give a Container Object out. So if the
+ // Return Value is a Container, we fix it up to be a
+ // Vector
+ //
+ if (o instanceof Container)
+ {
+ o = ((Container) o).asVector();
+ }
return o;
}
@@ -621,7 +628,6 @@
{
String s = testBoolean((String) value);
Boolean b = new Boolean(s);
- store.put(key, b);
return b;
}
else if (value == null)
@@ -707,7 +713,6 @@
else if (value instanceof String)
{
Byte b = new Byte((String) value);
- store.put(key, b);
return b;
}
else if (value == null)
@@ -793,7 +798,6 @@
else if (value instanceof String)
{
Double d = new Double((String) value);
- store.put(key, d);
return d;
}
else if (value == null)
@@ -879,7 +883,6 @@
else if (value instanceof String)
{
Float f = new Float((String) value);
- store.put(key, f);
return f;
}
else if (value == null)
@@ -973,7 +976,6 @@
else if (value instanceof String)
{
Integer i = new Integer((String) value);
- store.put(key, i);
return i;
}
else if (value == null)
@@ -1059,7 +1061,6 @@
else if (value instanceof String)
{
Long l = new Long((String) value);
- store.put(key, l);
return l;
}
else if (value == null)
@@ -1145,7 +1146,6 @@
else if (value instanceof String)
{
Short s = new Short((String) value);
- store.put(key, s);
return s;
}
else if (value == null)
@@ -1207,9 +1207,9 @@
return interpolate(defaultValue);
}
}
- else if (value instanceof Vector)
+ else if (value instanceof Container)
{
- return interpolate((String) ((Vector) value).get(0));
+ return interpolate((String) ((Container) value).get(0));
}
else
{
@@ -1225,32 +1225,38 @@
* @param key The configuration key.
* @return The associated string array if key is found.
* @exception ClassCastException is thrown if the key maps to an
- * object that is not a String/Vector.
+ * object that is not a String/Vector of Strings.
*/
public String[] getStringArray(String key)
{
Object value = store.get(key);
- // What's your vector, Victor?
- Vector vector;
+ String [] tokens;
+
if (value instanceof String)
{
- vector = new Vector(1);
- vector.addElement(interpolate((String) value));
+ tokens = new String [1];
+
+ tokens[0] = interpolate((String) value);
}
- else if (value instanceof Vector)
+ else if (value instanceof Container)
{
- vector = (Vector) value;
+ tokens = new String [((Container) value).size()];
+
+ for (int i = 0; i < tokens.length; i++)
+ {
+ tokens[i] = interpolate((String) ((Container) value).get(i));
+ }
}
else if (value == null)
{
if (defaults != null)
{
- return defaults.getStringArray(key);
+ tokens = defaults.getStringArray(key);
}
else
{
- return new String[0];
+ tokens = new String[0];
}
}
else
@@ -1258,13 +1264,6 @@
throw new ClassCastException(
'\'' + key + "' doesn't map to a String/Vector object");
}
-
- String[] tokens = new String[vector.size()];
- for (int i = 0; i < tokens.length; i++)
- {
- tokens[i] = (String) vector.elementAt(i);
- }
-
return tokens;
}
@@ -1293,35 +1292,35 @@
public Vector getVector(String key, Vector defaultValue)
{
Object value = store.get(key);
+ Vector v = null;
- if (value instanceof Vector)
+ if (value instanceof String)
{
- return (Vector) value;
+ v = new Vector(1);
+ v.addElement((String) value);
}
- else if (value instanceof String)
+ else if (value instanceof Container)
{
- Vector v = new Vector(1);
- v.addElement((String) value);
- store.put(key, v);
- return v;
+ v = ((Container) value).asVector();
}
else if (value == null)
{
if (defaults != null)
{
- return defaults.getVector(key, defaultValue);
+ v = defaults.getVector(key, defaultValue);
}
else
{
- return ((defaultValue == null) ?
- new Vector() : defaultValue);
+ v = ((defaultValue == null) ?
+ new Vector() : defaultValue);
}
}
else
{
throw new ClassCastException(
- '\'' + key + "' doesn't map to a Vector object");
+ '\'' + key + "' doesn't map to a Vector object: " + value + ", a "
+ value.getClass().getName());
}
+ return v;
}
/**
@@ -1383,4 +1382,88 @@
return buffer.toString().trim();
}
} // class PropertiesTokenizer
+
+
+ /**
+ * Private Wrapper class for Vector, so we can distinguish between
+ * Vector objects and our container
+ */
+ class Container
+ {
+ /** We're wrapping a List object (A vector) */
+ private List l = null;
+
+ /**
+ * C'tor
+ */
+ public Container()
+ {
+ l = new Vector(2);
+ }
+
+ /**
+ * Add an Object to the Container
+ *
+ * @param o The Object
+ */
+ public void add(Object o)
+ {
+ l.add(o);
+ }
+
+ /**
+ * Returns the current size of the Container
+ *
+ * @return The Number of elements in the container
+ */
+ public int size()
+ {
+ return l.size();
+ }
+
+ /**
+ * Returns the Element at an index
+ *
+ * @param index The Index
+ *
+ * @return The element at that index
+ */
+ public Object get(int index)
+ {
+ return l.get(index);
+ }
+
+ /**
+ * Returns an Iterator over the container
+ * objects
+ *
+ * @return An Iterator
+ */
+ public Iterator iterator()
+ {
+ return l.iterator();
+ }
+
+ /**
+ * Returns the Elements of the Container as
+ * a Vector. This is not the internal vector
+ * element but a shallow copy of the internal
+ * list. You may modify the returned list without
+ * modifying the container.
+ *
+ * @return A Vector containing the elements of
+ * the Container.
+ */
+
+ public Vector asVector()
+ {
+ Vector v = new Vector(l.size());
+
+ for (Iterator it = l.iterator(); it.hasNext(); )
+ {
+ v.add(it.next());
+ }
+ return v;
+ }
+ }
}
1.2 +42 -1
jakarta-commons-sandbox/configuration/src/test/org/apache/commons/configuration/TestPropertiesConfiguration.java
Index: TestPropertiesConfiguration.java
===================================================================
RCS file:
/home/cvs/jakarta-commons-sandbox/configuration/src/test/org/apache/commons/configuration/TestPropertiesConfiguration.java,v
retrieving revision 1.1
retrieving revision 1.2
diff -u -r1.1 -r1.2
--- TestPropertiesConfiguration.java 28 Aug 2002 19:43:13 -0000 1.1
+++ TestPropertiesConfiguration.java 3 Dec 2002 14:03:54 -0000 1.2
@@ -90,6 +90,47 @@
}
}
+ /**
+ * test if includes properites get loaded too
+ */
+ public void testLoadInclude()
+ {
+ PropertiesConfiguration conf = new PropertiesConfiguration();
+ InputStream input = this.getClass().getResourceAsStream(
+ "/org/apache/commons/configuration/test.properties");
+ try
+ {
+ conf.load(input);
+ String loaded = conf.getString("include.loaded");
+ assertEquals("true", loaded);
+ }
+ catch (Exception e)
+ {
+ System.err.println("Exception thrown: " + e);
+ }
+ }
+
+ /**
+ * test if includes properites get loaded too
+ */
+ public void testVector()
+ {
+ PropertiesConfiguration conf = new PropertiesConfiguration();
+ InputStream input = this.getClass().getResourceAsStream(
+ "/org/apache/commons/configuration/test.properties");
+ try
+ {
+ conf.load(input);
+ Vector loaded = conf.getVector("packages");
+ // we should get 3 packages here
+ assertEquals(3, loaded.size());
+ }
+ catch (Exception e)
+ {
+ System.err.println("Exception thrown: " + e);
+ }
+ }
+
public void testSave()
{
PropertiesConfiguration conf = new PropertiesConfiguration();
1.2 +4 -0
jakarta-commons-sandbox/configuration/src/test/org/apache/commons/configuration/test.properties
Index: test.properties
===================================================================
RCS file:
/home/cvs/jakarta-commons-sandbox/configuration/src/test/org/apache/commons/configuration/test.properties,v
retrieving revision 1.1
retrieving revision 1.2
diff -u -r1.1 -r1.2
--- test.properties 28 Aug 2002 19:43:13 -0000 1.1
+++ test.properties 3 Dec 2002 14:03:54 -0000 1.2
@@ -1,2 +1,6 @@
configuration.loaded = true
+packages = packagea
+
+include = include.properties
+
1.1
jakarta-commons-sandbox/configuration/src/test/org/apache/commons/configuration/include.properties
Index: include.properties
===================================================================
include.loaded = true
packages = packageb, packagec
--
To unsubscribe, e-mail: <mailto:[EMAIL PROTECTED]>
For additional commands, e-mail: <mailto:[EMAIL PROTECTED]>
