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

>>>>> On Mon, 26 Jul 2010 08:14:42 +0200, Martin Marinschek < mmarinschek(a)apache.org&gt; 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/showattachm... 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. 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.

[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(a)oracle.com | office: +1 407 458 0017 | homepage: | http://ridingthecrest.com/ | 03 work days until JSF 2.1 Milestone 2