[jsr-314-open-mirror] [jsr-314-open] [2.1 Spec Review] Transient State [Was: Fix UIData state saving model]

[Andy Schwartz]

Hey Martin -

On 10/20/10 1:18 PM, Martin Marinschek wrote:
> Hi Andy,
>
> I agree with the points:
>
> - get/putTransient should be on UIComponent, or TransientStateHelper
> needs to be exposed
> - no Serializable keys are necessary
>   

Great.

> But - I am not sure about removing the TransientStateHelper
> contract... It doesn't need to extend from StateHelper,

Yep.

>  that is true -
> but won't the code look funny if state-saved property handling looks
> different from transient property handling?
>   

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

> best regards,
>
> Martin
>
>   

-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: transient-state.patch
Url: http://lists.jboss.org/pipermail/jsr-314-open-mirror/attachments/20101021/eb8493cf/attachment-0004.pl 
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: transient-state-take2.patch
Url: http://lists.jboss.org/pipermail/jsr-314-open-mirror/attachments/20101021/eb8493cf/attachment-0005.pl