diff --git a/web/client-api/src/main/java/io/deephaven/web/client/api/JsPartitionedTable.java b/web/client-api/src/main/java/io/deephaven/web/client/api/JsPartitionedTable.java index 897e616ebc5..628ee3d5180 100644 --- a/web/client-api/src/main/java/io/deephaven/web/client/api/JsPartitionedTable.java +++ b/web/client-api/src/main/java/io/deephaven/web/client/api/JsPartitionedTable.java @@ -314,6 +314,8 @@ public void close() { if (subscription != null) { subscription.close(); } + + widget.close(); } } diff --git a/web/client-api/src/main/java/io/deephaven/web/client/api/WorkerConnection.java b/web/client-api/src/main/java/io/deephaven/web/client/api/WorkerConnection.java index f9611a1bc57..7d4ea477962 100644 --- a/web/client-api/src/main/java/io/deephaven/web/client/api/WorkerConnection.java +++ b/web/client-api/src/main/java/io/deephaven/web/client/api/WorkerConnection.java @@ -766,21 +766,18 @@ public Promise getObject(JsVariableDefinition definition) { return getFigure(definition); } else if (JsVariableType.PANDAS.equalsIgnoreCase(definition.getType())) { return getWidget(definition) - .then(widget -> widget.getExportedObjects()[0].fetch()); + .then(JsWidget::refetch) + .then(widget -> { + widget.close(); + return widget.getExportedObjects()[0].fetch(); + }); } else if (JsVariableType.PARTITIONEDTABLE.equalsIgnoreCase(definition.getType())) { return getPartitionedTable(definition); } else if (JsVariableType.HIERARCHICALTABLE.equalsIgnoreCase(definition.getType())) { return getHierarchicalTable(definition); } else { - if (JsVariableType.TABLEMAP.equalsIgnoreCase(definition.getType())) { - JsLog.warn( - "TableMap is now known as PartitionedTable, fetching as a plain widget. To fetch as a PartitionedTable use that as the type."); - } - if (JsVariableType.TREETABLE.equalsIgnoreCase(definition.getType())) { - JsLog.warn( - "TreeTable is now HierarchicalTable, fetching as a plain widget. To fetch as a HierarchicalTable use that as this type."); - } - return getWidget(definition); + warnLegacyTicketTypes(definition.getType()); + return getWidget(definition).then(JsWidget::refetch); } } @@ -810,6 +807,45 @@ public Promise getJsObject(JsPropertyMap definitionObject) { } } + public Promise getObject(TypedTicket typedTicket) { + if (JsVariableType.TABLE.equalsIgnoreCase(typedTicket.getType())) { + throw new IllegalArgumentException("wrong way to get a table from a ticket"); + } else if (JsVariableType.FIGURE.equalsIgnoreCase(typedTicket.getType())) { + return new JsFigure(this, c -> { + JsWidget widget = new JsWidget(this, typedTicket); + widget.refetch().then(ignore -> { + c.apply(null, makeFigureFetchResponse(widget)); + return null; + }); + }).refetch(); + } else if (JsVariableType.PANDAS.equalsIgnoreCase(typedTicket.getType())) { + return getWidget(typedTicket) + .then(JsWidget::refetch) + .then(widget -> { + widget.close(); + return widget.getExportedObjects()[0].fetch(); + }); + } else if (JsVariableType.PARTITIONEDTABLE.equalsIgnoreCase(typedTicket.getType())) { + return new JsPartitionedTable(this, new JsWidget(this, typedTicket)).refetch(); + } else if (JsVariableType.HIERARCHICALTABLE.equalsIgnoreCase(typedTicket.getType())) { + return new JsWidget(this, typedTicket).refetch().then(w -> Promise.resolve(new JsTreeTable(this, w))); + } else { + warnLegacyTicketTypes(typedTicket.getType()); + return getWidget(typedTicket).then(JsWidget::refetch); + } + } + + private static void warnLegacyTicketTypes(String ticketType) { + if (JsVariableType.TABLEMAP.equalsIgnoreCase(ticketType)) { + JsLog.warn( + "TableMap is now known as PartitionedTable, fetching as a plain widget. To fetch as a PartitionedTable use that as the type."); + } + if (JsVariableType.TREETABLE.equalsIgnoreCase(ticketType)) { + JsLog.warn( + "TreeTable is now HierarchicalTable, fetching as a plain widget. To fetch as a HierarchicalTable use that as this type."); + } + } + @JsMethod @SuppressWarnings("ConstantConditions") public JsRunnable subscribeToFieldUpdates(JsConsumer callback) { @@ -911,12 +947,8 @@ public Promise getPartitionedTable(JsVariableDefinition varD .then(widget -> new JsPartitionedTable(this, widget).refetch()); } - public Promise getTreeTable(JsVariableDefinition varDef) { - return getWidget(varDef).then(w -> Promise.resolve(new JsTreeTable(this, w))); - } - public Promise getHierarchicalTable(JsVariableDefinition varDef) { - return getWidget(varDef).then(w -> Promise.resolve(new JsTreeTable(this, w))); + return getWidget(varDef).then(JsWidget::refetch).then(w -> Promise.resolve(new JsTreeTable(this, w))); } public Promise getFigure(JsVariableDefinition varDef) { @@ -926,13 +958,9 @@ public Promise getFigure(JsVariableDefinition varDef) { return whenServerReady("get a figure") .then(server -> new JsFigure(this, c -> { - getWidget(varDef).then(widget -> { - FetchObjectResponse legacyResponse = new FetchObjectResponse(); - legacyResponse.setData(widget.getDataAsU8()); - legacyResponse.setType(widget.getType()); - legacyResponse.setTypedExportIdsList(Arrays.stream(widget.getExportedObjects()) - .map(JsWidgetExportedObject::typedTicket).toArray(TypedTicket[]::new)); - c.apply(null, legacyResponse); + getWidget(varDef).then(JsWidget::refetch).then(widget -> { + c.apply(null, makeFigureFetchResponse(widget)); + widget.close(); return null; }, error -> { c.apply(error, null); @@ -941,6 +969,15 @@ public Promise getFigure(JsVariableDefinition varDef) { }).refetch()); } + private static FetchObjectResponse makeFigureFetchResponse(JsWidget widget) { + FetchObjectResponse legacyResponse = new FetchObjectResponse(); + legacyResponse.setData(widget.getDataAsU8()); + legacyResponse.setType(widget.getType()); + legacyResponse.setTypedExportIdsList(Arrays.stream(widget.getExportedObjects()) + .map(JsWidgetExportedObject::typedTicket).toArray(TypedTicket[]::new)); + return legacyResponse; + } + private TypedTicket createTypedTicket(JsVariableDefinition varDef) { TypedTicket typedTicket = new TypedTicket(); typedTicket.setTicket(TableTicket.createTicket(varDef)); @@ -960,7 +997,7 @@ public Promise getWidget(JsVariableDefinition varDef) { public Promise getWidget(TypedTicket typedTicket) { return whenServerReady("get a widget") - .then(response -> new JsWidget(this, typedTicket).refetch()); + .then(response -> Promise.resolve(new JsWidget(this, typedTicket))); } public void registerSimpleReconnectable(HasLifecycle figure) { diff --git a/web/client-api/src/main/java/io/deephaven/web/client/api/console/JsVariableDefinition.java b/web/client-api/src/main/java/io/deephaven/web/client/api/console/JsVariableDefinition.java index 11473ea63be..50dda49996d 100644 --- a/web/client-api/src/main/java/io/deephaven/web/client/api/console/JsVariableDefinition.java +++ b/web/client-api/src/main/java/io/deephaven/web/client/api/console/JsVariableDefinition.java @@ -12,8 +12,8 @@ /** * A format to describe a variable available to be read from the server. Application fields are optional, and only * populated when a variable is provided by application mode. - * - * APIs which take a VariableDefinition` must at least be provided an object with a type and id field. + *

+ * APIs which take a VariableDefinition must at least be provided an object with a type and id field. */ @TsInterface @TsName(namespace = "dh.ide", name = "VariableDefinition") @@ -28,6 +28,10 @@ public class JsVariableDefinition { private final String applicationName; public JsVariableDefinition(String type, String title, String id, String description) { + // base64('s/') ==> 'cy8' + if (!id.startsWith("cy8")) { + throw new IllegalArgumentException("Cannot create a VariableDefinition from a non-scope ticket"); + } this.type = type; this.title = title == null ? JS_UNAVAILABLE : title; this.id = id; diff --git a/web/client-api/src/main/java/io/deephaven/web/client/api/tree/JsTreeTable.java b/web/client-api/src/main/java/io/deephaven/web/client/api/tree/JsTreeTable.java index a1f0569955c..c6ffa93cbd3 100644 --- a/web/client-api/src/main/java/io/deephaven/web/client/api/tree/JsTreeTable.java +++ b/web/client-api/src/main/java/io/deephaven/web/client/api/tree/JsTreeTable.java @@ -979,8 +979,7 @@ public void close() { connection.unregisterSimpleReconnectable(this); - // Presently it is never necessary to release widget tickets, since they can't be export tickets. - // connection.releaseTicket(widget.getTicket()); + connection.releaseTicket(widget.getTicket()); if (filteredTable != null) { filteredTable.release(); diff --git a/web/client-api/src/main/java/io/deephaven/web/client/api/widget/JsWidget.java b/web/client-api/src/main/java/io/deephaven/web/client/api/widget/JsWidget.java index e5449ec42a9..22f2d347551 100644 --- a/web/client-api/src/main/java/io/deephaven/web/client/api/widget/JsWidget.java +++ b/web/client-api/src/main/java/io/deephaven/web/client/api/widget/JsWidget.java @@ -4,7 +4,6 @@ package io.deephaven.web.client.api.widget; import com.vertispan.tsdefs.annotations.TsName; -import com.vertispan.tsdefs.annotations.TsTypeRef; import com.vertispan.tsdefs.annotations.TsUnion; import com.vertispan.tsdefs.annotations.TsUnionMember; import elemental2.core.ArrayBuffer; @@ -34,15 +33,57 @@ /** * A Widget represents a server side object that sends one or more responses to the client. The client can then * interpret these responses to see what to render, or how to respond. - * + *

* Most custom object types result in a single response being sent to the client, often with other exported objects, but * some will have streamed responses, and allow the client to send follow-up requests of its own. This class's API is * backwards compatible, but as such does not offer a way to tell the difference between a streaming or non-streaming * object type, the client code that handles the payloads is expected to know what to expect. See - * dh.WidgetMessageDetails for more information. - * + * {@link WidgetMessageDetails} for more information. + *

* When the promise that returns this object resolves, it will have the first response assigned to its fields. Later - * responses from the server will be emitted as "message" events. When the connection with the server ends + * responses from the server will be emitted as "message" events. When the connection with the server ends, the "close" + * event will be emitted. In this way, the connection will behave roughly in the same way as a WebSocket - either side + * can close, and after close no more messages will be processed. There can be some latency in closing locally while + * remote messages are still pending - it is up to implementations of plugins to handle this case. + *

+ * Also like WebSockets, the plugin API doesn't define how to serialize messages, and just handles any binary payloads. + * What it does handle however, is allowing those messages to include references to server-side objects with those + * payloads. Those server side objects might be tables or other built-in types in the Deephaven JS API, or could be + * objects usable through their own plugins. They also might have no plugin at all, allowing the client to hold a + * reference to them and pass them back to the server, either to the current plugin instance, or through another API. + * The {@code Widget} type does not specify how those objects should be used or their lifecycle, but leaves that + * entirely to the plugin. Messages will arrive in the order they were sent. + *

+ * This can suggest several patterns for how plugins operate: + *

    + *
  • The plugin merely exists to transport some other object to the client. This can be useful for objects which can + * easily be translated to some other type (like a Table) when the user clicks on it. An example of this is + * {@code pandas.DataFrame} will result in a widget that only contains a static + * {@link io.deephaven.web.client.api.JsTable}. Presently, the widget is immediately closed, and only the Table is + * provided to the JS API consumer.
    • + *
  • The plugin provides references to Tables and other objects, and those objects can live longer than the object + * which provided them. One concrete example of this could have been + * {@link io.deephaven.web.client.api.JsPartitionedTable} when fetching constituent tables, but it was implemented + * before bidirectional plugins were implemented. Another example of this is plugins that serve as a "factory", giving + * the user access to table manipulation/creation methods not supported by gRPC or the JS API.
    • + *
  • The plugin provides reference to Tables and other objects that only make sense within the context of the widget + * instance, so when the widget goes away, those objects should be released as well. This is also an example of + * {@link io.deephaven.web.client.api.JsPartitionedTable}, as the partitioned table tracks creation of new keys through + * an internal table instance.
    • + *
+ * + * Handling server objects in messages also has more than one potential pattern that can be used: + *
    + *
  • One object per message - the message clearly is about that object, no other details required.
    • + *
  • Objects indexed within their message - as each message comes with a list of objects, those objects can be + * referenced within the payload by index. This is roughly how {@link io.deephaven.web.client.api.widget.plot.JsFigure} + * behaves, where the figure descriptor schema includes an index for each created series, describing which table should + * be used, which columns should be mapped to each axis.
    • + *
  • Objects indexed since widget creation - each message would append its objects to a list created when the widget + * was first made, and any new exports that arrive in a new message would be appended to that list. Then, subsequent + * messages can reference objects already sent. This imposes a limitation where the client cannot release any exports + * without the server somehow signaling that it will never reference that export again.
    • + *
*/ // TODO consider reconnect support? This is somewhat tricky without understanding the semantics of the widget @TsName(namespace = "dh", name = "Widget") @@ -120,9 +161,10 @@ public Promise refetch() { messageStream.onStatus(status -> { if (!status.isOk()) { reject.onInvoke(status.getDetails()); - fireEvent(EVENT_CLOSE); - closeStream(); } + fireEvent(EVENT_CLOSE); + closeStream(); + connection.releaseTicket(getTicket()); }); messageStream.onEnd(status -> { closeStream(); @@ -175,6 +217,10 @@ public String getDataAsString() { return new String(Js.uncheckedCast(response.getData().getPayload_asU8()), StandardCharsets.UTF_8); } + /** + * @return the exported objects sent in the initial message from the server. The client is responsible for closing + * them when finished using them. + */ @Override @JsProperty public JsWidgetExportedObject[] getExportedObjects() { @@ -226,8 +272,7 @@ default ArrayBufferView asView() { * @param references an array of objects that can be safely sent to the server */ @JsMethod - public void sendMessage(MessageUnion msg, - @JsOptional JsArray<@TsTypeRef(ServerObject.Union.class) ServerObject> references) { + public void sendMessage(MessageUnion msg, @JsOptional JsArray references) { if (messageStream == null) { return; } @@ -249,7 +294,7 @@ public void sendMessage(MessageUnion msg, } for (int i = 0; references != null && i < references.length; i++) { - ServerObject reference = references.getAt(i); + ServerObject reference = references.getAt(i).asServerObject(); data.addReferences(reference.typedTicket()); } diff --git a/web/client-api/src/main/java/io/deephaven/web/client/api/widget/JsWidgetExportedObject.java b/web/client-api/src/main/java/io/deephaven/web/client/api/widget/JsWidgetExportedObject.java index a8948d990fa..d6df425e1e9 100644 --- a/web/client-api/src/main/java/io/deephaven/web/client/api/widget/JsWidgetExportedObject.java +++ b/web/client-api/src/main/java/io/deephaven/web/client/api/widget/JsWidgetExportedObject.java @@ -5,22 +5,28 @@ import com.vertispan.tsdefs.annotations.TsInterface; import com.vertispan.tsdefs.annotations.TsName; +import elemental2.core.JsArray; import elemental2.promise.Promise; +import io.deephaven.javascript.proto.dhinternal.io.deephaven.proto.session_pb.ExportRequest; import io.deephaven.javascript.proto.dhinternal.io.deephaven.proto.table_pb.ExportedTableCreationResponse; +import io.deephaven.javascript.proto.dhinternal.io.deephaven.proto.ticket_pb.Ticket; import io.deephaven.javascript.proto.dhinternal.io.deephaven.proto.ticket_pb.TypedTicket; import io.deephaven.web.client.api.Callbacks; +import io.deephaven.web.client.api.JsLazy; import io.deephaven.web.client.api.JsTable; import io.deephaven.web.client.api.ServerObject; import io.deephaven.web.client.api.WorkerConnection; -import io.deephaven.web.client.api.console.JsVariableDefinition; import io.deephaven.web.client.api.console.JsVariableType; +import io.deephaven.web.client.fu.JsLog; import io.deephaven.web.client.state.ClientTableState; import jsinterop.annotations.JsMethod; +import jsinterop.annotations.JsNullable; import jsinterop.annotations.JsProperty; /** - * Represents a server-side object that may not yet have been fetched by the client. Does not memoize its result, so - * fetch() should only be called once, and calling close() on this object will also close the result of the fetch. + * Represents a server-side object that may not yet have been fetched by the client. When this object will no longer be + * used, if {@link #fetch()} is not called on this object, then {@link #close()} must be to ensure server-side resources + * are correctly freed. */ @TsInterface @TsName(namespace = "dh", name = "WidgetExportedObject") @@ -29,13 +35,46 @@ public class JsWidgetExportedObject implements ServerObject { private final TypedTicket ticket; + private final JsLazy> fetched; + public JsWidgetExportedObject(WorkerConnection connection, TypedTicket ticket) { this.connection = connection; this.ticket = ticket; + this.fetched = JsLazy.of(() -> { + if (getType() == null) { + return Promise.reject("Exported object has no type, can't be fetched"); + } + if (getType().equals(JsVariableType.TABLE)) { + return Callbacks.grpcUnaryPromise(c -> { + connection.tableServiceClient().getExportedTableCreationResponse(ticket.getTicket(), + connection.metadata(), + c::apply); + }).then(etcr -> { + ClientTableState cts = connection.newStateFromUnsolicitedTable(etcr, "table for widget"); + JsTable table = new JsTable(connection, cts); + // never attempt a reconnect, since we might have a different widget schema entirely + table.addEventListener(JsTable.EVENT_DISCONNECT, ignore -> table.close()); + return Promise.resolve(table); + }); + } else { + return this.connection.getObject(ticket); + } + }); } + /** + * Returns the type of this export, typically one of {@link JsVariableType}, but may also include plugin types. If + * null, this object cannot be fetched, but can be passed to the server, such as via + * {@link JsWidget#sendMessage(JsWidget.MessageUnion, JsArray)}. + * + * @return the string type of this server-side object, or null. + */ + @JsNullable @JsProperty public String getType() { + if (ticket.getType().isEmpty()) { + return null; + } return ticket.getType(); } @@ -47,32 +86,55 @@ public TypedTicket typedTicket() { return typedTicket; } + /** + * Exports another copy of this reference, allowing it to be fetched separately. Results in rejection if the ticket + * was already closed (either by calling {@link #close()} or closing the object returned from {@link #fetch()}). + * + * @return a promise returning a reexported copy of this object, still referencing the same server-side object. + */ + @JsMethod + public Promise reexport() { + Ticket reexportedTicket = connection.getConfig().newTicket(); + + // Future optimization - we could "race" these by running the export in the background, to avoid + // an extra round trip. + return Callbacks.grpcUnaryPromise(c -> { + ExportRequest req = new ExportRequest(); + req.setSourceId(ticket.getTicket()); + req.setResultId(reexportedTicket); + connection.sessionServiceClient().exportFromTicket(req, connection.metadata(), c::apply); + }).then(success -> { + TypedTicket typedTicket = new TypedTicket(); + typedTicket.setTicket(reexportedTicket); + typedTicket.setType(ticket.getType()); + return Promise.resolve(new JsWidgetExportedObject(connection, typedTicket)); + }); + } + + /** + * Returns a promise that will fetch the object represented by this reference. Multiple calls to this will return + * the same instance. + * + * @return a promise that will resolve to a client side object that represents the reference on the server. + */ @JsMethod public Promise fetch() { - if (getType().equals(JsVariableType.TABLE)) { - return Callbacks.grpcUnaryPromise(c -> { - connection.tableServiceClient().getExportedTableCreationResponse(ticket.getTicket(), - connection.metadata(), - c::apply); - }).then(etcr -> { - ClientTableState cts = connection.newStateFromUnsolicitedTable(etcr, "table for widget"); - JsTable table = new JsTable(connection, cts); - // never attempt a reconnect, since we might have a different widget schema entirely - table.addEventListener(JsTable.EVENT_DISCONNECT, ignore -> table.close()); - return Promise.resolve(table); - }); - } else { - return this.connection.getObject( - new JsVariableDefinition(ticket.getType(), null, ticket.getTicket().getTicket_asB64(), null)); + if (getType() != null) { + return fetched.get(); } + return Promise.reject("Can't fetch an object with no type (i.e. no server plugin implementation)"); } /** - * Releases the server-side resources associated with this object, regardless of whether or not other client-side - * objects exist that also use that object. + * Releases the server-side resources associated with this object, regardless of whether other client-side objects + * exist that also use that object. Should not be called after fetch() has been invoked. */ @JsMethod public void close() { - connection.releaseTicket(ticket.getTicket()); + if (!fetched.isAvailable()) { + connection.releaseTicket(ticket.getTicket()); + } else { + JsLog.warn("Cannot close, already fetched. Instead, close the fetched object."); + } } } diff --git a/web/client-api/src/main/java/io/deephaven/web/client/ide/IdeSession.java b/web/client-api/src/main/java/io/deephaven/web/client/ide/IdeSession.java index 6d8e1b69cb0..75bedece3c8 100644 --- a/web/client-api/src/main/java/io/deephaven/web/client/ide/IdeSession.java +++ b/web/client-api/src/main/java/io/deephaven/web/client/ide/IdeSession.java @@ -130,12 +130,12 @@ public Promise getFigure(String name) { */ public Promise getTreeTable(String name) { return connection.getVariableDefinition(name, JsVariableType.HIERARCHICALTABLE) - .then(connection::getTreeTable); + .then(connection::getHierarchicalTable); } public Promise getHierarchicalTable(String name) { return connection.getVariableDefinition(name, JsVariableType.HIERARCHICALTABLE) - .then(connection::getTreeTable); + .then(connection::getHierarchicalTable); } public Promise getObject(@TsTypeRef(JsVariableDescriptor.class) JsPropertyMap definitionObject) {