call destroyData before removing item from keyMapper

Author: vursen

flow-data/src/main/java/com/vaadin/flow/data/provider/hierarchy/HierarchicalDataCommunicator.java

@@ -745,8 +745,8 @@ public void confirmUpdate(int updateId) {
         // Remove those items from the keyMapper and dataGenerator to
         // release any associated client-side resources.
         itemsToRemove.forEach((item) -> {
-            keyMapper.remove(item);
             dataGenerator.destroyData(item);
+            keyMapper.remove(item);
         });
     }
 

flow-data/src/test/java/com/vaadin/flow/data/provider/hierarchy/HierarchicalDataCommunicatorDataGenerationTest.java

@@ -17,6 +17,7 @@
 
 import java.util.Arrays;
 
+import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
 import org.mockito.Mock;
@@ -48,6 +49,16 @@ public void init() {
         dataCommunicator.setDataProvider(treeDataProvider, null);
 
         compositeDataGenerator.addDataGenerator(dataGenerator);
+
+        Mockito.doAnswer((invocation) -> {
+            Item item = invocation.getArgument(0);
+
+            Assert.assertTrue(
+                    "Item should be in keyMapper when generateData is called",
+                    dataCommunicator.getKeyMapper().has(item));
+
+            return null;
+        }).when(dataGenerator).destroyData(Mockito.any());
     }
 
     @Test
@@ -153,24 +164,4 @@ public void collapseItems_destroyDataCalledForCollapsedChildrenInKeyMapper() {
         Mockito.verify(dataGenerator, Mockito.never())
                 .destroyData(new Item("Item 1"));
     }
-
-    @Test
-    public void collapseItems_destroyDataCalledBeforeItemRemovedFromKeyMapper() {
-        populateTreeData(treeData, 4, 2, 2);
-        dataCommunicator.expand(
-                Arrays.asList(new Item("Item 0"), new Item("Item 0-0")));
-        dataCommunicator.setViewportRange(0, 4);
-        fakeClientCommunication();
-
-        Mockito.doAnswer((invocation) -> {
-            Item item = invocation.getArgument(0);
-            if (!dataCommunicator.getKeyMapper().has(item)) {
-                throw new AssertionError(
-                        "Item should still be in keyMapper when destroyData is called");
-            }
-            return null;
-        }).when(dataGenerator).destroyData(Mockito.any());
-
-        dataCommunicator.collapse(new Item("Item 0"));
-    }
 }