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 a9160c0da2b..4816007c64d 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 @@ -117,7 +117,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; @@ -157,7 +157,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(() -> { @@ -255,7 +255,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) { @@ -309,7 +309,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 a064a1ef415..68bc260d813 100644 --- a/server/src/test/java/io/deephaven/server/runner/DeephavenApiServerTestBase.java +++ b/server/src/test/java/io/deephaven/server/runner/DeephavenApiServerTestBase.java @@ -86,7 +86,8 @@ interface Builder { @Rule public final GrpcCleanupRule grpcCleanup = new GrpcCleanupRule(); - private ExecutionContext executionContext; + @Inject + ExecutionContext executionContext; private SafeCloseable executionContextCloseable; private LogBuffer logBuffer; @@ -129,7 +130,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()