dev@javaserverfaces.java.net

[REVIEW] Back ports to JSF_1_1_ROLLING

From: Ryan Lubke <Ryan.Lubke_at_Sun.COM>
Date: Wed, 25 Jan 2006 15:31:33 -0800

Back port of issue 222, and partially of 125.

See attached change bundle.



Backport of issue 222 (https://javaserverfaces.dev.java.net/issues/show_bug.cgi?id=222)
Partial backport of issue 125.



SECTION: Modified Files
----------------------------
M jsf-api/src/javax/faces/component/UIComponentBase.java
  - use getChildCount() where possible to avoid
    creating List.
  - Still have the issue with getFacets() creating a new
    Map, but avoid creating an Iterator by checking Map
    size before iteration.
  - port updated implementation of getFacetsAndChildren()
  

M jsf-api/src/javax/faces/render/Renderer.java
  - use getChildCount()

M jsf-api/test/javax/faces/component/UIComponentBaseTestCase.java
  - add test case

M jsf-ri/src/com/sun/faces/application/StateManagerImpl.java
  - use getFacetsAndChildren()


SECTION: Diffs
----------------------------
Index: jsf-api/src/javax/faces/component/UIComponentBase.java
===================================================================
RCS file: /cvs/javaserverfaces-sources/jsf-api/src/javax/faces/component/UIComponentBase.java,v
retrieving revision 1.101.26.2
diff -u -r1.101.26.2 UIComponentBase.java
--- jsf-api/src/javax/faces/component/UIComponentBase.java 6 Jan 2006 19:59:19 -0000 1.101.26.2
+++ jsf-api/src/javax/faces/component/UIComponentBase.java 25 Jan 2006 23:21:13 -0000
@@ -10,6 +10,14 @@
 package javax.faces.component;
 
 
+import javax.faces.FacesException;
+import javax.faces.context.FacesContext;
+import javax.faces.el.ValueBinding;
+import javax.faces.event.AbortProcessingException;
+import javax.faces.event.FacesEvent;
+import javax.faces.event.FacesListener;
+import javax.faces.render.Renderer;
+
 import java.beans.IntrospectionException;
 import java.beans.Introspector;
 import java.beans.PropertyDescriptor;
@@ -20,6 +28,7 @@
 import java.util.AbstractSet;
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
@@ -28,16 +37,6 @@
 import java.util.NoSuchElementException;
 import java.util.Set;
 import java.util.WeakHashMap;
-import javax.faces.FacesException;
-import javax.faces.FactoryFinder;
-import javax.faces.context.FacesContext;
-import javax.faces.el.ValueBinding;
-import javax.faces.event.AbortProcessingException;
-import javax.faces.event.FacesEvent;
-import javax.faces.event.FacesListener;
-import javax.faces.render.Renderer;
-import javax.faces.render.RenderKit;
-import javax.faces.render.RenderKitFactory;
 
 
 /**
@@ -70,6 +69,23 @@
      * references to the classes will eventually expire.</p>
      */
     private static WeakHashMap descriptors = new WeakHashMap();
+
+ private static final Iterator EMPTY_ITERATOR = new Iterator() {
+
+ public void remove() {
+ throw new UnsupportedOperationException();
+ }
+
+ public Object next() {
+ throw new NoSuchElementException("Empty Iterator");
+ }
+
+ public boolean hasNext() {
+ return false;
+ }
+ };
+
+ private static final Object[] EMPTY_ARRAY = new Object[0];
  
     /**
      * Reference to the map of <code>PropertyDescriptor</code>s for this class
@@ -586,7 +602,7 @@
     public Map getFacets() {
 
         if (facets == null) {
- facets = new FacetsMap();
+ facets = new FacetsMap(6);
         }
         return (facets);
 
@@ -607,20 +623,20 @@
 
     public Iterator getFacetsAndChildren() {
 
- List combined = new ArrayList();
- if (this.facets != null) {
- Iterator facets = getFacets().values().iterator();
- while (facets.hasNext()) {
- combined.add(facets.next());
- }
- }
- if (this.children != null) {
- Iterator kids = getChildren().iterator();
- while (kids.hasNext()) {
- combined.add(kids.next());
- }
- }
- return (new FacetsAndChildrenIterator(combined));
+ int childCount = getChildren().size();
+ int facetCount = getFacets().size();
+
+ // if there are neither facets nor children
+ if (childCount == 0 && facetCount == 0) {
+ return EMPTY_ITERATOR;
+ } else if (childCount == 0) { // only facets and no children
+ return Collections.unmodifiableCollection(getFacets().values())
+ .iterator();
+ } else if (facetCount == 0) { // only children and no facets
+ return Collections.unmodifiableCollection(getChildren()).iterator();
+ } else { // both children and facets
+ return new FacetsAndChildrenIterator(this);
+ }
 
     }
 
@@ -951,47 +967,50 @@
             return null;
         }
         Object [] stateStruct = new Object[2];
- Object [] childState = null;
+ Object [] childState = EMPTY_ARRAY;
         
         // Process this component itself
         stateStruct[MY_STATE] = saveState(context);
         
         // Process all the children of this component
- int i = 0, len = getChildren().size() + getFacets().keySet().size();
-
- childState = new Object[len];
- stateStruct[CHILD_STATE] = childState;
- Iterator kids = getChildren().iterator();
- while (kids.hasNext()) {
- UIComponent kid = (UIComponent) kids.next();
- Object cState = kid.processSaveState(context);
- if (cState == null) {
- continue;
- } else {
- childState[i++] = cState;
- }
- }
-
- Iterator myFacets = getFacets().keySet().iterator();
- String facetName = null;
- UIComponent facet = null;
- Object facetState = null;
- Object[][] facetSaveState = null;
- while (myFacets.hasNext()) {
- facetName = (String) myFacets.next();
- facet = (UIComponent) getFacets().get(facetName);
- if (!facet.isTransient()) {
- facetState = facet.processSaveState(context);
- facetSaveState = new Object[1][2];
- facetSaveState[0][0] = facetName;
- facetSaveState[0][1] = facetState;
- childState[i] = facetSaveState;
+ int len = getChildCount();
+ len += getFacets().size();
+
+ if (len > 0) {
+ childState = new Object[len];
+ int i = 0;
+ if (getChildCount() > 0) {
+ for (Iterator kids = getChildren().iterator(); kids.hasNext() ;) {
+ UIComponent kid = (UIComponent) kids.next();
+ Object cState = kid.processSaveState(context);
+ if (cState == null) {
+ continue;
+ } else {
+ childState[i++] = cState;
+ }
+ }
             }
- else {
- childState[i] = null;
+
+ if (getFacets().size() > 0) {
+ for (Iterator it = getFacets().entrySet().iterator();
+ it.hasNext(); ) {
+ Map.Entry entry = (Map.Entry) it.next();
+ String facetName = (String) entry.getKey();
+ UIComponent facet = (UIComponent) entry.getValue();
+ if (!facet.isTransient()) {
+ Object facetState = facet.processSaveState(context);
+ Object[][] facetSaveState = new Object[1][2];
+ facetSaveState[0][0] = facetName;
+ facetSaveState[0][1] = facetState;
+ childState[i] = facetSaveState;
+ } else {
+ childState[i] = null;
+ }
+ i++;
+ }
             }
- i++;
         }
+ stateStruct[CHILD_STATE] = childState;
         return stateStruct;
     }
     
@@ -1009,35 +1028,32 @@
         Object [] childState = (Object []) stateStruct[CHILD_STATE];
         
         // Process this component itself
- restoreState(context, stateStruct[MY_STATE]);
+ restoreState(context, stateStruct[MY_STATE]);
         
         int i = 0;
-
- // Process all the children of this component
- Iterator kids = getChildren().iterator();
- while (kids.hasNext()) {
- UIComponent kid = (UIComponent) kids.next();
- Object currentState = childState[i++];
- if (currentState == null) {
- continue;
- }
- kid.processRestoreState(context, currentState);
+
+ if (getChildCount() > 0) {
+ // Process all the children of this component
+ for (Iterator kids = getChildren().iterator(); kids.hasNext(); ) {
+ UIComponent kid = (UIComponent) kids.next();
+ Object currentState = childState[i++];
+ if (currentState == null) {
+ continue;
+ }
+ kid.processRestoreState(context, currentState);
+ }
         }
-
- int facetsSize = getFacets().size();
- int j = 0;
- Object[][] facetSaveState = null;
- String facetName = null;
- UIComponent facet = null;
- Object facetState = null;
- while (j < facetsSize) {
- if (null != (facetSaveState = (Object[][])childState[i++])) {
- facetName = (String) facetSaveState[0][0];
- facetState = facetSaveState[0][1];
- facet = (UIComponent) getFacets().get(facetName);
+
+ for (int j = 0, facetsSize = getFacets().size();
+ j < facetsSize;
+ ++j, i++) {
+ Object[][] facetSaveState = (Object[][]) childState[i];
+ if (facetSaveState != null) {
+ String facetName = (String) facetSaveState[0][0];
+ Object facetState = facetSaveState[0][1];
+ UIComponent facet = (UIComponent) getFacets().get(facetName);
                 facet.processRestoreState(context, facetState);
             }
- ++j;
         }
     }
     
@@ -1695,22 +1711,46 @@
 
 
     // Private implementation of Iterator for getFacetsAndChildren()
- private class FacetsAndChildrenIterator implements Iterator {
+ private static final class FacetsAndChildrenIterator implements Iterator {
 
- public FacetsAndChildrenIterator(List list) {
- this.iterator = list.iterator();
+ private Iterator iterator;
+ private boolean childMode;
+ private UIComponent c;
+
+ public FacetsAndChildrenIterator(UIComponent c) {
+ this.c = c;
         }
-
- private Iterator iterator;
-
+
+ private void update() {
+ if (iterator == null) {
+ // we must guarantee that 'iterator' is never null
+ if (c.getFacets().size() > 0) {
+ iterator = c.getFacets().values().iterator();
+ childMode = false;
+ } else if (c.getChildCount() > 0) {
+ iterator = c.getChildren().iterator();
+ childMode = true;
+ } else {
+ iterator = EMPTY_ITERATOR;
+ }
+ } else if (!childMode
+ && !iterator.hasNext()
+ && c.getChildCount() > 0) {
+ iterator = c.getChildren().iterator();
+ childMode = true;
+ }
+ }
+
         public boolean hasNext() {
- return (iterator.hasNext());
+ this.update();
+ return iterator.hasNext();
         }
-
+
         public Object next() {
- return (iterator.next());
+ this.update();
+ return iterator.next();
         }
-
+
         public void remove() {
             throw new UnsupportedOperationException();
         }
@@ -1722,6 +1762,14 @@
     // required by UIComponent.getFacets()
     private class FacetsMap extends HashMap {
 
+ public FacetsMap() {
+ super();
+ }
+
+ public FacetsMap(int capacity) {
+ super(capacity);
+ }
+
         public void clear() {
             Iterator keys = keySet().iterator();
             while (keys.hasNext()) {
@@ -2160,7 +2208,6 @@
             last = null;
         }
 
- }
-
+ }
 
 }
Index: jsf-api/src/javax/faces/render/Renderer.java
===================================================================
RCS file: /cvs/javaserverfaces-sources/jsf-api/src/javax/faces/render/Renderer.java,v
retrieving revision 1.33
diff -u -r1.33 Renderer.java
--- jsf-api/src/javax/faces/render/Renderer.java 26 Feb 2004 20:31:12 -0000 1.33
+++ jsf-api/src/javax/faces/render/Renderer.java 25 Jan 2006 23:21:13 -0000
@@ -106,19 +106,21 @@
      * or <code>component</code> is <code>null</code>
      */
     public void encodeChildren(FacesContext context, UIComponent component)
- throws IOException {
+ throws IOException {
         if (context == null || component == null) {
             throw new NullPointerException();
         }
- Iterator kids = component.getChildren().iterator();
- while (kids.hasNext()) {
- UIComponent kid = (UIComponent) kids.next();
- kid.encodeBegin(context);
- if (kid.getRendersChildren()) {
- kid.encodeChildren(context);
- }
- kid.encodeEnd(context);
- }
+ if (component.getChildCount() > 0) {
+ for (Iterator kids = component.getChildren().iterator();
+ kids.hasNext(); ) {
+ UIComponent kid = (UIComponent) kids.next();
+ kid.encodeBegin(context);
+ if (kid.getRendersChildren()) {
+ kid.encodeChildren(context);
+ }
+ kid.encodeEnd(context);
+ }
+ }
     }
 
 
Index: jsf-api/test/javax/faces/component/UIComponentBaseTestCase.java
===================================================================
RCS file: /cvs/javaserverfaces-sources/jsf-api/test/javax/faces/component/UIComponentBaseTestCase.java,v
retrieving revision 1.25
diff -u -r1.25 UIComponentBaseTestCase.java
--- jsf-api/test/javax/faces/component/UIComponentBaseTestCase.java 7 Apr 2004 17:39:25 -0000 1.25
+++ jsf-api/test/javax/faces/component/UIComponentBaseTestCase.java 25 Jan 2006 23:21:13 -0000
@@ -799,5 +799,46 @@
 
     }
 
+ public void testGetFacetsAndChildren() {
+
+ UIComponent testComponent = new TestComponent();
+ UIComponent child1 = new TestComponent("child1");
+ UIComponent child2 = new TestComponent("child2");
+ UIComponent child3 = new TestComponent("child3");
+ UIComponent facet1 = new TestComponent("facet1");
+ UIComponent facet2 = new TestComponent("facet2");
+ UIComponent facet3 = new TestComponent("facet3");
+
+ testComponent.getChildren().add(child1);
+ testComponent.getChildren().add(child2);
+ testComponent.getChildren().add(child3);
+ testComponent.getFacets().put("facet1", facet1);
+ testComponent.getFacets().put("facet2", facet2);
+ testComponent.getFacets().put("facet3", facet3);
+
+ Iterator iter = testComponent.getFacetsAndChildren();
+ Object cur = null;
+ assertTrue(iter.hasNext());
+
+ // facets are returned in an undefined order.
+ cur = iter.next();
+ assertTrue(cur == facet1 || cur == facet2 || cur == facet3);
+ cur = iter.next();
+ assertTrue(cur == facet1 || cur == facet2 || cur == facet3);
+ cur = iter.next();
+ assertTrue(cur == facet1 || cur == facet2 || cur == facet3);
+
+ // followed by components, in the order added
+ cur = iter.next();
+ assertTrue(cur == child1);
+ cur = iter.next();
+ assertTrue(cur == child2);
+ cur = iter.next();
+ assertTrue(cur == child3);
+
+ assertTrue(!iter.hasNext());
+
+ }
+
 
 }
