dev@javaserverfaces.java.net

[REVIEW] Fix for issue 243 and minor perf enhancement

From: Ryan Lubke <Ryan.Lubke_at_Sun.COM>
Date: Tue, 21 Feb 2006 11:39:18 -0800


 - Fix for issue 243 (needs to be backported to JSF_1_1_ROLLING)
 - Minor performance enhancement


SECTION: Modified Files
----------------------------
A jsf-ri/src/com/sun/faces/io/FastStringWriter.java
 - Add StringWriter backed by StringBuilder.
   Simple tests show a modest improvement doing so.

M jsf-ri/src/com/sun/faces/application/ViewHandlerImpl.java
M jsf-ri/src/com/sun/faces/util/DebugUtil.java
 - make use of FastStringWriter

M jsf-ri/src/com/sun/faces/config/ManagedBeanFactoryImpl.java
 - (Issue 243) follow JSP spec for converting String to numeric types.
   If String value has zero length, then the numeric value
   should be 0.
 - leverage varargs in message calls

M jsf-ri/test/com/sun/faces/config/TestManagedBeanFactory.java
 - Update test to validate the above ManagedBeanFactoryImpl change


SECTION: Diffs
----------------------------
Index: jsf-ri/src/com/sun/faces/application/ViewHandlerImpl.java
===================================================================
RCS file: /cvs/javaserverfaces-sources/jsf-ri/src/com/sun/faces/application/ViewHandlerImpl.java,v
retrieving revision 1.65
diff -u -r1.65 ViewHandlerImpl.java
--- jsf-ri/src/com/sun/faces/application/ViewHandlerImpl.java 10 Feb 2006 22:29:01 -0000 1.65
+++ jsf-ri/src/com/sun/faces/application/ViewHandlerImpl.java 21 Feb 2006 19:25:32 -0000
@@ -33,14 +33,6 @@
 
 package com.sun.faces.application;
 
-import java.io.IOException;
-import java.io.StringWriter;
-import java.util.Iterator;
-import java.util.Locale;
-import java.util.Map;
-import java.util.logging.Level;
-import java.util.logging.Logger;
-
 import javax.faces.FacesException;
 import javax.faces.FactoryFinder;
 import javax.faces.application.StateManager;
@@ -59,9 +51,18 @@
 import javax.servlet.http.HttpServletResponse;
 import javax.servlet.jsp.jstl.core.Config;
 
+import java.io.IOException;
+import java.io.Writer;
+import java.util.Iterator;
+import java.util.Locale;
+import java.util.Map;
+import java.util.logging.Level;
+import java.util.logging.Logger;
+
 import com.sun.faces.RIConstants;
-import com.sun.faces.util.Util;
+import com.sun.faces.io.FastStringWriter;
 import com.sun.faces.util.MessageUtils;
