# HG changeset patch
# User attila
# Date 1513083897 -3600
# Node ID 2bf9071e8dce92d9f3e10df6c93ed942b9a01d6a
# Parent  ddba406af760ccb69771b6bdfba44a2325a45ec1
8191905: Add a REMOVE StandardOperation to Dynalink
Reviewed-by: hannesw, sundar

diff -r ddba406af760 -r 2bf9071e8dce src/jdk.dynalink/share/classes/jdk/dynalink/StandardOperation.java
--- a/src/jdk.dynalink/share/classes/jdk/dynalink/StandardOperation.java	Tue Dec 12 14:04:05 2017 +0100
+++ b/src/jdk.dynalink/share/classes/jdk/dynalink/StandardOperation.java	Tue Dec 12 14:04:57 2017 +0100
@@ -111,6 +111,15 @@
      */
     SET,
     /**
+     * Removes the value from a namespace defined on an object. Call sites with this
+     * operation should have a signature of
+     * <code>(receiver,&nbsp;name)&rarr;void</code> or
+     * <code>(receiver)&rarr;void</code> when used with {@link NamedOperation},
+     * with all parameters being of any type (either primitive
+     * or reference). This operation must always be used as part of a {@link NamespaceOperation}.
+     */
+    REMOVE,
+    /**
      * Call a callable object. Call sites with this operation should have a
      * signature of <code>(callable,&nbsp;receiver,&nbsp;arguments...)&rarr;value</code>,
      * with all parameters and return type being of any type (either primitive or
diff -r ddba406af760 -r 2bf9071e8dce src/jdk.dynalink/share/classes/jdk/dynalink/beans/BeanLinker.java
--- a/src/jdk.dynalink/share/classes/jdk/dynalink/beans/BeanLinker.java	Tue Dec 12 14:04:05 2017 +0100
+++ b/src/jdk.dynalink/share/classes/jdk/dynalink/beans/BeanLinker.java	Tue Dec 12 14:04:57 2017 +0100
@@ -107,7 +107,8 @@
 
 /**
  * A class that provides linking capabilities for a single POJO class. Normally not used directly, but managed by
- * {@link BeansLinker}.
+ * {@link BeansLinker}. Most of the functionality is provided by the {@link AbstractJavaLinker} superclass; this
+ * class adds length and element operations for arrays and collections.
  */
 class BeanLinker extends AbstractJavaLinker implements TypeBasedGuardingDynamicLinker {
     BeanLinker(final Class<?> clazz) {
@@ -147,6 +148,8 @@
                     return getElementGetter(req.popNamespace());
                 } else if (op == StandardOperation.SET) {
                     return getElementSetter(req.popNamespace());
+                } else if (op == StandardOperation.REMOVE) {
+                    return getElementRemover(req.popNamespace());
                 }
             }
         }
@@ -228,7 +231,7 @@
         // dealing with an array, or a list or map, but hey...
         // Note that for arrays and lists, using LinkerServices.asType() will ensure that any language specific linkers
         // in use will get a chance to perform any (if there's any) implicit conversion to integer for the indices.
