Repository: eclipse-vertx/vert.x

Problem statement:
There is no proper way to store mutable objects in a local map
It is possible only for JsonObject, byte array and some other copyable objects known to vert.x.
It would be usefull to have mechanism to store mutable user objects in a local map if they can be copied.


Hints:

Patch:
diff --git a/src/main/java/io/vertx/core/buffer/Buffer.java b/src/main/java/io/vertx/core/buffer/Buffer.java
--- a/src/main/java/io/vertx/core/buffer/Buffer.java
+++ b/src/main/java/io/vertx/core/buffer/Buffer.java
@@ -19,6 +19,7 @@
 import io.vertx.core.ServiceHelper;
 import io.vertx.core.json.JsonArray;
 import io.vertx.core.json.JsonObject;
+import io.vertx.core.shareddata.Shareable;
 import io.vertx.core.shareddata.impl.ClusterSerializable;
 import io.vertx.core.spi.BufferFactory;
 
@@ -36,7 +37,7 @@
  * @author <a href="http://tfox.org">Tim Fox</a>
  */
 @VertxGen
-public interface Buffer extends ClusterSerializable {
+public interface Buffer extends ClusterSerializable, Shareable {
 
   /**
    * Create a new, empty buffer.
diff --git a/src/main/java/io/vertx/core/json/JsonArray.java b/src/main/java/io/vertx/core/json/JsonArray.java
--- a/src/main/java/io/vertx/core/json/JsonArray.java
+++ b/src/main/java/io/vertx/core/json/JsonArray.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.core.shareddata.impl.ClusterSerializable;
 
 import java.time.Instant;
@@ -33,7 +34,7 @@
  *
  * @author <a href="http://tfox.org">Tim Fox</a>
  */
-public class JsonArray implements Iterable<Object>, ClusterSerializable {
+public class JsonArray implements Iterable<Object>, ClusterSerializable, Shareable {
 
   private List<Object> list;
 
@@ -571,6 +572,7 @@ public String encodePrettily() {
    *
    * @return a copy
    */
+  @Override
   public JsonArray copy() {
     List<Object> copiedList = new ArrayList<>(list.size());
     for (Object val: list) {
diff --git a/src/main/java/io/vertx/core/json/JsonObject.java b/src/main/java/io/vertx/core/json/JsonObject.java
--- a/src/main/java/io/vertx/core/json/JsonObject.java
+++ b/src/main/java/io/vertx/core/json/JsonObject.java
@@ -12,6 +12,7 @@
 
 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;
 
 import java.nio.charset.StandardCharsets;
@@ -34,7 +35,7 @@
  *
  * @author <a href="http://tfox.org">Tim Fox</a>
  */
-public class JsonObject implements Iterable<Map.Entry<String, Object>>, ClusterSerializable {
+public class JsonObject implements Iterable<Map.Entry<String, Object>>, ClusterSerializable, Shareable {
 
   private Map<String, Object> map;
 
@@ -784,6 +785,7 @@ public Buffer toBuffer() {
    *
    * @return a copy of the object
    */
+  @Override
   public JsonObject copy() {
     Map<String, Object> copiedMap;
     if (map instanceof LinkedHashMap) {
diff --git a/src/main/java/io/vertx/core/shareddata/LocalMap.java b/src/main/java/io/vertx/core/shareddata/LocalMap.java
--- a/src/main/java/io/vertx/core/shareddata/LocalMap.java
+++ b/src/main/java/io/vertx/core/shareddata/LocalMap.java
@@ -24,13 +24,12 @@
 /**
  * Local maps can be used to share data safely in a single Vert.x instance.
  * <p>
- * The map only allows immutable keys and values in the map, OR certain mutable objects such as {@link io.vertx.core.buffer.Buffer}
- * instances which will be copied when they are added to the map.
+ * By default the map allows immutable keys and values.
+ * Custom keys and values should implement {@link Shareable} interface. The map returns their copies.
  * <p>
  * This ensures there is no shared access to mutable state from different threads (e.g. different event loops) in the
  * Vert.x instance, and means you don't have to protect access to that state using synchronization or locks.
  * <p>
- * Your own objects can be marked as immutable by implementing the {@link Shareable} interface.
  *
  * Since the version 3.4, this class extends the {@link Map} interface. However some methods are only accessible in Java.
  *
diff --git a/src/main/java/io/vertx/core/shareddata/Shareable.java b/src/main/java/io/vertx/core/shareddata/Shareable.java
--- a/src/main/java/io/vertx/core/shareddata/Shareable.java
+++ b/src/main/java/io/vertx/core/shareddata/Shareable.java
@@ -12,18 +12,28 @@
 package io.vertx.core.shareddata;
 
 /**
- * A marker interface which allows you to put arbitrary objects into a {@link io.vertx.core.shareddata.LocalMap}.
+ * An interface which allows you to put arbitrary objects into a {@link io.vertx.core.shareddata.LocalMap}.
  * <p>
- * Normally local maps only allow immutable objects or other copiable objects such as {@link io.vertx.core.buffer.Buffer}
- * instances in order to avoid shared access to mutable state.
+ * Normally local maps only allow immutable or copiable objects in order to avoid shared access to mutable state.
  * <p>
  * However if you have an object that you know is thread-safe you can mark it with this interface and then you
  * will be able to add it to {@link io.vertx.core.shareddata.LocalMap} instances.
  * <p>
+ * Mutable object that you want to store in a {@link io.vertx.core.shareddata.LocalMap}
+ * should override {@link Shareable#copy()} method.
+ * <p>
  * Use this interface with caution.
  * <p>
  *
  * @author <a href="http://tfox.org">Tim Fox</a>
  */
 public interface Shareable {
+
+  /**
+   * Returns a copy of the object.
+   * Only mutable objects should provide a custom implementation of the method.
+   */
+  default Shareable copy() {
+    return this;
+  }
 }
diff --git a/src/main/java/io/vertx/core/shareddata/impl/Checker.java b/src/main/java/io/vertx/core/shareddata/impl/Checker.java
--- a/src/main/java/io/vertx/core/shareddata/impl/Checker.java
+++ b/src/main/java/io/vertx/core/shareddata/impl/Checker.java
@@ -32,9 +32,6 @@ static void checkType(Object obj) {
         obj instanceof Byte ||
         obj instanceof Character ||
         obj instanceof byte[] ||
-        obj instanceof Buffer ||
-        obj instanceof JsonObject ||
-        obj instanceof JsonArray ||
         obj instanceof Shareable) {
     } else {
       throw new IllegalArgumentException("Invalid type for shareddata data structure: " + obj.getClass().getName());
@@ -42,19 +39,14 @@ static void checkType(Object obj) {
   }
 
   static <T> T copyIfRequired(T obj) {
-    if (obj instanceof JsonObject) {
-      return (T)((JsonObject)obj).copy();
-    } else if (obj instanceof JsonArray) {
-      return (T) ((JsonArray) obj).copy();
-    } else if (obj instanceof byte[]) {
+    if (obj instanceof byte[]) {
       //Copy it
       byte[] bytes = (byte[]) obj;
       byte[] copy = new byte[bytes.length];
       System.arraycopy(bytes, 0, copy, 0, bytes.length);
       return (T) copy;
-    } else if (obj instanceof Buffer) {
-      //Copy it
-      return (T) ((Buffer) obj).copy();
+    } else if (obj instanceof Shareable) {
+      return (T) ((Shareable) obj).copy();
     } else {
       return obj;
     }


Test patch:
diff --git a/src/test/java/io/vertx/test/core/LocalSharedDataTest.java b/src/test/java/io/vertx/test/core/LocalSharedDataTest.java
--- a/src/test/java/io/vertx/test/core/LocalSharedDataTest.java
+++ b/src/test/java/io/vertx/test/core/LocalSharedDataTest.java
@@ -15,11 +15,13 @@
 import io.vertx.core.json.JsonArray;
 import io.vertx.core.json.JsonObject;
 import io.vertx.core.shareddata.LocalMap;
+import io.vertx.core.shareddata.Shareable;
 import io.vertx.core.shareddata.SharedData;
 import org.junit.Test;
 
 import java.util.*;
 import java.util.concurrent.atomic.AtomicInteger;
+import java.util.function.BiFunction;
 
 import static io.vertx.test.core.TestUtils.*;
 
@@ -326,7 +328,58 @@ public void testValuesCopied() {
     assertFalse(containsExact(values, json3));
   }
 
+  @Test
+  public void testCopyOnGet() {
+    testMapOperationResult(LocalMap::get);
+  }
+
+  @Test
+  public void testCopyOnPutIfAbsent() {
+    testMapOperationResult((map, key) -> map.putIfAbsent(key, new ShareableObject("some other test data")));
+  }
 
+  @Test
+  public void testCopyOnRemove() {
+    testMapOperationResult(LocalMap::remove);
+  }
+
+  private void testMapOperationResult(BiFunction<LocalMap<String, ShareableObject>, String, ShareableObject> operation) {
+    final String key = "key";
+    final ShareableObject value = new ShareableObject("some test data");
+    final LocalMap<String, ShareableObject> map = vertx.sharedData().getLocalMap("foo");
+    assertNull(map.put(key, value));
+
+    final ShareableObject result = operation.apply(map, key);
+
+    assertEquals(value, result);
+    assertNotSame(value, result);
+  }
+
+  private static class ShareableObject implements Shareable {
+    private final String data;
+
+    public ShareableObject(String data) {
+      this.data = data;
+    }
+
+    @Override
+    public Shareable copy() {
+      return new ShareableObject(data);
+    }
+
+    @Override
+    public boolean equals(Object o) {
+      if (this == o) return true;
+      if (o == null || getClass() != o.getClass()) return false;
+      final ShareableObject another = (ShareableObject) o;
+      return data.equals(another.data);
+    }
+
+    @Override
+    public int hashCode() {
+      return data.hashCode();
+    }
+  }
 
   class SomeOtherClass {
   }