You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@myfaces.apache.org by bs...@apache.org on 2013/04/21 05:25:49 UTC
svn commit: r1470258 - in /myfaces/trinidad/trunk:
trinidad-api/src/main/java/org/apache/myfaces/trinidad/component/
trinidad-api/src/test/java/org/apache/myfaces/trinidad/component/
trinidad-api/src/test/java/org/apache/myfaces/trinidad/render/ trinid...
Author: bsullivan
Date: Sun Apr 21 03:25:49 2013
New Revision: 1470258
URL: http://svn.apache.org/r1470258
Log:
[TRINIDAD-2378] UIXComponentBase should override clearCachedClientId to avoid calling getId() at times when the UIViewRoot isn't present
The current implementation of clearCachedClientIds on UIXComponent is final and always calls setId() with the result of getId(). The problem is that calling getId() requires a call to UIViewRoot.getUniqueId() if the component does not already have an id. Unfortunately, there are times when the UIViewRoot isn't attached to the FacesContext when we need to clear cached client ids. The easiest and fastest solution is to make clearCachedClientIds() non-final and allow UIXComponentBase to override the method to clear out its cached clientId directly.
Modified:
myfaces/trinidad/trunk/trinidad-api/src/main/java/org/apache/myfaces/trinidad/component/UIXComponent.java
myfaces/trinidad/trunk/trinidad-api/src/main/java/org/apache/myfaces/trinidad/component/UIXComponentBase.java
myfaces/trinidad/trunk/trinidad-api/src/test/java/org/apache/myfaces/trinidad/component/ClientIdCachingTest.java
myfaces/trinidad/trunk/trinidad-api/src/test/java/org/apache/myfaces/trinidad/render/RenderUtilsTest.java
myfaces/trinidad/trunk/trinidad-impl/src/test/java/org/apache/myfaces/trinidadinternal/renderkit/CoreRenderKitTest.java
Modified: myfaces/trinidad/trunk/trinidad-api/src/main/java/org/apache/myfaces/trinidad/component/UIXComponent.java
URL: http://svn.apache.org/viewvc/myfaces/trinidad/trunk/trinidad-api/src/main/java/org/apache/myfaces/trinidad/component/UIXComponent.java?rev=1470258&r1=1470257&r2=1470258&view=diff
==============================================================================
--- myfaces/trinidad/trunk/trinidad-api/src/main/java/org/apache/myfaces/trinidad/component/UIXComponent.java (original)
+++ myfaces/trinidad/trunk/trinidad-api/src/main/java/org/apache/myfaces/trinidad/component/UIXComponent.java Sun Apr 21 03:25:49 2013
@@ -773,7 +773,7 @@ abstract public class UIXComponent exten
/**
* Clears all of the cached clientIds in this component subtree
*/
- public final void clearCachedClientIds()
+ public void clearCachedClientIds()
{
clearCachedClientIds(this);
}
@@ -784,13 +784,32 @@ abstract public class UIXComponent exten
*/
public static void clearCachedClientIds(UIComponent component)
{
+ if (component instanceof UIXComponent)
+ {
+ ((UIXComponent)component).clearCachedClientIds();
+ }
+ else
+ {
+ _clearCachedClientIds(component);
+ }
+ }
+
+ /**
+ * Default implementation of clearing the cached client ids
+ */
+ private static void _clearCachedClientIds(UIComponent component)
+ {
+ // clear this component
String id = component.getId();
component.setId(id);
+ // clear the children
Iterator<UIComponent> allChildren = component.getFacetsAndChildren();
while (allChildren.hasNext())
+ {
clearCachedClientIds(allChildren.next());
+ }
}
/**
Modified: myfaces/trinidad/trunk/trinidad-api/src/main/java/org/apache/myfaces/trinidad/component/UIXComponentBase.java
URL: http://svn.apache.org/viewvc/myfaces/trinidad/trunk/trinidad-api/src/main/java/org/apache/myfaces/trinidad/component/UIXComponentBase.java?rev=1470258&r1=1470257&r2=1470258&view=diff
==============================================================================
--- myfaces/trinidad/trunk/trinidad-api/src/main/java/org/apache/myfaces/trinidad/component/UIXComponentBase.java (original)
+++ myfaces/trinidad/trunk/trinidad-api/src/main/java/org/apache/myfaces/trinidad/component/UIXComponentBase.java Sun Apr 21 03:25:49 2013
@@ -44,6 +44,7 @@ import javax.faces.component.ContextCall
import javax.faces.component.NamingContainer;
import javax.faces.component.StateHolder;
import javax.faces.component.UIComponent;
+import javax.faces.component.UIViewRoot;
import javax.faces.component.behavior.ClientBehavior;
import javax.faces.component.behavior.ClientBehaviorHolder;
import javax.faces.context.ExternalContext;
@@ -499,26 +500,50 @@ abstract public class UIXComponentBase e
String clientId = getId();
// Search for an ancestor that is a naming container
- UIComponent containerComponent = getParent();
- while (null != containerComponent)
+ UIComponent lastParent = null;
+ UIComponent currParent = getParent();
+
+ while (true)
{
- if (containerComponent instanceof NamingContainer)
+ // prepend the NamingContainer portion of the id
+ if (currParent instanceof NamingContainer)
{
String contClientId;
// Pass additional context information to naming containers which extend UIXComponent:
- if (containerComponent instanceof UIXComponent)
- contClientId = ((UIXComponent)containerComponent).getContainerClientId(context, this);
+ if (currParent instanceof UIXComponent)
+ contClientId = ((UIXComponent)currParent).getContainerClientId(context, this);
else
- contClientId = containerComponent.getContainerClientId(context);
+ contClientId = currParent.getContainerClientId(context);
StringBuilder bld = __getSharedStringBuilder();
bld.append(contClientId).append(NamingContainer.SEPARATOR_CHAR).append(clientId);
clientId = bld.toString();
break;
}
-
- containerComponent = containerComponent.getParent();
+ else if (currParent == null)
+ {
+ if (lastParent instanceof UIViewRoot)
+ {
+ // we got to the top of the component tree, so done looping
+ break;
+ }
+ else
+ {
+ // the component isn't in the component tree, which can cause the cached client id to be wrong.
+
+ // =-= btsulliv see Trinidad-2374. We can't do this right now because of a couple of bogus Trinidad Renderers
+ break;
+
+ /*
+ throw new IllegalStateException("Calling getClientId() on component " + this +
+ " when it is not in the component tree. Ancestor path:" +_getAncestorPath());
+ */
+ }
+ }
+
+ lastParent = currParent;
+ currParent = lastParent.getParent();
}
Renderer renderer = getRenderer(context);
@@ -528,6 +553,58 @@ abstract public class UIXComponentBase e
return clientId;
}
+ private List<UIComponent> _getAncestors()
+ {
+ List<UIComponent> ancestors = new ArrayList<UIComponent>();
+
+ UIComponent parent = getParent();
+
+ while (parent != null)
+ {
+ ancestors.add(parent);
+
+ parent = parent.getParent();
+ }
+
+ Collections.reverse(ancestors);
+
+ return ancestors;
+ }
+
+ private String _getAncestorPath()
+ {
+ StringBuilder ancestorPath = new StringBuilder(1000);
+
+ List<UIComponent> ancestors = _getAncestors();
+
+ if (ancestors.isEmpty())
+ {
+ return "<none>";
+ }
+ else
+ {
+ Iterator<UIComponent> ancestorsIter = ancestors.iterator();
+
+ boolean first = true;
+
+ while (ancestorsIter.hasNext())
+ {
+ if (!first)
+ {
+ ancestorPath.append('/');
+ }
+ else
+ {
+ first = false;
+ }
+
+ ancestorPath.append(ancestorsIter.next().toString());
+ }
+
+ return ancestorPath.toString();
+ }
+ }
+
@Override
public String getClientId(FacesContext context)
{
@@ -593,7 +670,10 @@ abstract public class UIXComponentBase e
// make sure that we always have an id
if (_id == null)
{
- _id = FacesContext.getCurrentInstance().getViewRoot().createUniqueId();
+ FacesContext context = FacesContext.getCurrentInstance();
+ UIViewRoot viewRoot = context.getViewRoot();
+
+ _id = viewRoot.createUniqueId();
}
return _id;
@@ -642,7 +722,7 @@ abstract public class UIXComponentBase e
{
// only validate if the id has actually changed
if ((_id == null) || !_id.equals(id))
- {
+ {
_validateId(id);
_id = id;
@@ -650,7 +730,7 @@ abstract public class UIXComponentBase e
// of our children
if ((_clientId != null) && (this instanceof NamingContainer))
{
- clearCachedClientIds(this);
+ clearCachedClientIds();
}
}
}
@@ -663,6 +743,24 @@ abstract public class UIXComponentBase e
_clientId = null;
}
+ /**
+ * Clears all of the cached clientIds in this component subtree
+ */
+ @Override
+ public void clearCachedClientIds()
+ {
+ // clear our clientId
+ _clientId = null;
+
+ // clear the children
+ Iterator<UIComponent> allChildren = getFacetsAndChildren();
+
+ while (allChildren.hasNext())
+ {
+ clearCachedClientIds(allChildren.next());
+ }
+ }
+
@Override
abstract public String getFamily();
@@ -689,9 +787,12 @@ abstract public class UIXComponentBase e
{
// set the reference
_parent = parent;
- _resetClientId();
+
+ boolean isInView = parent.isInView();
+
+ _resetClientId(isInView);
- if (parent.isInView())
+ if (isInView)
{
// trigger the ADD_EVENT and call setInView(true)
// recursive for all kids/facets...
@@ -703,7 +804,9 @@ abstract public class UIXComponentBase e
}
else
{
- if (_parent != null && _parent.isInView())
+ boolean wasInView = _parent != null && _parent.isInView();
+
+ if (wasInView)
{
// trigger the "remove event" lifecycle
// and call setInView(false) for all children/facets
@@ -711,24 +814,36 @@ abstract public class UIXComponentBase e
_publishPreRemoveFromViewEvent(getFacesContext(), this);
}
- // (un)set the reference
- _parent = parent;
- _resetClientId();
+ // clear the references
+ _parent = null;
+ _resetClientId(false);
}
}
}
- private void _resetClientId()
+ private void _resetClientId(boolean isInView)
{
// clear cached client ids if necessary
if (_clientId != null)
{
- String newClientId = _calculateClientId(FacesContext.getCurrentInstance());
-
+ String newClientId;
+
+ // if the component is currently in the component tree, calculate the new clientId, to see if it has changed
+ if (isInView)
+ {
+ newClientId = _calculateClientId(FacesContext.getCurrentInstance());
+ }
+ else
+ {
+ newClientId = null;
+ }
+
// if our clientId changed as a result of being reparented (because we moved
// between NamingContainers for instance) then we need to clear out
+ boolean clearCachedIds = !_clientId.equals(newClientId);
+
// all of the cached client ids for our subtree
- if (!_clientId.equals(newClientId))
+ if (clearCachedIds)
{
clearCachedClientIds();
_clientId = newClientId;
Modified: myfaces/trinidad/trunk/trinidad-api/src/test/java/org/apache/myfaces/trinidad/component/ClientIdCachingTest.java
URL: http://svn.apache.org/viewvc/myfaces/trinidad/trunk/trinidad-api/src/test/java/org/apache/myfaces/trinidad/component/ClientIdCachingTest.java?rev=1470258&r1=1470257&r2=1470258&view=diff
==============================================================================
--- myfaces/trinidad/trunk/trinidad-api/src/test/java/org/apache/myfaces/trinidad/component/ClientIdCachingTest.java (original)
+++ myfaces/trinidad/trunk/trinidad-api/src/test/java/org/apache/myfaces/trinidad/component/ClientIdCachingTest.java Sun Apr 21 03:25:49 2013
@@ -27,8 +27,6 @@ import junit.framework.TestSuite;
import org.apache.myfaces.trinidadbuild.test.FacesTestCase;
-import org.apache.myfaces.trinidad.component.UIXPanel;
-
public class ClientIdCachingTest extends FacesTestCase
{
public static final Test suite()
@@ -69,12 +67,16 @@ public class ClientIdCachingTest extends
TestNamingContainer d = new TestNamingContainer(); d.setId("d");
TestPanel e = new TestPanel(); e.setId("e");
TestPanel g = new TestPanel(); g.setId("g");
+
+ FacesContext context = FacesContext.getCurrentInstance();
+ context.getViewRoot().getChildren().add(a);
+
a.getChildren().add(b);
b.getChildren().add(d);
b.getChildren().add(g);
d.getChildren().add(e);
- FacesContext context = FacesContext.getCurrentInstance();
+
assertEquals("a:b:d:e", e.getClientId(context));
}
@@ -85,13 +87,16 @@ public class ClientIdCachingTest extends
TestNamingContainer d = new TestNamingContainer(); d.setId("d");
TestPanel e = new TestPanel(); e.setId("e");
TestPanel g = new TestPanel(); g.setId("g");
+
+ FacesContext context = FacesContext.getCurrentInstance();
+ context.getViewRoot().getChildren().add(a);
+
a.getChildren().add(b);
b.getChildren().add(d);
b.getChildren().add(g);
d.getChildren().add(e);
// prime
- FacesContext context = FacesContext.getCurrentInstance();
assertEquals("a:b:d:e", e.getClientId(context));
// set the component's id using accessor
@@ -121,6 +126,10 @@ public class ClientIdCachingTest extends
TestPanel e = new TestPanel(); e.setId("e");
TestPanel f = new TestPanel(); f.setId("f");
TestPanel g = new TestPanel(); g.setId("g");
+
+ FacesContext context = FacesContext.getCurrentInstance();
+ context.getViewRoot().getChildren().add(a);
+
a.getChildren().add(b);
a.getChildren().add(c);
b.getChildren().add(d);
@@ -129,7 +138,6 @@ public class ClientIdCachingTest extends
d.getChildren().add(f);
// prime
- FacesContext context = FacesContext.getCurrentInstance();
assertEquals("a:b:d:e", e.getClientId(context));
// move within same NamingContainer--no clientId change
Modified: myfaces/trinidad/trunk/trinidad-api/src/test/java/org/apache/myfaces/trinidad/render/RenderUtilsTest.java
URL: http://svn.apache.org/viewvc/myfaces/trinidad/trunk/trinidad-api/src/test/java/org/apache/myfaces/trinidad/render/RenderUtilsTest.java?rev=1470258&r1=1470257&r2=1470258&view=diff
==============================================================================
--- myfaces/trinidad/trunk/trinidad-api/src/test/java/org/apache/myfaces/trinidad/render/RenderUtilsTest.java (original)
+++ myfaces/trinidad/trunk/trinidad-api/src/test/java/org/apache/myfaces/trinidad/render/RenderUtilsTest.java Sun Apr 21 03:25:49 2013
@@ -101,6 +101,9 @@ public class RenderUtilsTest extends Fac
table1.setId("table1");
TestUIXPanel rootPanel = new TestUIXPanel();
rootPanel.setId("rootPanel");
+
+ context.getViewRoot().getChildren().add(rootPanel);
+
rootPanel.getChildren().add(button1);
rootPanel.getChildren().add(table1);
TestUIXPanel tableChild = new TestUIXPanel();
@@ -166,6 +169,8 @@ public class RenderUtilsTest extends Fac
TestUIXPanel rootButton = new TestUIXPanel();
rootButton.setId("rootButton");
+ context.getViewRoot().getChildren().add(form);
+
form.getChildren().add(ncRoot);
form.getChildren().add(rootButton);
ncRoot.getChildren().add(button1);
Modified: myfaces/trinidad/trunk/trinidad-impl/src/test/java/org/apache/myfaces/trinidadinternal/renderkit/CoreRenderKitTest.java
URL: http://svn.apache.org/viewvc/myfaces/trinidad/trunk/trinidad-impl/src/test/java/org/apache/myfaces/trinidadinternal/renderkit/CoreRenderKitTest.java?rev=1470258&r1=1470257&r2=1470258&view=diff
==============================================================================
--- myfaces/trinidad/trunk/trinidad-impl/src/test/java/org/apache/myfaces/trinidadinternal/renderkit/CoreRenderKitTest.java (original)
+++ myfaces/trinidad/trunk/trinidad-impl/src/test/java/org/apache/myfaces/trinidadinternal/renderkit/CoreRenderKitTest.java Sun Apr 21 03:25:49 2013
@@ -104,9 +104,9 @@ public class CoreRenderKitTest extends R
}
}
- static private List<SuiteDefinition> _definitions =
+ private static final List<SuiteDefinition> _definitions =
new ArrayList<SuiteDefinition>();
- private static HashSet<String> _sHtmlComponents;
+ private static final HashSet<String> _sHtmlComponents;
static
{
@@ -159,7 +159,7 @@ public class CoreRenderKitTest extends R
RenderKitBootstrap.getGeckoAgent(),
false));
- _sHtmlComponents = new HashSet<String>(5);
+ _sHtmlComponents = new HashSet<String>(8);
_sHtmlComponents.add("org.apache.myfaces.trinidad.HtmlBody");
_sHtmlComponents.add("org.apache.myfaces.trinidad.HtmlFrame");
_sHtmlComponents.add("org.apache.myfaces.trinidad.HtmlFrameBorderLayout");