Merge pull request #575 from johnjaylward/fix-similar-compare-numbers

Fix similar compare numbers

build.gradle

@@ -23,7 +23,7 @@ repositories {
 }
 
 dependencies {
-    testImplementation 'junit:junit:4.12'
+    testImplementation 'junit:junit:4.13.1'
     testImplementation 'com.jayway.jsonpath:json-path:2.1.0'
     testImplementation 'org.mockito:mockito-core:1.9.5'
 }

pom.xml

@@ -80,7 +80,7 @@
         <dependency>
             <groupId>junit</groupId>
             <artifactId>junit</artifactId>
-            <version>4.12</version>
+            <version>4.13.1</version>
             <scope>test</scope>
         </dependency>
         <dependency>

src/main/java/org/json/JSONArray.java

@@ -1374,6 +1374,8 @@ public boolean similar(Object other) {
                 if (!((JSONArray)valueThis).similar(valueOther)) {
                     return false;
                 }
+            } else if (valueThis instanceof Number && valueOther instanceof Number) {
+                return JSONObject.isNumberSimilar((Number)valueThis, (Number)valueOther);
             } else if (!valueThis.equals(valueOther)) {
                 return false;
             }

src/main/java/org/json/JSONObject.java

@@ -1161,8 +1161,7 @@ static BigDecimal objectToBigDecimal(Object val, BigDecimal defaultValue) {
             return new BigDecimal((BigInteger) val);
         }
         if (val instanceof Double || val instanceof Float){
-            final double d = ((Number) val).doubleValue();
-            if(Double.isNaN(d)) {
+            if (!numberIsFinite((Number)val)) {
                 return defaultValue;
             }
             return new BigDecimal(((Number) val).doubleValue());
@@ -1212,11 +1211,10 @@ static BigInteger objectToBigInteger(Object val, BigInteger defaultValue) {
             return ((BigDecimal) val).toBigInteger();
         }
         if (val instanceof Double || val instanceof Float){
-            final double d = ((Number) val).doubleValue();
-            if(Double.isNaN(d)) {
+            if (!numberIsFinite((Number)val)) {
                 return defaultValue;
             }
-            return new BigDecimal(d).toBigInteger();
+            return new BigDecimal(((Number) val).doubleValue()).toBigInteger();
         }
         if (val instanceof Long || val instanceof Integer
                 || val instanceof Short || val instanceof Byte){
@@ -2073,6 +2071,8 @@ public boolean similar(Object other) {
                     if (!((JSONArray)valueThis).similar(valueOther)) {
                         return false;
                     }
+                } else if (valueThis instanceof Number && valueOther instanceof Number) {
+                    return isNumberSimilar((Number)valueThis, (Number)valueOther);
                 } else if (!valueThis.equals(valueOther)) {
                     return false;
                 }
@@ -2083,6 +2083,55 @@ public boolean similar(Object other) {
         }
     }
 
+    /**
+     * Compares two numbers to see if they are similar.
+     * 
+     * If either of the numbers are Double or Float instances, then they are checked to have
+     * a finite value. If either value is not finite (NaN or ±infinity), then this
+     * function will always return false. If both numbers are finite, they are first checked
+     * to be the same type and implement {@link Comparable}. If they do, then the actual
+     * {@link Comparable#compareTo(Object)} is called. If they are not the same type, or don't
+     * implement Comparable, then they are converted to {@link BigDecimal}s. Finally the 
+     * BigDecimal values are compared using {@link BigDecimal#compareTo(BigDecimal)}.
+     * 
+     * @param l the Left value to compare. Can not be <code>null</code>.
+     * @param r the right value to compare. Can not be <code>null</code>.
+     * @return true if the numbers are similar, false otherwise.
+     */
+    static boolean isNumberSimilar(Number l, Number r) {
+        if (!numberIsFinite(l) || !numberIsFinite(r)) {
+            // non-finite numbers are never similar
+            return false;
+        }
+
+        // if the classes are the same and implement Comparable
+        // then use the built in compare first.
+        if(l.getClass().equals(r.getClass()) && l instanceof Comparable) {
+            @SuppressWarnings({ "rawtypes", "unchecked" })
+            int compareTo = ((Comparable)l).compareTo(r);
+            return compareTo==0;
+        }
+
+        // BigDecimal should be able to handle all of our number types that we support through
+        // documentation. Convert to BigDecimal first, then use the Compare method to
+        // decide equality.
+        final BigDecimal lBigDecimal = objectToBigDecimal(l, null);
+        final BigDecimal rBigDecimal = objectToBigDecimal(r, null);
+        if (lBigDecimal == null || rBigDecimal == null) {
+            return false;
+        }
+        return lBigDecimal.compareTo(rBigDecimal) == 0;
+    }
+
+    private static boolean numberIsFinite(Number n) {
+        if (n instanceof Double && (((Double) n).isInfinite() || ((Double) n).isNaN())) {
+            return false;
+        } else if (n instanceof Float && (((Float) n).isInfinite() || ((Float) n).isNaN())) {
+            return false;
+        }
+        return true;
+    }
+
     /**
      * Tests if the value should be tried as a decimal. It makes no test if there are actual digits.
      * 
@@ -2216,18 +2265,8 @@ public static Object stringToValue(String string) {
      *             If o is a non-finite number.
      */
     public static void testValidity(Object o) throws JSONException {
-        if (o != null) {
-            if (o instanceof Double) {
-                if (((Double) o).isInfinite() || ((Double) o).isNaN()) {
-                    throw new JSONException(
-                            "JSON does not allow non-finite numbers.");
-                }
-            } else if (o instanceof Float) {
-                if (((Float) o).isInfinite() || ((Float) o).isNaN()) {
-                    throw new JSONException(
-                            "JSON does not allow non-finite numbers.");
-                }
-            }
+        if (o instanceof Number && !numberIsFinite((Number) o)) {
+            throw new JSONException("JSON does not allow non-finite numbers.");
         }
     }
 
@@ -2354,7 +2393,7 @@ public static String valueToString(Object value) throws JSONException {
      */
     public static Object wrap(Object object) {
         try {
-            if (object == null) {
+            if (NULL.equals(object)) {
                 return NULL;
             }
             if (object instanceof JSONObject || object instanceof JSONArray

src/test/java/org/json/junit/JSONObjectTest.java

@@ -115,10 +115,17 @@ public void verifySimilar() {
                 .put("key2", 2)
                 .put("key3", new String(string1));
 
-        assertFalse("Should eval to false", obj1.similar(obj2));
+        JSONObject obj4 = new JSONObject()
+                .put("key1", "abc")
+                .put("key2", 2.0)
+                .put("key3", new String(string1));
 
+        assertFalse("Should eval to false", obj1.similar(obj2));
+
         assertTrue("Should eval to true", obj1.similar(obj3));
 
+        assertTrue("Should eval to true", obj1.similar(obj4));
+
     }
 
     @Test