Provide gRPC api for input tables (#1440)

This pull request is broken into three commits to start (with JS API client code to follow in a later pull request):

 * ClassUtil improvements
 * Proposed MutableInputTable api enhancements
 * Proposed gRPC api and implementation

ClassUtil got its lookupClass from the dataobjects ClassUtil, but this implementation isn't thread safe, and while it supports both Class.getName() and Class.getCanonicalName() array notation, it only supports getName() inner type notation. Additionally, it initializes the class on first mention, which could mean for example that an arbitrary doPut could force the server to load a class, even if not serializable. This commit still supports (ignores) generic arguments provided to the class, but otherwise delegates to common-lang3 to actually find the class, and provide the class instance without initializing it.

The MutableInputTable changes are intended to make it easier to use from gRPC on the server, and removes some details where the DHE openapi was used to provide implementation details of the input table.

The gRPC API adds a createInputTable call in table service (still has some TODOs for discussion), and provides a separate gRPC service to interact with the input table itself.

Partial #1271

Base/build.gradle

@@ -1,6 +1,8 @@
 dependencies {
     compile depLog4j, depTrove3, depAnnotations, depCommonsCompress
 
+    compile depCommonsLang3
+
     compile 'io.deephaven:hash:0.1.0'
 
     testCompile fileTree(dir: "${rootDir}/test-libs", include: ['*.jar'])

Base/src/main/java/io/deephaven/base/ClassUtil.java

@@ -4,91 +4,55 @@
 
 package io.deephaven.base;
 
-import org.apache.log4j.Logger;
+import org.apache.commons.lang3.ClassUtils;
 
-import java.lang.reflect.Array;
-import java.lang.reflect.Field;
-import java.lang.reflect.Modifier;
-import java.util.HashMap;
 import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
 
 public final class ClassUtil {
-    public static String getBaseName(final String s) {
-        int i = s.lastIndexOf(".");
-        return (i == -1) ? s : s.substring(i + 1);
-    }
-
-    public static void dumpFinals(final Logger log, final String prefix, final Object p) {
-        final Class c = p.getClass();
-        Field[] fields = c.getDeclaredFields();
-        final int desiredMods = (Modifier.PUBLIC | Modifier.FINAL);
-        for (Field f : fields) {
-            if ((f.getModifiers() & desiredMods) == 0)
-                continue;
-            try {
-                final String tName = f.getType().getName();
-                final String name = f.getName();
-                final Object value = f.get(p);
-                log.info(prefix
-                        + tName
-                        + " " + name
-                        + " = " + value.toString());
-            } catch (Exception ignored) {
-            }
-        }
-    }
 
     public static <T> Class<T> generify(Class c) {
         return c;
     }
 
-    private static final Map<String, Class<?>> classMap = new HashMap<>();
-    public static Map<String, Class<?>> primitives = new HashMap<>();
-
-    static {
-        primitives.put("boolean", boolean.class);
-        primitives.put("int", int.class);
-        primitives.put("double", double.class);
-        primitives.put("long", long.class);
-        primitives.put("byte", byte.class);
-        primitives.put("short", short.class);
-        primitives.put("char", char.class);
-        primitives.put("float", float.class);
-    }
+    private static final Map<String, Class<?>> classMap = new ConcurrentHashMap<>();
 
     private static Class<?> getJavaType(String selectedType) throws ClassNotFoundException {
-        int arrayCount = 0;
-        while (selectedType.endsWith("[]")) {
-            selectedType = selectedType.substring(0, selectedType.length() - 2);
-            ++arrayCount;
-        }
-        Class<?> result = primitives.get(selectedType);
-        if (result == null && selectedType.startsWith("java.lang.")) {
-            result = primitives.get(selectedType.substring("java.lang.".length()));
-        }
-        if (result == null) {
-            result = Class.forName(selectedType.split("<")[0]);
-        }
-        if (arrayCount > 0) {
-            final int[] dimensions = new int[arrayCount];
-            result = Array.newInstance(result, dimensions).getClass();
-        }
-        return result;
+        // Given string might have generics, remove those before delegating to
+        // commons-lang3 for lookup implementation. Greedily match from first
+        // '<' to last '>' and remove all, so that the two types of array
+        // notation are retained.
+        String noGenerics = selectedType.replaceAll("<.*>", "");
+
+        return ClassUtils.getClass(noGenerics, false);
     }
 
+    /**
+     * Finds and caches Class instances based on name. This implementation can handle the strings created by
+     * {@link Class#getName()} and {@link Class#getCanonicalName()}, and some mixture of the two. JNI names are not
+     * supported.
+     *
+     * @param name the name of the class to lookup.
+     * @return A class instance
+     * @throws ClassNotFoundException if the class cannot be found
+     */
     public static Class<?> lookupClass(final String name) throws ClassNotFoundException {
         Class<?> result = classMap.get(name);
         if (result == null) {
             try {
                 result = getJavaType(name);
                 classMap.put(name, result);
             } catch (ClassNotFoundException e) {
-                classMap.put(name, ClassUtil.class);
+                // Note that this prevents some runtime fix to the classpath and retrying
+                classMap.put(name, FailedToResolve.class);
                 throw e;
             }
-        } else if (result == ClassUtil.class) {
+        } else if (result == FailedToResolve.class) {
             throw new ClassNotFoundException(name);
         }
         return result;
     }
+
+    private static class FailedToResolve {
+    }
 }

Base/src/test/java/io/deephaven/base/ClassUtilTest.java

@@ -0,0 +1,116 @@
+package io.deephaven.base;
+
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.Map;
+import java.util.function.Function;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertSame;
+
+@RunWith(Parameterized.class)
+public class ClassUtilTest {
+    @Parameterized.Parameters
+    public static List<Function<Class<?>, String>[]> naming() {
+        return Arrays.asList(
+                new Function[] {(Function<Class<?>, String>) Class::getName},
+                new Function[] {(Function<Class<?>, String>) Class::getCanonicalName});
+    }
+
+    private final Function<Class<?>, String> namer;
+
+    public ClassUtilTest(Function<Class<?>, String> namer) {
+        this.namer = namer;
+    }
+
+    private Class<?> lookup(Class<?> clazz) throws ClassNotFoundException {
+        String string = namer.apply(clazz);
+        System.out.println(string);
+        return ClassUtil.lookupClass(string);
+    }
+
+    private void assertRoundTrip(Class<?> clazz) throws ClassNotFoundException {
+        assertSame(clazz, lookup(clazz));
+    }
+
+    @Test
+    public void lookupClassSimple() throws ClassNotFoundException {
+        assertRoundTrip(String.class);
+        assertRoundTrip(ClassUtilTest.class);
+    }
+
+    @Test
+    public void lookupClassPrimitive() throws ClassNotFoundException {
+        assertRoundTrip(int.class);
+        assertRoundTrip(Integer.class);
+        assertRoundTrip(char.class);
+
+        // assertRoundTrip(void.class);
+        assertRoundTrip(Void.class);
+    }
+
+    @Test
+    public void lookupClassArray() throws ClassNotFoundException {
+        assertRoundTrip(String[].class);
+        assertRoundTrip(String[][][][][][].class);
+        assertRoundTrip(ClassUtilTest[][][][][][].class);
+
+        assertRoundTrip(int[].class);
+    }
+
+    @Test
+    public void lookupClassInner() throws ClassNotFoundException {
+        assertRoundTrip(StaticInnerClass.class);
+        assertRoundTrip(InnerClass.class);
+        assertRoundTrip(StaticInnerClass.StaticInnerStaticInnerClass.class);
+        assertRoundTrip(StaticInnerClass.InnerStaticInnerClass.class);
+        assertRoundTrip(InnerClass.InnerInnerClass.class);
+        assertRoundTrip(Outer.class);
+    }
+
+    @Test
+    public void lookupClassWithClinit() throws ClassNotFoundException {
+        assertRoundTrip(VerifyNotInitialized.class);
+        assertFalse(verifyNotInitializedWasNotInitialized);
+    }
+
+    @Test
+    public void testGenericStrings() throws ClassNotFoundException {
+        assertSame(Map.Entry.class, ClassUtil.lookupClass(namer.apply(Map.Entry.class) + "<String, String>"));
+        // note that this name isn't quite legal for getName(), will try a few iterations to make sure we DWIM
+        assertSame(Map.Entry[].class, ClassUtil.lookupClass(namer.apply(Map.Entry.class) + "<String, String>[]"));
+        assertSame(Map.Entry[].class, ClassUtil.lookupClass(namer.apply(Map.Entry[].class) + "<String, String>"));
+        assertSame(Map.Entry[].class, ClassUtil.lookupClass("[L" + namer.apply(Map.Entry.class) + "<String, String>;"));
+    }
+
+    public static class StaticInnerClass {
+
+        public class StaticInnerStaticInnerClass {
+        }
+        public static class InnerStaticInnerClass {
+        }
+    }
+
+    public class InnerClass {
+        public class InnerInnerClass {
+        }
+    }
+
+    // Do not reset this by hand, that won't cause the class to be re-initialized
+    public static boolean verifyNotInitializedWasNotInitialized = false;
+}
+
+
+class Outer {
+}
+
+
+class VerifyNotInitialized {
+    static {
+        ClassUtilTest.verifyNotInitializedWasNotInitialized = true;
+    }
+}

DB/src/main/java/io/deephaven/db/tables/TableDefinition.java

@@ -234,6 +234,7 @@ public String getColumnNamesAsString() {
      *
      * @param other the other definition
      * @return {@code this} table definition, but in the the column order of {@code other}
+     * @throws IncompatibleTableDefinitionException if the definitions are not compatible
      */
     public TableDefinition checkMutualCompatibility(@NotNull final TableDefinition other) {
         TableDefinition result = checkCompatibility(other, false);

DB/src/main/java/io/deephaven/db/util/config/MutableInputTable.java

@@ -1,31 +1,41 @@
 package io.deephaven.db.util.config;
 
+import io.deephaven.db.exceptions.ArgumentException;
+import io.deephaven.db.tables.ColumnDefinition;
 import io.deephaven.db.tables.Table;
 import io.deephaven.db.tables.TableDefinition;
 import io.deephaven.db.v2.utils.Index;
-import io.deephaven.web.shared.data.InputTableDefinition;
 
 import java.io.IOException;
-import java.util.Arrays;
+import java.util.List;
+import java.util.stream.Collectors;
 
 /**
- * A minimal interface for mutable tables that can be changed over the OpenAPI.
+ * A minimal interface for mutable shared tables, providing the ability to write to the table instance this is attached
+ * to. MutableInputTable instances are set on the table as an attribute.
+ *
+ * Implementations of this interface will make their own guarantees about how atomically changes will be applied and
+ * what operations they support.
  */
 public interface MutableInputTable extends InputTableRowSetter, InputTableEnumGetter {
+
     /**
-     * Get the key and value columns names for this table.
+     * Gets the names of the key columns.
      *
-     * @return the InputTableDefinition.
+     * @return a list with the names of the key columns of this input table
      */
-    InputTableDefinition getDefinition();
+    List<String> getKeyNames();
 
     /**
-     * Get the names of the key columns
+     * Gets the names of the value columns. By default, any column not marked as a key column is a value column.
      *
-     * @return an array with the names of our key columns
+     * @return a list with the names of the value columns of this input table
      */
-    default String[] getKeyNames() {
-        return getDefinition().getKeys();
+    default List<String> getValueNames() {
+        List<String> keyNames = getKeyNames();
+        return getTableDefinition().getColumnNames().stream()
+                .filter(colName -> !keyNames.contains(colName))
+                .collect(Collectors.toList());
     }
 
     /**
@@ -36,7 +46,50 @@ default String[] getKeyNames() {
     TableDefinition getTableDefinition();
 
     /**
-     * Write newData to this table.
+     * Helper to check if a table is compatible with this table, so that it could be added as contents.
+     *
+     * @param tableToApply the table to check if it can used to add or modify this input table
+     * @throws TableDefinition.IncompatibleTableDefinitionException if the definitions are not compatible
+     */
+    default void validateAddOrModify(final Table tableToApply) {
+        getTableDefinition().checkMutualCompatibility(tableToApply.getDefinition());
+    }
+
+    /**
+     * Validates that the given table definition is suitable to be passed to {@link #delete(Table)}.
+     *
+     * @param tableToDelete the definition of the table to delete
+     * @throws UnsupportedOperationException if this table does not support deletes
+     * @throws io.deephaven.db.exceptions.ArgumentException if the given definition isn't compatible to be used to
+     *         delete
+     */
+    default void validateDelete(Table tableToDelete) {
+        final TableDefinition keyDefinition = tableToDelete.getDefinition();
+        final TableDefinition thisDefinition = getTableDefinition();
+        final StringBuilder error = new StringBuilder();
+        final List<String> keyNames = getKeyNames();
+        for (String keyColumn : keyNames) {
+            final ColumnDefinition<?> colDef = keyDefinition.getColumn(keyColumn);
+            final ColumnDefinition<?> thisColDef = thisDefinition.getColumn(keyColumn);
+            if (colDef == null) {
+                error.append("Key Column \"").append(keyColumn).append("\" does not exist.\n");
+            } else if (!colDef.isCompatible(thisColDef)) {
+                error.append("Key Column \"").append(keyColumn).append("\" is not compatible.\n");
+            }
+        }
+        final List<String> extraKeys = keyDefinition.getColumnNames().stream().filter(kd -> !keyNames.contains(kd))
+                .collect(Collectors.toList());
+        if (!extraKeys.isEmpty()) {
+            error.append("Unknown key columns: ").append(extraKeys);
+        }
+        if (error.length() > 0) {
+            throw new ArgumentException("Invalid Key Table Definition: " + error.toString());
+        }
+    }
+
+    /**
+     * Write newData to this table. This method will block until the rows are added. Added rows with keys that match
+     * existing rows will instead replace those rows, if supported.
      *
      * @param newData the data to write to this table
      *
@@ -50,6 +103,7 @@ default String[] getKeyNames() {
      *
      * @param table The rows to delete.
      * @throws IOException If a problem occurred while deleting the rows.
+     * @throws UnsupportedOperationException if this table does not support deletes
      */
     default void delete(Table table) throws IOException {
         delete(table, table.getIndex());
@@ -60,9 +114,12 @@ default void delete(Table table) throws IOException {
      * deleted.
      *
      * @param table The rows to delete.
-     * @throws IOException If a problem occurred while deleting the rows.
+     * @throws IOException if a problem occurred while deleting the rows
+     * @throws UnsupportedOperationException if this table does not support deletes
      */
-    void delete(Table table, Index index) throws IOException;
+    default void delete(Table table, Index index) throws IOException {
+        throw new UnsupportedOperationException("Table does not support deletes");
+    }
 
     /**
      * Return a user-readable description of this MutableInputTable.
@@ -86,7 +143,7 @@ default void delete(Table table) throws IOException {
      * @return true if columnName is a key column, false otherwise
      */
     default boolean isKey(String columnName) {
-        return Arrays.asList(getDefinition().getKeys()).contains(columnName);
+        return getKeyNames().contains(columnName);
     }
 
     /**
@@ -97,7 +154,7 @@ default boolean isKey(String columnName) {
      * @return true if columnName exists in this MutableInputTable
      */
     default boolean hasColumn(String columnName) {
-        return isKey(columnName) || Arrays.asList(getDefinition().getValues()).contains(columnName);
+        return getTableDefinition().getColumnNames().contains(columnName);
     }
 
     /**