Deprecate implicit String-to-UTF8-bytes ticket methods (deephaven#5405)

The implicit String-to-UTF8-bytes ticket methods are potentially confusing when the user really wants to get a query scoped variable ticket. From the perspective of the java client, this may materialize as errors that look something like: > Exception in thread "main" io.deephaven.client.impl.TableHandle$UncheckedTableHandleException: io.deephaven.client.impl.TableHandle$TableHandleException: io.grpc.StatusRuntimeException: FAILED_PRECONDITION: Could not resolve 'sourceId': no resolver for route '84' (byte) In this case, we can see that the user likely had some variable name that starts with "t" (ascii 84) and tried to use the variable name directly with one of the String-to-UTF8-bytes ticket methods. This PR aims to deprecates those methods and internally removes any usage that we have on them.
rcaudy · May 14, 2024 · 7a73398 · 7a73398
1 parent 6ffecac
commit 7a73398
Show file tree

Hide file tree

Showing 7 changed files with 54 additions and 25 deletions.
diff --git a/engine/api/src/main/java/io/deephaven/engine/table/TableFactory.java b/engine/api/src/main/java/io/deephaven/engine/table/TableFactory.java
@@ -10,6 +10,7 @@
 import io.deephaven.qst.table.TicketTable;
 import io.deephaven.qst.table.TimeTable;
 
+import java.nio.charset.StandardCharsets;
 import java.time.Duration;
 import java.time.Instant;
 import java.util.Arrays;
@@ -221,14 +222,16 @@ public static Table merge(Table[] tables) {
     }
 
     /**
-     * Equivalent to {@code of(TicketTable.of(ticket))}.
+     * Equivalent to {@code of(TicketTable.of(ticket.getBytes(StandardCharsets.UTF_8)))}.
      *
      * @param ticket the ticket string
      * @return the ticket table
-     * @see TicketTable#of(String)
+     * @see TicketTable#of(byte[])
+     * @deprecated prefer {@link #ticket(byte[])}
      */
+    @Deprecated
     public static Table ticket(String ticket) {
-        return of(TicketTable.of(ticket));
+        return of(TicketTable.of(ticket.getBytes(StandardCharsets.UTF_8)));
     }
 
     /**

diff --git a/engine/sql/src/main/java/io/deephaven/engine/sql/Sql.java b/engine/sql/src/main/java/io/deephaven/engine/sql/Sql.java
@@ -25,11 +25,11 @@
 import io.deephaven.sql.TableInformation;
 import io.deephaven.util.annotations.ScriptApi;
 
+import java.nio.charset.StandardCharsets;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
-import java.util.stream.Collectors;
 
 /**
  * Experimental SQL execution. Subject to change.
@@ -64,14 +64,18 @@ private static TableSpec parseSql(String sql, Map<String, Table> scope, Map<Tick
         return SqlAdapter.parseSql(sql, scope(scope, out));
     }
 
+    private static TicketTable sqlref(String tableName) {
+        return TicketTable.of(("sqlref/" + tableName).getBytes(StandardCharsets.UTF_8));
+    }
+
     private static Scope scope(Map<String, Table> scope, Map<TicketTable, Table> out) {
         final ScopeStaticImpl.Builder builder = ScopeStaticImpl.builder();
         for (Entry<String, Table> e : scope.entrySet()) {
             final String tableName = e.getKey();
             final Table table = e.getValue();
             // The TicketTable can technically be anything unique (incrementing number, random, ...), but for
             // visualization purposes it makes sense to use the (already unique) table name.
-            final TicketTable spec = TicketTable.of("sqlref/" + tableName);
+            final TicketTable spec = sqlref(tableName);
             final List<String> qualifiedName = List.of(tableName);
             final TableHeader header = adapt(table.getDefinition());
             builder.addTables(TableInformation.of(qualifiedName, header, spec));

diff --git a/java-client/flight-examples/src/main/java/io/deephaven/client/examples/DoPutSpray.java b/java-client/flight-examples/src/main/java/io/deephaven/client/examples/DoPutSpray.java
@@ -17,6 +17,7 @@
 import picocli.CommandLine.Command;
 import picocli.CommandLine.Parameters;
 
+import java.nio.charset.StandardCharsets;
 import java.util.List;
 import java.util.concurrent.Callable;
 import java.util.concurrent.ExecutionException;
@@ -56,7 +57,8 @@ public Void call() throws Exception {
                 .addShutdownHook(new Thread(() -> onShutdown(scheduler, sourceFactory.managedChannel())));
 
         try (final FlightSession sourceSession = sourceFactory.newFlightSession()) {
-            try (final TableHandle sourceHandle = sourceSession.session().execute(TicketTable.of(ticket))) {
+            try (final TableHandle sourceHandle =
+                    sourceSession.session().execute(TicketTable.of(ticket.getBytes(StandardCharsets.UTF_8)))) {
                 for (ConnectOptions other : connects.subList(1, connects.size())) {
                     final Factory otherFactory = FlightSessionFactoryConfig.builder()
                             .clientConfig(other.config())

diff --git a/qst/src/main/java/io/deephaven/qst/TableCreator.java b/qst/src/main/java/io/deephaven/qst/TableCreator.java
@@ -12,6 +12,7 @@
 import io.deephaven.qst.table.TicketTable;
 import io.deephaven.qst.table.TimeTable;
 
+import java.nio.charset.StandardCharsets;
 import java.time.Duration;
 import java.time.Instant;
 import java.util.Arrays;
@@ -245,18 +246,21 @@ default TABLE merge(TABLE[] tables) {
     }
 
     /**
-     * Equivalent to {@code of(TicketTable.of(ticket))}.
+     * Create a ticket table with the UTF-8 bytes from the {@code ticket} string. Equivalent to
+     * {@code of(TicketTable.of(ticket.getBytes(StandardCharsets.UTF_8)))}.
      *
      * @param ticket the ticket string
      * @return the ticket table
-     * @see TicketTable#of(String)
+     * @see TicketTable#of(byte[])
+     * @deprecated prefer {@link #ticket(byte[])} or other explicit methods on {@link TicketTable}
      */
+    @Deprecated
     default TABLE ticket(String ticket) {
-        return of(TicketTable.of(ticket));
+        return of(TicketTable.of(ticket.getBytes(StandardCharsets.UTF_8)));
     }
 
     /**
-     * Equivalent to {@code of(TicketTable.of(ticket))}.
+     * Create a ticket table with the {@code ticket} bytes. Equivalent to {@code of(TicketTable.of(ticket))}.
      *
      * @param ticket the ticket
      * @return the ticket table

diff --git a/qst/src/main/java/io/deephaven/qst/table/TableSpec.java b/qst/src/main/java/io/deephaven/qst/table/TableSpec.java
@@ -11,12 +11,7 @@
 import io.deephaven.qst.TableCreator.TableToOperations;
 import org.immutables.value.Value.Derived;
 
-import java.io.BufferedInputStream;
-import java.io.IOException;
-import java.io.InputStream;
-import java.io.ObjectInputStream;
-import java.nio.file.Files;
-import java.nio.file.Path;
+import java.nio.charset.StandardCharsets;
 import java.util.Collection;
 
 /**
@@ -57,10 +52,24 @@ static TableSpec of(TableCreationLogic logic) {
         return logic.create(TableCreatorImpl.INSTANCE);
     }
 
+    /**
+     * Create a ticket table with the UTF-8 bytes from the {@code ticket} string.
+     *
+     * @param ticket the ticket
+     * @return the ticket table
+     * @deprecated prefer {@link #ticket(byte[])}
+     */
+    @Deprecated
     static TicketTable ticket(String ticket) {
-        return TicketTable.of(ticket);
+        return TicketTable.of(ticket.getBytes(StandardCharsets.UTF_8));
     }
 
+    /**
+     * Create a ticket table with the {@code ticket} bytes.
+     *
+     * @param ticket the ticket
+     * @return the ticket table
+     */
     static TicketTable ticket(byte[] ticket) {
         return TicketTable.of(ticket);
     }

diff --git a/qst/src/main/java/io/deephaven/qst/table/TicketTable.java b/qst/src/main/java/io/deephaven/qst/table/TicketTable.java
@@ -33,7 +33,9 @@ public static TicketTable of(byte[] ticket) {
      *
      * @param ticket the ticket string
      * @return the ticket table
+     * @deprecated prefer to be explicit and either use {@link #of(byte[])} or {@link #fromQueryScopeField(String)}
      */
+    @Deprecated
     public static TicketTable of(String ticket) {
         return ImmutableTicketTable.of(ticket.getBytes(StandardCharsets.UTF_8));
     }
@@ -45,7 +47,7 @@ public static TicketTable of(String ticket) {
      * @return the ticket table
      */
     public static TicketTable fromQueryScopeField(String fieldName) {
-        return of("s/" + fieldName);
+        return of(("s/" + fieldName).getBytes(StandardCharsets.UTF_8));
     }
 
     /**
@@ -56,7 +58,7 @@ public static TicketTable fromQueryScopeField(String fieldName) {
      * @return the ticket table
      */
     public static TicketTable fromApplicationField(String applicationId, String fieldName) {
-        return of("a/" + applicationId + "/f/" + fieldName);
+        return of(("a/" + applicationId + "/f/" + fieldName).getBytes(StandardCharsets.UTF_8));
     }
 
     /**

diff --git a/sql/src/test/java/io/deephaven/sql/SqlAdapterTest.java b/sql/src/test/java/io/deephaven/sql/SqlAdapterTest.java
@@ -14,6 +14,7 @@
 
 import java.io.IOException;
 import java.net.URISyntaxException;
+import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.List;
@@ -349,9 +350,13 @@ private static Scope scope() {
         return ScopeStaticImpl.empty();
     }
 
+    private static TicketTable scan(String name) {
+        return TicketTable.of(("scan/" + name).getBytes(StandardCharsets.UTF_8));
+    }
+
     private static Scope scope(String name, TableHeader header) {
         return ScopeStaticImpl.builder()
-                .addTables(TableInformation.of(List.of(name), header, TicketTable.of("scan/" + name)))
+                .addTables(TableInformation.of(List.of(name), header, scan(name)))
                 .build();
     }
 
@@ -360,8 +365,8 @@ private static Scope scope(
             String name2, TableHeader header2) {
         return ScopeStaticImpl.builder()
                 .addTables(
-                        TableInformation.of(List.of(name1), header1, TicketTable.of("scan/" + name1)),
-                        TableInformation.of(List.of(name2), header2, TicketTable.of("scan/" + name2)))
+                        TableInformation.of(List.of(name1), header1, scan(name1)),
+                        TableInformation.of(List.of(name2), header2, scan(name2)))
                 .build();
     }
 
@@ -371,9 +376,9 @@ private static Scope scope(
             String name3, TableHeader header3) {
         return ScopeStaticImpl.builder()
                 .addTables(
-                        TableInformation.of(List.of(name1), header1, TicketTable.of("scan/" + name1)),
-                        TableInformation.of(List.of(name2), header2, TicketTable.of("scan/" + name2)),
-                        TableInformation.of(List.of(name3), header3, TicketTable.of("scan/" + name3)))
+                        TableInformation.of(List.of(name1), header1, scan(name1)),
+                        TableInformation.of(List.of(name2), header2, scan(name2)),
+                        TableInformation.of(List.of(name3), header3, scan(name3)))
                 .build();
     }
 }