Index: jsf-ri/src/com/sun/faces/application/StateManagerImpl.java
===================================================================
RCS file: /cvs/javaserverfaces-sources/jsf-ri/src/com/sun/faces/application/StateManagerImpl.java,v
retrieving revision 1.24.12.4
diff -u -r1.24.12.4 StateManagerImpl.java
--- jsf-ri/src/com/sun/faces/application/StateManagerImpl.java 20 Jan 2006 17:31:01 -0000 1.24.12.4
+++ jsf-ri/src/com/sun/faces/application/StateManagerImpl.java 25 Jan 2006 23:21:13 -0000
@@ -62,15 +62,13 @@
     int noOfViewsInLogicalView = 0;
     
     public SerializedView saveSerializedView(FacesContext context)
- throws IllegalStateException{
- SerializedView result = null;
- Object treeStructure = null;
- Object componentState = null;
+ throws IllegalStateException{
+
         // irrespective of method to save the tree, if the root is transient
         // no state information needs to be persisted.
         UIViewRoot viewRoot = context.getViewRoot();
         if (viewRoot.isTransient()) {
- return result;
+ return null;
         }
         
         // honor the requirement to check for id uniqueness
@@ -81,11 +79,10 @@
             log.debug("Begin creating serialized view for "
                     + viewRoot.getViewId());
          }
- result = new SerializedView(treeStructure =
- getTreeStructureToSave(context),
- componentState =
- getComponentStateToSave(context));
- Util.doAssert(treeStructure instanceof Serializable);
+
+ Object treeStructure = getTreeStructureToSave(context);
+ Object componentState = getComponentStateToSave(context);
+ Util.doAssert(treeStructure instanceof Serializable);
         Util.doAssert(componentState instanceof Serializable);
 
          if (log.isDebugEnabled()) {
@@ -138,13 +135,13 @@
                     logicalMap.put(idInLogicalMap, actualMap);
                 }
                 String id = idInLogicalMap + NamingContainer.SEPARATOR_CHAR +
- idInActualMap;
- result = new SerializedView(id, null);
+ idInActualMap;
                 actualMap.put(idInActualMap, stateArray);
+ return new SerializedView(id, null);
             }
         }
 
