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]>

Reply via email to