Skip to content

Commit

Permalink
fix!: Avoid duplicated field bindings (#13340)
Browse files Browse the repository at this point in the history
If custom binding is added or completed after the call to Binder.bindInstanceFields
the field is bound twice and this may lead to potential multiple application
of converters, producing wrong representation and value for the field.
This change ignores incomplete bindings during `bindInstanceFields()`
process and overwrites existing bindings when `Binding.bind()` is
invoked after `bindInstanceFields()`.

Fixes #13314
  • Loading branch information
mcollovati committed Mar 25, 2022
1 parent 8e0351c commit 7358741
Show file tree
Hide file tree
Showing 2 changed files with 73 additions and 1 deletion.
20 changes: 19 additions & 1 deletion flow-data/src/main/java/com/vaadin/flow/data/binder/Binder.java
Expand Up @@ -945,6 +945,10 @@ public Binding<BEAN, TARGET> bind(ValueProvider<BEAN, TARGET> getter,
BindingImpl<BEAN, FIELDVALUE, TARGET> binding = new BindingImpl<>(
this, getter, setter);

// Remove existing binding for same field to avoid potential
// multiple application of converter
getBinder().bindings.removeIf(
registeredBinding -> registeredBinding.getField() == field);
getBinder().bindings.add(binding);
if (getBinder().getBean() != null) {
binding.initFieldValue(getBinder().getBean(), true);
Expand Down Expand Up @@ -3193,6 +3197,21 @@ private boolean bindProperty(Object objectWithMemberFields,
(Class<? extends HasValue<?, ?>>) memberField
.getType());
initializeField(objectWithMemberFields, memberField, field);
} else if (incompleteBindings != null
&& incompleteBindings.get(field) != null) {
// A manual binding is ongoing for this field, so we should not
// automatically bind it, otherwise we may silently overwrite
// current configuration.
// 'bindInstanceFields' states that it doesn't override existing
// bindings and that incomplete bindings may be completed also
// after method call
LoggerFactory.getLogger(Binder.class.getName()).debug(
"Binding for member '{}' of class '{}' will not be automatically "
+ "created because a custom binding has been started "
+ "but not yet finalized with 'bind()' invocation.",
memberField.getName(),
objectWithMemberFields.getClass().getName());
return false;
}
forField(field).bind(property);
return true;
Expand Down Expand Up @@ -3288,7 +3307,6 @@ private boolean handleProperty(Field field, Object objectWithMemberFields,

Boolean isPropertyBound = propertyHandler.apply(propertyName,
descriptor.get().getType());
assert boundProperties.containsKey(propertyName);
return isPropertyBound;
}

Expand Down
Expand Up @@ -650,4 +650,58 @@ public void bindInstanceFields_tentativelyBoundFieldAndNotBoundField() {
// thrown later if the binding is not completed)
binder.bindInstanceFields(form);
}

@Test
public void bindInstanceFields_customBindingAfterInvoke_automaticBindingOverwritten() {
BindOnlyOneField form = new BindOnlyOneField();
form.firstName = new TestTextField();
Binder<Person> binder = new Binder<>(Person.class);

binder.bindInstanceFields(form);
Binder.BindingBuilder<Person, String> binding = binder
.forField(form.firstName)
.withConverter(str -> str.substring(str.length() / 2),
str -> str + str);
binding.bind(Person::getFirstName, Person::setFirstName);

Person person = new Person();
person.setFirstName("Hello!");
binder.setBean(person);
Assert.assertEquals("Hello!Hello!", form.firstName.getValue());
}

@Test
public void bindInstanceFields_incompleteBindingBoundAfterInvoke_automaticBindingOverwritten() {
BindOnlyOneField form = new BindOnlyOneField();
form.firstName = new TestTextField();
Binder<Person> binder = new Binder<>(Person.class);

Binder.BindingBuilder<Person, String> binding = binder
.forField(form.firstName)
.withConverter(str -> str.substring(str.length() / 2),
str -> str + str);
binder.bindInstanceFields(form);
binding.bind(Person::getFirstName, Person::setFirstName);

Person person = new Person();
person.setFirstName("Hello!");
binder.setBean(person);
Assert.assertEquals("Hello!Hello!", form.firstName.getValue());
}

@Test
public void bindInstanceFields_incompleteBinding_fieldIgnored() {
BindOnlyOneField form = new BindOnlyOneField();
form.firstName = new TestTextField();
Binder<Person> binder = new Binder<>(Person.class);

binder.forField(form.firstName).withConverter(
str -> str.substring(str.length() / 2), str -> str + str);
binder.bindInstanceFields(form);

Assert.assertFalse(
"Expecting incomplete binding to be ignored by Binder, but field was bound",
binder.getBinding("firstName").isPresent());
}

}

0 comments on commit 7358741

Please sign in to comment.