+import com.sun.faces.util.Util;
 
 /**
  * <B>ViewHandlerImpl</B> is the default implementation class for ViewHandler.
@@ -151,7 +152,7 @@
         ServletResponse response = (ServletResponse) extContext.getResponse();
         
         ResponseWriter oldWriter = context.getResponseWriter();
- StringWriter strWriter = new StringWriter();
+ Writer strWriter = new FastStringWriter(2048);
         ResponseWriter newWriter = null;
         if (null != oldWriter) {
             newWriter = oldWriter.cloneWithWriter(strWriter);
@@ -177,7 +178,7 @@
         }
         context.setResponseWriter(responseWriter);
         
- String bodyContent = strWriter.getBuffer().toString();
+ String bodyContent = strWriter.toString();
         replaceMarkers(bodyContent, context);
         
         if (null != oldWriter) {
Index: jsf-ri/src/com/sun/faces/config/ManagedBeanFactoryImpl.java
===================================================================
RCS file: /cvs/javaserverfaces-sources/jsf-ri/src/com/sun/faces/config/ManagedBeanFactoryImpl.java,v
retrieving revision 1.6
diff -u -r1.6 ManagedBeanFactoryImpl.java
--- jsf-ri/src/com/sun/faces/config/ManagedBeanFactoryImpl.java 14 Feb 2006 19:59:42 -0000 1.6
+++ jsf-ri/src/com/sun/faces/config/ManagedBeanFactoryImpl.java 21 Feb 2006 19:25:32 -0000
@@ -186,11 +186,11 @@
         Class<?> managedBeanClass = this.getManagedBeanClass();
         if (managedBeanClass != null) {
             postConstructMethods =
- this.getMethodsWithAnnotation(managedBeanClass,
- Annotations.POST_CONSTRUCT);
+ getMethodsWithAnnotation(managedBeanClass,
+ Annotations.POST_CONSTRUCT);
             preDestroyMethods =
- this.getMethodsWithAnnotation(managedBeanClass,
- Annotations.PRE_DESTROY);
+ getMethodsWithAnnotation(managedBeanClass,
+ Annotations.PRE_DESTROY);
         }
     }
 
@@ -254,7 +254,7 @@
         return null;
     }
     
- public Class getManagedBeanClass() {
+ public final Class getManagedBeanClass() {
         try {
             ClassLoader loader = Thread.currentThread().getContextClassLoader();
             if (loader == null) {
@@ -550,7 +550,7 @@
             strKey = null,
             strValue = null;
 
- if (null == mapEntries || 0 == valuesFromConfig.length) {
+ if (valuesFromConfig.length == 0) {
             if (LOGGER.isLoggable(Level.FINE)) {
                 LOGGER.fine("null or zero length array");
             }
@@ -758,8 +758,8 @@
                 !java.util.List.class.isAssignableFrom(propertyType)) {
                 throw new FacesException(MessageUtils.getExceptionMessageString(
                     MessageUtils.MANAGED_BEAN_CANNOT_SET_LIST_ARRAY_PROPERTY_ID,
- new Object[] { propertyName,
- managedBean.getManagedBeanName() }));
+ propertyName,
+ managedBean.getManagedBeanName()));
             }
         }
 
@@ -782,9 +782,9 @@
                 if (!(result instanceof List)) {
                     // throw an exception
                     throw new FacesException(MessageUtils.getExceptionMessageString(
- MessageUtils.MANAGED_BEAN_EXISTING_VALUE_NOT_LIST_ID,
- new Object[] { propertyName,
- managedBean.getManagedBeanName() }));
+ MessageUtils.MANAGED_BEAN_EXISTING_VALUE_NOT_LIST_ID,
+ propertyName,
+ managedBean.getManagedBeanName()));
                 }
                 valuesForBean = (List<Object>) result;
             }
@@ -890,8 +890,8 @@
             !java.util.Map.class.isAssignableFrom(propertyType)) {
             throw new FacesException(MessageUtils.getExceptionMessageString(
                     MessageUtils.MANAGED_BEAN_CANNOT_SET_MAP_PROPERTY_ID,
- new Object[] { propertyName,
- managedBean.getManagedBeanName() }));
+ propertyName,
+ managedBean.getManagedBeanName()));
         }
 
 
@@ -942,44 +942,58 @@
 
     private Object getConvertedValueConsideringPrimitives(Object value,
                                                           Class valueType)
- throws FacesException {
+ throws FacesException {
         if (null != value && null != valueType) {
+ String valueString = value.toString();
             if (valueType.isEnum()) {
- value = Enum.valueOf(valueType, value.toString());
+ value = Enum.valueOf(valueType, valueString);
             }
             else if (valueType == Boolean.TYPE ||
                 valueType == java.lang.Boolean.class) {
- value = value.toString().toLowerCase().equals("true")
- ? Boolean.TRUE : Boolean.FALSE;
+ value = Boolean.valueOf(valueString);
             } else if (valueType == Byte.TYPE ||
                 valueType == java.lang.Byte.class) {
- value = new Byte(value.toString());
+ value = (valueString.length() == 0
+ ? 0
+ : Byte.valueOf(valueString));
             } else if (valueType == Double.TYPE ||
- valueType == java.lang.Double.class) {
- value = new Double(value.toString());
+ valueType == java.lang.Double.class) {
+ value = (valueString.length() == 0
+ ? 0
+ : Double.valueOf(valueString));
             } else if (valueType == Float.TYPE ||
- valueType == java.lang.Float.class) {
- value = new Float(value.toString());
+ valueType == java.lang.Float.class) {
+ value = (valueString.length() == 0
+ ? 0
+ : Float.valueOf(valueString));
             } else if (valueType == Integer.TYPE ||
- valueType == java.lang.Integer.class) {
- value = new Integer(value.toString());
+ valueType == java.lang.Integer.class) {
+ value = (valueString.length() == 0
+ ? 0
+ : Integer.valueOf(valueString));
             } else if (valueType == Character.TYPE ||
- valueType == java.lang.Character.class) {
- value = value.toString().charAt(0);
+ valueType == java.lang.Character.class) {
+ value = (valueString.length() == 0
+ ? 0
+ : valueString.charAt(0));
             } else if (valueType == Short.TYPE ||
- valueType == java.lang.Short.class) {
- value = new Short(value.toString());
+ valueType == java.lang.Short.class) {
+ value = (valueString.length() == 0
+ ? 0
+ : Short.valueOf(valueString));
             } else if (valueType == Long.TYPE ||
- valueType == java.lang.Long.class) {
- value = new Long(value.toString());
+ valueType == java.lang.Long.class) {
+ value = (valueString.length() == 0
+ ? 0
+ : Long.valueOf(valueString));
             } else if (valueType == String.class) {
             } else if (!valueType.isAssignableFrom(value.getClass())) {
                 throw new FacesException(MessageUtils.getExceptionMessageString(
- MessageUtils.MANAGED_BEAN_TYPE_CONVERSION_ERROR_ID,
- new Object[] { value.toString(),
- value.getClass(),
- valueType,
- managedBean.getManagedBeanName()}));
+ MessageUtils.MANAGED_BEAN_TYPE_CONVERSION_ERROR_ID,
+ value.toString(),
+ value.getClass(),
+ valueType,
+ managedBean.getManagedBeanName()));
 
             }
         }
@@ -1040,12 +1054,7 @@
         //if the managed bean's scope is "none" but the scope of the
         //referenced object is not "none", scope is invalid
         if (scope == Scope.NONE) {
- if (valueScope != Scope.NONE) {
- return false;
- }
- else {
- return true;
- }
+ return valueScope == Scope.NONE;
         }
        
         //if the managed bean's scope is "request" it is able to refer
@@ -1057,19 +1066,14 @@
         //if the managed bean's scope is "session" it is able to refer
         //to objects in other "session", "application", or "none" scopes
         if (scope == Scope.SESSION) {
- if (valueScope == Scope.REQUEST) {
- return false;
- }
- return true;
+ return valueScope != Scope.REQUEST;
         }
 
         //if the managed bean's scope is "application" it is able to refer
         //to objects in other "application", or "none" scopes
         if (scope == Scope.APPLICATION) {
- if (valueScope == Scope.REQUEST || valueScope == Scope.SESSION) {
- return false;
- }
- return true;
+ return !(valueScope == Scope.REQUEST
+ || valueScope == Scope.SESSION);
         }
 
         //the managed bean is required to be in either "request", "session",
@@ -1110,29 +1114,29 @@
             shortestScope = Scope.NONE.ordinal(),
             currentScope = Scope.NONE.ordinal();
         Scope
- scope = Scope.NONE,
+ lScope = Scope.NONE,
             result = Scope.NONE;
         
         // loop over the expressions
         while (iter.hasNext()) {
- scope = getScopeForSingleExpression((String)iter.next());
+ lScope = getScopeForSingleExpression((String)iter.next());
             // don't consider none
- if (null == scope || scope == Scope.NONE) {
+ if (null == lScope || lScope == Scope.NONE) {
                 continue;
             }
 
- currentScope = scope.ordinal();
+ currentScope = lScope.ordinal();
             
             // if we have no basis for comparison
             if (Scope.NONE.ordinal() == shortestScope) {
                 shortestScope = currentScope;
- result = scope;
+ result = lScope;
             }
             else {
                 // we have a basis for comparison
                 if (currentScope < shortestScope) {
                     shortestScope = currentScope;
- result = scope;
+ result = lScope;
                 }
             }
         }
Index: jsf-ri/src/com/sun/faces/util/DebugUtil.java
===================================================================
RCS file: /cvs/javaserverfaces-sources/jsf-ri/src/com/sun/faces/util/DebugUtil.java,v
retrieving revision 1.32
diff -u -r1.32 DebugUtil.java
--- jsf-ri/src/com/sun/faces/util/DebugUtil.java 11 Jan 2006 15:28:14 -0000 1.32
+++ jsf-ri/src/com/sun/faces/util/DebugUtil.java 21 Feb 2006 19:25:32 -0000
@@ -43,6 +43,7 @@
 import javax.faces.model.SelectItem;
 
 import com.sun.faces.renderkit.RenderKitUtils;
+import com.sun.faces.io.FastStringWriter;
 
 /**
  * <B>DebugUtil</B> is a class ...
@@ -138,7 +139,7 @@
      * logger.log(DebugUtil.printTree(root));
      */
     public static String printTree(UIComponent root) {
- StringWriter writer = new StringWriter();
+ Writer writer = new FastStringWriter(1024);
         printTree(root, writer);
         return writer.toString();
     }
