Skip to content

Commit

Permalink
fix-JsonObject#equals-method-fails-when-comparing-integers-with-floats
Browse files Browse the repository at this point in the history
Context:
The current implementation of JsonObject#equals fails to equate numeric types correctly, leading to false comparisons:
1. `2f` is not considered equal to `2`.
2. `2D` is not considered equal to `2`.

Changes:
This PR updates JsonObject#equals to correctly handle comparisons between different numeric types (float, double, intege, long), ensuring logical equivalence where expected.
Additionally, new assertions have been added to JsonObjectTest#testNumberEquality to validate these changes.

This PR tries to fix exactly the same by modifying the JsonObject#equals.
Adds new assertions to JsonObjectTest#testNumberEquality.
  • Loading branch information
sai-manohar-veerubhotla authored and vietj committed Apr 29, 2024
1 parent 5991c28 commit 2b49feb
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 23 deletions.
71 changes: 48 additions & 23 deletions src/main/java/io/vertx/core/json/JsonObject.java
Original file line number Diff line number Diff line change
Expand Up @@ -584,7 +584,7 @@ public JsonArray getJsonArray(String key) {

/**
* Get the binary value with the specified key.
*
* <p>
* JSON itself has no notion of a binary, this extension complies to the RFC-7493, so this method assumes there is a
* String value with the key and it contains a Base64 encoded binary, which it decodes if found and returns.
*
Expand Down Expand Up @@ -616,7 +616,7 @@ public byte[] getBinary(String key) {

/**
* Get the {@code Buffer} value with the specified key.
*
* <p>
* JSON itself has no notion of a binary, this extension complies to the RFC-7493, so this method assumes there is a
* String value with the key and it contains a Base64 encoded binary, which it decodes if found and returns.
*
Expand Down Expand Up @@ -650,7 +650,7 @@ public Buffer getBuffer(String key) {

/**
* Get the instant value with the specified key.
*
* <p>
* JSON itself has no notion of a temporal types, this extension allows ISO 8601 string formatted dates with timezone
* always set to zero UTC offset, as denoted by the suffix "Z" to be parsed as a instant value.
* {@code YYYY-MM-DDTHH:mm:ss.sssZ} is the default format used by web browser scripting. This extension complies to
Expand Down Expand Up @@ -927,8 +927,8 @@ public Set<String> fieldNames() {
/**
* Put a null value into the JSON object with the specified key.
*
* @param key the key
* @return a reference to this, so the API can be used fluently
* @param key the key
* @return a reference to this, so the API can be used fluently
*/
public JsonObject putNull(String key) {
Objects.requireNonNull(key);
Expand Down Expand Up @@ -962,7 +962,7 @@ public Object remove(String key) {

/**
* Merge in another JSON object.
*
* <p>
* This is the equivalent of putting all the entries of the other JSON object into this object. This is not a deep
* merge, entries containing (sub) JSON objects will be replaced entirely.
*
Expand Down Expand Up @@ -1089,7 +1089,7 @@ public JsonObject copy(Function<Object, ?> cloner) {

/**
* Get the underlying {@code Map} as is.
*
* <p>
* This map may contain values that are not the types returned by the {@code JsonObject} and
* with an unpredictable representation of the value, e.g you might get a JSON object
* as a {@link JsonObject} or as a {@link Map}.
Expand All @@ -1103,7 +1103,7 @@ public Map<String, Object> getMap() {
/**
* Get a Stream over the entries in the JSON object. The values in the stream will follow
* the same rules as defined in {@link #getValue(String)}, respecting the JSON requirements.
*
* <p>
* To stream the raw values, use the storage object stream instead:
* <pre>{@code
* jsonObject
Expand Down Expand Up @@ -1187,32 +1187,57 @@ public boolean equals(Object o) {
continue;
}
// special case for numbers
if (thisValue instanceof Number && otherValue instanceof Number && thisValue.getClass() != otherValue.getClass()) {
Number n1 = (Number) thisValue;
Number n2 = (Number) otherValue;
// floating point values
if (thisValue instanceof Float || thisValue instanceof Double || otherValue instanceof Float || otherValue instanceof Double) {
// compare as floating point double
if (n1.doubleValue() == n2.doubleValue()) {
// same value check the next entry

if (thisValue instanceof Number && otherValue instanceof Number) {
if (thisValue.getClass() == otherValue.getClass()) {
if (thisValue.equals(otherValue)) {
continue;
}
}
if (thisValue instanceof Integer || thisValue instanceof Long || otherValue instanceof Integer || otherValue instanceof Long) {
// compare as integer long
if (n1.longValue() == n2.longValue()) {
// same value check the next entry
continue;
} else {
// meaning that the numbers are different types
Number n1 = (Number) thisValue;
Number n2 = (Number) otherValue;
if ((thisValue instanceof Float || thisValue instanceof Double) &&
(otherValue instanceof Float || otherValue instanceof Double)) {
// compare as floating point double
if (n1.doubleValue() == n2.doubleValue()) {
// same value, check the next entry
continue;
}
}

if ((thisValue instanceof Integer || thisValue instanceof Long) &&
(otherValue instanceof Integer || otherValue instanceof Long)) {
// compare as integer long
if (n1.longValue() == n2.longValue()) {
// same value, check the next entry
continue;
}
}


// if its either integer or long and the other is float or double or vice versa,
// compare as floating point double
if ((thisValue instanceof Integer || thisValue instanceof Long) &&
(otherValue instanceof Float || otherValue instanceof Double) ||
(thisValue instanceof Float || thisValue instanceof Double) &&
(otherValue instanceof Integer || otherValue instanceof Long)) {
// compare as floating point double
if (n1.doubleValue() == n2.doubleValue()) {
// same value, check the next entry
continue;
}
}
}
}

// special case for char sequences
if (thisValue instanceof CharSequence && otherValue instanceof CharSequence && thisValue.getClass() != otherValue.getClass()) {
CharSequence s1 = (CharSequence) thisValue;
CharSequence s2 = (CharSequence) otherValue;

if (Objects.equals(s1.toString(), s2.toString())) {
// same value check the next entry
// same value, check the next entry
continue;
}
}
Expand Down
5 changes: 5 additions & 0 deletions src/test/java/io/vertx/core/json/JsonObjectTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -1574,6 +1574,11 @@ public void testNumberEquality() {
assertNumberNotEquals(4f, 5f);
assertNumberNotEquals(4f, 5D);
assertNumberNotEquals(4D, 5D);
assertNumberEquals(2f, 2);
assertNumberEquals(2D, 2);
assertNumberNotEquals(2.3D, 2);
assertNumberNotEquals(2.3f, 2);

}

private void assertNumberEquals(Number value1, Number value2) {
Expand Down

0 comments on commit 2b49feb

Please sign in to comment.