[jira] [Commented] (MYFACES-2924) Component bindings are not reset when explicit navigation to the same page is derived from action

["Leonardo Uribe (JIRA)" <de...@myfaces.apache.org>]

    [ https://issues.apache.org/jira/browse/MYFACES-2924?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13029041#comment-13029041 ] 

Leonardo Uribe commented on MYFACES-2924:
-----------------------------------------

Attached to this issue there is a proposal to handle "binding" cleanup gracefully.

In few words, the idea is if there is a "binding" set for a component, attach a component system event listener that will set to null to the "binding" value expression (only if is readOnly, that means if there is a setter for it). Later, if there is a normal navigation on a POST request, before replace the view with the new one, do a full visitTree with SKIP_ITERATION enabled to publish PreDisposeViewEvent over all components, to give the chance to the listener to do the proper cleanup.

Do it in this way is necessary to preserve t:aliasBean working (because use visitTree gives the chance to t:aliasBean to do its hack). 

It is known that do a full visitTree is expensive in time, so I did a profile session (using NetBeans Profiler and MyFaces Tests) to get a better idea if the patch proposed should be applied or not. 

The first thing I notice was ComponentSupport.findChildByTagId is expensive too, and in numbers is equivalent to a full visitTree (almost 1:1). If the view is small, visitTree becomes cheaper, but if the view is big and there are not too many NamingContainer components, findChildByTagId becomes more expensive.

Now, the effect of the patch proposed is all code related to findChildByTagId is not called on the first request, and if we are considering a navigation case, we are saving twice (on restore view and on build the next view), so the final effect is the code will perform better (each buildView takes aprox. 10% less time!) and additionally this issue will be fixed, being conformant with the spec.

I think this code should be definitively committed, but since it could be a controversial change, I want to let some time if anyone has something to say.

 If not objections I'll commit this code soon. 

> Component bindings are not reset when explicit navigation to the same page is derived from action
> -------------------------------------------------------------------------------------------------
>
>                 Key: MYFACES-2924
>                 URL: https://issues.apache.org/jira/browse/MYFACES-2924
>             Project: MyFaces Core
>          Issue Type: Bug
>          Components: JSR-314
>    Affects Versions: 2.0.1
>            Reporter: Leonardo Uribe
>            Assignee: Leonardo Uribe
>         Attachments: MYFACES-2924-1.patch
>
>
> It is possible to cause a duplicate id exception doing the following steps:
> - Render one page (let's call mypage.xhtml) with a component that has a binding to a request scope managed bean
> - Click the button
> mypage.xhtml
> ....
> <x:component binding="#{bean.component}/>
>     <f:facet name="header">
>         /*.... some components here with non generated id ....*/
>     </f:facet name="header">
> </x:component>
> <h:commandButton action="#{bean.someMethod}">
> ....
> bean
> public String someMethod()
> {
>     return "mypage";
> }
> That example should work without problem. The problem is raised by an optimization done in myfaces:
> UIInstructionHandler
>             if (mctx.isRefreshingTransientBuild())
>             {
>                 c = ComponentSupport.findChildByTagId(parent, id);
>                 /*......*/
> If we are not refreshing the current view, it does not have sense to try to find a component that will not be found, right?
> The spec is clear about the effect of the previous example:
> JSF 2.0 section 7.4.2 : "...A rule match always causes a new view to be created, losing the state of the old view. This includes clearing out the view map....."
> but if you have component bindings on the page, all components inside the component with the binding will preserve the state. There is one simple solution from the user point of view (and this is what everyone usually does in this case):
> public String someMethod()
> {
>     return null;
> }
> no rule match means no navigation, the view state is preserved and finally no duplicate id exception. 
> If we disable the optimization code, the code will work, but there is still one problem with the spec.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira