Vaadin 8 - Grid multi-select bug

Hello, please correct me if I'm wrong but it seems there is a bug with the Grid and multi-select. When you enable multi-select, and display the "Select All" checkbox, and then select the checkbox to "Select All", it selects everything as expected. However, if you then de-select one item, it does not update the selected items list in the code. It still has all the items in the grid as part of the selected items list in the code. No events fire either...

Please advise.

Thanks

Which Vaadin 8 version you are using?

I tried the following in our demo https://demo.vaadin.com/sampler/#ui/grids-and-trees/grid/multi-select

• Click select all
• Deselect e.g. Issue #3 -> You will see that "Select All" checkbox will change state
• Use combobox below Grid to change status of the selected to "In progress" -> Notice, Issue #3 status will remain "New" is it was not selected

So I conclude it seems to work ok with the latest Vaadin 8.10.2 release.

Hi Tatu, I'm using the latest version. I have verified the sampler as well, but I can't see the full code in there on when the update button is clicked. We are just checking the selectedItmens method in the multi selection model, and it stays the same.

Can we possibly get the full snippet of code of the sampler?

Thanks

Hi Tatu, just checking if you have any more advise?

Wessel

Hello Tatu. Another issue with the grid.  If I click "select all" and begin to scroll down, I see that some records are not selected.  And when I scroll back up, some that were originally selected are not selected anymore.  This is something I wouldnt be able to see through the sampler, as my datatset is over 2000 records in this scenario. I also want to mention that data is being pulled from a database using the Dataprovider.

As Tatu noted, the grid multi-select seems to be working as expected. This can be verified using the following snippet (tested on Vaadin release 8.10.1):

public class MyUI extends UI {

	@Override
	protected void init(VaadinRequest vaadinRequest) {
		List<Issue> issueList = new ArrayList<>();
		issueList.add(new Issue("Issue #1", "new"));
		issueList.add(new Issue("Issue #2", "new"));
		issueList.add(new Issue("Issue #3", "new"));
		Grid<Issue> grid = new Grid<>(Issue.class);
		grid.setItems(issueList);
		grid.setSelectionMode(SelectionMode.MULTI);
		
		Label selected = new Label();
		selected.setCaption("Selected issues: ");
		grid.addSelectionListener(e -> selected.setValue(e.getAllSelectedItems().toString()));

		final VerticalLayout layout = new VerticalLayout();
		layout.addComponents(grid, selected);
		setContent(layout);
	}

	/**
	 * Example object.
	 */
	public static class Issue {

		private String issueName;
		private String status;

		public Issue() {

		}

		public Issue(String issueName, String status) {
			this.issueName = issueName;
			this.status = status;
		}

		public String getIssueName() {
			return issueName;
		}

		public void setIssueName(String issueName) {
			this.issueName = issueName;
		}

		public String getStatus() {
			return status;
		}

		public void setStatus(String status) {
			this.status = status;
		}

		@Override
		public String toString() {
			return issueName;
		}

	}

	@WebServlet(urlPatterns = "/*", name = "MyUIServlet", asyncSupported = true)
	@VaadinServletConfiguration(ui = MyUI.class, productionMode = false)
	public static class MyUIServlet extends VaadinServlet {
	}

}

Hi, we use a data provider and it does not work with it. Can you verify on your end using a data provider?

I'm not sure how you're using the data provider. Could you share your code (or update the above snippet) to show when the multiselect is failing?

Shavon Tate: Hello Tatu. Another issue with the grid.  If I click "select all" and begin to scroll down, I see that some records are not selected.  And when I scroll back up, some that were originally selected are not selected anymore.  This is something I wouldnt be able to see through the sampler, as my datatset is over 2000 records in this scenario. I also want to mention that data is being pulled from a database using the Dataprovider.

If you are not using in memory data provider by default the select all check box should not be there at all. See JavaDoc of grid.asMultiSelect().setSelectAllCheckBoxVisibility(visibility);

Sets the select all checkbox visibility mode. 

The default value is SelectAllCheckBoxVisibility.DEFAULT, which means that the checkbox is only visible if the grid's data provider is in- memory.

Also see the warning of grid.asMultiSelect().setSelectAllCheckBoxVisibility(SelectAllCheckBoxVisibility.VISIBLE);

Shows the select all checkbox, regardless of data provider used. 

For a lazy data provider, selecting all will result in to all rows being fetched from backend to application memory!

This however may depend on how you have implemented your data privider.

