You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@myfaces.apache.org by ar...@apache.org on 2011/05/02 23:24:00 UTC
svn commit: r1098796 - in /myfaces/trinidad/trunk/trinidad-api/src/main:
java/org/apache/myfaces/trinidad/component/UIXCollection.java
xrts/org/apache/myfaces/trinidad/resource/LoggerBundle.xrts
Author: arobinson74
Date: Mon May 2 21:24:00 2011
New Revision: 1098796
URL: http://svn.apache.org/viewvc?rev=1098796&view=rev
Log:
TRINIDAD-2096 - improve how the UIXCollection implements its component context change
Modified:
myfaces/trinidad/trunk/trinidad-api/src/main/java/org/apache/myfaces/trinidad/component/UIXCollection.java
myfaces/trinidad/trunk/trinidad-api/src/main/xrts/org/apache/myfaces/trinidad/resource/LoggerBundle.xrts
Modified: myfaces/trinidad/trunk/trinidad-api/src/main/java/org/apache/myfaces/trinidad/component/UIXCollection.java
URL: http://svn.apache.org/viewvc/myfaces/trinidad/trunk/trinidad-api/src/main/java/org/apache/myfaces/trinidad/component/UIXCollection.java?rev=1098796&r1=1098795&r2=1098796&view=diff
==============================================================================
--- myfaces/trinidad/trunk/trinidad-api/src/main/java/org/apache/myfaces/trinidad/component/UIXCollection.java (original)
+++ myfaces/trinidad/trunk/trinidad-api/src/main/java/org/apache/myfaces/trinidad/component/UIXCollection.java Mon May 2 21:24:00 2011
@@ -38,6 +38,7 @@ import javax.faces.component.visit.Visit
import javax.faces.component.visit.VisitResult;
import javax.faces.context.FacesContext;
import javax.faces.event.AbortProcessingException;
+import javax.faces.event.ComponentSystemEvent;
import javax.faces.event.FacesEvent;
import javax.faces.event.PhaseId;
import javax.faces.render.Renderer;
@@ -133,7 +134,10 @@ public abstract class UIXCollection exte
// rowKey/rowData):
Object currencyKey = getRowKey();
event = new TableRowEvent(this, event, currencyKey);
- super.queueEvent(event);
+
+ // Queue a CollectionContextEvent in order to allow this class to setup the component change
+ // before sub-classes attempt to process the table row event instance.
+ super.queueEvent(new CollectionContextEvent(this, event));
}
/**
@@ -146,20 +150,37 @@ public abstract class UIXCollection exte
public void broadcast(FacesEvent event)
throws AbortProcessingException
{
- // For "TableRowEvents", set up the data before firing the
- // event to the actual component.
- if (event instanceof TableRowEvent)
- {
- TableRowEvent rowEvent = (TableRowEvent) event;
- Object old = getRowKey();
- setRowKey(rowEvent.getCurrencyKey());
- FacesEvent wrapped = rowEvent.getEvent();
- wrapped.getComponent().broadcast(wrapped);
- setRowKey(old);
+ // Unwrap CollectionContextEvent events so that the original event is broadcast
+ // within a component change event context.
+ if (event instanceof CollectionContextEvent)
+ {
+ _setupContextChange();
+ try
+ {
+ this.broadcast(((CollectionContextEvent)event).getEvent());
+ }
+ finally
+ {
+ _tearDownContextChange();
+ }
}
else
{
- super.broadcast(event);
+ // For "TableRowEvents", set up the data before firing the
+ // event to the actual component.
+ if (event instanceof TableRowEvent)
+ {
+ TableRowEvent rowEvent = (TableRowEvent) event;
+ Object old = getRowKey();
+ setRowKey(rowEvent.getCurrencyKey());
+ FacesEvent wrapped = rowEvent.getEvent();
+ wrapped.getComponent().broadcast(wrapped);
+ setRowKey(old);
+ }
+ else
+ {
+ super.broadcast(event);
+ }
}
}
@@ -174,30 +195,38 @@ public abstract class UIXCollection exte
if (context == null)
throw new NullPointerException();
- _init();
+ _setupContextChange();
+ try
+ {
+ _init();
- InternalState iState = _getInternalState(true);
- iState._isFirstRender = false;
+ InternalState iState = _getInternalState(true);
+ iState._isFirstRender = false;
- if (!isRendered())
- return;
+ if (!isRendered())
+ return;
- __flushCachedModel();
+ __flushCachedModel();
- // Make sure _hasEvent is false.
- iState._hasEvent = false;
+ // Make sure _hasEvent is false.
+ iState._hasEvent = false;
- // =-=AEW Because I'm getting the state in decode(), I need
- // to do it before iterating over the children - otherwise,
- // they'll be working off the wrong startIndex. When state
- // management is integrated, I can likely put this back in the
- // usual order
+ // =-=AEW Because I'm getting the state in decode(), I need
+ // to do it before iterating over the children - otherwise,
+ // they'll be working off the wrong startIndex. When state
+ // management is integrated, I can likely put this back in the
+ // usual order
- // Process this component itself
- decode(context);
+ // Process this component itself
+ decode(context);
- // Process all facets and children of this component
- decodeChildren(context);
+ // Process all facets and children of this component
+ decodeChildren(context);
+ }
+ finally
+ {
+ _tearDownContextChange();
+ }
}
@Override
@@ -238,48 +267,56 @@ public abstract class UIXCollection exte
@Override
public Object processSaveState(FacesContext context)
{
- // If we saved state in the middle of processing a row,
- // then make sure that we revert to a "null" rowKey while
- // saving state; this is necessary to ensure that the
- // current row's state is properly preserved, and that
- // the children are reset to their default state.
- Object currencyKey = _getCurrencyKey();
-
- // since this is the end of the request, we expect the row currency to be reset back to null
- // setting it and leaving it there might introduce multiple issues, so log a warning here
- if (currencyKey != null)
- {
- if (_LOG.isWarning())
- {
- String scopedId = ComponentUtils.getScopedIdForComponent(this, context.getViewRoot());
- String viewId = context.getViewRoot()==null?null:context.getViewRoot().getViewId();
- _LOG.warning("ROWKEY_NOT_RESET", new Object[]{scopedId, viewId});
+ _setupContextChange();
+ try
+ {
+ // If we saved state in the middle of processing a row,
+ // then make sure that we revert to a "null" rowKey while
+ // saving state; this is necessary to ensure that the
+ // current row's state is properly preserved, and that
+ // the children are reset to their default state.
+ Object currencyKey = _getCurrencyKey();
+
+ // since this is the end of the request, we expect the row currency to be reset back to null
+ // setting it and leaving it there might introduce multiple issues, so log a warning here
+ if (currencyKey != null)
+ {
+ if (_LOG.isWarning())
+ {
+ String scopedId = ComponentUtils.getScopedIdForComponent(this, context.getViewRoot());
+ String viewId = context.getViewRoot()==null?null:context.getViewRoot().getViewId();
+ _LOG.warning("ROWKEY_NOT_RESET", new Object[]{scopedId, viewId});
+ }
}
- }
- Object initKey = _getCurrencyKeyForInitialStampState();
- if (currencyKey != initKey) // beware of null currencyKeys if equals() is used
- {
- setRowKey(initKey);
- }
+ Object initKey = _getCurrencyKeyForInitialStampState();
+ if (currencyKey != initKey) // beware of null currencyKeys if equals() is used
+ {
+ setRowKey(initKey);
+ }
- Object savedState = super.processSaveState(context);
+ Object savedState = super.processSaveState(context);
- if (currencyKey != initKey) // beware of null currencyKeys if equals() is used
- {
- setRowKey(currencyKey);
- }
+ if (currencyKey != initKey) // beware of null currencyKeys if equals() is used
+ {
+ setRowKey(currencyKey);
+ }
- // Finally clean up any internal model state that we might be holding on to. We do not want to hold onto any
- // application data in between requests
- InternalState iState = _getInternalState(false);
- if (iState != null)
+ // Finally clean up any internal model state that we might be holding on to. We do not want to hold onto any
+ // application data in between requests
+ InternalState iState = _getInternalState(false);
+ if (iState != null)
+ {
+ iState._value = null;
+ iState._model= null;
+ }
+
+ return savedState;
+ }
+ finally
{
- iState._value = null;
- iState._model= null;
+ _tearDownContextChange();
}
-
- return savedState;
}
@Override
@@ -509,6 +546,8 @@ public abstract class UIXCollection exte
*/
public void setRowKey(Object rowKey)
{
+ _verifyComponentInContext();
+
preRowDataChange();
getCollectionModel().setRowKey(rowKey);
postRowDataChange();
@@ -526,6 +565,8 @@ public abstract class UIXCollection exte
*/
public void setRowIndex(int rowIndex)
{
+ _verifyComponentInContext();
+
preRowDataChange();
getCollectionModel().setRowIndex(rowIndex);
postRowDataChange();
@@ -586,37 +627,73 @@ public abstract class UIXCollection exte
@Override
public final void encodeBegin(FacesContext context) throws IOException
{
- _init();
+ _setupContextChange();
+ boolean teardown = true;
+ try
+ {
+ _init();
- InternalState istate = _getInternalState(true);
- // we must not clear the currency cache everytime. only clear
- // it in response to specific events: bug 4773659
-
- // TODO all this code should be removed and moved into the renderer:
- if (istate._clearTokenCache)
- {
- istate._clearTokenCache = false;
- ClientRowKeyManager keyMgr = getClientRowKeyManager();
- if (keyMgr instanceof DefaultClientKeyManager)
- ((DefaultClientKeyManager) keyMgr).clear();
- }
- __flushCachedModel();
+ InternalState istate = _getInternalState(true);
+ // we must not clear the currency cache everytime. only clear
+ // it in response to specific events: bug 4773659
+
+ // TODO all this code should be removed and moved into the renderer:
+ if (istate._clearTokenCache)
+ {
+ istate._clearTokenCache = false;
+ ClientRowKeyManager keyMgr = getClientRowKeyManager();
+ if (keyMgr instanceof DefaultClientKeyManager)
+ ((DefaultClientKeyManager) keyMgr).clear();
+ }
+ __flushCachedModel();
+
+ Object assertKey = null;
+ assert ((assertKey = getRowKey()) != null) || true;
+ __encodeBegin(context);
+ // make sure that the rendering code preserves the currency:
+ assert _assertKeyPreserved(assertKey) : "CurrencyKey not preserved";
- Object assertKey = null;
- assert ((assertKey = getRowKey()) != null) || true;
- __encodeBegin(context);
- // make sure that the rendering code preserves the currency:
- assert _assertKeyPreserved(assertKey) : "CurrencyKey not preserved";
+ teardown = false;
+ }
+ finally
+ {
+ if (teardown)
+ {
+ // Tear down on errors & exceptions
+ _tearDownContextChange();
+ }
+ }
}
@Override
public void encodeEnd(FacesContext context) throws IOException
{
- Object assertKey = null;
- assert ((assertKey = getRowKey()) != null) || true;
- super.encodeEnd(context);
- // make sure that the rendering code preserves the currency:
- assert _assertKeyPreserved(assertKey) : "CurrencyKey not preserved";
+ try
+ {
+ Object assertKey = null;
+ assert ((assertKey = getRowKey()) != null) || true;
+ super.encodeEnd(context);
+ // make sure that the rendering code preserves the currency:
+ assert _assertKeyPreserved(assertKey) : "CurrencyKey not preserved";
+ }
+ finally
+ {
+ _tearDownContextChange();
+ }
+ }
+
+ @Override
+ protected void setupVisitingContext(FacesContext context)
+ {
+ super.setupVisitingContext(context);
+ _setupContextChange();
+ }
+
+ @Override
+ protected void tearDownVisitingContext(FacesContext context)
+ {
+ _tearDownContextChange();
+ super.tearDownVisitingContext(context);
}
private boolean _assertKeyPreserved(Object oldKey)
@@ -740,6 +817,61 @@ public abstract class UIXCollection exte
setRowKey(rowkey);
}
+ public void processRestoreState(
+ FacesContext context,
+ Object state)
+ {
+ _setupContextChange();
+ try
+ {
+ super.processRestoreState(context, state);
+ }
+ finally
+ {
+ _tearDownContextChange();
+ }
+ }
+
+ public void processUpdates(FacesContext context)
+ {
+ _setupContextChange();
+ try
+ {
+ super.processUpdates(context);
+ }
+ finally
+ {
+ _tearDownContextChange();
+ }
+ }
+
+ public void processValidators(FacesContext context)
+ {
+ _setupContextChange();
+ try
+ {
+ super.processValidators(context);
+ }
+ finally
+ {
+ _tearDownContextChange();
+ }
+ }
+
+ public void processEvent(ComponentSystemEvent event)
+ throws AbortProcessingException
+ {
+ _setupContextChange();
+ try
+ {
+ super.processEvent(event);
+ }
+ finally
+ {
+ _tearDownContextChange();
+ }
+ }
+
/**
* Gets the client-id of this component, without any NamingContainers.
* This id changes depending on the currency Object.
@@ -794,19 +926,6 @@ public abstract class UIXCollection exte
" and currencyKey:"+getRowKey());
}
- ComponentContextManager compCtxMgr = null;
- if (!_inSuspendOrResume)
- {
- compCtxMgr = RequestContext.getCurrentInstance().getComponentContextManager();
- ComponentContextChange change = compCtxMgr.peekChange();
- if (change instanceof CollectionComponentChange &&
- ((CollectionComponentChange)change)._component == this)
- {
- // Remove the component context change if one was added
- compCtxMgr.popChange();
- }
- }
-
InternalState iState = _getInternalState(true);
if (rowData == null)
{
@@ -840,13 +959,6 @@ public abstract class UIXCollection exte
Map<String, Object> varStatusMap = createVarStatusMap();
iState._prevVarStatus = _setELVar(iState._varStatus, varStatusMap);
}
-
- // If there is a current row, push a component change so that we may clear the
- // var and var status should a visit tree occur
- if (!_inSuspendOrResume)
- {
- compCtxMgr.pushChange(new CollectionComponentChange(this));
- }
}
_restoreStampState();
@@ -1962,6 +2074,61 @@ public abstract class UIXCollection exte
return b.equals(a);
}
+ private void _setupContextChange()
+ {
+ ComponentContextManager compCtxMgr =
+ RequestContext.getCurrentInstance().getComponentContextManager();
+
+ compCtxMgr.pushChange(new CollectionComponentChange(this));
+ }
+
+ private void _tearDownContextChange()
+ {
+ try
+ {
+ ComponentContextManager compCtxMgr =
+ RequestContext.getCurrentInstance().getComponentContextManager();
+ ComponentContextChange change = compCtxMgr.peekChange();
+
+ if (change instanceof CollectionComponentChange &&
+ ((CollectionComponentChange)change)._component == this)
+ {
+ // Remove the component context change if one was added
+ compCtxMgr.popChange();
+ }
+ else
+ {
+ _LOG.severe("COLLECTION_CHANGE_TEARDOWN", new Object[] { getId(), change });
+ }
+ }
+ catch (RuntimeException re)
+ {
+ _LOG.severe(re);
+ }
+ }
+
+ private void _verifyComponentInContext()
+ {
+ if (_inSuspendOrResume)
+ {
+ return;
+ }
+
+ ComponentContextManager compCtxMgr =
+ RequestContext.getCurrentInstance().getComponentContextManager();
+ ComponentContextChange change = compCtxMgr.peekChange();
+
+ if (!(change instanceof CollectionComponentChange) ||
+ ((CollectionComponentChange)change)._component != this)
+ {
+ _LOG.warning("COLLECTION_NOT_IN_CONTEXT", getId());
+ if (_LOG.isFine())
+ {
+ Thread.currentThread().dumpStack();
+ }
+ }
+ }
+
private static final class DefaultClientKeyManager extends ClientRowKeyManager
{
public void clear()
@@ -2095,11 +2262,12 @@ public abstract class UIXCollection exte
public void suspend(
FacesContext facesContext)
{
- _oldRowIndex = _component.getRowIndex();
+ _rowKey = _component.getRowKey();
+
_component._inSuspendOrResume = true;
try
{
- _component.setRowIndex(-1);
+ _component.setRowKey(null);
}
finally
{
@@ -2113,7 +2281,7 @@ public abstract class UIXCollection exte
_component._inSuspendOrResume = true;
try
{
- _component.setRowIndex(_oldRowIndex);
+ _component.setRowKey(_rowKey);
}
finally
{
@@ -2136,7 +2304,21 @@ public abstract class UIXCollection exte
}
private final UIXCollection _component;
- private int _oldRowIndex;
+ private Object _rowKey;
+ }
+
+ private static class CollectionContextEvent
+ extends WrapperEvent
+ {
+ public CollectionContextEvent(
+ UIComponent source,
+ FacesEvent event)
+ {
+ super(source, event);
+ }
+
+ @SuppressWarnings("compatibility:-7639429485707197863")
+ private static final long serialVersionUID = 1L;
}
// do not assign a non-null value. values should be assigned lazily. this is
Modified: myfaces/trinidad/trunk/trinidad-api/src/main/xrts/org/apache/myfaces/trinidad/resource/LoggerBundle.xrts
URL: http://svn.apache.org/viewvc/myfaces/trinidad/trunk/trinidad-api/src/main/xrts/org/apache/myfaces/trinidad/resource/LoggerBundle.xrts?rev=1098796&r1=1098795&r2=1098796&view=diff
==============================================================================
--- myfaces/trinidad/trunk/trinidad-api/src/main/xrts/org/apache/myfaces/trinidad/resource/LoggerBundle.xrts (original)
+++ myfaces/trinidad/trunk/trinidad-api/src/main/xrts/org/apache/myfaces/trinidad/resource/LoggerBundle.xrts Mon May 2 21:24:00 2011
@@ -477,4 +477,10 @@
<!-- ROWKEY_NOT_RESET -->
<resource key="ROWKEY_NOT_RESET">row key might not be reset correctly at the end of the request. Component ID: {0} ViewId: {1}</resource>
+<!-- COLLECTION_CHANGE_TEARDOWN -->
+<resource key="COLLECTION_CHANGE_TEARDOWN">The component change that was on the stack is not the required one to properly tear down the context of this component. The component tree is in an invalid state and further errors may result. Component ID: {0} Invalid component change: {1}</resource>
+
+<!-- COLLECTION_NOT_IN_CONTEXT -->
+<resource key="COLLECTION_NOT_IN_CONTEXT">The row key or row index of a UIXCollection component is being changed outside of the component's context. Changing the key or index of a collection when the collection is not currently being visited, invoked on, broadcasting an event or processing a lifecycle method, is not valid. Data corruption and errors may result from this call. Component ID: {0}. Turn on fine level logging for a stack trace of this call.</resource>
+
</resources>