You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@turbine.apache.org by he...@apache.org on 2005/09/09 17:35:59 UTC

svn commit: r279799 - in /jakarta/turbine/core/branches/TURBINE_2_3_BRANCH/src: java/org/apache/turbine/util/parser/BaseValueParser.java test/org/apache/turbine/util/parser/BaseValueParserTest.java

Author: henning
Date: Fri Sep  9 08:35:52 2005
New Revision: 279799

URL: http://svn.apache.org/viewcvs?rev=279799&view=rev
Log:
Clean up the internal parameter management, make sure that
null values never hit the parameters array.

Remove a number of StringUtils.isEmpty calls, the converter methods
will throw NPEs if problems arise; catch these and treat like
NumberFormatExceptions

Ruthlessly exploit the existence of ArrayUtils.add() in c-c to
simplify the add(key, string) method.

Replace getStrings() calls with getParam, making sure that overriding
getStrings will not mess with our internal parameter management.

Make sure that all getXXX methods behave similar with non-existing
keys. Put an Unit test on this.

Don't treat the String "null" as null value. This should fix a number
of complaints from the users list. This is no longer necessary when
the contents of the parameters map is carefully controlled to never
contain null values.

Clean up toString() to actually work not only on BaseValueParser but
also on derived classes. ArrayIterators rule!



Modified:
    jakarta/turbine/core/branches/TURBINE_2_3_BRANCH/src/java/org/apache/turbine/util/parser/BaseValueParser.java
    jakarta/turbine/core/branches/TURBINE_2_3_BRANCH/src/test/org/apache/turbine/util/parser/BaseValueParserTest.java

Modified: jakarta/turbine/core/branches/TURBINE_2_3_BRANCH/src/java/org/apache/turbine/util/parser/BaseValueParser.java
URL: http://svn.apache.org/viewcvs/jakarta/turbine/core/branches/TURBINE_2_3_BRANCH/src/java/org/apache/turbine/util/parser/BaseValueParser.java?rev=279799&r1=279798&r2=279799&view=diff
==============================================================================
--- jakarta/turbine/core/branches/TURBINE_2_3_BRANCH/src/java/org/apache/turbine/util/parser/BaseValueParser.java (original)
+++ jakarta/turbine/core/branches/TURBINE_2_3_BRANCH/src/java/org/apache/turbine/util/parser/BaseValueParser.java Fri Sep  9 08:35:52 2005
@@ -34,6 +34,8 @@
 import java.util.Map;
 import java.util.Set;
 
+import org.apache.commons.collections.iterators.ArrayIterator;
+import org.apache.commons.lang.ArrayUtils;
 import org.apache.commons.lang.StringUtils;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
@@ -226,20 +228,9 @@
     {
         if (value != null)
         {
-            String[] items = this.getStrings(name);
-            if (items == null)
-            {
-                items = new String[1];
-                items[0] = value;
-                putParam(name, items);
-            }
-            else
-            {
-                String[] newItems = new String[items.length + 1];
-                System.arraycopy(items, 0, newItems, 0, items.length);
-                newItems[items.length] = value;
-                putParam(name, newItems);
-            }
+            String [] items = this.getParam(name);
+            items = (String []) ArrayUtils.add(items, value);
+            putParam(name, items);
         }
     }
 
@@ -253,9 +244,17 @@
      */
     public void add(String name, String [] value)
     {
-        for (int i = 0 ; i < value.length; i++)
+        // ArrayUtils.addAll() looks promising but it would also add
+        // null values into the parameters array, so we can't use that.
+        if (value != null)
         {
-            add(name, value[i]);
+            for (int i = 0 ; i < value.length; i++)
+            {
+                if (value[i] != null)
+                {
+                    add(name, value[i]);
+                }
+            }
         }
     }
 
