From 8b14faf417f6b8c5517d1efe852067d9cf5fde28 Mon Sep 17 00:00:00 2001
From: Ugur Saglam <106508695+ugur-vaadin@users.noreply.github.com>
Date: Tue, 29 Nov 2022 11:59:27 +0200
Subject: [PATCH] fix: remove unnecessary clear condition (#15282)

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
---
 .../HierarchicalCommunicationController.java  |  2 +-
 .../HierarchicalCommunicatorDataTest.java     | 30 ++++++++++++++++++-
 2 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/flow-data/src/main/java/com/vaadin/flow/data/provider/hierarchy/HierarchicalCommunicationController.java b/flow-data/src/main/java/com/vaadin/flow/data/provider/hierarchy/HierarchicalCommunicationController.java
index 5833669a482..08da30f82d3 100644
--- a/flow-data/src/main/java/com/vaadin/flow/data/provider/hierarchy/HierarchicalCommunicationController.java
+++ b/flow-data/src/main/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/flow-data/src/test/java/com/vaadin/flow/data/provider/hierarchy/HierarchicalCommunicatorDataTest.java b/flow-data/src/test/java/com/vaadin/flow/data/provider/hierarchy/HierarchicalCommunicatorDataTest.java
index 2db26ad11eb..2b8b0a939f2 100644
--- a/flow-data/src/test/java/com/vaadin/flow/data/provider/hierarchy/HierarchicalCommunicatorDataTest.java
+++ b/flow-data/src/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 -> {