From 9d10b70f0673b0204b4d56e9850f3a99664d62c5 Mon Sep 17 00:00:00 2001
From: Colin Alworth <colinalworth@deephaven.io>
Date: Thu, 12 Sep 2024 14:54:50 -0500
Subject: [PATCH 1/2] fix: Don't recreate JS TreeTable subscriptions when
 viewport changes

Resolves an issue where subscription recreation would release an export
ticket that was still needed for the next subscription.

Also improves tree viewport performance by sending changes instead of
waiting for a new subscription to exist.

Fixes #6053
---
 .../deephaven/web/client/api/tree/JsTreeTable.java  | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

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 7163ae034d9..72e7ea11116 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
@@ -599,7 +599,8 @@ private void replaceSubscription(RebuildStep step) {
                     viewTicket.release();
                     viewTicket = null;
                 }
-            case SUBSCRIPTION:
+
+                // In all of the above cases, we replace the subscription
                 if (stream != null) {
                     stream.then(stream -> {
                         stream.close();
@@ -607,6 +608,16 @@ private void replaceSubscription(RebuildStep step) {
                     });
                     stream = null;
                 }
+                break;
+            case SUBSCRIPTION:
+                // If it exists, adjust the existing subscription, otherwise create a new one
+                if (stream != null) {
+                    stream.then(subscription -> {
+                        subscription.setViewport(firstRow, lastRow, Js.uncheckedCast(columns), (double) updateInterval);
+                        return null;
+                    });
+                    return;
+                }
         }
 
         Promise<TreeSubscription> stream = Promise.resolve(defer())

From 169e44f360af93a519a417cd766ee912863d7761 Mon Sep 17 00:00:00 2001
From: Colin Alworth <colinalworth@deephaven.io>
Date: Fri, 13 Sep 2024 10:22:47 -0500
Subject: [PATCH 2/2] Add tests for this bug

---
 .../client/api/HierarchicalTableTestGwt.java  | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/web/client-api/src/test/java/io/deephaven/web/client/api/HierarchicalTableTestGwt.java b/web/client-api/src/test/java/io/deephaven/web/client/api/HierarchicalTableTestGwt.java
index 54ace6db598..191b8d73f72 100644
--- a/web/client-api/src/test/java/io/deephaven/web/client/api/HierarchicalTableTestGwt.java
+++ b/web/client-api/src/test/java/io/deephaven/web/client/api/HierarchicalTableTestGwt.java
@@ -50,6 +50,16 @@ public void testStaticTreeTable() {
                                         JsTreeTable.EVENT_UPDATED, 2001d);
                             }).then(event -> {
                                 assertEquals(10d, event.detail.getTreeSize());
+                                assertEquals(10, event.detail.getRows().length);
+
+
+                                // move the viewport and try again
+                                treeTable.setViewport(5, 50, treeTable.getColumns(), null);
+                                return treeTable.<TreeViewportData>nextEvent(
+                                        JsTreeTable.EVENT_UPDATED, 2002d);
+                            }).then(event -> {
+                                assertEquals(10d, event.detail.getTreeSize());
+                                assertEquals(5, event.detail.getRows().length);
 
                                 treeTable.close();
 
@@ -133,6 +143,15 @@ public void testRefreshingTreeTable() {
                                 assertEquals(green, data.getFormat(1, idCol));
                                 assertEquals(green, row2.getFormat(idCol));
                                 assertEquals(green, idCol.getFormat(row2));
+
+                                // Move the viewport and make sure we get the correct data
+                                treeTable.setViewport(5, 49, treeTable.getColumns(), null);
+                                return treeTable.<TreeViewportData>nextEvent(
+                                        JsTreeTable.EVENT_UPDATED, 2002d);
+                            }).then(event -> {
+                                assertEquals(10d, event.detail.getTreeSize());
+                                assertEquals(5, event.detail.getRows().length);
+
                                 return Promise.resolve(treeTable);
                             })
                             .then(event -> {