@@ -453,7 +452,7 @@
     public Boolean getBooleanObject(String name, Boolean defaultValue)
     {
         Boolean result = getBooleanObject(name);
-        return (result==null ? defaultValue : result);
+        return (result == null ? defaultValue : result);
     }
 
     /**
@@ -495,17 +494,20 @@
     {
         double result = defaultValue;
         String value = getString(name);
-        if (StringUtils.isNotEmpty(value))
+
+        try
         {
-            try
-            {
-                result = Double.valueOf(value).doubleValue();
-            }
-            catch (NumberFormatException e)
-            {
-                logConvertionFailure(name, value, "Double");
-            }
+            result = Double.parseDouble(value);
+        }
+        catch (NumberFormatException e)
+        {
+            logConvertionFailure(name, value, "Double");
+        }
+        catch (NullPointerException e)
+        {
+            logConvertionFailure(name, value, "Double");
         }
+
         return result;
     }
 
@@ -531,22 +533,23 @@
     public double[] getDoubles(String name)
     {
         double[] result = null;
-        String value[] = getStrings(name);
+        String value[] = getParam(name);
         if (value != null)
         {
             result = new double[value.length];
             for (int i = 0; i < value.length; i++)
             {
-                if (StringUtils.isNotEmpty(value[i]))
+                try
                 {
-                    try
-                    {
-                        result[i] = Double.parseDouble(value[i]);
-                    }
-                    catch (NumberFormatException e)
-                    {
-                        logConvertionFailure(name, value[i], "Double");
-                    }
+                    result[i] = Double.parseDouble(value[i]);
+                }
+                catch (NumberFormatException e)
+                {
+                    logConvertionFailure(name, value[i], "Double");
+                }
+                catch (NullPointerException e)
+                {
+                    logConvertionFailure(name, value[i], "Double");
                 }
             }
         }
@@ -564,7 +567,7 @@
     public Double getDoubleObject(String name, Double defaultValue)
     {
         Double result = getDoubleObject(name);
-        return (result==null ? defaultValue : result);
+        return (result == null ? defaultValue : result);
     }
 
     /**
@@ -578,16 +581,18 @@
     {
         Double result = null;
         String value = getString(name);
-        if (StringUtils.isNotEmpty(value))
+
+        try
         {
-            try
-            {
-                result = new Double(value);
-            }
-            catch(NumberFormatException e)
-            {
-                logConvertionFailure(name, value, "Double");
-            }
+            result = new Double(value);
+        }
+        catch(NumberFormatException e)
+        {
+            logConvertionFailure(name, value, "Double");
+        }
+        catch(NullPointerException e)
+        {
+            logConvertionFailure(name, value, "Double");
         }
         return result;
     }
@@ -602,22 +607,23 @@
     public Double[] getDoubleObjects(String name)
     {
         Double[] result = null;
-        String value[] = getStrings(convert(name));
+        String value[] = getParam(name);
         if (value != null)
         {
             result = new Double[value.length];
             for (int i = 0; i < value.length; i++)
             {
-                if (StringUtils.isNotEmpty(value[i]))
+                try
                 {
-                    try
-                    {
-                        result[i] = Double.valueOf(value[i]);
-                    }
-                    catch (NumberFormatException e)
-                    {
-                        logConvertionFailure(name, value[i], "Double");
-                    }
+                    result[i] = Double.valueOf(value[i]);
+                }
+                catch (NumberFormatException e)
+                {
+                    logConvertionFailure(name, value[i], "Double");
+                }
+                catch (NullPointerException e)
+                {
+                    logConvertionFailure(name, value[i], "Double");
                 }
             }
         }
@@ -636,17 +642,20 @@
     {
         float result = defaultValue;
         String value = getString(name);
-        if (StringUtils.isNotEmpty(value))
+
+        try
         {
-            try
-            {
-                result = Float.valueOf(value).floatValue();
-            }
-            catch (NumberFormatException e)
-            {
-                logConvertionFailure(name, value, "Float");
-            }
+            result = Float.parseFloat(value);
+        }
+        catch (NumberFormatException e)
+        {
+            logConvertionFailure(name, value, "Float");
         }
+        catch (NullPointerException e)
+        {
+            logConvertionFailure(name, value, "Float");
+        }
+
         return result;
     }
 
@@ -672,22 +681,23 @@
     public float[] getFloats(String name)
     {
         float[] result = null;
-        String value[] = getStrings(name);
+        String value[] = getParam(name);
         if (value != null)
         {
             result = new float[value.length];
             for (int i = 0; i < value.length; i++)
             {
-                if (StringUtils.isNotEmpty(value[i]))
+                try
                 {
-                    try
-                    {
-                        result[i] = Float.parseFloat(value[i]);
-                    }
-                    catch (NumberFormatException e)
-                    {
-                        logConvertionFailure(name, value[i], "Float");
-                    }
+                    result[i] = Float.parseFloat(value[i]);
+                }
+                catch (NumberFormatException e)
+                {
+                    logConvertionFailure(name, value[i], "Float");
+                }
+                catch (NullPointerException e)
+                {
+                    logConvertionFailure(name, value[i], "Float");
                 }
             }
         }
@@ -705,7 +715,7 @@
     public Float getFloatObject(String name, Float defaultValue)
     {
         Float result = getFloatObject(name);
-        return (result==null ? defaultValue : result);
+        return (result == null ? defaultValue : result);
     }
 
     /**
@@ -719,17 +729,20 @@
     {
         Float result = null;
         String value = getString(name);
-        if (StringUtils.isNotEmpty(value))
+
+        try
         {
-            try
-            {
-                result = new Float(value);
-            }
-            catch(NumberFormatException e)
-            {
-                logConvertionFailure(name, value, "Float");
-            }
+            result = new Float(value);
+        }
+        catch(NumberFormatException e)
+        {
+            logConvertionFailure(name, value, "Float");
+        }
+        catch(NullPointerException e)
+        {
+            logConvertionFailure(name, value, "Float");
         }
+
         return result;
     }
 
@@ -743,22 +756,23 @@
     public Float[] getFloatObjects(String name)
     {
         Float[] result = null;
-        String value[] = getStrings(convert(name));
+        String value[] = getParam(name);
         if (value != null)
         {
             result = new Float[value.length];
             for (int i = 0; i < value.length; i++)
             {
-                if (StringUtils.isNotEmpty(value[i]))
+                try
                 {
-                    try
-                    {
-                        result[i] = Float.valueOf(value[i]);
-                    }
-                    catch (NumberFormatException e)
-                    {
-                        logConvertionFailure(name, value[i], "Float");
-                    }
+                    result[i] = Float.valueOf(value[i]);
+                }
+                catch (NumberFormatException e)
+                {
+                    logConvertionFailure(name, value[i], "Float");
+                }
+                catch (NullPointerException e)
+                {
+                    logConvertionFailure(name, value[i], "Float");
                 }
             }
         }
@@ -777,17 +791,20 @@
     {
         BigDecimal result = defaultValue;
         String value = getString(name);
-        if (StringUtils.isNotEmpty(value))
+
+        try
         {
-            try
-            {
-                result = new BigDecimal(value);
-            }
-            catch (NumberFormatException e)
-            {
-                logConvertionFailure(name, value, "BigDecimal");
-            }
+            result = new BigDecimal(value);
         }
+        catch (NumberFormatException e)
+        {
+            logConvertionFailure(name, value, "BigDecimal");
+        }
+        catch (NullPointerException e)
+        {
+            logConvertionFailure(name, value, "BigDecimal");
+        }
+
         return result;
     }
 
@@ -813,22 +830,23 @@
     public BigDecimal[] getBigDecimals(String name)
     {
         BigDecimal[] result = null;
-        String value[] = getStrings(name);
+        String value[] = getParam(name);
         if (value != null)
         {
             result = new BigDecimal[value.length];
             for (int i = 0; i < value.length; i++)
             {
-                if(StringUtils.isNotEmpty(value[i]))
+                try
                 {
-                    try
-                    {
-                        result[i] = new BigDecimal(value[i]);
-                    }
-                    catch (NumberFormatException e)
-                    {
-                        logConvertionFailure(name, value[i], "BigDecimal");
-                    }
+                    result[i] = new BigDecimal(value[i]);
+                }
+                catch (NumberFormatException e)
+                {
+                    logConvertionFailure(name, value[i], "BigDecimal");
+                }
+                catch (NullPointerException e)
+                {
+                    logConvertionFailure(name, value[i], "BigDecimal");
                 }
             }
         }
@@ -847,17 +865,20 @@
     {
         int result = defaultValue;
         String value = getString(name);
-        if (StringUtils.isNotEmpty(value))
+
+        try
         {
-            try
-            {
-                result = Integer.valueOf(value).intValue();
-            }
-            catch (NumberFormatException e)
-            {
-                logConvertionFailure(name, value, "Integer");
-            }
+            result = Integer.valueOf(value).intValue();
+        }
+        catch (NumberFormatException e)
+        {
+            logConvertionFailure(name, value, "Integer");
+        }
+        catch (NullPointerException e)
+        {
+            logConvertionFailure(name, value, "Integer");
         }
+
         return result;
     }
 
@@ -925,22 +946,23 @@
     public int[] getInts(String name)
     {
         int[] result = null;
-        String value[] = getStrings(name);
+        String value[] = getParam(name);
         if (value != null)
         {
             result = new int[value.length];
             for (int i = 0; i < value.length; i++)
             {
-                if (StringUtils.isNotEmpty(value[i]))
+                try
                 {
-                    try
-                    {
-                        result[i] = Integer.parseInt(value[i]);
-                    }
-                    catch (NumberFormatException e)
-                    {
-                        logConvertionFailure(name, value[i], "Integer");
-                    }
+                    result[i] = Integer.parseInt(value[i]);
+                }
+                catch (NumberFormatException e)
+                {
+                    logConvertionFailure(name, value[i], "Integer");
+                }
+                catch (NullPointerException e)
+                {
+                    logConvertionFailure(name, value[i], "Integer");
                 }
             }
         }
@@ -958,7 +980,7 @@
     public Integer getIntObject(String name, Integer defaultValue)
     {
         Integer result = getIntObject(name);
-        return (result==null ? defaultValue : result);
+        return (result == null ? defaultValue : result);
     }
 
     /**
@@ -972,17 +994,20 @@
     {
         Integer result = null;
         String value = getString(name);
-        if (StringUtils.isNotEmpty(value))
+
+        try
         {
-            try
-            {
-                result = new Integer(value);
-            }
-            catch(NumberFormatException e)
-            {
-                logConvertionFailure(name, value, "Integer");
-            }
+            result = new Integer(value);
+        }
+        catch(NumberFormatException e)
+        {
+            logConvertionFailure(name, value, "Integer");
         }
+        catch(NullPointerException e)
+        {
+            logConvertionFailure(name, value, "Integer");
+        }
+
         return result;
     }
 
@@ -996,22 +1021,23 @@
     public Integer[] getIntObjects(String name)
     {
         Integer[] result = null;
-        String value[] = getStrings(convert(name));
+        String value[] = getParam(name);
         if (value != null)
         {
             result = new Integer[value.length];
             for (int i = 0; i < value.length; i++)
             {
-                if (StringUtils.isNotEmpty(value[i]))
+                try
                 {
-                    try
-                    {
-                        result[i] = Integer.valueOf(value[i]);
-                    }
-                    catch (NumberFormatException e)
-                    {
-                        logConvertionFailure(name, value[i], "Integer");
-                    }
+                    result[i] = Integer.valueOf(value[i]);
+                }
+                catch (NumberFormatException e)
+                {
+                    logConvertionFailure(name, value[i], "Integer");
+                }
+                catch (NullPointerException e)
+                {
+                    logConvertionFailure(name, value[i], "Integer");
                 }
             }
         }
@@ -1043,17 +1069,20 @@
     {
         long result = defaultValue;
         String value = getString(name);
-        if (StringUtils.isNotEmpty(value))
+
+        try
         {
-            try
-            {
-                result = Long.valueOf(value).longValue();
-            }
-            catch (NumberFormatException e)
-            {
-                logConvertionFailure(name, value, "Long");
-            }
+            result = Long.valueOf(value).longValue();
+        }
+        catch (NumberFormatException e)
+        {
+            logConvertionFailure(name, value, "Long");
         }
+        catch (NullPointerException e)
+        {
+            logConvertionFailure(name, value, "Long");
+        }
+
         return result;
     }
 
@@ -1079,22 +1108,23 @@
     public long[] getLongs(String name)
     {
         long[] result = null;
-        String value[] = getStrings(name);
+        String value[] = getParam(name);
         if (value != null)
         {
             result = new long[value.length];
             for (int i = 0; i < value.length; i++)
             {
-                if (StringUtils.isNotEmpty(value[i]))
+                try
                 {
-                    try
-                    {
-                        result[i] = Long.parseLong(value[i]);
-                    }
-                    catch (NumberFormatException e)
-                    {
-                        logConvertionFailure(name, value[i], "Long");
-                    }
+                    result[i] = Long.parseLong(value[i]);
+                }
+                catch (NumberFormatException e)
+                {
+                    logConvertionFailure(name, value[i], "Long");
+                }
+                catch (NullPointerException e)
+                {
+                    logConvertionFailure(name, value[i], "Long");
                 }
             }
         }
@@ -1111,22 +1141,23 @@
     public Long[] getLongObjects(String name)
     {
         Long[] result = null;
-        String value[] = getStrings(convert(name));
+        String value[] = getParam(name);
         if (value != null)
         {
             result = new Long[value.length];
             for (int i = 0; i < value.length; i++)
             {
-                if (StringUtils.isNotEmpty(value[i]))
+                try
                 {
-                    try
-                    {
-                        result[i] = Long.valueOf(value[i]);
-                    }
-                    catch (NumberFormatException e)
-                    {
-                        logConvertionFailure(name, value[i], "Long");
-                    }
+                    result[i] = Long.valueOf(value[i]);
+                }
+                catch (NumberFormatException e)
+                {
+                    logConvertionFailure(name, value[i], "Long");
+                }
+                catch (NullPointerException e)
+                {
+                    logConvertionFailure(name, value[i], "Long");
                 }
             }
         }
@@ -1144,17 +1175,20 @@
     {
         Long result = null;
         String value = getString(name);
-        if (StringUtils.isNotEmpty(value))
+
+        try
         {
-            try
-            {
-                result = new Long(value);
-            }
-            catch(NumberFormatException e)
-            {
-                logConvertionFailure(name, value, "Long");
-            }
+            result = new Long(value);
+        }
+        catch(NumberFormatException e)
+        {
+            logConvertionFailure(name, value, "Long");
+        }
+        catch(NullPointerException e)
+        {
+            logConvertionFailure(name, value, "Long");
         }
+
         return result;
     }
 
@@ -1169,7 +1203,7 @@
     public Long getLongObject(String name, Long defaultValue)
     {
         Long result = getLongObject(name);
-        return (result==null ? defaultValue : result);
+        return (result == null ? defaultValue : result);
     }
 
     /**
@@ -1184,17 +1218,20 @@
     {
         byte result = defaultValue;
         String value = getString(name);
-        if (StringUtils.isNotEmpty(value))
+
+        try
         {
-            try
-            {
-                result = Byte.valueOf(value).byteValue();
-            }
-            catch (NumberFormatException e)
-            {
-                logConvertionFailure(name, value, "Byte");
-            }
+            result = Byte.valueOf(value).byteValue();
+        }
+        catch (NumberFormatException e)
+        {
+            logConvertionFailure(name, value, "Byte");
         }
+        catch (NullPointerException e)
+        {
+            logConvertionFailure(name, value, "Byte");
+        }
+
         return result;
     }
 
@@ -1224,7 +1261,7 @@
     {
         byte result[] = null;
         String value = getString(name);
-        if (StringUtils.isNotEmpty(value))
+        if (value != null)
         {
             result = value.getBytes(getCharacterEncoding());
         }
@@ -1242,7 +1279,7 @@
     public Byte getByteObject(String name, Byte defaultValue)
     {
         Byte result = getByteObject(name);
-        return (result==null ? defaultValue : result);
+        return (result == null ? defaultValue : result);
     }
 
     /**
@@ -1256,17 +1293,20 @@
     {
         Byte result = null;
         String value = getString(name);
-        if (StringUtils.isNotEmpty(value))
+
+        try
         {
-            try
-            {
-                result = new Byte(value);
-            }
-            catch(NumberFormatException e)
-            {
-                logConvertionFailure(name, value, "Byte");
-            }
+            result = new Byte(value);
+        }
+        catch(NumberFormatException e)
+        {
+            logConvertionFailure(name, value, "Byte");
+        }
+        catch(NullPointerException e)
+        {
+            logConvertionFailure(name, value, "Byte");
         }
+
         return result;
     }
 
@@ -1275,31 +1315,15 @@
      * exist, return null.
      *
      * @param name A String with the name.
-     * @return A String.
+     * @return A String or null if the key is unknown.
      */
     public String getString(String name)
     {
-        String result = null;
-        try
-        {
-            Object value = getParam(name);
-            if (value != null)
-            {
-                value = ((String[]) value)[0];
-            }
-            if (value == null || value.equals("null"))
-            {
-                return null;
-            }
-            result = (String) value;
-        }
-        catch (ClassCastException e)
-        {
-            log.fatal("Parameter ("
-                    + name + ") wasn not stored as a String", e);
-        }
+        String [] value = getParam(name);
 
-        return result;
+        return (value == null
+                || value.length == 0)
+                ? null : value[0];
     }
 
     /**
@@ -1359,7 +1383,7 @@
      */
     public String[] getStrings(String name)
     {
-        return (String[]) getParam(name);
+        return getParam(name);
     }
 
     /**
@@ -1372,7 +1396,7 @@
      */
     public String[] getStrings(String name, String[] defaultValue)
     {
-        String[] value = getStrings(name);
+        String[] value = getParam(name);
 
         return (value == null || value.length == 0)
             ? defaultValue : value;
@@ -1415,7 +1439,7 @@
      */
     public Object[] getObjects(String name)
     {
-        return getStrings(name);
+        return getParam(name);
     }
 
     /**
@@ -1631,46 +1655,36 @@
         for (Iterator iter = keySet().iterator(); iter.hasNext();)
         {
             String name = (String) iter.next();
-            try
+
+            sb.append('{');
+            sb.append(name);
+            sb.append('=');
+            String [] params = this.getParam(name);
+
+            if (params == null)
             {
-                sb.append('{');
-                sb.append(name);
-                sb.append('=');
-                String[] params = this.getStrings(name);
-                if (params.length <= 1)
-                {
-                    sb.append(params[0]);
-                }
-                else
-                {
-                    for (int i = 0; i < params.length; i++)
-                    {
-                        if (i != 0)
-                        {
-                            sb.append(", ");
-                        }
-                        sb.append('[')
-                                .append(params[i])
-                                .append(']');
-                    }
-                }
-                sb.append("}\n");
+                sb.append("unknown?");
             }
-            catch (Exception ee)
+            else if (params.length == 0)
             {
-                try
-                {
-                    sb.append('{');
-                    sb.append(name);
-                    sb.append('=');
-                    sb.append("ERROR?");
-                    sb.append("}\n");
-                }
-                catch (Exception eee)
+                sb.append("empty");
+            }
+            else
+            {
+                sb.append('[');
+                for (Iterator it = new ArrayIterator(params); it.hasNext(); )
                 {
+                    sb.append(it.next());
+                    if (it.hasNext())
+                    {
+                        sb.append(", ");
+                    }
                 }
+                sb.append(']');
             }
+            sb.append("}\n");
         }
+
         return sb.toString();
     }
 
@@ -1778,9 +1792,12 @@
     private void logConvertionFailure(String paramName,
                                       String value, String type)
     {
-        log.warn("Parameter (" + paramName
-                + ") with value of ("
-                + value + ") could not be converted to a " + type);
+        if (log.isWarnEnabled())
+        {
+            log.warn("Parameter (" + paramName
+                    + ") with value of ("
+                    + value + ") could not be converted to a " + type);
+        }
     }
 
     /**

Modified: jakarta/turbine/core/branches/TURBINE_2_3_BRANCH/src/test/org/apache/turbine/util/parser/BaseValueParserTest.java
URL: http://svn.apache.org/viewcvs/jakarta/turbine/core/branches/TURBINE_2_3_BRANCH/src/test/org/apache/turbine/util/parser/BaseValueParserTest.java?rev=279799&r1=279798&r2=279799&view=diff
==============================================================================
--- jakarta/turbine/core/branches/TURBINE_2_3_BRANCH/src/test/org/apache/turbine/util/parser/BaseValueParserTest.java (original)
+++ jakarta/turbine/core/branches/TURBINE_2_3_BRANCH/src/test/org/apache/turbine/util/parser/BaseValueParserTest.java Fri Sep  9 08:35:52 2005
@@ -483,11 +483,17 @@
 
         assertEquals("Wrong string value", testValue, bvp.getString("foo"));
 
-        bvp.remove("foo");
+        assertNotNull(bvp.remove("foo"));
 
         assertEquals("Wrong number of keys", 0, bvp.keySet().size());
 
         assertNull(bvp.getString("foo"));
+
+        // Test non-existing key
+        assertNull(bvp.remove("baz"));
+
+        // Test removing null value
+        assertNull(bvp.remove(null));
     }
         
     public void testRemoveArray()
@@ -1169,6 +1175,96 @@
 
         assertEquals("Wrong number of keys", 1, bvp.keySet().size());
 
+    }
+
+    public void testAddNullArrays()
+    {
+        String [] res = null;
+
+        BaseValueParser bvp = new BaseValueParser();
+        assertEquals("Wrong number of keys", 0, bvp.keySet().size());
+
+        bvp.add("foo", new String [] { "foo", "bar" });
+        res = bvp.getStrings("foo");
+        assertEquals("Wrong number of keys", 1, bvp.keySet().size());
+        assertEquals("Wrong number of values", 2, res.length);
+
+        // null value should not change contents
+        bvp.add("foo", (String) null);
+        res = bvp.getStrings("foo");
+        assertEquals("Wrong number of keys", 1, bvp.keySet().size());
+        assertEquals("Wrong number of values", 2, res.length);
+
+        // null value should not change contents
+        bvp.add("foo", (String []) null);
+        res = bvp.getStrings("foo");
+        assertEquals("Wrong number of keys", 1, bvp.keySet().size());
+        assertEquals("Wrong number of values", 2, res.length);
+
+        // empty String array should not change contents
+        bvp.add("foo", new String [0]);
+        res = bvp.getStrings("foo");
+        assertEquals("Wrong number of keys", 1, bvp.keySet().size());
+        assertEquals("Wrong number of values", 2, res.length);
+
+        // String array with null value should not change contents
+        bvp.add("foo", new String [] { null });
+        res = bvp.getStrings("foo");
+        assertEquals("Wrong number of keys", 1, bvp.keySet().size());
+        assertEquals("Wrong number of values", 2, res.length);
+
+        // String array with null value should only add non-null values
+        bvp.add("foo", new String [] { "bla", null, "foo" });
+        res = bvp.getStrings("foo");
+        assertEquals("Wrong number of keys", 1, bvp.keySet().size());
+        assertEquals("Wrong number of values", 4, res.length);
+    }
+
+    public void testNonExistingResults()
+    {
+        BaseValueParser bvp = new BaseValueParser();
+        assertEquals("Wrong number of keys", 0, bvp.keySet().size());
+
+        assertEquals("Wrong value for non existing key", 0.0, bvp.getDouble("foo"), 0.001);
+        assertNull(bvp.getDoubles("foo"));
+        assertNull(bvp.getDoubleObject("foo"));
+        assertNull(bvp.getDoubleObjects("foo"));
+
+        assertEquals("Wrong number of keys", 0, bvp.keySet().size());
+
+        assertNull(bvp.getString("foo"));
+        assertNull(bvp.getStrings("foo"));
+
+        assertEquals("Wrong value for non existing key", 0.0f, bvp.getFloat("foo"), 0.001);
+        assertNull(bvp.getFloats("foo"));
+        assertNull(bvp.getFloatObject("foo"));
+        assertNull(bvp.getFloatObjects("foo"));
+
+        assertEquals("Wrong number of keys", 0, bvp.keySet().size());
+
+        assertEquals("Wrong value for non existing key", 0.0, bvp.getBigDecimal("foo").doubleValue(), 0.001);
+        assertNull(bvp.getBigDecimals("foo"));
+
+        assertEquals("Wrong number of keys", 0, bvp.keySet().size());
+
+        assertEquals("Wrong value for non existing key", 0, bvp.getInt("foo"));
+        assertNull(bvp.getInts("foo"));
+        assertNull(bvp.getIntObject("foo"));
+        assertNull(bvp.getIntObjects("foo"));
+
+        assertEquals("Wrong number of keys", 0, bvp.keySet().size());
+
+        assertEquals("Wrong value for non existing key", 0, bvp.getLong("foo"));
+        assertNull(bvp.getLongs("foo"));
+        assertNull(bvp.getLongObject("foo"));
+        assertNull(bvp.getLongObjects("foo"));
+
+        assertEquals("Wrong number of keys", 0, bvp.keySet().size());
+
+        assertEquals("Wrong value for non existing key", 0, bvp.getByte("foo"));
+        assertNull(bvp.getByteObject("foo"));
+
+        assertEquals("Wrong number of keys", 0, bvp.keySet().size());
     }
 }
 



---------------------------------------------------------------------
To unsubscribe, e-mail: turbine-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: turbine-dev-help@jakarta.apache.org


Re: svn commit: r279799 - in /jakarta/turbine/core/branches/TURBINE_2_3_BRANCH/src:

Posted by "Henning P. Schmiedehausen" <hp...@intermeta.de>.
Scott Eade <se...@backstagetech.com.au> writes:

>I can't say I like all of the new warnings produced by this change.  For
>example if I have:

>    int resultsPerPage = data.getParameters().getInt("resultsPerPage", 10);

>and "resultsPerPage" is not in the parameters a WARN is now logged:

>    Parameter (resultsPerPage) with value of (null) could not be
>converted to a Integer

>Surely if a default is provided a warning should not be logged.

You are right. I will look into this.

	Best regards
		Henning

-- 
Dipl.-Inf. (Univ.) Henning P. Schmiedehausen          INTERMETA GmbH
hps@intermeta.de        +49 9131 50 654 0   http://www.intermeta.de/

RedHat Certified Engineer -- Jakarta Turbine Development  -- hero for hire
   Linux, Java, perl, Solaris -- Consulting, Training, Development

		      4 - 8 - 15 - 16 - 23 - 42

---------------------------------------------------------------------
To unsubscribe, e-mail: turbine-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: turbine-dev-help@jakarta.apache.org


Re: svn commit: r279799 - in /jakarta/turbine/core/branches/TURBINE_2_3_BRANCH/src: java/org/apache/turbine/util/parser/BaseValueParser.java test/org/apache/turbine/util/parser/BaseValueParserTest.java

Posted by Scott Eade <se...@backstagetech.com.au>.
henning@apache.org wrote:

>Author: henning
>Date: Fri Sep  9 08:35:52 2005
>New Revision: 279799
>
>URL: http://svn.apache.org/viewcvs?rev=279799&view=rev
>Log:
>Clean up the internal parameter management, make sure that
>null values never hit the parameters array.
>
>Remove a number of StringUtils.isEmpty calls, the converter methods
>will throw NPEs if problems arise; catch these and treat like
>NumberFormatExceptions
>
>Ruthlessly exploit the existence of ArrayUtils.add() in c-c to
>simplify the add(key, string) method.
>
>Replace getStrings() calls with getParam, making sure that overriding
>getStrings will not mess with our internal parameter management.
>
>Make sure that all getXXX methods behave similar with non-existing
>keys. Put an Unit test on this.
>
>Don't treat the String "null" as null value. This should fix a number
>of complaints from the users list. This is no longer necessary when
>the contents of the parameters map is carefully controlled to never
>contain null values.
>
>Clean up toString() to actually work not only on BaseValueParser but
>also on derived classes. ArrayIterators rule!
>
>
>
>Modified:
>    jakarta/turbine/core/branches/TURBINE_2_3_BRANCH/src/java/org/apache/turbine/util/parser/BaseValueParser.java
>    jakarta/turbine/core/branches/TURBINE_2_3_BRANCH/src/test/org/apache/turbine/util/parser/BaseValueParserTest.java
>
>Modified: jakarta/turbine/core/branches/TURBINE_2_3_BRANCH/src/java/org/apache/turbine/util/parser/BaseValueParser.java
>URL: http://svn.apache.org/viewcvs/jakarta/turbine/core/branches/TURBINE_2_3_BRANCH/src/java/org/apache/turbine/util/parser/BaseValueParser.java?rev=279799&r1=279798&r2=279799&view=diff
>  
>
<snip />

I can't say I like all of the new warnings produced by this change.  For
example if I have:

    int resultsPerPage = data.getParameters().getInt("resultsPerPage", 10);

and "resultsPerPage" is not in the parameters a WARN is now logged:

    Parameter (resultsPerPage) with value of (null) could not be
converted to a Integer

Surely if a default is provided a warning should not be logged.

Scott

-- 
Scott Eade
Backstage Technologies Pty. Ltd.
http://www.backstagetech.com.au


---------------------------------------------------------------------
To unsubscribe, e-mail: turbine-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: turbine-dev-help@jakarta.apache.org