MyFaces Core
  1. MyFaces Core
  2. MYFACES-736

Number converter not working with Number

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Won't Fix
    • Affects Version/s: 1.1.2-SNAPSHOT
    • Fix Version/s: 1.1.2
    • Component/s: General
    • Labels:
      None

      Description

      If the backing bean setter takes a Number argument (Integer, Float, ...), the converter fails :
      Conversion Error "XXX: Error during model data update.

      If the setter uses a primitive type, then it works fine.

      This worked with the trunk version from September 30.
      There must have been a change that broke it since.

        Activity

        Hide
        Sylvain Vieujot added a comment -

        In fact, it's not working with primitive int either.
        BUT it's working fine with primitive float.

        Show
        Sylvain Vieujot added a comment - In fact, it's not working with primitive int either. BUT it's working fine with primitive float.
        Hide
        Keijo Nurmes added a comment -

        I was testing 1.1.1RC3 and noticed that h:inputText with f:convertNumber
        didn't work any more. As Sylvain described, it throws
        'Conversion Error "XXX: Error during model data update.'
        Stack trace shows that code breaks in org.apache.myfaces.el.ValueBindingImpl.setValue

        This is easy to reproduce with sample1.jsp in simple example.
        Just throw <f:convertNumber/> inside <h:inputText id="number1"...

        What seems to happen is:
        In javax.faces.component.UIInput.validate the submittedValue is converted to
        Object convertedValue = getConvertedValue(context, submittedValue);

        In org.apache.myfaces.renderkit._SharedRendererUtils. findUIOutputConverter
        If you have convertNumber, converter is retrieved by
        Converter converter = component.getConverter();
        and is javax.faces.convert.NumberConverter

        If you don't have convertNumber the converter is retrieved by
        ValueBinding vb = component.getValueBinding("value");
        Class valueType = vb.getType(facesContext);
        facesContext.getApplication().createConverter(valueType);
        in my application the backing bean contains field of type short
        so javax.faces.convert.ShortConverter is returned.

        The type of convertedValue (in UIInput) is java.lang.long if NumberConverter is used and
        java.lang.short if Short.Converter is used.

        If the field type in backing bean is short and convertedValue is long,
        then later org.apache.myfaces.el.ValueBindingImpl.setValue can't find a setter to the field.

        I run out of time now, so I continue debugging later if necessary.
        (Hope somebody can solve this before )

        Regards, Keijo

        Stack trace from simple example .........

        javax.faces.el.EvaluationException: Cannot set value for expression '#

        {calcForm.number1}

        ' to a new value of type java.lang.Long
        at org.apache.myfaces.el.ValueBindingImpl.setValue(ValueBindingImpl.java:304)
        at javax.faces.component.UIInput.updateModel(UIInput.java:226)
        at javax.faces.component.UIInput.processUpdates(UIInput.java:165)
        at javax.faces.component.UIForm.processUpdates(UIForm.java:85)
        at javax.faces.component.UIComponentBase.processUpdates(UIComponentBase.java:428)
        at javax.faces.component.UIComponentBase.processUpdates(UIComponentBase.java:428)
        at javax.faces.component.UIViewRoot.processUpdates(UIViewRoot.java:153)
        at org.apache.myfaces.lifecycle.LifecycleImpl.updateModelValues(LifecycleImpl.java:277)
        at org.apache.myfaces.lifecycle.LifecycleImpl.execute(LifecycleImpl.java:81)
        at javax.faces.webapp.FacesServlet.service(FacesServlet.java:106)

        Show
        Keijo Nurmes added a comment - I was testing 1.1.1RC3 and noticed that h:inputText with f:convertNumber didn't work any more. As Sylvain described, it throws 'Conversion Error "XXX: Error during model data update.' Stack trace shows that code breaks in org.apache.myfaces.el.ValueBindingImpl.setValue This is easy to reproduce with sample1.jsp in simple example. Just throw <f:convertNumber/> inside <h:inputText id="number1"... What seems to happen is: In javax.faces.component.UIInput.validate the submittedValue is converted to Object convertedValue = getConvertedValue(context, submittedValue); In org.apache.myfaces.renderkit._SharedRendererUtils. findUIOutputConverter If you have convertNumber, converter is retrieved by Converter converter = component.getConverter(); and is javax.faces.convert.NumberConverter If you don't have convertNumber the converter is retrieved by ValueBinding vb = component.getValueBinding("value"); Class valueType = vb.getType(facesContext); facesContext.getApplication().createConverter(valueType); in my application the backing bean contains field of type short so javax.faces.convert.ShortConverter is returned. The type of convertedValue (in UIInput) is java.lang.long if NumberConverter is used and java.lang.short if Short.Converter is used. If the field type in backing bean is short and convertedValue is long, then later org.apache.myfaces.el.ValueBindingImpl.setValue can't find a setter to the field. I run out of time now, so I continue debugging later if necessary. (Hope somebody can solve this before ) Regards, Keijo Stack trace from simple example ......... javax.faces.el.EvaluationException: Cannot set value for expression '# {calcForm.number1} ' to a new value of type java.lang.Long at org.apache.myfaces.el.ValueBindingImpl.setValue(ValueBindingImpl.java:304) at javax.faces.component.UIInput.updateModel(UIInput.java:226) at javax.faces.component.UIInput.processUpdates(UIInput.java:165) at javax.faces.component.UIForm.processUpdates(UIForm.java:85) at javax.faces.component.UIComponentBase.processUpdates(UIComponentBase.java:428) at javax.faces.component.UIComponentBase.processUpdates(UIComponentBase.java:428) at javax.faces.component.UIViewRoot.processUpdates(UIViewRoot.java:153) at org.apache.myfaces.lifecycle.LifecycleImpl.updateModelValues(LifecycleImpl.java:277) at org.apache.myfaces.lifecycle.LifecycleImpl.execute(LifecycleImpl.java:81) at javax.faces.webapp.FacesServlet.service(FacesServlet.java:106)
        Hide
        Keijo Nurmes added a comment -

        Just tested with binaries from myfaces-1.1.0.zip and it does not suffer this issue.

        Show
        Keijo Nurmes added a comment - Just tested with binaries from myfaces-1.1.0.zip and it does not suffer this issue.
        Hide
        Keijo Nurmes added a comment -

        Found it.

        It was in ValueBindingImpl. If I revert the ValueBindingImpl part of this patch convertNumber works just fine.

        Revision: 293105
        Date: 15:45:03, 2. 10. 2005
        Message:
        MYFACES-623: revert PropertyResolverImpl changes to r289849; removed coercion in ValueBindingImpl.setValue


        Modified : /myfaces/impl/branches/1_1_1/src/java/org/apache/myfaces/el/PropertyResolverImpl.java
        Modified : /myfaces/impl/branches/1_1_1/src/java/org/apache/myfaces/el/ValueBindingImpl.java

        Regards, Keijo

        Show
        Keijo Nurmes added a comment - Found it. It was in ValueBindingImpl. If I revert the ValueBindingImpl part of this patch convertNumber works just fine. Revision: 293105 Date: 15:45:03, 2. 10. 2005 Message: MYFACES-623 : revert PropertyResolverImpl changes to r289849; removed coercion in ValueBindingImpl.setValue Modified : /myfaces/impl/branches/1_1_1/src/java/org/apache/myfaces/el/PropertyResolverImpl.java Modified : /myfaces/impl/branches/1_1_1/src/java/org/apache/myfaces/el/ValueBindingImpl.java Regards, Keijo
        Hide
        Keijo Nurmes added a comment -

        Anyone care to comment?
        Is it possible to revert the ValueBindingImpl.setValue patch. (2.10.2005)
        (I have been running reverted version of 1.1.1 since 26.10.2005 without complications)

        Or is there better suggestions, how to hande <f:convertNumber/> issue.?

        regards, Keijo.

        Show
        Keijo Nurmes added a comment - Anyone care to comment? Is it possible to revert the ValueBindingImpl.setValue patch. (2.10.2005) (I have been running reverted version of 1.1.1 since 26.10.2005 without complications) Or is there better suggestions, how to hande <f:convertNumber/> issue.? regards, Keijo.
        Hide
        Martin Marinschek added a comment -

        Obviously this is linked to MYFACES-623 - and removing the coercion there was not the right solution.

        regards,

        Martin

        Show
        Martin Marinschek added a comment - Obviously this is linked to MYFACES-623 - and removing the coercion there was not the right solution. regards, Martin
        Hide
        Oliver Rossmueller added a comment -

        As Martin pointed out there was a reason why coercion was removed in ValueBindingImpl.setValue: it is against the spec and therefore breaks the TCK. So I'm sorry but from my POV it is not possible to revert the ValueBindingImpl.setValue patch (2.10.2005). From the comments above I suppose the problem is more related to converters as in the exmple mentioned a short is required but the value is converted to long. It might be possible that the converter implementations rely on the coercion that was done by ValueBindingImpl and which is gone now.

        Show
        Oliver Rossmueller added a comment - As Martin pointed out there was a reason why coercion was removed in ValueBindingImpl.setValue: it is against the spec and therefore breaks the TCK. So I'm sorry but from my POV it is not possible to revert the ValueBindingImpl.setValue patch (2.10.2005). From the comments above I suppose the problem is more related to converters as in the exmple mentioned a short is required but the value is converted to long. It might be possible that the converter implementations rely on the coercion that was done by ValueBindingImpl and which is gone now.
        Hide
        Keijo Nurmes added a comment -

        Olivier, this is your comment in issue 623.

        ------------------
        "I checked the spec document and there is no word about coercion in the 5.1.4 Set Value Semantics section. I also had a look at the RI sources and there is no coercion in ValueBindingImpl.setValue, either. So I suppose it's save to remove coercion in MyFaces' ValueBindingImpl, too. From my POV it's even required to do so to comply with the spec.

        In my interpretion of the spec it's up to the application developer to provide a converter where necessary, but it's not required for the JSF runtime to try to guess the right conversion to match UI and model."
        ------------------

        How can this be against the spec if it's not mentioned in it ?
        Didn't myfaces pass the TCK tests without this modification?
        IMHO the last sentence in that comment pretty much ruins the usability of myfaces.
        (Imagine every component working like this.. )

        If everybody agrees the removal of coercion in ValueBindingImpl.setValue (as it is now)
        I ( or somebody else) have to figure out how to bring back some intelligence to convertNumber.

        regards, Keijo

        Show
        Keijo Nurmes added a comment - Olivier, this is your comment in issue 623. ------------------ "I checked the spec document and there is no word about coercion in the 5.1.4 Set Value Semantics section. I also had a look at the RI sources and there is no coercion in ValueBindingImpl.setValue, either. So I suppose it's save to remove coercion in MyFaces' ValueBindingImpl, too. From my POV it's even required to do so to comply with the spec. In my interpretion of the spec it's up to the application developer to provide a converter where necessary, but it's not required for the JSF runtime to try to guess the right conversion to match UI and model." ------------------ How can this be against the spec if it's not mentioned in it ? Didn't myfaces pass the TCK tests without this modification? IMHO the last sentence in that comment pretty much ruins the usability of myfaces. (Imagine every component working like this.. ) If everybody agrees the removal of coercion in ValueBindingImpl.setValue (as it is now) I ( or somebody else) have to figure out how to bring back some intelligence to convertNumber. regards, Keijo
        Hide
        Gerald Müllan added a comment -

        Oliver removed the coercion to be spec compliant - we weren't before.

        do you have the same problems with the RI?

        regards,

        Martin

        Show
        Gerald Müllan added a comment - Oliver removed the coercion to be spec compliant - we weren't before. do you have the same problems with the RI? regards, Martin
        Hide
        Keijo Nurmes added a comment -

        RI is not an option for me. ( Using tiles etc... )

        The current behaviour of convertNumber does not feel right.
        I just want to make sure before changing my applications (or implementing my own convertNumber) that this is here to stay.

        regards, Keijo

        Show
        Keijo Nurmes added a comment - RI is not an option for me. ( Using tiles etc... ) The current behaviour of convertNumber does not feel right. I just want to make sure before changing my applications (or implementing my own convertNumber) that this is here to stay. regards, Keijo
        Hide
        Martin Marinschek added a comment -

        Even if you don't use the RI- maybe you could check out a small sample app with the RI.

        If you find out that the same behaviour exists in the RI, you need to go to the spec group and tell them to roll up their sleeves

        If the behaviour is different in the RI, we'll need to fix that.

        regards,

        Martin

        Show
        Martin Marinschek added a comment - Even if you don't use the RI- maybe you could check out a small sample app with the RI. If you find out that the same behaviour exists in the RI, you need to go to the spec group and tell them to roll up their sleeves If the behaviour is different in the RI, we'll need to fix that. regards, Martin
        Hide
        Keijo Nurmes added a comment -

        I did a testrun with RI.

        RI seems to behave quite alike.

        I did modify jsf-guessNumber sample application.

        After adding <f:convertNumber/> in <h:inputText id="userNo".. tag I get
        java.lang.IllegalArgumentException: argument type mismatch

        If I change UserNumberBean.java userNumber from Integer to Long
        Applications seems to work fine with convertNumber .

        Now I am totally out of time, so I leave it here....

        regards,
        Keijo

        Show
        Keijo Nurmes added a comment - I did a testrun with RI. RI seems to behave quite alike. I did modify jsf-guessNumber sample application. After adding <f:convertNumber/> in <h:inputText id="userNo".. tag I get java.lang.IllegalArgumentException: argument type mismatch If I change UserNumberBean.java userNumber from Integer to Long Applications seems to work fine with convertNumber . Now I am totally out of time, so I leave it here.... regards, Keijo
        Hide
        Sylvain Vieujot added a comment -

        I didn't find how to fix this yet, but here is a workaround.
        As NumberFormat.parse returns either a Long or a Double, the workaroud way is :

        • For Integer values, use only Long or long accessor types.
        • For non Integer, use only Number (as an integer value, like 3, will return a Long, and a non integer, as 3.14 will return a Double). So, using a Double or double will fail for integer values. So, Number is the only safe way.
        Show
        Sylvain Vieujot added a comment - I didn't find how to fix this yet, but here is a workaround. As NumberFormat.parse returns either a Long or a Double, the workaroud way is : For Integer values, use only Long or long accessor types. For non Integer, use only Number (as an integer value, like 3, will return a Long, and a non integer, as 3.14 will return a Double). So, using a Double or double will fail for integer values. So, Number is the only safe way.
        Hide
        Pawel Koloszko added a comment -

        What are the plans regarding this problem? I have just switched from 1.0.9 to 1.1.1 and because of many number converters in my application this problem is critical for me. Beacuse of that I've been forced to switch to 1.1.0, but I am also not very happy with it, because of problems with jsCookMenu.

        Show
        Pawel Koloszko added a comment - What are the plans regarding this problem? I have just switched from 1.0.9 to 1.1.1 and because of many number converters in my application this problem is critical for me. Beacuse of that I've been forced to switch to 1.1.0, but I am also not very happy with it, because of problems with jsCookMenu.
        Hide
        Mike Kienenberger added a comment -

        It seems clear that the intended data type for numberConverter setters is "Number". That's probably why it's called convertNumber.

        Using any other type is going to cause problems since you don't know how the converter is going to return the value.

        It's trivial enough to use Number.floatValue() or Number.intValue() in your setter if you want a specific conversion.

        Other than that, your only option is to write your own converter. This issue should be marked Won't Fix, and the issue subject should probably be changed to "Number converter not working with arbitrary Number subclasses"

        Show
        Mike Kienenberger added a comment - It seems clear that the intended data type for numberConverter setters is "Number". That's probably why it's called convertNumber. Using any other type is going to cause problems since you don't know how the converter is going to return the value. It's trivial enough to use Number.floatValue() or Number.intValue() in your setter if you want a specific conversion. Other than that, your only option is to write your own converter. This issue should be marked Won't Fix, and the issue subject should probably be changed to "Number converter not working with arbitrary Number subclasses"

          People

          • Assignee:
            Mike Kienenberger
            Reporter:
            Sylvain Vieujot
          • Votes:
            1 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development