Binder exception after 24.3.3 to 24.3.5 upgrade

jcgueriaud1 commented 6 months ago

Description of the bug

When I'm calling binder.readBean() I have a BindingException

com.vaadin.flow.data.binder.BindingException: An exception has been thrown inside binding logic for the field element [label='First Name']

Expected behavior

No exception

Minimal reproducible example

It happens when I'm using binder.bindInstanceFields(this); then bind a field. It doesn't happen if I'm binding the field then binder.bindInstanceFields(this);

In 24.3.3, both code were working.

        // Configure Form
        binder = new BeanValidationBinder<>(SamplePerson.class);

        // Bind fields. This is where you'd define e.g. validation rules
        // working binder.forField(firstName).bind("firstName");
        binder.bindInstanceFields(this);
        // not working
        binder.forField(firstName).bind("firstName");

There is a repository to reproduce it: (click on a row to get the exception) https://github.com/jcgueriaud1/binder-test

Versions

Vaadin / Flow version: 24.3.5 and 24.4.0.alpha5
Java version: 21
OS version: macOs

jcgueriaud1 commented 6 months ago

It's probably related to this changes: https://github.com/vaadin/flow/pull/18535

mcollovati commented 6 months ago

Looking at the stack trace, it seems a duplicate of (or related to) #18702

mcollovati commented 6 months ago

The issue is caused by this code in validate() method

        BindingValidationStatus<?> status = bindingValidationStatuses.stream()
                .filter(s -> targetBinding.equals(s.getBinding())).findFirst()
                .orElse(null);

        getValidationStatusHandler().statusChange(new BinderValidationStatus<>(
                this, Collections.singletonList(status),
                Collections.emptyList()));

Basically, in the reproducer, status is null, so the BinderValidationStatus objects gets a collection with a null element. To be investigated the reason for the null status. It might be because the code is binding the same field twice; probably some cleanup is missing on the second binding.

mcollovati commented 6 months ago

When the field is bound for the second time, the previous binding is removed from Binder, but unbind is not called, so some listeners are still attached to the field.

In BindingBuilderImpl.bind() we have to change the following part to also unbind the field

getBinder().bindings.removeIf(registeredBinding -> registeredBinding.getField() == field);

mcollovati commented 6 months ago

Fixes in the latest 24.4 SNAPSHOT seems to prevent the error in the example project. However, I think the replaced binding should be unbound anyway.

vaadin-bot commented 6 months ago

This ticket/PR has been released with Vaadin 24.3.6.

vaadin-bot commented 6 months ago

This ticket/PR has been released with Vaadin 24.2.11.

vaadin / flow