incubator-adffaces-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From awi...@apache.org
Subject svn commit: r533186 - in /incubator/adffaces/trunk/trinidad/trinidad-api/src/main/java/org/apache/myfaces/trinidad: change/ webapp/
Date Fri, 27 Apr 2007 18:31:46 GMT
Author: awiner
Date: Fri Apr 27 11:31:45 2007
New Revision: 533186

URL: http://svn.apache.org/viewvc?view=rev&rev=533186
Log:
ADFFACES-466: Issues in change persistence framework code
Checked in patch from K Srinath Reddy, which fixes the following issues:
- Various synchronization problems with the datastructures in SessionChangeManager
- _getUniqueIdForComponent() method in BaseChangeManager doesn't return the unique ID in the
component tree instead it returns the unique id in the containing NamingContainer 
- Change are applied on every request instead of only when the tree is newly created
- In the AddChildComponentChange, if the child with the already existing ID is added, the
child is being removed and the new child is added which is faulty

Modified:
    incubator/adffaces/trunk/trinidad/trinidad-api/src/main/java/org/apache/myfaces/trinidad/change/AddChildComponentChange.java
    incubator/adffaces/trunk/trinidad/trinidad-api/src/main/java/org/apache/myfaces/trinidad/change/BaseChangeManager.java
    incubator/adffaces/trunk/trinidad/trinidad-api/src/main/java/org/apache/myfaces/trinidad/change/ChangeManager.java
    incubator/adffaces/trunk/trinidad/trinidad-api/src/main/java/org/apache/myfaces/trinidad/change/ChangeUtils.java
    incubator/adffaces/trunk/trinidad/trinidad-api/src/main/java/org/apache/myfaces/trinidad/change/SessionChangeManager.java
    incubator/adffaces/trunk/trinidad/trinidad-api/src/main/java/org/apache/myfaces/trinidad/webapp/UIXComponentTag.java

Modified: incubator/adffaces/trunk/trinidad/trinidad-api/src/main/java/org/apache/myfaces/trinidad/change/AddChildComponentChange.java
URL: http://svn.apache.org/viewvc/incubator/adffaces/trunk/trinidad/trinidad-api/src/main/java/org/apache/myfaces/trinidad/change/AddChildComponentChange.java?view=diff&rev=533186&r1=533185&r2=533186
==============================================================================
--- incubator/adffaces/trunk/trinidad/trinidad-api/src/main/java/org/apache/myfaces/trinidad/change/AddChildComponentChange.java
(original)
+++ incubator/adffaces/trunk/trinidad/trinidad-api/src/main/java/org/apache/myfaces/trinidad/change/AddChildComponentChange.java
Fri Apr 27 11:31:45 2007
@@ -22,6 +22,8 @@
 
 import javax.faces.component.UIComponent;
 
