[jsr-314-open-mirror] [jsr-314-open] Fix UIData state saving model (issue 153)

[Leonardo Uribe]

Hi

I attached on myfaces issue an alternate patch, fixing the problem described

before. Really, there is a lot of ways to solve this issue, the important
here is
propose concrete alternatives, so the people that has the decision could do
it taking into account as much information as possible.

As a side note, I checked if localValueSet is a transient property, but it
is not,
it always has been saved and restored from myfaces 1.1.2 and on mojarra
2.0.x it is saved too. There are some few tomahawk components that has
transient properties, but the only case in jsf api is UIForm.submitted
(maybe
"transient" property, but I don't think so).

regards,

Leonardo Uribe

2010/3/19 Leonardo Uribe <lu4242 at gmail.com>

> Hi
>
> Yes, I know this too. In few words, all "transient" properties, that means,
>
> all properties that are not stored in the state are not saved. There are
> very
> few examples. To solve this one, there are multiple alternatives. I was
> thinking
> on extend StateHelper interface to add some methods like
> putTransientProperty and getTransientProperty. Trinidad stores all
> properties
> on the FacesBean object.
>
> Anyway, I think an "mixed" solution could solve this problem: create an
> interface called RowStateTransientHolder, so only the components that
> implements this type of attributes could have the chance to
> save and restore its state.
>
> But note in any case, it is necessary to do something like this to solve
> the
> use case described.
>
> I didn't knew that localValueSet was a transient property too.
>
> regards,
>
> Leonardo Uribe
>
> 2010/3/19 Alexander Smirnov <asmirnov at exadel.com>
>
> I've thought about these problems for a long time too. Your proposal has
>> a disadvantage because per-row state is quite different from the
>> component state. Take a look for the UIInput implementation:
>> localValueSet field is never saved in the state while it should be vary
>> from row to row. The same thing happens to the UIForm.submitted
>> attribute, that has been eventually added to the UIData spec instead of
>> common solution, that would save/restore all local variables. My
>> proposal was a separate RowStateHolder interface with
>> save/restoreRowState methods there component developer can preserve
>> internal variables. In the your stenario, there should be some flag that
>> tells component to save all values, even these that are not supposed to
>> be preserved between requests.
>>
>> On 03/19/2010 10:27 AM, Leonardo Uribe wrote:
>> > In short, this topic is an attempt to add full state to components
>> > inside UIData. I'll do a brief resume, so people can understand this one
>> > easily.
>> >
>> > UIData uses the same component instances to render multiple rows.
>> > Suppose this example:
>> >
>> > <h:dataTable id="cities" var="city" value="#{country.cities}">
>> >     <h:column>
>> >         <h:outputText value="#{city}"  />
>> >     </h:column>
>> > </h:dataTable>
>> >
>> > In the component tree it is created this hierarchy:
>> >
>> > HtmlDatatable
>> >   UIColumn
>> >     HtmlOutputText
>> >
>> > If we have 10 cities, the same component is used over and over to render
>> > all 10 cities. The reason to do that in this way is keep state as small
>> > as possible.
>> >
>> > Now let's suppose something like this:
>> >
>> > <h:dataTable id="cities" var="city" value="#{country.cities}">
>> >     <h:column>
>> >         <h:inputText value="#{city}"  />
>> >     </h:column>
>> > </h:dataTable>
>> >
>> > It was changed the output component for an input one. If this table is
>> > in a form and the values are submitted, the same component is used to
>> > apply request values, validate and apply them to the model (update
>> > process). To make this possible, UIData class has some code to preserve
>> > this values between phases (using EditableValueHolder interface), so
>> > when each phase is processed, all rows are traversed and you get the
>> > expected behavior.
>> >
>> > Now suppose something more complex: We have a code that use
>> > invokeOnComponent to change the style of my inputText. In theory, only
>> > one row should change. But in fact, all rows are rendered with the same
>> > color. Why? because we use the same component to render all rows, and we
>> > don't preserve the children component state between rows.
>> >
>> > There is a lot of issues, questions, and side effects related to this
>> > issue, but just to put some of them here:
>> >
>> > TOMAHAWK-1062 InputTextArea doesn't work properly inside facet
>> DetailStamp
>> > TOMAHAWK-96 Data table Scroller not working the dataTable which was
>> > actually contained in other DataTable
>> >
>> https://javaserverfaces-spec-public.dev.java.net/issues/show_bug.cgi?id=153
>> > Problems with UIData state saving
>> >
>> > Also, it is well know that one reason why people uses c:forEach in
>> > facelets, is because this one create "full" components per each row. It
>> > is very easy to find articles on internet.
>> >
>> > Now, with jsf 2.0 we have partial state saving, so we have a chance to
>> > fix this one once and for all. I tried fix this one per months (maybe
>> > years!), but talking with Martin Marinschek on JSFDays, some ideas came
>> > out and finally it was found a possibility to fix this one using the
>> > existing api and with little changes on the spec.
>> >
>> > The proposal is this:
>> >
>> > 1. Do not call UIComponent.markInitialState() on
>> > ComponentTagHandlerDelegate, as ComponentHandler javadoc says, instead
>> > call it after PostAddToViewEvent are published on vdl.buildView(). We
>> > need to call it from root to nodes, so the parent component is marked
>> > first. I know the place where this call comes is from trinidad tag
>> > handler, but this call needs to be fixed in a more predictable way.
>> > 2. Use an attribute on facesContext to identify when the VDL is marking
>> > the initial state (in myfaces there is already an attribute called
>> > "org.apache.myfaces.MARK_INITIAL_STATE"). This is necessary to indicate
>> > UIData instances that it is time to save the full state of all component
>> > children,
>> > 3. Allow UIData to hold a map where the key are client ids and the value
>> > are the deltas of all components per row. This map should be saved and
>> > restored.
>> > 4. Change UIData.setRowIndex() to restore and save the component state.
>> >
>> > I'll attach a patch on myfaces issue tracker with the algorithm proposed
>> > (because it is based on myfaces codebase). It was tested and it works.
>> > But note it is necessary to fix the javadoc for
>> > UIData.markInitialState(), ComponentHandler and maybe vdl.buildView(),
>> > so the intention is propose this change for jsf 2.0 rev A. Note this
>> > works only with PSS enabled because without it we don't have a place to
>> > notify UIData instances that it is necessary to get the full state.
>> > Also, note this patch preserve backward compatibility, because the old
>> > way to store/save is applied after the full state is restored.
>> >
>> > Really, I have the strong temptation to apply some similar code on
>> > myfaces UIRepeat component (because this class is private), but I prefer
>> > first ask to EG to know what you guys think about it.
>> >
>> > Suggestions are welcome,
>> >
>> > regards,
>> >
>> > Leonardo Uribe
>> >
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.jboss.org/pipermail/jsr-314-open-mirror/attachments/20100319/d8e01e75/attachment-0003.html