- return result;
+ return new SerializedView(treeStructure, componentState);
     }
 
 
@@ -159,46 +156,28 @@
 
 
     protected void checkIdUniqueness(FacesContext context,
- UIComponent component, Set componentIds) throws IllegalStateException{
- UIComponent kid;
- // deal with children that are marked transient.
- Iterator kids = component.getChildren().iterator();
- String id;
- String messageString;
- while (kids.hasNext()) {
- kid = (UIComponent) kids.next();
- // check for id uniqueness
- id = kid.getClientId(context);
- if (id != null && !componentIds.add(id)) {
- messageString = Util.getExceptionMessageString(
- Util.DUPLICATE_COMPONENT_ID_ERROR_ID,
- new Object[]{id});
- if (log.isErrorEnabled()) {
- log.error(messageString);
- }
- throw new IllegalStateException(messageString);
- }
-
- checkIdUniqueness(context, kid, componentIds);
- }
- // deal with facets that are marked transient.
- kids = component.getFacets().values().iterator();
- while (kids.hasNext()) {
- kid = (UIComponent) kids.next();
- // check for id uniqueness
- id = kid.getClientId(context);
- if (id != null && !componentIds.add(id)) {
- messageString = Util.getExceptionMessageString(
- Util.DUPLICATE_COMPONENT_ID_ERROR_ID,
- new Object[]{id});
- if (log.isErrorEnabled()) {
- log.error(messageString);
+ UIComponent component,
+ Set componentIds)
+ throws IllegalStateException{
+ // deal with children/facets that are marked transient.
+ for (Iterator i = component.getFacetsAndChildren(); i.hasNext(); ) {
+
+ UIComponent kid = (UIComponent) i.next();
+ // check for id uniqueness
+ String id = kid.getClientId(context);
+ if (componentIds.add(id)) {
+ checkIdUniqueness(context, kid, componentIds);
+ } else {
+ if (id != null && !componentIds.add(id)) {
+ String messageString = Util.getExceptionMessageString(
+ Util.DUPLICATE_COMPONENT_ID_ERROR_ID,
+ new Object[]{id});
+ if (log.isErrorEnabled()) {
+ log.error(messageString);
+ }
+ throw new IllegalStateException(messageString);
                 }
- throw new IllegalStateException(messageString);
- }
-
- checkIdUniqueness(context, kid, componentIds);
-
+ }
         }
     }