+import org.apache.myfaces.trinidad.logging.TrinidadLogger;
+
 /**
  * Change specialization for adding a child component.
  * While applying this Change, the child component is re-created and added to
@@ -88,7 +90,9 @@
     UIComponent removableChild = ChangeUtils.getChildForId(uiComponent, newChildId);
   
     if (removableChild != null)
-      children.remove(removableChild);
+    {
+      throw new IllegalStateException("Attempt to add a duplicate ID " + newChildId);
+    }
     
     if (_insertBeforeId == null)
     {
@@ -97,26 +101,19 @@
     }
     else
     {
-      int index = _getChildIndex(uiComponent, _insertBeforeId);
-      children.add(index, child);
+      int index = ChangeUtils.getChildIndexForId(uiComponent, _insertBeforeId);
+      if(index == -1)
+      {
+        children.add(child);
+      }
+      else
+      {
+        children.add(index, child);
+      }
     }
   }
   
-  private static int _getChildIndex(UIComponent parent, String childId)
-  {
-    int numChildren = parent.getChildCount();
-    if (childId == null)
-      return numChildren;
-    UIComponent child = ChangeUtils.getChildForId(parent, childId);
-    if (child == null)
-    {
-      return numChildren;
-    }
-    else
-    {
-      return parent.getChildren().indexOf(child);
-    }
-  }
-
   private final String _insertBeforeId;
-}
+  static private final TrinidadLogger _LOG =
+    TrinidadLogger.createTrinidadLogger(AddChildComponentChange.class);
+}
\ No newline at end of file

Modified: incubator/adffaces/trunk/trinidad/trinidad-api/src/main/java/org/apache/myfaces/trinidad/change/BaseChangeManager.java
URL: http://svn.apache.org/viewvc/incubator/adffaces/trunk/trinidad/trinidad-api/src/main/java/org/apache/myfaces/trinidad/change/BaseChangeManager.java?view=diff&rev=533186&r1=533185&r2=533186
==============================================================================
--- incubator/adffaces/trunk/trinidad/trinidad-api/src/main/java/org/apache/myfaces/trinidad/change/BaseChangeManager.java
(original)
+++ incubator/adffaces/trunk/trinidad/trinidad-api/src/main/java/org/apache/myfaces/trinidad/change/BaseChangeManager.java
Fri Apr 27 11:31:45 2007
@@ -19,9 +19,9 @@
 package org.apache.myfaces.trinidad.change;
 
 import java.util.Iterator;
-import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
+import java.util.concurrent.CopyOnWriteArrayList;
 
 import javax.faces.component.NamingContainer;
 import javax.faces.component.UIComponent;
@@ -161,7 +161,7 @@
 
     if (changeListForComponent == null)
     {
-      changeListForComponent = new LinkedList<ComponentChange>();
+      changeListForComponent = new CopyOnWriteArrayList<ComponentChange>();
       componentToChangesMap.put(uniqueIdForComponent, changeListForComponent);
     }
 
@@ -181,17 +181,18 @@
     //  own closest ancestor that is a NamingContainer. Matches algorithm of
     //  UIComponent.findComponent().
     UIComponent ancestor = uiComponent;
+    StringBuffer uniqueIdBuffer = new StringBuffer();
+    uniqueIdBuffer.append(uiComponent.getId());
     while (ancestor != null)
     {
       if (ancestor instanceof NamingContainer)
-        break;
+      {
+        uniqueIdBuffer.insert(0,new StringBuffer().append(ancestor.getId()).
+          append(NamingContainer.SEPARATOR_CHAR));
+      }
       ancestor = ancestor.getParent();
     }
-    String namingContainerId = (ancestor == null) ? "":ancestor.getId();
-
-    return namingContainerId +
-           NamingContainer.SEPARATOR_CHAR +
-           uiComponent.getId();
+    return uniqueIdBuffer.toString();
   }
 
   /**

Modified: incubator/adffaces/trunk/trinidad/trinidad-api/src/main/java/org/apache/myfaces/trinidad/change/ChangeManager.java
URL: http://svn.apache.org/viewvc/incubator/adffaces/trunk/trinidad/trinidad-api/src/main/java/org/apache/myfaces/trinidad/change/ChangeManager.java?view=diff&rev=533186&r1=533185&r2=533186
==============================================================================
--- incubator/adffaces/trunk/trinidad/trinidad-api/src/main/java/org/apache/myfaces/trinidad/change/ChangeManager.java
(original)
+++ incubator/adffaces/trunk/trinidad/trinidad-api/src/main/java/org/apache/myfaces/trinidad/change/ChangeManager.java
Fri Apr 27 11:31:45 2007
@@ -171,7 +171,8 @@
 
   /**
   * Retrieve the identifiers of all components on this request that have Changes
-  *  associated with them.
+  *  associated with them for the viewId specified in the facesContext.
+  * @param facesContext
   * @return An Iterator that can be used to access the collection of component
   *          identifiers. Returns null if there are no such components.
   */

Modified: incubator/adffaces/trunk/trinidad/trinidad-api/src/main/java/org/apache/myfaces/trinidad/change/ChangeUtils.java
URL: http://svn.apache.org/viewvc/incubator/adffaces/trunk/trinidad/trinidad-api/src/main/java/org/apache/myfaces/trinidad/change/ChangeUtils.java?view=diff&rev=533186&r1=533185&r2=533186
==============================================================================
--- incubator/adffaces/trunk/trinidad/trinidad-api/src/main/java/org/apache/myfaces/trinidad/change/ChangeUtils.java
(original)
+++ incubator/adffaces/trunk/trinidad/trinidad-api/src/main/java/org/apache/myfaces/trinidad/change/ChangeUtils.java
Fri Apr 27 11:31:45 2007
@@ -64,6 +64,36 @@
   }
   
   /**
+   * Given a parent component and the identifier for the child, looks up among
+   * the children for a child with the specified identifier and returns the index
+   * of the child
+   * Returns -1 if there were to be no such child
+   * @param parent
+   * @param childId the identifier of child to be searched in the parent's 
+   * children
+   */
+  @SuppressWarnings("unchecked")
+  public static int getChildIndexForId(UIComponent parent, String childId)
+  {
+    if (parent == null)
+      throw new NullPointerException("Parent cannot be null");
+
+    int numChildren = parent.getChildCount();
+    if (numChildren == 0)
+      return -1;
+
+    List<UIComponent> children = parent.getChildren();      
+    UIComponent child;    
+    for (int i=0; i<numChildren; i++)
+    {
+      child = children.get(i);
+      if ( childId.equals(child.getId()) )
+        return i;
+    }
+    return -1;
+  }
+  
+  /**
    * Given a node representing a component, returns the named facet's Element.
    * @param componentNode The node to search for a facet contained in it.
    * @param facetName The name of the facet to search for.

Modified: incubator/adffaces/trunk/trinidad/trinidad-api/src/main/java/org/apache/myfaces/trinidad/change/SessionChangeManager.java
URL: http://svn.apache.org/viewvc/incubator/adffaces/trunk/trinidad/trinidad-api/src/main/java/org/apache/myfaces/trinidad/change/SessionChangeManager.java?view=diff&rev=533186&r1=533185&r2=533186
==============================================================================
--- incubator/adffaces/trunk/trinidad/trinidad-api/src/main/java/org/apache/myfaces/trinidad/change/SessionChangeManager.java
(original)
+++ incubator/adffaces/trunk/trinidad/trinidad-api/src/main/java/org/apache/myfaces/trinidad/change/SessionChangeManager.java
Fri Apr 27 11:31:45 2007
@@ -18,9 +18,9 @@
  */
 package org.apache.myfaces.trinidad.change;
 
