MyFaces Core
  1. MyFaces Core
  2. MYFACES-1890

Numberconverter has issue with bigdecimal

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.2.0, 1.2.2, 1.2.3
    • Fix Version/s: 1.2.10, 2.0.3
    • Component/s: None
    • Labels:
      None

      Description

      Due to a potential bug in BigDecimal there is a bug, when you use BigDecimal
      with a NumberConverter.

      Like
      <tr:inputText value="#

      {bean.number}

      " ...>
      <tr:convertNumber />
      </tr:inputText>

      For instance, when the entered value is "333.111" the actual stored value is 333.1109999999999899955582804977893829345703125

      There is a mathematic explanation for that in here:
      http://en.wikipedia.org/wiki/Floating_point#Accuracy_problems

        Issue Links

          Activity

          Matthias Weßendorf created issue -
          Matthias Weßendorf made changes -
          Field Original Value New Value
          Link This issue is a clone of TRINIDAD-1124 [ TRINIDAD-1124 ]
          Matthias Weßendorf made changes -
          Fix Version/s 1.2.9-core [ 12313157 ]
          Fix Version/s 1.0.9-core [ 12313158 ]
          Key TRINIDAD-1148 MYFACES-1890
          Project MyFaces Trinidad [ 12310661 ] MyFaces Core [ 10600 ]
          Matthias Weßendorf made changes -
          Affects Version/s 1.2.3 [ 12313144 ]
          Affects Version/s  1.2.0 [ 12312576 ]
          Affects Version/s 1.2.2 [ 12312932 ]
          Hide
          Matthias Weßendorf added a comment -

          Fixed in Trunk.
          Since the RI also made this change, there will be no problem regarding the TCK.
          (since now the NumberConverter always returns a BigDecimal).
          Not a big deal, since that guy becomes faster and faster.
          EL engine is also able to "convert" to double, etc, since BigDecimal implements java.lang.Nummer as well

          Show
          Matthias Weßendorf added a comment - Fixed in Trunk. Since the RI also made this change, there will be no problem regarding the TCK. (since now the NumberConverter always returns a BigDecimal). Not a big deal, since that guy becomes faster and faster. EL engine is also able to "convert" to double, etc, since BigDecimal implements java.lang.Nummer as well
          Matthias Weßendorf made changes -
          Fix Version/s 1.2.4-SNAPSHOT [ 12313145 ]
          Status Open [ 1 ] Resolved [ 5 ]
          Resolution Fixed [ 1 ]
          Hide
          Leonardo Uribe added a comment -

          Matthias filled:

          https://javaserverfaces-spec-public.dev.java.net/issues/show_bug.cgi?id=448

          The solution provided is correct, but causes some problems with tck, so it will switch back temporally to the old behavior. After release this issue will be closed.

          Show
          Leonardo Uribe added a comment - Matthias filled: https://javaserverfaces-spec-public.dev.java.net/issues/show_bug.cgi?id=448 The solution provided is correct, but causes some problems with tck, so it will switch back temporally to the old behavior. After release this issue will be closed.
          Leonardo Uribe made changes -
          Resolution Fixed [ 1 ]
          Status Resolved [ 5 ] Reopened [ 4 ]
          Leonardo Uribe made changes -
          Fix Version/s 1.2.4-SNAPSHOT [ 12313145 ]
          Hide
          Leonardo Uribe added a comment -

          Checking myfaces-commons-converters TypedNumberConverter and reading again this issue and TRINIDAD-1124, the conclusion is the only thing we can do is check if the ValueExpression return BigDecimal and if that so, call setParseBigDecimal(true). It could be a little bit expensive compared with the time required to do the operation, but it is better fix it and pass TCK tests. Instead, on TypedNumberConverter we can take advantage of destType property so the EL call to getType() will not be required in that case.

          Show
          Leonardo Uribe added a comment - Checking myfaces-commons-converters TypedNumberConverter and reading again this issue and TRINIDAD-1124 , the conclusion is the only thing we can do is check if the ValueExpression return BigDecimal and if that so, call setParseBigDecimal(true). It could be a little bit expensive compared with the time required to do the operation, but it is better fix it and pass TCK tests. Instead, on TypedNumberConverter we can take advantage of destType property so the EL call to getType() will not be required in that case.
          Leonardo Uribe made changes -
          Status Reopened [ 4 ] Resolved [ 5 ]
          Fix Version/s 1.2.10-SNAPSHOT [ 12315114 ]
          Fix Version/s 2.0.3-SNAPSHOT [ 12315349 ]
          Resolution Fixed [ 1 ]
          Hide
          Matthias Weßendorf added a comment -

          it's not expensive

          Show
          Matthias Weßendorf added a comment - it's not expensive
          Leonardo Uribe made changes -
          Fix Version/s 1.2.10 [ 12315978 ]
          Fix Version/s 2.0.3 [ 12315976 ]
          Fix Version/s 1.2.10-SNAPSHOT [ 12315114 ]
          Fix Version/s 2.0.3-SNAPSHOT [ 12315349 ]
          Leonardo Uribe made changes -
          Status Resolved [ 5 ] Closed [ 6 ]
          Hide
          Paul Nicolucci added a comment -

          I've run into an issue where this fix does not work. When dealing with a composite component:

          valueBinding.getType(facesContext.getElContext());

          returns java.lang.Object since the MapELResolver.getType returns Object.class in accordance with the EL Spec, the JavaDoc can be found here:

          http://java.sun.com/javaee/5/docs/api/javax/el/MapELResolver.html

          The fix was made in the following JIRA issue for the TomCat EL:
          https://issues.apache.org/bugzilla/show_bug.cgi?id=51177

          So in the case of a composite component the MyFaces Implementation still has an issue. I just wanted to make a note on this issue, I can open another JIRA issue if needed.

          Could a context parameter be added to set df.setParseBigDecimal(true); so that we can avoid the problem faced here. I think the only reason this was not made the default behavior was to keep some samples from failing or existing applications that depend on the behavior from failing.

          I'm willing to create,test and contribute the fix. Please let me know what your thoughts are on this issue.

          Thanks,

          Paul Nicolucci

          Show
          Paul Nicolucci added a comment - I've run into an issue where this fix does not work. When dealing with a composite component: valueBinding.getType(facesContext.getElContext()); returns java.lang.Object since the MapELResolver.getType returns Object.class in accordance with the EL Spec, the JavaDoc can be found here: http://java.sun.com/javaee/5/docs/api/javax/el/MapELResolver.html The fix was made in the following JIRA issue for the TomCat EL: https://issues.apache.org/bugzilla/show_bug.cgi?id=51177 So in the case of a composite component the MyFaces Implementation still has an issue. I just wanted to make a note on this issue, I can open another JIRA issue if needed. Could a context parameter be added to set df.setParseBigDecimal(true); so that we can avoid the problem faced here. I think the only reason this was not made the default behavior was to keep some samples from failing or existing applications that depend on the behavior from failing. I'm willing to create,test and contribute the fix. Please let me know what your thoughts are on this issue. Thanks, Paul Nicolucci

            People

            • Assignee:
              Matthias Weßendorf
              Reporter:
              Matthias Weßendorf
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development