Browse Source

fix: Avoid processing value change event due writing back of converted value (#12360)

This is both a optimization by skipping duplicate validation round and avoids ConcurrentModificationExpectation being thrown certain corner cases.
tags/8.14.0.alpha1
Tatu Lund 2 years ago
parent
commit
b379bb8428
No account linked to committer's email address

+ 4
- 1
server/src/main/java/com/vaadin/data/Binder.java View File

@@ -1103,6 +1103,7 @@ public class Binder<BEAN> implements Serializable {

private Registration onValueChange;
private boolean valueInit = false;
private boolean convertedBack = false;

/**
* Contains all converters and validators chained together in the
@@ -1283,7 +1284,8 @@ public class Binder<BEAN> implements Serializable {
private void handleFieldValueChange(
ValueChangeEvent<FIELDVALUE> event) {
// Don't handle change events when setting initial value
if (valueInit) {
if (valueInit || convertedBack) {
convertedBack = false;
return;
}

@@ -1313,6 +1315,7 @@ public class Binder<BEAN> implements Serializable {
if (convertBackToPresentation && value != null) {
FIELDVALUE converted = convertToFieldType(value);
if (!Objects.equals(field.getValue(), converted)) {
convertedBack = true;
getField().setValue(converted);
}
}

+ 31
- 0
server/src/test/java/com/vaadin/data/BinderTest.java View File

@@ -29,6 +29,7 @@ import org.junit.rules.ExpectedException;
import com.vaadin.data.Binder.Binding;
import com.vaadin.data.Binder.BindingBuilder;
import com.vaadin.data.converter.StringToBigDecimalConverter;
import com.vaadin.data.converter.StringToDoubleConverter;
import com.vaadin.data.converter.StringToIntegerConverter;
import com.vaadin.data.validator.IntegerRangeValidator;
import com.vaadin.data.validator.NotEmptyValidator;
@@ -43,6 +44,8 @@ import org.hamcrest.CoreMatchers;

public class BinderTest extends BinderTestBase<Binder<Person>, Person> {

private int count;

@Rule
/*
* transient to avoid interfering with serialization tests that capture a
@@ -1487,6 +1490,34 @@ public class BinderTest extends BinderTestBase<Binder<Person>, Person> {
binder.readBean(new AtomicReference<>(null));
}

// See: https://github.com/vaadin/framework/issues/12356
@Test
public void validationShouldNotRunTwice() {
TextField salaryField = new TextField();
count = 0;
item.setSalaryDouble(100d);
binder.forField(salaryField)
.withConverter(new StringToDoubleConverter(""))
.bind(Person::getSalaryDouble, Person::setSalaryDouble);
binder.setBean(item);
binder.addValueChangeListener(event -> {
count++;
});

salaryField.setValue("1000");
assertTrue(binder.isValid());

salaryField.setValue("salary");
assertFalse(binder.isValid());

salaryField.setValue("2000");

// Without fix for #12356 count will be 5
assertEquals(3, count);

assertEquals(new Double(2000), item.getSalaryDouble());
}

private TextField createNullAnd42RejectingFieldWithEmptyValue(
String emptyValue) {
return new TextField() {

Loading…
Cancel
Save