Skip to content

Commit

Permalink
Fix up TreeTables
Browse files Browse the repository at this point in the history
- JsTreeTable was using the ticket from the widget directly, which doesn't work now if the widget releases it's ticket when the stream disconnects (fetch-only widgets)
- Need to export the table as an exportedObject so we don't release it entirely
- Will need Colin to review, this seems a little strange.
  • Loading branch information
mofojed committed Dec 8, 2023
1 parent 7b97ff5 commit 901ad90
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ public void writeCompatibleObjectTo(
.setSnapshotSchema(BarrageUtil.schemaBytes(
fbb -> HierarchicalTableSchemaUtil.makeSchemaPayload(fbb, hierarchicalTable)))
.build();
exporter.reference(object);

result.writeTo(out);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,8 @@ private enum RebuildStep {
// The current filter and sort state
private List<FilterCondition> filters = new ArrayList<>();
private List<Sort> sorts = new ArrayList<>();

private TypedTicket sourceTicket;
private TicketAndPromise<?> filteredTable;
private TicketAndPromise<?> sortedTable;

Expand Down Expand Up @@ -396,6 +398,7 @@ private enum RebuildStep {
public JsTreeTable(WorkerConnection workerConnection, JsWidget widget) {
this.connection = workerConnection;
this.widget = widget;
this.sourceTicket = widget.getExportedObjects()[0].takeTicket();

// register for same-session disconnect/reconnect callbacks
this.connection.registerSimpleReconnectable(this);
Expand Down Expand Up @@ -509,7 +512,7 @@ public JsTreeTable(WorkerConnection workerConnection, JsWidget widget) {
.newState(this, (c, newState, metadata) -> {
HierarchicalTableSourceExportRequest exportRequest = new HierarchicalTableSourceExportRequest();
exportRequest.setResultTableId(newState.getHandle().makeTicket());
exportRequest.setHierarchicalTableId(widget.getTicket());
exportRequest.setHierarchicalTableId(sourceTicket.getTicket());
connection.hierarchicalTableServiceClient().exportSource(exportRequest, connection.metadata(),
c::apply);
}, "source for hierarchical table")
Expand All @@ -521,15 +524,15 @@ private TicketAndPromise<?> prepareFilter() {
return filteredTable;
}
if (nextFilters.isEmpty()) {
return new TicketAndPromise<>(widget.getTicket(), connection);
return new TicketAndPromise<>(sourceTicket.getTicket(), connection);
}
Ticket ticket = connection.getConfig().newTicket();
filteredTable = new TicketAndPromise<>(ticket, Callbacks.grpcUnaryPromise(c -> {

HierarchicalTableApplyRequest applyFilter = new HierarchicalTableApplyRequest();
applyFilter.setFiltersList(
nextFilters.stream().map(FilterCondition::makeDescriptor).toArray(Condition[]::new));
applyFilter.setInputHierarchicalTableId(widget.getTicket());
applyFilter.setInputHierarchicalTableId(sourceTicket.getTicket());
applyFilter.setResultHierarchicalTableId(ticket);
connection.hierarchicalTableServiceClient().apply(applyFilter, connection.metadata(), c::apply);
}), connection);
Expand Down Expand Up @@ -650,7 +653,7 @@ private void replaceSubscription(RebuildStep step) {
this.alwaysFireNextEvent = false;

JsLog.debug("Sending tree table request", this,
LazyString.of(() -> widget.getTicket().getTicket_asB64()),
LazyString.of(() -> sourceTicket.getTicket().getTicket_asB64()),
columnsBitset,
range,
alwaysFireEvent);
Expand Down Expand Up @@ -977,6 +980,8 @@ public void close() {

JsLog.debug("Closing tree table", this);

connection.releaseTicket(sourceTicket.getTicket());

connection.unregisterSimpleReconnectable(this);

if (filteredTable != null) {
Expand Down Expand Up @@ -1010,7 +1015,7 @@ public void close() {

@Override
public TypedTicket typedTicket() {
return widget.typedTicket();
return sourceTicket;
}

/**
Expand Down

0 comments on commit 901ad90

Please sign in to comment.