Skip to content

Commit 8c639de

Browse files
vietjvietj
committed
Provide a coherent implementation of hashCode for JsonObject/JsonArray with respect to equals.
Motivation: JsonArray/JsonObject hashCode implementations rely on the wrapped data structure hashCode implementation and on the generic Object hashCode function. 1. The hash value might differ from data structure implementations. 2. Number coercion is performed in equals which allows case were we do have equality (after coercion) but hash differs (e.g. 4L and 4D) Changes: JsonObject/JsonArray implement hashCode methods. Number's hashCode is computed against the hashCode of their double value.
1 parent 66917da commit 8c639de

File tree

4 files changed

+136
-103
lines changed

4 files changed

+136
-103
lines changed

‎src/main/java/io/vertx/core/json/JsonArray.java‎

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
package io.vertx.core.json;
1313

1414
import io.vertx.core.buffer.Buffer;
15+
import io.vertx.core.json.impl.JsonUtil;
1516
import io.vertx.core.shareddata.ClusterSerializable;
1617
import io.vertx.core.shareddata.Shareable;
1718

@@ -20,7 +21,7 @@
2021
import java.util.function.Function;
2122
import java.util.stream.Stream;
2223

23-
import static io.vertx.core.json.JsonObject.compareObjects;
24+
import static io.vertx.core.json.impl.JsonUtil.compare;
2425
import static io.vertx.core.json.impl.JsonUtil.*;
2526
import static java.time.format.DateTimeFormatter.ISO_INSTANT;
2627

