Skip to content

Commit

Permalink
Eagerly update bean if initial value is changed (#4138)
Browse files Browse the repository at this point in the history 
Related to vaadin/framework#9000
  • Loading branch information
@Legioth @caalador
Legioth authored and caalador committed May 22, 2018
1 parent 042a252 commit bbaeba9
Show file tree
Hide file tree
Showing 2 changed files with 90 additions and 37 deletions.
56 changes: 35 additions & 21 deletions 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 @@ -180,7 +180,7 @@ default BindingValidationStatus<TARGET> validate() {
* @param readOnly
* {@code true} to set binding read-only; {@code false} to
* enable writes
*
*
* @throws IllegalStateException
* if trying to make binding read-write and the setter is
* {@code null}
Expand All @@ -189,9 +189,9 @@ default BindingValidationStatus<TARGET> validate() {

/**
* Gets the current read-only status for this Binding.
*
*
* @see #setReadOnly(boolean)
*
*
* @return {@code true} if read-only; {@code false} if not
*/
boolean isReadOnly();
Expand Down Expand Up @@ -264,7 +264,7 @@ public interface BindingBuilder<BEAN, TARGET> extends Serializable {
* <p>
* <strong>Note:</strong> when a {@code null} setter is given the field
* will be marked as readonly by invoking {@link HasValue#setReadOnly}.
*
*
* @param getter
* the function to get the value of the property to the
* field, not null
Expand Down Expand Up @@ -296,7 +296,7 @@ Binding<BEAN, TARGET> bind(ValueProvider<BEAN, TARGET> getter,
* <p>
* <strong>Note:</strong> when the binding is <i>read-only</i> the field
* will be marked as readonly by invoking {@link HasValue#setReadOnly}.
*
*
* @param propertyName
* the name of the property to bind, not null
* @return the newly created binding
Expand Down Expand Up @@ -564,11 +564,9 @@ default BindingBuilder<BEAN, TARGET> withNullRepresentation(
TARGET nullRepresentation) {
return withConverter(
fieldValue -> Objects.equals(fieldValue, nullRepresentation)
? null
: fieldValue,
? null : fieldValue,
modelValue -> Objects.isNull(modelValue)
? nullRepresentation
: modelValue);
? nullRepresentation : modelValue);
}

/**
Expand Down Expand Up @@ -759,7 +757,7 @@ public Binding<BEAN, TARGET> bind(ValueProvider<BEAN, TARGET> getter,

getBinder().bindings.add(binding);
if (getBinder().getBean() != null) {
binding.initFieldValue(getBinder().getBean());
binding.initFieldValue(getBinder().getBean(), true);
}
if (setter == null) {
binding.getField().setReadOnly(true);
Expand Down Expand Up @@ -1077,22 +1075,35 @@ protected ValueContext createValueContext() {
*
* @param bean
* the bean to fetch the property value from
* @param writeBackChangedValues
* <code>true</code> if the bean value should be updated if
* the value is different after converting to and from the
* presentation value; <code>false</code> to avoid updating
* the bean value
*/
private void initFieldValue(BEAN bean) {
private void initFieldValue(BEAN bean, boolean writeBackChangedValues) {
assert bean != null;
assert onValueChange != null;
onValueChange.remove();
try {
getField().setValue(convertDataToFieldType(bean));
TARGET originalValue = getter.apply(bean);
field.setValue(convertToFieldType(originalValue));

if (writeBackChangedValues && setter != null) {
doConversion().ifOk(convertedValue -> {
if (!Objects.equals(originalValue, convertedValue)) {
setter.accept(bean, convertedValue);
}
});
}
} finally {
// Lambda instead of methref because of parser bug in Eclipse
onValueChange = getField().addValueChangeListener(
event -> this.handleFieldValueChange(event));
}
}

private FIELDVALUE convertDataToFieldType(BEAN bean) {
TARGET target = getter.apply(bean);
private FIELDVALUE convertToFieldType(TARGET target) {
ValueContext valueContext = createValueContext();
return converterValidatorChain.convertToPresentation(target,
valueContext);
Expand Down Expand Up @@ -1144,8 +1155,7 @@ public BindingValidationStatusHandler getValidationStatusHandler() {

@Override
public void read(BEAN bean) {
field.setValue(converterValidatorChain.convertToPresentation(
getter.apply(bean), createValueContext()));
field.setValue(convertToFieldType(getter.apply(bean)));
}

@Override
Expand Down Expand Up @@ -1494,7 +1504,7 @@ public <FIELDVALUE> BindingBuilder<BEAN, FIELDVALUE> forMemberField(
* TextField nameField = new TextField();
* binder.bind(nameField, Person::getName, Person::setName);
* </pre>
*
*
* <p>
* <strong>Note:</strong> when a {@code null} setter is given the field will
* be marked as read-only by invoking
Expand Down Expand Up @@ -1567,6 +1577,10 @@ public <FIELDVALUE> Binding<BEAN, FIELDVALUE> bind(
* Any change made in the fields also runs validation for the field
* {@link Binding} and bean level validation for this binder (bean level
* validators are added using {@link Binder#withValidator(Validator)}.
* <p>
* After updating each field, the value is read back from the field and the
* bean's property value is updated if it has been changed from the original
* value by the field or a converter.
*
* @see #readBean(Object)
* @see #writeBean(Object)
Expand All @@ -1586,7 +1600,7 @@ public void setBean(BEAN bean) {
} else {
doRemoveBean(false);
this.bean = bean;
getBindings().forEach(b -> b.initFieldValue(bean));
getBindings().forEach(b -> b.initFieldValue(bean, true));
// if there has been field value change listeners that trigger
// validation, need to make sure the validation errors are cleared
getValidationStatusHandler().statusChange(
Expand Down Expand Up @@ -1627,7 +1641,8 @@ public void readBean(BEAN bean) {
clearFields();
} else {
changedBindings.clear();
getBindings().forEach(binding -> binding.initFieldValue(bean));
getBindings()
.forEach(binding -> binding.initFieldValue(bean, false));
getValidationStatusHandler().statusChange(
BinderValidationStatus.createUnresolvedStatus(this));
fireStatusChangeEvent(false);
Expand Down Expand Up @@ -2399,8 +2414,7 @@ private <FIELDVALUE> Converter<FIELDVALUE, FIELDVALUE> createNullRepresentationA
Converter<FIELDVALUE, FIELDVALUE> nullRepresentationConverter = Converter
.from(fieldValue -> fieldValue,
modelValue -> Objects.isNull(modelValue)
? field.getEmptyValue()
: modelValue,
? field.getEmptyValue() : modelValue,
Throwable::getMessage);
ConverterDelegate<FIELDVALUE> converter = new ConverterDelegate<>(
nullRepresentationConverter);
Expand Down
71 changes: 55 additions & 16 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,17 @@

package com.vaadin.flow.data.binder;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.isEmptyString;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertSame;
import static org.junit.Assert.assertTrue;

import java.util.HashMap;
import java.util.Map;
import java.util.Objects;
Expand All @@ -37,18 +48,6 @@
import com.vaadin.flow.tests.data.bean.Person;
import com.vaadin.flow.tests.data.bean.Sex;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.isEmptyString;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertSame;
import static org.junit.Assert.assertTrue;

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

private Map<HasValue<?, ?>, String> componentErrors = new HashMap<>();
Expand Down Expand Up @@ -365,7 +364,7 @@ public String getEmptyValue() {
binder.setBean(namelessPerson);

assertTrue(nullTextField.isEmpty());
Assert.assertEquals(null, namelessPerson.getFirstName());
Assert.assertEquals("null", namelessPerson.getFirstName());

// Change value, see that textfield is not empty and bean is updated.
nullTextField.setValue("");
Expand Down Expand Up @@ -429,9 +428,8 @@ public void beanBinder_withConverter_nullRepresentationIsNotDisabled() {
String customNullPointerRepresentation = "foo";
Binder<Person> binder = new Binder<>(Person.class);
binder.forField(nameField)
.withConverter(value -> value,
value -> value == null ? customNullPointerRepresentation
: value)
.withConverter(value -> value, value -> value == null
? customNullPointerRepresentation : value)
.bind("firstName");

Person person = new Person();
Expand Down Expand Up @@ -1107,4 +1105,45 @@ public void setReadOnly_binding() {
assertTrue("Binding should be readonly", binding.isReadOnly());
assertTrue("Name field should be readonly", nameField.isReadOnly());
}

@Test
public void nonSymetricValue_setBean_writtenToBean() {
binder.bind(nameField, Person::getLastName, Person::setLastName);

Assert.assertNull(item.getLastName());

binder.setBean(item);

Assert.assertEquals("", item.getLastName());
}

@Test
public void nonSymmetricValue_readBean_beanNotTouched() {
binder.bind(nameField, Person::getLastName, Person::setLastName);
binder.addValueChangeListener(
event -> Assert.fail("No value change event should be fired"));

Assert.assertNull(item.getLastName());

binder.readBean(item);

Assert.assertNull(item.getLastName());
}

@Test
public void symetricValue_setBean_beanNotUpdated() {
binder.bind(nameField, Person::getFirstName, Person::setFirstName);

binder.setBean(new Person() {
@Override
public String getFirstName() {
return "First";
}

@Override
public void setFirstName(String firstName) {
Assert.fail("Setter should not be called");
}
});
}
}

0 comments on commit bbaeba9

Please sign in to comment.