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

Tue Aug 17 17:02:01 EDT 2010

Hi

2010/8/13 Ed Burns <edward.burns at oracle.com>

> >>>>> On Mon, 26 Jul 2010 08:14:42 +0200, Martin Marinschek <
> mmarinschek at apache.org> said:
>
> MM> Hi guys,
> MM> sorry, it took me a while to review the patch, but this is Leonardo´s
> MM> suggestion for partial state saving in tables. He also included a test
> MM> webapp, and a test for Mojarra.
>
> It's no problem.  I'm not surprised it took so long.  For me, at least,
> this is a very complicated issue to understand.  At this point in the
> life of JSF, it's these kinds of issues that remain unsolved.
>
> Here is my response.
>
>
> https://javaserverfaces-spec-public.dev.java.net/nonav/issues/showattachment.cgi/268/message.txt
>
> Included inline for convenience.
>
> I'm now going to just apply Leonardo's patch and see what happens to our
> existing automated tests.
>
>
Note by default this feature is not active, so I think to test it fully it
is necessary to activate it setting
preserveRowComponentState to true.


> LU> In theory, any solution to this issue must do something like this:
>
> LU> - Save the "initial state" of all component children, excluding
> LU> facets of nearest children (usually instances of UIColumn), after
> LU> the component tree is build.
>
> This is just the normal mark initial state, right?
>
>
Yes, but for UIData, it must save the initial state of all children in a
structure, so
when setRowIndex() is called, it can restore the component state as it was
before markInitialState().


> LU> - When setRowIndex method is called, it should save the "partial" or
> LU> "delta" state of the current row somewhere, change the row, and
> LU> finally restore the state using the "initial state" of the row and
> LU> the delta saved, if any.
>
> Yes, this makes sense.
>
> LU> Based on the previous ideas, the following (personal) conclusions
> LU> arise:
>
> LU> 1. Use a property to enable disable this behavior is preferred,
> LU> because it is possible to have a non "reliable" model, or the user
> LU> could have situations where it is not required to preserve the state
> LU> per component. The patches adds a property called
> LU> "preserveRowComponentState".
>
> That's fine with me.
>
> LU> 2. It is necessary to know "when" we can calculate the "initial
> LU> state" of the rows: In the proposed patches, the idea is use
> LU> UIData.markInitialState and check if we should save the "initial"
> LU> state. To do that I'm considering two options:
>
> LU>    a. Move markInitialState calls, so they will be called after the
> LU> component tree is built by facelets vdl. In this way we can ensure
> LU> we are calling from parents to children, and we can call saveState
> LU> directly on all children.
>
> LU>    b. Do not move the calls, but take into consideration that some
> LU> children could already be marked, so for those ones we need to call
> LU> clearInitialState, saveState and then markInitialState.
>
> LU>    Why two options?.
>
> LU>   Some time ago, myfaces used a listener attached to
> LU> PostAddToViewEvent for "relocate" components. In theory we have 4
> LU> cases: h:outputScript, h:outputStylesheet, cc:insertChildren and
> LU> cc:insertFacet.
>
> Mojarra still uses a listener for this.
>
> LU> PostAddToViewEvent listener is used on
> LU> h:outputScript and h:outputStylesheet by the spec, but it was
> LU> necessary to use it on cc:insertChildren and cc:insertFacet, because
> LU> there was a problem with nested composite components. But right now,
> LU> after some discussions and new ideas provided, myfaces core 2.0.1
> LU> has a algorithm that uses a variant of facelets template client api,
> LU> so cc:insertChildren and cc:insertFacet does not use a
> LU> PostAddToViewEvent listener anymore.
>
> Andy Schwartz suggested Mojarra move to using the template client
> approach for cc:insertChildren and cc:insertFacet in issue 1680 [1].  We
> opted for a simpler option to allow the Facelets tagHandler to correctly
> locate the relocated child in the postback case.  Therefore, I'm very
> reluctant to adopt a fix to UIData state saving that will break our fix
> for 1680.
>
>
In myfaces we did something similar, but after thinking more about it and
take
into consideration the semantic of ui:composition, ui:define and ui:insert
it
becomes necessary to change it. The way suggested by Andy is better, because
it handle correctly interactions between nested composite components and
template tags (TemplateContext concept). I have some tests that shows it,
 but I think it is worth to propose this change on mojarra.


> LU>   Allow component relocation using PostAddToViewEvent causes a lot
> LU> of trouble. It is more, add/remove components "programatically"
> LU> causes a lot of problems too (that's another discussion). Just one
> LU> example: components relocated using cc:insertChildren or
> LU> cc:insertFacet does not preserve state, because on each request, the
> LU> components are relocated and the ones with the state are removed by
> LU> facelets refresh algorithm, used by c:if tag.
>
> This problem with c:if has been fixed with in issue 1527 [2].  If you
> find that the solution in that issue is faulty, please let the Mojarra
> team know.
>
> Ok, I'll take a look to this point.


> LU>   The option (a) consider that if PostAddToViewEvent listeners are
> LU> used to relocate components, only it is safe to save the "initial
> LU> state" after the tree is completely built, so we require a fix for
> LU> markInitialState() and some way to indicate we are calling this
> LU> method from the specified location.
>
> This sounds reasonable.
>
> LU>   The option (b) consider that PostAddToViewEvent should not be used
> LU> to relocate components. If components are relocated, the destiny
> LU> should be outside a UIData instance (h:outputScript and
> LU> h:outputStylesheet usually relocates to "head" or "body"), otherwise
> LU> the "initial state" will be invalid. Since latest myfaces code
> LU> allows this, the option (b) will work there. Anyway, I provide a
> LU> patch for mojarra, so people can see how should looks like, but to
> LU> make it work requires some big changes.
>
> Yes, it would.  And we're not in a position to want to make big changes
> now.
>
> LU> 3. The "delta" state of each row should be saved / restored
> LU> somewhere with the page: In the proposed patch the deltas are saved
> LU> with the state of UIData.
>
> That is a reasonable place to save it.
>
> LU> 4. A way to save/restore transient variables like UIForm submitted
> LU> should be provided. The proposed patch includes two new interfaces
> LU> called TransientStateHelper and TransientStateHolder to deal with
> LU> this stuff (save/restore and manipulate them). As a curious fact,
> LU> some days ago, a non very clean solution was applied on
> LU> "TRINIDAD-1849 subform in table not working" that could be solved
> LU> better with interfaces like the ones proposed here.
>
> Ok, this sounds good too.  I'm going to try to understand the patch now,
> but I think you've done some great work.  I just hope that it can be
> made to apply to Mojarra as well as your MyFaces.
>
>
Thanks Ed for take a look to this issue.

best regards,

Leonardo Uribe


> [1] https://javaserverfaces.dev.java.net/issues/show_bug.cgi?id=1680
>
> [2] https://javaserverfaces.dev.java.net/issues/show_bug.cgi?id=1527
>
>
> Ed
>
> --
> | edward.burns at oracle.com | office: +1 407 458 0017
> | homepage:               | http://ridingthecrest.com/
> | 03 work days until JSF 2.1 Milestone 2
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.jboss.org/pipermail/jsr-314-open-mirror/attachments/20100817/ad986da0/attachment-0002.html