Skip to content

Commit

Permalink
fix: JS Barrage snapshots must overwrite server viewport (deephaven#6148
Browse files Browse the repository at this point in the history 
)

Also fixed an error when the table (or viewport) is empty.

Fixes deephaven#6138
  • Loading branch information
@niloc132
niloc132 authored Sep 30, 2024
1 parent 057c0a6 commit cd81774
Show file tree
Hide file tree
Showing 2 changed files with 77 additions and 5 deletions.
17 changes: 13 additions & 4 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 @@ -383,9 +383,12 @@ public Promise<TableData> snapshot(JsRangeSet rows, Column[] columns) {
if (message != null) {
// Replace rowsets with flat versions
long resultSize = message.rowsIncluded.size();
message.rowsAdded = RangeSet.ofRange(rowsReceived.get(), rowsReceived.get() + resultSize - 1);
message.rowsIncluded = message.rowsAdded;
rowsReceived.add(resultSize);
if (resultSize != 0) {
message.rowsAdded = RangeSet.ofRange(rowsReceived.get(), rowsReceived.get() + resultSize - 1);
message.rowsIncluded = message.rowsAdded;
message.snapshotRowSet = null;
rowsReceived.add(resultSize);
}

// Update our table data with the complete message
snapshot.applyUpdates(message);
Expand Down Expand Up @@ -420,8 +423,14 @@ public Promise<TableData> snapshot(JsRangeSet rows, Column[] columns) {
doExchange.onEnd(status -> {
if (status.isOk()) {
// notify the caller that the snapshot is finished
RangeSet result;
if (rowsReceived.get() != 0) {
result = RangeSet.ofRange(0, rowsReceived.get() - 1);
} else {
result = RangeSet.empty();
}
resolve.onInvoke(new SubscriptionEventData(snapshot, rowStyleColumn, Js.uncheckedCast(columns),
RangeSet.ofRange(0, rowsReceived.get() - 1),
result,
RangeSet.empty(),
RangeSet.empty(),
null));
Expand Down
65 changes: 64 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 @@ -11,14 +11,21 @@
import io.deephaven.web.client.api.AbstractAsyncGwtTestCase;
import io.deephaven.web.client.api.Column;
import io.deephaven.web.client.api.HasEventHandling;
import io.deephaven.web.client.api.JsRangeSet;
import io.deephaven.web.client.api.JsTable;
import io.deephaven.web.client.api.TableData;
import io.deephaven.web.client.api.filter.FilterCondition;
import io.deephaven.web.client.api.filter.FilterValue;
import io.deephaven.web.shared.fu.RemoverFn;
import jsinterop.base.Any;
import jsinterop.base.Js;

import java.util.ArrayList;
import java.util.List;
import java.util.Objects;
import java.util.function.Supplier;
import java.util.stream.Collectors;
import java.util.stream.IntStream;

import static elemental2.dom.DomGlobal.console;

Expand All @@ -44,7 +51,8 @@ public class ViewportTestGwt extends AbstractAsyncGwtTestCase {
.script("growingBackward",
"growingForward.sort_descending(\"Timestamp\").format_columns(['I=I>2 ? GREEN : RED'])")
.script("blinkOne",
"time_table(\"PT00:00:01\").update([\"I=i\", \"J=1\"]).last_by(by=\"J\").where(\"I%2 != 0\")");
"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')");

public void testViewportOnStaticTable() {
connect(tables)
Expand Down Expand Up @@ -426,6 +434,61 @@ public void testViewportWithNoInitialItems() {
.then(this::finish).catch_(this::report);
}

public void testSnapshotFromViewport() {
connect(tables)
.then(table("big"))
.<Object>then(table -> {
delayTestFinish(20_001);

Column[] all = Js.uncheckedCast(table.getColumns());
// This table is static and non-flat, to make sure our calls will make sense to get data.
// Subscribe to a viewport, and grab some rows in a snapshot
TableViewportSubscription tableViewportSubscription = table.setViewport(100, 109);

List<Supplier<Promise<Object>>> tests = new ArrayList<>();
// Data within the viewport
JsRangeSet r = JsRangeSet.ofRange(101, 108);
tests.add(() -> tableViewportSubscription.snapshot(r, all)
.then(snapshot -> checkSnapshot(snapshot, r.getRange().size())));

// Each row within the viewport
tests.addAll(
IntStream.range(100, 110)
.<Supplier<Promise<Object>>>mapToObj(row -> () -> tableViewportSubscription
.snapshot(JsRangeSet.ofRange(row, row), all)
.then(data -> checkSnapshot(data, 1)))
.collect(Collectors.toList()));

// Data overlapping the viewport
JsRangeSet r1 = JsRangeSet.ofRange(0, 120);
tests.add(() -> tableViewportSubscription.snapshot(r1, all)
.then(snapshot -> checkSnapshot(snapshot, r1.getRange().size())));

// Empty snapshot
tests.add(() -> tableViewportSubscription.snapshot(JsRangeSet.ofItems(new double[0]), all)
.then(snapshot -> checkSnapshot(snapshot, 0)));


// Run the tests serially
return tests.stream().reduce((p1, p2) -> () -> p1.get().then(result -> p2.get())).get().get();
})
.then(this::finish).catch_(this::report);
}

private Promise<Object> checkSnapshot(TableData data, long expectedSize) {
Column intCol = data.getColumns().at(0);
Column strCol = data.getColumns().at(1);
assertEquals(expectedSize, data.getRows().length);
for (int i = 0; i < data.getRows().length; i++) {
Any intVal = data.getData(i, intCol);
assertNotNull(intVal);
Any strVal = data.getData(i, strCol);
assertNotNull(strVal);
assertEquals(intVal.toString(), strVal.asString());
}
return Promise.resolve(data);
}

private IThenable<JsTable> helperForViewportWithNoInitialItems(JsTable t, Column[] requestColumns,
JsArray<Column> expectedColumns) {
// wait until zero rows are present, so we can set the viewport and get a zero-row "snapshot"
Expand Down

0 comments on commit cd81774

Please sign in to comment.