svn commit: r1786080 - in /ofbiz/branches/release16.11: ./ framework/minilang/src/main/java/org/apache/ofbiz/minilang/method/conditional/Compare.java

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

svn commit: r1786080 - in /ofbiz/branches/release16.11: ./ framework/minilang/src/main/java/org/apache/ofbiz/minilang/method/conditional/Compare.java

jleroux@apache.org
Author: jleroux
Date: Wed Mar  8 21:28:37 2017
New Revision: 1786080

URL: http://svn.apache.org/viewvc?rev=1786080&view=rev
Log:
"Applied fix from trunk framework for revision: 1786079"
------------------------------------------------------------------------
r1786079 | jleroux | 2017-03-08 22:27:30 +0100 (mer. 08 mars 2017) | 9 lignes

Fixed: compareBigDecimals in org.ofbiz.minilang.method.conditional.Compare
does not compare certain values correctly
(OFBIZ-6386)

compareBigDecimals scales down and rounds up meaning you lose information and
the comparison result is not as expected

Thanks: Gareth Carter for the patch, Mridul Pathak and Jacopo for discussing the
topic in all details (see the Jira issue)
------------------------------------------------------------------------


Modified:
    ofbiz/branches/release16.11/   (props changed)
    ofbiz/branches/release16.11/framework/minilang/src/main/java/org/apache/ofbiz/minilang/method/conditional/Compare.java

Propchange: ofbiz/branches/release16.11/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Wed Mar  8 21:28:37 2017
@@ -10,5 +10,5 @@
 /ofbiz/branches/json-integration-refactoring:1634077-1635900
 /ofbiz/branches/multitenant20100310:921280-927264
 /ofbiz/branches/release13.07:1547657
-/ofbiz/ofbiz-framework/trunk:1783202,1783388,1784549,1784558,1784708,1785882,1785925
+/ofbiz/ofbiz-framework/trunk:1783202,1783388,1784549,1784558,1784708,1785882,1785925,1786079
 /ofbiz/trunk:1770481,1770490,1770540,1771440,1771448,1771516,1771935,1772346,1772880,1774772,1775441,1779724,1780659,1781109,1781125,1781979,1782498,1782520

Modified: ofbiz/branches/release16.11/framework/minilang/src/main/java/org/apache/ofbiz/minilang/method/conditional/Compare.java
URL: http://svn.apache.org/viewvc/ofbiz/branches/release16.11/framework/minilang/src/main/java/org/apache/ofbiz/minilang/method/conditional/Compare.java?rev=1786080&r1=1786079&r2=1786080&view=diff
==============================================================================
--- ofbiz/branches/release16.11/framework/minilang/src/main/java/org/apache/ofbiz/minilang/method/conditional/Compare.java (original)
+++ ofbiz/branches/release16.11/framework/minilang/src/main/java/org/apache/ofbiz/minilang/method/conditional/Compare.java Wed Mar  8 21:28:37 2017
@@ -18,8 +18,6 @@
  *******************************************************************************/
 package org.apache.ofbiz.minilang.method.conditional;
 
-import java.math.BigDecimal;
-import java.math.RoundingMode;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
@@ -49,18 +47,6 @@ public abstract class Compare {
         }
     }
 
