Repository: incubator-freemarker
Updated Branches:
  refs/heads/3 0b02af638 -> 2385a24d4


Forward ported from 2.3-gae: The default arithmetic engine 
(ArithmeticEngine.BIGDECIMAL_ENGINE) can now compare infinite (both positive 
and negative) to any other standard type. Earlier, since BigDecimal can't 
represent infinite, it was only working in certain special cases. Also did some 
performance optimizations to slightly decrease the impact and number of 
conversions to BigDecimal.


Project: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/repo
Commit: 
http://git-wip-us.apache.org/repos/asf/incubator-freemarker/commit/2385a24d
Tree: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/tree/2385a24d
Diff: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/diff/2385a24d

Branch: refs/heads/3
Commit: 2385a24d498a66cdab820aba69489dd1bedc5204
Parents: 0b02af6
Author: ddekany <ddek...@apache.org>
Authored: Sun Mar 11 22:23:39 2018 +0100
Committer: ddekany <ddek...@apache.org>
Committed: Sun Mar 11 22:23:39 2018 +0100

----------------------------------------------------------------------
 .../impl/BigDecimalArithmeticEngineTest.java    | 97 ++++++++++++++++++++
 .../impl/BigDecimalArithmeticEngine.java        | 84 ++++++++++++++++-
 .../freemarker/core/util/_NumberUtils.java      | 36 +++++++-
 3 files changed, 208 insertions(+), 9 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/2385a24d/freemarker-core-test/src/test/java/org/apache/freemarker/core/arithmetic/impl/BigDecimalArithmeticEngineTest.java
----------------------------------------------------------------------
diff --git 
a/freemarker-core-test/src/test/java/org/apache/freemarker/core/arithmetic/impl/BigDecimalArithmeticEngineTest.java
 
