dev@javaserverfaces.java.net

[REVIEW] Proposed fix for 505266

From: Ryan Lubke <Ryan.Lubke_at_Sun.COM>
Date: Mon, 02 Aug 2004 15:12:20 -0400

Correct a small spec compliance issue in ManagedBeanFactory.
If a property returns a List or a Map, when the getter is initially
called, do not call the setter once the managed properties
are added.

Unit and TCK tests pass.

SECTION: Modified Files
-------------------------------------
M src/com/sun/faces/config/ManagedBeanFactory.java
  - minor cleanup (imports, unused vars)
  - moved setProperty logic from case into respective methods
    that set the Map or Index properties since these methods
    contained the info needed to determine whether the setter
    was needed or not.


M test/com/sun/faces/TestBean.java
M test/com/sun/faces/config/TestManagedBeanFactory.java
  - updated tests to ensure the fix above worked.



SECTION: Diffs
-------------------------------------
Index: src/com/sun/faces/config/ManagedBeanFactory.java
===================================================================
RCS file:
/cvs/javaserverfaces-sources/jsf-ri/src/com/sun/faces/config/ManagedBeanFactory.java,v
retrieving revision 1.23
diff -u -r1.23 ManagedBeanFactory.java
--- src/com/sun/faces/config/ManagedBeanFactory.java 10 May 2004
19:56:04 -0000 1.23
+++ src/com/sun/faces/config/ManagedBeanFactory.java 2 Aug 2004
18:35:32 -0000
@@ -15,7 +15,6 @@
 import com.sun.faces.config.beans.ManagedPropertyBean;
 import com.sun.faces.config.beans.MapEntriesBean;
 import com.sun.faces.config.beans.MapEntryBean;
-import com.sun.faces.el.ValueBindingImpl;
 import com.sun.faces.util.Util;
 import org.apache.commons.beanutils.PropertyUtils;
 import org.apache.commons.logging.Log;
@@ -24,17 +23,17 @@
 import javax.faces.FacesException;
 import javax.faces.component.UIComponent;
 import javax.faces.context.FacesContext;
-import javax.faces.el.PropertyNotFoundException;
 import javax.faces.el.EvaluationException;
+import javax.faces.el.PropertyNotFoundException;
 import javax.faces.el.ReferenceSyntaxException;
 import javax.faces.el.ValueBinding;
 
 import java.lang.reflect.Array;
 import java.util.ArrayList;
-import java.util.List;
-import java.util.Map;
 import java.util.HashMap;
 import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
 
 /**
  * This class clreates a managed bean instance. It has a contract with
@@ -514,15 +513,10 @@
                 // and tries to set it into the bean.
                 switch (propertyType = getPropertyType(properties[i])) {
                     case TYPE_IS_LIST:
- value =
- getArrayOrListToSetIntoBean(bean,
properties[i]);
- PropertyUtils.setProperty(bean, propertyName,
- value);
+ setArrayOrListPropertiesIntoBean(bean,
properties[i]);
                         break;
                     case TYPE_IS_MAP:
- value = getMapToSetIntoBean(bean, properties[i]);
- PropertyUtils.setProperty(bean, propertyName,
- value);
+ setMapPropertiesIntoBean(bean,
properties[i]);
                         break;
                     case TYPE_IS_SIMPLE:
                         // if the config bean has no managed-property-class
@@ -619,24 +613,21 @@
      * the * List to array of the same type as the property and set the
      * property by * calling the setter method, or log an error if there
      * is no setter * method.</p></li>
- *
- * @return the array or list to store into the bean.
      */
 
- private Object getArrayOrListToSetIntoBean(Object bean,
- ManagedPropertyBean
property)
+ private void setArrayOrListPropertiesIntoBean(Object bean,
+ ManagedPropertyBean
property)
         throws Exception {
         Object result = null;
         boolean
             getterIsNull = true,
             getterIsArray = false;
         List
- valuesForBean = null,
- valuesFromConfig = null;
+ valuesForBean = null;
         Class
             valueType = java.lang.String.class,
             propertyType = null;
- Object value = null;
+
         String propertyName = property.getPropertyName();
 
         try {
@@ -741,7 +732,10 @@
             result = valuesForBean;
         }
 
- return result;
+ if (getterIsNull || getterIsArray) {
+ PropertyUtils.setProperty(bean, propertyName, result);
+ }
+
     }
 
 
@@ -766,14 +760,12 @@
      * property by calling the setter method, or log an error if there
      * is no setter method.</p></li>
      */
- private Map getMapToSetIntoBean(Object bean,
- ManagedPropertyBean property)
+ private void setMapPropertiesIntoBean(Object bean,
+ ManagedPropertyBean property)
         throws Exception {
         Map result = null;
         boolean getterIsNull = true;
         Class propertyType = null;
- List valuesFromConfig = null;
- Object value = null;
         String propertyName = property.getPropertyName();
 
         try {
@@ -806,7 +798,10 @@
 
         copyMapEntriesFromConfigToMap(property.getMapEntries(), result);
 
- return result;
+ if (getterIsNull) {
+ PropertyUtils.setProperty(bean, propertyName, result);
+ }
+
     }
 
 
Index: test/com/sun/faces/TestBean.java
===================================================================
RCS file:
/cvs/javaserverfaces-sources/jsf-ri/test/com/sun/faces/TestBean.java,v
retrieving revision 1.14
diff -u -r1.14 TestBean.java
--- test/com/sun/faces/TestBean.java 7 Apr 2004 17:22:13 -0000 1.14
+++ test/com/sun/faces/TestBean.java 2 Aug 2004 18:35:32 -0000
@@ -206,9 +206,10 @@
         imagePath = newImagePath;
     }
 
