diff options
author | Tatu Lund <tatu@vaadin.com> | 2021-08-06 18:30:53 +0300 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-08-06 18:30:53 +0300 |
commit | b379bb84280a34b6e1236e5d61a013a1e939a92e (patch) | |
tree | 45da6dfbc320049dfd5b44a7bc49c5a71613fdf7 | |
parent | 3ceda6b0756fe06b896e4b110e8b3dd53b5f3cbf (diff) | |
download | vaadin-framework-b379bb84280a34b6e1236e5d61a013a1e939a92e.tar.gz vaadin-framework-b379bb84280a34b6e1236e5d61a013a1e939a92e.zip |
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.
-rw-r--r-- | server/src/main/java/com/vaadin/data/Binder.java | 5 | ||||
-rw-r--r-- | server/src/test/java/com/vaadin/data/BinderTest.java | 31 |
2 files changed, 35 insertions, 1 deletions
diff --git a/server/src/main/java/com/vaadin/data/Binder.java b/server/src/main/java/com/vaadin/data/Binder.java index 1fb64c837f..110e260b0c 100644 --- a/server/src/main/java/com/vaadin/data/Binder.java +++ b/server/src/main/java/com/vaadin/data/Binder.java @@ -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); } } diff --git a/server/src/test/java/com/vaadin/data/BinderTest.java b/server/src/test/java/com/vaadin/data/BinderTest.java index 98f72e1197..fe529fa8f5 100644 --- a/server/src/test/java/com/vaadin/data/BinderTest.java +++ b/server/src/test/java/com/vaadin/data/BinderTest.java @@ -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() { |