HierarchicalDataProvider.refreshItem(null) throws NullPointerException, see video

[enver-haase]

Description of the bug

The 'parent' of a so-called "root node" is NULL. Hence it is very surprising that I cannot refresh the NULL root node. (Or shall I call it 'super root' now that the tree root's child nodes are called the 'root nodes'?)

Expected behavior

I expect refreshItem(null) to be equivalent to refreshAll().

Minimal reproducible example

package org.vaadin.example;

import com.vaadin.flow.component.grid.Grid;
import com.vaadin.flow.component.grid.dnd.GridDropLocation;
import com.vaadin.flow.component.grid.dnd.GridDropMode;
import com.vaadin.flow.component.notification.Notification;
import com.vaadin.flow.component.treegrid.TreeGrid;

import com.vaadin.flow.component.orderedlayout.VerticalLayout;
import com.vaadin.flow.data.provider.hierarchy.TreeData;
import com.vaadin.flow.router.Route;

import java.util.ArrayList;
import java.util.List;


@Route
public class MainView extends VerticalLayout {

    private static class DemoTreeData extends TreeData<String> {
        private DemoTreeData() {
            final String[] five = {"A", "B", "C", "D", "E"};

            addRootItems(five);

            for (String s : five) {
                List<String> children = new ArrayList<>();
                for (String ch : five) {
                    String child = s+ch;
                    children.add(child);
                }

                addItems(s, children);
            }
        }
    }
    public MainView() {

        TreeGrid<String> treeGrid = new TreeGrid<>();

        treeGrid.addHierarchyColumn(String::toString).setHeader("Name");

        treeGrid.setTreeData(new DemoTreeData());

        treeGrid.setDropMode(GridDropMode.ON_TOP_OR_BETWEEN);
        treeGrid.setSelectionMode(Grid.SelectionMode.SINGLE);
        treeGrid.setRowsDraggable(true);

        List<String> dragged = new ArrayList<>();
        treeGrid.addDragStartListener( evt -> {
            dragged.clear();
            dragged.addAll(evt.getDraggedItems());
        } );

        treeGrid.addDropListener( evt -> {
            try {
                if (GridDropLocation.ON_TOP == evt.getDropLocation()) {
                    evt.getDropTargetItem().ifPresent(s -> {
                        for (String drag : dragged) {
                            String oldParent = treeGrid.getTreeData().getParent(s);
                            treeGrid.getTreeData().setParent(drag, s);

                            //if (oldParent != null) {
                                // This throws a NullPointerException for the NULL parent.
                                treeGrid.getDataProvider().refreshItem(oldParent, true);
                                treeGrid.getDataProvider().refreshItem(s, true);
                            //}
                            //else {
                            //    treeGrid.getDataProvider().refreshAll();
                            //}
                        }
                    });
                }
            }
            catch (Exception e) {
                Notification.show("Exception: "+e);
            }
        } );

        treeGrid.setWidthFull();
        treeGrid.setHeight("800px");
        add(treeGrid);
    }

}

Versions

• Vaadin / Flow version: 24.3.10
• Java version: n/a
• OS version: n/a
• Browser version (if applicable): n/a
• Application Server (if applicable): n/a
• IDE (if applicable): n/a

[enver-haase]

https://github.com/vaadin/flow/assets/7870436/d84b0e41-6657-40cf-aba8-e4920734bd0d

[mshabarov]

To me the connection between null parent item in a TreeGrid and expectation that the refreshItem(null) has the same effect as refreshAll() is questionable.

If we apply this logic, this could be a problem potentially, because Flow would refresh all the items in component once you have unexpected null value for an item that is passed into this method.

It's probably better to have a reserved constant or object for the root element, or an additional method in TreeGrid.

By the way, why don't you just call refreshAll() ?

[enver-haase]

Not using refreshAll() to save resources, and also to point out crazy behaviours of TreeGrid and friends, because I need it fixed for a project. refreshAll() also exposes https://github.com/vaadin/flow-components/issues/1236 .