From a9fdc693a081c42018b7808b077c4b19337659fa Mon Sep 17 00:00:00 2001 From: Artur Date: Mon, 8 May 2017 14:37:21 +0300 Subject: [PATCH] Fix bean validation when using sub property bindings (#9248) Fixes #9242 --- .../java/com/vaadin/data/BeanPropertySet.java | 22 ++++- .../com/vaadin/data/BeanValidationBinder.java | 32 ++++++- .../java/com/vaadin/data/BeanBinderTest.java | 96 +++++++++++++++++-- 3 files changed, 135 insertions(+), 15 deletions(-) diff --git a/server/src/main/java/com/vaadin/data/BeanPropertySet.java b/server/src/main/java/com/vaadin/data/BeanPropertySet.java index f78945e9d1..0e94b14761 100644 --- a/server/src/main/java/com/vaadin/data/BeanPropertySet.java +++ b/server/src/main/java/com/vaadin/data/BeanPropertySet.java @@ -21,7 +21,6 @@ import java.io.IOException; import java.io.Serializable; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; -import java.util.Arrays; import java.util.Map; import java.util.Objects; import java.util.Optional; @@ -201,7 +200,17 @@ public class BeanPropertySet implements PropertySet { } } - private static class NestedBeanPropertyDefinition + /** + * Contains properties for a bean type which is nested in another + * definition. + * + * @since + * @param + * the bean type + * @param + * the value type returned by the getter and set by the setter + */ + public static class NestedBeanPropertyDefinition extends AbstractBeanPropertyDefinition { private final PropertyDefinition parent; @@ -250,6 +259,15 @@ public class BeanPropertySet implements PropertySet { return new SerializedPropertyDefinition(getPropertySet().beanType, parent.getName() + "." + getName()); } + + /** + * Gets the parent property definition. + * + * @return the property definition for the parent + */ + public PropertyDefinition getParent() { + return parent; + } } private static final ConcurrentMap, BeanPropertySet> instances = new ConcurrentHashMap<>(); diff --git a/server/src/main/java/com/vaadin/data/BeanValidationBinder.java b/server/src/main/java/com/vaadin/data/BeanValidationBinder.java index c152a67b0c..e125bc9862 100644 --- a/server/src/main/java/com/vaadin/data/BeanValidationBinder.java +++ b/server/src/main/java/com/vaadin/data/BeanValidationBinder.java @@ -19,6 +19,7 @@ import javax.validation.metadata.BeanDescriptor; import javax.validation.metadata.ConstraintDescriptor; import javax.validation.metadata.PropertyDescriptor; +import com.vaadin.data.BeanPropertySet.NestedBeanPropertyDefinition; import com.vaadin.data.util.BeanUtil; import com.vaadin.data.validator.BeanValidator; @@ -69,7 +70,7 @@ public class BeanValidationBinder extends Binder { *

* By default the {@link RequiredFieldConfigurator#DEFAULT} configurator is * used. - * + * * @param configurator * required indicator configurator, may be {@code null} */ @@ -80,9 +81,9 @@ public class BeanValidationBinder extends Binder { /** * Gets field required indicator configuration logic. - * + * * @see #setRequiredConfigurator(RequiredFieldConfigurator) - * + * * @return required indicator configurator, may be {@code null} */ public RequiredFieldConfigurator getRequiredConfigurator() { @@ -93,7 +94,8 @@ public class BeanValidationBinder extends Binder { protected BindingBuilder configureBinding( BindingBuilder binding, PropertyDefinition definition) { - BeanValidator validator = new BeanValidator(beanType, + Class actualBeanType = findBeanType(beanType, definition); + BeanValidator validator = new BeanValidator(actualBeanType, definition.getName()); if (requiredConfigurator != null) { configureRequired(binding, definition, validator); @@ -101,6 +103,28 @@ public class BeanValidationBinder extends Binder { return binding.withValidator(validator); } + /** + * Finds the bean type containing the property the given definition refers + * to. + * + * @param beanType + * the root beanType + * @param definition + * the definition for the property + * @return the bean type containing the given property + */ + @SuppressWarnings({ "rawtypes" }) + private Class findBeanType(Class beanType, + PropertyDefinition definition) { + if (definition instanceof NestedBeanPropertyDefinition) { + return ((NestedBeanPropertyDefinition) definition).getParent() + .getType(); + } else { + // Non nested properties must be defined in the main type + return beanType; + } + } + private void configureRequired(BindingBuilder binding, PropertyDefinition definition, BeanValidator validator) { assert requiredConfigurator != null; diff --git a/server/src/test/java/com/vaadin/data/BeanBinderTest.java b/server/src/test/java/com/vaadin/data/BeanBinderTest.java index d9707c7496..f494d71555 100644 --- a/server/src/test/java/com/vaadin/data/BeanBinderTest.java +++ b/server/src/test/java/com/vaadin/data/BeanBinderTest.java @@ -17,6 +17,8 @@ import org.junit.Assert; import org.junit.Before; import org.junit.Test; +import com.vaadin.data.BeanBinderTest.RequiredConstraints.SubConstraint; +import com.vaadin.data.BeanBinderTest.RequiredConstraints.SubSubConstraint; import com.vaadin.data.converter.StringToIntegerConverter; import com.vaadin.tests.data.bean.BeanToValidate; import com.vaadin.ui.CheckBoxGroup; @@ -33,7 +35,7 @@ public class BeanBinderTest private TextField number = new TextField(); } - private static class TestBean implements Serializable{ + private static class TestBean implements Serializable { private Set enums; private int number; @@ -54,7 +56,7 @@ public class BeanBinderTest } } - public static class RequiredConstraints implements Serializable{ + public static class RequiredConstraints implements Serializable { @NotNull @Max(10) private String firstname; @@ -67,7 +69,7 @@ public class BeanBinderTest private String lastname; private SubConstraint subfield; - + public String getFirstname() { return firstname; } @@ -91,8 +93,7 @@ public class BeanBinderTest public void setLastname(String lastname) { this.lastname = lastname; } - - + public SubConstraint getSubfield() { return subfield; } @@ -101,12 +102,15 @@ public class BeanBinderTest this.subfield = subfield; } + public static class SubConstraint implements Serializable { - public static class SubConstraint implements Serializable{ - @NotNull + @NotEmpty + @Size(min = 5) private String name; + private SubSubConstraint subsub; + public String getName() { return name; } @@ -114,6 +118,30 @@ public class BeanBinderTest public void setName(String name) { this.name = name; } + + public SubSubConstraint getSubsub() { + return subsub; + } + + public void setSubsub(SubSubConstraint subsub) { + this.subsub = subsub; + } + + } + + public static class SubSubConstraint implements Serializable { + + @Size(min = 10) + private String value; + + public String getValue() { + return value; + } + + public void setValue(String value) { + this.value = value; + } + } } @@ -363,14 +391,14 @@ public class BeanBinderTest Assert.assertTrue(field.isRequiredIndicatorVisible()); testSerialization(binder); } - + @Test public void subfield_name_fieldIsRequired() { BeanValidationBinder binder = new BeanValidationBinder<>( RequiredConstraints.class); RequiredConstraints bean = new RequiredConstraints(); bean.setSubfield(new RequiredConstraints.SubConstraint()); - + TextField field = new TextField(); binder.bind(field, "subfield.name"); binder.setBean(bean); @@ -379,6 +407,56 @@ public class BeanBinderTest testSerialization(binder); } + @Test + public void subsubfield_name_fieldIsRequired() { + BeanValidationBinder binder = new BeanValidationBinder<>( + RequiredConstraints.class); + RequiredConstraints bean = new RequiredConstraints(); + RequiredConstraints.SubConstraint subfield = new RequiredConstraints.SubConstraint(); + subfield.setSubsub(new SubSubConstraint()); + bean.setSubfield(subfield); + + TextField field = new TextField(); + binder.bind(field, "subfield.subsub.value"); + binder.setBean(bean); + + Assert.assertTrue(field.isRequiredIndicatorVisible()); + testSerialization(binder); + } + + @Test + public void subfield_name_valueCanBeValidated() { + BeanValidationBinder binder = new BeanValidationBinder<>( + RequiredConstraints.class); + TextField field = new TextField(); + + binder.bind(field, "subfield.name"); + RequiredConstraints bean = new RequiredConstraints(); + bean.setSubfield(new SubConstraint()); + binder.setBean(bean); + Assert.assertFalse(binder.validate().isOk()); + field.setValue("overfive"); + Assert.assertTrue(binder.validate().isOk()); + } + + @Test + public void subSubfield_name_valueCanBeValidated() { + BeanValidationBinder binder = new BeanValidationBinder<>( + RequiredConstraints.class); + TextField field = new TextField(); + + binder.bind(field, "subfield.subsub.value"); + RequiredConstraints bean = new RequiredConstraints(); + SubConstraint subfield = new SubConstraint(); + bean.setSubfield(subfield); + subfield.setSubsub(new SubSubConstraint()); + binder.setBean(bean); + + Assert.assertFalse(binder.validate().isOk()); + field.setValue("overtencharacters"); + Assert.assertTrue(binder.validate().isOk()); + } + private void assertInvalid(HasValue field, String message) { BinderValidationStatus status = binder.validate(); List> errors = status -- 2.39.5