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