Return-Path: X-Original-To: apmail-myfaces-commits-archive@www.apache.org Delivered-To: apmail-myfaces-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 96EFA3424 for ; Mon, 2 May 2011 21:24:22 +0000 (UTC) Received: (qmail 23792 invoked by uid 500); 2 May 2011 21:24:22 -0000 Delivered-To: apmail-myfaces-commits-archive@myfaces.apache.org Received: (qmail 23716 invoked by uid 500); 2 May 2011 21:24:22 -0000 Mailing-List: contact commits-help@myfaces.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: "MyFaces Development" Delivered-To: mailing list commits@myfaces.apache.org Received: (qmail 23709 invoked by uid 99); 2 May 2011 21:24:22 -0000 Received: from athena.apache.org (HELO athena.apache.org) (140.211.11.136) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 02 May 2011 21:24:22 +0000 X-ASF-Spam-Status: No, hits=-2000.0 required=5.0 tests=ALL_TRUSTED X-Spam-Check-By: apache.org Received: from [140.211.11.4] (HELO eris.apache.org) (140.211.11.4) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 02 May 2011 21:24:20 +0000 Received: by eris.apache.org (Postfix, from userid 65534) id 81B042388A19; Mon, 2 May 2011 21:24:00 +0000 (UTC) Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: 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 Date: Mon, 02 May 2011 21:24:00 -0000 To: commits@myfaces.apache.org From: arobinson74@apache.org X-Mailer: svnmailer-1.0.8 Message-Id: <20110502212400.81B042388A19@eris.apache.org> 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 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 @@ row key might not be reset correctly at the end of the request. Component ID: {0} ViewId: {1} + +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} + + +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. +