From 7b4338ccd5f4a09be02f54b18372bf4fc574e8e7 Mon Sep 17 00:00:00 2001 From: Kristian Kraljic Date: Tue, 5 Oct 2021 12:19:50 +0200 Subject: [PATCH] fix: make `ImmutableJsonArray/Object.equals` behave better Make `ImmutableJsonArray/Object.equals` behave like `ImmutableBuffer.equals`, so that it can compare to regular `JsonArrayObject`s. --- .../internal/buffer/ImmutableBuffer.java | 4 +- .../internal/json/ImmutableJsonArray.java | 39 ++++++++++++++----- .../internal/json/ImmutableJsonObject.java | 36 ++++++++++++----- .../internal/json/ImmutableJsonArrayTest.java | 29 +++++++++++++- .../json/ImmutableJsonObjectTest.java | 30 +++++++++++++- 5 files changed, 115 insertions(+), 23 deletions(-) diff --git a/src/main/java/io/neonbee/internal/buffer/ImmutableBuffer.java b/src/main/java/io/neonbee/internal/buffer/ImmutableBuffer.java index 02cdc01a..5ecbe267 100644 --- a/src/main/java/io/neonbee/internal/buffer/ImmutableBuffer.java +++ b/src/main/java/io/neonbee/internal/buffer/ImmutableBuffer.java @@ -523,8 +523,8 @@ public ImmutableBuffer copy() { // implements equal, it is impossible to fulfill this property. our aim is that buffers with the same content equal // each other, regardless of it's content, so buffer.equal(immutableBuffer) will never be true, while // immutableBuffer.equal(buffer) and immutableBuffer.equal(immutableBuffer) will, as long as the content is equal - public boolean equals(Object object) { - return buffer.equals(object instanceof ImmutableBuffer ? ((ImmutableBuffer) object).buffer : object); + public boolean equals(Object other) { + return buffer.equals(other instanceof ImmutableBuffer ? ((ImmutableBuffer) other).buffer : other); } @Override diff --git a/src/main/java/io/neonbee/internal/json/ImmutableJsonArray.java b/src/main/java/io/neonbee/internal/json/ImmutableJsonArray.java index c417d9b4..807df9ff 100644 --- a/src/main/java/io/neonbee/internal/json/ImmutableJsonArray.java +++ b/src/main/java/io/neonbee/internal/json/ImmutableJsonArray.java @@ -15,6 +15,16 @@ public final class ImmutableJsonArray extends JsonArray { */ public static final ImmutableJsonArray EMPTY = new ImmutableJsonArray(); + // store the source array to be able to compare it + private final JsonArray array; + + /** + * Create an empty instance. + */ + public ImmutableJsonArray() { + this(emptyList()); + } + /** * Create a new instance with the given string. * @@ -24,21 +34,14 @@ public ImmutableJsonArray(String json) { this(new JsonArray(json)); } - /** - * Create an empty instance. - */ - public ImmutableJsonArray() { - this(emptyList()); - } - /** * Create an instance from a List. The List is not copied. * * @param list The list to be converted to an {@link ImmutableJsonArray} */ - @SuppressWarnings({ "unchecked", "rawtypes" }) + @SuppressWarnings("rawtypes") public ImmutableJsonArray(List list) { - super(Collections.unmodifiableList(list)); + this(new JsonArray(list)); } /** @@ -55,8 +58,10 @@ public ImmutableJsonArray(Buffer buf) { * * @param arr the existing JSON array */ + @SuppressWarnings("unchecked") public ImmutableJsonArray(JsonArray arr) { - this(arr.getList()); + super(Collections.unmodifiableList(arr.getList())); + this.array = arr instanceof ImmutableJsonArray ? ((ImmutableJsonArray) arr).array : arr; } @Override @@ -85,6 +90,20 @@ public ImmutableJsonArray copy() { return this; } + @Override + // this method violates the symmetric property of how equal should be implement, because as of how JsonArray + // implements equal, it is impossible to fulfill this property. our aim is that arrays with the same content equal + // each other, so array.equal(immutableArray) will never be true, while immutableArray.equal(array) and + // immutableArray.equal(immutableArray) will, as long as the content is equal + public boolean equals(Object other) { + return array.equals(other instanceof ImmutableJsonArray ? ((ImmutableJsonArray) other).array : other); + } + + @Override + public int hashCode() { + return array.hashCode(); + } + /** * Creates a mutable copy of this array. * diff --git a/src/main/java/io/neonbee/internal/json/ImmutableJsonObject.java b/src/main/java/io/neonbee/internal/json/ImmutableJsonObject.java index 2083ec44..780122ec 100644 --- a/src/main/java/io/neonbee/internal/json/ImmutableJsonObject.java +++ b/src/main/java/io/neonbee/internal/json/ImmutableJsonObject.java @@ -18,6 +18,16 @@ public final class ImmutableJsonObject extends JsonObject { */ public static final ImmutableJsonObject EMPTY = new ImmutableJsonObject(); + // store the source object to be able to compare it + private final JsonObject object; + + /** + * Create a new, empty instance. + */ + public ImmutableJsonObject() { + this(emptyMap()); + } + /** * Create an instance from a string of JSON. * @@ -27,20 +37,13 @@ public ImmutableJsonObject(String json) { this(new JsonObject(json)); } - /** - * Create a new, empty instance. - */ - public ImmutableJsonObject() { - this(emptyMap()); - } - /** * Create an instance from a Map. The Map is not copied. * * @param map the map to create the instance from. */ public ImmutableJsonObject(Map map) { - super(Collections.unmodifiableMap(map)); + this(new JsonObject(map)); } /** @@ -58,7 +61,8 @@ public ImmutableJsonObject(Buffer buf) { * @param obj the existing JSON object */ public ImmutableJsonObject(JsonObject obj) { - this(obj.getMap()); + super(Collections.unmodifiableMap(obj.getMap())); + this.object = obj instanceof ImmutableJsonObject ? ((ImmutableJsonObject) obj).object : obj; } @Override @@ -108,6 +112,20 @@ public ImmutableJsonObject copy() { return this; } + @Override + // this method violates the symmetric property of how equal should be implement, because as of how JsonObject + // implements equal, it is impossible to fulfill this property. our aim is that object with the same content equal + // each other, so object.equal(immutableObject) will never be true, while immutableObject.equal(object) and + // immutableObject.equal(immutableObject) will, as long as the content is equal + public boolean equals(Object other) { + return object.equals(other instanceof ImmutableJsonObject ? ((ImmutableJsonObject) other).object : other); + } + + @Override + public int hashCode() { + return object.hashCode(); + } + /** * Creates a mutable copy of this object. * diff --git a/src/test/java/io/neonbee/internal/json/ImmutableJsonArrayTest.java b/src/test/java/io/neonbee/internal/json/ImmutableJsonArrayTest.java index fbcc0509..872bf35c 100644 --- a/src/test/java/io/neonbee/internal/json/ImmutableJsonArrayTest.java +++ b/src/test/java/io/neonbee/internal/json/ImmutableJsonArrayTest.java @@ -1,6 +1,7 @@ package io.neonbee.internal.json; import static com.google.common.truth.Truth.assertThat; +import static io.neonbee.internal.json.ImmutableJsonArray.EMPTY; import static java.util.Collections.emptyList; import static java.util.Collections.unmodifiableList; import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; @@ -20,6 +21,13 @@ void testImmutableClass() { assertThat(new ImmutableJsonArray().getList().getClass()).isEqualTo(unmodifiableList(emptyList()).getClass()); } + @Test + void testEmpty() { + assertThat(EMPTY).isEmpty(); + assertThat(EMPTY.getList()).isEmpty(); + assertThat(new ImmutableJsonArray()).isEqualTo(EMPTY); + } + @SuppressWarnings("unchecked") @Test void testImmutable() { @@ -91,6 +99,25 @@ void testMutableCopyIsMutable() { @Test void testCopyIsAlsoNotMutable() { - assertThrows(UnsupportedOperationException.class, () -> new ImmutableJsonArray().copy().add(true)); + ImmutableJsonArray array = new ImmutableJsonArray(); + assertThrows(UnsupportedOperationException.class, () -> array.copy().add(true)); + assertThat(array.copy()).isSameInstanceAs(array); + } + + @Test + void testStandardMethods() { + JsonArray array = new JsonArray().add(1).add("foo").add(new JsonObject()); + ImmutableJsonArray immutableArray = new ImmutableJsonArray(array); + + assertThat(immutableArray.toString()).isEqualTo(array.toString()); + assertThat(immutableArray.hashCode()).isEqualTo(array.hashCode()); + + assertThat(immutableArray).isEqualTo(array); + assertThat(immutableArray).isEqualTo(new ImmutableJsonArray(array)); + assertThat(immutableArray).isEqualTo(new ImmutableJsonArray(immutableArray)); + assertThat(immutableArray).isEqualTo(new JsonArray().add(1).add("foo").add(new JsonObject())); + assertThat(immutableArray) + .isEqualTo(new ImmutableJsonArray(new JsonArray().add(1).add("foo").add(new JsonObject()))); + assertThat(immutableArray).isNotEqualTo(EMPTY); } } diff --git a/src/test/java/io/neonbee/internal/json/ImmutableJsonObjectTest.java b/src/test/java/io/neonbee/internal/json/ImmutableJsonObjectTest.java index 5043ed93..9c1b0d39 100644 --- a/src/test/java/io/neonbee/internal/json/ImmutableJsonObjectTest.java +++ b/src/test/java/io/neonbee/internal/json/ImmutableJsonObjectTest.java @@ -1,6 +1,7 @@ package io.neonbee.internal.json; import static com.google.common.truth.Truth.assertThat; +import static io.neonbee.internal.json.ImmutableJsonObject.EMPTY; import static java.util.Collections.emptyMap; import static java.util.Collections.unmodifiableMap; import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; @@ -20,6 +21,13 @@ void testImmutableClass() { assertThat(new ImmutableJsonObject().getMap().getClass()).isEqualTo(unmodifiableMap(emptyMap()).getClass()); } + @Test + void testEmpty() { + assertThat(EMPTY).isEmpty(); + assertThat(EMPTY.getMap()).isEmpty(); + assertThat(new ImmutableJsonObject()).isEqualTo(EMPTY); + } + @Test void testImmutable() { ImmutableJsonObject jsonObject = new ImmutableJsonObject(); @@ -95,6 +103,26 @@ void testMutableCopyIsMutable() { @Test void testCopyIsAlsoNotMutable() { - assertThrows(UnsupportedOperationException.class, () -> new ImmutableJsonObject().copy().put("keyY", true)); + ImmutableJsonObject object = new ImmutableJsonObject(); + assertThrows(UnsupportedOperationException.class, () -> object.copy().put("keyY", true)); + assertThat(object.copy()).isSameInstanceAs(object); + } + + @Test + void testStandardMethods() { + JsonObject array = new JsonObject().put("foo", 1).put("bar", "baz").put("bam", new JsonObject()); + ImmutableJsonObject immutableArray = new ImmutableJsonObject(array); + + assertThat(immutableArray.toString()).isEqualTo(array.toString()); + assertThat(immutableArray.hashCode()).isEqualTo(array.hashCode()); + + assertThat(immutableArray).isEqualTo(array); + assertThat(immutableArray).isEqualTo(new ImmutableJsonObject(array)); + assertThat(immutableArray).isEqualTo(new ImmutableJsonObject(immutableArray)); + assertThat(immutableArray) + .isEqualTo(new JsonObject().put("foo", 1).put("bar", "baz").put("bam", new JsonObject())); + assertThat(immutableArray).isEqualTo( + new ImmutableJsonObject(new JsonObject().put("foo", 1).put("bar", "baz").put("bam", new JsonObject()))); + assertThat(immutableArray).isNotEqualTo(EMPTY); } }