com.vaadin.ui.components.grid.MultiSelectionModelImpl: SelectAll behaves strangely

[wfischlein]

Vaadin Framework version 8.4.5, com.vaadin.ui.components.grid.MultiSelectionModelImpl
scenario:

• create a grid that holds n items that are NOT supposed to implement equals/hashcode (n > 1)
• make it multi-selectable and make the select-all box visible - like this:
  MultiSelectionModel selectionModel = (MultiSelectionModel) grid.setSelectionMode(Grid.SelectionMode.MULTI);
  selectionModel.setSelectAllCheckBoxVisibility(MultiSelectionModel.SelectAllCheckBoxVisibility.VISIBLE);
• override 'getId' in the dataProvider of the grid to make the items properly identifyable
• add a selection listener to the grid
• run the test app, watch the events passed to the listener
• click 'select all' -> n items are passed (correct)
• unselect one if the items: n - 1 items are passed (correct)
• click 'select all' again: (n - 1) plus n items are passed (wrong. expected: n items)

Problem:
com.vaadin.ui.components.grid.MultiSelectionModelImpl#onSelectAll (vaadin 8.4.5) line 327 adding to the allItems-Set doesn’t care about the getId-method of the dataProvider but relies on the hashcode-method of the given item

[AndreiBoaghe]

Are you serious? It's not about Vaadin...

HashMap calculates hash for the provided key (for your item), on put() method calling.

    public boolean add(E e) {
        return map.put(e, PRESENT)==null;
    }

    public V put(K key, V value) {
        return putVal(hash(key), key, value, false, true);
    }

So, I think you should check your implementation of equals() and hashcode().

[tsuoanttila]

@AndreiBoaghe getId is a way that is supposed to work properly in the case of elements not providing a proper equals or hashCode implementation. The id is not properly taken into account by MultiSelectionModel so it is in Vaadin.

We have chosen to provide this option for our users to provide an identifier and it is properly used in most cases in the selection Components and Models.

[wfischlein]

Yes, hashmap is hashmap, but on selectAll the dataprovider-items themselves are checked against the hashmap, but the values returned from getId should be in there instead

[wfischlein]

And no, the implementation of Long.equals, and Long.hashcode won't be touched ;)