fix: remove unnecessary clear condition (#15282) (#15304)

For hierarchical data, even when the child components are filtered out, the clear method on ArrayUpdater.Update in HierarchicalCommunicationController is never called. This is caused by an if check, in which the start index of previousActive is compared to the newly calculated assumedSize. Since there might be items to clear, the newly calculated size should have no effect on whether we clear the items or not. This check is not present in the DataCommunicator counterpart within the method collectChangesToSend. This PR removes the aforementioned clear condition. Add test to ensure that the expanded children are cleared properly when filtered out. Fixes #4132 Co-authored-by: Ugur Saglam <[email protected]>
vaadin · Nov 29, 2022 · f1a0e86 · f1a0e86
1 parent d524bbc
commit f1a0e86
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 2 deletions.
diff --git a/...ain/java/com/vaadin/flow/data/provider/hierarchy/HierarchicalCommunicationController.java b/...ain/java/com/vaadin/flow/data/provider/hierarchy/HierarchicalCommunicationController.java
@@ -210,7 +210,7 @@ private void set(Range effectiveRequested, HierarchicalUpdate update) {
     }
 
     private void clear(int start, int length, HierarchicalUpdate update) {
-        if (length == 0 || start >= assumedSize) {
+        if (length == 0) {
             return;
         }
         if (parentKey == null) {

diff --git a/...c/test/java/com/vaadin/flow/data/provider/hierarchy/HierarchicalCommunicatorDataTest.java b/...c/test/java/com/vaadin/flow/data/provider/hierarchy/HierarchicalCommunicatorDataTest.java
@@ -19,7 +19,9 @@
 import java.util.Arrays;
 import java.util.List;
 import java.util.stream.IntStream;
+import java.util.stream.Stream;
 
+import com.vaadin.flow.function.SerializablePredicate;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
@@ -81,7 +83,10 @@ public int hashCode() {
     private MockUI ui;
     private Element element;
 
-    private static class UpdateQueue implements HierarchicalUpdate {
+    private boolean parentClearCalled = false;
+
+    private class UpdateQueue implements HierarchicalUpdate {
+
         @Override
         public void clear(int start, int length) {
         }
@@ -104,6 +109,7 @@ public void set(int start, List<JsonValue> items, String parentKey) {
 
         @Override
         public void clear(int start, int length, String parentKey) {
+            parentClearCalled = true;
         }
 
         @Override
@@ -213,6 +219,28 @@ public void uniqueKeyProviderIsNotSet_keysGeneratedByKeyMapper() {
                         communicator.getKeyMapper().get(i)));
     }
 
+    @Test
+    public void expandRoot_filterOutAllChildren_clearCalled() {
+        parentClearCalled = false;
+
+        communicator.expand(ROOT);
+        fakeClientCommunication();
+
+        communicator.setParentRequestedRange(0, 50, ROOT);
+        fakeClientCommunication();
+
+        SerializablePredicate<Item> filter = item -> ROOT.equals(item);
+        communicator.setFilter(filter);
+        fakeClientCommunication();
+
+        dataProvider.refreshItem(ROOT, true);
+        fakeClientCommunication();
+
+        communicator.reset();
+
+        Assert.assertTrue(parentClearCalled);
+    }
+
     private void fakeClientCommunication() {
         ui.getInternals().getStateTree().runExecutionsBeforeClientResponse();
         ui.getInternals().getStateTree().collectChanges(ignore -> {