Make mutating operations on wrapped maps work

Currently, wrapped maps use the passed-in Map as-is. This produces intuitive
results when the Map is mutated, but as soon as you try to modify the
DynamicObject, you get cryptic errors due to attempts to use this Map as an
IPersistentMap or IObj (in the case of metadata operations).

This change introduces a wrapper which proxies non-mutating calls through to
the underlying map directly, but which creates a copy of the map (as a
PersistentHashMap) if mutating operations are performed.

I considered simply having wrap() copy the map directly, but I felt that the
name 'wrap' would be seen to imply that mutations on the underlying map would
be reflected in the wrapped DynamicObject, so I chose this route instead.

Author: bdonlan

src/main/java/com/github/rschmitt/dynamicobject/DynamicObject.java

@@ -161,7 +161,8 @@ static <T> Stream<T> deserializeFressianStream(InputStream is, Class<T> type) {
     }
 
     /**
-     * Use the supplied {@code map} to back an instance of {@code type}.
+     * Use the supplied {@code map} to back an instance of {@code type}. The map will be copied upon any modification
+     * attempt, but until then will reflect changes made to the underlying map.
      */
     static <D extends DynamicObject<D>> D wrap(Map map, Class<D> type) {
         return Instances.wrap(map, type);

src/main/java/com/github/rschmitt/dynamicobject/internal/Instances.java

@@ -1,5 +1,6 @@
 package com.github.rschmitt.dynamicobject.internal;
 
+import clojure.lang.IPersistentMap;
 import com.github.rschmitt.dynamicobject.DynamicObject;
 import net.fushizen.invokedynamic.proxy.DynamicProxy;
 
@@ -23,7 +24,15 @@ public static <D extends DynamicObject<D>> D wrap(Map map, Class<D> type) {
         if (map instanceof DynamicObject)
             return type.cast(map);
 
-        return createIndyProxy(map, type);
+        return createIndyProxy(convertMap(map), type);
+    }
+
+    private static Map convertMap(Map map) {
+        if (map instanceof IPersistentMap) {
+            return map;
+        }
+
+        return (Map) WrappingMap.create(map);
     }
 
     private static <D extends DynamicObject<D>> D createIndyProxy(Map map, Class<D> type) {

src/main/java/com/github/rschmitt/dynamicobject/internal/WrappingMap.java

@@ -0,0 +1,140 @@
+package com.github.rschmitt.dynamicobject.internal;
+
+import clojure.lang.Cons;
+import clojure.lang.IMeta;
+import clojure.lang.IObj;
+import clojure.lang.IPersistentMap;
+import clojure.lang.PersistentHashMap;
+import com.github.rschmitt.collider.ClojureMap;
+import net.fushizen.invokedynamic.proxy.DynamicProxy;
+
+import java.lang.invoke.CallSite;
+import java.lang.invoke.ConstantCallSite;
+import java.lang.invoke.MethodHandle;
+import java.lang.invoke.MethodHandles;
+import java.lang.invoke.MethodType;
+import java.lang.reflect.Method;
+import java.util.Map;
+
+import static java.lang.invoke.MethodHandles.publicLookup;
+import static java.lang.invoke.MethodType.methodType;
+
+/**
+ * A map class that wraps some other Map; however, it also implements IPersistentMap and IObj, creating a copy of the
+ * original map when IPersistentMap or IObj methods such as assoc are invoked.
+ */
+abstract class WrappingMap implements IMeta {
+    private static final IPersistentMap EMPTY_MAP = PersistentHashMap.create();
+
+    protected final Map backingMap;
+
+    protected WrappingMap(Map backingMap) {
+        this.backingMap = backingMap;
+    }
+
+    @Override
+    public IPersistentMap meta() {
+        // Avoid copying the map if we're doing a read-only metadata access.
+        return EMPTY_MAP;
+    }
+
+    static Map create(Map other) {
+        try {
+            return (Map)proxy_ctor.invokeExact(other);
+        } catch (Throwable t) {
+            throw new Error("unexpected exception", t);
+        }
+    }
+
+    private static final MethodHandle get_backingMap;
+    private static final MethodHandle proxy_ctor;
+
+    static {
+        try {
+            get_backingMap = MethodHandles.lookup().findGetter(WrappingMap.class, "backingMap", Map.class);
+            proxy_ctor = DynamicProxy.builder()
+                        .withConstructor(Map.class)
+                        .withSuperclass(WrappingMap.class)
+                        .withInterfaces(IPersistentMap.class, IObj.class, Map.class)
+                        .withInvocationHandler(WrappingMap::invocationHandler)
+                        .build()
+                        .constructor()
+                        .asType(methodType(Map.class, Map.class));
+        } catch (Exception e) {
+            throw new Error(e);
+        }
+    }
+
+    private static CallSite invocationHandler(
+            MethodHandles.Lookup lookup,
+            String methodName,
+            MethodType methodType,
+            MethodHandle superHandle
+    ) throws Throwable {
+        // Forward calls that are overridden on WrappingMap to that implementation
+        try {
+            Method m = WrappingMap.class.getDeclaredMethod(methodName, methodType.dropParameterTypes(0, 1).parameterArray());
+
+            // since the method exists, we can just use superHandle
+            return new ConstantCallSite(superHandle.asType(methodType));
+        } catch (NoSuchMethodException e) {
+            // continue
+        }
+
+        CallSite result;
+
+        // Forward calls that are declared on Map, or Object to the backing map.
+        result = forwardCalls(Map.class, methodName, methodType);
+        if (result != null) return result;
+
+        result = forwardCalls(Object.class, methodName, methodType);
+        if (result != null) return result;
+
+        // Any other calls are IPersistentMap calls. We'll want to construct a PersistentHashMap and reinvoke the call
+        // on it.
+        MethodHandle makeMap = MethodHandles.lookup().findVirtual(WrappingMap.class, "createPersistentMap", methodType(IPersistentMap.class));
+
+        MethodType targetMethodType = methodType.dropParameterTypes(0, 1);
+
+        MethodHandle target;
+        try {
+            target = publicLookup().findVirtual(IPersistentMap.class, methodName, targetMethodType);
+        } catch (NoSuchMethodException e) {
+            // It's not on IPersistentMap, so try IObj instead
+            target = publicLookup().findVirtual(IObj.class, methodName, targetMethodType);
+            // If we're successful, we need to do an asType cast since IPersistentMap doesn't extend IObj
+            makeMap = makeMap.asType(methodType(IObj.class, WrappingMap.class));
+        }
+
+        MethodHandle createMapAndForward = MethodHandles.filterArguments(target, 0, makeMap);
+
+        return new ConstantCallSite(createMapAndForward.asType(methodType));
+    }
+
+    private static CallSite forwardCalls(
+            Class<?> klass,
+            String methodName,
+            MethodType methodType
+    ) throws Throwable {
+        try {
+            MethodHandle mapHandle = publicLookup().findVirtual(klass, methodName, methodType.changeParameterType(0, Map.class));
+            // ok, this is a call to the class in question, we just need to look up the backing map and invoke the
+            // method on it instead
+            MethodHandle combinedHandle = MethodHandles.filterArguments(mapHandle, 0, get_backingMap);
+
+            return new ConstantCallSite(combinedHandle.asType(methodType));
+        } catch (NoSuchMethodException e) {
+            return null;
+        }
+    }
+
+    @SuppressWarnings("unused") // invoked by reflection
+    protected IPersistentMap createPersistentMap() {
+        return PersistentHashMap.create(backingMap);
+    }
+
+    @SuppressWarnings("unused") // invoked by reflection
+    private static Object throwUnsupported() {
+        throw new UnsupportedOperationException();
+    }
+}

src/test/java/com/github/rschmitt/dynamicobject/MapTest.java

@@ -5,6 +5,7 @@
 import static java.lang.String.format;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNull;
 
 import org.junit.After;
 import org.junit.Before;
@@ -13,6 +14,8 @@
 import clojure.lang.EdnReader;
 import clojure.lang.PersistentHashMap;
 
+import java.util.HashMap;
+
 public class MapTest {
     static final String SimpleEdn = "{:str \"expected value\", :i 4, :d 3.14}";
     static final String NestedEdn = format("{:version 1, :simple %s}", SimpleEdn);
@@ -52,11 +55,55 @@ public void unknownFieldsAreSerialized() {
         assertEquals(TaggedEdn, actualEdn);
     }
 
+    @Test
+    public void wrappedMapGettersWork() throws Exception {
+        HashMap<Object, Object> map = new HashMap<>();
+        map.put("foo", "bar");
+
+        TestObject obj = DynamicObject.wrap(map, TestObject.class);
+
+        assertEquals("bar", obj.foo());
+    }
+
+
+    @Test
+    public void wrappedMapSettersWork() throws Exception {
+        HashMap<Object, Object> map = new HashMap<>();
+        map.put("foo", "bar");
+
+        TestObject obj = DynamicObject.wrap(map, TestObject.class);
+        obj = obj.withFoo("quux");
+
+        assertEquals("quux", obj.foo());
+    }
+
+    @Test
+    public void wrappedMapMetaWorks() throws Exception {
+        HashMap<Object, Object> map = new HashMap<>();
+        map.put("foo", "bar");
+
+        TestObject obj = DynamicObject.wrap(map, TestObject.class);
+
+        assertNull(obj.getMeta());
+        obj = obj.withMeta("x");
+        assertEquals("x", obj.getMeta());
+
+        assertEquals(1, obj.getMap().size());
+    }
+
     private void binaryRoundTrip(Object expected) {
         Object actual = DynamicObject.fromFressianByteArray(DynamicObject.toFressianByteArray(expected));
         assertEquals(expected, actual);
     }
 
     public interface EmptyObject extends DynamicObject<EmptyObject> {
     }
+
+    public interface TestObject extends DynamicObject<TestObject> {
+        @Key("foo") String foo();
+        @Key("foo") TestObject withFoo(String foo);
+
+        @Meta @Key(":meta") String getMeta();
+        @Meta @Key(":meta") TestObject withMeta(String meta);
+    }
 }