Skip to content

Commit

Permalink
fix: Improve JS API handling of formatting data (#6664)
Browse files Browse the repository at this point in the history 
Cells with formatted rows would error when read in two specific ways:
either if the subscription/snapshot call was made before the table to be
read was resolved, or if the format data was read directly from the
TableData rather than from the row or the column.

Additionally fixes a bug where an unresolved table with a viewport being
created on it cannot be snapshotted.

Fixes DH-18634
Fixes DH-18784
  • Loading branch information
@niloc132
niloc132 authored Feb 26, 2025
1 parent 66adb42 commit 4d57893
Show file tree
Hide file tree
Showing 3 changed files with 237 additions and 16 deletions.
10 changes: 6 additions & 4 deletions ...api/src/main/java/io/deephaven/web/client/api/subscription/AbstractTableSubscription.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ protected enum Status {
private final SubscriptionType subscriptionType;
private final ClientTableState state;
private final WorkerConnection connection;
protected final int rowStyleColumn;
protected int rowStyleColumn;
private JsArray<Column> columns;
private BitSet columnBitSet;
protected RangeSet viewportRowSet;
Expand All @@ -99,8 +99,6 @@ protected AbstractTableSubscription(
this.subscriptionType = subscriptionType;
this.state = state;
this.connection = connection;
rowStyleColumn = state.getRowFormatColumn() == null ? TableData.NO_ROW_FORMAT_COLUMN
: state.getRowFormatColumn().getIndex();

revive();
}
Expand All @@ -116,6 +114,10 @@ protected void revive() {
// already closed
return;
}

rowStyleColumn = s.getRowFormatColumn() == null ? TableData.NO_ROW_FORMAT_COLUMN
: state.getRowFormatColumn().getIndex();

WebBarrageSubscription.ViewportChangedHandler viewportChangedHandler = this::onViewportChange;
WebBarrageSubscription.DataChangedHandler dataChangedHandler = this::onDataChanged;

Expand Down Expand Up @@ -475,7 +477,7 @@ public Format getFormat(long index, Column column) {
cellColors = wrapper == null ? 0 : wrapper.getWrapped();
}
if (rowStyleColumn != NO_ROW_FORMAT_COLUMN) {
LongWrapper wrapper = subscription.getData(index, column.getStyleColumnIndex()).uncheckedCast();
LongWrapper wrapper = subscription.getData(index, rowStyleColumn).uncheckedCast();
rowColors = wrapper == null ? 0 : wrapper.getWrapped();
}
if (column.getFormatStringColumnIndex() != null) {
Expand Down
24 changes: 13 additions & 11 deletions ...api/src/main/java/io/deephaven/web/client/api/subscription/TableViewportSubscription.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -347,15 +347,16 @@ public Promise<TableData> snapshot(JsRangeSet rows, Column[] columns) {
.useDeephavenNulls(true)
.build();

WebBarrageSubscription snapshot = WebBarrageSubscription.subscribe(
SubscriptionType.SNAPSHOT, state(),
(serverViewport1, serverColumns, serverReverseViewport) -> {
},
(rowsAdded, rowsRemoved, totalMods, shifted, modifiedColumnSet) -> {
});
LazyPromise<TableData> promise = new LazyPromise<>();
state().onRunning(cts -> {
WebBarrageSubscription snapshot = WebBarrageSubscription.subscribe(
SubscriptionType.SNAPSHOT, cts,
(serverViewport1, serverColumns, serverReverseViewport) -> {
},
(rowsAdded, rowsRemoved, totalMods, shifted, modifiedColumnSet) -> {
});

WebBarrageStreamReader reader = new WebBarrageStreamReader();
return new Promise<>((resolve, reject) -> {
WebBarrageStreamReader reader = new WebBarrageStreamReader();

BiDiStream<FlightData, FlightData> doExchange = connection().<FlightData, FlightData>streamFactory().create(
headers -> connection().flightServiceClient().doExchange(headers),
Expand Down Expand Up @@ -421,19 +422,20 @@ SubscriptionType.SNAPSHOT, state(),
} else {
result = RangeSet.empty();
}
resolve.onInvoke(new SubscriptionEventData(snapshot, rowStyleColumn, Js.uncheckedCast(columns),
promise.succeed(new SubscriptionEventData(snapshot, rowStyleColumn, Js.uncheckedCast(columns),
result,
RangeSet.empty(),
RangeSet.empty(),
null));
} else {
reject.onInvoke(status);
promise.fail(status);
}
});

doExchange.send(payload);
doExchange.end();

});
}, promise::fail, () -> promise.fail("Table was closed"));
return promise.asPromise();
}
}
219 changes: 218 additions & 1 deletion web/client-api/src/test/java/io/deephaven/web/client/api/subscription/ViewportTestGwt.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,18 @@
import elemental2.promise.Promise;
import io.deephaven.web.client.api.AbstractAsyncGwtTestCase;
import io.deephaven.web.client.api.Column;
import io.deephaven.web.client.api.CustomColumn;
import io.deephaven.web.client.api.DateWrapper;
import io.deephaven.web.client.api.Format;
import io.deephaven.web.client.api.JsRangeSet;
import io.deephaven.web.client.api.JsTable;
import io.deephaven.web.client.api.Sort;
import io.deephaven.web.client.api.TableData;
import io.deephaven.web.client.api.event.Event;
import io.deephaven.web.client.api.event.HasEventHandling;
import io.deephaven.web.client.api.filter.FilterCondition;
import io.deephaven.web.client.api.filter.FilterValue;
import io.deephaven.web.shared.fu.JsConsumer;
import io.deephaven.web.shared.fu.RemoverFn;
import jsinterop.base.Any;
import jsinterop.base.Js;
Expand Down Expand Up @@ -48,7 +51,8 @@ public class ViewportTestGwt extends AbstractAsyncGwtTestCase {
".format_columns(['I=I>2 ? GREEN : RED', 'I = Decimal(`0.00%`)', 'Timestamp = Date(`yyyy_MM_dd`)'])")
.script("blinkOne",
"time_table(\"PT00:00:01\").update([\"I=i\", \"J=1\"]).last_by(by=\"J\").where(\"I%2 != 0\")")
.script("big", "empty_table(1_000_000).update_view(['I=i', 'Str=``+I']).where('I % 2 == 0')");
.script("big", "empty_table(1_000_000).update_view(['I=i', 'Str=``+I']).where('I % 2 == 0')")
.script("small", "big.head(100)");

public void testViewportOnStaticTable() {
connect(tables)
Expand Down Expand Up @@ -642,6 +646,219 @@ private void assertThrowsException(Runnable r) {
}
}

public void testSnapshotWithFormatting() {
connect(tables)
.then(table("small"))
.then(t -> {
delayTestFinish(7876);
// Add a timestamp column, and format the number/timestamp, style the row and cells
t.applyCustomColumns(JsArray.of(
JsTable.CustomColumnArgUnionType
.of("Timestamp=epochNanosToInstant((i + 1740000000) * 1_000_000_000)"),
JsTable.CustomColumnArgUnionType
.of(new CustomColumn("I", CustomColumn.TYPE_FORMAT_COLOR, "background(GREEN)")),
JsTable.CustomColumnArgUnionType
.of(new CustomColumn("Str", CustomColumn.TYPE_FORMAT_NUMBER, "`$###,##0.00`")),
JsTable.CustomColumnArgUnionType.of(new CustomColumn("Timestamp",
CustomColumn.TYPE_FORMAT_DATE, "`HH-mm-ss-SSSSSSSSS`"))));
// Wait for this to resolve, part of DH-18634 is that already running tables behave differently
return assertEventFiresOnce(t, JsTable.EVENT_CUSTOMCOLUMNSCHANGED, 2025).then(table -> {
Column iColumn = table.findColumn("I");
Column strColumn = table.findColumn("Str");
Column timestampColumn = table.findColumn("Timestamp");

JsConsumer<TableData> check = data -> {
assertEquals(10, data.getRows().length);
TableData.Row row = data.getRows().getAt(0);

Format iFormat = row.getFormat(iColumn);
assertNotNull(iFormat);
assertEquals("#008000", iFormat.getBackgroundColor());
assertEquals(iFormat, iColumn.getFormat(row));
assertEquals(iFormat, data.getFormat(0, iColumn));

Format strFormat = row.getFormat(strColumn);
assertNotNull(strFormat);
assertEquals("$###,##0.00", strFormat.getFormatString());
assertNull(strFormat.getBackgroundColor());
assertEquals(strFormat, strColumn.getFormat(row));
assertEquals(strFormat, data.getFormat(0, strColumn));

Format timestampFormat = row.getFormat(timestampColumn);
assertNotNull(timestampFormat);
assertEquals("HH-mm-ss-SSSSSSSSS", timestampFormat.getFormatString());
assertNull(timestampFormat.getBackgroundColor());
assertEquals(timestampFormat, timestampColumn.getFormat(row));
assertEquals(timestampFormat, data.getFormat(0, timestampColumn));
};

return Promise.resolve((Object) null).then(ignore -> {
TableViewportSubscription subscription = table.setViewport(0, 9, null);

Promise<JsTable> viewportCheck = assertUpdateReceived(table, check::apply, 2507);
Promise<Void> snapshotCheck =
subscription.snapshot(JsRangeSet.ofRange(0, 9), Js.cast(table.getColumns()))
.then(data -> {
check.apply(data);
return null;
});
return Promise.all(viewportCheck, snapshotCheck);
}).then(ignore -> {
table.applySort(new Sort[] {iColumn.sort().asc()});
TableViewportSubscription subscription = table.setViewport(0, 9, null);

Promise<JsTable> viewportCheck = assertUpdateReceived(table, check::apply, 2508);
Promise<Void> snapshotCheck =
subscription.snapshot(JsRangeSet.ofRange(0, 9), Js.cast(table.getColumns()))
.then(data -> {
check.apply(data);
return null;
});
return Promise.all(viewportCheck, snapshotCheck);
}).then(ignore -> Promise.resolve(table));
});
})
.then(t -> {
delayTestFinish(7877);
// Repeat, this time also with a row style
t.applyCustomColumns(JsArray.of(
JsTable.CustomColumnArgUnionType
.of("Timestamp=epochNanosToInstant((i + 1740000000) * 1_000_000_000)"),
JsTable.CustomColumnArgUnionType
.of(new CustomColumn("I", CustomColumn.TYPE_FORMAT_COLOR, "background(GREEN)")),
JsTable.CustomColumnArgUnionType
.of(new CustomColumn("Str", CustomColumn.TYPE_FORMAT_NUMBER, "`$###,##0.00`")),
JsTable.CustomColumnArgUnionType.of(new CustomColumn("Timestamp",
CustomColumn.TYPE_FORMAT_DATE, "`HH-mm-ss-SSSSSSSSS`")),
JsTable.CustomColumnArgUnionType.of(Column.formatRowColor("background(RED)"))));
// Wait for this to resolve, part of DH-18634 is that already running tables behave differently
return assertEventFiresOnce(t, JsTable.EVENT_CUSTOMCOLUMNSCHANGED, 2025).then(table -> {
Column iColumn = table.findColumn("I");
Column strColumn = table.findColumn("Str");
Column timestampColumn = table.findColumn("Timestamp");

JsConsumer<TableData> check = data -> {
assertEquals(10, data.getRows().length);
TableData.Row row = data.getRows().getAt(0);

Format iFormat = row.getFormat(iColumn);
assertNotNull(iFormat);
assertEquals("#008000", iFormat.getBackgroundColor());
assertEquals(iFormat, iColumn.getFormat(row));
assertEquals(iFormat, data.getFormat(0, iColumn));

Format strFormat = row.getFormat(strColumn);
assertNotNull(strFormat);
assertEquals("$###,##0.00", strFormat.getFormatString());
assertEquals("#ff0000", strFormat.getBackgroundColor());
assertEquals(strFormat, strColumn.getFormat(row));
assertEquals(strFormat, data.getFormat(0, strColumn));

Format timestampFormat = row.getFormat(timestampColumn);
assertNotNull(timestampFormat);
assertEquals("HH-mm-ss-SSSSSSSSS", timestampFormat.getFormatString());
assertEquals("#ff0000", timestampFormat.getBackgroundColor());
assertEquals(timestampFormat, timestampColumn.getFormat(row));
assertEquals(timestampFormat, data.getFormat(0, timestampColumn));
};

return Promise.resolve((Object) null).then(ignore -> {
TableViewportSubscription subscription = table.setViewport(0, 9, null);

Promise<JsTable> viewportCheck = assertUpdateReceived(table, check::apply, 2507);
Promise<Void> snapshotCheck =
subscription.snapshot(JsRangeSet.ofRange(0, 9), Js.cast(table.getColumns()))
.then(data -> {
check.apply(data);
return null;
});
return Promise.all(viewportCheck, snapshotCheck);
}).then(ignore -> {
table.applySort(new Sort[] {iColumn.sort().asc()});
TableViewportSubscription subscription = table.setViewport(0, 9, null);

Promise<JsTable> viewportCheck = assertUpdateReceived(table, check::apply, 2508);
Promise<Void> snapshotCheck =
subscription.snapshot(JsRangeSet.ofRange(0, 9), Js.cast(table.getColumns()))
.then(data -> {
check.apply(data);
return null;
});
return Promise.all(viewportCheck, snapshotCheck);
}).then(ignore -> Promise.resolve(table));
});
})
.then(t -> {
delayTestFinish(7878);
// Repeat, once more with a row style but no column style
t.applyCustomColumns(JsArray.of(
JsTable.CustomColumnArgUnionType
.of("Timestamp=epochNanosToInstant((i + 1740000000) * 1_000_000_000)"),
JsTable.CustomColumnArgUnionType
.of(new CustomColumn("Str", CustomColumn.TYPE_FORMAT_NUMBER, "`$###,##0.00`")),
JsTable.CustomColumnArgUnionType.of(new CustomColumn("Timestamp",
CustomColumn.TYPE_FORMAT_DATE, "`HH-mm-ss-SSSSSSSSS`")),
JsTable.CustomColumnArgUnionType.of(Column.formatRowColor("background(RED)"))));
// Wait for this to resolve, part of DH-18634 is that already running tables behave differently
return assertEventFiresOnce(t, JsTable.EVENT_CUSTOMCOLUMNSCHANGED, 2025).then(table -> {
Column iColumn = table.findColumn("I");
Column strColumn = table.findColumn("Str");
Column timestampColumn = table.findColumn("Timestamp");

JsConsumer<TableData> check = data -> {
assertEquals(10, data.getRows().length);
TableData.Row row = data.getRows().getAt(0);

Format iFormat = row.getFormat(iColumn);
assertNotNull(iFormat);
assertEquals("#ff0000", iFormat.getBackgroundColor());
assertEquals(iFormat, iColumn.getFormat(row));
assertEquals(iFormat, data.getFormat(0, iColumn));

Format strFormat = row.getFormat(strColumn);
assertNotNull(strFormat);
assertEquals("$###,##0.00", strFormat.getFormatString());
assertEquals("#ff0000", strFormat.getBackgroundColor());
assertEquals(strFormat, strColumn.getFormat(row));
assertEquals(strFormat, data.getFormat(0, strColumn));

Format timestampFormat = row.getFormat(timestampColumn);
assertNotNull(timestampFormat);
assertEquals("HH-mm-ss-SSSSSSSSS", timestampFormat.getFormatString());
assertEquals("#ff0000", timestampFormat.getBackgroundColor());
assertEquals(timestampFormat, timestampColumn.getFormat(row));
assertEquals(timestampFormat, data.getFormat(0, timestampColumn));
};

return Promise.resolve((Object) null).then(ignore -> {
TableViewportSubscription subscription = table.setViewport(0, 9, null);

Promise<JsTable> viewportCheck = assertUpdateReceived(table, check::apply, 2507);
Promise<Void> snapshotCheck =
subscription.snapshot(JsRangeSet.ofRange(0, 9), Js.cast(table.getColumns()))
.then(data -> {
check.apply(data);
return null;
});
return Promise.all(viewportCheck, snapshotCheck);
}).then(ignore -> {
table.applySort(new Sort[] {iColumn.sort().asc()});
TableViewportSubscription subscription = table.setViewport(0, 9, null);

Promise<JsTable> viewportCheck = assertUpdateReceived(table, check::apply, 2508);
Promise<Void> snapshotCheck =
subscription.snapshot(JsRangeSet.ofRange(0, 9), Js.cast(table.getColumns()))
.then(data -> {
check.apply(data);
return null;
});
return Promise.all(viewportCheck, snapshotCheck);
}).then(ignore -> Promise.resolve(table));
});
})
.then(this::finish).catch_(this::report);
}

@Override
public String getModuleName() {
return "io.deephaven.web.DeephavenIntegrationTest";
Expand Down

0 comments on commit 4d57893

Please sign in to comment.