Handle multiple keys better,

bug reported by Wolfgang Hoschek


git-svn-id: https://svn.apache.org/repos/asf/jakarta/commons/proper/collections/trunk@131794 13f79535-47bb-0310-9956-ffa450edef68

src/java/org/apache/commons/collections/ExtendedProperties.java

@@ -123,7 +123,7 @@ import java.util.Vector;
  * it, go ahead and tune it up!
  *
  * @since Commons Collections 1.0
- * @version $Revision: 1.22 $ $Date: 2004/03/17 21:09:08 $
+ * @version $Revision: 1.23 $ $Date: 2004/06/21 23:39:25 $
  * 
  * @author <a href="mailto:stefano@apache.org">Stefano Mazzocchi</a>
  * @author <a href="mailto:jon@latchkey.com">Jon S. Stevens</a>
@@ -639,78 +639,24 @@ public class ExtendedProperties extends Hashtable {
      * <code>["file", "classpath"]</code>
      *
      * @param key  the key to add
-     * @param token  the value to add
+     * @param value  the value to add
      */
-    public void addProperty(String key, Object token) {
+    public void addProperty(String key, Object value) {
-        Object obj = this.get(key);
+        if (value instanceof String) {
-
+            String str = (String) value;
-        /*
+            if (str.indexOf(PropertiesTokenizer.DELIMITER) > 0) {
-         *  $$$ GMJ
+                // token contains commas, so must be split apart then added
-         *  FIXME : post 1.0 release, we need to not assume
+                PropertiesTokenizer tokenizer = new PropertiesTokenizer(str);
-         *  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 (obj instanceof String) {
-            Vector v = new Vector(2);
-            v.addElement(obj);
-            v.addElement(token);
-            put(key, v);
-            
-        } else if (obj instanceof Vector) {
-            ((Vector) obj).addElement(token);
-            
-        } 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) {
-                    
-                PropertiesTokenizer tokenizer = new PropertiesTokenizer((String) token);
-
                 while (tokenizer.hasMoreTokens()) {
-                    String value = tokenizer.nextToken();
+                    String token = tokenizer.nextToken();
-
+                    addPropertyInternal(key, unescape(token));
-                    /*
-                     * 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, unescape(value));
                 }
             } else {
-                /*
+                // token contains no commas, so can be simply added
-                 * We want to keep track of the order the keys
+                addPropertyInternal(key, unescape(str));
-                 * 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.
-                 */
-                if (token instanceof String) {
-                    token = unescape((String) token);
-                }
-                addPropertyDirect(key, token);
             }
+        } else {
+            addPropertyInternal(key, value);
         }
 
         // Adding a property connotes initialization
@@ -721,56 +667,48 @@ public class ExtendedProperties extends Hashtable {
      * Adds a key/value pair to the map.  This routine does
      * no magic morphing.  It ensures the keylist is maintained
      *
-     * @param key key to use for mapping
+     * @param key  the key to store at
-     * @param obj object to store
+     * @param value  the decoded object to store
      */
-    private void addPropertyDirect(String key, Object obj) {
+    private void addPropertyDirect(String key, Object value) {
         // safety check
         if (!containsKey(key)) {
             keysAsListed.add(key);
         }
-        put(key, obj);
+        put(key, value);
     }
 
     /**
-     * Sets a string property w/o checking for commas - used
+     * Adds a decoded property to the map w/o checking for commas - used
      * internally when a property has been broken up into
      * strings that could contain escaped commas to prevent
      * the inadvertent vectorization.
      * <p>
      * Thanks to Leon Messerschmidt for this one.
      *
+     * @param key  the key to store at
+     * @param value  the decoded object to store
      */
-    private void addStringProperty(String key, String token) {
+    private void addPropertyInternal(String key, Object value) {
-        Object obj = this.get(key);
+        Object current = this.get(key);
 
-        /*
+        if (current instanceof String) {
-         *  $$$ GMJ
+            // one object already in map - convert it to a vector
-         *  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 (obj instanceof String) {
             Vector v = new Vector(2);
-            v.addElement(obj);
+            v.addElement(current);
-            v.addElement(token);
+            v.addElement(value);
             put(key, v);
             
-        } else if (obj instanceof Vector) {
+        } else if (current instanceof Vector) {
-            ((Vector) obj).addElement(token);
+            // already a vector - just add the new token
+            ((Vector) current).addElement(value);
             
         } else {
-            addPropertyDirect(key, token);
+            // brand new key - store in keysAsListed to retain order
+            if (!containsKey(key)) {
+                keysAsListed.add(key);
+            }
+            put(key, value);
         }
     }
 

src/test/org/apache/commons/collections/TestExtendedProperties.java

@@ -26,7 +26,7 @@ import junit.framework.TestSuite;
 /**
  * Tests some basic functions of the ExtendedProperties class.
  * 
- * @version $Revision: 1.12 $ $Date: 2004/02/18 01:20:35 $
+ * @version $Revision: 1.13 $ $Date: 2004/06/21 23:39:25 $
  * 
  * @author Geir Magnusson Jr.
  * @author Mohan Kishore
@@ -171,4 +171,85 @@ public class TestExtendedProperties extends TestCase {
         }
     }
     
+    public void testMultipleSameKey1() throws Exception {
+        ExtendedProperties ep1 = new ExtendedProperties();
+
+        /*
+        initialize using:
+        one=a
+        one=b,c
+        */
+        String s1 = "one=a\none=b,c\n";
+        byte[] bytes = s1.getBytes();
+        ByteArrayInputStream bais = new ByteArrayInputStream(bytes);
+        ep1.load(bais);
+        assertEquals(1, ep1.size());
+        assertEquals(3, ep1.getVector("one").size());
+        assertEquals("a", ep1.getVector("one").get(0));
+        assertEquals("b", ep1.getVector("one").get(1));
+        assertEquals("c", ep1.getVector("one").get(2));
+    }
+    
+    public void testMultipleSameKey2() throws Exception {
+        ExtendedProperties ep1 = new ExtendedProperties();
+
+        /*
+        initialize using:
+        one=a,b
+        one=c,d
+        */
+        String s1 = "one=a,b\none=c,d\n";
+        byte[] bytes = s1.getBytes();
+        ByteArrayInputStream bais = new ByteArrayInputStream(bytes);
+        ep1.load(bais);
+        assertEquals(1, ep1.size());
+        assertEquals(4, ep1.getVector("one").size());
+        assertEquals("a", ep1.getVector("one").get(0));
+        assertEquals("b", ep1.getVector("one").get(1));
+        assertEquals("c", ep1.getVector("one").get(2));
+        assertEquals("d", ep1.getVector("one").get(3));
+    }
+    
+    public void testMultipleSameKey3() throws Exception {
+        ExtendedProperties ep1 = new ExtendedProperties();
+
+        /*
+        initialize using:
+        one=a,b
+        one=c
+        */
+        String s1 = "one=a,b\none=c\n";
+        byte[] bytes = s1.getBytes();
+        ByteArrayInputStream bais = new ByteArrayInputStream(bytes);
+        ep1.load(bais);
+        assertEquals(1, ep1.size());
+        assertEquals(3, ep1.getVector("one").size());
+        assertEquals("a", ep1.getVector("one").get(0));
+        assertEquals("b", ep1.getVector("one").get(1));
+        assertEquals("c", ep1.getVector("one").get(2));
+    }
+    
+    public void testMultipleSameKeyByCode() throws Exception {
+        ExtendedProperties ep1 = new ExtendedProperties();
+
+        ep1.addProperty("one", "a");
+        assertEquals(1, ep1.size());
+        assertEquals(1, ep1.getVector("one").size());
+        assertEquals("a", ep1.getVector("one").get(0));
+        
+        ep1.addProperty("one", Boolean.TRUE);
+        assertEquals(1, ep1.size());
+        assertEquals(2, ep1.getVector("one").size());
+        assertEquals("a", ep1.getVector("one").get(0));
+        assertEquals(Boolean.TRUE, ep1.getVector("one").get(1));
+        
+        ep1.addProperty("one", "c,d");
+        assertEquals(1, ep1.size());
+        assertEquals(4, ep1.getVector("one").size());
+        assertEquals("a", ep1.getVector("one").get(0));
+        assertEquals(Boolean.TRUE, ep1.getVector("one").get(1));
+        assertEquals("c", ep1.getVector("one").get(2));
+        assertEquals("d", ep1.getVector("one").get(3));
+    }
+    
 }