Here's a basic layout of what we have, with an example query to the backend DB using the dataprovider

public class MyUI extends UI {

    @Override
    protected void init(VaadinRequest vaadinRequest) {
        Grid<Items> grid = new ItemsGrid();
        grid.UpdateGrid();
    }
}

public class ItemsGrid extends Grid<Items> {
    private DataProvider<Item, Void> dataProvider;
    private Label selected = new Label();
    
    public ItemsGrid() {
        setSelectionMode(Grid.SelectionMode.MULTI);
        ((MultiSelectionModelImpl)getSelectionModel()).setSelectAllCheckBoxVisibility(MultiSelectionModel.SelectAllCheckBoxVisibility.VISIBLE);

        addColumn(item -> item.getName()).setCaption("Item Name");

        addSelectionListener(e -> {
            selected.setValue(e.getAllSelectedItems().toString());
        });
    }

    private void CreateDataProvider() {
        dataProvider = DataProvider.fromCallbacks(
                query -> MyUI.getMyService().getItemByNativeQuery(GetDataQuery(query.getLimit(), query.getOffset())).stream(),
                query -> MyUI.getMyService().getItemCountByNativeQuery(GetDataCountQuery())
        );
    }

    public void UpdateGrid() {


        if(dataProvider == null){
            CreateDataProvider();
            setDataProvider(dataProvider);
        }
        else
            dataProvider.refreshAll();

        deselectAll();
    }

    private String GetDataQuery(int limit, int offset){

        String nq = "select c.* from item_table as c ";
        nq += " limit " + limit + " offset " + offset;

        return nq;
    }

    private String GetDataCountQuery() {

        String nq = "select count(distinct c.id) from item_table as c ";

        return nq;
    }
}

Hi Tatu, any advise on the snippet above? In that scenario, the selectionListener is never hit when un-checking one of the items after selecting all...

Wessel

Hi again! Even with lazy loading in place everything seem to be working as expected. Here's a complete example

public class MyUI extends UI {

	@Override
	protected void init(final VaadinRequest vaadinRequest) {
		final ItemsGrid grid = new ItemsGrid();
		grid.updateGrid();

		final TextArea selected = new TextArea();
		grid.addSelectionListener(e -> selected.setValue(e.getAllSelectedItems().toString()));

		final VerticalLayout layout = new VerticalLayout();
		layout.addComponents(grid, selected);
		setContent(layout);
	}

	public class ItemsGrid extends Grid<Item> {
		private DataProvider<Item, Void> dataProvider;
		private ItemService service;

		public ItemsGrid() {
			setSelectionMode(Grid.SelectionMode.MULTI);
			((MultiSelectionModelImpl) getSelectionModel())
					.setSelectAllCheckBoxVisibility(MultiSelectionModel.SelectAllCheckBoxVisibility.VISIBLE);

			addColumn(item -> item.getItemName()).setCaption("Item Name");
		}

		private void createDataProvider() {
			this.dataProvider = DataProvider.fromCallbacks(query -> {
				final int offset = query.getOffset();
				final int limit = query.getLimit();
				return getMyService().fetchItems(offset, limit).stream();
			}, query -> getMyService().getItemCount());
		}

		public void updateGrid() {

			if (this.dataProvider == null) {
				createDataProvider();
				setDataProvider(this.dataProvider);
			} else {
				this.dataProvider.refreshAll();
			}

			deselectAll();
		}

		private ItemService getMyService() {
			if (this.service == null) {
				this.service = new ItemService();
			}
			return this.service;
		}
	}

	// Item Bean
	public static class Item {
		private String itemName;

		public Item() {}

		public Item(final String itemName) {
			this.itemName = itemName;
		}

		public String getItemName() {
			return this.itemName;
		}

		public void setItemName(final String itemName) {
			this.itemName = itemName;
		}

		@Override
		public String toString() {
			return this.itemName;
		}
	}

	public static class ItemService {
		List<Item> itemList;

		ItemService() {
			this.itemList = createItemList(100);
		}

		public List<Item> fetchItems(final int offset, final int limit) {
			final List<Item> currItemList = new ArrayList<>();
			for (int i = offset; (i < (offset + limit)) && (i < this.itemList.size()); i++) {
				currItemList.add(this.itemList.get(i));
			}
			return currItemList;
		}

		public int getItemCount() {
			return this.itemList.size();
		}

