From 1bc30975a08ae2e7a5fbdccf2777273be860136a Mon Sep 17 00:00:00 2001 From: Paulo Lopes Date: Tue, 10 Dec 2019 08:35:52 +0100 Subject: [PATCH 1/3] Relax getString() to perform autocast --- src/main/java/io/vertx/core/json/JsonArray.java | 11 ++++------- src/main/java/io/vertx/core/json/JsonObject.java | 13 ++++--------- src/test/java/io/vertx/core/json/JsonArrayTest.java | 4 ++-- .../java/io/vertx/core/json/JsonObjectTest.java | 10 +++++----- 4 files changed, 15 insertions(+), 23 deletions(-) diff --git a/src/main/java/io/vertx/core/json/JsonArray.java b/src/main/java/io/vertx/core/json/JsonArray.java index 90c021bfd..0dfb93e3a 100644 --- a/src/main/java/io/vertx/core/json/JsonArray.java +++ b/src/main/java/io/vertx/core/json/JsonArray.java @@ -95,8 +95,7 @@ public class JsonArray implements Iterable, ClusterSerializable, Shareab * Get the String at position {@code pos} in the array, * * @param pos the position in the array - * @return the String, or null if a null value present - * @throws java.lang.ClassCastException if the value cannot be converted to String + * @return the String (or String representation), or null if a null value present */ public String getString(int pos) { Object val = list.get(pos); @@ -105,17 +104,15 @@ public class JsonArray implements Iterable, ClusterSerializable, Shareab return null; } - if (val instanceof CharSequence) { - return val.toString(); - } else if (val instanceof Instant) { + if (val instanceof Instant) { return ISO_INSTANT.format((Instant) val); } else if (val instanceof byte[]) { return BASE64_ENCODER.encodeToString((byte[]) val); } else if (val instanceof Enum) { return ((Enum) val).name(); + } else { + return val.toString(); } - - throw new ClassCastException("class " + val.getClass().getName() + " cannot be cast to class java.lang.String"); } /** diff --git a/src/main/java/io/vertx/core/json/JsonObject.java b/src/main/java/io/vertx/core/json/JsonObject.java index 193df81cc..1f3c5ff5d 100644 --- a/src/main/java/io/vertx/core/json/JsonObject.java +++ b/src/main/java/io/vertx/core/json/JsonObject.java @@ -10,7 +10,6 @@ */ package io.vertx.core.json; -import io.vertx.codegen.annotations.Fluent; import io.vertx.core.buffer.Buffer; import io.vertx.core.shareddata.Shareable; import io.vertx.core.shareddata.impl.ClusterSerializable; @@ -124,8 +123,7 @@ public class JsonObject implements Iterable>, ClusterS * {@code byte[]} and {@code Enum} which can be converted to String. * * @param key the key to return the value for - * @return the value or null if no value for that key - * @throws java.lang.ClassCastException if the value is not a String + * @return the value string representation or null if no value for that key */ public String getString(String key) { Objects.requireNonNull(key); @@ -134,17 +132,15 @@ public class JsonObject implements Iterable>, ClusterS return null; } - if (val instanceof CharSequence) { - return val.toString(); - } else if (val instanceof Instant) { + if (val instanceof Instant) { return ISO_INSTANT.format((Instant) val); } else if (val instanceof byte[]) { return BASE64_ENCODER.encodeToString((byte[]) val); } else if (val instanceof Enum) { return ((Enum) val).name(); + } else { + return val.toString(); } - - throw new ClassCastException("class " + val.getClass().getName() + " cannot be cast to class java.lang.String"); } /** @@ -729,7 +725,6 @@ public class JsonObject implements Iterable>, ClusterS /** * Remove all the entries in this JSON object */ - @Fluent public JsonObject clear() { map.clear(); return this; diff --git a/src/test/java/io/vertx/core/json/JsonArrayTest.java b/src/test/java/io/vertx/core/json/JsonArrayTest.java index 0751813e0..934b95d45 100644 --- a/src/test/java/io/vertx/core/json/JsonArrayTest.java +++ b/src/test/java/io/vertx/core/json/JsonArrayTest.java @@ -193,9 +193,9 @@ public class JsonArrayTest { jsonArray.add(123); try { jsonArray.getString(1); - fail(); + // OK, non string types are casted to string using .toString() } catch (ClassCastException e) { - // OK + fail(); } jsonArray.addNull(); assertNull(jsonArray.getString(2)); diff --git a/src/test/java/io/vertx/core/json/JsonObjectTest.java b/src/test/java/io/vertx/core/json/JsonObjectTest.java index 984819674..daab82b46 100644 --- a/src/test/java/io/vertx/core/json/JsonObjectTest.java +++ b/src/test/java/io/vertx/core/json/JsonObjectTest.java @@ -331,9 +331,9 @@ public class JsonObjectTest { jsonObject.put("bar", 123); try { jsonObject.getString("bar"); - fail(); - } catch (ClassCastException e) { // Ok + } catch (ClassCastException e) { + fail(); } // Null and absent values @@ -357,10 +357,10 @@ public class JsonObjectTest { assertEquals("bar", jsonObject.getString("foo", null)); jsonObject.put("bar", 123); try { - jsonObject.getString("bar", "wibble"); - fail(); + assertEquals("123", jsonObject.getString("bar", "wibble")); + // OK, non string types are casted to string using .toString() } catch (ClassCastException e) { - // Ok + fail(); } // Null and absent values From 2079353d076cba58723c5e7159a779cba3ab6af0 Mon Sep 17 00:00:00 2001 From: Paulo Lopes Date: Tue, 10 Dec 2019 08:53:06 +0100 Subject: [PATCH 2/3] allow shareables in copy() --- .../io/vertx/core/json/impl/JsonUtil.java | 12 +++--- .../io/vertx/core/json/JsonArrayTest.java | 39 +++++++++++++++++++ .../io/vertx/core/json/JsonObjectTest.java | 39 +++++++++++++++++++ 3 files changed, 85 insertions(+), 5 deletions(-) diff --git a/src/main/java/io/vertx/core/json/impl/JsonUtil.java b/src/main/java/io/vertx/core/json/impl/JsonUtil.java index f1ea7828e..746f54647 100644 --- a/src/main/java/io/vertx/core/json/impl/JsonUtil.java +++ b/src/main/java/io/vertx/core/json/impl/JsonUtil.java @@ -12,6 +12,7 @@ package io.vertx.core.json.impl; import io.vertx.core.json.JsonArray; import io.vertx.core.json.JsonObject; +import io.vertx.core.shareddata.Shareable; import java.math.BigDecimal; import java.time.Instant; @@ -82,7 +83,7 @@ public final class JsonUtil { public static Object checkAndCopy(Object val) { if (val == null) { // OK - } else if (val instanceof Number && !(val instanceof BigDecimal)) { + } else if (val instanceof Number) { // OK } else if (val instanceof Boolean) { // OK @@ -91,11 +92,12 @@ public final class JsonUtil { } else if (val instanceof Character) { // OK } else if (val instanceof CharSequence) { + // CharSequences are not immutable, so we force toString() to become immutable val = val.toString(); - } else if (val instanceof JsonObject) { - val = ((JsonObject) val).copy(); - } else if (val instanceof JsonArray) { - val = ((JsonArray) val).copy(); + } else if (val instanceof Shareable) { + // Shareable objects know how to copy themselves, this covers: + // JsonObject, JsonArray or any user defined type that can shared across the cluster + val = ((Shareable) val).copy(); } else if (val instanceof Map) { val = (new JsonObject((Map) val)).copy(); } else if (val instanceof List) { diff --git a/src/test/java/io/vertx/core/json/JsonArrayTest.java b/src/test/java/io/vertx/core/json/JsonArrayTest.java index 934b95d45..e079bb95a 100644 --- a/src/test/java/io/vertx/core/json/JsonArrayTest.java +++ b/src/test/java/io/vertx/core/json/JsonArrayTest.java @@ -12,6 +12,7 @@ package io.vertx.core.json; import io.vertx.core.buffer.Buffer; +import io.vertx.core.shareddata.Shareable; import io.vertx.test.core.TestUtils; import org.junit.Before; import org.junit.Test; @@ -1250,4 +1251,42 @@ public class JsonArrayTest { assertEquals(bytes, json.getBinary(1)); assertSame(bytes, json.getBinary(1)); } + + @Test + public void testBigDecimal() { + BigDecimal bd1 = + new BigDecimal("124567890.0987654321"); + + // storing BigDecimal should not be an issue + JsonArray json = new JsonArray(); + json.add(bd1); + assertEquals(bd1, json.getValue(0)); + assertSame(bd1, json.getValue(0)); + + // copy() should allow it too. + JsonArray json2 = json.copy(); + // encode + assertEquals("[124567890.0987654321]", json.encode()); + } + + @Test + public void testShareable() { + + Shareable myShareable = new Shareable() { + @Override + public Shareable copy() { + return this; + } + }; + + // storing Shareable should not be an issue + JsonArray json = new JsonArray(); + json.add(myShareable); + assertEquals(myShareable, json.getValue(0)); + assertSame(myShareable, json.getValue(0)); + + // copy() should allow it too. + JsonArray json2 = json.copy(); + } + } diff --git a/src/test/java/io/vertx/core/json/JsonObjectTest.java b/src/test/java/io/vertx/core/json/JsonObjectTest.java index daab82b46..8db43d508 100644 --- a/src/test/java/io/vertx/core/json/JsonObjectTest.java +++ b/src/test/java/io/vertx/core/json/JsonObjectTest.java @@ -13,6 +13,7 @@ package io.vertx.core.json; import io.vertx.core.buffer.Buffer; import io.vertx.core.http.HttpMethod; +import io.vertx.core.shareddata.Shareable; import io.vertx.test.core.TestUtils; import org.junit.Before; import org.junit.Test; @@ -1734,6 +1735,44 @@ public class JsonObjectTest { assertEquals(bytes, json.getBinary("bytes")); assertSame(bytes, json.getBinary("bytes")); } + + @Test + public void testBigDecimal() { + BigDecimal bd1 = + new BigDecimal("124567890.0987654321"); + + // storing BigDecimal should not be an issue + JsonObject json = new JsonObject(); + json.put("bd1", bd1); + assertEquals(bd1, json.getValue("bd1")); + assertSame(bd1, json.getValue("bd1")); + + // copy() should allow it too. + JsonObject json2 = json.copy(); + // same for encode + assertEquals("{\"bd1\":124567890.0987654321}", json.encode()); + } + + @Test + public void testShareable() { + + Shareable myShareable = new Shareable() { + @Override + public Shareable copy() { + return this; + } + }; + + // storing Shareable should not be an issue + JsonObject json = new JsonObject(); + json.put("0", myShareable); + assertEquals(myShareable, json.getValue("0")); + assertSame(myShareable, json.getValue("0")); + + // copy() should allow it too. + JsonObject json2 = json.copy(); + } + } From b0ce2a9bae9e82899139938b22788bc66ee33b34 Mon Sep 17 00:00:00 2001 From: Paulo Lopes Date: Fri, 10 Jan 2020 13:13:32 +0100 Subject: [PATCH 3/3] Added more checks --- .../io/vertx/core/json/JsonObjectTest.java | 27 ++++++++++--------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/src/test/java/io/vertx/core/json/JsonObjectTest.java b/src/test/java/io/vertx/core/json/JsonObjectTest.java index 8db43d508..adff96465 100644 --- a/src/test/java/io/vertx/core/json/JsonObjectTest.java +++ b/src/test/java/io/vertx/core/json/JsonObjectTest.java @@ -12,7 +12,6 @@ package io.vertx.core.json; import io.vertx.core.buffer.Buffer; -import io.vertx.core.http.HttpMethod; import io.vertx.core.shareddata.Shareable; import io.vertx.test.core.TestUtils; import org.junit.Before; @@ -22,6 +21,7 @@ import java.math.BigDecimal; import java.time.Instant; import java.time.temporal.ChronoUnit; import java.util.*; +import java.util.concurrent.atomic.AtomicInteger; import java.util.stream.Collectors; import static io.vertx.core.json.impl.JsonUtil.BASE64_DECODER; @@ -330,12 +330,8 @@ public class JsonObjectTest { jsonObject.put("foo", "bar"); assertEquals("bar", jsonObject.getString("foo")); jsonObject.put("bar", 123); - try { - jsonObject.getString("bar"); - // Ok - } catch (ClassCastException e) { - fail(); - } + // should not fail and be casted to string + assertEquals("123", jsonObject.getString("bar")); // Null and absent values jsonObject.putNull("foo"); @@ -357,12 +353,9 @@ public class JsonObjectTest { assertEquals("bar", jsonObject.getString("foo", "wibble")); assertEquals("bar", jsonObject.getString("foo", null)); jsonObject.put("bar", 123); - try { - assertEquals("123", jsonObject.getString("bar", "wibble")); - // OK, non string types are casted to string using .toString() - } catch (ClassCastException e) { - fail(); - } + + // OK, non string types are casted to string using .toString() + assertEquals("123", jsonObject.getString("bar", "wibble")); // Null and absent values jsonObject.putNull("foo"); @@ -1756,9 +1749,12 @@ public class JsonObjectTest { @Test public void testShareable() { + final AtomicInteger cnt = new AtomicInteger(0); + Shareable myShareable = new Shareable() { @Override public Shareable copy() { + cnt.incrementAndGet(); return this; } }; @@ -1770,7 +1766,12 @@ public class JsonObjectTest { assertSame(myShareable, json.getValue("0")); // copy() should allow it too. + assertEquals(0, cnt.get()); JsonObject json2 = json.copy(); + assertEquals(1, cnt.get()); + // verify the copy + assertEquals(myShareable, json2.getValue("0")); + assertSame(myShareable, json2.getValue("0")); } }