b/freemarker-core-test/src/test/java/org/apache/freemarker/core/arithmetic/impl/BigDecimalArithmeticEngineTest.java
new file mode 100644
index 0000000..422ec16
--- /dev/null
+++ 
b/freemarker-core-test/src/test/java/org/apache/freemarker/core/arithmetic/impl/BigDecimalArithmeticEngineTest.java
@@ -0,0 +1,97 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.freemarker.core.arithmetic.impl;
+
+import static 
org.apache.freemarker.core.arithmetic.impl.BigDecimalArithmeticEngine.*;
+import static org.junit.Assert.*;
+
+import java.math.BigDecimal;
+import java.math.BigInteger;
+
+import org.junit.Test;
+
+public class BigDecimalArithmeticEngineTest {
+
+    @Test
+    public void compareNumbersZeroTest() {
+        assertEquals(0, INSTANCE.compareNumbers(0, new BigDecimal("-0")));
+        assertEquals(0, INSTANCE.compareNumbers(0.0, new BigDecimal("-0")));
+        assertEquals(0, INSTANCE.compareNumbers(-0.0, new BigDecimal("+0")));
+        assertEquals(0, INSTANCE.compareNumbers(-0.0, 0.0));
+    }
+
+    @Test
+    public void compareNumbersNoRoundingGlitchTest() {
+        assertEquals(0, INSTANCE.compareNumbers(1.1, new BigDecimal("1.1")));
+        assertEquals(-1, INSTANCE.compareNumbers(Double.MIN_VALUE, 
Math.nextUp(Double.MIN_VALUE)));
+        assertEquals(-1, INSTANCE.compareNumbers(
+                Double.MIN_VALUE, new BigDecimal("" + 
Math.nextUp(Double.MIN_VALUE))));
+    }
+
+    @Test
+    public void compareNumbersSameTypeTest() {
+        assertEquals(-1, INSTANCE.compareNumbers(1, 2));
+        assertEquals(-1, INSTANCE.compareNumbers(1L, 2L));
+        assertEquals(-1, INSTANCE.compareNumbers((short) 1, (short) 2));
+        assertEquals(-1, INSTANCE.compareNumbers((byte) 1, (byte) 2));
+        assertEquals(-1, INSTANCE.compareNumbers(1.0, 2.0));
+        assertEquals(-1, INSTANCE.compareNumbers(1.0f, 2.0f));
+        assertEquals(-1, INSTANCE.compareNumbers(BigDecimal.ONE, 
BigDecimal.TEN));
+    }
+
+    @Test
+    public void compareNumbersScaleDoesNotMatterTest() {
+        assertEquals(0, INSTANCE.compareNumbers(1.0, new BigDecimal("1")));
+        assertEquals(0, INSTANCE.compareNumbers(1.0, new BigDecimal("1.00")));
+        assertEquals(0, INSTANCE.compareNumbers(1.0f, new 
BigDecimal("0.1E1")));
+        assertEquals(0, INSTANCE.compareNumbers(1, new BigDecimal("1.0")));
+    }
+
+    @Test
+    public void compareNumbersInfinityTest() {
+        for (boolean isFloat : new boolean[] { false, true }) {
+            Number negInf = isFloat ? Float.NEGATIVE_INFINITY : 
Double.NEGATIVE_INFINITY;
+            Number posInf = isFloat ? Float.POSITIVE_INFINITY : 
Double.POSITIVE_INFINITY;
+            
+            assertEquals(-1, INSTANCE.compareNumbers(negInf, posInf));
+            assertEquals(1, INSTANCE.compareNumbers(posInf, negInf));
+            assertEquals(0, INSTANCE.compareNumbers(posInf, posInf));
+            assertEquals(0, INSTANCE.compareNumbers(negInf, negInf));
+            
+            Number otherNegInf = !isFloat ? Float.NEGATIVE_INFINITY : 
Double.NEGATIVE_INFINITY;
+            Number otherPosInf = !isFloat ? Float.POSITIVE_INFINITY : 
Double.POSITIVE_INFINITY;
+            assertEquals(-1, INSTANCE.compareNumbers(negInf, otherPosInf));
+            assertEquals(1, INSTANCE.compareNumbers(posInf, otherNegInf));
+            assertEquals(0, INSTANCE.compareNumbers(posInf, otherPosInf));
+            assertEquals(0, INSTANCE.compareNumbers(negInf, otherNegInf));
+            
+            for (Number one : new Number[] {
+                    BigDecimal.ONE, 1.0, 1f, 1, 1L, (byte) 1, (short) 1, 
BigInteger.ONE,
+                    BigDecimal.ONE.negate(), -1.0, -1f, -1, -1L, (byte) -1, 
(short) -1, BigInteger.ONE.negate(),
+                    BigDecimal.ZERO, 0.0, 0f, 0, 0L, (byte) 0, (short) 0, 
BigInteger.ZERO }) {
+                assertEquals(-1, INSTANCE.compareNumbers(negInf, one));
+                assertEquals(1, INSTANCE.compareNumbers(posInf, one));
+                assertEquals(1, INSTANCE.compareNumbers(one, negInf));
+                assertEquals(-1, INSTANCE.compareNumbers(one, posInf));
+            }
+        }
+    }
+    
+}

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/2385a24d/freemarker-core/src/main/java/org/apache/freemarker/core/arithmetic/impl/BigDecimalArithmeticEngine.java
----------------------------------------------------------------------
diff --git 
a/freemarker-core/src/main/java/org/apache/freemarker/core/arithmetic/impl/BigDecimalArithmeticEngine.java
 
