fix: prevent client updates when handling events from client (#6982)

* fix: prevent client updates when handling events from client * address sonar issue
vaadin · Dec 31, 2024 · 77bff59 · 77bff59
1 parent 9a3fc8e
commit 77bff59
Show file tree

Hide file tree

Showing 4 changed files with 58 additions and 7 deletions.
diff --git a/...nt/vaadin-dashboard-flow/src/main/java/com/vaadin/flow/component/dashboard/Dashboard.java b/...nt/vaadin-dashboard-flow/src/main/java/com/vaadin/flow/component/dashboard/Dashboard.java
@@ -64,6 +64,9 @@
 @NpmPackage(value = "@vaadin/dashboard", version = "24.7.0-alpha2")
 public class Dashboard extends Component implements HasWidgets, HasSize {
 
+    private static final ThreadLocal<Boolean> suppressClientUpdates = ThreadLocal
+            .withInitial(() -> false);
+
     private final List<Component> childrenComponents = new ArrayList<>();
 
     private final DashboardChildDetachHandler childDetachHandler;
@@ -541,8 +544,17 @@ Component getItem(int nodeId) {
         }).filter(Objects::nonNull).findAny().orElseThrow();
     }
 
+    private void withoutClientUpdate(Runnable action) {
+        suppressClientUpdates.set(true);
+        try {
+            action.run();
+        } finally {
+            suppressClientUpdates.remove();
+        }
+    }
+
     void updateClient() {
-        if (pendingUpdate) {
+        if (suppressClientUpdates.get() || pendingUpdate) {
             return;
         }
         pendingUpdate = true;
@@ -661,7 +673,6 @@ private void initItemMovedClientEventListener() {
                 return;
             }
             handleItemMovedClientEvent(e, itemKey, itemsKey, sectionKey);
-            updateClient();
         }).addEventData(itemKey).addEventData(itemsKey)
                 .addEventData(sectionKey);
     }
@@ -705,7 +716,6 @@ private void initItemResizedClientEventListener() {
                 return;
             }
             handleItemResizedClientEvent(e, idKey, colspanKey, rowspanKey);
-            updateClient();
         }).addEventData(idKey).addEventData(colspanKey)
                 .addEventData(rowspanKey);
     }
@@ -718,8 +728,10 @@ private void handleItemResizedClientEvent(DomEvent e, String idKey,
         DashboardWidget resizedWidget = getWidgets().stream()
                 .filter(child -> nodeId == child.getElement().getNode().getId())
                 .findAny().orElseThrow();
-        resizedWidget.setRowspan(rowspan);
-        resizedWidget.setColspan(colspan);
+        withoutClientUpdate(() -> {
+            resizedWidget.setColspan(colspan);
+            resizedWidget.setRowspan(rowspan);
+        });
         fireEvent(new DashboardItemResizedEvent(this, true, resizedWidget,
                 getChildren().toList()));
     }
@@ -731,14 +743,13 @@ private void initItemRemovedClientEventListener() {
                 return;
             }
             handleItemRemovedClientEvent(e, idKey);
-            updateClient();
         }).addEventData(idKey);
     }
 
     private void handleItemRemovedClientEvent(DomEvent e, String idKey) {
         int nodeId = (int) e.getEventData().getNumber(idKey);
         Component removedItem = getItem(nodeId);
-        removedItem.removeFromParent();
+        withoutClientUpdate(removedItem::removeFromParent);
         fireEvent(new DashboardItemRemovedEvent(this, true, removedItem,
                 getChildren().toList()));
     }

diff --git a/...shboard-flow/src/test/java/com/vaadin/flow/component/dashboard/DashboardItemMoveTest.java b/...shboard-flow/src/test/java/com/vaadin/flow/component/dashboard/DashboardItemMoveTest.java
@@ -83,6 +83,18 @@ public void setDashboardNotEditable_moveWidget_orderIsNotUpdated() {
         Assert.assertEquals(expectedRootLevelNodeIds, getRootLevelNodeIds());
     }
 
+    @Test
+    public void moveWidget_noClientUpdate() {
+        getUi().getInternals().dumpPendingJavaScriptInvocations();
+
+        assertRootLevelItemMoved(0, 1);
+
+        fakeClientCommunication();
+
+        Assert.assertTrue(getUi().getInternals()
+                .dumpPendingJavaScriptInvocations().isEmpty());
+    }
+
     @Test
     public void moveWidget_eventCorrectlyFired() {
         int initialIndex = 0;

diff --git a/...board-flow/src/test/java/com/vaadin/flow/component/dashboard/DashboardItemResizeTest.java b/...board-flow/src/test/java/com/vaadin/flow/component/dashboard/DashboardItemResizeTest.java
@@ -74,6 +74,18 @@ public void setDashboardNotEditable_resizeWidget_sizeIsNotUpdated() {
         Assert.assertEquals(1, widgetToResize.getRowspan());
     }
 
+    @Test
+    public void resizeWidget_noClientUpdate() {
+        getUi().getInternals().dumpPendingJavaScriptInvocations();
+
+        assertWidgetResized(0, 2, 1);
+
+        fakeClientCommunication();
+
+        Assert.assertTrue(getUi().getInternals()
+                .dumpPendingJavaScriptInvocations().isEmpty());
+    }
+
     @Test
     public void resizeWidget_eventCorrectlyFired() {
         DashboardWidget resizedWidget = (DashboardWidget) dashboard

diff --git a/...aadin-dashboard-flow/src/test/java/com/vaadin/flow/component/dashboard/DashboardTest.java b/...aadin-dashboard-flow/src/test/java/com/vaadin/flow/component/dashboard/DashboardTest.java
@@ -955,6 +955,22 @@ public void setDashboardEditable_removeWidget_widgetIsRemoved() {
         Assert.assertFalse(actualNodeIds.contains(nodeIdToBeRemoved));
     }
 
+    @Test
+    public void setDashboardEditable_removeWidget_noClientUpdate() {
+        DashboardWidget widgetToRemove = getNewWidget();
+        dashboard.add(widgetToRemove);
+        dashboard.setEditable(true);
+        fakeClientCommunication();
+        getUi().getInternals().dumpPendingJavaScriptInvocations();
+
+        int nodeIdToBeRemoved = widgetToRemove.getElement().getNode().getId();
+        DashboardTestHelper.fireItemRemovedEvent(dashboard, nodeIdToBeRemoved);
+        fakeClientCommunication();
+
+        Assert.assertTrue(getUi().getInternals()
+                .dumpPendingJavaScriptInvocations().isEmpty());
+    }
+
     @Test
     public void setDashboardEditable_removeWidget_eventCorrectlyFired() {
         dashboard.setEditable(true);