Re: [jsr-314-open-mirror] [jsr-314-open] [153-UIDataStateSaving] spec clarifications requested by Andy
by Ed Burns
>>>>> On Thu, 04 Nov 2010 16:28:44 -0400, Andy Schwartz <andy.schwartz(a)oracle.com> said:
AS> Gang -
AS> On 11/4/10 2:14 PM, Ed Burns wrote:
EB> Andy, if you want to suggest alternate, and more high level, spec
EB> language, please feel free to do so, but as you know it may not get into
EB> the final 2.1 spec due to time constraints.
EB>
EB> I will currently undertake a drastic simplification of the spec language
EB> for this feature,
AS> Ed and I had a quick chat offline. We agreed that simplification is the
AS> way to go. I would be happy with something along the lines of this:
I have taken this text verbatim with one exception, shown here.
EB> If this property is set to true, the UIData will take steps to ensure
EB> that modifications to its iterated children will be preserved on a
EB> per-row basis. This allows applications to modify component
EB> properties, such as the style class, for a specific row, rather than
EB> having such modifications apply to all rows.
EB>
EB> To accomplish this, UIData calls saveState() and saveTransientState()
EB> on its children to capture their state on exiting each row. When
EB> re-entering the row, restoreState() and restoreTransientState() are
EB> called in order to reinitialize the children to the correct state for
EB> the new row.
I added a sentance here: All of this action must take place during
processing of setRowIndex().
I'm sorry, Andy, but I want to be a little more specific.
EB> once we introduce the new and improved UIData state saving mechanism,
EB> UIComponent.saveState() will be called at multiple points in time:
EB>
EB> 1. During state saving (the traditional case).
EB> 2. In response to UIData.markInitialState().
EB> 3. In response to UIData.setRowIndex().
EB> It seems like we'll need some way to distinguish between these cases.
AS> Ed reminded me that we have already added a way to detect this.
AS> Unfortunately, it is specific to tree visiting. From VisitHint:
EB> /**
EB> * <p class="changed_added_2_1">Hint that indicates that the visit is
EB> * being performed for state saving purposes.</p>
EB> *
EB> * @since 2.0
EB> */
EB> EXECUTE_STATE_SAVING
AS> While this visit hint was added at my request, I now realize that the
AS> requirement for knowing whether we are performing state saving is more
AS> generic than tree visiting. As such, I would like to ask that we:
AS> 1. Remove this VisitHint value. And...
AS> 2. Add an equivalent FacesContext attribute
AS> #2 will make it possible to address the concerns that I raised regarding
AS> Trinidad view root caching compatibility and may be useful in other
AS> scenarios as well.
AS> If anyone has objections to this, or suggestions for the
AS> setRowStatePreserved() specification, please let us know as soon as
AS> possible. Ed will be handing off the specification to the JCP very soon!
I have committed this. Many thanks to Ken Paulsen for his code review.
Now that JSF is a hobby for him, and not a day-job, I really appreciate
it.
Ed
--
| edward.burns(a)oracle.com | office: +1 407 458 0017
| homepage: | http://ridingthecrest.com/
| 3 work days until German Oracle User's Group Conference
| 10 work days until GlassFish 3.1 Hard Code Freeze
13 years, 6 months
Re: [jsr-314-open-mirror] [jsr-314-open] [153-UIDataStateSaving] spec clarifications requested by Andy
by Ed Burns
>>>>> On Wed, 3 Nov 2010 15:00:39 -0500, Leonardo Uribe <lu4242(a)gmail.com> said:
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.
EB> No, it's the state before we adjusted the row index. Leonardo, it is
EB> vitally important that you and Andy and I agree on this.
LU> The "per-row state saved in step a. above" in other words means the
LU> "delta" state from the row that will be changed. If we are on row 1,
LU> to change to row 2 we need to restore the state of row 2, and check
LU> if it is null or not, If it is null we use only the initial rolled
LU> up state, but if is not, we use both the initial rolled up state and
LU> the "per-row state saved".
After reading Leonardo's comments, I continue to believe that the text I
wrote in response to Andy's 19 October email is correct. Here is the
relevant text from the UIData javadoc. The latest Javadoc for that
class is attached.
EB> If the rolled up state saved during the call to markInitialState()
EB> is not null or empty, perform the reverse of the two save
EB> operations, as described next. This is necessary to indicate that
EB> transient state from the previous row must be discarded, in order to
EB> not pollute the state of the current row.
EB> * If the per-row state saved in step a. above (before we adjusted
EB> the row index) is null, traverse the children and restore the
EB> child state using the initial rolled up state.
EB> * If the per-row state saved in step a. above (before we adjusted
EB> the row index) is not null, traverse the children and restore the
EB> child state using the state saved during step a., using the
EB> initial rolled up state only as a backup in the case that per-row
EB> state is not available.
[...]
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.
EB> Perhaps, but I'm concerned with getting it wrong. The current text was
EB> approved by Leonardo so I'm inclined to go with it.
LU> In few words, it just say how to restore the "transient state". It
LU> is possible in this part I just forget to simplify things.
LU> The time I wrote that part I was thinking if there was some kind of
LU> manipulation on transient state properties before build view time,
LU> those values should be "propagated" to child rows, but the intention
LU> of this state is to have a very short life time (vdl build time or
LU> render time). If a value is necessary to be stored more than the
LU> current request, use StateHelper. After thinking about it carefully,
LU> the conclusion was that is not necessary, so we can just assume a
LU> "null" or "empty" state for the transient map.
Therefore, I will not take action to simplify the wording on this point.
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?
EB> I don't know.
LU> It is possible to prevent the full tree traversal, but to do that it
LU> is necessary to change all code that uses PostAddToViewEvent to
LU> relocate components, and that means refactor composite component
LU> solution as is. This was done on MyFaces, so there cc:insertChildren
LU> and cc:insertFacet does not use a Listener attached to that event
LU> there, but Mojarra uses it.
LU> Anyway, users could add components on PostAddToViewEvent to the tree, so
LU> in theory the call to markInitialState() should be done after that event is
LU> handled.
Again, I will not take action on this point raised by Andy.
AS> 3. Are there cases where it might be useful to enable per-row
AS> transient state saving without also enabling non-transient state
AS> saving (which seems more expensive)? I wonder whether it should be
AS> possible to enable these independently, in which case we may want to
AS> consider using an enum property instead of a boolean. Also
AS> wondering whether transient row state saving should just be on by
AS> default or possibly always on.
EB> Leonardo?
LU> It is possible, but shouldn't that condition just depends on the
LU> component child itself ?. For example, UIForm has submitted
LU> property that in the future will be stored in transient map, so this
LU> component requires transient state be always on.
Again, I will not take action on this point raised by Andy.
AS> While I agree that it would make sense for rowStatePreserved to be
AS> honored regardless of whether partial/full state saving is on,
AS> simply stating that:
EB> Note that this delta state is saved regardless of whether or not
EB> partial state saving has been enabled or disabled for this
EB> application.
AS> Is not nearly sufficient to cover the full state saving case.
I have removed that blithe statement and replaced it with this text, at
the very top of the setRowStatePreserved() javadoc, where the entire
feature is effectively specified.
"Note that because all of the actions related to this property are
conceptually related to the call to markInitialState, these actions are
not invoked for applications with partial state saving disabled.
AS> Part of the problem is that the spec for rowStatePreserved is overly
AS> detailed in what implementations are required to do. The spec is
AS> very explicit about how to implement this feature for the partial
AS> state saving case, but this same approach does not work for the full
AS> state saving case. We either need to describe both cases in detail,
AS> or revise the specification to provide a higher-level description of
AS> the expected behavior, rather than explicit implementation
AS> instructions.
Recall again the July Expert Community meeting. At that meeting I said
something like, "We're really tight on resources, so we're only going to
do a few things. If you want to submit some stuff, the door is open,
but you'll have to do the heavy lifting in providing the impl, spec, and
automated test content. Leonardo contributed this entire feature;
the impl and automated test were spot on and worked well. The spec
language was insufficient in my opinion so that's how we ended up with
what we have.
Andy, if you want to suggest alternate, and more high level, spec
language, please feel free to do so, but as you know it may not get into
the final 2.1 spec due to time constraints.
I will currently undertake a drastic simplification of the spec language
for this feature, but we really need to do a better job of reigning in
such last minute scrambles.
Ed
--
| edward.burns(a)oracle.com | office: +1 407 458 0017
| homepage: | http://ridingthecrest.com/
| 4 work days until German Oracle User's Group Conference
| 11 work days until GlassFish 3.1 Hard Code Freeze
13 years, 6 months
Re: [jsr-314-open-mirror] [jsr-314-open] composite components: targets attribute revisited
by Ed Burns
>>>>> On Tue, 2 Nov 2010 08:03:51 +0100, Martin Marinschek <mmarinschek(a)apache.org> said:
MM> - As for 2.1 versus 2.2: I think that the basic fixes need to be in
MM> 2.1 - so that people can pass in multiple actions, and use actions,
MM> actionListeners, etc. independent of needing to provide references to
MM> the implementation - which just not work, as we have seen trying out
MM> this approach in the real world. The rest could also be in 2.2 - I
MM> would still be in favor of having this in 2.1, but of course all of
MM> the timing is entirely Ed's decision.
I appreciate your recognition of the agreement we reached in the JSF
expert community meetings we had in July. If you want to listen to the
audio, you'll find that we agreed to keep the scope of 2.1 limited [1].
Therefore, I will definitively state that this most recent round of
discussion will not be represented in 2.1.
Ed
[1] https://javaserverfaces-spec-public.dev.java.net/servlets/ProjectDocument...
--
| edward.burns(a)oracle.com | office: +1 407 458 0017
| homepage: | http://ridingthecrest.com/
| 6 work days until German Oracle User's Group Conference
| 13 work days until GlassFish 3.1 Hard Code Freeze
13 years, 6 months
[jsr-314-open-mirror] [jsr-314-open] [153-UIDataStateSaving] spec clarifications requested by Andy
by Ed Burns
>>>>> On Tue, 19 Oct 2010 12:35:05 -0400, Andy Schwartz <andy.schwartz(a)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.
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?
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.
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.
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.
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?
Ed
--
| edward.burns(a)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
13 years, 6 months
Re: [jsr-314-open-mirror] [jsr-314-open] composite components: targets attribute revisited
by Ed Burns
>>>>> On Thu, 28 Oct 2010 12:56:53 -0500, Leonardo Uribe <lu4242(a)gmail.com> said:
LU> Hi
LU> To be more explicit, this is the example that should fail:
LU> <ez:loginPanel id="loginPanel" model="#{bean}">
LU> <f:actionListener for="loginEvent"
LU> binding="#{bean.loginEventListener}" />
LU> <f:actionListener for="loginEvent"
LU> binding="#{bean.loginEventListener2}" />
LU> <f:actionListener for="cancelEvent"
LU> binding="#{bean.cancelEventListener}" />
LU> </ez:loginPanel>
LU> <composite:interface name="loginPanel">
LU> <composite:actionSource name="loginEvent" />
LU> <composite:actionSource name="cancelEvent" />
LU> </composite:interface>
LU> <composite:implementation>
LU> <h:commandButton name="button1">
LU> <f:actionListener
LU> binding="#{cc.actionSource.loginEvent}"/>
LU> </h:commandButton>
LU> <x:mycompositecomponent name="button2">
LU> <f:actionListener
LU> binding="#{cc.actionSource.cancelEvent}" for="someOtherEvent"/>
LU> </x:mycompositecomponent>
LU> </composite:implementation>
LU> In this case, the binding #{cc.actionSource.loginEvent} does not point to
LU> just
LU> one actionListener.
GP> Leo, IMO your example wouldn't need to fail: the nested
GP> actionListener with binding="#{cc.actionSource.loginEvent}" would
GP> need to execute *all* actionListeners that have been bound to
GP> "loginEvent". In this case "#{bean.loginEventListener}" and
GP> "#{bean.loginEventListener2}" would reside in a Map named
GP> cc.actionSource.loginEvent and could both be executed. Wouldn't this
GP> work?
The type of the binding attribute is specified thus.
Value binding expression that evaluates to an object that implements
javax.faces.event.ActionListener
How would you make that handle a list?
Sure, we could go and add a raft of new "bindingList" attributes to all
of the attached object handlers in f:, but is it worth the complexity?
Ed
--
| edward.burns(a)oracle.com | office: +1 407 458 0017
| homepage: | http://ridingthecrest.com/
| 9 work days until German Oracle User's Group Conference
13 years, 6 months
[jsr-314-open-mirror] [jsr-314-open] [901-DeprecateTargetsConcept]
by Ed Burns
>>>>> On Fri, 29 Oct 2010 21:06:31 -0600, David Geary <clarity.training(a)gmail.com> said:
DG> 2010/10/29 Jakob Korherr <jakob.korherr(a)gmail.com>
JK> Thanks, Andy! Frankly I also do not really like the term "insert"
JK> here, because - as you said - it just does not fit that well. However
JK> I really, really like "implements" - this is just soo much better :)
JK> <h:commandButton ....>
JK> <cc:implementsActionSource name="myActionSource" />
JK> </h:commandButton>
JK> Really beautiful!
DG> It seems that I am perhaps the only dissenting voice here, but I
DG> don't care for this solution.
DG> To those of us that understand the rationale for removing targets
DG> and adding these cc:implements... tags, the new tags make perfect
DG> sense. But to the uninitiated, they will be bewildering. What does
DG> it mean for a button to "implements action source"? Buttons already
DG> implement action source. IMO, targets are much easier to understand,
DG> and to explain.
DG> I understand the urge to remove the targets attribute based on their
DG> OO impurity, but I think the solution could use some more
DG> thought. There are already too many arcane oddities in JSF, whose
DG> rationale is only intuited by high priests, and I hate to see us
DG> adding more.
Spoken like someone who remembers our December 2007 EG meeting when we
finalized the "targets" concept. Yes, this needs more time.
Ed
--
| edward.burns(a)oracle.com | office: +1 407 458 0017
| homepage: | http://ridingthecrest.com/
| 6 work days until German Oracle User's Group Conference
| 13 work days until GlassFish 3.1 Hard Code Freeze
13 years, 6 months