-        if(declaredType.isArray()) {
+        if(declaredType.isArray() && arrayMethod != null) {
             return new GuardedInvocationComponentAndCollectionType(
                     createInternalFilteredGuardedInvocationComponent(arrayMethod.apply(declaredType), linkerServices),
                     CollectionType.ARRAY);
@@ -240,7 +243,7 @@
             return new GuardedInvocationComponentAndCollectionType(
                     createInternalFilteredGuardedInvocationComponent(mapMethod, linkerServices),
                     CollectionType.MAP);
-        } else if(clazz.isArray()) {
+        } else if(clazz.isArray() && arrayMethod != null) {
             return new GuardedInvocationComponentAndCollectionType(
                     getClassGuardedInvocationComponent(linkerServices.filterInternalObjects(arrayMethod.apply(clazz)), callSiteType),
                     CollectionType.ARRAY);
@@ -450,7 +453,7 @@
     }
 
     @SuppressWarnings("unused")
-    private static void noOpSetter() {
+    private static void noOp() {
     }
 
     private static final MethodHandle SET_LIST_ELEMENT = Lookup.PUBLIC.findVirtual(List.class, "set",
@@ -459,12 +462,14 @@
     private static final MethodHandle PUT_MAP_ELEMENT = Lookup.PUBLIC.findVirtual(Map.class, "put",
             MethodType.methodType(Object.class, Object.class, Object.class));
 
-    private static final MethodHandle NO_OP_SETTER_2;
-    private static final MethodHandle NO_OP_SETTER_3;
+    private static final MethodHandle NO_OP_1;
+    private static final MethodHandle NO_OP_2;
+    private static final MethodHandle NO_OP_3;
     static {
-        final MethodHandle noOpSetter = Lookup.findOwnStatic(MethodHandles.lookup(), "noOpSetter", void.class);
-        NO_OP_SETTER_2 = dropObjectArguments(noOpSetter, 2);
-        NO_OP_SETTER_3 = dropObjectArguments(noOpSetter, 3);
+        final MethodHandle noOp = Lookup.findOwnStatic(MethodHandles.lookup(), "noOp", void.class);
+        NO_OP_1 = dropObjectArguments(noOp, 1);
+        NO_OP_2 = dropObjectArguments(noOp, 2);
+        NO_OP_3 = dropObjectArguments(noOp, 3);
     }
 
     private GuardedInvocationComponent getElementSetter(final ComponentLinkRequest req) throws Exception {
@@ -503,7 +508,39 @@
             return gic.replaceInvocation(binder.bind(invocation));
         }
 
-        return guardComponentWithRangeCheck(gicact, callSiteType, nextComponent, binder, isFixedKey ? NO_OP_SETTER_2 : NO_OP_SETTER_3);
+        return guardComponentWithRangeCheck(gicact, callSiteType, nextComponent, binder, isFixedKey ? NO_OP_2 : NO_OP_3);
+    }
+
+    private static final MethodHandle REMOVE_LIST_ELEMENT = Lookup.PUBLIC.findVirtual(List.class, "remove",
+            MethodType.methodType(Object.class, int.class));
+
+    private static final MethodHandle REMOVE_MAP_ELEMENT = Lookup.PUBLIC.findVirtual(Map.class, "remove",
+            MethodType.methodType(Object.class, Object.class));
+
+    private GuardedInvocationComponent getElementRemover(final ComponentLinkRequest req) throws Exception {
+        final CallSiteDescriptor callSiteDescriptor = req.getDescriptor();
+        final Object name = req.name;
+        final boolean isFixedKey = name != null;
+        assertParameterCount(callSiteDescriptor, isFixedKey ? 1 : 2);
+        final LinkerServices linkerServices = req.linkerServices;
+        final MethodType callSiteType = callSiteDescriptor.getMethodType();
+        final GuardedInvocationComponent nextComponent = getNextComponent(req);
+
+        final GuardedInvocationComponentAndCollectionType gicact = guardedInvocationComponentAndCollectionType(
+                callSiteType, linkerServices, null, REMOVE_LIST_ELEMENT, REMOVE_MAP_ELEMENT);
+
+        if (gicact == null) {
+            // Can't remove elements for objects that are neither lists, nor maps.
+            return nextComponent;
+        }
+
+        final Object typedName = getTypedName(name, gicact.collectionType == CollectionType.MAP, linkerServices);
+        if (typedName == INVALID_NAME) {
+            return nextComponent;
+        }
+
+        return guardComponentWithRangeCheck(gicact, callSiteType, nextComponent,
+                new Binder(linkerServices, callSiteType, typedName), isFixedKey ? NO_OP_1: NO_OP_2);
     }
 
     private static final MethodHandle GET_COLLECTION_LENGTH = Lookup.PUBLIC.findVirtual(Collection.class, "size",
diff -r ddba406af760 -r 2bf9071e8dce src/jdk.dynalink/share/classes/jdk/dynalink/beans/BeansLinker.java
--- a/src/jdk.dynalink/share/classes/jdk/dynalink/beans/BeansLinker.java	Tue Dec 12 14:04:05 2017 +0100
+++ b/src/jdk.dynalink/share/classes/jdk/dynalink/beans/BeansLinker.java	Tue Dec 12 14:04:57 2017 +0100
@@ -113,6 +113,8 @@
  * <li> expose elements of native Java arrays, {@link java.util.List} and {@link java.util.Map} objects as
  * {@link StandardOperation#GET} and {@link StandardOperation#SET} operations in the
  * {@link StandardNamespace#ELEMENT} namespace;</li>
+ * <li> expose removal of elements of {@link java.util.List} and {@link java.util.Map} objects as
+ * {@link StandardOperation#REMOVE} operation in the {@link StandardNamespace#ELEMENT} namespace;</li>
  * <li>expose a virtual property named {@code length} on Java arrays, {@link java.util.Collection} and
  * {@link java.util.Map} objects;</li>
  * <li>expose {@link StandardOperation#NEW} on instances of {@link StaticClass}
diff -r ddba406af760 -r 2bf9071e8dce test/nashorn/src/jdk/dynalink/beans/test/BeanLinkerTest.java
--- a/test/nashorn/src/jdk/dynalink/beans/test/BeanLinkerTest.java	Tue Dec 12 14:04:05 2017 +0100
+++ b/test/nashorn/src/jdk/dynalink/beans/test/BeanLinkerTest.java	Tue Dec 12 14:04:57 2017 +0100
@@ -30,6 +30,7 @@
 import static jdk.dynalink.StandardOperation.CALL;
 import static jdk.dynalink.StandardOperation.GET;
 import static jdk.dynalink.StandardOperation.NEW;
+import static jdk.dynalink.StandardOperation.REMOVE;
 import static jdk.dynalink.StandardOperation.SET;
 
 import java.lang.invoke.CallSite;
@@ -39,7 +40,9 @@
 import java.security.AccessControlException;
 import java.util.ArrayList;
 import java.util.Date;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 import jdk.dynalink.CallSiteDescriptor;
 import jdk.dynalink.DynamicLinker;
 import jdk.dynalink.DynamicLinkerFactory;
@@ -90,6 +93,7 @@
     private static final Operation GET_ELEMENT = GET.withNamespace(ELEMENT);
     private static final Operation GET_METHOD = GET.withNamespace(METHOD);
     private static final Operation SET_ELEMENT = SET.withNamespace(ELEMENT);
+    private static final Operation REMOVE_ELEMENT = REMOVE.withNamespace(ELEMENT);
 
     private static final MethodHandle findThrower(final String name) {
         try {
@@ -121,8 +125,8 @@
             final CallSiteDescriptor desc = req.getCallSiteDescriptor();
             final Operation op = desc.getOperation();
             final Operation baseOp = NamedOperation.getBaseOperation(op);
-            if (baseOp != GET_ELEMENT && baseOp != SET_ELEMENT) {
-                // We only handle GET_ELEMENT and SET_ELEMENT.
+            if (baseOp != GET_ELEMENT && baseOp != SET_ELEMENT && baseOp != REMOVE_ELEMENT) {
+                // We only handle GET_ELEMENT, SET_ELEMENT and REMOVE_ELEMENT.
                 return null;
             }
 
@@ -538,4 +542,85 @@
             }
         }
     }
+
+    @Test(dataProvider = "flags")
+    public void removeElementFromListTest(final boolean publicLookup) throws Throwable {
+        final MethodType mt = MethodType.methodType(void.class, Object.class, int.class);
+        final CallSite cs = createCallSite(publicLookup, REMOVE_ELEMENT, mt);
+
+        final List<Integer> list = new ArrayList<>(List.of(23, 430, -4354));
+
+        cs.getTarget().invoke(list, 1);
+        Assert.assertEquals(list, List.of(23, -4354));
+        cs.getTarget().invoke(list, 1);
+        Assert.assertEquals(list, List.of(23));
+        cs.getTarget().invoke(list, 0);
+        Assert.assertEquals(list, List.of());
+        try {
+            cs.getTarget().invoke(list, -1);
+            throw new RuntimeException("expected IndexOutOfBoundsException");
+        } catch (final IndexOutOfBoundsException ex) {
+        }
+
+        try {
+            cs.getTarget().invoke(list, list.size());
+            throw new RuntimeException("expected IndexOutOfBoundsException");
+        } catch (final IndexOutOfBoundsException ex) {
+        }
+    }
+
+    @Test(dataProvider = "flags")
+    public void removeElementFromListWithFixedKeyTest(final boolean publicLookup) throws Throwable {
+        final MethodType mt = MethodType.methodType(void.class, Object.class);
+
+        final List<Integer> list = new ArrayList<>(List.of(23, 430, -4354));
+
+        createCallSite(publicLookup, REMOVE_ELEMENT.named(1), mt).getTarget().invoke(list);
+        Assert.assertEquals(list, List.of(23, -4354));
+        createCallSite(publicLookup, REMOVE_ELEMENT.named(1), mt).getTarget().invoke(list);
+        Assert.assertEquals(list, List.of(23));
+        createCallSite(publicLookup, REMOVE_ELEMENT.named(0), mt).getTarget().invoke(list);
+        Assert.assertEquals(list, List.of());
+        try {
+            createCallSite(publicLookup, REMOVE_ELEMENT.named(-1), mt).getTarget().invoke(list);
+            throw new RuntimeException("expected IndexOutOfBoundsException");
+        } catch (final IndexOutOfBoundsException ex) {
+        }
+
+        try {
+            createCallSite(publicLookup, REMOVE_ELEMENT.named(list.size()), mt).getTarget().invoke(list);
+            throw new RuntimeException("expected IndexOutOfBoundsException");
+        } catch (final IndexOutOfBoundsException ex) {
+        }
+    }
+
+    @Test(dataProvider = "flags")
+    public void removeElementFromMapTest(final boolean publicLookup) throws Throwable {
+        final MethodType mt = MethodType.methodType(void.class, Object.class, Object.class);
+        final CallSite cs = createCallSite(publicLookup, REMOVE_ELEMENT, mt);
+
+        final Map<String, String> map = new HashMap<>(Map.of("k1", "v1", "k2", "v2", "k3", "v3"));
+
+        cs.getTarget().invoke(map, "k2");
+        Assert.assertEquals(map, Map.of("k1", "v1", "k3", "v3"));
+        cs.getTarget().invoke(map, "k4");
+        Assert.assertEquals(map, Map.of("k1", "v1", "k3", "v3"));
+        cs.getTarget().invoke(map, "k1");
+        Assert.assertEquals(map, Map.of("k3", "v3"));
+    }
+
+
+    @Test(dataProvider = "flags")
+    public void removeElementFromMapWithFixedKeyTest(final boolean publicLookup) throws Throwable {
+        final MethodType mt = MethodType.methodType(void.class, Object.class);
+
+        final Map<String, String> map = new HashMap<>(Map.of("k1", "v1", "k2", "v2", "k3", "v3"));
+
+        createCallSite(publicLookup, REMOVE_ELEMENT.named("k2"), mt).getTarget().invoke(map);
+        Assert.assertEquals(map, Map.of("k1", "v1", "k3", "v3"));
+        createCallSite(publicLookup, REMOVE_ELEMENT.named("k4"), mt).getTarget().invoke(map);
+        Assert.assertEquals(map, Map.of("k1", "v1", "k3", "v3"));
+        createCallSite(publicLookup, REMOVE_ELEMENT.named("k1"), mt).getTarget().invoke(map);
+        Assert.assertEquals(map, Map.of("k3", "v3"));
+    }
 }