Skip to content

Commit

Permalink
fix: don't update bean property which is read-only bound (#10768)
Browse files Browse the repository at this point in the history 
fixes #9446
  • Loading branch information
Denis committed Apr 23, 2021
1 parent 35cd98e commit 1689e00
Show file tree
Hide file tree
Showing 2 changed files with 107 additions and 29 deletions.
2 changes: 1 addition & 1 deletion flow-data/src/main/java/com/vaadin/flow/data/binder/Binder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1195,7 +1195,7 @@ private void initFieldValue(BEAN bean, boolean writeBackChangedValues) {
TARGET originalValue = getter.apply(bean);
convertAndSetFieldValue(originalValue);

if (writeBackChangedValues && setter != null) {
if (writeBackChangedValues && setter != null && !readOnly) {
doConversion().ifOk(convertedValue -> {
if (!Objects.equals(originalValue, convertedValue)) {
setter.accept(bean, convertedValue);
Expand Down
134 changes: 106 additions & 28 deletions flow-data/src/test/java/com/vaadin/flow/data/binder/BinderTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package com.vaadin.flow.data.binder;

import java.io.Serializable;
import java.util.HashMap;
import java.util.Locale;
import java.util.Map;
Expand Down Expand Up @@ -340,15 +341,13 @@ public void save_bound_beanAsDraft_setBean() {

private void do_test_save_bound_beanAsDraft(boolean setBean) {
Binder<Person> binder = new Binder<>();
binder.forField(nameField)
.withValidator((value,context) -> {
if (value.equals("Mike")) {
return ValidationResult.ok();
} else {
return ValidationResult.error("value must be Mike");
}
})
.bind(Person::getFirstName, Person::setFirstName);
binder.forField(nameField).withValidator((value, context) -> {
if (value.equals("Mike")) {
return ValidationResult.ok();
} else {
return ValidationResult.error("value must be Mike");
}
}).bind(Person::getFirstName, Person::setFirstName);
binder.forField(ageField)
.withConverter(new StringToIntegerConverter(""))
.bind(Person::getAge, Person::setAge);
Expand All @@ -375,20 +374,22 @@ private void do_test_save_bound_beanAsDraft(boolean setBean) {
// fails
assertEquals(age, person.getAge());

binder.writeBeanAsDraft(person,true);
binder.writeBeanAsDraft(person, true);
// name is now written despite validation as write was forced
assertEquals(fieldValue, person.getFirstName());
}

@Test
public void save_bound_bean_disable_validation_binding() throws ValidationException {
public void save_bound_bean_disable_validation_binding()
throws ValidationException {
Binder<Person> binder = new Binder<>();
Binding<Person, String> nameBinding = binder.forField(nameField)
.withValidator((value,context) -> {
if (value.equals("Mike")) return ValidationResult.ok();
else return ValidationResult.error("value must be Mike");
})
.bind(Person::getFirstName, Person::setFirstName);
.withValidator((value, context) -> {
if (value.equals("Mike"))
return ValidationResult.ok();
else
return ValidationResult.error("value must be Mike");
}).bind(Person::getFirstName, Person::setFirstName);
binder.forField(ageField)
.withConverter(new StringToIntegerConverter(""))
.bind(Person::getAge, Person::setAge);
Expand All @@ -412,14 +413,15 @@ public void save_bound_bean_disable_validation_binding() throws ValidationExcept
}

@Test
public void save_bound_bean_disable_validation_binder() throws ValidationException {
public void save_bound_bean_disable_validation_binder()
throws ValidationException {
Binder<Person> binder = new Binder<>();
binder.forField(nameField)
.withValidator((value,context) -> {
if (value.equals("Mike")) return ValidationResult.ok();
else return ValidationResult.error("value must be Mike");
})
.bind(Person::getFirstName, Person::setFirstName);
binder.forField(nameField).withValidator((value, context) -> {
if (value.equals("Mike"))
return ValidationResult.ok();
else
return ValidationResult.error("value must be Mike");
}).bind(Person::getFirstName, Person::setFirstName);
binder.forField(ageField)
.withConverter(new StringToIntegerConverter(""))
.bind(Person::getAge, Person::setAge);
Expand Down Expand Up @@ -615,13 +617,15 @@ public void setRequired_withErrorMessage_fieldGetsRequiredIndicatorAndValidator(
TestTextField textField = new TestTextField();
assertFalse(textField.isRequiredIndicatorVisible());

BindingBuilder<Person, String> bindingBuilder = binder.forField(textField);
BindingBuilder<Person, String> bindingBuilder = binder
.forField(textField);
assertFalse(textField.isRequiredIndicatorVisible());

bindingBuilder.asRequired("foobar");
assertTrue(textField.isRequiredIndicatorVisible());

Binding<Person, String> binding = bindingBuilder.bind(Person::getFirstName, Person::setFirstName);
Binding<Person, String> binding = bindingBuilder
.bind(Person::getFirstName, Person::setFirstName);
binder.setBean(item);
assertThat(textField.getErrorMessage(), isEmptyString());

Expand All @@ -642,12 +646,15 @@ public void setRequired_withErrorMessage_fieldGetsRequiredIndicatorAndValidator(
public void settingAsRequiredEnabledFalseWhenNoAsRequired() {
TestTextField textField = new TestTextField();

BindingBuilder<Person, String> bindingBuilder = binder.forField(textField);
Binding<Person, String> binding = bindingBuilder.bind(Person::getFirstName, Person::setFirstName);
BindingBuilder<Person, String> bindingBuilder = binder
.forField(textField);
Binding<Person, String> binding = bindingBuilder
.bind(Person::getFirstName, Person::setFirstName);

binder.readBean(item);

// TextField input is not set required, this should trigger IllegalStateExceptipon
// TextField input is not set required, this should trigger
// IllegalStateExceptipon
binding.setAsRequiredEnabled(false);
}

Expand Down Expand Up @@ -1584,6 +1591,39 @@ public void addValueListenerFromStatusListener_listenerAdded() {
innerListenerInvoked.get());
}

@Test
public void setBean_readOnlyBinding_propertyBinding_valueIsNotUpdated() {
Binder<ExampleBean> binder = new Binder<>(ExampleBean.class);

binder.forField(nameField).withNullRepresentation("")
.withConverter(new TestConverter()).bind("vals")
.setReadOnly(true);

ExampleBean bean = new ExampleBean();
SubPropClass val = new SubPropClass();
bean.setVals(val);
binder.setBean(bean);

Assert.assertSame(val, bean.getVals());
}

@Test
public void setBean_readOnlyBinding_accessorsBiding_valueIsNotUpdated() {
Binder<ExampleBean> binder = new Binder<>(ExampleBean.class);

binder.forField(nameField).withNullRepresentation("")
.withConverter(new TestConverter())
.bind(ExampleBean::getVals, ExampleBean::setVals)
.setReadOnly(true);

ExampleBean bean = new ExampleBean();
SubPropClass val = new SubPropClass();
bean.setVals(val);
binder.setBean(bean);

Assert.assertSame(val, bean.getVals());
}

private TestTextField createNullRejectingFieldWithEmptyValue(
String emptyValue) {
return new TestTextField() {
Expand Down Expand Up @@ -1612,4 +1652,42 @@ private Binder<AtomicReference<Integer>> createIntegerConverterBinder(
.bind(AtomicReference::get, AtomicReference::set);
return binder;
}

public static class ExampleBean implements Serializable {
private SubPropClass vals;

public SubPropClass getVals() {
return vals;
}

public void setVals(SubPropClass vals) {
this.vals = vals;
}
}

public static class SubPropClass implements Serializable {
private String val1 = "Val1";

@Override
public String toString() {
return val1;
}
}

public static class TestConverter
implements Converter<String, SubPropClass> {

@Override
public Result<SubPropClass> convertToModel(String value,
ValueContext context) {
return Result.ok(null);
}

@Override
public String convertToPresentation(SubPropClass value,
ValueContext context) {
return value != null ? value.toString() : null;
}
};

}

0 comments on commit 1689e00

Please sign in to comment.