Hey Martin - On 10/20/10 1:18 PM, Martin Marinschek wrote: Great. Yep. Perhaps. Though if we have get/putTransient() and save/restoreTransientState() imlementations on UIComponent itself - ie. if all UIComponents implicitly support transient state, it isn't clear to me how much value is added by additionally providing TransientStateHelper, and come to think of it, even TransientStateHolder. We don't have any code that currently tests for/casts to TransientStateHolder. Do we expect that some classes other than UIComponent will participate in transient state saving in the future? In any case, I have attached two Mojarra patches that demonstrate possible solutions to the issues that we have been discussing: The first patch does the following: - Serializable -> Object for keys - TransientStateHelper no longer extends StateHelper - UIComponent.getTransientStateHelper() methods are public - The 1-arg versions of getTransientStateHelper() and getTransient() are now final (convenience methods) - While I was in the code, I optimized the save/restoreTransientState() methods to avoid unnecessary instantiation of the ComponentStateHelper The second patch takes a different approach: - Serializable -> Object for keys - TransientStateHelper and TransientStateHolder are no longer used (and can/should be removed) - UIComponent.getTransientStateHelper() methods have been removed - get/putTransient() methods are now exposed on UIComponent - The 1-arg version of getTransient() is now final (convenience method) The second approach seems conceptually simpler to me - no need to introduce the TransientStateHelper/TransientStateHolder concepts/contracts. I don't see much loss of functionality with this approach. Yes, without TransientStateHolder, only UIComponents can participate in transient state saving, but at the moment I don't see the use case for non-UIComponent-centric transient state saving. If this use case comes up in the future, we could easily re-introduce TransientStateHolder. I think I prefer the second approach, but the most important thing is that we get some solution in before 2.1 goes final. Thoughts? Andy Index: jsf-api/src/main/java/javax/faces/component/UIComponent.java =================================================================== --- jsf-api/src/main/java/javax/faces/component/UIComponent.java (revision 8675) +++ jsf-api/src/main/java/javax/faces/component/UIComponent.java (working copy) @@ -251,7 +251,7 @@ * on what has been set. */ List<String> attributesThatAreSet; - TransientStateHelper stateHelper = null; + ComponentStateHelper stateHelper = null; UIComponent compositeParent; @@ -547,7 +547,7 @@ * @since 2.1 */ - protected TransientStateHelper getTransientStateHelper() + public final TransientStateHelper getTransientStateHelper() { return getTransientStateHelper(true); } @@ -565,7 +565,7 @@ * @since 2.1 */ - protected TransientStateHelper getTransientStateHelper(boolean create) { + public TransientStateHelper getTransientStateHelper(boolean create) { if (create && stateHelper == null) { stateHelper = new ComponentStateHelper(this); @@ -584,7 +584,12 @@ public void restoreTransientState(FacesContext context, Object state) { - getTransientStateHelper().restoreTransientState(context, state); + boolean forceCreate = (state != null); + TransientStateHelper helper = getTransientStateHelper(forceCreate); + + if (helper != null) { + helper.restoreTransientState(context, state); + } } /** @@ -597,7 +602,9 @@ public Object saveTransientState(FacesContext context) { - return getTransientStateHelper().saveTransientState(context); + TransientStateHelper helper = getTransientStateHelper(false); + + return (helper == null) ? null : helper.saveTransientState(context); } private boolean isInView; Index: jsf-api/src/main/java/javax/faces/component/ComponentStateHelper.java =================================================================== --- jsf-api/src/main/java/javax/faces/component/ComponentStateHelper.java (revision 8675) +++ jsf-api/src/main/java/javax/faces/component/ComponentStateHelper.java (working copy) @@ -56,13 +56,13 @@ * of the StateHolder interface. */ @SuppressWarnings({"unchecked"}) -class ComponentStateHelper implements StateHelper , TransientStateHelper, TransientStateHolder { +class ComponentStateHelper implements StateHelper , TransientStateHelper { private UIComponent component; private boolean isTransient; private Map<Serializable, Object> deltaMap; private Map<Serializable, Object> defaultMap; - private Map<Serializable, Object> transientState; + private Map<Object, Object> transientState; // ------------------------------------------------------------ Constructors @@ -444,12 +444,12 @@ } - public Object getTransient(Serializable key) + public Object getTransient(Object key) { return (transientState == null) ? null : transientState.get(key); } - public Object getTransient(Serializable key, Object defaultValue) + public Object getTransient(Object key, Object defaultValue) { Object returnValue = (transientState == null) ? null : transientState.get(key); if (returnValue != null) @@ -459,11 +459,11 @@ return defaultValue; } - public Object putTransient(Serializable key, Object value) + public Object putTransient(Object key, Object value) { if (transientState == null) { - transientState = new HashMap<Serializable, Object>(); + transientState = new HashMap<Object, Object>(); } return transientState.put(key, value); } @@ -471,7 +471,7 @@ @SuppressWarnings("unchecked") public void restoreTransientState(FacesContext context, Object state) { - transientState = (Map<Serializable, Object>) state; + transientState = (Map<Object, Object>) state; } public Object saveTransientState(FacesContext context) Index: jsf-api/src/main/java/javax/faces/component/TransientStateHelper.java =================================================================== --- jsf-api/src/main/java/javax/faces/component/TransientStateHelper.java (revision 8675) +++ jsf-api/src/main/java/javax/faces/component/TransientStateHelper.java (working copy) @@ -61,7 +61,7 @@ * @since 2.1 * */ -public interface TransientStateHelper extends StateHelper, TransientStateHolder +public interface TransientStateHelper extends TransientStateHolder { /** * <p class="changed_added_2_1">Return the value currently @@ -69,7 +69,7 @@ * @param key the key for which the value should be returned. * @since 2.1 */ - public Object getTransient(Serializable key); + public Object getTransient(Object key); /** * <p class="changed_added_2_1">Performs the same logic as {@link @@ -81,7 +81,7 @@ * the call to <code>get()</code>. * @since 2.1 */ - public Object getTransient(Serializable key, Object defaultValue); + public Object getTransient(Object key, Object defaultValue); /** * <p class="changed_added_2_1">Return the previously stored value @@ -93,5 +93,5 @@ * @param value the value * @since 2.1 */ - public Object putTransient(Serializable key, Object value); + public Object putTransient(Object key, Object value); } Index: jsf-api/src/main/java/javax/faces/component/UIComponent.java =================================================================== --- jsf-api/src/main/java/javax/faces/component/UIComponent.java (revision 8675) +++ jsf-api/src/main/java/javax/faces/component/UIComponent.java (working copy) @@ -109,7 +109,7 @@ */ -public abstract class UIComponent implements PartialStateHolder, TransientStateHolder, SystemEventListenerHolder, +public abstract class UIComponent implements PartialStateHolder, SystemEventListenerHolder, ComponentSystemEventListener { private static Logger LOGGER = Logger.getLogger("javax.faces.component", @@ -251,7 +251,7 @@ * on what has been set. */ List<String> attributesThatAreSet; - TransientStateHelper stateHelper = null; + ComponentStateHelper stateHelper = null; UIComponent compositeParent; @@ -538,43 +538,6 @@ } /** - * <p class="changed_added_2_1">Return the {@link - * TransientStateHelper} instance for this <code>UIComponent</code> - * instance. The default implementation simply calls through to - * {@link #getTransientStateHelper(boolean)} passing <code>true</code> - * as the argument.</p> - * - * @since 2.1 - */ - - protected TransientStateHelper getTransientStateHelper() - { - return getTransientStateHelper(true); - } - - /** - * <p class="changed_added_2_1">Return the {@link - * TransientStateHelper} instance for this <code>UIComponent</code> - * instance.</p> - * - * @param create if <code>true</code> create, if necessary, any - * internal data structures. If <code>false</code>, do not create - * any instances. In this case, it is possible for this method to - * return <code>null</code>. - * - * @since 2.1 - */ - - protected TransientStateHelper getTransientStateHelper(boolean create) { - - if (create && stateHelper == null) { - stateHelper = new ComponentStateHelper(this); - } - return stateHelper; - - } - - /** * <p class="changed_added_2_1">For components that need to support * the concept of transient state, this method will restore any * state saved on a prior call to {@link #saveTransientState}.</p> @@ -584,7 +547,13 @@ public void restoreTransientState(FacesContext context, Object state) { - getTransientStateHelper().restoreTransientState(context, state); + if ((state != null) && stateHelper == null) { + stateHelper = new ComponentStateHelper(this); + } + + if (stateHelper != null) { + stateHelper.restoreTransientState(context, state); + } } /** @@ -597,12 +566,30 @@ public Object saveTransientState(FacesContext context) { - return getTransientStateHelper().saveTransientState(context); + return (stateHelper != null) ? stateHelper.saveTransientState(context) : null; } - private boolean isInView; + public final Object getTransient(Object key) + { + return getTransient(key, null); + } + public Object getTransient(Object key, Object defaultValue) + { + return (stateHelper == null) ? null : stateHelper.getTransient(key, defaultValue); + } + public Object putTransient(Object key, Object value) + { + if ((value != null) && stateHelper == null) { + stateHelper = new ComponentStateHelper(this); + } + + return (stateHelper != null) ? stateHelper.putTransient(key, value) : null; + } + + private boolean isInView; + /** * <p class="changed_added_2_0">Return <code>true</code> if this * component is within the view hierarchy otherwise Index: jsf-api/src/main/java/javax/faces/component/ComponentStateHelper.java =================================================================== --- jsf-api/src/main/java/javax/faces/component/ComponentStateHelper.java (revision 8675) +++ jsf-api/src/main/java/javax/faces/component/ComponentStateHelper.java (working copy) @@ -56,13 +56,13 @@ * of the StateHolder interface. */ @SuppressWarnings({"unchecked"}) -class ComponentStateHelper implements StateHelper , TransientStateHelper, TransientStateHolder { +class ComponentStateHelper implements StateHelper { private UIComponent component; private boolean isTransient; private Map<Serializable, Object> deltaMap; private Map<Serializable, Object> defaultMap; - private Map<Serializable, Object> transientState; + private Map<Object, Object> transientState; // ------------------------------------------------------------ Constructors @@ -444,12 +444,12 @@ } - public Object getTransient(Serializable key) + public Object getTransient(Object key) { return (transientState == null) ? null : transientState.get(key); } - public Object getTransient(Serializable key, Object defaultValue) + public Object getTransient(Object key, Object defaultValue) { Object returnValue = (transientState == null) ? null : transientState.get(key); if (returnValue != null) @@ -459,11 +459,11 @@ return defaultValue; } - public Object putTransient(Serializable key, Object value) + public Object putTransient(Object key, Object value) { if (transientState == null) { - transientState = new HashMap<Serializable, Object>(); + transientState = new HashMap<Object, Object>(); } return transientState.put(key, value); } @@ -471,7 +471,7 @@ @SuppressWarnings("unchecked") public void restoreTransientState(FacesContext context, Object state) { - transientState = (Map<Serializable, Object>) state; + transientState = (Map<Object, Object>) state; } public Object saveTransientState(FacesContext context) Index: jsf-api/src/main/java/javax/faces/component/UIForm.java =================================================================== --- jsf-api/src/main/java/javax/faces/component/UIForm.java (revision 8675) +++ jsf-api/src/main/java/javax/faces/component/UIForm.java (working copy) @@ -141,12 +141,12 @@ * <p class="changed_modified_2_1">This property must be kept as a * transient property using the {@link - * UIComponent#getTransientStateHelper}.</p> + * UIComponent#putTransient}.</p> */ public boolean isSubmitted() { //return (this.submitted); - return (Boolean) getTransientStateHelper().getTransient(PropertyKeys.submitted, false); + return (Boolean) getTransient(PropertyKeys.submitted, false); } @@ -166,12 +166,12 @@ * <p class="changed_modified_2_1">This property must be kept as a * transient property using the {@link - * UIComponent#getTransientStateHelper}.</p> + * UIComponent#putTransient}.</p> */ public void setSubmitted(boolean submitted) { //this.submitted = submitted; - getTransientStateHelper().putTransient(PropertyKeys.submitted, submitted); + putTransient(PropertyKeys.submitted, submitted); } /**