+ //
-------------------------------------------------------------------------
 
- protected ArrayList indexProperties = null;
-
+ protected ArrayList indexProperties = new ArrayList();
+ boolean listSetterCalled;
 
     public ArrayList getIndexProperties() {
         return indexProperties;
@@ -216,9 +217,35 @@
 
 
     public void setIndexProperties(ArrayList newIndexProperties) {
+ listSetterCalled = true;
         indexProperties = newIndexProperties;
     }
 
+ public boolean getListSetterCalled() {
+ return listSetterCalled;
+ }
+
+ //
-------------------------------------------------------------------------
+
+ protected ArrayList indexPropertiesNull;
+ boolean listNullSetterCalled;
+
+ public ArrayList getIndexPropertiesNull() {
+ return indexPropertiesNull;
+ }
+
+
+ public void setIndexPropertiesNull(ArrayList newIndexPropertiesNull) {
+ listNullSetterCalled = true;
+ this.indexPropertiesNull = newIndexPropertiesNull;
+ }
+
+ public boolean getListNullSetterCalled() {
+ return listNullSetterCalled;
+ }
+
+ //
-------------------------------------------------------------------------
+
 
     protected ArrayList indexIntegerProperties = null;
 
@@ -233,9 +260,10 @@
 
     }
 
+ //
-------------------------------------------------------------------------
 
     protected Map mapProperty = new HashMap();
-
+ private boolean mapPropertySetterCalled = false;
 
     public Map getMapProperty() {
         return mapProperty;
@@ -243,8 +271,33 @@
 
 
     public void setMapProperty(Map mapProperty) {
+ mapPropertySetterCalled = true;
         this.mapProperty = mapProperty;
     }
+
+ public boolean getMapPropertySetterCalled() {
+ return mapPropertySetterCalled;
+ }
+
+ //
-------------------------------------------------------------------------
+
+ protected Map mapPropertyNull;
+ private boolean mapPropertyNullSetterCalled = false;
+
+ public Map getMapPropertyNull() {
+ return mapPropertyNull;
+ }
+
+ public void setMapPropertyNull(Map mapPropertyNull) {
+ mapPropertyNullSetterCalled = true;
+ this.mapPropertyNull = mapPropertyNull;
+ }
+
+ public boolean getMapPropertyNullSetterCalled() {
+ return mapPropertyNullSetterCalled;
+ }
+
+ //
-------------------------------------------------------------------------
 
 
     protected String modelLabel = "model label";
Index: 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.22
diff -u -r1.22 TestManagedBeanFactory.java
--- test/com/sun/faces/config/TestManagedBeanFactory.java 10 May 2004
19:56:15 -0000 1.22
+++ test/com/sun/faces/config/TestManagedBeanFactory.java 2 Aug 2004
18:35:32 -0000
@@ -207,7 +207,12 @@
         listEntries.addValue("bar");
         property.setListEntries(listEntries);
 
+ ManagedPropertyBean property2 = new ManagedPropertyBean();
+ property2.setPropertyName("indexPropertiesNull");
+ property2.setListEntries(listEntries);
+
         bean.addManagedProperty(property);
+ bean.addManagedProperty(property2);
 
         mbf = new ManagedBeanFactory(bean);
 
@@ -219,6 +224,12 @@
         assertTrue(properties.get(5).equals("foo"));
         assertTrue(properties.get(6).equals("bar"));
 
+ // setter shouldn't be called if bean getter returns List
+ assertTrue(!testBean.getListSetterCalled());
+
+ // setter should be called if bean getter returned null
+ assertTrue(testBean.getListNullSetterCalled());
+
         //make sure scope is stored properly
         assertTrue(mbf.getScope().equals("session"));
     }
@@ -240,7 +251,12 @@
         property.setPropertyName("mapProperty");
         property.setMapEntries(mapEntries);
 
+ ManagedPropertyBean property2 = new ManagedPropertyBean();
+ property2.setPropertyName("mapPropertyNull");
+ property2.setMapEntries(mapEntries);
+
         bean.addManagedProperty(property);
+ bean.addManagedProperty(property2);
         mbf = new ManagedBeanFactory(bean);
 
         //testing with a property set
@@ -250,6 +266,12 @@
         HashMap mapProperty = (HashMap)
             testBean.getMapProperty();
         assertTrue(((String) mapProperty.get("name")).equals("Justyna"));
+
+ // setter shouldn't be called if bean getter returns Map
+ assertTrue(!testBean.getMapPropertySetterCalled());
+
+ // setter should be called if bean getter returned null
+ assertTrue(testBean.getMapPropertyNullSetterCalled());
 
         //make sure scope is stored properly
         assertTrue(mbf.getScope().equals("session"));


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_javaserverfaces.dev.java.net
For additional commands, e-mail: dev-help_at_javaserverfaces.dev.java.net