From 54adac7d3314e7857b6c04e372040d547ae18c84 Mon Sep 17 00:00:00 2001
From: Vaadin Bot <vaadin-bot@users.noreply.github.com>
Date: Tue, 29 Nov 2022 12:12:25 +0100
Subject: [PATCH] fix: remove unnecessary clear condition (#15282) (#15307)

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 <106508695+ugur-vaadin@users.noreply.github.com>
---
 .../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 4b050710359..3cd861f0fc4 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 1c6db18f890..72046572f32 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;
@@ -90,7 +92,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) {
         }
@@ -113,6 +118,7 @@ public void set(int start, List<JsonValue> items, String parentKey) {
 
         @Override
         public void clear(int start, int length, String parentKey) {
+            parentClearCalled = true;
         }
 
         @Override
@@ -222,6 +228,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 -> {