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

[Ed Burns]

>>>>> 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.

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?

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.

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.

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.

[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