Is this normal behaviour for data providers?

Flow
1

Hi,

When calling refreshAll() on a CheckboxGroup’s data provider after replacing all items, it seems to internally call updateCheckbox on old checkboxes that reference outdated data.

Is this the expected behavior, or could it be a bug?

Here’s a minimal example that reproduces the issue:

public class VaadinTestMainUI extends VerticalLayout {

  public VaadinTestMainUI() {
    Data data = new Data(new String[] {"1", "2", "3"});
    ListDataProvider<Row> dataProvider = new ListDataProvider<>(data.getRows());
    
    
    CheckboxGroup<Row> checkBoxGroup = new CheckboxGroup<>();
    checkBoxGroup.setItemLabelGenerator(Row::getValue); // 2. java.lang.ArrayIndexOutOfBoundsException
    checkBoxGroup.setDataProvider(dataProvider);
    
    add(checkBoxGroup);
    add(new Button("Click me", e -> {
      data.setData(new String[] {"1", "2"});
      dataProvider.getItems().clear();
      dataProvider.getItems().addAll(data.getRows());
      dataProvider.refreshAll(); // 1. java.lang.ArrayIndexOutOfBoundsException: Index 2 out of bounds for length 2
    }));
  }
  
  
  private static class Row {
    private int index;
    private Data data;
    public Row(Data data, int index) {
      this.data = data;
      this.index = index;
    }
    public String getValue() {
      return data.getValue(index);
    }
  }
  
  private static class Data {
    private String[] data;
    public Data(String[] data) {
      this.data = data;
    }
    public void setData(String[] data) {
      this.data = data;
      
    }
    public String getValue(int index) {
      return data[index]; // 3. thrown from here
    }
    
    public List<Row> getRows() {
      List<Row> rows = new ArrayList<>();
      for (int i = 0; i < data.length; i++) {
        rows.add(new Row(this, i));
      }
      return rows;
    }
  }
}

stacktrace:

java.lang.ArrayIndexOutOfBoundsException: Index 2 out of bounds for length 2
	at **.VaadinTestMainUI$Data.getValue(VaadinTestMenuUI.java:56)
	at **.VaadinTestMainUI$Row.getValue(VaadinTestMainUI.java:42)
	at **.VaadinTestMainUI.lambda$2(VaadinTestMainUI.java)
	at com.vaadin.flow.component.checkbox.CheckboxGroup.updateCheckbox(CheckboxGroup.java:875)
	********
	at java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:596)
	at com.vaadin.flow.component.checkbox.CheckboxGroup.refreshCheckboxes(CheckboxGroup.java:852)
	at com.vaadin.flow.component.checkbox.CheckboxGroup.setValue(CheckboxGroup.java:466)
	at com.vaadin.flow.component.checkbox.CheckboxGroup.setValue(CheckboxGroup.java:114)
	at com.vaadin.flow.component.HasValue.clear(HasValue.java:180)
	at com.vaadin.flow.component.checkbox.CheckboxGroup$2.onDiscard(CheckboxGroup.java:379)
	at com.vaadin.flow.component.shared.SelectionPreservationHandler.handleDataChange(SelectionPreservationHandler.java:87)
	at com.vaadin.flow.component.checkbox.CheckboxGroup.handleDataChange(CheckboxGroup.java:394)
	********
	at java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:596)
	at com.vaadin.flow.data.provider.AbstractDataProvider.fireEvent(AbstractDataProvider.java:132)
	at com.vaadin.flow.data.provider.AbstractDataProvider.refreshAll(AbstractDataProvider.java:72)
	*** button click **
2

The behavior is configurable nowadays, you probably want:

checkBoxGroup.setSelectionPreservationMode(SelectionPreservationMode.DISCARD);

2 Likes
3

My question was really about whether it is intentional that when calling refreshAll() with the default DISCARD setting the Vaadin framework refreshes the state of checkboxes even though they will be removed just a few steps later. It does not cause any issues on my side I just found it surprising when debugging the internals of the framework as it seemed like an unnecessary step. Using PRESERVE_ALL does not eliminate this unnecessary step either since in that case the developer would have to manually clear the selection before calling refreshAll() which would still cause the checkboxes that will later be removed to be refreshed.

4

That is strange.

The clear method should do setValue(getEmptyValue()), which should be empty set in CheckboxGroup, thus it should not iterate anything, and refreshCheckboxes forEach is skiped.

5

In version 24.8.2, getCheckboxItems() inside refreshCheckboxes() returns all the old checkboxes. You can see this in the stack trace I shared in my first post. I think the issue is that when calling refreshAll() and intending to discard the selection, there should be a way to clear the selection without triggering any UI updates. Setting an empty selection in this context should not result in any visual changes, since the components are about to be removed and rebuilt anyway.