-import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
 
 import javax.faces.context.FacesContext;
 
@@ -42,7 +42,7 @@
    * @param viewId viewID for request
    * @param createIfNecessary <code>true</code> if Map should be created if not
    *        already present
-   * @return Map of componentID tokens to Lists of Changes
+   * @return Synchronized Map of componentID tokens to Lists of Changes
    */
   @SuppressWarnings("unchecked")
   @Override
@@ -59,7 +59,7 @@
     {
       if (!createIfNecessary)
         return null;
-      viewToChangesMap = new HashMap<String, Map<String, List<ComponentChange>>>();
+      viewToChangesMap = new ConcurrentHashMap<String, Map<String, List<ComponentChange>>>();
       sessMap.put(_CHANGE_KEY, viewToChangesMap);
     }
     
@@ -69,7 +69,7 @@
     {
       if (!createIfNecessary)
         return null;
-      componentToChangesMap = new HashMap<String, List<ComponentChange>>();
+      componentToChangesMap = new ConcurrentHashMap<String, List<ComponentChange>>();
       viewToChangesMap.put(viewId, componentToChangesMap);
     }
     return componentToChangesMap;

Modified: incubator/adffaces/trunk/trinidad/trinidad-api/src/main/java/org/apache/myfaces/trinidad/webapp/UIXComponentTag.java
URL: http://svn.apache.org/viewvc/incubator/adffaces/trunk/trinidad/trinidad-api/src/main/java/org/apache/myfaces/trinidad/webapp/UIXComponentTag.java?view=diff&rev=533186&r1=533185&r2=533186
==============================================================================
--- incubator/adffaces/trunk/trinidad/trinidad-api/src/main/java/org/apache/myfaces/trinidad/webapp/UIXComponentTag.java
(original)
+++ incubator/adffaces/trunk/trinidad/trinidad-api/src/main/java/org/apache/myfaces/trinidad/webapp/UIXComponentTag.java
Fri Apr 27 11:31:45 2007
@@ -113,8 +113,8 @@
   public int doEndTag() throws JspException
   {
     UIComponent component = getComponentInstance();
-    if (isSuppressed())
-      _applyChanges(getFacesContext(), component, getCreated());
+    if (isSuppressed() && getCreated())
+      _applyChanges(getFacesContext(), component);
     return super.doEndTag();
   }
 
@@ -122,8 +122,8 @@
   protected void encodeBegin() throws java.io.IOException
   {
     UIComponent component = getComponentInstance();
-    if (!isSuppressed())
-      _applyChanges(getFacesContext(), component, getCreated());
+    if (!isSuppressed() && getCreated())
+      _applyChanges(getFacesContext(), component);
     super.encodeBegin();
   }
 
@@ -563,8 +563,7 @@
 
   private static void _applyChanges(
     FacesContext facesContext,
-    UIComponent uiComponent,
-    boolean isCreated)
+    UIComponent uiComponent)
   {
     RequestContext afc = RequestContext.getCurrentInstance();
     Iterator<ComponentChange> changeIter =
@@ -575,19 +574,9 @@
     while (changeIter.hasNext())
     {
       ComponentChange change = changeIter.next();
-
-      //pu: If we did not create the component during tag execution, do not
-      //  apply any AttributeChange. This is because we do not have enough
-      //  mechanism to take care of such cases for now. Users could always apply
-      //  such Changes explicitly in their backing bean after creating component.
-      boolean isChangeApplicable =
-        ( (change instanceof AttributeComponentChange) && !isCreated) ? false:true;
-
-      if (isChangeApplicable)
-      {
-        change.changeComponent(uiComponent);
-      }
-
+      
+      change.changeComponent(uiComponent);
+      
       //pu: In case this Change has added a new component/facet, the added
       //  component could have its own Changes, that may need to be applied here.
       if (change instanceof AddComponentChange)
@@ -597,7 +586,7 @@
 
         if (newAddedComponent != null)
         {
-          _applyChanges(facesContext, newAddedComponent, true);
+          _applyChanges(facesContext, newAddedComponent);
         }
       }
     }



Mime
View raw message