From 0f39506e3c1ca335951be4ed3bdfc5ca3c77dbb3 Mon Sep 17 00:00:00 2001 From: Aleksi Hietanen Date: Tue, 3 Jan 2017 09:53:06 +0200 Subject: Automatically complete custom bindings in BeanBinder#bindInstanceFields (#8012) Closes vaadin/framework8-issues#511 --- .../src/main/java/com/vaadin/data/BeanBinder.java | 83 +++++++++++++++++++++- server/src/main/java/com/vaadin/data/Binder.java | 41 +++++++++++ .../test/java/com/vaadin/data/BeanBinderTest.java | 58 +++++++++++++++ 3 files changed, 179 insertions(+), 3 deletions(-) (limited to 'server') diff --git a/server/src/main/java/com/vaadin/data/BeanBinder.java b/server/src/main/java/com/vaadin/data/BeanBinder.java index 8f79ff4f88..898f2cd37d 100644 --- a/server/src/main/java/com/vaadin/data/BeanBinder.java +++ b/server/src/main/java/com/vaadin/data/BeanBinder.java @@ -25,8 +25,10 @@ import java.lang.reflect.Type; import java.util.ArrayList; import java.util.Arrays; import java.util.HashSet; +import java.util.IdentityHashMap; import java.util.List; import java.util.Locale; +import java.util.Map; import java.util.Objects; import java.util.Optional; import java.util.Set; @@ -226,6 +228,7 @@ public class BeanBinder extends Binder { errorMessageProvider); } + @SuppressWarnings("unchecked") @Override public Binding bind(String propertyName) { checkUnbound(); @@ -252,6 +255,7 @@ public class BeanBinder extends Binder { value)); } finally { getBinder().boundProperties.add(propertyName); + getBinder().incompleteMemberFieldBindings.remove(getField()); } } @@ -315,7 +319,8 @@ public class BeanBinder extends Binder { } private final Class beanType; - private final Set boundProperties; + private final Set boundProperties = new HashSet<>(); + private final Map, BeanBindingImpl> incompleteMemberFieldBindings = new IdentityHashMap<>(); /** * Creates a new {@code BeanBinder} supporting beans of the given type. @@ -326,7 +331,6 @@ public class BeanBinder extends Binder { public BeanBinder(Class beanType) { BeanUtil.checkBeanValidationAvailable(); this.beanType = beanType; - boundProperties = new HashSet<>(); } @Override @@ -335,6 +339,29 @@ public class BeanBinder extends Binder { return (BeanBindingBuilder) super.forField(field); } + /** + * Creates a new binding for the given field. The returned builder may be + * further configured before invoking {@link #bindInstanceFields(Object)}. + * Unlike with the {@link #forField(HasValue)} method, no explicit call to + * {@link BeanBindingBuilder#bind(String)} is needed to complete this + * binding in the case that the name of the field matches a field name found + * in the bean. + * + * @param + * the value type of the field + * @param field + * the field to be bound, not null + * @return the new binding builder + * + * @see #forField(HasValue) + * @see #bindInstanceFields(Object) + */ + public BeanBindingBuilder forMemberField( + HasValue field) { + incompleteMemberFieldBindings.put(field, null); + return forField(field); + } + /** * Binds the given field to the property with the given name. The getter and * setter methods of the property are looked up with bean introspection and @@ -372,12 +399,28 @@ public class BeanBinder extends Binder { return (BeanBinder) super.withValidator(validator); } + @SuppressWarnings("unchecked") @Override protected BeanBindingImpl createBinding( HasValue field, Converter converter, BindingValidationStatusHandler handler) { Objects.requireNonNull(field, "field cannot be null"); Objects.requireNonNull(converter, "converter cannot be null"); + if (incompleteMemberFieldBindings.containsKey(field)) { + BeanBindingImpl newBinding = doCreateBinding( + field, converter, handler); + incompleteMemberFieldBindings.put(field, newBinding); + return newBinding; + } else { + return (BeanBindingImpl) super.createBinding( + field, converter, handler); + } + } + + @Override + protected BeanBindingImpl doCreateBinding( + HasValue field, Converter converter, + BindingValidationStatusHandler handler) { return new BeanBindingImpl<>(this, field, converter, handler); } @@ -431,10 +474,25 @@ public class BeanBinder extends Binder { .filter(memberField -> HasValue.class .isAssignableFrom(memberField.getType())) .forEach(memberField -> handleProperty(memberField, + objectWithMemberFields, (property, type) -> bindProperty(objectWithMemberFields, memberField, property, type))); } + @SuppressWarnings("unchecked") + private BeanBindingImpl getIncompleteMemberFieldBinding( + Field memberField, Object objectWithMemberFields) { + memberField.setAccessible(true); + try { + return incompleteMemberFieldBindings + .get(memberField.get(objectWithMemberFields)); + } catch (IllegalArgumentException | IllegalAccessException e) { + throw new RuntimeException(e); + } finally { + memberField.setAccessible(false); + } + } + /** * Binds {@code property} with {@code propertyType} to the field in the * {@code objectWithMemberFields} instance using {@code memberField} as a @@ -537,6 +595,17 @@ public class BeanBinder extends Binder { return memberFieldInOrder; } + @Override + protected void checkBindingsCompleted(String methodName) { + if (!incompleteMemberFieldBindings.isEmpty()) { + throw new IllegalStateException( + "All bindings created with forMemberField must " + + "be completed with bindInstanceFields before calling " + + methodName); + } + super.checkBindingsCompleted(methodName); + } + private void initializeField(Object objectWithMemberFields, Field memberField, HasValue value) { try { @@ -551,8 +620,9 @@ public class BeanBinder extends Binder { } } - private void handleProperty(Field field, + private void handleProperty(Field field, Object objectWithMemberFields, BiConsumer> propertyHandler) { + Optional descriptor = getPropertyDescriptor(field); if (!descriptor.isPresent()) { @@ -564,6 +634,13 @@ public class BeanBinder extends Binder { return; } + BeanBindingImpl tentativeBinding = getIncompleteMemberFieldBinding( + field, objectWithMemberFields); + if (tentativeBinding != null) { + tentativeBinding.bind(propertyName); + return; + } + propertyHandler.accept(propertyName, descriptor.get().getPropertyType()); boundProperties.add(propertyName); diff --git a/server/src/main/java/com/vaadin/data/Binder.java b/server/src/main/java/com/vaadin/data/Binder.java index fab163a1c5..e818894f15 100644 --- a/server/src/main/java/com/vaadin/data/Binder.java +++ b/server/src/main/java/com/vaadin/data/Binder.java @@ -131,6 +131,13 @@ public class Binder implements Serializable { */ public interface BindingBuilder extends Serializable { + /** + * Gets the field the binding is being built for. + * + * @return the field this binding is being built for + */ + public HasValue getField(); + /** * Completes this binding using the given getter and setter functions * representing a backing bean property. The functions are used to @@ -547,6 +554,7 @@ public class Binder implements Serializable { getBinder().fireStatusChangeEvent(false); bound = true; + getBinder().incompleteBindings.remove(getField()); return binding; } @@ -650,6 +658,11 @@ public class Binder implements Serializable { "cannot modify binding: already bound to a property"); } } + + @Override + public HasValue getField() { + return field; + } } /** @@ -951,6 +964,8 @@ public class Binder implements Serializable { private final Set> bindings = new LinkedHashSet<>(); + private final Map, BindingBuilder> incompleteBindings = new IdentityHashMap<>(); + private final List> validators = new ArrayList<>(); private final Map, ConverterDelegate> initialConverters = new IdentityHashMap<>(); @@ -1085,6 +1100,7 @@ public class Binder implements Serializable { * bean */ public void setBean(BEAN bean) { + checkBindingsCompleted("setBean"); if (bean == null) { if (this.bean != null) { doRemoveBean(true); @@ -1128,6 +1144,7 @@ public class Binder implements Serializable { */ public void readBean(BEAN bean) { Objects.requireNonNull(bean, "bean cannot be null"); + checkBindingsCompleted("readBean"); setHasChanges(false); bindings.forEach(binding -> binding.initFieldValue(bean)); @@ -1510,6 +1527,15 @@ public class Binder implements Serializable { protected BindingBuilder createBinding( HasValue field, Converter converter, BindingValidationStatusHandler handler) { + BindingBuilder newBinding = doCreateBinding(field, + converter, handler); + incompleteBindings.put(field, newBinding); + return newBinding; + } + + protected BindingBuilder doCreateBinding( + HasValue field, Converter converter, + BindingValidationStatusHandler handler) { return new BindingBuilderImpl<>(this, field, converter, handler); } @@ -1694,4 +1720,19 @@ public class Binder implements Serializable { return converter; } + /** + * Throws if this binder has incomplete bindings. + * + * @param methodName + * name of the method where this call is originated from + * @throws IllegalStateException + * if this binder has incomplete bindings + */ + protected void checkBindingsCompleted(String methodName) { + if (!incompleteBindings.isEmpty()) { + throw new IllegalStateException( + "All bindings created with forField must be completed before calling " + + methodName); + } + } } diff --git a/server/src/test/java/com/vaadin/data/BeanBinderTest.java b/server/src/test/java/com/vaadin/data/BeanBinderTest.java index 1b9f2f0ac7..3433aee1a8 100644 --- a/server/src/test/java/com/vaadin/data/BeanBinderTest.java +++ b/server/src/test/java/com/vaadin/data/BeanBinderTest.java @@ -13,8 +13,10 @@ import org.junit.Test; import com.vaadin.data.BeanBinder.BeanBindingBuilder; import com.vaadin.data.Binder.BindingBuilder; +import com.vaadin.data.converter.StringToIntegerConverter; import com.vaadin.tests.data.bean.BeanToValidate; import com.vaadin.ui.CheckBoxGroup; +import com.vaadin.ui.TextField; public class BeanBinderTest extends BinderTestBase, BeanToValidate> { @@ -24,10 +26,12 @@ public class BeanBinderTest private class TestClass { private CheckBoxGroup enums; + private TextField number = new TextField(); } private class TestBean { private Set enums; + private int number; public Set getEnums() { return enums; @@ -36,6 +40,14 @@ public class BeanBinderTest public void setEnums(Set enums) { this.enums = enums; } + + public int getNumber() { + return number; + } + + public void setNumber(int number) { + this.number = number; + } } @Before @@ -50,9 +62,55 @@ public class BeanBinderTest public void bindInstanceFields_parameters_type_erased() { BeanBinder otherBinder = new BeanBinder<>(TestBean.class); TestClass testClass = new TestClass(); + otherBinder.forField(testClass.number) + .withConverter(new StringToIntegerConverter("")) + .bind("number"); + + // Should correctly bind the enum field without throwing + otherBinder.bindInstanceFields(testClass); + } + + @Test + public void bindInstanceFields_automatically_binds_incomplete_forMemberField_bindings() { + BeanBinder otherBinder = new BeanBinder<>(TestBean.class); + TestClass testClass = new TestClass(); + + otherBinder.forMemberField(testClass.number) + .withConverter(new StringToIntegerConverter("")); + otherBinder.bindInstanceFields(testClass); + + TestBean bean = new TestBean(); + otherBinder.setBean(bean); + testClass.number.setValue("50"); + assertEquals(50, bean.number); + } + + @Test(expected = IllegalStateException.class) + public void bindInstanceFields_does_not_automatically_bind_incomplete_forField_bindings() { + BeanBinder otherBinder = new BeanBinder<>(TestBean.class); + TestClass testClass = new TestClass(); + + otherBinder.forField(testClass.number) + .withConverter(new StringToIntegerConverter("")); + + // Should throw an IllegalStateException since the binding for number is + // not completed with bind otherBinder.bindInstanceFields(testClass); } + @Test(expected = IllegalStateException.class) + public void incomplete_forMemberField_bindings() { + BeanBinder otherBinder = new BeanBinder<>(TestBean.class); + TestClass testClass = new TestClass(); + + otherBinder.forMemberField(testClass.number) + .withConverter(new StringToIntegerConverter("")); + + // Should throw an IllegalStateException since the forMemberField + // binding has not been completed + otherBinder.setBean(new TestBean()); + } + @Test public void fieldBound_bindBean_fieldValueUpdated() { binder.bind(nameField, "firstname"); -- cgit v1.2.3