[jboss-svn-commits] JBoss Portal SVN: r5208 - in trunk: common/src/main/org/jboss/portal/common/util common/src/main/org/jboss/portal/test/common portlet/src/main/org/jboss/portal/portlet portlet/src/main/org/jboss/portal/portlet/impl/jsr168 portlet/src/main/org/jboss/portal/portlet/state portlet/src/main/org/jboss/portal/test/portlet portlet/src/main/org/jboss/portal/test/portlet/jsr168/tck/portleturl server/src/main/org/jboss/portal/server/deployment

jboss-svn-commits at lists.jboss.org jboss-svn-commits at lists.jboss.org
Fri Sep 15 13:13:03 EDT 2006


Author: julien at jboss.com
Date: 2006-09-15 13:12:48 -0400 (Fri, 15 Sep 2006)
New Revision: 5208

Added:
   trunk/portlet/src/main/org/jboss/portal/portlet/impl/jsr168/PortletUtils.java
Modified:
   trunk/common/src/main/org/jboss/portal/common/util/ParameterMap.java
   trunk/common/src/main/org/jboss/portal/common/util/TypedMap.java
   trunk/common/src/main/org/jboss/portal/test/common/ParameterMapTestCase.java
   trunk/portlet/src/main/org/jboss/portal/portlet/Parameters.java
   trunk/portlet/src/main/org/jboss/portal/portlet/impl/jsr168/ActionResponseImpl.java
   trunk/portlet/src/main/org/jboss/portal/portlet/impl/jsr168/PortletURLImpl.java
   trunk/portlet/src/main/org/jboss/portal/portlet/state/AbstractPropertyMap.java
   trunk/portlet/src/main/org/jboss/portal/test/portlet/ParametersTestCase.java
   trunk/portlet/src/main/org/jboss/portal/test/portlet/jsr168/tck/portleturl/PortletUrlSequenceBuilder.java
   trunk/server/src/main/org/jboss/portal/server/deployment/PortalWebAppFactory.java
Log:
- more consistent handling of key/value validity in TypedMap with respect to the Map contract
- adapted that contract for the subclasses of TypedMap

Modified: trunk/common/src/main/org/jboss/portal/common/util/ParameterMap.java
===================================================================
--- trunk/common/src/main/org/jboss/portal/common/util/ParameterMap.java	2006-09-15 09:14:04 UTC (rev 5207)
+++ trunk/common/src/main/org/jboss/portal/common/util/ParameterMap.java	2006-09-15 17:12:48 UTC (rev 5208)
@@ -30,27 +30,42 @@
 public abstract class ParameterMap extends TypedMap
 {
 
-   protected void assertKeyValidity(Object value)
+   /**
+    * Only accept non null string objects.
+    *
+    * @throws NullPointerException if the value is null
+    * @throws ClassCastException if the value is not an instance of string
+    */
+   protected void assertKeyValidity(Object value) throws NullPointerException, ClassCastException
    {
       if (value == null)
       {
-         throw new IllegalArgumentException("Key must not be null");
+         throw new NullPointerException("Key must not be null");
       }
       if (value instanceof String == false)
       {
-         throw new IllegalArgumentException("Key should be a String");
+         throw new ClassCastException("Key should be a String");
       }
    }
 
-   protected Object getInternalValue(Object value)
+   /**
+    * Only check are made to the value. The only valid values accepted
+    * are string arrays with non zero length and containing non null
+    * values.
+    *
+    * @throws NullPointerException if the value is null
+    * @throws ClassCastException if the value type is not an array of string
+    * @throws IllegalArgumentException if the array length is zero or one of the array value is null
+    */
+   protected Object getInternalValue(Object value) throws NullPointerException, ClassCastException, IllegalArgumentException
    {
       if (value == null)
       {
-         throw new IllegalArgumentException("Value must not be null");
+         throw new NullPointerException("Value must not be null");
       }
       if (value instanceof String[] == false)
       {
-         throw new IllegalArgumentException("Value should be a String[]");
+         throw new ClassCastException("Value should be a String[]");
       }
       String[] strings = (String[])value;
       if (strings.length == 0)

Modified: trunk/common/src/main/org/jboss/portal/common/util/TypedMap.java
===================================================================
--- trunk/common/src/main/org/jboss/portal/common/util/TypedMap.java	2006-09-15 09:14:04 UTC (rev 5207)
+++ trunk/common/src/main/org/jboss/portal/common/util/TypedMap.java	2006-09-15 17:12:48 UTC (rev 5208)
@@ -39,21 +39,24 @@
    protected abstract Map getDelegate();
 
    /**
-    * Throw an IllegalArgumentException if the key is not valid.
+    * @throws ClassCastException if the key is of an inappropriate type for this map
+    * @throws NullPointerException if the key is null and this map does not not permit null keys
     */
-   protected abstract void assertKeyValidity(Object key) throws IllegalArgumentException;
+   protected abstract void assertKeyValidity(Object key) throws ClassCastException, NullPointerException;
 
    /**
-    * Unwrap the value to the the internal value.
+    * Unwrap the value to the the internal value that will be stored in the map.
     *
     * @param value the value to unwrap
     * @return the unwrapped value
-    * @throws IllegalArgumentException if the type is not correct.
+    * @throws ClassCastException if the class of the specified value prevents it from being stored in this map
+    * @throws IllegalArgumentException if some aspect of this key prevents it from being stored in this map
+    * @throws NullPointerException this map does not permit null values, and the specified value is null
     */
-   protected abstract Object getInternalValue(Object value) throws IllegalArgumentException;
+   protected abstract Object getInternalValue(Object value) throws IllegalArgumentException, ClassCastException, NullPointerException;
 
    /**
-    * Wrap the internal value to its external representation.
+    * Wrap the internal value into its external representation.
     */
    protected abstract Object getExternalValue(Object value);
 

Modified: trunk/common/src/main/org/jboss/portal/test/common/ParameterMapTestCase.java
===================================================================
--- trunk/common/src/main/org/jboss/portal/test/common/ParameterMapTestCase.java	2006-09-15 09:14:04 UTC (rev 5207)
+++ trunk/common/src/main/org/jboss/portal/test/common/ParameterMapTestCase.java	2006-09-15 17:12:48 UTC (rev 5208)
@@ -75,7 +75,7 @@
          pm.put(new Object(), new String[]{"bar"});
          fail();
       }
-      catch (IllegalArgumentException expected)
+      catch (ClassCastException expected)
       {
       }
       try
@@ -83,6 +83,14 @@
          pm.put("foo", new Object[]{});
          fail();
       }
