Skip to content

Commit

Permalink
fix: avoid ConcurrentModificationException in Binder (#10793) (#10796)
Browse files Browse the repository at this point in the history
fixes #9217

Co-authored-by: Denis <denis@vaadin.com>
  • Loading branch information
vaadin-bot and Denis committed Apr 26, 2021
1 parent 8dff1b5 commit c0d36d4
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 9 deletions.
24 changes: 15 additions & 9 deletions flow-data/src/main/java/com/vaadin/flow/data/binder/Binder.java
Expand Up @@ -1914,8 +1914,7 @@ public void readBean(BEAN bean) {
* if some of the bound field values fail to validate
*/
public void writeBean(BEAN bean) throws ValidationException {
BinderValidationStatus<BEAN> status = doWriteIfValid(bean,
new ArrayList<>(bindings));
BinderValidationStatus<BEAN> status = doWriteIfValid(bean, bindings);
if (status.hasErrors()) {
throw new ValidationException(status.getFieldValidationErrors(),
status.getBeanValidationErrors());
Expand Down Expand Up @@ -1981,7 +1980,7 @@ public void writeBeanAsDraft(BEAN bean, boolean forced) {
* updated, {@code false} otherwise
*/
public boolean writeBeanIfValid(BEAN bean) {
return doWriteIfValid(bean, new ArrayList<>(bindings)).isOk();
return doWriteIfValid(bean, bindings).isOk();
}

/**
Expand All @@ -2004,19 +2003,26 @@ private BinderValidationStatus<BEAN> doWriteIfValid(BEAN bean,
Objects.requireNonNull(bean, "bean cannot be null");
List<ValidationResult> binderResults = Collections.emptyList();

// make a copy of the incoming bindings to avoid their modifications
// during validation
Collection<Binding<BEAN, ?>> currentBindings = new ArrayList<>(
bindings);

// First run fields level validation, if no validation errors then
// update bean
List<BindingValidationStatus<?>> bindingResults = bindings.stream()
.map(b -> b.validate(false)).collect(Collectors.toList());
List<BindingValidationStatus<?>> bindingResults = currentBindings
.stream().map(b -> b.validate(false))
.collect(Collectors.toList());

if (bindingResults.stream()
.noneMatch(BindingValidationStatus::isError)) {
// Store old bean values so we can restore them if validators fail
Map<Binding<BEAN, ?>, Object> oldValues = getBeanState(bean,
bindings);
currentBindings);

bindings.forEach(binding -> ((BindingImpl<BEAN, ?, ?>) binding)
.writeFieldValue(bean));
currentBindings
.forEach(binding -> ((BindingImpl<BEAN, ?, ?>) binding)
.writeFieldValue(bean));
// Now run bean level validation against the updated bean
binderResults = validateBean(bean);
if (binderResults.stream().anyMatch(ValidationResult::isError)) {
Expand All @@ -2027,7 +2033,7 @@ private BinderValidationStatus<BEAN> doWriteIfValid(BEAN bean,
* Changes have been successfully saved. The set is only cleared
* when the changes are stored in the currently set bean.
*/
bindings.clear();
changedBindings.clear();
} else if (getBean() == null) {
/*
* When using readBean and writeBean there is no knowledge of
Expand Down
Expand Up @@ -1652,6 +1652,30 @@ public void setBean_readOnlyBinding_accessorsBiding_valueIsNotUpdated() {
Assert.assertSame(val, bean.getVals());
}

@Test
public void invalidUsage_modifyFieldsInsideValidator_binderDoesNotThrow() {
TestTextField field = new TestTextField();

AtomicBoolean validatorIsExecuted = new AtomicBoolean();
binder.forField(field).asRequired().withValidator((val, context) -> {
nameField.setValue("foo");
ageField.setValue("bar");
validatorIsExecuted.set(true);
return ValidationResult.ok();
}).bind(Person::getEmail, Person::setEmail);

binder.forField(nameField).bind(Person::getFirstName,
Person::setFirstName);
binder.forField(ageField).bind(Person::getLastName,
Person::setLastName);

binder.setBean(new Person());

field.setValue("baz");
// mostly self control, the main check is: not exception is thrown
Assert.assertTrue(validatorIsExecuted.get());
}

private TestTextField createNullRejectingFieldWithEmptyValue(
String emptyValue) {
return new TestTextField() {
Expand Down

0 comments on commit c0d36d4

Please sign in to comment.