[jira] Commented: (MYFACES-2489) Clean up the viewId calculation algorithm

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

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

Leonardo Uribe commented on MYFACES-2489:
-----------------------------------------

This patch reveals different problems with the current implementation, and apply it could lead to misunderstandings. Let's go by parts:

1. Change JspViewHandlerImpl is bad. Right now, myfaces has 3 ViewHandler implementations:

org.apache.myfaces.application.jsp.JspViewHandlerImpl
org.apache.myfaces.view.facelets.FaceletViewHandler
org.apache.myfaces.application.ViewHandlerImpl

JspViewHandlerImpl and FaceletViewHandler are reference, and should not be changed/removed if possible (in theory one user could use them). The one used in jsf 2.0 is ViewHandlerImpl.

2. The spec section 7.6.2.1 ViewDeclarationLanguage.createView() says:

"....If no viewId could be identified, or the viewId is exactly equal to the servlet mapping, send the response error
code SC_NOT_FOUND with a suitable message to the client...."

It seems there is something wrong here. ViewDeclarationLanguageBase calls vdl.calculateViewId to check this condition, but looking in deep this is called from ViewHandlerImpl.createView. Now look this code on JspViewHandlerImpl.createView:

    public UIViewRoot createView(FacesContext facesContext, String viewId)
    {
        String calculatedViewId = viewId;
        try
        {
            calculatedViewId = getViewHandlerSupport().calculateViewId(facesContext, viewId);
        }
        catch (InvalidViewIdException e)
        {
            sendSourceNotFound(facesContext, e.getMessage());
        }

It seems an error on the spec. I think the intention is do this in ViewHandlerImpl, because vdl.createView could be called from vdl.restoreView and ViewHandlerImpl.restoreView converts it.

3. Just to keep in mind, DefaultRestoreViewSupport and DefaultViewHandlerSupport are two complete different classes. Before refactor them it is necessary to solve the previous point (maybe create another issue, link it with this one, and solve that there). Do not remove DefaultRestoreViewSupport or DefaultViewHandlerSupport,  just mark them as deprecated and add a note "this class was replaced by.....". One idea could be add a new helper for RestoreViewExecutor and go back DefaultRestoreViewSupport to jsf 1.2 form.

> Clean up the viewId calculation algorithm
> -----------------------------------------
>
>                 Key: MYFACES-2489
>                 URL: https://issues.apache.org/jira/browse/MYFACES-2489
>             Project: MyFaces Core
>          Issue Type: Task
>          Components: JSR-314
>    Affects Versions: 2.0.0-alpha-2
>            Reporter: Jakob Korherr
>         Attachments: MYFACES-2489-core.patch, MYFACES-2489-shared.patch, myfaces-2489.patch
>
>
> The whole viewId calculation process is a big mess. There is DefaultRestoreViewSupport with calculateViewId and deriveViewId and there is DefaultViewHandlerSupport with calculateViewId and calculateAndCheckViewId.
> Furthermore each viewId gets calculated twice (e.g. first from test.jsf to test.xhtml and then from test.xhtml to test.xhtml, which is not necessary).

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.