+      catch (ClassCastException expected)
+      {
+      }
+      try
+      {
+         pm.put("foo", new String[]{});
+         fail();
+      }
       catch (IllegalArgumentException expected)
       {
       }
@@ -109,9 +117,25 @@
       Map.Entry entry = (Map.Entry)tmp;
       try
       {
+         entry.setValue(null);
+         fail();
+      }
+      catch (NullPointerException expected)
+      {
+      }
+      try
+      {
          entry.setValue(new Object[]{});
          fail();
       }
+      catch (ClassCastException expected)
+      {
+      }
+      try
+      {
+         entry.setValue(new String[]{});
+         fail();
+      }
       catch (IllegalArgumentException expected)
       {
       }

Modified: trunk/portlet/src/main/org/jboss/portal/portlet/Parameters.java
===================================================================
--- trunk/portlet/src/main/org/jboss/portal/portlet/Parameters.java	2006-09-15 09:14:04 UTC (rev 5207)
+++ trunk/portlet/src/main/org/jboss/portal/portlet/Parameters.java	2006-09-15 17:12:48 UTC (rev 5208)
@@ -36,7 +36,7 @@
  * @author <a href="mailto:julien at jboss.org">Julien Viet</a>
  * @version $Revision$
  */
-public class Parameters extends ParameterMap implements Serializable
+public final class Parameters extends ParameterMap implements Serializable
 {
 
    /** The serialVersionUID */
@@ -107,6 +107,10 @@
     */
    public String[] getValues(String name) throws IllegalArgumentException
    {
+      if (name == null)
+      {
+         throw new IllegalArgumentException("No null name");
+      }
       return (String[])get(name);
    }
 

Modified: trunk/portlet/src/main/org/jboss/portal/portlet/impl/jsr168/ActionResponseImpl.java
===================================================================
--- trunk/portlet/src/main/org/jboss/portal/portlet/impl/jsr168/ActionResponseImpl.java	2006-09-15 09:14:04 UTC (rev 5207)
+++ trunk/portlet/src/main/org/jboss/portal/portlet/impl/jsr168/ActionResponseImpl.java	2006-09-15 17:12:48 UTC (rev 5208)
@@ -153,6 +153,9 @@
 
    public void setRenderParameters(Map map)
    {
+      PortletUtils.checkRenderParameterMap(map);
+
+      //
       if (decision == WANT_NOTHING || decision == WANT_RENDER)
       {
          ((ParametersStateString)((RenderResult)result).getNavigationalState()).replace(map);
@@ -166,6 +169,9 @@
 
    public void setRenderParameter(String name, String value)
    {
+      PortletUtils.checkRenderParameter(name, value);
+
+      //
       if (decision == WANT_NOTHING || decision == WANT_RENDER)
       {
          ((ParametersStateString)((RenderResult)result).getNavigationalState()).setValue(name, value);
@@ -179,6 +185,9 @@
 
    public void setRenderParameter(String name, String[] values)
    {
+      PortletUtils.checkRenderParameter(name, values);
+
+      //
       if (decision == WANT_NOTHING || decision == WANT_RENDER)
       {
          ((ParametersStateString)((RenderResult)result).getNavigationalState()).setValues(name, values);

Modified: trunk/portlet/src/main/org/jboss/portal/portlet/impl/jsr168/PortletURLImpl.java
===================================================================
--- trunk/portlet/src/main/org/jboss/portal/portlet/impl/jsr168/PortletURLImpl.java	2006-09-15 09:14:04 UTC (rev 5207)
+++ trunk/portlet/src/main/org/jboss/portal/portlet/impl/jsr168/PortletURLImpl.java	2006-09-15 17:12:48 UTC (rev 5208)
@@ -95,28 +95,25 @@
     */
    public void setParameter(String name, String value)
    {
-      if (value == null)
-      {
-         throw new IllegalArgumentException("value cannot be null");
-      }
+      PortletUtils.checkRenderParameter(name, value);
+
+      //
       url.getInternalParameters().setValue(name, value);
    }
 
    public void setParameter(String name, String[] values)
    {
-      if (values == null)
-      {
-         throw new IllegalArgumentException("value cannot be null");
-      }
+      PortletUtils.checkRenderParameter(name, values);
+
+      //
       url.getInternalParameters().setValues(name, values);
    }
 
    public void setParameters(Map parameters)
    {
-      if (parameters == null)
-      {
-         throw new IllegalArgumentException("No map provided");
-      }
+      PortletUtils.checkRenderParameterMap(parameters);
+
+      //
       url.getInternalParameters().replace(parameters);
    }
 

Added: trunk/portlet/src/main/org/jboss/portal/portlet/impl/jsr168/PortletUtils.java
===================================================================
--- trunk/portlet/src/main/org/jboss/portal/portlet/impl/jsr168/PortletUtils.java	2006-09-15 09:14:04 UTC (rev 5207)
+++ trunk/portlet/src/main/org/jboss/portal/portlet/impl/jsr168/PortletUtils.java	2006-09-15 17:12:48 UTC (rev 5208)
@@ -0,0 +1,86 @@
+/*
+* JBoss, Home of Professional Open Source
+* Copyright 2005, JBoss Inc., and individual contributors as indicated
+* by the @authors tag. See the copyright.txt in the distribution for a
+* full listing of individual contributors.
+*
+* This is free software; you can redistribute it and/or modify it
+* under the terms of the GNU Lesser General Public License as
+* published by the Free Software Foundation; either version 2.1 of
+* the License, or (at your option) any later version.
+*
+* This software is distributed in the hope that it will be useful,
+* but WITHOUT ANY WARRANTY; without even the implied warranty of
+* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+* Lesser General Public License for more details.
+*
+* You should have received a copy of the GNU Lesser General Public
+* License along with this software; if not, write to the Free
+* Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
+* 02110-1301 USA, or see the FSF site: http://www.fsf.org.
+*/
+package org.jboss.portal.portlet.impl.jsr168;
+
+import java.util.Iterator;
+import java.util.Map;
+
+/**
+ * @author <a href="mailto:julien at jboss.org">Julien Viet</a>
+ * @version $Revision: 1.1 $
+ */
+public class PortletUtils
+{
+   public static void checkRenderParameterMap(Map map)
+   {
+      if (map == null)
+      {
+         throw new IllegalArgumentException("Map cannot be null");
+      }
+      for (Iterator i = map.entrySet().iterator(); i.hasNext();)
+      {
+         Map.Entry entry = (Map.Entry)i.next();
+         Object key = entry.getKey();
+         Object value = entry.getValue();
+         if (key == null)
+         {
+            throw new IllegalArgumentException("Key cannot be null");
+         }
+         if (key instanceof String == false)
+         {
+            throw new IllegalArgumentException("Key must be a String and not " + key.getClass());
+         }
+         if (value == null)
+         {
+            throw new IllegalArgumentException("Value cannot be null");
+         }
+         if (value instanceof String[] == false)
+         {
+            throw new IllegalArgumentException("Value must be a String[] and not " + value.getClass());
+         }
+      }
+   }
+
+   public static void checkRenderParameter(String name, String value)
+   {
+      if (name == null)
+      {
+         throw new IllegalArgumentException("Name cannot be null");
+      }
+      if (value == null)
+      {
+         throw new IllegalArgumentException("Value cannot be null");
+      }
+   }
+
+   public static void checkRenderParameter(String name, String[] values)
+   {
+      if (name == null)
+      {
+         throw new IllegalArgumentException("Name cannot be null");
+      }
+      if (values == null)
+      {
+         throw new IllegalArgumentException("Values cannot be null");
+      }
+   }
+}

Modified: trunk/portlet/src/main/org/jboss/portal/portlet/state/AbstractPropertyMap.java
===================================================================
--- trunk/portlet/src/main/org/jboss/portal/portlet/state/AbstractPropertyMap.java	2006-09-15 09:14:04 UTC (rev 5207)
+++ trunk/portlet/src/main/org/jboss/portal/portlet/state/AbstractPropertyMap.java	2006-09-15 17:12:48 UTC (rev 5208)
@@ -34,20 +34,41 @@
    /** The serialVersionUID */
    private static final long serialVersionUID = -5889928820592375820L;
 
+   /**
+    * Only accept non null string objects.
+    *
+    * @throws NullPointerException if the value is null
+    * @throws ClassCastException if the value is not an instance of string
+    */
    protected void assertKeyValidity(Object value)
    {
-      if (!(value instanceof String))
+      if (value == null)
       {
-         throw new IllegalArgumentException("Key should be a String");
+         throw new ClassCastException("Key must not be null");
       }
+      if (value instanceof String == false)
+      {
+         throw new ClassCastException("Key must be a String");
+      }
    }
 
+   /**
+    * Only check are made to the value. The only valid values accepted
+    * are non null instance of <code>org.jboss.portal.common.value.Value</code>.
+    *
+    * @throws NullPointerException if the value is null
+    * @throws ClassCastException if the value type is not an instance of <code>org.jboss.portal.common.value.Value</code>
+    */
    protected Object getInternalValue(Object value)
    {
-      if (!(value instanceof Value))
+      if (value == null)
       {
-         throw new IllegalArgumentException("Value should be a Value");
+         throw new NullPointerException("Value must not be null");
       }
+      if (value instanceof Value == false)
+      {
+         throw new ClassCastException("Value must be a Value");
+      }
       return value;
    }
 
@@ -65,7 +86,7 @@
       return (Value)get(key);
    }
 
-   public void setProperty(String key, Value value) throws IllegalArgumentException, UnsupportedOperationException
+   public void setProperty(String key, Value value) throws IllegalArgumentException
    {
       if (key == null)
       {

Modified: trunk/portlet/src/main/org/jboss/portal/test/portlet/ParametersTestCase.java
===================================================================
--- trunk/portlet/src/main/org/jboss/portal/test/portlet/ParametersTestCase.java	2006-09-15 09:14:04 UTC (rev 5207)
+++ trunk/portlet/src/main/org/jboss/portal/test/portlet/ParametersTestCase.java	2006-09-15 17:12:48 UTC (rev 5208)
@@ -61,7 +61,7 @@
          param.getValue(null);
          fail("Expected IllegalArgumentException");
       }
-      catch (IllegalArgumentException e)
+      catch (NullPointerException e)
       {
       }
    }
@@ -79,7 +79,7 @@
          param.setValue(null, "b");
          fail("Expected IllegalArgumentException");
       }
-      catch (IllegalArgumentException e)
+      catch (NullPointerException e)
       {
       }
    }
@@ -103,7 +103,7 @@
          param.remove(null);
          fail("Expected IllegalArgumentException");
       }
-      catch (IllegalArgumentException e)
+      catch (NullPointerException e)
       {
       }
    }
@@ -130,7 +130,7 @@
          param.setValues(null, new String[] { "a" });
          fail("Expected IllegalArgumentException");
       }
-      catch (IllegalArgumentException e)
+      catch (NullPointerException e)
       {
       }
    }
@@ -142,7 +142,7 @@
          param.setValues("a", null);
          fail("Expected IllegalArgumentException");
       }
-      catch (IllegalArgumentException e)
+      catch (NullPointerException e)
       {
       }
    }
@@ -189,9 +189,8 @@
       other.setValue("a", "b");
       other.setValues("c", new String[] { "d", "e" });
       param.replace(other);
-      assertEquals(param.getValue("a"), "b");
-      assertTrue(Arrays.equals(param.getValues("c"), new String[] {
-            "d", "e" }));
+      assertEquals("b", param.getValue("a"));
+      assertTrue(Arrays.equals(param.getValues("c"), new String[] {"d", "e" }));
    }
 
    public void testCopyConstructorWithNullParameters()
@@ -233,6 +232,7 @@
    public void testReplaceWithInvalidMap()
    {
       Map[] maps = buildInvalidMaps();
+      Class[] exceptionClasses = buildExceptionClasses();
       for (int i = 0; i < maps.length; i++)
       {
          try
@@ -241,8 +241,9 @@
             param.replace(map);
             fail("Expected IllegalArgumentException with map=" + map);
          }
-         catch (IllegalArgumentException e)
+         catch (Exception e)
          {
+            assertTrue(exceptionClasses[i].isAssignableFrom(e.getClass()));
          }
       }
    }
