Updated Branches: refs/heads/wicket-6.x 4bb19a48f -> 49922cdd5
WICKET-5418 properly handle groups in NonNull constraint Project: http://git-wip-us.apache.org/repos/asf/wicket/repo Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/49922cdd Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/49922cdd Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/49922cdd Branch: refs/heads/wicket-6.x Commit: 49922cdd5ccba3b97c59106d3aea769b8468962e Parents: 4bb19a4 Author: Igor Vaynberg <[email protected]> Authored: Fri Nov 15 12:30:22 2013 -0800 Committer: Igor Vaynberg <[email protected]> Committed: Fri Nov 15 12:30:34 2013 -0800 ---------------------------------------------------------------------- .../wicket-bean-validation/pom.xml | 6 + .../bean/validation/PropertyValidator.java | 212 ++++++++++--------- .../PropertyValidatorRequiredTest.java | 196 +++++++++++++++++ 3 files changed, 318 insertions(+), 96 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/wicket/blob/49922cdd/wicket-experimental/wicket-bean-validation/pom.xml ---------------------------------------------------------------------- diff --git a/wicket-experimental/wicket-bean-validation/pom.xml b/wicket-experimental/wicket-bean-validation/pom.xml index 9f3895c..460e7fc 100644 --- a/wicket-experimental/wicket-bean-validation/pom.xml +++ b/wicket-experimental/wicket-bean-validation/pom.xml @@ -25,5 +25,11 @@ <groupId>org.apache.wicket</groupId> <artifactId>wicket-core</artifactId> </dependency> + <dependency> + <groupId>org.hibernate</groupId> + <artifactId>hibernate-validator</artifactId> + <version>4.3.1.Final</version> + <scope>test</scope> + </dependency> </dependencies> </project> http://git-wip-us.apache.org/repos/asf/wicket/blob/49922cdd/wicket-experimental/wicket-bean-validation/src/main/java/org/apache/wicket/bean/validation/PropertyValidator.java ---------------------------------------------------------------------- diff --git a/wicket-experimental/wicket-bean-validation/src/main/java/org/apache/wicket/bean/validation/PropertyValidator.java b/wicket-experimental/wicket-bean-validation/src/main/java/org/apache/wicket/bean/validation/PropertyValidator.java index 032a818..93bd4c4 100644 --- a/wicket-experimental/wicket-bean-validation/src/main/java/org/apache/wicket/bean/validation/PropertyValidator.java +++ b/wicket-experimental/wicket-bean-validation/src/main/java/org/apache/wicket/bean/validation/PropertyValidator.java @@ -1,6 +1,10 @@ package org.apache.wicket.bean.validation; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.HashSet; import java.util.Iterator; +import java.util.List; import java.util.Set; import javax.validation.ConstraintViolation; @@ -18,40 +22,43 @@ import org.apache.wicket.validation.IValidatable; import org.apache.wicket.validation.IValidator; /** - * Validator that delegates to the bean validation framework. The integration has to be first - * configured using {@link BeanValidationConfiguration}. + * Validator that delegates to the bean validation framework. The integration + * has to be first configured using {@link BeanValidationConfiguration}. * * <p> - * The validator must be provided a {@link Property}, unless one can be resolved from the component - * implicitly. By default the configuration contains the {@link DefaultPropertyResolver} so - * {@link PropertyModel}s are supported out of the box - when attached to a component with a - * property model the property does not need to be specified explicitly. + * The validator must be provided a {@link Property}, unless one can be resolved + * from the component implicitly. By default the configuration contains the + * {@link DefaultPropertyResolver} so {@link PropertyModel}s are supported out + * of the box - when attached to a component with a property model the property + * does not need to be specified explicitly. * </p> * * <p> - * The validator will set the required flag on the form component it is attached to based on the - * presence of the @NotNull annotation. Notice, the required flag will only be set to {@code true}, - * components with the required flag already set to {@code true} will not have the flag set to - * {@code false} by this validator. + * The validator will set the required flag on the form component it is attached + * to based on the presence of the @NotNull annotation. Notice, the required + * flag will only be set to {@code true}, components with the required flag + * already set to {@code true} will not have the flag set to {@code false} by + * this validator. * </p> * * <p> - * The validator will allow {@link ITagModifier}s configured in {@link BeanValidationConfiguration} - * to mutate the markup tag of the component it is attached to. + * The validator will allow {@link ITagModifier}s configured in + * {@link BeanValidationConfiguration} to mutate the markup tag of the component + * it is attached to. * </p> * * <p> - * The validator specifies default error messages in the {@code PropertyValidator.properties} file. - * These values can be overridden in the application subclass' property files globally or in the - * page or panel properties locally. See this file for the default messages supported. + * The validator specifies default error messages in the + * {@code PropertyValidator.properties} file. These values can be overridden in + * the application subclass' property files globally or in the page or panel + * properties locally. See this file for the default messages supported. * </p> * * @author igor * * @param <T> */ -public class PropertyValidator<T> extends Behavior implements IValidator<T> -{ +public class PropertyValidator<T> extends Behavior implements IValidator<T> { private static final Class<?>[] EMPTY = new Class<?>[0]; private FormComponent<T> component; @@ -67,49 +74,41 @@ public class PropertyValidator<T> extends Behavior implements IValidator<T> */ private boolean requiredFlagSet; - public PropertyValidator(Class<?>... groups) - { + public PropertyValidator(Class<?>... groups) { this(null, groups); } - public PropertyValidator(IModel<Class<?>[]> groups) - { + public PropertyValidator(IModel<Class<?>[]> groups) { this(null, groups); } - public PropertyValidator(Property property, Class<?>... groups) - { + public PropertyValidator(Property property, Class<?>... groups) { this(property, new GroupsModel(groups)); } - public PropertyValidator(Property property, IModel<Class<?>[]> groups) - { + public PropertyValidator(Property property, IModel<Class<?>[]> groups) { this.property_ = property; this.groups_ = groups; } - private Property getProperty() - { - if (property_ == null) - { - property_ = BeanValidationConfiguration.get().resolveProperty(component); - if (property_ == null) - { + private Property getProperty() { + if (property_ == null) { + property_ = BeanValidationConfiguration.get().resolveProperty( + component); + if (property_ == null) { throw new IllegalStateException( - "Could not resolve Property from component: " + - component + - ". Either specify the Property in the constructor or use a model that works in combination with a " + - IPropertyResolver.class.getSimpleName() + - " to resolve the Property automatically"); + "Could not resolve Property from component: " + + component + + ". Either specify the Property in the constructor or use a model that works in combination with a " + + IPropertyResolver.class.getSimpleName() + + " to resolve the Property automatically"); } } return property_; } - private Class<?>[] getGroups() - { - if (groups_ == null) - { + private Class<?>[] getGroups() { + if (groups_ == null) { return EMPTY; } return groups_.getObject(); @@ -117,106 +116,127 @@ public class PropertyValidator<T> extends Behavior implements IValidator<T> @SuppressWarnings("unchecked") @Override - public void bind(Component component) - { - if (this.component != null) - { + public void bind(Component component) { + if (this.component != null) { throw new IllegalStateException( // - "This validator has already been added to component: " + this.component + - ". This validator does not support reusing instances, please create a new one"); + "This validator has already been added to component: " + + this.component + + ". This validator does not support reusing instances, please create a new one"); } - if (!(component instanceof FormComponent)) - { - throw new IllegalStateException(getClass().getSimpleName() + - " can only be added to FormComponents"); + if (!(component instanceof FormComponent)) { + throw new IllegalStateException(getClass().getSimpleName() + + " can only be added to FormComponents"); } - // TODO add a validation key that appends the type so we can have different messages for - // @Size on String vs Collection - done but need to add a key for each superclass/interface + // TODO add a validation key that appends the type so we can have + // different messages for + // @Size on String vs Collection - done but need to add a key for each + // superclass/interface - this.component = (FormComponent<T>)component; + this.component = (FormComponent<T>) component; } @Override - public void onConfigure(Component component) - { + public void onConfigure(Component component) { super.onConfigure(component); - if (requiredFlagSet == false) - { - // "Required" flag is calculated upon component's model property, so we must ensure, - // that model object is accessible (i.e. component is already added in a page). + if (requiredFlagSet == false) { + // "Required" flag is calculated upon component's model property, so + // we must ensure, + // that model object is accessible (i.e. component is already added + // in a page). requiredFlagSet = true; - setComponentRequiredFlag(); + if (isRequired()) { + this.component.setRequired(true); + } } } @Override - public void detach(Component component) - { + public void detach(Component component) { super.detach(component); - if (groups_ != null) - { + if (groups_ != null) { groups_.detach(); } } - /** - * Marks the form component required if necessary - */ - private void setComponentRequiredFlag() - { + private List<NotNull> findNotNullConstraints() { BeanValidationContext config = BeanValidationConfiguration.get(); Validator validator = config.getValidator(); Property property = getProperty(); - // if the property has a NotNull constraint mark the form component required + List<NotNull> constraints = new ArrayList<NotNull>(); + + Iterator<ConstraintDescriptor<?>> it = new ConstraintIterator( + validator, property); - Iterator<ConstraintDescriptor<?>> it = new ConstraintIterator(validator, property); - while (it.hasNext()) - { + while (it.hasNext()) { ConstraintDescriptor<?> desc = it.next(); - if (desc.getAnnotation().annotationType().equals(NotNull.class)) - { - component.setRequired(true); - break; + if (desc.getAnnotation().annotationType().equals(NotNull.class)) { + constraints.add((NotNull) desc.getAnnotation()); + } + } + + return constraints; + } + + boolean isRequired() { + List<NotNull> constraints = findNotNullConstraints(); + + if (constraints.isEmpty()) { + return false; + } + + HashSet<Class<?>> validatorGroups = new HashSet<Class<?>>(); + validatorGroups.addAll(Arrays.asList(getGroups())); + + for (NotNull constraint : constraints) { + if (constraint.groups().length == 0 && validatorGroups.isEmpty()) { + return true; + } + + for (Class<?> constraintGroup : constraint.groups()) { + if (validatorGroups.contains(constraintGroup)) { + return true; + } } } + + return false; } @Override @SuppressWarnings({ "rawtypes", "unchecked" }) - public void onComponentTag(Component component, ComponentTag tag) - { + public void onComponentTag(Component component, ComponentTag tag) { super.onComponentTag(component, tag); BeanValidationContext config = BeanValidationConfiguration.get(); Validator validator = config.getValidator(); Property property = getProperty(); - // find any tag modifiers that apply to the constraints of the property being validated + // find any tag modifiers that apply to the constraints of the property + // being validated // and allow them to modify the component tag - Iterator<ConstraintDescriptor<?>> it = new ConstraintIterator(validator, property, - getGroups()); + Iterator<ConstraintDescriptor<?>> it = new ConstraintIterator( + validator, property, getGroups()); - while (it.hasNext()) - { + while (it.hasNext()) { ConstraintDescriptor<?> desc = it.next(); - ITagModifier modifier = config.getTagModifier(desc.getAnnotation().annotationType()); + ITagModifier modifier = config.getTagModifier(desc.getAnnotation() + .annotationType()); - if (modifier != null) - { - modifier.modify((FormComponent<?>)component, tag, desc.getAnnotation()); + if (modifier != null) { + modifier.modify((FormComponent<?>) component, tag, + desc.getAnnotation()); } } } @SuppressWarnings("unchecked") @Override - public void validate(IValidatable<T> validatable) - { + public void validate(IValidatable<T> validatable) { BeanValidationContext config = BeanValidationConfiguration.get(); Validator validator = config.getValidator(); @@ -224,14 +244,14 @@ public class PropertyValidator<T> extends Behavior implements IValidator<T> // validate the value using the bean validator - Set<?> violations = validator.validateValue(property.getOwner(), property.getName(), - validatable.getValue(), getGroups()); + Set<?> violations = validator.validateValue(property.getOwner(), + property.getName(), validatable.getValue(), getGroups()); // iterate over violations and report them - for (ConstraintViolation<?> violation : (Set<ConstraintViolation<?>>)violations) - { - validatable.error(config.getViolationTranslator().convert(violation)); + for (ConstraintViolation<?> violation : (Set<ConstraintViolation<?>>) violations) { + validatable.error(config.getViolationTranslator() + .convert(violation)); } } http://git-wip-us.apache.org/repos/asf/wicket/blob/49922cdd/wicket-experimental/wicket-bean-validation/src/test/java/org/apache/wicket/bean/validation/PropertyValidatorRequiredTest.java ---------------------------------------------------------------------- diff --git a/wicket-experimental/wicket-bean-validation/src/test/java/org/apache/wicket/bean/validation/PropertyValidatorRequiredTest.java b/wicket-experimental/wicket-bean-validation/src/test/java/org/apache/wicket/bean/validation/PropertyValidatorRequiredTest.java new file mode 100644 index 0000000..e5ee0cc --- /dev/null +++ b/wicket-experimental/wicket-bean-validation/src/test/java/org/apache/wicket/bean/validation/PropertyValidatorRequiredTest.java @@ -0,0 +1,196 @@ +package org.apache.wicket.bean.validation; + +import static org.junit.Assert.*; + +import javax.validation.constraints.NotNull; + +import org.apache.wicket.MarkupContainer; +import org.apache.wicket.markup.IMarkupResourceStreamProvider; +import org.apache.wicket.markup.html.WebPage; +import org.apache.wicket.markup.html.form.Form; +import org.apache.wicket.markup.html.form.FormComponent; +import org.apache.wicket.markup.html.form.TextField; +import org.apache.wicket.mock.MockApplication; +import org.apache.wicket.model.PropertyModel; +import org.apache.wicket.util.resource.IResourceStream; +import org.apache.wicket.util.resource.StringResourceStream; +import org.apache.wicket.util.tester.WicketTester; +import org.apache.wicket.util.tester.WicketTesterScope; +import org.junit.Rule; +import org.junit.Test; + +public class PropertyValidatorRequiredTest { + @Rule + public static WicketTesterScope scope = new WicketTesterScope() { + protected WicketTester create() { + return new WicketTester(new TestApplication()); + }; + }; + + @Test + public void test() { + TestPage page = scope.getTester().startPage(TestPage.class); + + // no group + assertTrue(page.input1.isRequired()); + assertFalse(page.input2.isRequired()); + assertFalse(page.input3.isRequired()); + assertFalse(page.input4.isRequired()); + + // group1 + assertFalse(page.input5.isRequired()); + assertTrue(page.input6.isRequired()); + assertFalse(page.input7.isRequired()); + assertTrue(page.input8.isRequired()); + + // group2 + assertFalse(page.input9.isRequired()); + assertFalse(page.input10.isRequired()); + assertTrue(page.input11.isRequired()); + assertTrue(page.input12.isRequired()); + + // group1+group2 + assertFalse(page.input13.isRequired()); + assertTrue(page.input14.isRequired()); + assertTrue(page.input15.isRequired()); + assertTrue(page.input16.isRequired()); + + // group3 + assertFalse(page.input17.isRequired()); + assertFalse(page.input18.isRequired()); + assertFalse(page.input19.isRequired()); + assertFalse(page.input20.isRequired()); + + } + + public static class TestApplication extends MockApplication { + @Override + protected void init() { + super.init(); + new BeanValidationConfiguration().configure(this); + } + } + + public static class TestPage extends WebPage implements + IMarkupResourceStreamProvider { + + private TestBean bean = new TestBean(); + private FormComponent<String> input1, input2, input3, input4, input5, + input6, input7, input8, input9, input10, input11, input12, + input13, input14, input15, input16, input17, input18, input19, + input20; + + public TestPage() { + Form<?> form = new Form<Void>("form"); + add(form); + + input1 = new TextField<String>("input1", new PropertyModel<String>( + this, "bean.property")) + .add(new PropertyValidator<String>()); + input2 = new TextField<String>("input2", new PropertyModel<String>( + this, "bean.propertyOne")) + .add(new PropertyValidator<String>()); + input3 = new TextField<String>("input3", new PropertyModel<String>( + this, "bean.propertyTwo")) + .add(new PropertyValidator<String>()); + input4 = new TextField<String>("input4", new PropertyModel<String>( + this, "bean.propertyOneTwo")) + .add(new PropertyValidator<String>()); + + input5 = new TextField<String>("input5", new PropertyModel<String>( + this, "bean.property")).add(new PropertyValidator<String>( + GroupOne.class)); + input6 = new TextField<String>("input6", new PropertyModel<String>( + this, "bean.propertyOne")) + .add(new PropertyValidator<String>(GroupOne.class)); + input7 = new TextField<String>("input7", new PropertyModel<String>( + this, "bean.propertyTwo")) + .add(new PropertyValidator<String>(GroupOne.class)); + input8 = new TextField<String>("input8", new PropertyModel<String>( + this, "bean.propertyOneTwo")) + .add(new PropertyValidator<String>(GroupOne.class)); + + input9 = new TextField<String>("input9", new PropertyModel<String>( + this, "bean.property")).add(new PropertyValidator<String>( + GroupTwo.class)); + input10 = new TextField<String>("input10", + new PropertyModel<String>(this, "bean.propertyOne")) + .add(new PropertyValidator<String>(GroupTwo.class)); + input11 = new TextField<String>("input11", + new PropertyModel<String>(this, "bean.propertyTwo")) + .add(new PropertyValidator<String>(GroupTwo.class)); + input12 = new TextField<String>("input12", + new PropertyModel<String>(this, "bean.propertyOneTwo")) + .add(new PropertyValidator<String>(GroupTwo.class)); + + input13 = new TextField<String>("input13", + new PropertyModel<String>(this, "bean.property")) + .add(new PropertyValidator<String>(GroupOne.class, + GroupTwo.class)); + input14 = new TextField<String>("input14", + new PropertyModel<String>(this, "bean.propertyOne")) + .add(new PropertyValidator<String>(GroupOne.class, + GroupTwo.class)); + input15 = new TextField<String>("input15", + new PropertyModel<String>(this, "bean.propertyTwo")) + .add(new PropertyValidator<String>(GroupOne.class, + GroupTwo.class)); + input16 = new TextField<String>("input16", + new PropertyModel<String>(this, "bean.propertyOneTwo")) + .add(new PropertyValidator<String>(GroupOne.class, + GroupTwo.class)); + + input17 = new TextField<String>("input17", + new PropertyModel<String>(this, "bean.property")) + .add(new PropertyValidator<String>(GroupThree.class)); + input18 = new TextField<String>("input18", + new PropertyModel<String>(this, "bean.propertyOne")) + .add(new PropertyValidator<String>(GroupThree.class)); + input19 = new TextField<String>("input19", + new PropertyModel<String>(this, "bean.propertyTwo")) + .add(new PropertyValidator<String>(GroupThree.class)); + input20 = new TextField<String>("input20", + new PropertyModel<String>(this, "bean.propertyOneTwo")) + .add(new PropertyValidator<String>(GroupThree.class)); + + form.add(input1, input2, input3, input4, input5, input6, input7, + input8, input9, input10, input11, input12, input13, + input14, input15, input16, input17, input18, input19, + input20); + + } + + @Override + public IResourceStream getMarkupResourceStream( + MarkupContainer container, Class<?> containerClass) { + return new StringResourceStream( + "<form wicket:id='form'><input wicket:id='input1'/><input wicket:id='input2'/><input wicket:id='input3'/><input wicket:id='input4'/><input wicket:id='input5'/><input wicket:id='input6'/><input wicket:id='input7'/><input wicket:id='input8'/><input wicket:id='input9'/><input wicket:id='input10'/><input wicket:id='input11'/><input wicket:id='input12'/><input wicket:id='input13'/><input wicket:id='input14'/><input wicket:id='input15'/><input wicket:id='input16'/><input wicket:id='input17'/><input wicket:id='input18'/><input wicket:id='input19'/><input wicket:id='input20'/></form>"); + } + + } + + public static interface GroupOne { + } + + public static interface GroupTwo { + } + + public static interface GroupThree { + } + + public static class TestBean { + @NotNull + String property; + + @NotNull(groups = { GroupOne.class }) + String propertyOne; + + @NotNull(groups = { GroupTwo.class }) + String propertyTwo; + + @NotNull(groups = { GroupOne.class, GroupTwo.class }) + String propertyOneTwo; + + } + +}
