fix: detect cyclic references in TreeData.setParent() (#23401) (#23406)

Previously, TreeData.setParent() only checked for direct self-parenting
(item.equals(parent)) but did not detect transitive cycles where a
descendant becomes an ancestor. This could corrupt the tree structure,
for example when drag-and-drop in TreeGrid moved a parent node under
one of its descendants.

Added isAncestorOf() helper method that walks up the ancestor chain
to detect if the item being moved is an ancestor of the proposed new
parent. If so, setParent() now throws IllegalArgumentException with
a clear error message.

The cycle check has O(h) complexity where h is the tree height, which
is acceptable for typical tree operations.

Fixes #19337

Co-authored-by: Marco Collovati <marco@vaadin.com>

Author: vaadin-bot

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

@@ -400,6 +400,14 @@ public void setParent(T item, T parent) {
                     "Item cannot be the parent of itself");
         }
 
+        // Check for cyclic reference - item cannot become descendant of its
+        // own descendant
+        if (parent != null && isAncestorOf(item, parent)) {
+            throw new IllegalArgumentException(
+                    "Setting '" + parent + "' as parent of '" + item
+                            + "' would create a cycle in the tree hierarchy");
+        }
+
         T oldParent = itemToWrapperMap.get(item).getParent();
 
         if (!Objects.equals(oldParent, parent)) {
@@ -488,4 +496,29 @@ private void addItemsRecursively(Collection<T> items,
             addItemsRecursively(childItems, childItemProvider);
         });
     }
+
+    /**
+     * Checks if the potential ancestor is an ancestor of the potential
+     * descendant by walking up the ancestor chain from the descendant.
+     *
+     * @param potentialAncestor
+     *            the item to check as a potential ancestor
+     * @param potentialDescendant
+     *            the item to check as a potential descendant
+     * @return true if {@code potentialAncestor} is an ancestor of
+     *         {@code potentialDescendant}
+     */
+    private boolean isAncestorOf(T potentialAncestor, T potentialDescendant) {
+        if (potentialDescendant == null) {
+            return false;
+        }
+        T current = itemToWrapperMap.get(potentialDescendant).getParent();
+        while (current != null) {
+            if (current.equals(potentialAncestor)) {
+                return true;
+            }
+            current = itemToWrapperMap.get(current).getParent();
+        }
+        return false;
+    }
 }

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

@@ -27,6 +27,7 @@
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
+import org.junit.Assert;
 import org.junit.Test;
 
 import com.vaadin.flow.data.provider.DataProviderTestBase;
@@ -167,6 +168,39 @@ public void treeData_move_after_sibling_different_parents() {
         data.moveAfterSibling(root0, wrongSibling);
     }
 
+    @Test
+    public void treeData_setParent_direct_cycle_throws() {
+        // Test direct cycle: parent -> child, then try to set child as parent's
+        // parent
+        StrBean parent = rootData.get(0);
+        StrBean child = data.getChildren(parent).get(0);
+
+        // This should throw because it would create cycle: child -> parent ->
+        // child
+        IllegalArgumentException exception = Assert.assertThrows(
+                IllegalArgumentException.class,
+                () -> data.setParent(parent, child));
+        assertTrue(exception.getMessage().contains("would create a cycle"));
+        assertTrue(exception.getMessage().contains(parent.toString()));
+        assertTrue(exception.getMessage().contains(child.toString()));
+    }
+
+    @Test
+    public void treeData_setParent_multi_level_cycle_throws() {
+        // Test multi-level cycle: A -> B -> C, then try to set C as A's parent
+        StrBean itemA = rootData.get(0);
+        StrBean itemB = data.getChildren(itemA).get(0);
+        StrBean itemC = data.getChildren(itemB).get(0);
+
+        // This should throw because it would create cycle: C -> A -> B -> C
+        IllegalArgumentException exception = Assert.assertThrows(
+                IllegalArgumentException.class,
+                () -> data.setParent(itemA, itemC));
+        assertTrue(exception.getMessage().contains("would create a cycle"));
+        assertTrue(exception.getMessage().contains(itemA.toString()));
+        assertTrue(exception.getMessage().contains(itemC.toString()));
+    }
+
     @Test
     public void treeData_root_items() {
         TreeData<String> data = new TreeData<>();