Merge pull request #3223 from eclipse-vertx/issues/json-next-step

Allow Shareable on JSON copy + Autocast getString()

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

@@ -95,8 +95,7 @@ public class JsonArray implements Iterable<Object>, 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<Object>, 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");
   }
 
   /**

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<Map.Entry<String, Object>>, 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<Map.Entry<String, Object>>, 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<Map.Entry<String, Object>>, ClusterS
   /**
    * Remove all the entries in this JSON object
    */
-  @Fluent
   public JsonObject clear() {
     map.clear();
     return this;

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) {

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;
@@ -193,9 +194,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));
@@ -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();
+  }
+
 }

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

@@ -12,7 +12,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;
@@ -21,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;
@@ -329,12 +330,8 @@ public class JsonObjectTest {
     jsonObject.put("foo", "bar");
     assertEquals("bar", jsonObject.getString("foo"));
     jsonObject.put("bar", 123);
-    try {
-      jsonObject.getString("bar");
-      fail();
-    } catch (ClassCastException e) {
-      // Ok
-    }
+    // should not fail and be casted to string
+    assertEquals("123", jsonObject.getString("bar"));
 
     // Null and absent values
     jsonObject.putNull("foo");
@@ -356,12 +353,9 @@ public class JsonObjectTest {
     assertEquals("bar", jsonObject.getString("foo", "wibble"));
     assertEquals("bar", jsonObject.getString("foo", null));
     jsonObject.put("bar", 123);
-    try {
-      jsonObject.getString("bar", "wibble");
-      fail();
-    } catch (ClassCastException e) {
-      // Ok
-    }
+
+    // OK, non string types are casted to string using .toString()
+    assertEquals("123", jsonObject.getString("bar", "wibble"));
 
     // Null and absent values
     jsonObject.putNull("foo");
@@ -1734,6 +1728,52 @@ 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() {
+
+    final AtomicInteger cnt = new AtomicInteger(0);
+
+    Shareable myShareable = new Shareable() {
+      @Override
+      public Shareable copy() {
+        cnt.incrementAndGet();
+        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.
+    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"));
+  }
+
 }