From d3dae15340701dd12c3412bbc6f8c0dff660ee0e Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Mon, 22 Jan 2024 11:27:08 -0600 Subject: [PATCH] Extract variable api from ScriptSession, let ScriptSession guard reads (#4970) This patch moves from assuming that the ScriptSession owns the ExecContext for locking (with UpdateGraph) and variable access (with QueryScope). Previously, the groovy script session used a lock to interact with its query scope, but python did not under the incorrect assumption that the GIL would guard those reads. Instead, ScriptSession implementations ensure that reads will be safe. Partial #4040 --- .../python/PythonDeephavenSession.java | 106 +++---- .../engine/context/EmptyQueryScope.java | 44 ++- .../engine/context/ExecutionContext.java | 20 +- .../engine/context/PoisonedQueryScope.java | 41 ++- .../deephaven/engine/context/QueryScope.java | 286 +++--------------- .../engine/context/StandaloneQueryScope.java | 114 +++++++ .../java/io/deephaven/engine/sql/Sql.java | 23 +- .../engine/util/AbstractScriptSession.java | 182 ++++++----- .../engine/util/DelegatingScriptSession.java | 41 +-- .../engine/util/GroovyDeephavenSession.java | 68 ++--- .../util/NoLanguageDeephavenSession.java | 55 ++-- .../deephaven/engine/util/ScriptSession.java | 65 +--- .../engine/util/VariableProvider.java | 21 -- .../engine/table/impl/FuzzerTest.java | 8 +- .../scripts/TestGroovyDeephavenSession.java | 34 +-- .../deephaven/client/ObjectServiceTest.java | 15 +- .../lang/completion/ChunkerCompleter.java | 25 +- .../lang/completion/CompletionRequest.java | 12 +- .../completion/ChunkerCompleterMixin.groovy | 4 +- .../ChunkerCompletionHandlerTest.groovy | 53 +--- .../lang/completion/ChunkerGroovyTest.groovy | 7 - .../completion/ChunkerParseTestMixin.groovy | 6 +- .../lang/completion/ChunkerParserTest.groovy | 8 - .../lang/completion/ChunkerPythonTest.groovy | 8 - ...lumnExpressionCompletionHandlerTest.groovy | 55 ++-- .../console/ConsoleServiceGrpcImpl.java | 14 +- .../server/console/ScopeTicketResolver.java | 67 ++-- .../completer/JavaAutoCompleteObserver.java | 4 +- .../completer/PythonAutoCompleteObserver.java | 13 +- .../server/uri/QueryScopeResolver.java | 12 +- .../ApplicationServiceGrpcImplTest.java | 2 +- .../server/appmode/ApplicationTest.java | 3 +- .../runner/DeephavenApiServerTestBase.java | 4 +- .../test/FlightMessageRoundTripTest.java | 249 ++++++++------- 34 files changed, 711 insertions(+), 958 deletions(-) create mode 100644 engine/context/src/main/java/io/deephaven/engine/context/StandaloneQueryScope.java delete mode 100644 engine/table/src/main/java/io/deephaven/engine/util/VariableProvider.java diff --git a/Integrations/src/main/java/io/deephaven/integrations/python/PythonDeephavenSession.java b/Integrations/src/main/java/io/deephaven/integrations/python/PythonDeephavenSession.java index 2a57d8cad1f..9ca382093ad 100644 --- a/Integrations/src/main/java/io/deephaven/integrations/python/PythonDeephavenSession.java +++ b/Integrations/src/main/java/io/deephaven/integrations/python/PythonDeephavenSession.java @@ -24,7 +24,6 @@ import io.deephaven.plugin.type.ObjectTypeLookup.NoOp; import io.deephaven.util.SafeCloseable; import io.deephaven.util.annotations.ScriptApi; -import io.deephaven.util.annotations.VisibleForTesting; import io.deephaven.util.thread.NamingThreadFactory; import io.deephaven.util.thread.ThreadInitializationFactory; import org.jetbrains.annotations.NotNull; @@ -32,6 +31,7 @@ import org.jpy.KeyError; import org.jpy.PyDictWrapper; import org.jpy.PyInputMode; +import org.jpy.PyLib; import org.jpy.PyLib.CallableKind; import org.jpy.PyModule; import org.jpy.PyObject; @@ -41,14 +41,13 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; -import java.util.Collections; -import java.util.LinkedHashMap; import java.util.Map; import java.util.Objects; import java.util.Set; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.function.Consumer; +import java.util.function.Predicate; import java.util.stream.Collectors; /** @@ -150,13 +149,6 @@ public Thread newThread(@NotNull Runnable r) { } } - @Override - @VisibleForTesting - public QueryScope newQueryScope() { - // depend on the GIL instead of local synchronization - return new UnsynchronizedScriptSessionQueryScope(this); - } - /** * Finds the specified script; and runs it as a file, or if it is a stream writes it to a temporary file in order to * run it. @@ -183,20 +175,14 @@ private void runScript(String script) throws IOException { } } - @NotNull + @SuppressWarnings("unchecked") @Override - public Object getVariable(String name) throws QueryScope.MissingVariableException { - return scope + protected T getVariable(String name) throws QueryScope.MissingVariableException { + return (T) scope .getValue(name) - .orElseThrow(() -> new QueryScope.MissingVariableException("No variable for: " + name)); + .orElseThrow(() -> new QueryScope.MissingVariableException("Missing variable " + name)); } - @Override - public T getVariable(String name, T defaultValue) { - return scope - .getValueUnchecked(name) - .orElse(defaultValue); - } @SuppressWarnings("unused") @ScriptApi @@ -224,13 +210,6 @@ protected void evaluate(String command, String scriptName) { } } - @Override - public Map getVariables() { - final Map outMap = new LinkedHashMap<>(); - scope.getEntriesMap().forEach((key, value) -> outMap.put(key, maybeUnwrap(value))); - return outMap; - } - protected static class PythonSnapshot implements Snapshot, SafeCloseable { private final PyDictWrapper dict; @@ -270,59 +249,62 @@ protected Changes createDiff(PythonSnapshot from, PythonSnapshot to, RuntimeExce final String name = change.call(String.class, "__getitem__", int.class, 0); final PyObject fromValue = change.call(PyObject.class, "__getitem__", int.class, 1); final PyObject toValue = change.call(PyObject.class, "__getitem__", int.class, 2); - applyVariableChangeToDiff(diff, name, maybeUnwrap(fromValue), maybeUnwrap(toValue)); + applyVariableChangeToDiff(diff, name, unwrapObject(fromValue), unwrapObject(toValue)); } return diff; } } - private Object maybeUnwrap(Object o) { - if (o instanceof PyObject) { - return maybeUnwrap((PyObject) o); - } - return o; - } - - private Object maybeUnwrap(PyObject o) { - if (o == null) { - return null; - } - final Object javaObject = module.javaify(o); - if (javaObject != null) { - return javaObject; - } - return o; - } - @Override - public Set getVariableNames() { - return Collections.unmodifiableSet(scope.getKeys().collect(Collectors.toSet())); + protected Set getVariableNames(Predicate allowName) { + return PyLib.ensureGil(() -> scope.getKeys().filter(allowName).collect(Collectors.toUnmodifiableSet())); } @Override - public boolean hasVariableName(String name) { + protected boolean hasVariable(String name) { return scope.containsKey(name); } @Override - public synchronized void setVariable(String name, @Nullable Object newValue) { - final PyDictWrapper globals = scope.mainGlobals(); - if (newValue == null) { - try { - globals.delItem(name); - } catch (KeyError key) { - // ignore - } - } else { - if (!(newValue instanceof PyObject)) { - newValue = PythonObjectWrapper.wrap(newValue); + protected synchronized Object setVariable(String name, @Nullable Object newValue) { + Object old = PyLib.ensureGil(() -> { + final PyDictWrapper globals = scope.mainGlobals(); + + if (newValue == null) { + try { + return globals.unwrap().callMethod("pop", name); + } catch (KeyError key) { + return null; + } + } else { + Object wrapped; + if (newValue instanceof PyObject) { + wrapped = newValue; + } else { + wrapped = PythonObjectWrapper.wrap(newValue); + } + // This isn't thread safe, we're relying on the GIL being kind to us (as we have historically done). + // There is no built-in for "replace a variable and return the old one". + Object prev = globals.get(name); + globals.setItem(name, wrapped); + return prev; } - globals.setItem(name, newValue); - } + }); // Observe changes from this "setVariable" (potentially capturing previous or concurrent external changes from // other threads) observeScopeChanges(); + + // This doesn't return the same Java instance of PyObject, so we won't decref it properly, but + // again, that is consistent with how we've historically treated these references. + return old; + } + + @Override + protected Map getAllValues() { + return PyLib + .ensureGil(() -> scope.getEntries() + .collect(Collectors.toUnmodifiableMap(Map.Entry::getKey, Map.Entry::getValue))); } @Override diff --git a/engine/context/src/main/java/io/deephaven/engine/context/EmptyQueryScope.java b/engine/context/src/main/java/io/deephaven/engine/context/EmptyQueryScope.java index f382e9b7d5f..d2b3eb7cca2 100644 --- a/engine/context/src/main/java/io/deephaven/engine/context/EmptyQueryScope.java +++ b/engine/context/src/main/java/io/deephaven/engine/context/EmptyQueryScope.java @@ -3,10 +3,16 @@ */ package io.deephaven.engine.context; +import io.deephaven.engine.liveness.LivenessReferent; +import org.jetbrains.annotations.NotNull; + +import java.lang.ref.WeakReference; import java.util.Collections; +import java.util.Map; import java.util.Set; +import java.util.stream.Stream; -public class EmptyQueryScope extends QueryScope { +public class EmptyQueryScope implements QueryScope { public final static EmptyQueryScope INSTANCE = new EmptyQueryScope(); private EmptyQueryScope() {} @@ -22,7 +28,7 @@ public boolean hasParamName(String name) { } @Override - protected QueryScopeParam createParam(String name) throws MissingVariableException { + public QueryScopeParam createParam(String name) throws MissingVariableException { throw new MissingVariableException("Missing variable " + name); } @@ -42,7 +48,37 @@ public void putParam(String name, T value) { } @Override - public void putObjectFields(Object object) { - throw new IllegalStateException("EmptyQueryScope cannot create parameters"); + public Map toMap() { + return Collections.emptyMap(); + } + + @Override + public boolean tryManage(@NotNull LivenessReferent referent) { + throw new UnsupportedOperationException("tryManage"); + } + + @Override + public boolean tryUnmanage(@NotNull LivenessReferent referent) { + throw new UnsupportedOperationException("tryUnmanage"); + } + + @Override + public boolean tryUnmanage(@NotNull Stream referents) { + throw new UnsupportedOperationException("tryUnmanage"); + } + + @Override + public boolean tryRetainReference() { + throw new UnsupportedOperationException("tryRetainReference"); + } + + @Override + public void dropReference() { + throw new UnsupportedOperationException("dropReference"); + } + + @Override + public WeakReference getWeakReference() { + throw new UnsupportedOperationException("getWeakReference"); } } diff --git a/engine/context/src/main/java/io/deephaven/engine/context/ExecutionContext.java b/engine/context/src/main/java/io/deephaven/engine/context/ExecutionContext.java index 386eb7879c3..673ae1ff745 100644 --- a/engine/context/src/main/java/io/deephaven/engine/context/ExecutionContext.java +++ b/engine/context/src/main/java/io/deephaven/engine/context/ExecutionContext.java @@ -172,6 +172,22 @@ public ExecutionContext withOperationInitializer(final OperationInitializer oper operationInitializer); } + /** + * Returns, or creates, an execution context with the given value for {@code queryScope} and existing values for the + * other members. + * + * @param queryScope the query scope to use instead + * @return the execution context + */ + public ExecutionContext withQueryScope(QueryScope queryScope) { + if (queryScope == this.queryScope) { + return this; + } + return new ExecutionContext(isSystemic, authContext, queryLibrary, queryScope, queryCompiler, updateGraph, + operationInitializer); + } + + /** * Execute runnable within this execution context. */ @@ -333,7 +349,7 @@ public Builder emptyQueryScope() { */ @ScriptApi public Builder newQueryScope() { - this.queryScope = new QueryScope.StandaloneImpl(); + this.queryScope = new StandaloneQueryScope(); return this; } @@ -371,7 +387,7 @@ public Builder captureQueryScopeVars(String... vars) { return newQueryScope(); } - this.queryScope = new QueryScope.StandaloneImpl(); + this.queryScope = new StandaloneQueryScope(); final QueryScopeParam[] params = getContext().getQueryScope().getParams(Arrays.asList(vars)); for (final QueryScopeParam param : params) { diff --git a/engine/context/src/main/java/io/deephaven/engine/context/PoisonedQueryScope.java b/engine/context/src/main/java/io/deephaven/engine/context/PoisonedQueryScope.java index dda0ad0a035..4da6964287c 100644 --- a/engine/context/src/main/java/io/deephaven/engine/context/PoisonedQueryScope.java +++ b/engine/context/src/main/java/io/deephaven/engine/context/PoisonedQueryScope.java @@ -3,11 +3,16 @@ */ package io.deephaven.engine.context; +import io.deephaven.engine.liveness.LivenessReferent; import io.deephaven.util.ExecutionContextRegistrationException; +import org.jetbrains.annotations.NotNull; +import java.lang.ref.WeakReference; +import java.util.Map; import java.util.Set; +import java.util.stream.Stream; -public class PoisonedQueryScope extends QueryScope { +public class PoisonedQueryScope implements QueryScope { public static final PoisonedQueryScope INSTANCE = new PoisonedQueryScope(); @@ -28,7 +33,7 @@ public boolean hasParamName(String name) { } @Override - protected QueryScopeParam createParam(String name) throws MissingVariableException { + public QueryScopeParam createParam(String name) throws MissingVariableException { return fail(); } @@ -48,7 +53,37 @@ public void putParam(String name, T value) { } @Override - public void putObjectFields(Object object) { + public Map toMap() { + return fail(); + } + + @Override + public boolean tryManage(@NotNull LivenessReferent referent) { + return fail(); + } + + @Override + public boolean tryUnmanage(@NotNull LivenessReferent referent) { + return fail(); + } + + @Override + public boolean tryUnmanage(@NotNull Stream referents) { + return fail(); + } + + @Override + public boolean tryRetainReference() { + return fail(); + } + + @Override + public void dropReference() { fail(); } + + @Override + public WeakReference getWeakReference() { + return fail(); + } } diff --git a/engine/context/src/main/java/io/deephaven/engine/context/QueryScope.java b/engine/context/src/main/java/io/deephaven/engine/context/QueryScope.java index cccf14b22bd..2ea696b3189 100644 --- a/engine/context/src/main/java/io/deephaven/engine/context/QueryScope.java +++ b/engine/context/src/main/java/io/deephaven/engine/context/QueryScope.java @@ -3,46 +3,34 @@ */ package io.deephaven.engine.context; -import io.deephaven.time.DateTimeUtils; -import io.deephaven.hash.KeyedObjectHashMap; -import io.deephaven.hash.KeyedObjectKey; +import io.deephaven.engine.liveness.LivenessNode; import io.deephaven.base.log.LogOutput; import io.deephaven.base.log.LogOutputAppendable; -import io.deephaven.api.util.NameValidator; -import io.deephaven.util.QueryConstants; import org.jetbrains.annotations.NotNull; -import java.lang.reflect.Field; -import java.time.Duration; -import java.time.Instant; -import java.time.Period; -import java.util.*; +import java.util.Collection; +import java.util.Map; +import java.util.Objects; +import java.util.Set; /** - * Variable scope used to resolve parameter values during query execution. + * Variable scope used to resolve parameter values during query execution and to expose named objects to users. Objects + * passed in will have their liveness managed by the scope. */ -public abstract class QueryScope implements LogOutputAppendable { +public interface QueryScope extends LivenessNode, LogOutputAppendable { /** * Adds a parameter to the default instance {@link QueryScope}, or updates the value of an existing parameter. + * Objects passed in will have their liveness managed by the scope. * * @param name String name of the parameter to add. * @param value value to assign to the parameter. * @param type of the parameter/value. */ - public static void addParam(final String name, final T value) { + static void addParam(final String name, final T value) { ExecutionContext.getContext().getQueryScope().putParam(name, value); } - /** - * Adds an object's declared fields to the scope. - * - * @param object object whose fields will be added. - */ - public static void addObjectFields(final Object object) { - ExecutionContext.getContext().getQueryScope().putObjectFields(object); - } - /** * Gets a parameter from the default instance {@link QueryScope}. * @@ -51,19 +39,15 @@ public static void addObjectFields(final Object object) { * @return parameter value. * @throws MissingVariableException variable name is not defined. */ - public static T getParamValue(final String name) throws MissingVariableException { + static T getParamValue(final String name) throws MissingVariableException { return ExecutionContext.getContext().getQueryScope().readParamValue(name); } - // ----------------------------------------------------------------------------------------------------------------- - // Implementation - // ----------------------------------------------------------------------------------------------------------------- - /** * A type of RuntimeException thrown when a variable referenced within the {@link QueryScope} is not defined or, * more likely, has not been added to the scope. */ - public static class MissingVariableException extends RuntimeException { + class MissingVariableException extends RuntimeException { public MissingVariableException(final String message, final Throwable cause) { super(message, cause); @@ -78,51 +62,6 @@ private MissingVariableException(final Throwable cause) { } } - /** - * Apply conversions to certain scope variable values. - * - * @param value value - * @return value, or an appropriately converted substitute. - */ - private static Object applyValueConversions(final Object value) { - if (value instanceof String) { - final String stringValue = (String) value; - - if (stringValue.length() > 0 && stringValue.charAt(0) == '\'' - && stringValue.charAt(stringValue.length() - 1) == '\'') { - final String datetimeString = stringValue.substring(1, stringValue.length() - 1); - - final Instant instant = DateTimeUtils.parseInstantQuiet(datetimeString); - if (instant != null) { - return instant; - } - - final long localTime = DateTimeUtils.parseDurationNanosQuiet(datetimeString); - if (localTime != QueryConstants.NULL_LONG) { - return localTime; - } - - final Period period = DateTimeUtils.parsePeriodQuiet(datetimeString); - if (period != null) { - return period; - } - - final Duration duration = DateTimeUtils.parseDurationQuiet(datetimeString); - if (duration != null) { - return duration; - } - - throw new RuntimeException("Cannot parse datetime/time/period : " + stringValue); - } - } - - return value; - } - - // ----------------------------------------------------------------------------------------------------------------- - // Scope manipulation helper methods - // ----------------------------------------------------------------------------------------------------------------- - /** * Get an array of Params by name. See createParam(name) implementations for details. * @@ -130,7 +69,7 @@ private static Object applyValueConversions(final Object value) { * @return A newly-constructed array of newly-constructed Params. * @throws QueryScope.MissingVariableException If any of the named scope variables does not exist. */ - public final QueryScopeParam[] getParams(final Collection names) throws MissingVariableException { + default QueryScopeParam[] getParams(final Collection names) throws MissingVariableException { final QueryScopeParam[] result = new QueryScopeParam[names.size()]; int pi = 0; for (final String name : names) { @@ -139,16 +78,12 @@ public final QueryScopeParam[] getParams(final Collection names) throws return result; } - // ----------------------------------------------------------------------------------------------------------------- - // General scope manipulation methods - // ----------------------------------------------------------------------------------------------------------------- - /** * Get all known scope variable names. * * @return A collection of scope variable names. */ - public abstract Set getParamNames(); + Set getParamNames(); /** * Check if the scope has the given name. @@ -156,7 +91,7 @@ public final QueryScopeParam[] getParams(final Collection names) throws * @param name param name * @return True iff the scope has the given param name */ - public abstract boolean hasParamName(String name); + boolean hasParamName(String name); /** * Get a QueryScopeParam by name. @@ -165,48 +100,57 @@ public final QueryScopeParam[] getParams(final Collection names) throws * @return newly-constructed QueryScopeParam (name + value-snapshot pair). * @throws QueryScope.MissingVariableException If any of the named scope variables does not exist. */ - protected abstract QueryScopeParam createParam(final String name) throws MissingVariableException; + QueryScopeParam createParam(final String name) throws MissingVariableException; /** - * Get the value of a given scope parameter by name. + * Get the value of a given scope parameter by name. Callers may want to unwrap language-specific values using + * {@link #unwrapObject(Object)} before using them. * * @param name parameter name. * @return parameter value. * @throws QueryScope.MissingVariableException If no such scope parameter exists. */ - public abstract T readParamValue(final String name) throws MissingVariableException; + T readParamValue(final String name) throws MissingVariableException; /** - * Get the value of a given scope parameter by name. + * Get the value of a given scope parameter by name. Callers may want to unwrap language-specific values using + * {@link #unwrapObject(Object)} before using them. * * @param name parameter name. * @param defaultValue default parameter value. * @return parameter value, or the default parameter value, if the value is not present. */ - public abstract T readParamValue(final String name, final T defaultValue); + T readParamValue(final String name, final T defaultValue); /** - * Add a parameter to the scope. + * Add a parameter to the scope. Objects passed in will have their liveness managed by the scope. * * @param name parameter name. * @param value parameter value. */ - public abstract void putParam(final String name, final T value); + void putParam(final String name, final T value); /** - * Add an object's public members (referenced reflectively, not a shallow copy!) to this scope if supported. - * Note: This is an optional method. + * Returns an immutable map with all objects in the scope. Callers may want to unwrap language-specific values using + * {@link #unwrapObject(Object)} before using them. * - * @param object object to add public members from. + * @return an immutable map with all known variables and their values. */ - public abstract void putObjectFields(final Object object); + Map toMap(); - // ----------------------------------------------------------------------------------------------------------------- - // LogOutputAppendable implementation - // ----------------------------------------------------------------------------------------------------------------- + /** + * Removes any wrapping that exists on a scope param object so that clients can fetch them. Defaults to returning + * the object itself. + * + * @param object the scoped object + * @return an obj which can be consumed by a client + */ + default Object unwrapObject(Object object) { + return object; + } @Override - public LogOutput append(@NotNull final LogOutput logOutput) { + default LogOutput append(@NotNull final LogOutput logOutput) { logOutput.append('{'); for (final String paramName : getParamNames()) { final Object paramValue = readParamValue(paramName); @@ -222,156 +166,4 @@ public LogOutput append(@NotNull final LogOutput logOutput) { } return logOutput.nl().append('}'); } - - // ----------------------------------------------------------------------------------------------------------------- - // Map-based implementation, with remote scope and object reflection support - // ----------------------------------------------------------------------------------------------------------------- - - public static class StandaloneImpl extends QueryScope { - - private final KeyedObjectHashMap valueRetrievers = - new KeyedObjectHashMap<>(new ValueRetrieverNameKey()); - - public StandaloneImpl() {} - - @Override - public Set getParamNames() { - return valueRetrievers.keySet(); - } - - @Override - public boolean hasParamName(String name) { - return valueRetrievers.containsKey(name); - } - - @Override - protected QueryScopeParam createParam(final String name) throws MissingVariableException { - // noinspection unchecked - final ValueRetriever valueRetriever = valueRetrievers.get(name); - if (valueRetriever == null) { - throw new MissingVariableException("Missing variable " + name); - } - return valueRetriever.createParam(); - } - - @Override - public T readParamValue(final String name) throws MissingVariableException { - // noinspection unchecked - final ValueRetriever valueRetriever = valueRetrievers.get(name); - if (valueRetriever == null) { - throw new MissingVariableException("Missing variable " + name); - } - return valueRetriever.getValue(); - } - - @Override - public T readParamValue(final String name, final T defaultValue) { - // noinspection unchecked - final ValueRetriever valueRetriever = valueRetrievers.get(name); - if (valueRetriever == null) { - return defaultValue; - } - return valueRetriever.getValue(); - } - - @Override - public void putParam(final String name, final T value) { - NameValidator.validateQueryParameterName(name); - // TODO: Can I get rid of this applyValueConversions? It's too inconsistent to feel safe. - valueRetrievers.put(name, new SimpleValueRetriever<>(name, applyValueConversions(value))); - } - - public void putObjectFields(final Object object) { - for (final Field field : object.getClass().getDeclaredFields()) { - valueRetrievers.put(field.getName(), new ReflectiveValueRetriever(object, field)); - } - } - - private static abstract class ValueRetriever { - - private final String name; - - protected ValueRetriever(String name) { - this.name = name; - } - - public String getName() { - return name; - } - - public abstract T getValue(); - - public abstract Class getType(); - - public abstract QueryScopeParam createParam(); - } - - private static class ValueRetrieverNameKey extends KeyedObjectKey.Basic { - - @Override - public String getKey(ValueRetriever valueRetriever) { - return valueRetriever.getName(); - } - } - - private static class SimpleValueRetriever extends ValueRetriever { - - private final T value; - - public SimpleValueRetriever(final String name, final T value) { - super(name); - this.value = value; - } - - @Override - public T getValue() { - return value; - } - - @Override - public Class getType() { - // noinspection unchecked - return (Class) (value != null ? value.getClass() : Object.class); - } - - @Override - public QueryScopeParam createParam() { - return new QueryScopeParam<>(getName(), getValue()); - } - } - - private static class ReflectiveValueRetriever extends ValueRetriever { - - private final Object object; - private final Field field; - - public ReflectiveValueRetriever(final Object object, final Field field) { - super(field.getName()); - this.object = object; - this.field = field; - field.setAccessible(true); - } - - @Override - public T getValue() { - try { - // noinspection unchecked - return (T) field.get(object); - } catch (IllegalAccessException e) { - throw new RuntimeException(e); - } - } - - @Override - public Class getType() { - // noinspection unchecked - return (Class) field.getType(); - } - - @Override - public QueryScopeParam createParam() { - return new QueryScopeParam<>(getName(), getValue()); - } - } - } } diff --git a/engine/context/src/main/java/io/deephaven/engine/context/StandaloneQueryScope.java b/engine/context/src/main/java/io/deephaven/engine/context/StandaloneQueryScope.java new file mode 100644 index 00000000000..3461992aaa0 --- /dev/null +++ b/engine/context/src/main/java/io/deephaven/engine/context/StandaloneQueryScope.java @@ -0,0 +1,114 @@ +package io.deephaven.engine.context; + +import io.deephaven.api.util.NameValidator; +import io.deephaven.engine.liveness.LivenessArtifact; +import io.deephaven.engine.liveness.LivenessReferent; +import io.deephaven.engine.updategraph.DynamicNode; +import io.deephaven.hash.KeyedObjectHashMap; +import io.deephaven.hash.KeyedObjectKey; + +import java.util.Map; +import java.util.Set; +import java.util.stream.Collectors; + +/** + * Map-based implementation, extending LivenessArtifact to manage the objects passed into it. + */ +public class StandaloneQueryScope extends LivenessArtifact implements QueryScope { + + private final KeyedObjectHashMap> valueRetrievers = + new KeyedObjectHashMap<>(new ValueRetrieverNameKey()); + + @Override + public Set getParamNames() { + return valueRetrievers.keySet(); + } + + @Override + public boolean hasParamName(String name) { + return valueRetrievers.containsKey(name); + } + + @Override + public QueryScopeParam createParam(final String name) throws MissingVariableException { + final ValueRetriever valueRetriever = valueRetrievers.get(name); + if (valueRetriever == null) { + throw new MissingVariableException("Missing variable " + name); + } + // noinspection unchecked + return (QueryScopeParam) valueRetriever.createParam(); + } + + @Override + public T readParamValue(final String name) throws MissingVariableException { + final ValueRetriever valueRetriever = valueRetrievers.get(name); + if (valueRetriever == null) { + throw new MissingVariableException("Missing variable " + name); + } + // noinspection unchecked + return (T) valueRetriever.getValue(); + } + + @Override + public T readParamValue(final String name, final T defaultValue) { + final ValueRetriever valueRetriever = valueRetrievers.get(name); + if (valueRetriever == null) { + return defaultValue; + } + // noinspection unchecked + return (T) valueRetriever.getValue(); + } + + @Override + public void putParam(final String name, final T value) { + NameValidator.validateQueryParameterName(name); + if (value instanceof LivenessReferent && DynamicNode.notDynamicOrIsRefreshing(value)) { + manage((LivenessReferent) value); + } + ValueRetriever oldValueRetriever = + valueRetrievers.put(name, new ValueRetriever<>(name, (Object) value)); + + if (oldValueRetriever != null) { + Object oldValue = oldValueRetriever.getValue(); + if (oldValue instanceof LivenessReferent && DynamicNode.notDynamicOrIsRefreshing(oldValue)) { + unmanage((LivenessReferent) oldValue); + } + } + } + + @Override + public Map toMap() { + return valueRetrievers.entrySet().stream() + .collect(Collectors.toUnmodifiableMap(Map.Entry::getKey, e -> e.getValue().value)); + } + + private static class ValueRetriever { + + protected final T value; + private final String name; + + protected ValueRetriever(String name, final T value) { + this.name = name; + this.value = value; + } + + public String getName() { + return name; + } + + public T getValue() { + return value; + } + + public QueryScopeParam createParam() { + return new QueryScopeParam<>(getName(), getValue()); + } + } + + private static class ValueRetrieverNameKey extends KeyedObjectKey.Basic> { + @Override + public String getKey(ValueRetriever valueRetriever) { + return valueRetriever.getName(); + } + } +} 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 index 9fd5af47723..be9bae117f0 100644 --- a/engine/sql/src/main/java/io/deephaven/engine/sql/Sql.java +++ b/engine/sql/src/main/java/io/deephaven/engine/sql/Sql.java @@ -3,12 +3,11 @@ import io.deephaven.base.log.LogOutput; import io.deephaven.base.log.LogOutput.ObjFormatter; import io.deephaven.engine.context.ExecutionContext; +import io.deephaven.engine.context.QueryScope; import io.deephaven.engine.table.ColumnDefinition; import io.deephaven.engine.table.Table; import io.deephaven.engine.table.TableDefinition; import io.deephaven.engine.table.impl.TableCreatorImpl; -import io.deephaven.engine.util.AbstractScriptSession.ScriptSessionQueryScope; -import io.deephaven.engine.util.ScriptSession; import io.deephaven.internal.log.LoggerFactory; import io.deephaven.io.logger.Logger; import io.deephaven.qst.column.header.ColumnHeader; @@ -27,6 +26,7 @@ import java.util.List; import java.util.Map; import java.util.Map.Entry; +import java.util.stream.Collectors; /** * Experimental SQL execution. Subject to change. @@ -80,19 +80,18 @@ private static Scope scope(Map scope, Map out } private static Map currentScriptSessionNamedTables() { - final Map scope = new HashMap<>(); // getVariables() is inefficient // See SQLTODO(catalog-reader-implementation) - for (Entry e : currentScriptSession().getVariables().entrySet()) { - if (e.getValue() instanceof Table) { - scope.put(e.getKey(), (Table) e.getValue()); - } - } - return scope; - } + QueryScope queryScope = ExecutionContext.getContext().getQueryScope(); + final Map scope = queryScope + .toMap() + .entrySet() + .stream() + .map(e -> Map.entry(e.getKey(), queryScope.unwrapObject(e.getValue()))) + .filter(e -> e.getValue() instanceof Table) + .collect(Collectors.toMap(Entry::getKey, e -> (Table) e.getValue())); - private static ScriptSession currentScriptSession() { - return ((ScriptSessionQueryScope) ExecutionContext.getContext().getQueryScope()).scriptSession(); + return scope; } private static TableHeader adapt(TableDefinition tableDef) { diff --git a/engine/table/src/main/java/io/deephaven/engine/util/AbstractScriptSession.java b/engine/table/src/main/java/io/deephaven/engine/util/AbstractScriptSession.java index 4e8e908610f..bce3c2f8236 100644 --- a/engine/table/src/main/java/io/deephaven/engine/util/AbstractScriptSession.java +++ b/engine/table/src/main/java/io/deephaven/engine/util/AbstractScriptSession.java @@ -10,29 +10,31 @@ import io.deephaven.configuration.CacheDir; import io.deephaven.engine.context.QueryCompiler; import io.deephaven.engine.context.ExecutionContext; +import io.deephaven.engine.liveness.LivenessArtifact; +import io.deephaven.engine.liveness.LivenessReferent; import io.deephaven.engine.liveness.LivenessScope; import io.deephaven.engine.liveness.LivenessScopeStack; import io.deephaven.engine.table.PartitionedTable; import io.deephaven.engine.table.Table; -import io.deephaven.engine.table.TableDefinition; import io.deephaven.engine.context.QueryScope; import io.deephaven.engine.context.QueryScopeParam; import io.deephaven.engine.table.hierarchical.HierarchicalTable; +import io.deephaven.engine.updategraph.DynamicNode; import io.deephaven.engine.updategraph.OperationInitializer; import io.deephaven.engine.updategraph.UpdateGraph; import io.deephaven.plugin.type.ObjectType; import io.deephaven.plugin.type.ObjectTypeLookup; import io.deephaven.util.SafeCloseable; -import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import java.io.File; import java.io.IOException; import java.nio.file.Path; -import java.util.LinkedHashSet; +import java.util.Map; import java.util.Optional; import java.util.Set; import java.util.UUID; +import java.util.function.Predicate; import static io.deephaven.engine.table.Table.NON_DISPLAY_TABLE; @@ -40,8 +42,8 @@ * This class exists to make all script sessions to be liveness artifacts, and provide a default implementation for * evaluateScript which handles liveness and diffs in a consistent way. */ -public abstract class AbstractScriptSession extends LivenessScope - implements ScriptSession, VariableProvider { +public abstract class AbstractScriptSession extends LivenessArtifact + implements ScriptSession { private static final Path CLASS_CACHE_LOCATION = CacheDir.get().resolve("script-session-classes"); @@ -60,13 +62,13 @@ private static void createOrClearDirectory(final File directory) { } } + private final ObjectTypeLookup objectTypeLookup; + private final Listener changeListener; private final File classCacheDirectory; + private final ScriptSessionQueryScope queryScope; protected final ExecutionContext executionContext; - private final ObjectTypeLookup objectTypeLookup; - private final Listener changeListener; - private S lastSnapshot; protected AbstractScriptSession( @@ -82,7 +84,7 @@ protected AbstractScriptSession( classCacheDirectory = CLASS_CACHE_LOCATION.resolve(UuidCreator.toString(scriptCacheId)).toFile(); createOrClearDirectory(classCacheDirectory); - final QueryScope queryScope = newQueryScope(); + queryScope = new ScriptSessionQueryScope(); final QueryCompiler compilerContext = QueryCompiler.create(classCacheDirectory, Thread.currentThread().getContextClassLoader()); @@ -138,15 +140,17 @@ public synchronized final Changes evaluateScript(final String script, @Nullable RuntimeException evaluateErr = null; final Changes diff; + // retain any objects which are created in the executed code, we'll release them when the script session // closes try (final S initialSnapshot = takeSnapshot(); - final SafeCloseable ignored = LivenessScopeStack.open(this, false)) { + final SafeCloseable ignored = LivenessScopeStack.open(queryScope, false)) { try { // Actually evaluate the script; use the enclosing auth context, since AbstractScriptSession's // ExecutionContext never has a non-null AuthContext executionContext.withAuthContext(ExecutionContext.getContext().getAuthContext()) + .withQueryScope(queryScope) .apply(() -> evaluate(script, scriptName)); } catch (final RuntimeException err) { evaluateErr = err; @@ -228,6 +232,11 @@ public Changes evaluateScript(Path scriptPath) { @Override protected void destroy() { super.destroy(); + + // Release a reference to the query scope, so if we're the last ones holding the bag, the scope and its contents + // can go away + queryScope.release(); + // Clear our session's script directory: if (classCacheDirectory.exists()) { FileUtils.deleteRecursively(classCacheDirectory); @@ -243,85 +252,81 @@ protected void destroy() { */ protected abstract void evaluate(String command, @Nullable String scriptName); + public QueryScope getQueryScope() { + return queryScope; + } + /** - * @return a query scope for this session; only invoked during construction + * Retrieve a variable from the script session's bindings. Values may need to be unwrapped. + * + * @param name the name of the variable to retrieve + * @return the variable value + * @throws QueryScope.MissingVariableException if the variable does not exist */ - protected abstract QueryScope newQueryScope(); + protected abstract T getVariable(String name) throws QueryScope.MissingVariableException; - @Override - public Class getVariableType(final String var) { - final Object result = getVariable(var, null); - if (result == null) { - return null; - } else if (result instanceof Table) { - return Table.class; - } else { - return result.getClass(); - } - } + /** + * Retrieves all variable names present in the session's scope. + * + * @param allowName a predicate to decide if a name should be in the returned immutable set + * @return an immutable set of variable names + */ + protected abstract Set getVariableNames(Predicate allowName); + /** + * Check if the scope has the given variable name. + * + * @param name the variable name + * @return True iff the scope has the given variable name + */ + protected abstract boolean hasVariable(String name); - @Override - public TableDefinition getTableDefinition(final String var) { - Object o = getVariable(var, null); - return o instanceof Table ? ((Table) o).getDefinition() : null; - } + /** + * Inserts a value into the script's scope. + * + * @param name the variable name to set + * @param value the new value of the variable + * @return the old value for this name, if any. As with {@link #getVariable(String)}, may need to be unwrapped. + */ + protected abstract Object setVariable(String name, @Nullable Object value); - @Override - public VariableProvider getVariableProvider() { - return this; - } + /** + * Returns an immutable map with all known variables and their values. + * + * @return an immutable map with all known variables and their values. As with {@link #getVariable(String)}, values + * may need to be unwrapped. + */ + protected abstract Map getAllValues(); // ----------------------------------------------------------------------------------------------------------------- // ScriptSession-based QueryScope implementation, with no remote scope or object reflection support // ----------------------------------------------------------------------------------------------------------------- - public abstract static class ScriptSessionQueryScope extends QueryScope { - final ScriptSession scriptSession; - - public ScriptSessionQueryScope(ScriptSession scriptSession) { - this.scriptSession = scriptSession; - } - - @Override - public void putObjectFields(Object object) { - throw new UnsupportedOperationException(); - } - + public class ScriptSessionQueryScope extends LivenessScope implements QueryScope { + /** + * Internal workaround to support python calling pushScope. + */ public ScriptSession scriptSession() { - return scriptSession; - } - } - - public static class UnsynchronizedScriptSessionQueryScope extends ScriptSessionQueryScope { - public UnsynchronizedScriptSessionQueryScope(@NotNull final ScriptSession scriptSession) { - super(scriptSession); + return AbstractScriptSession.this; } @Override public Set getParamNames() { - final Set result = new LinkedHashSet<>(); - for (final String name : scriptSession.getVariableNames()) { - if (NameValidator.isValidQueryParameterName(name)) { - result.add(name); - } - } - return result; + return getVariableNames(NameValidator::isValidQueryParameterName); } @Override public boolean hasParamName(String name) { - return NameValidator.isValidQueryParameterName(name) && scriptSession.hasVariableName(name); + return NameValidator.isValidQueryParameterName(name) && hasVariable(name); } @Override - protected QueryScopeParam createParam(final String name) + public QueryScopeParam createParam(final String name) throws QueryScope.MissingVariableException { if (!NameValidator.isValidQueryParameterName(name)) { throw new QueryScope.MissingVariableException("Name " + name + " is invalid"); } - // noinspection unchecked - return new QueryScopeParam<>(name, (T) scriptSession.getVariable(name)); + return new QueryScopeParam<>(name, readParamValue(name)); } @Override @@ -330,7 +335,7 @@ public T readParamValue(final String name) throws QueryScope.MissingVariable throw new QueryScope.MissingVariableException("Name " + name + " is invalid"); } // noinspection unchecked - return (T) scriptSession.getVariable(name); + return (T) getVariable(name); } @Override @@ -338,49 +343,40 @@ public T readParamValue(final String name, final T defaultValue) { if (!NameValidator.isValidQueryParameterName(name)) { return defaultValue; } - return scriptSession.getVariable(name, defaultValue); - } - - @Override - public void putParam(final String name, final T value) { - scriptSession.setVariable(NameValidator.validateQueryParameterName(name), value); - } - } - public static class SynchronizedScriptSessionQueryScope extends UnsynchronizedScriptSessionQueryScope { - public SynchronizedScriptSessionQueryScope(@NotNull final ScriptSession scriptSession) { - super(scriptSession); + try { + // noinspection unchecked + return (T) getVariable(name); + } catch (MissingVariableException e) { + return defaultValue; + } } @Override - public synchronized Set getParamNames() { - return super.getParamNames(); - } + public void putParam(final String name, final T value) { + NameValidator.validateQueryParameterName(name); + if (value instanceof LivenessReferent && DynamicNode.notDynamicOrIsRefreshing(value)) { + manage((LivenessReferent) value); + } - @Override - public synchronized boolean hasParamName(String name) { - return super.hasParamName(name); - } + Object oldValue = setVariable(name, value); - @Override - protected synchronized QueryScopeParam createParam(final String name) - throws QueryScope.MissingVariableException { - return super.createParam(name); - } + Object unwrappedOldValue = unwrapObject(oldValue); - @Override - public synchronized T readParamValue(final String name) throws QueryScope.MissingVariableException { - return super.readParamValue(name); + if (unwrappedOldValue instanceof LivenessReferent + && DynamicNode.notDynamicOrIsRefreshing(unwrappedOldValue)) { + unmanage((LivenessReferent) unwrappedOldValue); + } } @Override - public synchronized T readParamValue(final String name, final T defaultValue) { - return super.readParamValue(name, defaultValue); + public Map toMap() { + return AbstractScriptSession.this.getAllValues(); } @Override - public synchronized void putParam(final String name, final T value) { - super.putParam(name, value); + public Object unwrapObject(Object object) { + return AbstractScriptSession.this.unwrapObject(object); } } } diff --git a/engine/table/src/main/java/io/deephaven/engine/util/DelegatingScriptSession.java b/engine/table/src/main/java/io/deephaven/engine/util/DelegatingScriptSession.java index f67afedb5b9..b9f3814fd2d 100644 --- a/engine/table/src/main/java/io/deephaven/engine/util/DelegatingScriptSession.java +++ b/engine/table/src/main/java/io/deephaven/engine/util/DelegatingScriptSession.java @@ -12,7 +12,6 @@ import java.lang.ref.WeakReference; import java.nio.file.Path; import java.util.HashSet; -import java.util.Map; import java.util.Objects; import java.util.Set; import java.util.stream.Stream; @@ -63,20 +62,9 @@ public void observeScopeChanges() { delegate.observeScopeChanges(); } - @NotNull @Override - public Object getVariable(String name) throws QueryScope.MissingVariableException { - return delegate.getVariable(name); - } - - @Override - public T getVariable(String name, T defaultValue) { - return delegate.getVariable(name, defaultValue); - } - - @Override - public VariableProvider getVariableProvider() { - return delegate.getVariableProvider(); + public QueryScope getQueryScope() { + return delegate.getQueryScope(); } @Override @@ -89,26 +77,6 @@ public Changes evaluateScript(Path scriptPath) { return contextualizeChanges(delegate.evaluateScript(scriptPath)); } - @Override - public Map getVariables() { - return delegate.getVariables(); - } - - @Override - public Set getVariableNames() { - return delegate.getVariableNames(); - } - - @Override - public boolean hasVariableName(String name) { - return delegate.hasVariableName(name); - } - - @Override - public void setVariable(String name, Object value) { - delegate.setVariable(name, value); - } - @Override public String scriptType() { return delegate.scriptType(); @@ -143,9 +111,4 @@ public void dropReference() { public WeakReference getWeakReference() { return delegate.getWeakReference(); } - - @Override - public void release() { - delegate.release(); - } } 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 index 00e396654e7..9cb12275e73 100644 --- a/engine/table/src/main/java/io/deephaven/engine/util/GroovyDeephavenSession.java +++ b/engine/table/src/main/java/io/deephaven/engine/util/GroovyDeephavenSession.java @@ -7,7 +7,6 @@ import groovy.lang.Binding; import groovy.lang.GroovyClassLoader; import groovy.lang.GroovyShell; -import groovy.lang.MissingPropertyException; import io.deephaven.api.agg.Aggregation; import io.deephaven.api.updateby.BadDataBehavior; import io.deephaven.api.updateby.DeltaControl; @@ -53,7 +52,6 @@ import org.codehaus.groovy.control.Phases; import org.codehaus.groovy.control.customizers.ImportCustomizer; import org.codehaus.groovy.tools.GroovyClass; -import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import javax.tools.JavaFileObject; @@ -74,6 +72,7 @@ import java.time.ZonedDateTime; import java.util.*; import java.util.concurrent.ConcurrentHashMap; +import java.util.function.Predicate; import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Collectors; @@ -136,6 +135,7 @@ protected Class loadClass(String name, boolean resolve) throws ClassNotFoundE private final ImportCustomizer loadedGroovyScriptImports = new ImportCustomizer(); private final Set dynamicClasses = new HashSet<>(); + private final Map bindingBackingMap = Collections.synchronizedMap(new LinkedHashMap<>()); private final GroovyShell groovyShell; private int counter; @@ -178,8 +178,8 @@ public GroovyDeephavenSession( consoleConfig.getCompilationCustomizers().add(consoleImports); consoleConfig.setTargetDirectory(executionContext.getQueryCompiler().getFakeClassDestination()); - - groovyShell = new GroovyShell(scriptClassLoader, consoleConfig) { + Binding binding = new Binding(bindingBackingMap); + groovyShell = new GroovyShell(scriptClassLoader, binding, consoleConfig) { protected synchronized String generateScriptName() { return GroovyDeephavenSession.this.generateScriptName(); } @@ -251,11 +251,6 @@ private String generateScriptName() { return script + "_" + (++counter) + ".groovy"; } - @Override - public QueryScope newQueryScope() { - return new SynchronizedScriptSessionQueryScope(this); - } - public static InputStream findScript(String relativePath) throws IOException { return new ScriptFinder(DEFAULT_SCRIPT_PATH).findScript(relativePath); } @@ -284,23 +279,14 @@ public void runScriptOnce(String script) throws IOException { executedScripts.add(script); } - @NotNull @Override - public Object getVariable(String name) throws QueryScope.MissingVariableException { - try { - return groovyShell.getContext().getVariable(name); - } catch (MissingPropertyException mpe) { - throw new QueryScope.MissingVariableException("No binding for: " + name, mpe); - } - } - - @Override - public T getVariable(String name, T defaultValue) { - try { - // noinspection unchecked - return (T) getVariable(name); - } catch (QueryScope.MissingVariableException e) { - return defaultValue; + protected T getVariable(String name) { + synchronized (bindingBackingMap) { + if (bindingBackingMap.containsKey(name)) { + // noinspection unchecked + return (T) bindingBackingMap.get(name); + } + throw new QueryScope.MissingVariableException("Missing variable " + name); } } @@ -712,12 +698,6 @@ private static boolean isAnInteger(final String s) { } } - @Override - public Map getVariables() { - // noinspection unchecked - return Collections.unmodifiableMap(groovyShell.getContext().getVariables()); - } - @Override protected GroovySnapshot emptySnapshot() { return new GroovySnapshot(Collections.emptyMap()); @@ -763,23 +743,35 @@ public void close() { } } - public Set getVariableNames() { - // noinspection unchecked - return Collections.unmodifiableSet(groovyShell.getContext().getVariables().keySet()); + @Override + protected Set getVariableNames(Predicate allowName) { + synchronized (bindingBackingMap) { + return bindingBackingMap.keySet().stream().filter(allowName).collect(Collectors.toUnmodifiableSet()); + } } @Override - public boolean hasVariableName(String name) { - return groovyShell.getContext().hasVariable(name); + protected boolean hasVariable(String name) { + return bindingBackingMap.containsKey(name); } @Override - public void setVariable(String name, @Nullable Object newValue) { - groovyShell.getContext().setVariable(NameValidator.validateQueryParameterName(name), newValue); + protected Object setVariable(String name, @Nullable Object newValue) { + NameValidator.validateQueryParameterName(name); + + Object oldValue = bindingBackingMap.put(name, newValue); // Observe changes from this "setVariable" (potentially capturing previous or concurrent external changes from // other threads) observeScopeChanges(); + return oldValue; + } + + @Override + protected Map getAllValues() { + synchronized (bindingBackingMap) { + return Map.copyOf(bindingBackingMap); + } } public Binding getBinding() { diff --git a/engine/table/src/main/java/io/deephaven/engine/util/NoLanguageDeephavenSession.java b/engine/table/src/main/java/io/deephaven/engine/util/NoLanguageDeephavenSession.java index ac524bb4075..56abfb0756e 100644 --- a/engine/table/src/main/java/io/deephaven/engine/util/NoLanguageDeephavenSession.java +++ b/engine/table/src/main/java/io/deephaven/engine/util/NoLanguageDeephavenSession.java @@ -6,13 +6,14 @@ import io.deephaven.engine.context.QueryScope; import io.deephaven.engine.updategraph.OperationInitializer; import io.deephaven.engine.updategraph.UpdateGraph; -import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import java.util.Collections; import java.util.LinkedHashMap; import java.util.Map; import java.util.Set; +import java.util.function.Predicate; +import java.util.stream.Collectors; /** * ScriptSession implementation that simply allows variables to be exported. This is not intended for use in user @@ -37,31 +38,17 @@ public NoLanguageDeephavenSession( super(updateGraph, operationInitializer, null, null); this.scriptType = scriptType; - variables = new LinkedHashMap<>(); + variables = Collections.synchronizedMap(new LinkedHashMap<>()); } @Override - public QueryScope newQueryScope() { - return new SynchronizedScriptSessionQueryScope(this); - } - - @NotNull - @Override - public Object getVariable(String name) throws QueryScope.MissingVariableException { - final Object var = variables.get(name); - if (var != null) { - return var; - } - throw new QueryScope.MissingVariableException("No global variable for: " + name); - } - - @Override - public T getVariable(String name, T defaultValue) { - try { + protected T getVariable(String name) { + synchronized (variables) { + if (!variables.containsKey(name)) { + throw new QueryScope.MissingVariableException("Missing variable " + name); + } // noinspection unchecked - return (T) getVariable(name); - } catch (QueryScope.MissingVariableException e) { - return defaultValue; + return (T) variables.get(name); } } @@ -89,27 +76,31 @@ protected void evaluate(String command, @Nullable String scriptName) { } @Override - public Map getVariables() { - return Collections.unmodifiableMap(variables); - } - - @Override - public Set getVariableNames() { - return Collections.unmodifiableSet(variables.keySet()); + protected Set getVariableNames(Predicate allowName) { + synchronized (variables) { + return variables.keySet().stream().filter(allowName).collect(Collectors.toUnmodifiableSet()); + } } @Override - public boolean hasVariableName(String name) { + protected boolean hasVariable(String name) { return variables.containsKey(name); } @Override - public void setVariable(String name, @Nullable Object newValue) { - variables.put(name, newValue); + protected Object setVariable(String name, @Nullable Object newValue) { + return variables.put(name, newValue); // changeListener is always null for NoLanguageDeephavenSession; we have no mechanism for reporting scope // changes } + @Override + protected Map getAllValues() { + synchronized (variables) { + return Map.copyOf(variables); + } + } + @Override public String scriptType() { return SCRIPT_TYPE; diff --git a/engine/table/src/main/java/io/deephaven/engine/util/ScriptSession.java b/engine/table/src/main/java/io/deephaven/engine/util/ScriptSession.java index 1b7dea829b1..44280df2aa0 100644 --- a/engine/table/src/main/java/io/deephaven/engine/util/ScriptSession.java +++ b/engine/table/src/main/java/io/deephaven/engine/util/ScriptSession.java @@ -7,50 +7,23 @@ import io.deephaven.engine.context.QueryScope; import io.deephaven.engine.liveness.LivenessNode; import io.deephaven.engine.liveness.ReleasableLivenessManager; -import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import java.nio.file.Path; import java.util.LinkedHashMap; import java.util.Map; -import java.util.Set; /** * Interface for interactive console script sessions. */ -public interface ScriptSession extends ReleasableLivenessManager, LivenessNode { +public interface ScriptSession extends LivenessNode { /** - * Retrieve a variable from the script session's bindings. - *

- * Please use {@link ScriptSession#getVariable(String, Object)} if you expect the variable may not exist. + * Provides access to the query scope defined by the state in this script session. * - * @param name the variable to retrieve - * @return the variable - * @throws QueryScope.MissingVariableException if the variable does not exist + * @return an implementation defined QueryScope, allowing access to state in the script session */ - @NotNull - Object getVariable(String name) throws QueryScope.MissingVariableException; - - /** - * Retrieve a variable from the script session's bindings. If the variable is not present, return defaultValue. - * - * If the variable is present, but is not of type (T), a ClassCastException may result. - * - * @param name the variable to retrieve - * @param defaultValue the value to use when no value is present in the session's scope - * @param the type of the variable - * @return the value of the variable, or defaultValue if not present - */ - T getVariable(String name, T defaultValue); - - /** - * A {@link VariableProvider} instance, for services like autocomplete which may want a limited "just the variables" - * view of our session state. - * - * @return a VariableProvider instance backed by the global/binding context of this script session. - */ - VariableProvider getVariableProvider(); + QueryScope getQueryScope(); /** * Obtain an {@link ExecutionContext} instance for the current script session. This is the execution context that is @@ -122,36 +95,6 @@ default Changes evaluateScript(String script) { */ Changes evaluateScript(Path scriptPath); - /** - * Retrieves all of the variables present in the session's scope (e.g., Groovy binding, Python globals()). - * - * @return an unmodifiable map with variable names as the keys, and the Objects as the result - */ - Map getVariables(); - - /** - * Retrieves all of the variable names present in the session's scope - * - * @return an unmodifiable set of variable names - */ - Set getVariableNames(); - - /** - * Check if the scope has the given variable name - * - * @param name the variable name - * @return True iff the scope has the given variable name - */ - boolean hasVariableName(String name); - - /** - * Inserts a value into the script's scope. - * - * @param name the variable name to set - * @param value the new value of the variable - */ - void setVariable(String name, @Nullable Object value); - /** * @return a textual description of this script session's language for use in messages. */ diff --git a/engine/table/src/main/java/io/deephaven/engine/util/VariableProvider.java b/engine/table/src/main/java/io/deephaven/engine/util/VariableProvider.java deleted file mode 100644 index 8a7bde003c4..00000000000 --- a/engine/table/src/main/java/io/deephaven/engine/util/VariableProvider.java +++ /dev/null @@ -1,21 +0,0 @@ -/** - * Copyright (c) 2016-2022 Deephaven Data Labs and Patent Pending - */ -package io.deephaven.engine.util; - -import io.deephaven.engine.table.TableDefinition; - -import java.util.Set; - -/** - * - */ -public interface VariableProvider { - Set getVariableNames(); - - Class getVariableType(String var); - - T getVariable(String var, T defaultValue); - - TableDefinition getTableDefinition(String var); -} diff --git a/engine/table/src/test/java/io/deephaven/engine/table/impl/FuzzerTest.java b/engine/table/src/test/java/io/deephaven/engine/table/impl/FuzzerTest.java index e2ad87c6a63..fa54cca0271 100644 --- a/engine/table/src/test/java/io/deephaven/engine/table/impl/FuzzerTest.java +++ b/engine/table/src/test/java/io/deephaven/engine/table/impl/FuzzerTest.java @@ -124,7 +124,7 @@ private void testFuzzerScriptFile(final long timeSeed, String s, boolean realtim clock.now += DateTimeUtils.SECOND / 10 * timeRandom.nextInt(20); } - final TimeTable timeTable = (TimeTable) session.getVariable("tt"); + final TimeTable timeTable = session.getQueryScope().readParamValue("tt"); final int steps = TstUtils.SHORT_TESTS ? 20 : 100; @@ -164,7 +164,7 @@ public void testInterestingFuzzerSeeds() throws IOException, InterruptedExceptio validateBindingTables(session, hardReferences); final ControlledUpdateGraph updateGraph = ExecutionContext.getContext().getUpdateGraph().cast(); - final TimeTable timeTable = (TimeTable) session.getVariable("tt"); + final TimeTable timeTable = session.getQueryScope().readParamValue("tt"); for (int step = 0; step < fuzzDescriptor.steps; ++step) { final int fstep = step; updateGraph.runWithinUnitTestCycle(() -> { @@ -262,7 +262,7 @@ private void runLargeFuzzerSetWithSeed(long mainTestSeed, int firstRun, int last final long startTime = System.currentTimeMillis(); final long loopStart = System.currentTimeMillis(); - final TimeTable timeTable = (TimeTable) session.getVariable("tt"); + final TimeTable timeTable = session.getQueryScope().readParamValue("tt"); final RuntimeMemory.Sample sample = new RuntimeMemory.Sample(); final ControlledUpdateGraph updateGraph = ExecutionContext.getContext().getUpdateGraph().cast(); for (int step = 0; step < stepsToRun; ++step) { @@ -316,7 +316,7 @@ private void annotateBinding(GroovyDeephavenSession session) { private void addPrintListener( GroovyDeephavenSession session, final String variable, Map hardReferences) { - final Table table = (Table) session.getVariable(variable); + final Table table = session.getQueryScope().readParamValue(variable); System.out.println(variable); TableTools.showWithRowSet(table); System.out.println(); diff --git a/engine/table/src/test/java/io/deephaven/engine/util/scripts/TestGroovyDeephavenSession.java b/engine/table/src/test/java/io/deephaven/engine/util/scripts/TestGroovyDeephavenSession.java index 47926a61860..ddcffbfe219 100644 --- a/engine/table/src/test/java/io/deephaven/engine/util/scripts/TestGroovyDeephavenSession.java +++ b/engine/table/src/test/java/io/deephaven/engine/util/scripts/TestGroovyDeephavenSession.java @@ -65,7 +65,7 @@ public void teardown() { public T fetch(final String name, final Class clazz) { // note var is guaranteed to be non-null - final Object var = session.getVariable(name); + final Object var = session.getQueryScope().readParamValue(name); if (clazz.isAssignableFrom(var.getClass())) { // noinspection unchecked return (T) var; @@ -149,14 +149,14 @@ public void testGroovyClassReload() throws IOException { session.evaluateScript( "public class MyClass1 { public static int getMyVar() { return 42 ; } };\n ExecutionContext.getContext().getQueryLibrary().importClass(MyClass1);\nt = emptyTable(1).update(\"Var=MyClass1.getMyVar()\");\n", "Script1").throwIfError(); - final Table t = (Table) session.getVariable("t"); + final Table t = session.getQueryScope().readParamValue("t"); final int var1 = t.getColumnSource("Var").getInt(0); assertEquals(42, var1); session.evaluateScript( "public class MyClass1 { public static int getMyVar() { return 43 ; } };\n ExecutionContext.getContext().getQueryLibrary().importClass(MyClass1);\n t2 = emptyTable(1).update(\"Var=MyClass1.getMyVar()\");\n", "Script2").throwIfError(); - final Table t2 = (Table) session.getVariable("t2"); + final Table t2 = session.getQueryScope().readParamValue("t2"); final int var2 = t2.getColumnSource("Var").getInt(0); assertEquals(43, var2); @@ -166,7 +166,7 @@ public void testGroovyClassReload() throws IOException { session.evaluateScript("ExecutionContext.getContext().getQueryLibrary().importClass(" + getClass().getCanonicalName() + ".class);\n t3 = emptyTable(1).update(\"Var=" + getClass().getCanonicalName() + ".VALUE_FOR_IMPORT\");\n", "Script3").throwIfError(); - final Table t3 = (Table) session.getVariable("t3"); + final Table t3 = session.getQueryScope().readParamValue("t3"); final int var3 = t3.getColumnSource("Var").getInt(0); assertEquals(VALUE_FOR_IMPORT, var3); } @@ -454,7 +454,7 @@ public void testPotentialAmbiguousMethodCalls() { try { c = "primMin = min(" + Arrays.toString(a).substring(1, Arrays.toString(a).length() - 1) + ");\n"; session.evaluateScript(c).throwIfError(); - Integer primMin = (Integer) session.getVariable("primMin"); + Integer primMin = session.getQueryScope().readParamValue("primMin"); assertEquals(Numeric.min(a), primMin.intValue()); } catch (Exception e) { e.printStackTrace(); @@ -466,7 +466,7 @@ public void testPotentialAmbiguousMethodCalls() { c = "z = " + z + "; \n" + "d = " + d + "; \n" + "wrapMin = min(" + Arrays.toString(a).substring(1, Arrays.toString(a).length() - 1) + ", z);\n"; session.evaluateScript(c).throwIfError(); - Integer wrapperMin = (Integer) session.getVariable("wrapMin"); + Integer wrapperMin = session.getQueryScope().readParamValue("wrapMin"); assertEquals(Math.min(Numeric.min(a), z), wrapperMin.intValue()); } catch (Exception e) { e.printStackTrace(); @@ -477,7 +477,7 @@ public void testPotentialAmbiguousMethodCalls() { c = "z = " + z + "; \n" + "d = " + d + "; \n" + "m2 = max(" + Arrays.toString(a).substring(1, Arrays.toString(a).length() - 1) + ", z, d);\n"; session.evaluateScript(c).throwIfError(); - Double wrapperMax = (Double) session.getVariable("m2"); + Double wrapperMax = session.getQueryScope().readParamValue("m2"); assertEquals(5.0d, wrapperMax, 0.0d); } catch (Exception e) { e.printStackTrace(); @@ -488,7 +488,7 @@ public void testPotentialAmbiguousMethodCalls() { try { QueryScope.addParam("z", z); session.evaluateScript(c).throwIfError(); - final Table t = (Table) session.getVariable("t"); + final Table t = session.getQueryScope().readParamValue("t"); final int var2 = t.getColumnSource("Z").getInt(0); assertEquals(Numeric.min(Y, z), var2); } catch (Exception e) { @@ -500,7 +500,7 @@ public void testPotentialAmbiguousMethodCalls() { try { QueryScope.addParam("z", z); session.evaluateScript(c).throwIfError(); - final Table t = (Table) session.getVariable("t"); + final Table t = session.getQueryScope().readParamValue("t"); final double var2 = t.getColumnSource("Z").getDouble(0); assertEquals(Numeric.min(Y, 5d), var2, 1e-10); } catch (Exception e) { @@ -513,7 +513,7 @@ public void testPotentialAmbiguousMethodCalls() { QueryScope.addParam("z", z); QueryScope.addParam("d", d); session.evaluateScript(c).throwIfError(); - final Table t = (Table) session.getVariable("t"); + final Table t = session.getQueryScope().readParamValue("t"); final double var2 = t.getColumnSource("Z").getDouble(0); assertEquals(Numeric.min(Y, d), var2, 1e-10); } catch (Exception e) { @@ -525,7 +525,7 @@ public void testPotentialAmbiguousMethodCalls() { try { QueryScope.addParam("z", z); session.evaluateScript(c).throwIfError(); - final Table t = (Table) session.getVariable("t"); + final Table t = session.getQueryScope().readParamValue("t"); final int var2 = t.getColumnSource("Z").getInt(0); assertEquals(Numeric.max(Y, z), var2); } catch (Exception e) { @@ -538,7 +538,7 @@ public void testPotentialAmbiguousMethodCalls() { try { QueryScope.addParam("z", z); session.evaluateScript(c).throwIfError(); - final Table t = (Table) session.getVariable("t"); + final Table t = session.getQueryScope().readParamValue("t"); final double var2 = t.getColumnSource("Z").getDouble(0); assertEquals(Numeric.max(Y, 5d), var2, 1e-10); } catch (Exception e) { @@ -551,7 +551,7 @@ public void testPotentialAmbiguousMethodCalls() { QueryScope.addParam("z", z); QueryScope.addParam("d", d); session.evaluateScript(c).throwIfError(); - final Table t = (Table) session.getVariable("t"); + final Table t = session.getQueryScope().readParamValue("t"); final double var2 = t.getColumnSource("Z").getDouble(0); assertEquals(Numeric.max(Y, d), var2, 1e-10); } catch (Exception e) { @@ -563,7 +563,7 @@ public void testPotentialAmbiguousMethodCalls() { try { QueryScope.addParam("z", z); session.evaluateScript(c).throwIfError(); - final Table t = (Table) session.getVariable("t"); + final Table t = session.getQueryScope().readParamValue("t"); final int[] var2 = t.getColumnSource("Z", int[].class).get(0); assertArrayEquals(Sort.sort(Y, z), var2); } catch (Exception e) { @@ -578,7 +578,7 @@ public void testPotentialAmbiguousMethodCalls() { // QueryScope.addParam("z", z); // QueryScope.addParam("d", d); // session.evaluateScript(c).throwIfError(); - // final Table t = (Table) session.getVariable("t"); + // final Table t = session.getQueryScope().readParamValue("t"); // final Comparable[] var2 = t.getColumnSource("Z", Comparable[].class).get(0); // // noinspection unchecked // assertArrayEquals(Sort.sortObj(Y, d), var2); @@ -591,7 +591,7 @@ public void testPotentialAmbiguousMethodCalls() { try { QueryScope.addParam("z", z); session.evaluateScript(c).throwIfError(); - final Table t = (Table) session.getVariable("t"); + final Table t = session.getQueryScope().readParamValue("t"); final int[] var2 = t.getColumnSource("Z", int[].class).get(0); assertArrayEquals(Sort.sortDescending(Y, z), var2); } catch (Exception e) { @@ -628,7 +628,7 @@ public void testPotentialAmbiguousMethodCalls() { public void testMinInFormula() { QueryScope.addParam("d", 5d); session.evaluateScript("t = emptyTable(1).updateView(\"Y=1\", \"Z=min(Y,d)\")\n").throwIfError(); - final Table t = (Table) session.getVariable("t"); + final Table t = session.getQueryScope().readParamValue("t"); } } diff --git a/java-client/session-dagger/src/test/java/io/deephaven/client/ObjectServiceTest.java b/java-client/session-dagger/src/test/java/io/deephaven/client/ObjectServiceTest.java index 53a27e14a03..03a3401b840 100644 --- a/java-client/session-dagger/src/test/java/io/deephaven/client/ObjectServiceTest.java +++ b/java-client/session-dagger/src/test/java/io/deephaven/client/ObjectServiceTest.java @@ -16,6 +16,7 @@ import io.deephaven.client.impl.TableObject; import io.deephaven.client.impl.TypedTicket; import io.deephaven.client.impl.UnknownObject; +import io.deephaven.engine.context.ExecutionContext; import io.deephaven.engine.table.Table; import io.deephaven.engine.util.ScriptSession; import io.deephaven.engine.util.TableTools; @@ -94,7 +95,7 @@ public MyObjects(Table table, Figure customObject) { @Test public void fetchable() throws ExecutionException, InterruptedException, TimeoutException, InvalidProtocolBufferException, TableHandleException { - getScriptSession().setVariable("my_objects", myObjects()); + ExecutionContext.getContext().getQueryScope().putParam("my_objects", myObjects()); final TypedTicket tt = new TypedTicket(MyObjectsObjectType.NAME, new ScopeId("my_objects")); try ( final Fetchable fetchable = session.fetchable(tt).get(5, TimeUnit.SECONDS); @@ -106,7 +107,7 @@ public void fetchable() throws ExecutionException, InterruptedException, Timeout @Test public void fetch() throws ExecutionException, InterruptedException, TimeoutException, InvalidProtocolBufferException, TableHandleException { - getScriptSession().setVariable("my_objects", myObjects()); + ExecutionContext.getContext().getQueryScope().putParam("my_objects", myObjects()); final TypedTicket tt = new TypedTicket(MyObjectsObjectType.NAME, new ScopeId("my_objects")); try (final ServerData dataAndExports = session.fetch(tt).get(5, TimeUnit.SECONDS)) { checkMyObject(dataAndExports); @@ -115,9 +116,8 @@ public void fetch() throws ExecutionException, InterruptedException, TimeoutExce @Test public void bidirectional() throws InterruptedException, ExecutionException, TimeoutException { - final ScriptSession scriptSession = getScriptSession(); - scriptSession.setVariable("my_echo", EchoObjectType.INSTANCE); - scriptSession.setVariable("my_objects", myObjects()); + ExecutionContext.getContext().getQueryScope().putParam("my_echo", EchoObjectType.INSTANCE); + ExecutionContext.getContext().getQueryScope().putParam("my_objects", myObjects()); final TypedTicket echo = new TypedTicket(EchoObjectType.NAME, new ScopeId("my_echo")); final TypedTicket myObjects = new TypedTicket(MyObjectsObjectType.NAME, new ScopeId("my_objects")); try ( @@ -131,9 +131,8 @@ public void bidirectional() throws InterruptedException, ExecutionException, Tim @Test public void messageStream() throws InterruptedException { - final ScriptSession scriptSession = getScriptSession(); - scriptSession.setVariable("my_echo", EchoObjectType.INSTANCE); - scriptSession.setVariable("my_objects", myObjects()); + ExecutionContext.getContext().getQueryScope().putParam("my_echo", EchoObjectType.INSTANCE); + ExecutionContext.getContext().getQueryScope().putParam("my_objects", myObjects()); final TypedTicket echo = new TypedTicket(EchoObjectType.NAME, new ScopeId("my_echo")); final TypedTicket myObjects = new TypedTicket(MyObjectsObjectType.NAME, new ScopeId("my_objects")); final EchoHandler echoHandler = new EchoHandler(); diff --git a/open-api/lang-tools/src/main/java/io/deephaven/lang/completion/ChunkerCompleter.java b/open-api/lang-tools/src/main/java/io/deephaven/lang/completion/ChunkerCompleter.java index ca59bf2453e..258c4861c35 100644 --- a/open-api/lang-tools/src/main/java/io/deephaven/lang/completion/ChunkerCompleter.java +++ b/open-api/lang-tools/src/main/java/io/deephaven/lang/completion/ChunkerCompleter.java @@ -3,11 +3,11 @@ */ package io.deephaven.lang.completion; +import io.deephaven.engine.context.QueryScope; import io.deephaven.engine.table.ColumnDefinition; import io.deephaven.engine.table.Table; import io.deephaven.engine.table.TableDefinition; import io.deephaven.engine.context.QueryScope.MissingVariableException; -import io.deephaven.engine.util.VariableProvider; import io.deephaven.io.logger.Logger; import io.deephaven.lang.api.HasScope; import io.deephaven.lang.api.IsScope; @@ -44,16 +44,16 @@ public enum SearchDirection { public static final String PROP_SUGGEST_STATIC_METHODS = "suggest.all.static.methods"; private final Logger log; - private final VariableProvider variables; + private final QueryScope variables; private final CompletionLookups lookups; private String defaultQuoteType; private ParsedDocument doc; - public ChunkerCompleter(final Logger log, VariableProvider variables) { + public ChunkerCompleter(final Logger log, QueryScope variables) { this(log, variables, new CompletionLookups(Collections.emptySet())); } - public ChunkerCompleter(final Logger log, VariableProvider variables, CompletionLookups lookups) { + public ChunkerCompleter(final Logger log, QueryScope variables, CompletionLookups lookups) { this.log = log; this.variables = variables; this.lookups = lookups; @@ -574,7 +574,7 @@ private void assignCompletion(Collection results, Chunke // no value for this assignment... offer up anything from scope. FuzzyList sorted = new FuzzyList<>(""); - for (String varName : variables.getVariableNames()) { + for (String varName : variables.getParamNames()) { sorted.add(varName, varName); } for (String varName : sorted) { @@ -593,7 +593,7 @@ private void assignCompletion(Collection results, Chunke // Really, we should be adding all variable names like we do, then visiting all source, // removing anything which occurs later than here in source, and adding any assignments which // occur earlier in source-but-not-in-runtime-variable-pool. IDS-1517-23 - for (String varName : variables.getVariableNames()) { + for (String varName : variables.getParamNames()) { if (camelMatch(varName, startWith)) { sorted.add(varName, varName); } @@ -883,7 +883,7 @@ private void doMethodCompletion( private void doVariableCompletion(Collection results, String variablePrefix, Token replacing, CompletionRequest request) { FuzzyList sorter = new FuzzyList<>(variablePrefix); - for (String name : variables.getVariableNames()) { + for (String name : variables.getParamNames()) { if (!name.equals(variablePrefix) && camelMatch(name, variablePrefix)) { // only suggest names which are camel-case-matches (ignoring same-as-existing-variable names) sorter.add(name, name); @@ -1018,9 +1018,12 @@ private Optional> resolveScopeType( IsScope o = scope.get(0); - final Class type = variables.getVariableType(o.getName()); - if (type != null) { - return Optional.of(type); + final Object instance = variables.readParamValue(o.getName(), null); + if (instance != null) { + if (instance instanceof Table) { + return Optional.of(Table.class); + } + return Optional.of(instance.getClass()); } Optional> result = resolveScope(scope) .map(Object::getClass); @@ -1099,7 +1102,7 @@ private Optional resolveScope(List scope) { // TODO: also handle static classes / variables only present in source (code not run yet) IDS-1517-23 try { - final Object var = variables.getVariable(o.getName(), null); + final Object var = variables.readParamValue(o.getName(), null); if (var == null) { return Optional.empty(); } diff --git a/open-api/lang-tools/src/main/java/io/deephaven/lang/completion/CompletionRequest.java b/open-api/lang-tools/src/main/java/io/deephaven/lang/completion/CompletionRequest.java index 3d0ce0543e7..7c30c33f89f 100644 --- a/open-api/lang-tools/src/main/java/io/deephaven/lang/completion/CompletionRequest.java +++ b/open-api/lang-tools/src/main/java/io/deephaven/lang/completion/CompletionRequest.java @@ -4,9 +4,10 @@ package io.deephaven.lang.completion; import io.deephaven.base.verify.Require; +import io.deephaven.engine.context.QueryScope; import io.deephaven.engine.table.ColumnDefinition; +import io.deephaven.engine.table.Table; import io.deephaven.engine.table.TableDefinition; -import io.deephaven.engine.util.VariableProvider; import io.deephaven.internal.log.LoggerFactory; import io.deephaven.io.logger.Logger; import io.deephaven.lang.generated.ChunkerAssign; @@ -83,7 +84,7 @@ public CompletionRequest candidate(int index) { } public TableDefinition getTableDefinition(final ChunkerCompleter completer, final ParsedDocument doc, - VariableProvider variables, String name) { + QueryScope variables, String name) { // Each request maintains a local cache of looked-up table definitions, to avoid going to the VariableHandler // unless needed // Note that we do NOT go to the completer.getReferencedTables map at all; @@ -95,8 +96,11 @@ public TableDefinition getTableDefinition(final ChunkerCompleter completer, fina // variable exists. return localDefs.get(name); } - TableDefinition result = variables.getTableDefinition(name); - if (result == null) { + Object value = variables.readParamValue(name, null); + TableDefinition result = null; + if (value instanceof Table) { + result = ((Table) value).getDefinition(); + } else { // If the result was null, we can try to search for an assign statement that is initialized w/ something we // _can_ grok. final List assignment = completer.findAssignment(doc, this, name); diff --git a/open-api/lang-tools/src/test/groovy/io/deephaven/lang/completion/ChunkerCompleterMixin.groovy b/open-api/lang-tools/src/test/groovy/io/deephaven/lang/completion/ChunkerCompleterMixin.groovy index dee84d5d6ca..0193a6d3ffc 100644 --- a/open-api/lang-tools/src/test/groovy/io/deephaven/lang/completion/ChunkerCompleterMixin.groovy +++ b/open-api/lang-tools/src/test/groovy/io/deephaven/lang/completion/ChunkerCompleterMixin.groovy @@ -1,6 +1,6 @@ package io.deephaven.lang.completion -import io.deephaven.engine.util.VariableProvider +import io.deephaven.engine.context.QueryScope import io.deephaven.internal.log.LoggerFactory import io.deephaven.io.logger.Logger import io.deephaven.proto.backplane.script.grpc.CompletionItem @@ -10,7 +10,7 @@ import io.deephaven.lang.parse.ParsedDocument */ trait ChunkerCompleterMixin extends ChunkerParseTestMixin { - Set performSearch(ParsedDocument doc, int from, VariableProvider vars) { + Set performSearch(ParsedDocument doc, int from, QueryScope vars) { Logger log = LoggerFactory.getLogger(CompletionHandler) diff --git a/open-api/lang-tools/src/test/groovy/io/deephaven/lang/completion/ChunkerCompletionHandlerTest.groovy b/open-api/lang-tools/src/test/groovy/io/deephaven/lang/completion/ChunkerCompletionHandlerTest.groovy index 27a00a3dc04..fa07581e499 100644 --- a/open-api/lang-tools/src/test/groovy/io/deephaven/lang/completion/ChunkerCompletionHandlerTest.groovy +++ b/open-api/lang-tools/src/test/groovy/io/deephaven/lang/completion/ChunkerCompletionHandlerTest.groovy @@ -1,9 +1,10 @@ package io.deephaven.lang.completion +import io.deephaven.engine.context.QueryScope +import io.deephaven.engine.context.StandaloneQueryScope import io.deephaven.engine.context.TestExecutionContext import io.deephaven.engine.table.Table -import io.deephaven.engine.table.TableDefinition -import io.deephaven.engine.util.VariableProvider +import io.deephaven.engine.table.TableFactory import io.deephaven.internal.log.LoggerFactory import io.deephaven.io.logger.Logger import io.deephaven.lang.parse.CompletionParser @@ -49,24 +50,14 @@ class ChunkerCompletionHandlerTest extends Specification implements ChunkerCompl return this } - VariableProvider mockVars(Closure configure) { - return Mock(VariableProvider, configure) - } - @Unroll def "Complex chains of methods on tables can parse #src sanely at any index"(String src) { CompletionParser p = new CompletionParser() when: doc = p.parse(src) - VariableProvider vars = Mock(VariableProvider){ - _ * getVariableType('t') >> Table - _ * getVariableType(_) >> null - _ * getVariable(_, _) >> null - _ * getVariableNames() >> [] - _ * getTableDefinition('emptyTable') >> TableDefinition.of() - 0 * _ - } + QueryScope vars = new StandaloneQueryScope(); + vars.putParam('t', TableFactory.emptyTable(0)) then: assertAllValid(doc, src) @@ -96,13 +87,8 @@ t = emptyTable(10).update( t = emptyTable(10) u = t.''' CompletionParser p = new CompletionParser() - VariableProvider vars = Mock(VariableProvider){ - _ * getVariableType('t') >> Table - _ * getVariableType(_) >> null - _ * getVariable(_,_) >> null - _ * getVariableNames() >> [] - 0 * _ - } + QueryScope vars = new StandaloneQueryScope(); + vars.putParam('t', TableFactory.emptyTable(0)) when: doc = p.parse(src) @@ -137,10 +123,9 @@ u = t.''' doc = p.parse(src) LoggerFactory.getLogger(CompletionHandler) - VariableProvider variables = Mock(VariableProvider) { - _ * getVariableNames() >> ['emptyTable'] - 0 * _ - } + QueryScope variables = new StandaloneQueryScope(); + variables.putParam('emptyTable', TableFactory.emptyTable(0)) + when: "Cursor is at EOF, table name completion from t is returned" Set result = performSearch(doc, src.length(), variables) @@ -164,10 +149,8 @@ c = 3 p.update(uri, 1, [ makeChange(3, 0, src2) ]) doc = p.finish(uri) - VariableProvider variables = Mock(VariableProvider) { - _ * getVariableNames() >> ['emptyTable'] - 0 * _ - } + QueryScope variables = new StandaloneQueryScope(); + variables.putParam('emptyTable', TableFactory.emptyTable(0)) when: "Cursor is at EOF, table name completion from t is returned" Set result = performSearch(doc, (src1 + src2).length(), variables) @@ -194,20 +177,12 @@ b = 2 doc = p.parse(src) LoggerFactory.getLogger(CompletionHandler) - VariableProvider variables = Mock(VariableProvider) { - _ * getVariableNames() >> ['emptyTable'] - 0 * _ - } + QueryScope variables = new StandaloneQueryScope(); + variables.putParam('emptyTable', TableFactory.emptyTable(0)) when: "Cursor is in the comment after the variablename+dot and completion is requested" Set result = performSearch(doc, beforeCursor.length(), variables) then: "Expect the completion result to suggest nothing" result.size() == 0 } - @Override - VariableProvider getVariables() { - return Mock(VariableProvider) { - _ * getVariableNames() >> [] - } - } } diff --git a/open-api/lang-tools/src/test/groovy/io/deephaven/lang/completion/ChunkerGroovyTest.groovy b/open-api/lang-tools/src/test/groovy/io/deephaven/lang/completion/ChunkerGroovyTest.groovy index cd337ffbb5a..a2ed96dfb71 100644 --- a/open-api/lang-tools/src/test/groovy/io/deephaven/lang/completion/ChunkerGroovyTest.groovy +++ b/open-api/lang-tools/src/test/groovy/io/deephaven/lang/completion/ChunkerGroovyTest.groovy @@ -1,6 +1,5 @@ package io.deephaven.lang.completion -import io.deephaven.engine.util.VariableProvider import io.deephaven.lang.parse.CompletionParser import spock.lang.Specification @@ -112,10 +111,4 @@ l.add(new SomeClass().method(new Thing1.Thing2> [] - } - } } diff --git a/open-api/lang-tools/src/test/groovy/io/deephaven/lang/completion/ChunkerParseTestMixin.groovy b/open-api/lang-tools/src/test/groovy/io/deephaven/lang/completion/ChunkerParseTestMixin.groovy index c77bf009ebe..a03fce0f9e2 100644 --- a/open-api/lang-tools/src/test/groovy/io/deephaven/lang/completion/ChunkerParseTestMixin.groovy +++ b/open-api/lang-tools/src/test/groovy/io/deephaven/lang/completion/ChunkerParseTestMixin.groovy @@ -2,7 +2,7 @@ package io.deephaven.lang.completion import groovy.transform.CompileDynamic import groovy.transform.CompileStatic -import io.deephaven.engine.util.VariableProvider +import io.deephaven.engine.context.EmptyQueryScope import io.deephaven.internal.log.LoggerFactory import io.deephaven.io.logger.Logger import io.deephaven.lang.generated.Node @@ -38,8 +38,6 @@ trait ChunkerParseTestMixin { } } - abstract VariableProvider getVariables() - ParsedDocument parse(String command) { CompletionParser p = new CompletionParser(); doc = p.parse(command) @@ -57,7 +55,7 @@ trait ChunkerParseTestMixin { String doCompletion(String command, int completionPos, int resultIndex) { Logger log = LoggerFactory.getLogger(CompletionHandler) - ChunkerCompleter completer = new ChunkerCompleter(log, variables) + ChunkerCompleter completer = new ChunkerCompleter(log, EmptyQueryScope.INSTANCE) parse(command) Set results = completer.runCompletion(doc, completionPos) List result = results.toList() diff --git a/open-api/lang-tools/src/test/groovy/io/deephaven/lang/completion/ChunkerParserTest.groovy b/open-api/lang-tools/src/test/groovy/io/deephaven/lang/completion/ChunkerParserTest.groovy index 542ab36e105..e84c8e12922 100644 --- a/open-api/lang-tools/src/test/groovy/io/deephaven/lang/completion/ChunkerParserTest.groovy +++ b/open-api/lang-tools/src/test/groovy/io/deephaven/lang/completion/ChunkerParserTest.groovy @@ -1,7 +1,6 @@ package io.deephaven.lang.completion import groovy.text.SimpleTemplateEngine -import io.deephaven.engine.util.VariableProvider import io.deephaven.lang.generated.* import io.deephaven.lang.parse.CompletionParser import io.deephaven.lang.parse.LspTools @@ -222,13 +221,6 @@ t = db. q << [ '"' , "'" ] } - @Override - VariableProvider getVariables() { - return Mock(VariableProvider) { - _ * getVariableNames() >> [] - } - } - static String quotify(String str, String q) { new SimpleTemplateEngine().createTemplate(str) .make([ q: q]).toString() diff --git a/open-api/lang-tools/src/test/groovy/io/deephaven/lang/completion/ChunkerPythonTest.groovy b/open-api/lang-tools/src/test/groovy/io/deephaven/lang/completion/ChunkerPythonTest.groovy index 8ab97c7b01a..acd9781d6e0 100644 --- a/open-api/lang-tools/src/test/groovy/io/deephaven/lang/completion/ChunkerPythonTest.groovy +++ b/open-api/lang-tools/src/test/groovy/io/deephaven/lang/completion/ChunkerPythonTest.groovy @@ -1,6 +1,5 @@ package io.deephaven.lang.completion -import io.deephaven.engine.util.VariableProvider import io.deephaven.lang.parse.CompletionParser import spock.lang.Specification @@ -32,11 +31,4 @@ def typesafe(param: Type = None) -> int: assertAllValid(doc, src) } - - @Override - VariableProvider getVariables() { - return Mock(VariableProvider) { - _ * getVariableNames() >> [] - } - } } diff --git a/open-api/lang-tools/src/test/groovy/io/deephaven/lang/completion/ColumnExpressionCompletionHandlerTest.groovy b/open-api/lang-tools/src/test/groovy/io/deephaven/lang/completion/ColumnExpressionCompletionHandlerTest.groovy index a6c3cbe3031..1d7f95e55ca 100644 --- a/open-api/lang-tools/src/test/groovy/io/deephaven/lang/completion/ColumnExpressionCompletionHandlerTest.groovy +++ b/open-api/lang-tools/src/test/groovy/io/deephaven/lang/completion/ColumnExpressionCompletionHandlerTest.groovy @@ -1,19 +1,20 @@ package io.deephaven.lang.completion -import io.deephaven.base.clock.Clock +import io.deephaven.engine.context.QueryScope +import io.deephaven.engine.context.StandaloneQueryScope import io.deephaven.engine.context.TestExecutionContext; -import io.deephaven.engine.table.Table -import io.deephaven.engine.table.TableDefinition -import io.deephaven.engine.util.VariableProvider +import io.deephaven.engine.table.TableFactory import io.deephaven.internal.log.LoggerFactory import io.deephaven.io.logger.Logger import io.deephaven.lang.parse.CompletionParser import io.deephaven.proto.backplane.script.grpc.CompletionItem +import io.deephaven.qst.column.Column import io.deephaven.util.SafeCloseable import spock.lang.Specification import spock.lang.Unroll import java.time.Instant +import java.time.LocalDate class ColumnExpressionCompletionHandlerTest extends Specification implements ChunkerCompleterMixin { @@ -41,12 +42,11 @@ class ColumnExpressionCompletionHandlerTest extends Specification implements Chu doc = p.parse(src) Logger log = LoggerFactory.getLogger(CompletionHandler) - VariableProvider variables = Mock(VariableProvider) { - (0..1) * getVariableNames() >> ['t'] - (0..1) * getVariableType('t') >> Table - (0..1) * getTableDefinition('t') >> TableDefinition.from(['Date', 'DateTime'], [java.time.LocalDate, Instant] - ) - } + QueryScope variables = new StandaloneQueryScope() + variables.putParam("t", TableFactory.newTable( + Column.of('Date', LocalDate.class, new LocalDate[0]), + Column.of('DateTime', Instant.class, new Instant[0])) + ) ChunkerCompleter completer = new ChunkerCompleter(log, variables) @@ -91,17 +91,17 @@ t = t.updateView ( 'D doc = p.parse(src) Logger log = LoggerFactory.getLogger(CompletionHandler) - VariableProvider variables = Mock() { - (0..1) * getTableDefinition('t') >> TableDefinition.from( - ['Date', 'Delta', 'NotMeThough'], [String, Long, Integer]) - (0..1) * getVariableType('t') >> Table - (0..1) * getVariableNames() >> [] - } + QueryScope variables = new StandaloneQueryScope(); + variables.putParam('t', TableFactory.newTable( + Column.of('Date', String.class, new String[0]), + Column.of('Delta', Long.class, new Long[0]), + Column.of('NotMeThough', Integer.class, new Integer[0]), + )); ChunkerCompleter completer = new ChunkerCompleter(log, variables) when: "Cursor is at EOF, table name completion from t is returned" - Set result = completer.runCompletion(doc, pos) + Set result = completer.runCompletion(doc, pos) result.removeIf({it.textEdit.text == 'updateView('}) // t = t.where ( 'D @@ -131,10 +131,11 @@ t = t.update('A=') .update( 'B=') doc = p.parse(src) Logger log = LoggerFactory.getLogger(CompletionHandler) - VariableProvider variables = Mock(VariableProvider) { - _ * getTableDefinition('t') >> TableDefinition.from(['A1', 'A2'], [Long, Integer]) - 0 * _ - } + QueryScope variables = new StandaloneQueryScope() + variables.putParam('t', TableFactory.newTable( + Column.of('A1', Long.class, new Long[0]), + Column.of('A2', Integer.class, new Integer[0]), + )); ChunkerCompleter completer = new ChunkerCompleter(log, variables) @@ -188,10 +189,8 @@ t.where('""" doc = p.parse(src) Logger log = LoggerFactory.getLogger(CompletionHandler) - VariableProvider variables = Mock(VariableProvider) { - _ * getTableDefinition('t') >> null - 0 * _ - } + QueryScope variables = new StandaloneQueryScope() + variables.putParam('t', null); ChunkerCompleter completer = new ChunkerCompleter(log, variables) @@ -207,10 +206,4 @@ t.where('""" cleanup: System.clearProperty(ChunkerCompleter.PROP_SUGGEST_STATIC_METHODS) } - @Override - VariableProvider getVariables() { - return Mock(VariableProvider) { - _ * getVariableNames() >> [] - } - } } diff --git a/server/src/main/java/io/deephaven/server/console/ConsoleServiceGrpcImpl.java b/server/src/main/java/io/deephaven/server/console/ConsoleServiceGrpcImpl.java index 864d1c1ad51..2dc6bfffbd3 100644 --- a/server/src/main/java/io/deephaven/server/console/ConsoleServiceGrpcImpl.java +++ b/server/src/main/java/io/deephaven/server/console/ConsoleServiceGrpcImpl.java @@ -7,13 +7,13 @@ import com.google.rpc.Code; import io.deephaven.base.LockFreeArrayQueue; import io.deephaven.configuration.Configuration; +import io.deephaven.engine.context.ExecutionContext; import io.deephaven.engine.context.QueryScope; import io.deephaven.engine.table.Table; import io.deephaven.engine.table.impl.perf.QueryPerformanceNugget; import io.deephaven.engine.table.impl.perf.QueryPerformanceRecorder; import io.deephaven.engine.table.impl.util.RuntimeMemory; import io.deephaven.engine.table.impl.util.RuntimeMemory.Sample; -import io.deephaven.engine.updategraph.DynamicNode; import io.deephaven.engine.util.DelegatingScriptSession; import io.deephaven.engine.util.ScriptSession; import io.deephaven.extensions.barrage.util.GrpcUtil; @@ -284,13 +284,11 @@ public void bindTableToVariable( } exportBuilder.submit(() -> { - ScriptSession scriptSession = - exportedConsole != null ? exportedConsole.get() : scriptSessionProvider.get(); + QueryScope queryScope = exportedConsole != null ? exportedConsole.get().getQueryScope() + : ExecutionContext.getContext().getQueryScope(); + Table table = exportedTable.get(); - scriptSession.setVariable(request.getVariableName(), table); - if (DynamicNode.notDynamicOrIsRefreshing(table)) { - scriptSession.manage(table); - } + queryScope.putParam(request.getVariableName(), table); responseObserver.onNext(BindTableToVariableResponse.getDefaultInstance()); responseObserver.onCompleted(); }); @@ -310,7 +308,7 @@ public StreamObserver autoCompleteStream( final ScriptSession scriptSession = scriptSessionProvider.get(); scriptSession.evaluateScript( "from deephaven_internal.auto_completer import jedi_settings ; jedi_settings.set_scope(globals())"); - settings[0] = (PyObject) scriptSession.getVariable("jedi_settings"); + settings[0] = scriptSession.getQueryScope().readParamValue("jedi_settings"); } catch (Exception err) { if (!ALREADY_WARNED_ABOUT_NO_AUTOCOMPLETE.getAndSet(true)) { log.error().append("Autocomplete package not found; disabling autocomplete.").endl(); diff --git a/server/src/main/java/io/deephaven/server/console/ScopeTicketResolver.java b/server/src/main/java/io/deephaven/server/console/ScopeTicketResolver.java index 585b5534b67..ae9c58ed0ee 100644 --- a/server/src/main/java/io/deephaven/server/console/ScopeTicketResolver.java +++ b/server/src/main/java/io/deephaven/server/console/ScopeTicketResolver.java @@ -6,11 +6,9 @@ import com.google.protobuf.ByteStringAccess; import com.google.rpc.Code; import io.deephaven.base.string.EncodingInfo; +import io.deephaven.engine.context.ExecutionContext; import io.deephaven.engine.context.QueryScope; -import io.deephaven.engine.liveness.LivenessReferent; import io.deephaven.engine.table.Table; -import io.deephaven.engine.updategraph.DynamicNode; -import io.deephaven.engine.util.ScriptSession; import io.deephaven.proto.backplane.grpc.Ticket; import io.deephaven.proto.flight.util.TicketRouterHelper; import io.deephaven.proto.util.ByteHelper; @@ -24,7 +22,6 @@ import org.jetbrains.annotations.Nullable; import javax.inject.Inject; -import javax.inject.Provider; import javax.inject.Singleton; import java.nio.ByteBuffer; import java.nio.charset.CharacterCodingException; @@ -37,14 +34,10 @@ @Singleton public class ScopeTicketResolver extends TicketResolverBase { - private final Provider scriptSessionProvider; - @Inject public ScopeTicketResolver( - final AuthorizationProvider authProvider, - final Provider globalSessionProvider) { + final AuthorizationProvider authProvider) { super(authProvider, (byte) TICKET_PREFIX, FLIGHT_DESCRIPTOR_ROUTE); - this.scriptSessionProvider = globalSessionProvider; } @Override @@ -58,29 +51,29 @@ public SessionState.ExportObject flightInfoFor( // there is no mechanism to wait for a scope variable to resolve; require that the scope variable exists now final String scopeName = nameForDescriptor(descriptor, logId); - final ScriptSession gss = scriptSessionProvider.get(); - final Flight.FlightInfo flightInfo = - gss.getExecutionContext().getUpdateGraph().sharedLock().computeLocked(() -> { - Object scopeVar = gss.getVariable(scopeName, null); - if (scopeVar == null) { - throw Exceptions.statusRuntimeException(Code.NOT_FOUND, - "Could not resolve '" + logId + ": no variable exists with name '" + scopeName + "'"); - } - if (scopeVar instanceof Table) { - scopeVar = authorization.transform(scopeVar); - return TicketRouter.getFlightInfo((Table) scopeVar, descriptor, flightTicketForName(scopeName)); - } + QueryScope queryScope = ExecutionContext.getContext().getQueryScope(); + Object scopeVar = queryScope.unwrapObject(queryScope.readParamValue(scopeName, null)); + if (scopeVar == null) { + throw Exceptions.statusRuntimeException(Code.NOT_FOUND, + "Could not resolve '" + logId + ": no table exists with name '" + scopeName + "'"); + } + if (!(scopeVar instanceof Table)) { + throw Exceptions.statusRuntimeException(Code.NOT_FOUND, + "Could not resolve '" + logId + "': no table exists with name '" + scopeName + "'"); + } - throw Exceptions.statusRuntimeException(Code.NOT_FOUND, - "Could not resolve '" + logId + "': no variable exists with name '" + scopeName + "'"); - }); + Table transformed = authorization.transform((Table) scopeVar); + Flight.FlightInfo flightInfo = + TicketRouter.getFlightInfo(transformed, descriptor, flightTicketForName(scopeName)); return SessionState.wrapAsExport(flightInfo); } @Override public void forAllFlightInfo(@Nullable final SessionState session, final Consumer visitor) { - scriptSessionProvider.get().getVariables().forEach((varName, varObj) -> { + QueryScope queryScope = ExecutionContext.getContext().getQueryScope(); + queryScope.toMap().forEach((varName, varObj) -> { + varObj = queryScope.unwrapObject(varObj); if (varObj instanceof Table) { visitor.accept(TicketRouter.getFlightInfo((Table) varObj, descriptorForName(varName), flightTicketForName(varName))); @@ -102,17 +95,14 @@ public SessionState.ExportObject resolve( private SessionState.ExportObject resolve( @Nullable final SessionState session, final String scopeName, final String logId) { - final ScriptSession gss = scriptSessionProvider.get(); // fetch the variable from the scope right now - T export = gss.getExecutionContext().getUpdateGraph().sharedLock().computeLocked(() -> { - T scopeVar = null; - try { - // noinspection unchecked - scopeVar = (T) gss.unwrapObject(gss.getVariable(scopeName)); - } catch (QueryScope.MissingVariableException ignored) { - } - return scopeVar; - }); + T export = null; + try { + QueryScope queryScope = ExecutionContext.getContext().getQueryScope(); + // noinspection unchecked + export = (T) queryScope.unwrapObject(queryScope.readParamValue(scopeName)); + } catch (QueryScope.MissingVariableException ignored) { + } export = authorization.transform(export); @@ -157,12 +147,9 @@ private SessionState.ExportBuilder publish( .requiresSerialQueue() .require(resultExport) .submit(() -> { - final ScriptSession gss = scriptSessionProvider.get(); T value = resultExport.get(); - if (value instanceof LivenessReferent && DynamicNode.notDynamicOrIsRefreshing(value)) { - gss.manage((LivenessReferent) value); - } - gss.setVariable(varName, value); + ExecutionContext.getContext().getQueryScope().putParam(varName, value); + if (onPublish != null) { onPublish.run(); } diff --git a/server/src/main/java/io/deephaven/server/console/completer/JavaAutoCompleteObserver.java b/server/src/main/java/io/deephaven/server/console/completer/JavaAutoCompleteObserver.java index dfe76a3f095..338957936be 100644 --- a/server/src/main/java/io/deephaven/server/console/completer/JavaAutoCompleteObserver.java +++ b/server/src/main/java/io/deephaven/server/console/completer/JavaAutoCompleteObserver.java @@ -1,8 +1,8 @@ package io.deephaven.server.console.completer; import com.google.rpc.Code; +import io.deephaven.engine.context.QueryScope; import io.deephaven.engine.util.ScriptSession; -import io.deephaven.engine.util.VariableProvider; import io.deephaven.internal.log.LoggerFactory; import io.deephaven.io.logger.Logger; import io.deephaven.lang.completion.ChunkerCompleter; @@ -172,7 +172,7 @@ private GetCompletionItemsResponse getCompletionItems(GetCompletionItemsRequest SessionState.ExportObject exportedConsole, CompletionParser parser, StreamObserver responseObserver) { final ScriptSession scriptSession = exportedConsole.get(); - final VariableProvider vars = scriptSession.getVariableProvider(); + final QueryScope vars = scriptSession.getQueryScope(); final VersionedTextDocumentIdentifier doc = request.getTextDocument(); final CompletionLookups h = CompletionLookups.preload(scriptSession, customCompletionFactory); // The only stateful part of a completer is the CompletionLookups, which are already diff --git a/server/src/main/java/io/deephaven/server/console/completer/PythonAutoCompleteObserver.java b/server/src/main/java/io/deephaven/server/console/completer/PythonAutoCompleteObserver.java index 2a26905e474..4f44caa6ff3 100644 --- a/server/src/main/java/io/deephaven/server/console/completer/PythonAutoCompleteObserver.java +++ b/server/src/main/java/io/deephaven/server/console/completer/PythonAutoCompleteObserver.java @@ -11,14 +11,12 @@ import io.deephaven.server.console.ConsoleServiceGrpcImpl; import io.deephaven.server.session.SessionCloseableObserver; import io.deephaven.server.session.SessionState; -import io.deephaven.util.SafeCloseable; import io.grpc.stub.StreamObserver; import org.jpy.PyObject; import javax.inject.Provider; import java.util.ArrayList; import java.util.List; -import java.util.stream.Collectors; import static io.deephaven.extensions.barrage.util.GrpcUtil.safelyComplete; import static io.deephaven.extensions.barrage.util.GrpcUtil.safelyOnNext; @@ -49,7 +47,8 @@ public void onNext(AutoCompleteRequest value) { switch (value.getRequestCase()) { case OPEN_DOCUMENT: { final TextDocumentItem doc = value.getOpenDocument().getTextDocument(); - PyObject completer = (PyObject) scriptSession.get().getVariable("jedi_settings"); + PyObject completer = + (PyObject) scriptSession.get().getQueryScope().readParamValue("jedi_settings"); completer.callMethod("open_doc", doc.getText(), doc.getUri(), doc.getVersion()); break; } @@ -57,7 +56,8 @@ public void onNext(AutoCompleteRequest value) { ChangeDocumentRequest request = value.getChangeDocument(); final VersionedTextDocumentIdentifier text = request.getTextDocument(); - PyObject completer = (PyObject) scriptSession.get().getVariable("jedi_settings"); + PyObject completer = + (PyObject) scriptSession.get().getQueryScope().readParamValue("jedi_settings"); String uri = text.getUri(); int version = text.getVersion(); String document = completer.callMethod("get_doc", text.getUri()).getStringValue(); @@ -75,7 +75,8 @@ public void onNext(AutoCompleteRequest value) { break; } case CLOSE_DOCUMENT: { - PyObject completer = (PyObject) scriptSession.get().getVariable("jedi_settings"); + PyObject completer = + (PyObject) scriptSession.get().getQueryScope().readParamValue("jedi_settings"); CloseDocumentRequest request = value.getCloseDocument(); completer.callMethod("close_doc", request.getTextDocument().getUri()); break; @@ -110,7 +111,7 @@ private void handleAutocompleteRequest(AutoCompleteRequest request, request.getRequestId() > 0 ? request.getRequestId() : request.getGetCompletionItems().getRequestId(); try { final ScriptSession scriptSession = exportedConsole.get(); - PyObject completer = (PyObject) scriptSession.getVariable("jedi_settings"); + PyObject completer = scriptSession.getQueryScope().readParamValue("jedi_settings"); boolean canJedi = completer.callMethod("is_enabled").getBooleanValue(); if (!canJedi) { log.trace().append("Ignoring completion request because jedi is disabled").endl(); diff --git a/server/src/main/java/io/deephaven/server/uri/QueryScopeResolver.java b/server/src/main/java/io/deephaven/server/uri/QueryScopeResolver.java index 0a89d98b5c8..6b9839a753b 100644 --- a/server/src/main/java/io/deephaven/server/uri/QueryScopeResolver.java +++ b/server/src/main/java/io/deephaven/server/uri/QueryScopeResolver.java @@ -3,17 +3,15 @@ */ package io.deephaven.server.uri; -import io.deephaven.engine.util.ScriptSession; +import io.deephaven.engine.context.ExecutionContext; import io.deephaven.uri.DeephavenUri; import io.deephaven.uri.QueryScopeUri; import io.deephaven.uri.resolver.UriResolver; import io.deephaven.uri.resolver.UriResolversInstance; import javax.inject.Inject; -import javax.inject.Provider; import java.net.URI; import java.util.Collections; -import java.util.Objects; import java.util.Set; /** @@ -30,12 +28,8 @@ public static QueryScopeResolver get() { return UriResolversInstance.get().find(QueryScopeResolver.class).get(); } - private final Provider globalSessionProvider; - @Inject - public QueryScopeResolver(Provider globalSessionProvider) { - this.globalSessionProvider = Objects.requireNonNull(globalSessionProvider); - } + public QueryScopeResolver() {} @Override public Set schemes() { @@ -57,6 +51,6 @@ public Object resolve(QueryScopeUri uri) { } public Object resolve(String variableName) { - return globalSessionProvider.get().getVariable(variableName, null); + return ExecutionContext.getContext().getQueryScope().readParamValue(variableName, null); } } diff --git a/server/src/test/java/io/deephaven/server/appmode/ApplicationServiceGrpcImplTest.java b/server/src/test/java/io/deephaven/server/appmode/ApplicationServiceGrpcImplTest.java index 204581417b6..3d9d5edf847 100644 --- a/server/src/test/java/io/deephaven/server/appmode/ApplicationServiceGrpcImplTest.java +++ b/server/src/test/java/io/deephaven/server/appmode/ApplicationServiceGrpcImplTest.java @@ -89,7 +89,7 @@ public void onListFieldsSubscribeFailedObserver() { ScriptSession scriptSession = new NoLanguageDeephavenSession( ExecutionContext.getContext().getUpdateGraph(), ExecutionContext.getContext().getOperationInitializer()); - scriptSession.setVariable("key", "hello world"); + scriptSession.getQueryScope().putParam("key", "hello world"); ScriptSession.Changes changes = new ScriptSession.Changes(); changes.created.put("key", "Object"); applicationServiceGrpcImpl.onScopeChanges(scriptSession, changes); diff --git a/server/src/test/java/io/deephaven/server/appmode/ApplicationTest.java b/server/src/test/java/io/deephaven/server/appmode/ApplicationTest.java index be57f02c551..af0ad09ee98 100644 --- a/server/src/test/java/io/deephaven/server/appmode/ApplicationTest.java +++ b/server/src/test/java/io/deephaven/server/appmode/ApplicationTest.java @@ -29,12 +29,11 @@ public class ApplicationTest { @Rule public final EngineCleanup base = new EngineCleanup(); - private AbstractScriptSession session = null; + private AbstractScriptSession session = null; @After public void tearDown() { if (session != null) { - session.release(); session = null; } } diff --git a/server/src/test/java/io/deephaven/server/runner/DeephavenApiServerTestBase.java b/server/src/test/java/io/deephaven/server/runner/DeephavenApiServerTestBase.java index d3c683ce331..f457e5028b6 100644 --- a/server/src/test/java/io/deephaven/server/runner/DeephavenApiServerTestBase.java +++ b/server/src/test/java/io/deephaven/server/runner/DeephavenApiServerTestBase.java @@ -88,7 +88,8 @@ interface Builder { @Rule public final GrpcCleanupRule grpcCleanup = new GrpcCleanupRule(); - private ExecutionContext executionContext; + @Inject + ExecutionContext executionContext; private SafeCloseable executionContextCloseable; private LogBuffer logBuffer; @@ -131,7 +132,6 @@ public void setUp() throws Exception { .injectFields(this); final PeriodicUpdateGraph updateGraph = server.getUpdateGraph().cast(); - executionContext = TestExecutionContext.createForUnitTests().withUpdateGraph(updateGraph); executionContextCloseable = executionContext.open(); if (updateGraph.isUnitTestModeAllowed()) { updateGraph.enableUnitTestMode(); diff --git a/server/test/src/main/java/io/deephaven/server/test/FlightMessageRoundTripTest.java b/server/test/src/main/java/io/deephaven/server/test/FlightMessageRoundTripTest.java index f485cfb5d85..a60bc421d8a 100644 --- a/server/test/src/main/java/io/deephaven/server/test/FlightMessageRoundTripTest.java +++ b/server/test/src/main/java/io/deephaven/server/test/FlightMessageRoundTripTest.java @@ -202,14 +202,10 @@ public interface TestComponent { SessionService sessionService(); - AbstractScriptSession scriptSession(); - GrpcServer server(); TestAuthModule.BasicAuthTestImpl basicAuthHandler(); - Map authRequestHandlers(); - ExecutionContext executionContext(); TestAuthorizationProvider authorizationProvider(); @@ -225,7 +221,6 @@ public interface TestComponent { protected SessionService sessionService; private SessionState currentSession; - private AbstractScriptSession scriptSession; private SafeCloseable executionContext; private Location serverLocation; protected TestComponent component; @@ -252,7 +247,6 @@ public void setup() throws IOException { server.start(); localPort = server.getPort(); - scriptSession = component.scriptSession(); sessionService = component.sessionService(); serverLocation = Location.forGrpcInsecure("localhost", localPort); @@ -311,7 +305,6 @@ public void teardown() throws InterruptedException { clientChannel.shutdownNow(); sessionService.closeAllSessions(); - scriptSession.release(); executionContext.close(); closeClient(); @@ -374,7 +367,7 @@ public void testLoginHandshakeBasicAuth() { .allocator(new RootAllocator()) .build(); - scriptSession.setVariable("test", TableTools.emptyTable(10).update("I=i")); + ExecutionContext.getContext().getQueryScope().putParam("test", TableTools.emptyTable(10).update("I=i")); // do get cannot be invoked by unauthenticated user final Ticket ticket = new Ticket("s/test".getBytes(StandardCharsets.UTF_8)); @@ -395,7 +388,7 @@ public void testLoginHeaderBasicAuth() { .allocator(new RootAllocator()) .build(); - scriptSession.setVariable("test", TableTools.emptyTable(10).update("I=i")); + ExecutionContext.getContext().getQueryScope().putParam("test", TableTools.emptyTable(10).update("I=i")); // do get cannot be invoked by unauthenticated user final Ticket ticket = new Ticket("s/test".getBytes(StandardCharsets.UTF_8)); @@ -416,7 +409,7 @@ public void testLoginHeaderBasicAuth() { @Test public void testLoginHeaderCustomBearer() { closeClient(); - scriptSession.setVariable("test", TableTools.emptyTable(10).update("I=i")); + ExecutionContext.getContext().getQueryScope().putParam("test", TableTools.emptyTable(10).update("I=i")); // add the bearer token override final MutableBoolean tokenChanged = new MutableBoolean(); @@ -456,7 +449,7 @@ public void testLoginHandshakeAnonymous() { .allocator(new RootAllocator()) .build(); - scriptSession.setVariable("test", TableTools.emptyTable(10).update("I=i")); + ExecutionContext.getContext().getQueryScope().putParam("test", TableTools.emptyTable(10).update("I=i")); // do get cannot be invoked by unauthenticated user final Ticket ticket = new Ticket("s/test".getBytes(StandardCharsets.UTF_8)); @@ -491,7 +484,7 @@ public void testLoginHeaderAnonymous() { final String ANONYMOUS = "Anonymous"; closeClient(); - scriptSession.setVariable("test", TableTools.emptyTable(10).update("I=i")); + ExecutionContext.getContext().getQueryScope().putParam("test", TableTools.emptyTable(10).update("I=i")); final MutableBoolean tokenChanged = new MutableBoolean(); flightClient = FlightClient.builder().location(serverLocation) @@ -660,8 +653,8 @@ public void testFlightInfo() { () -> TableTools.timeTable(1_000_000).update("I = i")); // stuff table into the scope - scriptSession.setVariable(staticTableName, table); - scriptSession.setVariable(tickingTableName, tickingTable); + ExecutionContext.getContext().getQueryScope().putParam(staticTableName, table); + ExecutionContext.getContext().getQueryScope().putParam(tickingTableName, tickingTable); // test fetch info from scoped ticket assertInfoMatchesTable(flightClient.getInfo(arrowFlightDescriptorForName(staticTableName)), table); @@ -690,30 +683,28 @@ public void testGetSchema() { final Table tickingTable = ExecutionContext.getContext().getUpdateGraph().sharedLock().computeLocked( () -> TableTools.timeTable(1_000_000).update("I = i")); - try (final SafeCloseable ignored = LivenessScopeStack.open(scriptSession, false)) { - // stuff table into the scope - scriptSession.setVariable(staticTableName, table); - scriptSession.setVariable(tickingTableName, tickingTable); - - // test fetch info from scoped ticket - assertSchemaMatchesTable(flightClient.getSchema(arrowFlightDescriptorForName(staticTableName)).getSchema(), - table); - assertSchemaMatchesTable(flightClient.getSchema(arrowFlightDescriptorForName(tickingTableName)).getSchema(), - tickingTable); - - // test list flights which runs through scoped tickets - final MutableInt seenTables = new MutableInt(); - flightClient.listFlights(Criteria.ALL).forEach(fi -> { - seenTables.increment(); - if (fi.getDescriptor().equals(arrowFlightDescriptorForName(staticTableName))) { - assertInfoMatchesTable(fi, table); - } else { - assertInfoMatchesTable(fi, tickingTable); - } - }); + // stuff table into the scope + ExecutionContext.getContext().getQueryScope().putParam(staticTableName, table); + ExecutionContext.getContext().getQueryScope().putParam(tickingTableName, tickingTable); - Assert.eq(seenTables.intValue(), "seenTables.intValue()", 2); - } + // test fetch info from scoped ticket + assertSchemaMatchesTable(flightClient.getSchema(arrowFlightDescriptorForName(staticTableName)).getSchema(), + table); + assertSchemaMatchesTable(flightClient.getSchema(arrowFlightDescriptorForName(tickingTableName)).getSchema(), + tickingTable); + + // test list flights which runs through scoped tickets + final MutableInt seenTables = new MutableInt(); + flightClient.listFlights(Criteria.ALL).forEach(fi -> { + seenTables.increment(); + if (fi.getDescriptor().equals(arrowFlightDescriptorForName(staticTableName))) { + assertInfoMatchesTable(fi, table); + } else { + assertInfoMatchesTable(fi, tickingTable); + } + }); + + Assert.eq(seenTables.intValue(), "seenTables.intValue()", 2); } @Test @@ -721,74 +712,72 @@ public void testDoExchangeSnapshot() throws Exception { final String staticTableName = "flightInfoTest"; final Table table = TableTools.emptyTable(10).update("I = i", "J = i + 0.01"); - try (final SafeCloseable ignored = LivenessScopeStack.open(scriptSession, false)) { - // stuff table into the scope - scriptSession.setVariable(staticTableName, table); - - // build up a snapshot request - byte[] magic = new byte[] {100, 112, 104, 110}; // equivalent to '0x6E687064' (ASCII "dphn") - - FlightDescriptor fd = FlightDescriptor.command(magic); - - try (FlightClient.ExchangeReaderWriter erw = flightClient.doExchange(fd); - final RootAllocator allocator = new RootAllocator(Integer.MAX_VALUE)) { - - final FlatBufferBuilder metadata = new FlatBufferBuilder(); - - // use 0 for batch size and max message size to use server-side defaults - int optOffset = - BarrageSnapshotOptions.createBarrageSnapshotOptions(metadata, ColumnConversionMode.Stringify, - false, 0, 0); - - final int ticOffset = - BarrageSnapshotRequest.createTicketVector(metadata, - ScopeTicketHelper.nameToBytes(staticTableName)); - BarrageSnapshotRequest.startBarrageSnapshotRequest(metadata); - BarrageSnapshotRequest.addColumns(metadata, 0); - BarrageSnapshotRequest.addViewport(metadata, 0); - BarrageSnapshotRequest.addSnapshotOptions(metadata, optOffset); - BarrageSnapshotRequest.addTicket(metadata, ticOffset); - metadata.finish(BarrageSnapshotRequest.endBarrageSnapshotRequest(metadata)); - - final FlatBufferBuilder wrapper = new FlatBufferBuilder(); - final int innerOffset = wrapper.createByteVector(metadata.dataBuffer()); - wrapper.finish(BarrageMessageWrapper.createBarrageMessageWrapper( - wrapper, - 0x6E687064, // the numerical representation of the ASCII "dphn". - BarrageMessageType.BarrageSnapshotRequest, - innerOffset)); - - // extract the bytes and package them in an ArrowBuf for transmission - byte[] msg = wrapper.sizedByteArray(); - ArrowBuf data = allocator.buffer(msg.length); - data.writeBytes(msg); - - erw.getWriter().putMetadata(data); - erw.getWriter().completed(); - - // read everything from the server (expecting schema message and one data message) - int totalRowCount = 0; - while (erw.getReader().next()) { - final int offset = totalRowCount; - final VectorSchemaRoot root = erw.getReader().getRoot(); - final int rowCount = root.getRowCount(); - totalRowCount += rowCount; - - // check the values against the source table - org.apache.arrow.vector.IntVector iv = - (org.apache.arrow.vector.IntVector) root.getVector(0); - for (int i = 0; i < rowCount; ++i) { - assertEquals("int match:", DataAccessHelpers.getColumn(table, 0).get(offset + i), iv.get(i)); - } - org.apache.arrow.vector.Float8Vector dv = - (org.apache.arrow.vector.Float8Vector) root.getVector(1); - for (int i = 0; i < rowCount; ++i) { - assertEquals("double match: ", DataAccessHelpers.getColumn(table, 1).get(offset + i), - dv.get(i)); - } + // stuff table into the scope + ExecutionContext.getContext().getQueryScope().putParam(staticTableName, table); + + // build up a snapshot request + byte[] magic = new byte[] {100, 112, 104, 110}; // equivalent to '0x6E687064' (ASCII "dphn") + + FlightDescriptor fd = FlightDescriptor.command(magic); + + try (FlightClient.ExchangeReaderWriter erw = flightClient.doExchange(fd); + final RootAllocator allocator = new RootAllocator(Integer.MAX_VALUE)) { + + final FlatBufferBuilder metadata = new FlatBufferBuilder(); + + // use 0 for batch size and max message size to use server-side defaults + int optOffset = + BarrageSnapshotOptions.createBarrageSnapshotOptions(metadata, ColumnConversionMode.Stringify, + false, 0, 0); + + final int ticOffset = + BarrageSnapshotRequest.createTicketVector(metadata, + ScopeTicketHelper.nameToBytes(staticTableName)); + BarrageSnapshotRequest.startBarrageSnapshotRequest(metadata); + BarrageSnapshotRequest.addColumns(metadata, 0); + BarrageSnapshotRequest.addViewport(metadata, 0); + BarrageSnapshotRequest.addSnapshotOptions(metadata, optOffset); + BarrageSnapshotRequest.addTicket(metadata, ticOffset); + metadata.finish(BarrageSnapshotRequest.endBarrageSnapshotRequest(metadata)); + + final FlatBufferBuilder wrapper = new FlatBufferBuilder(); + final int innerOffset = wrapper.createByteVector(metadata.dataBuffer()); + wrapper.finish(BarrageMessageWrapper.createBarrageMessageWrapper( + wrapper, + 0x6E687064, // the numerical representation of the ASCII "dphn". + BarrageMessageType.BarrageSnapshotRequest, + innerOffset)); + + // extract the bytes and package them in an ArrowBuf for transmission + byte[] msg = wrapper.sizedByteArray(); + ArrowBuf data = allocator.buffer(msg.length); + data.writeBytes(msg); + + erw.getWriter().putMetadata(data); + erw.getWriter().completed(); + + // read everything from the server (expecting schema message and one data message) + int totalRowCount = 0; + while (erw.getReader().next()) { + final int offset = totalRowCount; + final VectorSchemaRoot root = erw.getReader().getRoot(); + final int rowCount = root.getRowCount(); + totalRowCount += rowCount; + + // check the values against the source table + org.apache.arrow.vector.IntVector iv = + (org.apache.arrow.vector.IntVector) root.getVector(0); + for (int i = 0; i < rowCount; ++i) { + assertEquals("int match:", DataAccessHelpers.getColumn(table, 0).get(offset + i), iv.get(i)); + } + org.apache.arrow.vector.Float8Vector dv = + (org.apache.arrow.vector.Float8Vector) root.getVector(1); + for (int i = 0; i < rowCount; ++i) { + assertEquals("double match: ", DataAccessHelpers.getColumn(table, 1).get(offset + i), + dv.get(i)); } - assertEquals(table.size(), totalRowCount); } + assertEquals(table.size(), totalRowCount); } } @@ -797,34 +786,32 @@ public void testDoExchangeProtocol() throws Exception { final String staticTableName = "flightInfoTest"; final Table table = TableTools.emptyTable(10).update("I = i", "J = i + 0.01"); - try (final SafeCloseable ignored = LivenessScopeStack.open(scriptSession, false)) { - // stuff table into the scope - scriptSession.setVariable(staticTableName, table); - - // java-flight requires us to send a message, but cannot add app metadata, send a dummy message - byte[] empty = new byte[] {}; - final FlightDescriptor fd = FlightDescriptor.command(empty); - try (FlightClient.ExchangeReaderWriter erw = flightClient.doExchange(fd); - final RootAllocator allocator = new RootAllocator(Integer.MAX_VALUE)) { + // stuff table into the scope + ExecutionContext.getContext().getQueryScope().putParam(staticTableName, table); - byte[] msg = new byte[0]; - ArrowBuf data = allocator.buffer(msg.length); - data.writeBytes(msg); + // java-flight requires us to send a message, but cannot add app metadata, send a dummy message + byte[] empty = new byte[] {}; + final FlightDescriptor fd = FlightDescriptor.command(empty); + try (FlightClient.ExchangeReaderWriter erw = flightClient.doExchange(fd); + final RootAllocator allocator = new RootAllocator(Integer.MAX_VALUE)) { - erw.getWriter().putMetadata(data); - erw.getWriter().completed(); + byte[] msg = new byte[0]; + ArrowBuf data = allocator.buffer(msg.length); + data.writeBytes(msg); - Exception exception = assertThrows(FlightRuntimeException.class, () -> { - erw.getReader().next(); - }); + erw.getWriter().putMetadata(data); + erw.getWriter().completed(); - String expectedMessage = "failed to receive Barrage request metadata"; - String actualMessage = exception.getMessage(); + Exception exception = assertThrows(FlightRuntimeException.class, () -> { + erw.getReader().next(); + }); - assertTrue(actualMessage.contains(expectedMessage)); - } + String expectedMessage = "failed to receive Barrage request metadata"; + String actualMessage = exception.getMessage(); + assertTrue(actualMessage.contains(expectedMessage)); } + } @Test @@ -846,7 +833,7 @@ public T transform(T source) { } }; - scriptSession.setVariable(tableName, table); + ExecutionContext.getContext().getQueryScope().putParam(tableName, table); // export from query scope to our session; this transforms the table assertEquals(0, numTransforms.intValue()); @@ -858,8 +845,8 @@ public T transform(T source) { assertEquals(1, numTransforms.intValue()); // check that the table was transformed - Object result = scriptSession.getVariable(resultTableName); - assertTrue(result instanceof Table); + Object result = ExecutionContext.getContext().getQueryScope().readParamValue(resultTableName, null); + assertTrue(result + "", result instanceof Table); assertEquals(1, ((Table) result).getColumnSources().size()); assertEquals(2, table.getColumnSources().size()); } @@ -870,7 +857,7 @@ public void testSimpleServiceAuthWiring() throws Exception { final String tableName = "testSimpleServiceAuthWiringTest"; final String resultTableName = tableName + "Result"; final Table table = TableTools.emptyTable(10).update("I = -i", "J = -i"); - scriptSession.setVariable(tableName, table); + ExecutionContext.getContext().getQueryScope().putParam(tableName, table); // export from query scope to our session; this transforms the table try (final TableHandle handle = clientSession.session().execute(TicketTable.fromQueryScopeField(tableName))) { @@ -903,7 +890,7 @@ public void testSimpleContextualAuthWiring() throws Exception { // stuff table into the scope final String tableName = "testSimpleContextualAuthWiringTest"; final Table table = TableTools.emptyTable(10).update("I = -i", "J = -i"); - scriptSession.setVariable(tableName, table); + ExecutionContext.getContext().getQueryScope().putParam(tableName, table); // export from query scope to our session; this transforms the table try (final TableHandle handle = clientSession.session().execute(TicketTable.fromQueryScopeField(tableName))) { @@ -1026,7 +1013,7 @@ private void assertRoundTripDataEqual(Table deephavenTable) throws Exception { public void testBarrageMessageAppendingMarshaller() { final int size = 100; final Table source = TableTools.emptyTable(size).update("I = ii", "J = `str_` + i"); - scriptSession.setVariable("test", source); + ExecutionContext.getContext().getQueryScope().putParam("test", source); // fetch schema over flight final SchemaResult schema = flightClient.getSchema(arrowFlightDescriptorForName("test")); @@ -1077,7 +1064,7 @@ public void testColumnsAsListFeature() throws Exception { final Table appendOnly = TableTools.timeTable("PT1s") .update("I = ii % 3", "J = `str_` + i"); final Table withMods = appendOnly.lastBy("I"); - scriptSession.setVariable("test", withMods); + ExecutionContext.getContext().getQueryScope().putParam("test", withMods); } final BarrageSubscriptionOptions options = BarrageSubscriptionOptions.builder()