Last Rnd Ryan Review

deephaven · Apr 23, 2024 · 9431244 · 9431244
1 parent 86070cb
commit 9431244
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 55 deletions.
diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/lang/QueryLanguageParser.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/lang/QueryLanguageParser.java
@@ -110,6 +110,7 @@
 import org.apache.commons.lang3.exception.ExceptionUtils;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
+import org.jetbrains.annotations.VisibleForTesting;
 import org.jpy.PyObject;
 
 import java.lang.reflect.Array;
@@ -189,32 +190,7 @@ public final class QueryLanguageParser extends GenericVisitorAdapter<Class<?>, Q
      *        imported.
      * @param variables A map of the names of scope variables to their types
      * @param variableTypeArguments A map of the names of scope variables to their type arguments
-     * @throws QueryLanguageParseException If any exception or error is encountered
-     */
-    @TestUseOnly
-    QueryLanguageParser(
-            String expression,
-            Collection<Package> packageImports,
-            Collection<Class<?>> classImports,
-            Collection<Class<?>> staticImports,
-            Map<String, Class<?>> variables,
-            Map<String, Class<?>[]> variableTypeArguments) throws QueryLanguageParseException {
-        this(expression, packageImports, classImports, staticImports, variables,
-                variableTypeArguments, null, null, true, null);
-    }
-
-    /**
-     * Create a QueryLanguageParser and parse the given {@code expression}. After construction, the
-     * {@link QueryLanguageParser.Result result} of parsing the {@code expression} is available with the
-     * {@link #getResult()}} method.
-     *
-     * @param expression The query language expression to parse
-     * @param packageImports Wildcard package imports
-     * @param classImports Individual class imports
-     * @param staticImports Wildcard static imports. All static variables and methods for the given classes are
-     *        imported.
-     * @param variables A map of the names of scope variables to their types
-     * @param variableTypeArguments A map of the names of scope variables to their type arguments
+     * @param unboxArguments If true it will unbox the query scope arguments
      * @param queryScopeVariables A mutable map of the names of query scope variables to their values
      * @param columnVariables A set of column variable names
      * @throws QueryLanguageParseException If any exception or error is encountered
@@ -228,9 +204,21 @@ public QueryLanguageParser(
             Map<String, Class<?>[]> variableTypeArguments,
             @Nullable Map<String, Object> queryScopeVariables,
             @Nullable Set<String> columnVariables,
+            boolean unboxArguments,
             @Nullable TimeLiteralReplacedExpression timeConversionResult) throws QueryLanguageParseException {
-        this(expression, packageImports, classImports, staticImports, variables,
-                variableTypeArguments, queryScopeVariables, columnVariables, true, timeConversionResult);
+        this(
+                expression,
+                packageImports,
+                classImports,
+                staticImports,
+                variables,
+                variableTypeArguments,
+                queryScopeVariables,
+                columnVariables,
+                unboxArguments,
+                false,
+                PyCallableWrapperJpyImpl.class.getName(),
+                timeConversionResult);
     }
 
     /**
@@ -245,37 +233,21 @@ public QueryLanguageParser(
      *        imported.
      * @param variables A map of the names of scope variables to their types
      * @param variableTypeArguments A map of the names of scope variables to their type arguments
-     * @param unboxArguments If true it will unbox the query scope arguments
-     * @param queryScopeVariables A mutable map of the names of query scope variables to their values
-     * @param columnVariables A set of column variable names
      * @throws QueryLanguageParseException If any exception or error is encountered
      */
-    public QueryLanguageParser(
+    @TestUseOnly
+    QueryLanguageParser(
             String expression,
             Collection<Package> packageImports,
             Collection<Class<?>> classImports,
             Collection<Class<?>> staticImports,
             Map<String, Class<?>> variables,
-            Map<String, Class<?>[]> variableTypeArguments,
-            @Nullable Map<String, Object> queryScopeVariables,
-            @Nullable Set<String> columnVariables,
-            boolean unboxArguments,
-            @Nullable TimeLiteralReplacedExpression timeConversionResult) throws QueryLanguageParseException {
-        this(
-                expression,
-                packageImports,
-                classImports,
-                staticImports,
-                variables,
-                variableTypeArguments,
-                queryScopeVariables,
-                columnVariables,
-                unboxArguments,
-                false,
-                PyCallableWrapperJpyImpl.class.getName(),
-                timeConversionResult);
+            Map<String, Class<?>[]> variableTypeArguments) throws QueryLanguageParseException {
+        this(expression, packageImports, classImports, staticImports, variables,
+                variableTypeArguments, null, null, true, null);
     }
 
+    @VisibleForTesting
     QueryLanguageParser(
             String expression,
             final Collection<Package> packageImports,

diff --git a/engine/table/src/main/java/io/deephaven/engine/util/GroovyDeephavenSession.java b/engine/table/src/main/java/io/deephaven/engine/util/GroovyDeephavenSession.java
@@ -86,6 +86,7 @@ public class GroovyDeephavenSession extends AbstractScriptSession<GroovySnapshot
 
     private static final String DEFAULT_SCRIPT_PATH = Configuration.getInstance()
             .getStringWithDefault("GroovyDeephavenSession.defaultScriptPath", ".");
+    private static final String DEFAULT_SCRIPT_PREFIX = "Script";
 
     private static final boolean INCLUDE_DEFAULT_IMPORTS_IN_LOADED_GROOVY =
             Configuration.getInstance()
@@ -135,7 +136,7 @@ protected Class<?> loadClass(String name, boolean resolve) throws ClassNotFoundE
 
     private static class DeephavenGroovyShell extends GroovyShell {
         private final AtomicInteger counter = new AtomicInteger();
-        private volatile String scriptPrefix = "Script";
+        private volatile String scriptPrefix = DEFAULT_SCRIPT_PREFIX;
 
         DeephavenGroovyShell(
                 final GroovyClassLoader loader,
@@ -339,11 +340,10 @@ protected void evaluate(String command, String scriptName) {
         final String lastCommand = fc.second;
         final String commandPrefix = fc.first;
 
-        final String newScriptPrefix = scriptName == null
-                ? groovyShell.scriptPrefix
+        final String currentScriptName = scriptName == null
+                ? DEFAULT_SCRIPT_PREFIX
                 : scriptName.replaceAll("[^0-9A-Za-z_]", "_").replaceAll("(^[0-9])", "_$1");
-        try (final SafeCloseable ignored = groovyShell.setScriptPrefix(newScriptPrefix)) {
-            final String currentScriptName = groovyShell.scriptPrefix;
+        try (final SafeCloseable ignored = groovyShell.setScriptPrefix(currentScriptName)) {
 
             updateClassloader(lastCommand);