		private List<Item> createItemList(final int len) {
			final List<Item> newItemList = new ArrayList<>();
			for (int i = 0; i < len; i++) {
				final Item newItem = new Item("item #" + i);
				newItemList.add(newItem);
			}
			return newItemList;
		}
	}

	@WebServlet(urlPatterns = "/*", name = "MyUIServlet", asyncSupported = true)
	@VaadinServletConfiguration(ui = MyUI.class, productionMode = false)
	public static class MyUIServlet extends VaadinServlet {
	}
}

Hello, we had to deploy without the select-all due to time, but we still need it to work.

Attached is a video to show you how it does not work. The issue is when you de-select one of the items, after you select-all.

My code is essentially the same as your example. The only difference we load from a database.

In the constructor:

setSelectionMode(Grid.SelectionMode.MULTI);		
((MultiSelectionModelImpl)getSelectionModel()).setSelectAllCheckBoxVisibility(MultiSelectionModel.SelectAllCheckBoxVisibility.VISIBLE);

Creating dataprovider:

dataProvider = DataProvider.fromCallbacks(
				query -> BaseUI.getCurrentUI().getTEKWaveService().getTcAccessControlCredentialByNativeQuery(GetDataQuery(query.getLimit(), query.getOffset())).stream(),
				query -> BaseUI.getCurrentUI().getTEKWaveService().getTcAccessControlCredentialCountByNativeQuery(GetDataCountQuery())
		);

Selection listener:

gridCredentials.addSelectionListener(e -> {
            Notification.show(String.valueOf(e.getAllSelectedItems().size()), Notification.Type.TRAY_NOTIFICATION).setPosition(Position.BOTTOM_CENTER);
        });

Here is the video

https://www.dropbox.com/s/gdiza7sfwlxecva/ice_video_20200414-140029.webm?dl=0

Not sure if that's the cause of your issue, but one thing I noticed in your earlier snippet is that in GetDataQuery() you select based on all records in the backend whereas in your GetDataCountQuery() you count the distinct records. This is not the right usage as I understand it. As described in the docs on dataprovider:

The results of the first and second callback must be symmetric so that fetching all available items using the first callback returns the number of items indicated by the second callback. Thus if you impose any restrictions on e.g. a database query in the first callback, you must also add the same restrictions for the second callback.

See https://vaadin.com/docs/v8/framework/datamodel/datamodel-providers.html#datamodel.dataproviders.lazy

Hi Tarek, thank you for your response. Aren't you doing the same in your example above? getItemCount() is returning the the full itemList count...

Indeed, actually as I note in my edited response (sorry about the confusion) I think the issue might be that in GetDataQuery() you select based on all records in the backend whereas in your GetDataCountQuery() you count the distinct records

Sorry that's not actually how it is in the production code, it was just a snippet as an example. Disregard the "distinct".

So should we apply the "offset" and "limit" to the count query as well?

And if that doesn't fix it, one other thing to double-check is whether the bean is properly implementing equals() and hashcode()

Thanks Tarek,

So should we apply the "offset" and "limit" to the count query as well?

Also, the bean is not overriding equals or hashcode at all...

I double checked the queries, and they are identical except for the count query not having limit and offset.

I even tried by limiting the data query to 20 rows, and the count query just being "select 20". Same result.

What it does is, when you click "Select All", and then de-select a row, no events are fired. Then you select that same row, and the event is fired. Then you de-select that same row, and the event IS fired.

So something is wrong with the "Select All" and then de-selecting a row.

Any other advise?

Hi Tarek,

Thanks again for your help. Turns out the issue was with overriding the equals and hashCode methods for the database entities. We were not overriding those at all. So it turns out you must do that for the select all to function correctly with more complex entities.

Thanks for the advise...

Tarek Oraby: ... As described in the docs on dataprovider:

The results of the first and second callback must be symmetric so that fetching all available items using the first callback returns the number of items indicated by the second callback. Thus if you impose any restrictions on e.g. a database query in the first callback, you must also add the same restrictions for the second callback.

The problem we encounter here is that the number of items in the database is changing rapidly so it happens every now an then that the number of fetched items is one less then the calculated count immediately before the fetch => IndexOutOfBoundsException is thrown. It seems very simple to avoid this exception by just stop iterating the fetched items at the end if that is earlier than some index reaches the value of count.

It seems very simple to avoid this exception by just stop iterating the fetched items at the end if that is earlier than some index reaches the value of count.

That seems like it could be a viable enhancement to the Grid indeed. It might be worth opening an enhancement request at https://github.com/vaadin/vaadin-grid/issues.