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 2010/03/11 00:34:24 UTC
svn commit: r921628 - in /myfaces/trinidad/trunk/trinidad-api/src:
main/java/org/apache/myfaces/trinidad/component/
test/java/org/apache/myfaces/trinidad/component/
test/java/org/apache/myfaces/trinidad/render/
Author: bsullivan
Date: Wed Mar 10 23:34:23 2010
New Revision: 921628
URL: http://svn.apache.org/viewvc?rev=921628&view=rev
Log:
JIRA-1669 client id caching improvements
Added:
myfaces/trinidad/trunk/trinidad-api/src/test/java/org/apache/myfaces/trinidad/component/ClientIdCachingTest.java
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/render/RenderUtilsTest.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=921628&r1=921627&r2=921628&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 Wed Mar 10 23:34:23 2010
@@ -316,10 +316,10 @@ abstract public class UIXComponent exten
* whose <code>visit</code> method will be called
* for each node visited.
* @return component implementations may return <code>true</code>
- * to indicate that the tree visit is complete (eg. all components
- * that need to be visited have been visited). This results in
- * the tree visit being short-circuited such that no more components
- * are visited.
+ * to indicate that the tree visit is complete (eg. all components
+ * that need to be visited have been visited). This results in
+ * the tree visit being short-circuited such that no more components
+ * are visited.
*
* @see VisitContext#invokeVisitCallback VisitContext.invokeVisitCallback()
*/
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=921628&r1=921627&r2=921628&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 Wed Mar 10 23:34:23 2010
@@ -30,6 +30,8 @@ import java.util.List;
import java.util.Map;
import java.util.Properties;
+import java.util.concurrent.atomic.AtomicReference;
+
import javax.el.ELContext;
import javax.el.ELException;
import javax.el.MethodExpression;
@@ -260,8 +262,6 @@ abstract public class UIXComponentBase e
}
}
-
-
/**
*/
@Override
@@ -281,7 +281,6 @@ abstract public class UIXComponentBase e
return getFacesBean().getValueBinding(key);
}
-
@Override
public void setValueBinding(String name, ValueBinding binding)
{
@@ -292,7 +291,6 @@ abstract public class UIXComponentBase e
getFacesBean().setValueBinding(key, binding);
}
-
@Override
public Map<String, Object> getAttributes()
{
@@ -344,40 +342,48 @@ abstract public class UIXComponentBase e
return clientId;
}
-
-
@Override
public String getClientId(FacesContext context)
{
- return _calculateClientId(context);
-/* TODO put back in when we fix all of the clientID caching issues
- String clientId = _clientId;
-
- if (clientId == null)
+ if (_isClientIdCachingEnabled(context))
{
- clientId = _calculateClientId(context);
+ String clientId = _clientId;
- if (_usesFacesBeanImpl)
- _clientId = clientId;
+ if (clientId == null)
+ {
+ clientId = _calculateClientId(context);
+
+ if (_usesFacesBeanImpl)
+ {
+ _clientId = clientId;
+ }
+ }
+ else
+ {
+ // for now validate success by checking the cached result against the dynamically
+ // generated result
+ String realID = _calculateClientId(context);
+
+ if (!clientId.equals(realID))
+ throw new IllegalStateException(
+ "cached client id " + clientId + " for " + this + " doesn't match client id:" + realID);
+ }
+
+ return clientId;
}
else
{
- assert clientId.equals(_calculateClientId(context)) :
- "cached client id " + _clientId + " for " + this + " doesn't match client id:" + _calculateClientId(context);
+ return _calculateClientId(context);
}
-
- return clientId;
-*/
}
-
/**
* Gets the identifier for the component. This implementation
* never returns a null id.
*/
@Override
public String getId()
- {
+ {
// determine whether we can use the optimized code path or not
if (_usesFacesBeanImpl)
{
@@ -410,7 +416,6 @@ abstract public class UIXComponentBase e
}
}
-
/**
* Sets the identifier for the component. The identifier
* must follow a subset of the syntax allowed in HTML:
@@ -423,8 +428,6 @@ abstract public class UIXComponentBase e
@Override
public void setId(String id)
{
- _clientId = null;
-
FacesBean facesBean = getFacesBean();
// if we are using a FacesBeanImpl, then the FacesBean will
@@ -452,19 +455,19 @@ abstract public class UIXComponentBase e
_validateId(id);
facesBean.setProperty(ID_KEY, id);
}
+
+ _clientId = null;
}
@Override
abstract public String getFamily();
-
@Override
public UIComponent getParent()
{
return _parent;
}
-
/**
* <p>Set the parent <code>UIComponent</code> of this
* <code>UIComponent</code>.</p>
@@ -475,17 +478,33 @@ abstract public class UIXComponentBase e
@Override
public void setParent(UIComponent parent)
{
- _parent = parent;
+ if (parent != _parent)
+ {
+ _parent = parent;
+
+ // clear cached client ids if necessary
+ if (_clientId != null)
+ {
+ String newClientId = _calculateClientId(FacesContext.getCurrentInstance());
+
+ // if our clientId changed as a result of being reparented (because we moved
+ // between NamingContainers for instance) then we need to clear out
+ // all of the cached client ids for our subtree
+ if (!_clientId.equals(newClientId))
+ {
+ clearCachedClientIds();
+ _clientId = newClientId;
+ }
+ }
+ }
}
-
@Override
public boolean isRendered()
{
return getBooleanProperty(RENDERED_KEY, true);
}
-
@Override
public void setRendered(boolean rendered)
{
@@ -534,7 +553,6 @@ abstract public class UIXComponentBase e
setProperty(RENDERER_TYPE_KEY, rendererType);
}
-
@Override
public boolean getRendersChildren()
{
@@ -545,13 +563,8 @@ abstract public class UIXComponentBase e
return renderer.getRendersChildren();
}
-
-
-
// ------------------------------------------------ Tree Management Methods
-
-
@Override
public UIComponent findComponent(String id)
{
@@ -622,8 +635,6 @@ abstract public class UIXComponentBase e
}
}
-
-
/**
* <p>Create (if necessary) and return a List of the children associated
* with this component.</p>
@@ -646,7 +657,6 @@ abstract public class UIXComponentBase e
return getChildren().size();
}
-
/**
* <p>Create (if necessary) and return a Map of the facets associated
* with this component.</p>
@@ -661,7 +671,6 @@ abstract public class UIXComponentBase e
return _facets;
}
-
@Override
public UIComponent getFacet(String facetName)
{
@@ -674,7 +683,6 @@ abstract public class UIXComponentBase e
return getFacets().get(facetName);
}
-
/**
* Returns an Iterator over the names of all facets.
* Unlike getFacets().keySet().iterator(), this does
@@ -763,7 +771,6 @@ abstract public class UIXComponentBase e
// ------------------------------------------- Lifecycle Processing Methods
-
@Override
public void decode(FacesContext context)
{
@@ -1036,7 +1043,6 @@ abstract public class UIXComponentBase e
return FacesContext.getCurrentInstance();
}
-
/**
* Delegates to LifecycleRenderer, if present,
* otherwise calls decodeChildrenImpl.
@@ -1071,7 +1077,6 @@ abstract public class UIXComponentBase e
}
}
-
/**
* Delegates to LifecycleRenderer, if present,
* otherwise calls validateChildrenImpl.
@@ -1107,7 +1112,6 @@ abstract public class UIXComponentBase e
}
}
-
/**
* Delegates to LifecycleRenderer, if present,
* otherwise calls upateChildrenImpl.
@@ -1293,7 +1297,6 @@ abstract public class UIXComponentBase e
return n.intValue();
}
-
/**
* Return the number of facets. This is more efficient than
* calling getFacets().size();
@@ -1307,7 +1310,6 @@ abstract public class UIXComponentBase e
return _facets.size();
}
-
/**
* Broadcast an event to a MethodBinding.
* This can be used to support MethodBindings such as the "actionListener"
@@ -1460,7 +1462,6 @@ abstract public class UIXComponentBase e
}
}
-
/**
* Override to calls the hooks for setting up and tearing down the
* context before the children are visited.
@@ -1473,7 +1474,7 @@ abstract public class UIXComponentBase e
String clientId,
ContextCallback callback)
throws FacesException
- {
+ {
String thisClientId = getClientId(context);
if (clientId.equals(thisClientId))
@@ -1584,7 +1585,6 @@ abstract public class UIXComponentBase e
}
}
-
static private UIComponent _findInsideOf(
UIComponent from,
String id)
@@ -1713,7 +1713,6 @@ abstract public class UIXComponentBase e
//private transient boolean _initialStateMarked;
private static final Iterator<String> _EMPTY_STRING_ITERATOR = CollectionUtils.emptyIterator();
-
private static final Iterator<UIComponent> _EMPTY_UICOMPONENT_ITERATOR =
CollectionUtils.emptyIterator();
@@ -1768,6 +1767,44 @@ abstract public class UIXComponentBase e
{
}
+ /**
+ * Temporary function controlling whether clientId caching is enabled
+ */
+ private static boolean _isClientIdCachingEnabled(FacesContext context)
+ {
+ if (context == null)
+ throw new IllegalArgumentException("FacesContext is null");
+
+ Boolean cacheClientIds = _sClientIdCachingEnabled.get();
+
+ if (cacheClientIds == null)
+ {
+ // get the servlet initialization parameter
+ String cachingParam = context.getExternalContext().getInitParameter(
+ _INIT_PROP_CLIENT_ID_CACHING_ENABLED);
+
+ Boolean cachingEnabled = (cachingParam != null)
+ ? Boolean.valueOf(cachingParam)
+ : Boolean.FALSE; // default to false
+
+ // cache the servlet initialization value
+ _sClientIdCachingEnabled.set(cachingEnabled ? Boolean.TRUE : Boolean.FALSE);
+
+ return cachingEnabled;
+ }
+ else
+ {
+ return cacheClientIds.booleanValue();
+ }
+ }
+
+ private static AtomicReference<Boolean> _sClientIdCachingEnabled =
+ new AtomicReference<Boolean>(null);
+
+ // temporary servlet initialization flag controlling whether client ID caching is enabled
+ private static final String _INIT_PROP_CLIENT_ID_CACHING_ENABLED =
+ "org.apache.myfaces.trinidadinternal.ENABLE_CLIENT_ID_CACHING";
+
static private final LifecycleRenderer _UNDEFINED_LIFECYCLE_RENDERER =
new ExtendedRendererImpl();
static private final Renderer _UNDEFINED_RENDERER = new RendererImpl();
Added: 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=921628&view=auto
==============================================================================
--- myfaces/trinidad/trunk/trinidad-api/src/test/java/org/apache/myfaces/trinidad/component/ClientIdCachingTest.java (added)
+++ myfaces/trinidad/trunk/trinidad-api/src/test/java/org/apache/myfaces/trinidad/component/ClientIdCachingTest.java Wed Mar 10 23:34:23 2010
@@ -0,0 +1,143 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.myfaces.trinidad.component;
+
+import javax.faces.component.NamingContainer;
+import javax.faces.context.FacesContext;
+import javax.faces.render.Renderer;
+
+import junit.framework.Test;
+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()
+ {
+ return new TestSuite(ClientIdCachingTest.class);
+ }
+
+ public static void main(String[] args) throws Throwable
+ {
+ junit.textui.TestRunner.run(suite());
+ }
+
+ public ClientIdCachingTest(
+ String testName)
+ {
+ super(testName);
+ }
+
+ static private class TestPanel extends UIXPanel
+ {
+ protected Renderer getRenderer(FacesContext context)
+ {
+ return null;
+ }
+ }
+
+ static private class TestNamingContainer extends TestPanel
+ implements NamingContainer
+ {
+ }
+
+ // Test nested NamingContainer callbacks
+ @SuppressWarnings("unchecked")
+ public void testBasic()
+ {
+ TestNamingContainer a = new TestNamingContainer(); a.setId("a");
+ TestNamingContainer b = new TestNamingContainer(); b.setId("b");
+ TestNamingContainer d = new TestNamingContainer(); d.setId("d");
+ TestPanel e = new TestPanel(); e.setId("e");
+ TestPanel g = new TestPanel(); g.setId("g");
+ 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));
+ }
+
+ public void testSetId()
+ {
+ TestNamingContainer a = new TestNamingContainer(); a.setId("a");
+ TestNamingContainer b = new TestNamingContainer(); b.setId("b");
+ TestNamingContainer d = new TestNamingContainer(); d.setId("d");
+ TestPanel e = new TestPanel(); e.setId("e");
+ TestPanel g = new TestPanel(); g.setId("g");
+ 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
+ e.setId("ePrime");
+ assertEquals("a:b:d:ePrime", e.getClientId(context));
+
+ // set the component's id using attribute map
+ e.getAttributes().put("id", "eDoublePrime");
+ assertEquals("a:b:d:eDoublePrime", e.getClientId(context));
+
+
+ // set an ancsestor's id using accessor
+ b.setId("bPrime");
+ assertEquals("a:bPrime:d:eDoublePrime", e.getClientId(context));
+
+ // set the component's id using attribute map
+ b.getAttributes().put("id", "bDoublePrime");
+ assertEquals("a:bDoublePrime:d:eDoublePrime", e.getClientId(context));
+ }
+
+ public void testMoving()
+ {
+ TestNamingContainer a = new TestNamingContainer(); a.setId("a");
+ TestNamingContainer b = new TestNamingContainer(); b.setId("b");
+ TestNamingContainer c = new TestNamingContainer(); c.setId("c");
+ TestNamingContainer d = new TestNamingContainer(); d.setId("d");
+ TestPanel e = new TestPanel(); e.setId("e");
+ TestPanel f = new TestPanel(); f.setId("f");
+ TestPanel g = new TestPanel(); g.setId("g");
+ a.getChildren().add(b);
+ a.getChildren().add(c);
+ b.getChildren().add(d);
+ b.getChildren().add(g);
+ d.getChildren().add(e);
+ d.getChildren().add(f);
+
+ // prime
+ FacesContext context = FacesContext.getCurrentInstance();
+ assertEquals("a:b:d:e", e.getClientId(context));
+
+ // move within same NamingContainer--no clientId change
+ f.getChildren().add(e);
+ assertEquals("a:b:d:e", e.getClientId(context));
+
+ // move between NamingContainers
+ g.getChildren().add(e);
+ assertEquals("a:b:e", e.getClientId(context));
+ }
+}
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=921628&r1=921627&r2=921628&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 Wed Mar 10 23:34:23 2010
@@ -31,8 +31,9 @@ import org.apache.myfaces.trinidad.compo
import org.apache.myfaces.trinidad.component.UIXInput;
import org.apache.myfaces.trinidad.component.UIXPanel;
+import org.apache.myfaces.trinidadbuild.test.FacesTestCase;
-public class RenderUtilsTest extends TestCase
+public class RenderUtilsTest extends FacesTestCase
{
public static final Test suite()
{
@@ -89,6 +90,8 @@ public class RenderUtilsTest extends Tes
@SuppressWarnings("unchecked")
public void testButtonAndNamingContainerSiblings()
{
+ FacesContext context = FacesContext.getCurrentInstance();
+
// rootPanel
// button1
// table1
@@ -105,41 +108,41 @@ public class RenderUtilsTest extends Tes
table1.getChildren().add(tableChild);
String relativeId =
- RenderUtils.getRelativeId(null, button1, "table1");
+ RenderUtils.getRelativeId(context, button1, "table1");
assertEquals("table1", relativeId);
relativeId =
- RenderUtils.getRelativeId(null, button1, ":table1");
+ RenderUtils.getRelativeId(context, button1, ":table1");
assertEquals("table1", relativeId);
// new way would find nothing, so we'd have to get something logical
relativeId =
- RenderUtils.getRelativeId(null, table1, "someRandomId");
+ RenderUtils.getRelativeId(context, table1, "someRandomId");
assertEquals("table1_Client:someRandomId", relativeId);
relativeId =
- RenderUtils.getRelativeId(null, table1, ":commandButton1");
+ RenderUtils.getRelativeId(context, table1, ":commandButton1");
assertEquals("commandButton1", relativeId);
// to get to the commandButton from the table, you need to pop out of the
// table
relativeId =
- RenderUtils.getRelativeId(null, table1, "::commandButton1");
+ RenderUtils.getRelativeId(context, table1, "::commandButton1");
assertEquals("commandButton1", relativeId);
// backward compatibility test -- this was the old syntax for siblings to the table.
// this should be found by looking at the nc's parent from findRelativeComponent
relativeId =
- RenderUtils.getRelativeId(null, table1, "commandButton1");
+ RenderUtils.getRelativeId(context, table1, "commandButton1");
assertEquals("commandButton1", relativeId);
// backward compatibility test -- this was the old syntax for children to the table.
relativeId =
- RenderUtils.getRelativeId(null, table1, "table1:tableChildId");
+ RenderUtils.getRelativeId(context, table1, "table1:tableChildId");
assertEquals("table1:tableChildId", relativeId);
// this is the new syntax for children to the table
relativeId =
- RenderUtils.getRelativeId(null, table1, "tableChildId");
+ RenderUtils.getRelativeId(context, table1, "tableChildId");
assertEquals("table1:tableChildId", relativeId);
}
@@ -150,6 +153,7 @@ public class RenderUtilsTest extends Tes
@SuppressWarnings("unchecked")
public void testRelativeSearch()
{
+ FacesContext context = FacesContext.getCurrentInstance();
// set up component hierarchy
@@ -195,13 +199,13 @@ public class RenderUtilsTest extends Tes
*/
String relativeId =
- RenderUtils.getRelativeId(null, input1, "::button1");
+ RenderUtils.getRelativeId(context, input1, "::button1");
// new way should pop OUT of ONE naming container and will find it
assertEquals("ncRoot:button1", relativeId);
relativeId =
- RenderUtils.getRelativeId(null, input1, ":::button1");
+ RenderUtils.getRelativeId(context, input1, ":::button1");
// new way should pop OUT of TWO naming containers and will find not find it
// since it is in ncRoot and the base is now the view root.
// so it goes to the old findRelativeComponent, and this will find it.
@@ -209,27 +213,27 @@ public class RenderUtilsTest extends Tes
relativeId =
- RenderUtils.getRelativeId(null, input1, "randomPeer");
+ RenderUtils.getRelativeId(context, input1, "randomPeer");
// randomPeer doesn't exist, so new way won't find it.
// uses code that doesn't need to find the component to return this:
assertEquals("nc1_Client:randomPeer", relativeId);
relativeId =
- RenderUtils.getRelativeId(null, input1, "::randomPeer");
+ RenderUtils.getRelativeId(context, input1, "::randomPeer");
// randomPeer doesn't exist, so new way won't find it.
// uses code that doesn't need to find the component to return this:
assertEquals("ncRoot_Client:randomPeer", relativeId);
// rootButton is child of form and sibling to ncRoot. It's 2 nc up from input1
relativeId =
- RenderUtils.getRelativeId(null, input1, ":::rootButton");
+ RenderUtils.getRelativeId(context, input1, ":::rootButton");
// new way should pop OUT of both NC with ::: and will find it
assertEquals("rootButton", relativeId);
// rootButton is child of form and sibling to ncRoot. It's 2 nc up from input1
relativeId =
- RenderUtils.getRelativeId(null, input1, "::rootButton");
+ RenderUtils.getRelativeId(context, input1, "::rootButton");
// new way should pop OUT of one NC with ::, so it can't find it
// the 'old' findRelativeComponent can't find it either.
// so it returns what the old getRelativeId would have returned
@@ -240,19 +244,19 @@ public class RenderUtilsTest extends Tes
// rootButton is child of form and sibling to ncRoot. It's 2 nc up from input1
relativeId =
- RenderUtils.getRelativeId(null, input1, "::::rootButton");
+ RenderUtils.getRelativeId(context, input1, "::::rootButton");
// new way should pop OUT of ALL NCs and will find it.
assertEquals("rootButton", relativeId);
relativeId =
- RenderUtils.getRelativeId(null, input1, "::::button1");
+ RenderUtils.getRelativeId(context, input1, "::::button1");
// new way should return this
assertEquals("button1", relativeId);
relativeId =
- RenderUtils.getRelativeId(null, input1, ":::::button1");
+ RenderUtils.getRelativeId(context, input1, ":::::button1");
assertEquals("button1", relativeId);
}