Hi<br><br><div class="gmail_quote">2010/11/3 Ed Burns <span dir="ltr"><<a href="mailto:edward.burns@oracle.com">edward.burns@oracle.com</a>></span><br><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
>>>>> On Tue, 19 Oct 2010 12:35:05 -0400, Andy Schwartz <<a href="mailto:andy.schwartz@oracle.com">andy.schwartz@oracle.com</a>> said:<br>
<br>
EB> When the markInitialState() method is called, save the state of the<br>
EB> direct children<br>
<br>
AS> Is it just the direct children? Don't we need to save the state for<br>
AS> non-direct descendents as well?<br>
<br>
I see what you mean. Yes, we should, and I've updated the spec language.<br>
<br>
AS> Should we mention that non-iterated facets of UIData are excluded<br>
AS> from this?<br>
<br>
I have mentioned that.<br>
<br>
EB> without a dependency on any particular row value. For discussion, this<br>
EB> rolled up state is called the "initial rolled up state".<br>
<br>
AS> Minor issue, but for some reason the term "rolled up state" doesn't seem<br>
AS> to be clicking for me. I think of this as the "full initial state" -<br>
AS> ie. we are doing a full state save to capture the initial state of these<br>
AS> components so that we can get back to this later.<br>
<br>
I'm going to stick with rolled up state. The word "initial" is used so<br>
frequently in this sort of discussion that I don't want to use it any more.<br>
<br>
EB> When setRowIndex(int) is called, the following additional actions must<br>
EB> be taken before the usual call to setRowIndex(int).<br>
<br>
AS> Instead of "before the usual call to setRowIndex()", maybe "before any<br>
AS> other work is performed"? It isn't clear what "the usual call" means.<br>
<br>
EB> Traverse the children as done with markInitialState(), saving the<br>
EB> state of each child. Because the<br>
EB> saveState(javax.faces.context.FacesContext) calls during this pass<br>
EB> happen after the call to markInitialState(), the state saved is the so<br>
EB> called "delta" state of each component.<br>
<br>
AS> What happens is partial state saving is disabled? Do we still implement<br>
AS> this behavior and just save/restore the full state, or do we bail on the<br>
AS> preserveRowComponentState behavior?<br>
<br>
I have clarified that the preserveRowComponentState is not impacted by<br>
the application partial state saving behavior.<br>
<br>
AS> BTW, we don't need to perform this traversal if the current row index<br>
AS> (the row index at the point in time when setRowIndex() is called) is -1.<br>
<br>
EB> Because this traversal happens during a per-row operation (in this<br>
EB> case, setRowIndex(int)) the rolled up state<br>
<br>
AS> Okay, so I guess "rolled up" state != "full initial state" - more like<br>
AS> the per-row saved state.<br>
<br>
<br>
EB> must be saved in a row-aware data structure. One implementation choice<br>
EB> would be to save the state from this pass in a Map keyed by the return<br>
EB> from UIComponent.getContainerClientId(javax.faces.context.FacesContext).<br>
<br>
AS> Though this might not work well if we need to save state for subtrees as<br>
AS> opposed to only direct children - ie. might be better to use the full<br>
AS> client id. Perhaps we should leave this implementation suggestion out?<br>
<br>
I have done so.<br>
<br>
EB> It is permissible for the rolled up state to be null or empty.<br>
EB><br>
EB> If the current row index is not -1, traverse the children as in the<br>
EB> previous step,<br>
<br>
AS> Do we need to perform two separate traversals? Woudn't it be more<br>
AS> efficient to consolidate these into a single traversal?<br>
<br>
Perhaps, but that's how Leonardo first implemented it and I am not<br>
inclined to try to optimize it here.<br>
<br></blockquote><div><br>Yes, it is possible. On the first implementation, I wanted to keep things simple<br>and understandable, so I keep them on different methods, but now the algorithm <br>is well understood.<br> <br>
</div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
EB> but, instead of calling saveState(javax.faces.context.FacesContext),<br>
EB> call UIComponent.saveTransientState(javax.faces.context.FacesContext).<br>
EB> Save the rolled up state in a separate row-aware data structure from<br>
EB> the one used in the preciding<br>
<br>
AS> preceding (sp)<br>
<br>
Thanks.<br>
<br>
EB> step.<br>
EB><br>
EB> Call DataModel.setRowIndex(int) on the model, as normal. Store the row<br>
EB> data as request scope attributes, as normal.<br>
EB><br>
EB> If the rolled up state saved during the call to markInitialState() is<br>
EB> not null or empty,<br>
<br>
AS> Since this is the "initial rolled up state" - ie. the full state,<br>
AS> wouldn't this be non-null?<br>
<br>
I'm not sure of the answer to this one. Leonardo?<br>
<br></blockquote><div><br>In theory, if UIData.markInitialState is called correctly, the "initial rolled up<br>state" is not null, so the check could be skipped. In the first versions of the<br>algorithm, the check was used to fallback to the previous algorithm, but now<br>
this is done using a check for the property ( isPreserveRowComponentState() ).<br><br></div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
EB> perform the reverse of the two save operations, as described next.<br>
EB> This is necessary to indicate that transient state from the previous<br>
EB> row must be discarded, in order to not pollute the state of the<br>
EB> current row.<br>
<br>
EB> If the per-row state saved in step a. above is null, traverse the<br>
EB> children and restore the child state using the initial rolled up state.<br>
<br>
AS> Does the "per-row state saved in step a. above" refer to the stave saved<br>
AS> for the row that we are leaving? Or for the state that we saved the<br>
AS> last time that visited the current row?<br>
<br>
AS> That is, if we are on row 1 and setRowIndex(2) is called, does the<br>
AS> "per-row state saved in step a" mean the state produced for row 1 before<br>
AS> we adjusted the index, or the state that we previously saved away the<br>
AS> last time we visited row 2?<br>
<br>
AS> This is important to specify clearly.<br>
<br>
It's the state before we adjusted the row index. I've clarified that.<br>
<br>
EB> If the per-row state saved in step a. above is not null, traverse the<br>
EB> children and restore the child state using the state saved during step<br>
EB> a., using the initial rolled up state only as a backup in the case<br>
EB> that per-row state is not available.<br>
<br>
AS> Okay, so I guess that "per-row state saved in step a. above" refers to<br>
AS> the previously saved state for the current row.<br>
<br>
No, it's the state before we adjusted the row index. Leonardo, it is<br>
vitally important that you and Andy and I agree on this.<br>
<br></blockquote><div><br>The "per-row state saved in step a. above" in other words means the "delta"<br>state from the row that will be changed. If we are on row 1, to change to row 2<br>we need to restore the state of row 2, and check if it is null or not, If it is null<br>
we use only the initial rolled up state, but if is not, we use both the initial<br>rolled up state and the "per-row state saved".<br> </div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
AS> However, since this is delta state, before we apply this isn't it<br>
AS> necessary to first restore the components to their initial state by<br>
AS> restoring the full initial saved state? If we don't restore the<br>
AS> components to their initial state before applying the delta state,<br>
AS> won't we run the risk that state from the previous row might bleed<br>
AS> over into the current row? (In the case where the state from the<br>
AS> previous from was null, this won't be an issue.)<br>
<br>
AS> Also, at some point is it necessary to tell the StateHelper that it<br>
AS> needs to clear out any previously saved deltas so that it can start<br>
AS> tracking deltas for the currently active row? Or does that happen<br>
AS> implicitly at some point, eg. when we restore the row state?<br>
<br>
EB> If the current row index is -1, traverse the children and pass null to<br>
EB> UIComponent.restoreTransientState(javax.faces.context.FacesContext,<br>
EB> java.lang.Object).<br>
<br>
AS> Hopefully we can consolidate the restore-related traversals as well.<br>
<br>
EB> If the current row index is not -1, take the following actions.<br>
<br>
EB> If the per-row state saved in step b. above is null, traverse the<br>
EB> children and restore the transient state by passing null to each<br>
EB> child's<br>
EB> UIComponent.restoreTransientState(javax.faces.context.FacesContext,<br>
EB> java.lang.Object) method<br>
EB><br>
EB> If the per-row state saved in step b. above is not null, traverse the<br>
EB> children and restore the transient state from the state saved in step<br>
EB> b. above, calling<br>
EB> UIComponent.restoreTransientState(javax.faces.context.FacesContext,<br>
EB> java.lang.Object) on each child, and passing the appropriate state.<br>
<br>
AS> Sounds like we are doing the same thing whether or not the transient<br>
AS> state is null, so perhaps we can simplify the wording.<br>
<br>
Perhaps, but I'm concerned with getting it wrong. The current text was<br>
approved by Leonardo so I'm inclined to go with it.<br>
<br></blockquote><div><br>In few words, it just say how to restore the "transient state". It is possible in this<br>part I just forget to simplify things. <br><br>The time I wrote that part I was thinking if there was some kind of manipulation <br>
on transient state properties before build view time, those values should be <br>"propagated" to child rows, but the intention of this state is to have a very <br>short life time (vdl build time or render time). If a value is necessary <br>
to be stored more than the current request, use StateHelper. After thinking about<br>it carefully, the conclusion was that is not necessary, so we can just assume a<br>"null" or "empty" state for the transient map. <br>
</div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
AS> Few other questions:<br>
<br>
AS> 1. Leonardo raised an issue regarding the timing of when<br>
AS> markInitialState() is called - ie. that markInitialState() needs to be<br>
AS> called on the parent before the children - otherwise UIData won't be<br>
AS> able to capture the full initial state of its children. How did we<br>
AS> solve this problem? Are there spec changes relating to this? Did we<br>
AS> find a way to do this that doesn't require introducing yet another full<br>
AS> tree traversal?<br>
<br>
I don't know.<br>
<br></blockquote><div><br>It is possible to prevent the full tree traversal, but to do that it is necessary to<br>change all code that uses PostAddToViewEvent to relocate components, and that<br>means refactor composite component solution as is. This was done on MyFaces,<br>
so there cc:insertChildren and cc:insertFacet does not use a Listener attached to<br>that event there, but Mojarra uses it.<br><br>Anyway, users could add components on PostAddToViewEvent to the tree, so <br>in theory the call to markInitialState() should be done after that event is handled.<br>
<br></div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
AS> 2. Regarding the name of the new property...<br>
AS> is/setPreserveRowComponentState() is a bit of a mouthful. Could we<br>
AS> maybe shorten this to is/setRowStatePreserved()?<br>
<br>
I like that a lot better.<br>
<br>
AS> 3. Are there cases where it might be useful to enable per-row transient<br>
AS> state saving without also enabling non-transient state saving (which<br>
AS> seems more expensive)? I wonder whether it should be possible to enable<br>
AS> these independently, in which case we may want to consider using an enum<br>
AS> property instead of a boolean. Also wondering whether transient row<br>
AS> state saving should just be on by default or possibly always on.<br>
<br>
Leonardo?<br>
<br></blockquote><div><br>It is possible, but shouldn't that condition just depends on the component child itself ?.<br>For example, UIForm has submitted property that in the future will be stored in transient<br>map, so this component requires transient state be always on.<br>
<br>best regards,<br><br>Leonardo Uribe<br> </div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
Ed<br>
<font color="#888888">--<br>
| <a href="mailto:edward.burns@oracle.com">edward.burns@oracle.com</a> | office: +1 407 458 0017<br>
| homepage: | <a href="http://ridingthecrest.com/" target="_blank">http://ridingthecrest.com/</a><br>
| 5 work days until German Oracle User's Group Conference<br>
| 12 work days until GlassFish 3.1 Hard Code Freeze<br>
</font></blockquote></div><br>