-    private static int compareBigDecimals(BigDecimal lBigDecimal, BigDecimal rBigDecimal) {
-        // Developers: Do not change this. We are comparing two fixed-point decimal numbers,
-        // not performing accounting calculations - so it is okay to specify the rounding mode.
-        int decimals = lBigDecimal.scale();
-        if (rBigDecimal.scale() < decimals) {
-            lBigDecimal = lBigDecimal.setScale(rBigDecimal.scale(), RoundingMode.UP);
-        } else if (decimals < rBigDecimal.scale()) {
-            rBigDecimal = rBigDecimal.setScale(decimals, RoundingMode.UP);
-        }
-        return lBigDecimal.compareTo(rBigDecimal);
-    }
-
     private static Map<String, Compare> createInstanceMap() {
         Map<String, Compare> writableMap = new HashMap<String, Compare>(10);
         writableMap.put("contains", new CompareContains());
@@ -132,12 +118,6 @@ public abstract class Compare {
             if (convertedRvalue == null) {
                 return false;
             }
-            if (convertedLvalue instanceof BigDecimal &&
-                convertedRvalue instanceof BigDecimal) {
-                BigDecimal lBigDecimal = (BigDecimal) convertedLvalue;
-                BigDecimal rBigDecimal = (BigDecimal) convertedRvalue;
-                return compareBigDecimals(lBigDecimal, rBigDecimal) == 0;
-            }
             if (convertedLvalue instanceof Comparable &&
                 convertedRvalue instanceof Comparable) {
                 Comparable<Object> comparable = UtilGenerics.cast(convertedLvalue);
@@ -154,12 +134,6 @@ public abstract class Compare {
             Object convertedLvalue = MiniLangUtil.convertType(lValue, type, locale, timeZone, format);
             Object convertedRvalue = MiniLangUtil.convertType(rValue, type, locale, timeZone, format);
             assertValuesNotNull(convertedLvalue, convertedRvalue);
-            if (convertedLvalue instanceof BigDecimal &&
-                convertedRvalue instanceof BigDecimal) {
-                BigDecimal lBigDecimal = (BigDecimal) convertedLvalue;
-                BigDecimal rBigDecimal = (BigDecimal) convertedRvalue;
-                return compareBigDecimals(lBigDecimal, rBigDecimal) > 0;
-            }
             if (convertedLvalue instanceof Comparable &&
                 convertedRvalue instanceof Comparable) {
                 Comparable<Object> comparable = UtilGenerics.cast(convertedLvalue);
@@ -176,12 +150,6 @@ public abstract class Compare {
             Object convertedLvalue = MiniLangUtil.convertType(lValue, type, locale, timeZone, format);
             Object convertedRvalue = MiniLangUtil.convertType(rValue, type, locale, timeZone, format);
             assertValuesNotNull(convertedLvalue, convertedRvalue);
-            if (convertedLvalue instanceof BigDecimal &&
-                convertedRvalue instanceof BigDecimal) {
-                BigDecimal lBigDecimal = (BigDecimal) convertedLvalue;
-                BigDecimal rBigDecimal = (BigDecimal) convertedRvalue;
-                return compareBigDecimals(lBigDecimal, rBigDecimal) >= 0;
-            }
             if (convertedLvalue instanceof Comparable &&
                 convertedRvalue instanceof Comparable) {
                 Comparable<Object> comparable = UtilGenerics.cast(convertedLvalue);
@@ -225,12 +193,6 @@ public abstract class Compare {
             Object convertedLvalue = MiniLangUtil.convertType(lValue, type, locale, timeZone, format);
             Object convertedRvalue = MiniLangUtil.convertType(rValue, type, locale, timeZone, format);
             assertValuesNotNull(convertedLvalue, convertedRvalue);
-            if (convertedLvalue instanceof BigDecimal &&
-                convertedRvalue instanceof BigDecimal) {
-                BigDecimal lBigDecimal = (BigDecimal) convertedLvalue;
-                BigDecimal rBigDecimal = (BigDecimal) convertedRvalue;
-                return compareBigDecimals(lBigDecimal, rBigDecimal) < 0;
-            }
             if (convertedLvalue instanceof Comparable &&
                 convertedRvalue instanceof Comparable) {
                 Comparable<Object> comparable = UtilGenerics.cast(convertedLvalue);
@@ -247,12 +209,6 @@ public abstract class Compare {
             Object convertedLvalue = MiniLangUtil.convertType(lValue, type, locale, timeZone, format);
             Object convertedRvalue = MiniLangUtil.convertType(rValue, type, locale, timeZone, format);
             assertValuesNotNull(convertedLvalue, convertedRvalue);
-            if (convertedLvalue instanceof BigDecimal &&
-                convertedRvalue instanceof BigDecimal) {
-                BigDecimal lBigDecimal = (BigDecimal) convertedLvalue;
-                BigDecimal rBigDecimal = (BigDecimal) convertedRvalue;
-                return compareBigDecimals(lBigDecimal, rBigDecimal) <= 0;
-            }
             if (convertedLvalue instanceof Comparable &&
                 convertedRvalue instanceof Comparable) {
                 Comparable<Object> comparable = UtilGenerics.cast(convertedLvalue);
@@ -274,12 +230,6 @@ public abstract class Compare {
             if (convertedRvalue == null) {
                 return true;
             }
-            if (convertedLvalue instanceof BigDecimal &&
-                convertedRvalue instanceof BigDecimal) {
-                BigDecimal lBigDecimal = (BigDecimal) convertedLvalue;
-                BigDecimal rBigDecimal = (BigDecimal) convertedRvalue;
-                return compareBigDecimals(lBigDecimal, rBigDecimal) != 0;
-            }
             if (convertedLvalue instanceof Comparable &&
                 convertedRvalue instanceof Comparable) {
                 Comparable<Object> comparable = UtilGenerics.cast(convertedLvalue);