b/freemarker-core/src/main/java/org/apache/freemarker/core/arithmetic/impl/BigDecimalArithmeticEngine.java
index d09d48f..827b0e8 100644
--- 
a/freemarker-core/src/main/java/org/apache/freemarker/core/arithmetic/impl/BigDecimalArithmeticEngine.java
+++ 
b/freemarker-core/src/main/java/org/apache/freemarker/core/arithmetic/impl/BigDecimalArithmeticEngine.java
@@ -40,16 +40,90 @@ public class BigDecimalArithmeticEngine extends 
ArithmeticEngine {
         // We try to find the result based on the sign (+/-/0) first, because:
         // - It's much faster than converting to BigDecial, and comparing to 0 
is the most common comparison.
         // - It doesn't require any type conversions, and thus things like 
"Infinity > 0" won't fail.
-        int firstSignum = _NumberUtils.getSignum(first);
+        int firstSignum = _NumberUtils.getSignum(first); 
         int secondSignum = _NumberUtils.getSignum(second);
         if (firstSignum != secondSignum) {
-            return firstSignum < secondSignum ? -1 : (firstSignum > 
secondSignum ? 1 : 0);
+            return firstSignum < secondSignum ? -1 : (firstSignum > 
secondSignum ? 1 : 0); 
         } else if (firstSignum == 0 && secondSignum == 0) {
             return 0;
         } else {
-            BigDecimal left = _NumberUtils.toBigDecimal(first);
-            BigDecimal right = _NumberUtils.toBigDecimal(second);
-            return left.compareTo(right);
+            // The most common case is comparing values of the same type. As 
BigDecimal can represent all of these
+            // with loseless round-trip (i.e., converting to BigDecimal and 
then back the original type gives the
+            // original value), we can avoid conversion to BigDecimal without 
changing the result.
+            if (first.getClass() == second.getClass()) {
+                // Bit of optimization for this is a very common case:
+                if (first instanceof BigDecimal) {
+                    return ((BigDecimal) first).compareTo((BigDecimal) second);
+                }
+                
+                if (first instanceof Integer) {
+                    return ((Integer) first).compareTo((Integer) second);
+                }
+                if (first instanceof Long) {
+                    return ((Long) first).compareTo((Long) second);
+                }
+                if (first instanceof Double) {
+                    return ((Double) first).compareTo((Double) second);
+                }
+                if (first instanceof Float) {
+                    return ((Float) first).compareTo((Float) second);
+                }
+                if (first instanceof Byte) {
+                    return ((Byte) first).compareTo((Byte) second);
+                }
+                if (first instanceof Short) {
+                    return ((Short) first).compareTo((Short) second);
+                }
+            }
+            // We are going to compare values of two different types.
+            
+            // Handle infinity before we try conversion to BigDecimal, as that 
BigDecimal can't represent that:
+            if (first instanceof Double) {
+                double firstD = first.doubleValue();
+                if (Double.isInfinite(firstD)) {
+                    if 
(_NumberUtils.hasTypeThatIsKnownToNotSupportInfiniteAndNaN(second)) {
+                        return  firstD == Double.NEGATIVE_INFINITY ? -1 : 1;
+                    }
+                    if (second instanceof Float) {
+                        return Double.compare(firstD, second.doubleValue());
+                    }
+                }
+            }
+            if (first instanceof Float) {
+                float firstF = first.floatValue();
+                if (Float.isInfinite(firstF)) {
+                    if 
(_NumberUtils.hasTypeThatIsKnownToNotSupportInfiniteAndNaN(second)) {
+                        return firstF == Float.NEGATIVE_INFINITY ? -1 : 1;
+                    }
+                    if (second instanceof Double) {
+                        return Double.compare(firstF, second.doubleValue());
+                    }
+                }
+            }
+            if (second instanceof Double) {
+                double secondD = second.doubleValue();
+                if (Double.isInfinite(secondD)) {
+                    if 
(_NumberUtils.hasTypeThatIsKnownToNotSupportInfiniteAndNaN(first)) {
+                        return secondD == Double.NEGATIVE_INFINITY ? 1 : -1;
+                    }
+                    if (first instanceof Float) {
+                        return Double.compare(first.doubleValue(), secondD);
+                    }
+                }
+            }
+            if (second instanceof Float) {
+                float secondF = second.floatValue();
+                if (Float.isInfinite(secondF)) {
+                    if 
(_NumberUtils.hasTypeThatIsKnownToNotSupportInfiniteAndNaN(first)) {
+                        return secondF == Float.NEGATIVE_INFINITY ? 1 : -1;
+                    }
+                    if (first instanceof Double) {
+                        return Double.compare(first.doubleValue(), secondF);
+                    }
+                }
+            }
+            
+            return 
_NumberUtils.toBigDecimal(first).compareTo(_NumberUtils.toBigDecimal(second));
         }
     }
 

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/2385a24d/freemarker-core/src/main/java/org/apache/freemarker/core/util/_NumberUtils.java
----------------------------------------------------------------------
diff --git 
a/freemarker-core/src/main/java/org/apache/freemarker/core/util/_NumberUtils.java
 
