vaadin / flow

Vaadin Flow is a Java framework binding Vaadin web components to Java. This is part of Vaadin 10+.
Apache License 2.0
605 stars 166 forks source link

IdentifierProvider is not taken into use for determining the equality of the items in ListBox and MultiSelectListBox #9170

Closed taefi closed 3 years ago

taefi commented 3 years ago

Description of the bug

ListBox and MultiSelectListBox are not taking the IdentifierProvider into use while determining the equality of the items for setValue and etc.

Minimal reproducible example

@Test
public void setIdentifierProvider_shouldSelectCorrectItems_dueToIdentifier() {
        CustomItem first = new CustomItem(1L, "First");
        CustomItem second = new CustomItem(2L, "Second");
        CustomItem third = new CustomItem(3L, "Third");
        List<CustomItem> items = new ArrayList<>(Arrays.asList(first, second,
                third));

        MultiSelectListBox<CustomItem> multiSelectListBox =
                new MultiSelectListBox<>();
        ListBoxListDataView<CustomItem> listDataView = multiSelectListBox
                .setItems(items);
        // Setting the following Identifier Provider should make the component
        // independent from the CustomItem's equals method implementation:
        listDataView.setIdentifierProvider(CustomItem::getId);

        multiSelectListBox.setValue(createSet(new CustomItem(1L)));

        long[] selectedIds = multiSelectListBox.getSelectedItems().stream()
                .mapToLong(CustomItem::getId)
                .toArray();
        Assert.assertArrayEquals(new long[] {1L}, selectedIds);

        // Make the names similar to the name of not selected one to mess
        // with the <equals> implementation in CustomItem:
        first.setName("Second");
        listDataView.refreshItem(first);
        third.setName("Second");
        listDataView.refreshItem(third);

        // Select the item not with the reference of existing item, but instead
        // with just the Id:
        multiSelectListBox.setValue(Collections.singleton(new CustomItem(2L)));

        selectedIds = multiSelectListBox.getSelectedItems()
                .stream()
                .mapToLong(CustomItem::getId)
                .toArray();

        // The following expects the selected item to be the one with Id = 2L
        // But it fails since the setValue ignores setting the new selection because 
        // the <equals> method of CustomItem is based on names.
        Assert.assertArrayEquals(new long[] {2L}, selectedIds);
}

public class CustomItem {
    private Long id;
    private String name;

    public CustomItem(Long id) {
        this(id, null);
    }

    public CustomItem(Long id, String name) {
        this.id = id;
        this.name = name;
    }

    public Long getId() {
        return id;
    }

    public String getName() {
        return name;
    }

    public void setName(String name) {
        this.name = name;
    }

    @Override
    public boolean equals(Object o) {
        if (this == o) return true;
        if (!(o instanceof CustomItem)) return false;
        CustomItem that = (CustomItem) o;
        return Objects.equals(getName(), that.getName());
    }

    @Override
    public int hashCode() {
        return Objects.hash(getName());
    }
}

Expected behavior

To set the selection of MultiSelectListBox based on the equality of the value provided as ID by the IdentifierProvider.

Actual behavior

It is setting the selection based the behavior of the equals method of the CustomItem.

Versions:

- Vaadin / Flow version: 18.0-SNAPSHOT
taefi commented 3 years ago

This issue is related to the vaadin-flow-component repository. The proper issue is created for this:

https://github.com/vaadin/vaadin-flow-components/issues/227