Skip to content

Commit ab13f0c

Browse files
niloc132niloc132
authored
fix: JS ChartData should write updates to the correct row (#6638) (#6643)
1 parent de0b2f0 commit ab13f0c

File tree

3 files changed

+290
-11
lines changed

3 files changed

+290
-11
lines changed

‎web/client-api/src/main/java/io/deephaven/web/client/api/widget/plot/ChartData.java

+3-4
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
import io.deephaven.web.client.api.Column;
88
import io.deephaven.web.client.api.JsTable;
99
import io.deephaven.web.client.api.TableData;
10-
import io.deephaven.web.client.api.subscription.AbstractTableSubscription;
1110
import io.deephaven.web.client.api.subscription.SubscriptionTableData;
1211
import io.deephaven.web.shared.data.Range;
1312
import io.deephaven.web.shared.data.RangeSet;
@@ -35,7 +34,7 @@ public ChartData(JsTable table) {
3534
this.table = table;
3635
}
3736

38-
public void update(AbstractTableSubscription.SubscriptionEventData tableData) {
37+
public void update(SubscriptionTableData tableData) {
3938
RangeSet positionsForAddedKeys = tableData.getFullIndex().getRange().invert(tableData.getAdded().getRange());
4039
RangeSet positionsForRemovedKeys = prevRanges.invert(tableData.getRemoved().getRange());
4140
RangeSet positionsForModifiedKeys =
@@ -82,14 +81,14 @@ private void replaceDataRange(SubscriptionTableData tableData, Range positions)
8281

8382
// rather than getting a slice and splicing it in, just update each value
8483
PrimitiveIterator.OfLong iter = keys.indexIterator();
85-
int i = 0;
84+
int i = (int) positions.getFirst();
8685
if (func == null) {
8786
while (iter.hasNext()) {
8887
arr.setAt(i++, tableData.getData(iter.next(), col));
8988
}
9089
} else {
9190
while (iter.hasNext()) {
92-
arr.setAt(i++, tableData.getData(iter.next(), col));
91+
arr.setAt(i++, func.apply(tableData.getData(iter.next(), col)));
9392
}
9493
}
9594
}

‎web/client-api/src/test/java/io/deephaven/web/client/api/AbstractAsyncGwtTestCase.java

+6-7
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,8 @@ private static Promise<Void> importDhInternal() {
5656
public static class TableSourceBuilder {
5757
private final List<String> pythonScripts = new ArrayList<>();
5858

59-
public TableSourceBuilder script(String script) {
60-
pythonScripts.add(script);
59+
public TableSourceBuilder script(String... script) {
60+
pythonScripts.addAll(Arrays.asList(script));
6161
return this;
6262
}
6363

@@ -156,7 +156,7 @@ protected Promise<IdeSession> connect(TableSourceBuilder tables) {
156156
return ideSession.then(session -> {
157157

158158
if (consoleTypes.includes("python")) {
159-
return runAllScriptsInOrder(ideSession, session, tables.pythonScripts);
159+
return runAllScriptsInOrder(session, tables.pythonScripts);
160160
}
161161
throw new IllegalStateException("Unsupported script type " + consoleTypes);
162162
});
@@ -165,9 +165,8 @@ protected Promise<IdeSession> connect(TableSourceBuilder tables) {
165165
});
166166
}
167167

168-
private Promise<IdeSession> runAllScriptsInOrder(CancellablePromise<IdeSession> ideSession, IdeSession session,
169-
List<String> code) {
170-
Promise<IdeSession> result = ideSession;
168+
private Promise<IdeSession> runAllScriptsInOrder(IdeSession session, List<String> code) {
169+
Promise<IdeSession> result = Promise.resolve(session);
171170
for (int i = 0; i < code.size(); i++) {
172171
final int index = i;
173172
result = result.then(ignore -> {
@@ -178,7 +177,7 @@ private Promise<IdeSession> runAllScriptsInOrder(CancellablePromise<IdeSession>
178177
if (r.getError() != null) {
179178
return Promise.reject(r.getError());
180179
}
181-
return ideSession;
180+
return Promise.resolve(session);
182181
});
183182
}
184183
return result;

‎web/client-api/src/test/java/io/deephaven/web/client/api/widget/plot/ChartDataTestGwt.java

+281
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,281 @@
1+
//
2+
// Copyright (c) 2016-2024 Deephaven Data Labs and Patent Pending
3+
//
4+
package io.deephaven.web.client.api.widget.plot;
5+
6+
import elemental2.core.JsArray;
7+
import elemental2.promise.Promise;
8+
import io.deephaven.web.client.api.AbstractAsyncGwtTestCase;
9+
import io.deephaven.web.client.api.Column;
10+
import io.deephaven.web.client.api.JsTable;
11+
import io.deephaven.web.client.api.subscription.AbstractTableSubscription;
12+
import io.deephaven.web.client.api.subscription.SubscriptionTableData;
13+
import io.deephaven.web.client.api.subscription.TableSubscription;
14+
import io.deephaven.web.shared.fu.JsFunction;
15+
import jsinterop.base.Any;
16+
import jsinterop.base.Js;
17+
import jsinterop.base.JsPropertyMap;
18+
19+
import java.io.Serializable;
20+
import java.util.function.BiConsumer;
21+
22+
public class ChartDataTestGwt extends AbstractAsyncGwtTestCase {
23+
private final AbstractAsyncGwtTestCase.TableSourceBuilder tables = new AbstractAsyncGwtTestCase.TableSourceBuilder()
24+
.script("from deephaven import time_table")
25+
.script("appending_table", "time_table('PT0.1s').update([\"A = i % 2\", \"B = `` + A\"])")
26+
.script("updating_table", "appending_table.last_by('A')");
27+
28+
private final TableSourceBuilder tickingData = new TableSourceBuilder()
29+
.script("from deephaven import input_table",
30+
"from deephaven import dtypes as dht",
31+
"import jpy",
32+
"from deephaven.execution_context import ExecutionContext, get_exec_ctx",
33+
"my_col_defs = {\"ID\": dht.int32, \"Value\": dht.string, \"Deleted\": dht.bool_}",
34+
"ug = jpy.get_type('io.deephaven.engine.updategraph.impl.EventDrivenUpdateGraph').newBuilder('test').existingOrBuild()",
35+
"exec_ctx = ExecutionContext(get_exec_ctx().j_object.withUpdateGraph(ug))",
36+
"with exec_ctx:\n" +
37+
" input = input_table(col_defs=my_col_defs, key_cols=\"ID\")\n" +
38+
" result = input.without_attributes('InputTable').where(\"!Deleted\").sort(\"ID\")");
39+
40+
@Override
41+
public String getModuleName() {
42+
return "io.deephaven.web.DeephavenIntegrationTest";
43+
}
44+
45+
public void testAppendingChartData() {
46+
connect(tables)
47+
.then(table("appending_table"))
48+
.then(table -> {
49+
ChartData chartData = new ChartData(table);
50+
TableSubscription sub = table.subscribe(table.getColumns());
51+
52+
// Handle at least one event with data
53+
int[] count = {0};
54+
55+
return new Promise<AbstractTableSubscription.SubscriptionEventData>((resolve, reject) -> {
56+
delayTestFinish(12301);
57+
sub.addEventListener(TableSubscription.EVENT_UPDATED, event -> {
58+
AbstractTableSubscription.SubscriptionEventData data =
59+
(AbstractTableSubscription.SubscriptionEventData) event.getDetail();
60+
chartData.update(data);
61+
62+
if (count[0] > 0) {
63+
// Don't test on the first update, could still be empty
64+
assertTrue(chartData.getColumn("A", arr -> arr, data).length > 0);
65+
66+
resolve.onInvoke(data);
67+
}
68+
count[0]++;
69+
});
70+
}).then(data -> {
71+
sub.close();
72+
table.close();
73+
return Promise.resolve(data);
74+
});
75+
})
76+
.then(this::finish).catch_(this::report);
77+
}
78+
79+
public void testModifiedChartData() {
80+
connect(tables)
81+
.then(table("updating_table"))
82+
.then(table -> {
83+
ChartData chartData = new ChartData(table);
84+
TableSubscription sub = table.subscribe(table.getColumns());
85+
86+
int[] count = {0};
87+
return new Promise<AbstractTableSubscription.SubscriptionEventData>((resolve, reject) -> {
88+
delayTestFinish(12302);
89+
sub.addEventListener(TableSubscription.EVENT_UPDATED, event -> {
90+
91+
AbstractTableSubscription.SubscriptionEventData data =
92+
(AbstractTableSubscription.SubscriptionEventData) event.getDetail();
93+
94+
chartData.update(data);
95+
if (count[0] > 0) {
96+
// Don't test on the first update, could still be empty
97+
assertTrue(chartData.getColumn("A", arr -> arr, data).length > 0);
98+
}
99+
100+
if (count[0] > 2) {
101+
// We've seen at least three updates, and must have modified rows at least once
102+
resolve.onInvoke((AbstractTableSubscription.SubscriptionEventData) event.getDetail());
103+
}
104+
count[0]++;
105+
});
106+
})
107+
.then(data -> {
108+
sub.close();
109+
table.close();
110+
return Promise.resolve(data);
111+
});
112+
})
113+
.then(this::finish).catch_(this::report);
114+
}
115+
116+
public void testGeneratedChartData() {
117+
connect(tickingData)
118+
.then(ideSession -> {
119+
delayTestFinish(9870);
120+
Promise<JsTable> inputPromise = ideSession.getTable("input", false);
121+
Promise<JsTable> resultPromise = ideSession.getTable("result", false);
122+
return inputPromise.then(JsTable::inputTable).then(delayFinish(9871)).then(input -> {
123+
delayTestFinish(9872);
124+
return resultPromise.then(result -> {
125+
delayTestFinish(9873);
126+
ChartData chartData = new ChartData(result);
127+
128+
// Keep the same function instance, as it is used for caching data
129+
JsFunction<Any, Any> dataFunction = arr -> Js.asAny("::" + arr);
130+
131+
BiConsumer<SubscriptionTableData, SubscriptionTableData> consistencyCheck =
132+
(subscriptionUpdate, snapshot) -> {
133+
chartData.update(subscriptionUpdate);
134+
// Verify the "Value" column, the only mutable column, has the same contents
135+
// regardless of source
136+
String[] expectedValues = new String[snapshot.getRows().length];
137+
snapshot.getRows().forEach((row, index) -> {
138+
expectedValues[index] = row.get(result.findColumn("Value")).asString();
139+
return null;
140+
});
141+
JsArray<Any> values = chartData.getColumn("Value", null, subscriptionUpdate);
142+
JsArray<Any> valuesWithPrefix =
143+
chartData.getColumn("Value", dataFunction, subscriptionUpdate);
144+
assertEquals(values.length, valuesWithPrefix.length);
145+
assertEquals(expectedValues.length, values.length);
146+
for (int i = 0; i < expectedValues.length; i++) {
147+
assertEquals(expectedValues[i], values.getAt(i).asString());
148+
assertEquals("::" + expectedValues[i],
149+
valuesWithPrefix.getAt(i).asString());
150+
}
151+
};
152+
return subscriptionValidator(result, result.getColumns())
153+
// Check initial state (empty)
154+
.check(consistencyCheck)
155+
// Add three rows
156+
.when(() -> {
157+
input.addRows(new JsPropertyMap[] {
158+
row(1, "A"),
159+
row(2, "B"),
160+
row(5, "E")
161+
}, null);
162+
})
163+
.check(consistencyCheck)
164+
// Add two more rows, causing a shift
165+
.when(() -> {
166+
input.addRows(new JsPropertyMap[] {
167+
row(3, "C"),
168+
row(4, "D")
169+
}, null);
170+
})
171+
.check(consistencyCheck)
172+
// Update an existing row
173+
.when(() -> {
174+
input.addRows(new JsPropertyMap[] {
175+
row(3, "F")
176+
}, null);
177+
})
178+
.check(consistencyCheck)
179+
// Delete two rows
180+
.when(() -> {
181+
input.addRows(new JsPropertyMap[] {
182+
delete(1),
183+
delete(3)
184+
}, null);
185+
})
186+
.check(consistencyCheck)
187+
.cleanup();
188+
});
189+
});
190+
})
191+
.then(this::finish).catch_(this::report);
192+
}
193+
194+
/**
195+
* Helper to validate subscription results by comparing an updated subscription against a fresh snapshot. Intended
196+
* to be generalized for use in viewport tests, but haven't yet done this.
197+
*/
198+
private SubscriptionValidator subscriptionValidator(JsTable table, JsArray<Column> subscriptionColumns) {
199+
TableSubscription subscription = table.subscribe(subscriptionColumns);
200+
201+
return new SubscriptionValidator() {
202+
Promise<?> nextStep = Promise.resolve(subscription);
203+
204+
@Override
205+
public SubscriptionValidator when(Runnable setup) {
206+
// Run the step when the previous step completes
207+
nextStep = nextStep.then(ignore -> {
208+
setup.run();
209+
return Promise.resolve(ignore);
210+
});
211+
return this;
212+
}
213+
214+
@Override
215+
public SubscriptionValidator check(BiConsumer<SubscriptionTableData, SubscriptionTableData> check) {
216+
// Run this as a step once the previous step completes, by waiting for the original subscription to
217+
// update, then creating a new subscription and waiting for that as well. Pass both updates to the
218+
// provided check function.
219+
nextStep = nextStep.then(ignore -> new Promise<>((resolve, reject) -> {
220+
try {
221+
subscription.addEventListenerOneShot(TableSubscription.EVENT_UPDATED, e1 -> {
222+
try {
223+
SubscriptionTableData data = (SubscriptionTableData) e1.getDetail();
224+
// Now that this update has happened, we make a new subscription to get a single
225+
// snapshot of the table
226+
TableSubscription checkSub = table.subscribe(subscriptionColumns);
227+
checkSub.addEventListenerOneShot(TableSubscription.EVENT_UPDATED, e2 -> {
228+
try {
229+
SubscriptionTableData checkData = (SubscriptionTableData) e2.getDetail();
230+
check.accept(data, checkData);
231+
resolve.onInvoke(checkData);
232+
} catch (Throwable e) {
233+
reject.onInvoke(e);
234+
} finally {
235+
checkSub.close();
236+
}
237+
});
238+
} catch (Throwable e) {
239+
reject.onInvoke(e);
240+
}
241+
});
242+
} catch (Throwable e) {
243+
reject.onInvoke(e);
244+
}
245+
}));
246+
return this;
247+
}
248+
249+
@Override
250+
public Promise<?> cleanup() {
251+
// Finished, clean up after ourselves, emit success/failure
252+
return nextStep.finally_(subscription::close);
253+
}
254+
};
255+
}
256+
257+
private interface SubscriptionValidator {
258+
/**
259+
* Performs some step that will lead to the table subscription being updated.
260+
*/
261+
SubscriptionValidator when(Runnable setup);
262+
263+
/**
264+
* Checks that the actual and expected data match. Must complete synchronously.
265+
*/
266+
SubscriptionValidator check(BiConsumer<SubscriptionTableData, SubscriptionTableData> check);
267+
268+
/**
269+
* Cleans up after validation work, returning a promise that the test can await.
270+
*/
271+
Promise<?> cleanup();
272+
}
273+
274+
private static JsPropertyMap<? extends Serializable> row(int id, String value) {
275+
return JsPropertyMap.of("ID", (double) id, "Value", value, "Deleted", false);
276+
}
277+
278+
private static JsPropertyMap<? extends Serializable> delete(int id) {
279+
return JsPropertyMap.of("ID", (double) id, "Value", "deleted", "Deleted", true);
280+
}
281+
}

0 commit comments

Comments
 (0)