@@ -266,6 +267,7 @@
    public void testAppendWithInvalidMap()
    {
       Map[] maps = buildInvalidMaps();
+      Class[] exceptionClasses = buildExceptionClasses();
       for (int i = 0; i < maps.length; i++)
       {
          try
@@ -274,8 +276,9 @@
             param.append(map);
             fail("Expected IllegalArgumentException with map=" + map);
          }
-         catch (IllegalArgumentException e)
+         catch (Exception e)
          {
+            assertTrue(exceptionClasses[i].isAssignableFrom(e.getClass()));
          }
       }
    }
@@ -304,6 +307,17 @@
       assertNull(param.getValue("a"));
    }
 
+   public Class[] buildExceptionClasses()
+   {
+      return new Class[]
+      {
+         NullPointerException.class,
+         IllegalArgumentException.class,
+         IllegalArgumentException.class,
+         ClassCastException.class
+      };
+   }
+
    public Map[] buildInvalidMaps()
    {
       Map map1 = new HashMap();

Modified: trunk/portlet/src/main/org/jboss/portal/test/portlet/jsr168/tck/portleturl/PortletUrlSequenceBuilder.java
===================================================================
--- trunk/portlet/src/main/org/jboss/portal/test/portlet/jsr168/tck/portleturl/PortletUrlSequenceBuilder.java	2006-09-15 09:14:04 UTC (rev 5207)
+++ trunk/portlet/src/main/org/jboss/portal/test/portlet/jsr168/tck/portleturl/PortletUrlSequenceBuilder.java	2006-09-15 17:12:48 UTC (rev 5208)
@@ -133,9 +133,12 @@
          {
             PortletURL url = response.createActionURL();
 
-            Map map = new HashMap();
+            //
             url.setParameter("key3", "some strange value to overwrite");
             url.setParameter("key4", "some strange value to overwrite 2");
+
+            //
+            Map map = new HashMap();
             map.put("key3", new String[]{"k3value1"});
             map.put("key4", new String[]{"k4value1", "k4value2", "k4value3"});
             url.setParameters(map);

Modified: trunk/server/src/main/org/jboss/portal/server/deployment/PortalWebAppFactory.java
===================================================================
--- trunk/server/src/main/org/jboss/portal/server/deployment/PortalWebAppFactory.java	2006-09-15 09:14:04 UTC (rev 5207)
+++ trunk/server/src/main/org/jboss/portal/server/deployment/PortalWebAppFactory.java	2006-09-15 17:12:48 UTC (rev 5208)
@@ -60,7 +60,8 @@
     */
    public PortalWebApp create(WebApplication webApp) throws CannotCreatePortletWebAppException
    {
-      switch (getVersion())
+      int version = getVersion();
+      switch (version)
       {
          case TOMCAT4:
             return new PortalWebTomcat4App(webApp);
@@ -70,7 +71,7 @@
             // For now trying to build TC6 with TC5 web app
             return new PortalWebTomcat5App(webApp, server);
          default:
-            throw new CannotCreatePortletWebAppException("JBossWeb cannot handle it");
+            throw new CannotCreatePortletWebAppException("JBossWeb cannot handle it : " + version);
       }
    }
 
@@ -92,7 +93,7 @@
             {
                return TOMCAT6;
             }
-            else if (result.startsWith("Apache Tomcat/6"))
+            else if (result.startsWith("Apache Tomcat/5"))
             {
                return TOMCAT5;
             }




More information about the jboss-svn-commits mailing list