fixed. looks like some code was formatted using a different formatter because some things that shouldnt have changed still did...but its a lot cleaner now. thanks for noticing. this just plays into my point, before all i had to do was import the projects into eclipse, now i have to remember to set the formatter. sucks. also will reduce the quality of patches we get from people, now we have to manually format the code after them.
-igor On Fri, Nov 15, 2013 at 12:36 PM, Martin Grigorov <[email protected]> wrote: > Hi Igor, > > > On Fri, Nov 15, 2013 at 10:30 PM, <[email protected]> wrote: > >> 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> { >> > > It seems you use different formatting style. Maybe because of the removed > .settings/ folders... > > It is very hard for me to review (and learn from) this change. > > >> 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; >> + >> + } >> + >> +} >> >>