b/freemarker-core/src/main/java/org/apache/freemarker/core/util/_NumberUtils.java
index 5cff40b..9c65eaf 100644
--- 
a/freemarker-core/src/main/java/org/apache/freemarker/core/util/_NumberUtils.java
+++ 
b/freemarker-core/src/main/java/org/apache/freemarker/core/util/_NumberUtils.java
@@ -39,7 +39,7 @@ public class _NumberUtils {
             return ((Double) num).isInfinite();
         } else if (num instanceof Float) {
             return ((Float) num).isInfinite();
-        } else if (isNonFPNumberOfSupportedClass(num)) {
+        } else if (hasTypeThatIsKnownToNotSupportInfiniteAndNaN(num)) {
             return false;
         } else {
             throw new UnsupportedNumberClassException(num.getClass());
@@ -51,7 +51,7 @@ public class _NumberUtils {
             return ((Double) num).isNaN();
         } else if (num instanceof Float) {
             return ((Float) num).isNaN();
-        } else if (isNonFPNumberOfSupportedClass(num)) {
+        } else if (hasTypeThatIsKnownToNotSupportInfiniteAndNaN(num)) {
             return false;
         } else {
             throw new UnsupportedNumberClassException(num.getClass());
@@ -110,7 +110,12 @@ public class _NumberUtils {
         // condition, but stripTrailingZeros was slower than setScale + 
compareTo.
     }
     
-    private static boolean isNonFPNumberOfSupportedClass(Number num) {
+    /**
+     * Tells if the type of the parameter number is known to not be able to 
represent infinite (positive or negative)
+     * and NaN. If this returns {@code false}, that doesn't mean that it can 
do that, because it's maybe just that this
+     * utility doesn't know that type.
+     */
+    public static boolean hasTypeThatIsKnownToNotSupportInfiniteAndNaN(Number 
num) {
         return num instanceof Integer || num instanceof BigDecimal || num 
instanceof Long
                 || num instanceof Short || num instanceof Byte || num 
instanceof BigInteger;
     }
@@ -199,10 +204,33 @@ public class _NumberUtils {
         return number;
     }
 
+    /**
+     * Convert a {@code Number} to {@link BigDecimal}.
+     * 
+     * @throws NumberFormatException
+     *             If the conversion is not possible, e.g. Infinite and NaN 
can't be converted to {@link BigDecimal}.
+     */
     public static BigDecimal toBigDecimal(Number num) {
+        if (num instanceof BigDecimal) {
+            return (BigDecimal) num;
+        }
+        if (num instanceof Integer || num instanceof Long || num instanceof 
Byte || num instanceof Short) {
+            return BigDecimal.valueOf(num.longValue());
+        }
+        if (num instanceof BigInteger) {
+            return new BigDecimal((BigInteger) num);
+        }
         try {
-            return num instanceof BigDecimal ? (BigDecimal) num : new 
BigDecimal(num.toString());
+            // Why toString? It's partly for backward compatibility. But it's 
also better for Double (and Float) values
+            // than new BigDecimal(someDouble), which is overly precise. For 
example, if you have `double d = 0.1`, then
+            // `new BigDecimal(d).equals(new BigDecimal("0.1"))` is `false`, 
while
+            // `new BigDecimal(Double.toString(d)).equals(new 
BigDecimal("0.1"))` is `true`.
+            return new BigDecimal(num.toString());
         } catch (NumberFormatException e) {
+            if (isInfinite(num)) {
+                throw new NumberFormatException("It's impossible to convert an 
infinte value ("
+                        + num.getClass().getSimpleName() + " " + num + ") to 
BigDecimal.");
+            }
             // The exception message is useless, so we add a new one:
             throw new NumberFormatException("Can't parse this as BigDecimal 
number: " + _StringUtils.jQuote(num));
         }

Reply via email to