@@ -242,7 +243,7 @@
      * logger.log(DebugUtil.printTree(root));
      */
     public static String printTree(TreeStructure root) {
- StringWriter writer = new StringWriter();
+ Writer writer = new FastStringWriter(1024);
         printTree(root, writer);
         return writer.toString();
     }
Index: jsf-ri/test/com/sun/faces/config/TestManagedBeanFactory.java
===================================================================
RCS file: /cvs/javaserverfaces-sources/jsf-ri/test/com/sun/faces/config/TestManagedBeanFactory.java,v
retrieving revision 1.28
diff -u -r1.28 TestManagedBeanFactory.java
--- jsf-ri/test/com/sun/faces/config/TestManagedBeanFactory.java 19 Oct 2005 19:51:30 -0000 1.28
+++ jsf-ri/test/com/sun/faces/config/TestManagedBeanFactory.java 21 Feb 2006 19:25:32 -0000
@@ -210,6 +210,62 @@
         //make sure scope is stored properly
         assertTrue(mbf.getScope() == Scope.SESSION);
     }
+
+ public void testSimpleNumericProperty() throws Exception {
+ // If a property value is "" ensure numeric properties
+ // are set to 0.
+ bean = new ManagedBeanBean();
+ bean.setManagedBeanClass(beanName);
+ bean.setManagedBeanScope("session");
+
+ property = new ManagedPropertyBean();
+ property.setPropertyName("byteProp");
+ property.setValue("");
+ bean.addManagedProperty(property);
+
+ property = new ManagedPropertyBean();
+ property.setPropertyName("charProp");
+ property.setValue("");
+ bean.addManagedProperty(property);
+
+ property = new ManagedPropertyBean();
+ property.setPropertyName("doubleProp");
+ property.setValue("");
+ bean.addManagedProperty(property);
+
+ property = new ManagedPropertyBean();
+ property.setPropertyName("floatProp");
+ property.setValue("");
+ bean.addManagedProperty(property);
+
+ property = new ManagedPropertyBean();
+ property.setPropertyName("intProp");
+ property.setValue("");
+ bean.addManagedProperty(property);
+
+ property = new ManagedPropertyBean();
+ property.setPropertyName("longProp");
+ property.setValue("");
+ bean.addManagedProperty(property);
+
+ property = new ManagedPropertyBean();
+ property.setPropertyName("shortProp");
+ property.setValue("");
+ bean.addManagedProperty(property);
+
+ mbf = new ManagedBeanFactoryImpl(bean);
+
+ assertNotNull(testBean = (TestBean) mbf.newInstance(getFacesContext()));
+
+ mbf = new ManagedBeanFactoryImpl(bean);
+ assertTrue(testBean.getByteProp() == 0);
+ assertTrue(testBean.getCharProp() == 0);
+ assertTrue(testBean.getDoubleProp() == 0);
+ assertTrue(testBean.getFloatProp() == 0);
+ assertTrue(testBean.getIntProp() == 0);
+ assertTrue(testBean.getLongProp() == 0);
+ assertTrue(testBean.getShortProp() == 0);
+ }
 
 
     public void testIndexProperty() throws Exception {