MyFaces Core
  1. MyFaces Core
  2. MYFACES-2645

The view state is saved before encodeAll() is called on every UIViewParameter in an AJAX request

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.0.0-beta-3
    • Fix Version/s: 2.0.0
    • Component/s: JSR-314
    • Labels:
      None

      Description

      UIViewParameter calls setSubmittedValue() in encodeAll() to store the current value of the view parameter in the state as the submitted value (this helps UIViewParameter to restore its value on a postback). UIViewParameter.encodeAll() itself is called by UIViewRoot.encodeEnd() for every UIViewParameter in the view.

      If the current request is an ajax-request, the view state is currently already generated in UIViewRoot.encodeChildren() (before UIViewRoot.encodeEnd()). At this time the submitted value for every UIViewParameter is null, because its encodeAll()-method was not called yet.

      Later, when UIViewRoot.encodeEnd() is called, UIViewParameter.encodeAll() is called and it sets the submitted value correctly, but due to the fact that the state has already been generated, these changes are dropped.

      This causes the value of every UIViewParameter to be set to null in the next request, which will most likely end in validation problems with the required validator.

      The related thread to this issue from the mailing list can be found at: http://www.junlu.com/list/43/611590.html

        Activity

        Hide
        Jakob Korherr added a comment -

        This particular issue can be resolved, because this is now working.

        However there are some general problems with UIViewParameters (see jsr-314-open mail), which we should handle in other JIRA issues.

        Show
        Jakob Korherr added a comment - This particular issue can be resolved, because this is now working. However there are some general problems with UIViewParameters (see jsr-314-open mail), which we should handle in other JIRA issues.
        Hide
        Jakob Korherr added a comment -

        To test things like that hopefully the GSoC project MYFACESTEST-6 will come in.

        Show
        Jakob Korherr added a comment - To test things like that hopefully the GSoC project MYFACESTEST-6 will come in.
        Hide
        Leonardo Uribe added a comment -

        Ok, now I understand. Yes, the solution is valid, so can let the committed solution. It is a good idea to ask to jsr-314-open about this issue, so let's see what happen. I would like to have some tests for this stuff, we can't test is using junit, so we need something else.

        Show
        Leonardo Uribe added a comment - Ok, now I understand. Yes, the solution is valid, so can let the committed solution. It is a good idea to ask to jsr-314-open about this issue, so let's see what happen. I would like to have some tests for this stuff, we can't test is using junit, so we need something else.
        Hide
        Jakob Korherr added a comment -

        This is the email, which I sent to jsr-314-open:

        Hi,

        Yesterday we received an issue about the state saving of an
        UIViewParameter in combination with an AJAX request on the MyFaces
        user list (see [1] for details). This opened some questions/problems:

        UIViewParameter uses the submittedValue to store its value in the
        state for the next postback, because obviously the view parameter
        won't be in the request parameter map of the next request (which is
        the postback). But because its submittedValue is null after the
        conversions and validations, it has to be set again before the state
        is generated. To accomplish that, the spec currently states to
        override encodeAll() and make the call to setSubmittedValue() there.
        In addition, encodeAll() has to be called in UIViewRoot.encodeEnd()
        for every UIViewParameter in the view. This works perfectly for normal
        requests, but when you are using an AJAX request the state is
        generated before UIViewRoot.encodeEnd() is called an thus the
        submittedValue for every UIViewParameter is null in the state. This
        means that the value of every UIViewParameter will be null in the
        following request.

        Now the easiest solution to this problem, which I also already
        committed to MyFaces trunk (again see [1] for details), is to do
        mostly the same as in UIViewRoot.encodeEnd() just also in
        PartialViewContext.
        processPartial() when the PhaseId is RENDER_RESPONSE before the state
        is generated to make it work for AJAX-requests.

        However I don't really like this solution, because we have to think of
        handling UIViewParameters in a special way every time we change
        something on any render mechanism. Why don't we just override
        saveState() on UIViewParameter and set the submittedValue there before
        the state is saved. This will have the same result, but without the
        code in UIViewRoot, PartialViewContext and UIViewParameter.encodeAll()
        and without future headaches. I also already uploaded a patch for this
        to the MyFaces issue at [1].

        The second thing I want to bring up here is an issue I ran into while
        testing the first one. Imagine you have a page with a required
        UIViewParameter called input. You will get a validation error as long
        as you don't access the view with ?input=abc. Now if you do that once,
        "abc" will be saved as the submittedValue in the state of the
        UIViewParameter for every postback and thus will be available for
        every further postback. If you now access the view again with a GET
        request (non-postback), but without ?input=abc, you will again get a
        validation error. However, if you hit any button or link on the view
        to generate another postback to the view, the validation error will be
        gone, because UIViewParameter takes the value from before ("abc") out
        of the model (managed-bean) and sets it in the state. Thus you haven't
        provided it via ?input=abc, but you will now have a value of "abc" for
        your UIViewParameter, which seems kinda wrong to me. The solution to
        this one is to get the value from the model to set it as the
        submittedValue in UIViewParameter only if the current request is a
        postback. However I don't know if this really is an error or the
        expected behaviour. I personally just think that it is weird.

        Many thanks for looking at this!

        Regards,
        Jakob

        [1] https://issues.apache.org/jira/browse/MYFACES-2645

        Show
        Jakob Korherr added a comment - This is the email, which I sent to jsr-314-open: Hi, Yesterday we received an issue about the state saving of an UIViewParameter in combination with an AJAX request on the MyFaces user list (see [1] for details). This opened some questions/problems: UIViewParameter uses the submittedValue to store its value in the state for the next postback, because obviously the view parameter won't be in the request parameter map of the next request (which is the postback). But because its submittedValue is null after the conversions and validations, it has to be set again before the state is generated. To accomplish that, the spec currently states to override encodeAll() and make the call to setSubmittedValue() there. In addition, encodeAll() has to be called in UIViewRoot.encodeEnd() for every UIViewParameter in the view. This works perfectly for normal requests, but when you are using an AJAX request the state is generated before UIViewRoot.encodeEnd() is called an thus the submittedValue for every UIViewParameter is null in the state. This means that the value of every UIViewParameter will be null in the following request. Now the easiest solution to this problem, which I also already committed to MyFaces trunk (again see [1] for details), is to do mostly the same as in UIViewRoot.encodeEnd() just also in PartialViewContext. processPartial() when the PhaseId is RENDER_RESPONSE before the state is generated to make it work for AJAX-requests. However I don't really like this solution, because we have to think of handling UIViewParameters in a special way every time we change something on any render mechanism. Why don't we just override saveState() on UIViewParameter and set the submittedValue there before the state is saved. This will have the same result, but without the code in UIViewRoot, PartialViewContext and UIViewParameter.encodeAll() and without future headaches. I also already uploaded a patch for this to the MyFaces issue at [1] . The second thing I want to bring up here is an issue I ran into while testing the first one. Imagine you have a page with a required UIViewParameter called input. You will get a validation error as long as you don't access the view with ?input=abc. Now if you do that once, "abc" will be saved as the submittedValue in the state of the UIViewParameter for every postback and thus will be available for every further postback. If you now access the view again with a GET request (non-postback), but without ?input=abc, you will again get a validation error. However, if you hit any button or link on the view to generate another postback to the view, the validation error will be gone, because UIViewParameter takes the value from before ("abc") out of the model (managed-bean) and sets it in the state. Thus you haven't provided it via ?input=abc, but you will now have a value of "abc" for your UIViewParameter, which seems kinda wrong to me. The solution to this one is to get the value from the model to set it as the submittedValue in UIViewParameter only if the current request is a postback. However I don't know if this really is an error or the expected behaviour. I personally just think that it is weird. Many thanks for looking at this! Regards, Jakob [1] https://issues.apache.org/jira/browse/MYFACES-2645
        Hide
        Jakob Korherr added a comment -

        Hi Leo,

        To your examples: I tested it with render="@all" and also with execute="@all", but it didn't work. And if you understand the background, you will know why it can't work that way.

        To your question: Basically yes, but in the case of UIViewParameter it aims to store the value in the state. You see, UIViewParameter takes the value of a URL parameter as input, but this value is only available for the first (GET) request. Every following postback won't include the value as a request parameter. Thus we need some way to keep the value from the first request in the state and the spec tells us to use the submittedValue here. Using the submittedValue to store the original value in the state has one advantage and one disadvantage. The advantage is that in the following request we just have to ignore a null-value (don't invoke setSubmittedValue(null)) in decode() and we will get our old submittedValue from the state. The disadvantage, however, is that we have to set the submittedValue before the state is generated, because after the conversions and validations have happend, the submittedValue is set to null by the JSF-lifecycle. To accomplish this, the spec tells us to invoke setSubmittedValue(getStringValue(...)) in UIViewParameter.encodeAll() and furthermore to invoke encodeAll() on every UIViewParameter in the view in UIViewRoot.encodeEnd(), which works perfectly for normal requests. For AJAX-requests, however, the state is generated before UIViewRoot.encodeEnd() is invoked and so the changes of the submittedValue of every UIViewParameter are lost. Thus I applied the code from UIViewRoot.encodeEnd() to PartialViewContextImpl.processPartialRendering() before the state of an AJAX-request is generated to have the changes of the submittedValue available in the AJAX-state.

        So you see my committed code is absolutely correct and should not be reverted. I guess it was just not thought about the correct handling of UIViewParameters in AJAX-requests when this part of the spec and the javadoc were written.

        However I wrote an email to jsr-314-open about this, because I think there is a better way to solve the problem (see the patch), but we can't apply this code now, because it breaks the spec.

        Show
        Jakob Korherr added a comment - Hi Leo, To your examples: I tested it with render="@all" and also with execute="@all", but it didn't work. And if you understand the background, you will know why it can't work that way. To your question: Basically yes, but in the case of UIViewParameter it aims to store the value in the state. You see, UIViewParameter takes the value of a URL parameter as input, but this value is only available for the first (GET) request. Every following postback won't include the value as a request parameter. Thus we need some way to keep the value from the first request in the state and the spec tells us to use the submittedValue here. Using the submittedValue to store the original value in the state has one advantage and one disadvantage. The advantage is that in the following request we just have to ignore a null-value (don't invoke setSubmittedValue(null)) in decode() and we will get our old submittedValue from the state. The disadvantage, however, is that we have to set the submittedValue before the state is generated, because after the conversions and validations have happend, the submittedValue is set to null by the JSF-lifecycle. To accomplish this, the spec tells us to invoke setSubmittedValue(getStringValue(...)) in UIViewParameter.encodeAll() and furthermore to invoke encodeAll() on every UIViewParameter in the view in UIViewRoot.encodeEnd(), which works perfectly for normal requests. For AJAX-requests, however, the state is generated before UIViewRoot.encodeEnd() is invoked and so the changes of the submittedValue of every UIViewParameter are lost. Thus I applied the code from UIViewRoot.encodeEnd() to PartialViewContextImpl.processPartialRendering() before the state of an AJAX-request is generated to have the changes of the submittedValue available in the AJAX-state. So you see my committed code is absolutely correct and should not be reverted. I guess it was just not thought about the correct handling of UIViewParameters in AJAX-requests when this part of the spec and the javadoc were written. However I wrote an email to jsr-314-open about this, because I think there is a better way to solve the problem (see the patch), but we can't apply this code now, because it breaks the spec.
        Hide
        Leonardo Uribe added a comment -

        The javadocs of UIViewParameter.encodeAll says this:

        ".......Called specially by UIViewRoot.encodeEnd(javax.faces.context.FacesContext), this method simply sets the submitted value to be the return from getStringValue(javax.faces.context.FacesContext)......"

        My question is why? isn't that bad?

        Show
        Leonardo Uribe added a comment - The javadocs of UIViewParameter.encodeAll says this: ".......Called specially by UIViewRoot.encodeEnd(javax.faces.context.FacesContext), this method simply sets the submitted value to be the return from getStringValue(javax.faces.context.FacesContext)......" My question is why? isn't that bad?
        Hide
        Leonardo Uribe added a comment -

        I don't believe that call encodeAll() on PartialViewContextImpl could be a good solution. My question is does this works?

        <h:commandButton value="ajax-button">
        <f:ajax render="input output messages" execute="@this" />
        </h:commandButton>

        At first view, there is something wrong with UIViewParameter. Why this method calls setSubmittedValue(getStringValue(context))? Doesn't that supposed to be called on update model phase? If a view contains view parameter, it should be executed the whole lifecycle, right? That code should work:

        <h:commandButton value="ajax-button">
        <f:ajax render="output messages" execute="input @this" />
        </h:commandButton>

        I think the solution should be done different, so I suggest the code committed should be reverted first.

        Show
        Leonardo Uribe added a comment - I don't believe that call encodeAll() on PartialViewContextImpl could be a good solution. My question is does this works? <h:commandButton value="ajax-button"> <f:ajax render="input output messages" execute="@this" /> </h:commandButton> At first view, there is something wrong with UIViewParameter. Why this method calls setSubmittedValue(getStringValue(context))? Doesn't that supposed to be called on update model phase? If a view contains view parameter, it should be executed the whole lifecycle, right? That code should work: <h:commandButton value="ajax-button"> <f:ajax render="output messages" execute="input @this" /> </h:commandButton> I think the solution should be done different, so I suggest the code committed should be reverted first.
        Hide
        Jakob Korherr added a comment -

        This patch (MYFACES-2645-spec-proposal.patch) removes UIViewParameter.encodeAll() and moves the setting of the submitted value to saveState(). In addition it removes all calls to UIViewParameter.encodeAll().

        On my opinion this is far better than the solution in the spec and I currently can't think of a scenario where this would break. I also tested it and it works just fine.

        I will send an email to jsr-314-open about this (and one other thing I discovered about UIViewParameter) and see what the EG thinks about it.

        Show
        Jakob Korherr added a comment - This patch ( MYFACES-2645 -spec-proposal.patch) removes UIViewParameter.encodeAll() and moves the setting of the submitted value to saveState(). In addition it removes all calls to UIViewParameter.encodeAll(). On my opinion this is far better than the solution in the spec and I currently can't think of a scenario where this would break. I also tested it and it works just fine. I will send an email to jsr-314-open about this (and one other thing I discovered about UIViewParameter) and see what the EG thinks about it.
        Hide
        Jakob Korherr added a comment -

        I just found out that Mojarra has kinda the same problem here (tested Mojarra 2.0.2 (FCS b10)).

        I used the following facelet page to test this one:

        <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN"
        "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
        <html xmlns="http://www.w3.org/1999/xhtml"
        xmlns:h="http://java.sun.com/jsf/html"
        xmlns:f="http://java.sun.com/jsf/core"
        xmlns:ui="http://java.sun.com/jsf/facelets">

        <f:metadata>
        <f:viewParam required="true" name="input" value="#

        {myBean.input}" />
        </f:metadata>

        <h:head>
        <title>View param</title>
        </h:head>
        <h:body>
        <h1>View param example by jakobk</h1>

        <h:form>
        <h:messages id="messages" />

        <h:commandButton value="normal button" />

        <h:commandButton value="ajax-button">
        <f:ajax render="output messages" execute="@this" />
        </h:commandButton>

        <h:outputText id="output" value="#{myBean.input}

        " />
        </h:form>

        <ui:debug />
        </h:body>
        </html>

        Now when I use exactly this facelet the same problem comes up in Mojarra, because it also does not invoke encodeAll() on the UIViewParameters and so they also lose their value after the second request. While setting render to @all did not solve this problem in MyFaces (because we are just rendering the contents of HtmlBody here), it would theoretically work in Mojarra (because they render the whole view), however I got a malformedXML error on the client side.

        The easiest solution to this problem is to make PartialViewContextImpl aware of the right usage of UIViewParameters. However I don't really like this solution, because we have to think of handling UIViewParameters in a special way every time we change something on any render mechanism. Maybe the "value-saving-code" of the UIViewParameters should not be included in every different rendering code, but in the state-saving part. Or maybe there is even a better way to save the value of a view parameter. But this has to be decided by the EG and changed in the spec.

        For now I will commit the easiest solution to this problem and open a spec bug.

        Show
        Jakob Korherr added a comment - I just found out that Mojarra has kinda the same problem here (tested Mojarra 2.0.2 (FCS b10)). I used the following facelet page to test this one: <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd"> <html xmlns="http://www.w3.org/1999/xhtml" xmlns:h="http://java.sun.com/jsf/html" xmlns:f="http://java.sun.com/jsf/core" xmlns:ui="http://java.sun.com/jsf/facelets"> <f:metadata> <f:viewParam required="true" name="input" value="# {myBean.input}" /> </f:metadata> <h:head> <title>View param</title> </h:head> <h:body> <h1>View param example by jakobk</h1> <h:form> <h:messages id="messages" /> <h:commandButton value="normal button" /> <h:commandButton value="ajax-button"> <f:ajax render="output messages" execute="@this" /> </h:commandButton> <h:outputText id="output" value="#{myBean.input} " /> </h:form> <ui:debug /> </h:body> </html> Now when I use exactly this facelet the same problem comes up in Mojarra, because it also does not invoke encodeAll() on the UIViewParameters and so they also lose their value after the second request. While setting render to @all did not solve this problem in MyFaces (because we are just rendering the contents of HtmlBody here), it would theoretically work in Mojarra (because they render the whole view), however I got a malformedXML error on the client side. The easiest solution to this problem is to make PartialViewContextImpl aware of the right usage of UIViewParameters. However I don't really like this solution, because we have to think of handling UIViewParameters in a special way every time we change something on any render mechanism. Maybe the "value-saving-code" of the UIViewParameters should not be included in every different rendering code, but in the state-saving part. Or maybe there is even a better way to save the value of a view parameter. But this has to be decided by the EG and changed in the spec. For now I will commit the easiest solution to this problem and open a spec bug.

          People

          • Assignee:
            Jakob Korherr
            Reporter:
            Jakob Korherr
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development