[jsr-314-open-mirror] [jsr-314-open] Fwd: Fix UIData state saving model

[Leonardo Uribe]

Hi

I attached a patch with better documentation for TransientStateHelper -
TransientStateHolder and the answers requested. If you need anything
else just let me know and I'll do it.

 I resume the comments below:

EB> Leonardo, why is the preceding state traversal saved into
EB> this funky two element array, whereas the next one, described
EB> below, saved into the normal Map?

It is necessary to save the "initial rolled up state" in a way that the
state could be restored without depend of  the row where the
component or the model is, so an implementation option could
be use an array like structure, so traversing it could restore the state.
Since the row index is attached to the client id of child components,
use a Map in this case will work on simple datatables but will not work
on nested datatables correctly.

EB>Traverse the children as done with markInitialState, saving the
EB>state of each child.  Because the saveState calls during this pass
EB>happen after the call to markInitialState, the state saved is the
EB>so called "delta" state of each component. Because this
EB>traversal happens during a per-row operation (in this case,
EB>setRowIndex) the rolled up state must be saved in a row-aware
EB>data structure.  One implementation choice would be to save
EB>the state from this pass in a Map keyed by the return from
EB>javax.faces.component.UIComponent#getContainerClientId.

The algorithm uses as key is call child.getClientId() that calls
UIData.getContainerClientId() later, but other alternative could be
use child.getClientId() but trim the value from
UIData.getContainerClientId(). That makes easier fix the
problem when a row is deleted.

EB> Why do we need to bother passing null to
EB> restoreTransientState()?  There must be a good reason, but I need
EB> to include it in the specification.</p>

It is necessary to pass null to restoreTransientState()
to indicate the transient state should discarded from the previous one,
otherwise the transient state could be shared between rows.



To include the patch in jsf 2.1, we need to resolve the following point
left:

- On UIData.markInitialState do not use a facesContext attribute
"com.sun.faces.MARK_INITIAL_STATE", instead it could be better to use
a variable on FacesContext to indicate when we are marking the initial
state,
like FacesContext.isProcessingEvents() works.

It could be event better to add flags to indicate when the current view is
being
built using pss and when the view is being refreshed. It comes in handy in
some
situations, but that's another issue (and I'm not found yet strong arguments
to
support this more than myfaces PSS solution uses it in multiple places).

best regards,

Leonardo

2010/8/26 Martin Marinschek <mmarinschek at apache.org>

> Hi all,
>
> thanks to Ed and Leonardo for this discussion - I really believe this
> is a positive example how any spec work should go ;)
>
> @Leonardo: if you need help with the wording, just ping me.
>
> @everyone else who is interested in this: please can you also invest
> some work to review this? this is really a delicate issue, and might
> change quite a lot of the UIData behaviour, so it makes sense you
> invest the time.
>
> best regards,
>
> Martin
>
> On 8/26/10, Ed Burns <edward.burns at oracle.com> wrote:
> >>>>>> On Wed, 25 Aug 2010 20:12:59 -0500, Leonardo Uribe <
> lu4242 at gmail.com>
> >>>>>> said:
> >
> > LU> The problem with allow it with a context-param is there is no way to
> > LU> "reset" the deltas or remove rows to synchronize the model with the
> > LU> component state.
> >
> > LU> Suppose a simple use case, where there is a datatable and an option
> > LU> that removes one row. Since we don't have a way to notify the
> > LU> component that the row has been removed the state of the deleted row
> > LU> will be applied on the next one.
> >
> > Ok, I see.
> >
> > Now, when I discussed with the expert community at a recent meeting the
> > process for accepting new features for 2.1, I tried to be as clear as
> > possible that we'd need the contributing party to do *all* of the work.
> > Leorando, you've done the code, and it's great, but the spec language is
> > even more important since this body, technically, is partially
> > responsible for the spec.  Oracle, as the sponsor, is solely responsible
> > for the impl.  Now, as I said, I'm delighted with your contribution of
> > an implementation for this feature, but I need you to author as much as
> > possible of the spec text.
> >
> > I've started by documenting the preserveRowComponentState property in
> > the UIData.setPreserveRowComponentState() method.
> >
> > Please take a look at the javadoc I've generated in this attachment [1]
> > to issue 153 [2].  I've posed some questions in there, you can anwser
> > them here.  Also, please correct my where I've gone wrong.
> >
> > If you could submit a text file with sketches of the javadoc for other
> > elements of this feature, for example, the new TransientStateHelper
> > interface, it would really increase the likelihood of this feature
> > making it into 2.1.
> >
> > [1]
> >
> https://javaserverfaces-spec-public.dev.java.net/nonav/issues/showattachment.cgi/271/153-javadocs.zip
> >
> > [2]
> >
> https://javaserverfaces-spec-public.dev.java.net/issues/show_bug.cgi?id=153
> >
> > --
> > | edward.burns at oracle.com | office: +1 407 458 0017
> > | homepage:               | http://ridingthecrest.com/
> > | 12 work days until JSF 2.1 Milestone 3
> >
>
>
> --
>
> http://www.irian.at
>
> Your JSF powerhouse -
> JSF Consulting, Development and
> Courses in English and German
>
> Professional Support for Apache MyFaces
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.jboss.org/pipermail/jsr-314-open-mirror/attachments/20100826/d791d290/attachment-0002.html