@@ -661,24 +662,29 @@ public String toString() {
661662
@Override
662663
public boolean equals(Object o) {
663664
// null check
664-
if (o == null)
665+
if (o == null) {
665666
return false;
667+
}
666668
// self check
667-
if (this == o)
669+
if (this == o) {
668670
return true;
671+
}
669672
// type check and cast
670-
if (getClass() != o.getClass())
673+
if (getClass() != o.getClass()) {
671674
return false;
675+
}
672676

673677
JsonArray other = (JsonArray) o;
674678
// size check
675-
if (this.size() != other.size())
679+
int size = this.size();
680+
if (size != other.size()) {
676681
return false;
682+
}
677683
// value comparison
678-
for (int i = 0; i < this.size(); i++) {
684+
for (int i = 0; i < size; i++) {
679685
Object thisValue = this.getValue(i);
680686
Object otherValue = other.getValue(i);
681-
if (thisValue != otherValue && !compareObjects(thisValue, otherValue)) {
687+
if (thisValue != otherValue && !compare(thisValue, otherValue)) {
682688
return false;
683689
}
684690
}
@@ -688,7 +694,11 @@ public boolean equals(Object o) {
688694

689695
@Override
690696
public int hashCode() {
691-
return list.hashCode();
697+
int h = 0;
698+
for (Object value : this) {
699+
h += JsonUtil.hashCode(value);
700+
}
701+
return h;
692702
}
693703

694704
@Override

‎src/main/java/io/vertx/core/json/JsonObject.java‎

Lines changed: 17 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
package io.vertx.core.json;
1212

1313
import io.vertx.core.buffer.Buffer;
14+
import io.vertx.core.json.impl.JsonUtil;
1415
import io.vertx.core.shareddata.ClusterSerializable;
1516
import io.vertx.core.shareddata.Shareable;
1617

@@ -1162,19 +1163,23 @@ public String toString() {
11621163
@Override
11631164
public boolean equals(Object o) {
11641165
// null check
1165-
if (o == null)
1166+
if (o == null) {
11661167
return false;
1168+
}
11671169
// self check
1168-
if (this == o)
1170+
if (this == o) {
11691171
return true;
1172+
}
11701173
// type check and cast
1171-
if (getClass() != o.getClass())
1174+
if (getClass() != o.getClass()) {
11721175
return false;
1176+
}
11731177

11741178
JsonObject other = (JsonObject) o;
11751179
// size check
1176-
if (this.size() != other.size())
1180+
if (this.size() != other.size()) {
11771181
return false;
1182+
}
11781183
// value comparison
11791184
for (String key : map.keySet()) {
11801185
if (!other.containsKey(key)) {
@@ -1183,63 +1188,23 @@ public boolean equals(Object o) {
11831188

11841189
Object thisValue = this.getValue(key);
11851190
Object otherValue = other.getValue(key);
1186-
if (thisValue != otherValue && !compareObjects(thisValue, otherValue)) {
1191+
if (thisValue != otherValue && !compare(thisValue, otherValue)) {
11871192
return false;
11881193
}
11891194
}
11901195
// all checks passed
11911196
return true;
11921197
}
11931198

1194-
static boolean compareObjects(Object o1, Object o2) {
1195-
if (o1 instanceof Number && o2 instanceof Number) {
1196-
if (o1.getClass() == o2.getClass()) {
1197-
return o1.equals(o2);
1198-
} else {
1199-
// meaning that the numbers are different types
1200-
Number n1 = (Number) o1;
1201-
Number n2 = (Number) o2;
1202-
return compareNumbers(n1, n2);
1203-
}
1204-
} else if (o1 instanceof CharSequence && o2 instanceof CharSequence && o1.getClass() != o2.getClass()) {
1205-
return Objects.equals(o1.toString(), o2.toString());
1206-
} else {
1207-
return Objects.equals(o1, o2);
1208-
}
1209-
}
1210-
1211-
private static boolean compareNumbers(Number n1, Number n2) {
1212-
if (isDecimalNumber(n1) && isDecimalNumber(n2)) {
1213-
// compare as floating point double
1214-
return n1.doubleValue() == n2.doubleValue();
1215-
} else if (isWholeNumber(n1) && isWholeNumber(n2)) {
1216-
// compare as integer long
1217-
return n1.longValue() == n2.longValue();
1218-
} else if (isWholeNumber(n1) && isDecimalNumber(n2) ||
1219-
isDecimalNumber(n1) && isWholeNumber(n2)) {
1220-
// if its either integer or long and the other is float or double or vice versa,
1221-
// compare as floating point double
1222-
return n1.doubleValue() == n2.doubleValue();
1223-
} else {
1224-
if (isWholeNumber(n1)) {
1225-
return n1.longValue() == n2.longValue();
1226-
} else {
1227-
return n1.doubleValue() == n2.doubleValue();
1228-
}
1229-
}
1230-
}
1231-
1232-
private static boolean isWholeNumber(Number thisValue) {
1233-
return thisValue instanceof Integer || thisValue instanceof Long;
1234-
}
1235-
1236-
private static boolean isDecimalNumber(Number thisValue) {
1237-
return thisValue instanceof Float || thisValue instanceof Double;
1238-
}
1239-
12401199
@Override
12411200
public int hashCode() {
1242-
return map.hashCode();
1201+
int h = 0;
1202+
for (Map.Entry<String, ?> entry : this) {
1203+
Object key = entry.getKey();
1204+
Object value = entry.getValue();
1205+
h += (key.hashCode() ^ JsonUtil.hashCode(value));
1206+
}
1207+
return h;
12431208
}
12441209

12451210
@Override

‎src/main/java/io/vertx/core/json/impl/JsonUtil.java‎

Lines changed: 57 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,7 @@
1616
import io.vertx.core.shareddata.Shareable;
1717

1818
import java.time.Instant;
19-
import java.util.Base64;
20-
import java.util.Iterator;
21-
import java.util.List;
22-
import java.util.Map;
19+
import java.util.*;
2320
import java.util.function.Function;
2421
import java.util.stream.Stream;
2522
import java.util.stream.StreamSupport;
@@ -128,4 +125,60 @@ public static <T> Stream<T> asStream(Iterator<T> sourceIterator) {
128125
Iterable<T> iterable = () -> sourceIterator;
129126
return StreamSupport.stream(iterable.spliterator(), false);
130127
}
128+
129+
public static int hashCode(Object value) {
130+
if (value == null) {
131+
return 0;
132+
} else if (value instanceof Number) {
133+
return Double.hashCode(((Number) value).doubleValue());
134+
} else {
135+
return value.hashCode();
136+
}
137+
}
138+
139+
public static boolean compare(Object o1, Object o2) {
140+
if (o1 instanceof Number && o2 instanceof Number) {
141+
if (o1.getClass() == o2.getClass()) {
142+
return o1.equals(o2);
143+
} else {
144+
// meaning that the numbers are different types
145+
Number n1 = (Number) o1;
146+
Number n2 = (Number) o2;
147+
return compareNumbers(n1, n2);
148+
}
149+
} else if (o1 instanceof CharSequence && o2 instanceof CharSequence && o1.getClass() != o2.getClass()) {
150+
return Objects.equals(o1.toString(), o2.toString());
151+
} else {
152+
return Objects.equals(o1, o2);
153+
}
154+
}
155+
156+
private static boolean compareNumbers(Number n1, Number n2) {
157+
if (isDecimalNumber(n1) && isDecimalNumber(n2)) {
158+
// compare as floating point double
159+
return n1.doubleValue() == n2.doubleValue();
160+
} else if (isWholeNumber(n1) && isWholeNumber(n2)) {
161+
// compare as integer long
162+
return n1.longValue() == n2.longValue();
163+
} else if (isWholeNumber(n1) && isDecimalNumber(n2) ||
164+
isDecimalNumber(n1) && isWholeNumber(n2)) {
165+
// if its either integer or long and the other is float or double or vice versa,
166+
// compare as floating point double
167+
return n1.doubleValue() == n2.doubleValue();
168+
} else {
169+
if (isWholeNumber(n1)) {
170+
return n1.longValue() == n2.longValue();
171+
} else {
172+
return n1.doubleValue() == n2.doubleValue();
173+
}
174+
}
175+
}
176+
177+
private static boolean isWholeNumber(Number thisValue) {
178+
return thisValue instanceof Integer || thisValue instanceof Long;
179+
}
180+
181+
private static boolean isDecimalNumber(Number thisValue) {
182+
return thisValue instanceof Float || thisValue instanceof Double;
183+
}
131184
}

‎src/test/java/io/vertx/core/json/JsonObjectTest.java‎

Lines changed: 44 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -1548,22 +1548,22 @@ public void testClusterSerializable() {
15481548

15491549
@Test
15501550
public void testNumberEquality() {
1551-
assertNumberEquals(4, 4);
1552-
assertNumberEquals(4, (long)4);
1553-
assertNumberEquals(4, 4f);
1554-
assertNumberEquals(4, 4D);
1555-
assertNumberEquals((long)4, (long)4);
1556-
assertNumberEquals((long)4, 4f);
1557-
assertNumberEquals((long)4, 4D);
1558-
assertNumberEquals(4f, 4f);
1559-
assertNumberEquals(4f, 4D);
1560-
assertNumberEquals(4D, 4D);
1561-
assertNumberEquals(4.1D, 4.1D);
1562-
assertNumberEquals(4.1f, 4.1f);
1551+
assertNumberEqualsAndHashCode(4, 4);
1552+
assertNumberEqualsAndHashCode(4, (long)4);
1553+
assertNumberEqualsAndHashCode(4, 4f);
1554+
assertNumberEqualsAndHashCode(4, 4D);
1555+
assertNumberEqualsAndHashCode((long)4, (long)4);
1556+
assertNumberEqualsAndHashCode((long)4, 4f);
1557+
assertNumberEqualsAndHashCode((long)4, 4D);
1558+
assertNumberEqualsAndHashCode(4f, 4f);
1559+
assertNumberEqualsAndHashCode(4f, 4D);
1560+
assertNumberEqualsAndHashCode(4D, 4D);
1561+
assertNumberEqualsAndHashCode(4.1D, 4.1D);
1562+
assertNumberEqualsAndHashCode(4.1f, 4.1f);
15631563
assertNumberNotEquals(4.1f, 4.1D);
1564-
assertNumberEquals(4.5D, 4.5D);
1565-
assertNumberEquals(4.5f, 4.5f);
1566-
assertNumberEquals(4.5f, 4.5D);
1564+
assertNumberEqualsAndHashCode(4.5D, 4.5D);
1565+
assertNumberEqualsAndHashCode(4.5f, 4.5f);
1566+
assertNumberEqualsAndHashCode(4.5f, 4.5D);
15671567
assertNumberNotEquals(4, 5);
15681568
assertNumberNotEquals(4, (long)5);
15691569
assertNumberNotEquals(4, 5D);
@@ -1574,34 +1574,39 @@ public void testNumberEquality() {
15741574
assertNumberNotEquals(4f, 5f);
15751575
assertNumberNotEquals(4f, 5D);
15761576
assertNumberNotEquals(4D, 5D);
1577-
assertNumberEquals(2f, 2);
1578-
assertNumberEquals(2D, 2);
1577+
assertNumberEqualsAndHashCode(2f, 2);
1578+
assertNumberEqualsAndHashCode(2D, 2);
15791579
assertNumberNotEquals(2.3D, 2);
15801580
assertNumberNotEquals(2.3f, 2);
1581-
assertNumberEquals(2, new BigDecimal(2));
1582-
assertNumberEquals(new BigDecimal(2), 2);
1583-
assertNumberEquals(2D, new BigDecimal(2));
1584-
assertNumberEquals(new BigDecimal(2), 2D);
1585-
assertNumberEquals(2, BigInteger.valueOf(2));
1586-
assertNumberEquals(BigInteger.valueOf(2), 2);
1587-
assertNumberEquals(2D, BigInteger.valueOf(2));
1588-
assertNumberEquals(BigInteger.valueOf(2), 2D);
1589-
assertNumberEquals(BigInteger.valueOf(2), new BigDecimal(2));
1590-
assertNumberEquals(new BigDecimal(2), BigInteger.valueOf(2));
1591-
}
1592-
1593-
private void assertNumberEquals(Number value1, Number value2) {
1594-
JsonObject o1 = new JsonObject().put("key", value1);
1595-
JsonObject o2 = new JsonObject().put("key", value2);
1581+
assertNumberEqualsAndHashCode(2, new BigDecimal(2));
1582+
assertNumberEqualsAndHashCode(new BigDecimal(2), 2);
1583+
assertNumberEqualsAndHashCode(2D, new BigDecimal(2));
1584+
assertNumberEqualsAndHashCode(new BigDecimal(2), 2D);
1585+
assertNumberEqualsAndHashCode(2, BigInteger.valueOf(2));
1586+
assertNumberEqualsAndHashCode(BigInteger.valueOf(2), 2);
1587+
assertNumberEqualsAndHashCode(2D, BigInteger.valueOf(2));
1588+
assertNumberEqualsAndHashCode(BigInteger.valueOf(2), 2D);
1589+
assertNumberEqualsAndHashCode(BigInteger.valueOf(2), new BigDecimal(2));
1590+
assertNumberEqualsAndHashCode(new BigDecimal(2), BigInteger.valueOf(2));
1591+
}
1592+
1593+
private void assertNumberEqualsAndHashCode(Number value1, Number value2) {
1594+
assertNumberEqualsAndHashCode(value1, value2, n -> new JsonObject().put("key", n));
1595+
assertNumberEqualsAndHashCode(value1, value2, n -> new JsonObject().put("key", Collections.singletonMap("key", n)));
1596+
assertNumberEqualsAndHashCode(value1, value2, n -> new JsonArray().add(n));
1597+
assertNumberEqualsAndHashCode(value1, value2, n -> new JsonArray().add(Collections.singletonList(n)));
1598+
}
1599+
1600+
private void assertNumberEqualsAndHashCode(Number value1, Number value2, Function<Number, Object> f) {
1601+
Object o1 = f.apply(value1);
1602+
Object o2 = f.apply(value2);
15961603
if (!o1.equals(o2)) {
15971604
fail("Was expecting " + value1.getClass().getSimpleName() + ":" + value1 + " == " +
1598-
value2.getClass().getSimpleName() + ":" + value2);
1605+
value2.getClass().getSimpleName() + ":" + value2);
15991606
}
1600-
JsonArray a1 = new JsonArray().add(value1);
1601-
JsonArray a2 = new JsonArray().add(value2);
1602-
if (!a1.equals(a2)) {
1603-
fail("Was expecting " + value1.getClass().getSimpleName() + ":" + value1 + " == " +
1604-
value2.getClass().getSimpleName() + ":" + value2);
1607+
if (o1.hashCode() != o2.hashCode()) {
1608+
fail("Was expecting " + value1.getClass().getSimpleName() + ":" + value1 + " and " +
1609+
value2.getClass().getSimpleName() + ":" + value2);
16051610
}
16061611
}
16071612

@@ -1905,7 +1910,7 @@ public void testNumberDefaults() {
19051910
1234567890123456789L,
19061911
Short.MAX_VALUE
19071912
}) {
1908-
assertNumberEquals(n, numbers.getNumber("missingKey", n));
1913+
assertNumberEqualsAndHashCode(n, numbers.getNumber("missingKey", n));
19091914
}
19101915
}
19111916

0 commit comments

Comments
 (0)