You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@wicket.apache.org by sv...@apache.org on 2019/04/19 18:24:25 UTC

[wicket] branch WICKET-6656-bean-validation-required created (now 0780849)

This is an automated email from the ASF dual-hosted git repository.

svenmeier pushed a change to branch WICKET-6656-bean-validation-required
in repository https://gitbox.apache.org/repos/asf/wicket.git.


      at 0780849  WICKET-6656 bean validation handle required annotations

This branch includes the following new commits:

     new 0780849  WICKET-6656 bean validation handle required annotations

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



[wicket] 01/01: WICKET-6656 bean validation handle required annotations

Posted by sv...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

svenmeier pushed a commit to branch WICKET-6656-bean-validation-required
in repository https://gitbox.apache.org/repos/asf/wicket.git

commit 0780849c9b974c0715ff476b792bef7d4c7587fd
Author: Sven Meier <sv...@apache.org>
AuthorDate: Fri Apr 19 09:56:39 2019 +0200

    WICKET-6656 bean validation handle required annotations
---
 .../wicket/bean/validation/AnnotationUtils.java    | 46 +++++++++++
 .../validation/BeanValidationConfiguration.java    | 46 ++++++++++-
 .../bean/validation/BeanValidationContext.java     | 14 ++++
 .../wicket/bean/validation/PropertyValidator.java  | 91 +++++-----------------
 .../validation/PropertyValidatorRequiredTest.java  | 21 ++---
 wicket-examples/pom.xml                            |  6 --
 .../bean/validation/BeanValidationApplication.java | 17 +++-
 .../bean/validation/BeanValidationPage.java        |  2 +-
 .../constraint/ValidPasswordValidator.java         |  6 +-
 9 files changed, 156 insertions(+), 93 deletions(-)

diff --git a/wicket-bean-validation/src/main/java/org/apache/wicket/bean/validation/AnnotationUtils.java b/wicket-bean-validation/src/main/java/org/apache/wicket/bean/validation/AnnotationUtils.java
new file mode 100644
index 0000000..be5e243
--- /dev/null
+++ b/wicket-bean-validation/src/main/java/org/apache/wicket/bean/validation/AnnotationUtils.java
@@ -0,0 +1,46 @@
+package org.apache.wicket.bean.validation;
+
+import java.lang.annotation.Annotation;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Set;
+
+import javax.validation.Validator;
+import javax.validation.groups.Default;
+import javax.validation.metadata.ConstraintDescriptor;
+
+class AnnotationUtils {
+
+	static List<ConstraintDescriptor<?>> findConstraints(Property property, Collection<Class<? extends Annotation>> annotationTypes)
+	{
+		BeanValidationContext config = BeanValidationConfiguration.get();
+		Validator validator = config.getValidator();
+
+		List<ConstraintDescriptor<?>> constraints = new ArrayList<>();
+
+		Iterator<ConstraintDescriptor<?>> it = new ConstraintIterator(validator, property);
+
+		while (it.hasNext())
+		{
+			ConstraintDescriptor<?> desc = it.next();
+			Annotation annotation = desc.getAnnotation();
+			Class<? extends Annotation> annotationType = annotation.annotationType();
+			if (annotationTypes.contains(annotationType))
+			{
+				constraints.add(desc);
+			}
+		}
+
+		return constraints;
+	}
+
+	static boolean canApplyToDefaultGroup(ConstraintDescriptor<?> constraint)
+	{
+		Set<Class<?>> groups = constraint.getGroups();
+		//the constraint can be applied to default group either if its group array is empty
+		//or if it contains javax.validation.groups.Default
+		return groups.size() == 0 || groups.contains(Default.class);
+	}
+}
diff --git a/wicket-bean-validation/src/main/java/org/apache/wicket/bean/validation/BeanValidationConfiguration.java b/wicket-bean-validation/src/main/java/org/apache/wicket/bean/validation/BeanValidationConfiguration.java
index d705a63..884ad9d 100644
--- a/wicket-bean-validation/src/main/java/org/apache/wicket/bean/validation/BeanValidationConfiguration.java
+++ b/wicket-bean-validation/src/main/java/org/apache/wicket/bean/validation/BeanValidationConfiguration.java
@@ -1,6 +1,8 @@
 package org.apache.wicket.bean.validation;
 
 import java.lang.annotation.Annotation;
+import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
@@ -8,7 +10,10 @@ import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.function.Supplier;
 
 import javax.validation.Validator;
+import javax.validation.constraints.NotEmpty;
+import javax.validation.constraints.NotNull;
 import javax.validation.constraints.Size;
+import javax.validation.metadata.ConstraintDescriptor;
 
 import org.apache.wicket.Application;
 import org.apache.wicket.MetaDataKey;
@@ -27,6 +32,28 @@ public class BeanValidationConfiguration implements BeanValidationContext
 	{
 	};
 
+	/**
+	 * Default list of annotations that make a component required.
+	 */
+	static final List<Class<? extends Annotation>> REQUIRED_ANNOTATIONS;
+	static
+	{
+		List<Class<? extends Annotation>> tmp = new ArrayList<>();
+		tmp.add(NotNull.class);
+		try
+		{
+			tmp.add(Class.forName("javax.validation.constraints.NotBlank")
+				.asSubclass(Annotation.class));
+			tmp.add(Class.forName("javax.validation.constraints.NotEmpty")
+				.asSubclass(Annotation.class));
+		}
+		catch (ClassNotFoundException e)
+		{
+			// ignore exception, we are using bean validation 1.1
+		}
+		REQUIRED_ANNOTATIONS = Collections.unmodifiableList(tmp);
+	}
+
 	private Supplier<Validator> validatorProvider = new DefaultValidatorProvider();
 
 	private IViolationTranslator violationTranslator = new DefaultViolationTranslator();
@@ -61,7 +88,6 @@ public class BeanValidationConfiguration implements BeanValidationContext
 		return this;
 	}
 
-
 	@Override
 	@SuppressWarnings("unchecked")
 	public <T extends Annotation> ITagModifier<T> getTagModifier(Class<T> annotationType)
@@ -131,8 +157,8 @@ public class BeanValidationConfiguration implements BeanValidationContext
 	 * Registers a violation translator
 	 *
 	 * @param violationTranslator
-	 *          A violation translator that will convert {@link javax.validation.ConstraintViolation}s
-	 *          into Wicket's {@link org.apache.wicket.validation.ValidationError}s
+	 *            A violation translator that will convert {@link javax.validation.ConstraintViolation}s into Wicket's
+	 *            {@link org.apache.wicket.validation.ValidationError}s
 	 */
 	public void setViolationTranslator(IViolationTranslator violationTranslator)
 	{
@@ -171,4 +197,18 @@ public class BeanValidationConfiguration implements BeanValidationContext
 		}
 		return null;
 	}
+
+	/**
+	 * By default {@link NotNull} and {@link NotEmpty} make a component required.
+	 * 
+	 * @param property
+	 *            property
+	 * @param groups
+	 *            groups
+	 */
+	@Override
+	public List<ConstraintDescriptor<?>> getRequiredConstraints(Property property)
+	{
+		return AnnotationUtils.findConstraints(property, REQUIRED_ANNOTATIONS);
+	}
 }
diff --git a/wicket-bean-validation/src/main/java/org/apache/wicket/bean/validation/BeanValidationContext.java b/wicket-bean-validation/src/main/java/org/apache/wicket/bean/validation/BeanValidationContext.java
index dd5c4a0..caf5936 100644
--- a/wicket-bean-validation/src/main/java/org/apache/wicket/bean/validation/BeanValidationContext.java
+++ b/wicket-bean-validation/src/main/java/org/apache/wicket/bean/validation/BeanValidationContext.java
@@ -1,8 +1,10 @@
 package org.apache.wicket.bean.validation;
 
 import java.lang.annotation.Annotation;
+import java.util.List;
 
 import javax.validation.Validator;
+import javax.validation.metadata.ConstraintDescriptor;
 
 import org.apache.wicket.markup.html.form.FormComponent;
 
@@ -36,6 +38,18 @@ public interface BeanValidationContext extends IPropertyResolver
 	 */
 	IViolationTranslator getViolationTranslator();
 
+	/**
+	 * Resolve the property for a component.
+	 * 
+	 * @param component component
+	 */
 	@Override
 	Property resolveProperty(FormComponent<?> component);
+	
+	/**
+	 * Which constraints make a component required.
+	 * 
+	 * @param property property
+	 */
+	List<ConstraintDescriptor<?>> getRequiredConstraints(Property property);
 }
\ No newline at end of file
diff --git a/wicket-bean-validation/src/main/java/org/apache/wicket/bean/validation/PropertyValidator.java b/wicket-bean-validation/src/main/java/org/apache/wicket/bean/validation/PropertyValidator.java
index 9cee8f8..6bfbbcf 100644
--- a/wicket-bean-validation/src/main/java/org/apache/wicket/bean/validation/PropertyValidator.java
+++ b/wicket-bean-validation/src/main/java/org/apache/wicket/bean/validation/PropertyValidator.java
@@ -1,9 +1,6 @@
 package org.apache.wicket.bean.validation;
 
-import java.lang.annotation.Annotation;
-import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.Collections;
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
@@ -11,8 +8,6 @@ import java.util.Set;
 
 import javax.validation.ConstraintViolation;
 import javax.validation.Validator;
-import javax.validation.constraints.NotNull;
-import javax.validation.groups.Default;
 import javax.validation.metadata.ConstraintDescriptor;
 
 import org.apache.wicket.Component;
@@ -21,8 +16,8 @@ import org.apache.wicket.markup.ComponentTag;
 import org.apache.wicket.markup.html.form.FormComponent;
 import org.apache.wicket.model.IModel;
 import org.apache.wicket.model.PropertyModel;
+import org.apache.wicket.validation.INullAcceptingValidator;
 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
@@ -57,33 +52,16 @@ import org.apache.wicket.validation.IValidator;
  * 
  * @param <T>
  */
-public class PropertyValidator<T> extends Behavior implements IValidator<T>
+public class PropertyValidator<T> extends Behavior implements INullAcceptingValidator<T>
 {
 	private static final Class<?>[] EMPTY = new Class<?>[0];
-	private static final List<Class<? extends Annotation>> NOT_NULL_ANNOTATIONS;
-	static
-	{
-		List<Class<? extends Annotation>> tmp = new ArrayList<>();
-		tmp.add(NotNull.class);
-		try
-		{
-			tmp.add(Class.forName("javax.validation.constraints.NotBlank")
-				.asSubclass(Annotation.class));
-			tmp.add(Class.forName("javax.validation.constraints.NotEmpty")
-				.asSubclass(Annotation.class));
-		}
-		catch (ClassNotFoundException e)
-		{
-			// ignore exception, we are using bean validation 1.1
-		}
-		NOT_NULL_ANNOTATIONS = Collections.unmodifiableList(tmp);
-	}
 
 	private FormComponent<T> component;
 
 	// the trailing underscore means that these members should not be used
 	// directly. ALWAYS use the respective getter instead.
 	private Property property_;
+
 	private final IModel<Class<?>[]> groups_;
 
 	/**
@@ -114,14 +92,17 @@ public class PropertyValidator<T> extends Behavior implements IValidator<T>
 
 	/**
 	 * To support debugging, trying to provide useful information where possible
+	 * 
 	 * @return
 	 */
-	private String createUnresolvablePropertyMessage(FormComponent<T> component) {
+	private String createUnresolvablePropertyMessage(FormComponent<T> component)
+	{
 		String baseMessage = "Could not resolve Bean Property from component: " + component
-				+ ". (Hints:) Possible causes are a typo in the PropertyExpression, a null reference or a model that does not work in combination with a "
-				+ IPropertyResolver.class.getSimpleName() + ".";
-        IModel<?> model = ValidationModelResolver.resolvePropertyModelFrom(component);
-		if (model != null) {
+			+ ". (Hints:) Possible causes are a typo in the PropertyExpression, a null reference or a model that does not work in combination with a "
+			+ IPropertyResolver.class.getSimpleName() + ".";
+		IModel<?> model = ValidationModelResolver.resolvePropertyModelFrom(component);
+		if (model != null)
+		{
 			baseMessage += " Model : " + model;
 		}
 		return baseMessage;
@@ -157,15 +138,14 @@ public class PropertyValidator<T> extends Behavior implements IValidator<T>
 		if (this.component != null)
 		{
 			throw new IllegalStateException( //
-				"This validator has already been added to component: "
-					+ this.component
+				"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");
+			throw new IllegalStateException(
+				getClass().getSimpleName() + " can only be added to FormComponents");
 		}
 
 		// TODO add a validation key that appends the type so we can have
@@ -187,6 +167,7 @@ public class PropertyValidator<T> extends Behavior implements IValidator<T>
 			// that model object is accessible (i.e. component is already added
 			// in a page).
 			requiredFlagSet = true;
+
 			if (isRequired())
 			{
 				this.component.setRequired(true);
@@ -204,52 +185,28 @@ public class PropertyValidator<T> extends Behavior implements IValidator<T>
 		}
 	}
 
-	private List<ConstraintDescriptor<?>> findNotNullConstraints(List<Class<? extends Annotation>> notNullAnnotationTypes)
+	protected boolean isRequired()
 	{
 		BeanValidationContext config = BeanValidationConfiguration.get();
-		Validator validator = config.getValidator();
-		Property property = getProperty();
-
-		List<ConstraintDescriptor<?>> constraints = new ArrayList<>();
-
-		Iterator<ConstraintDescriptor<?>> it = new ConstraintIterator(validator, property);
-
-		while (it.hasNext())
-		{
-			ConstraintDescriptor<?> desc = it.next();
-			Annotation annotation = desc.getAnnotation();
-			Class<? extends Annotation> annotationType = annotation.annotationType();
-			if (notNullAnnotationTypes.contains(annotationType))
-			{
-				constraints.add(desc);
-			}
-		}
-
-		return constraints;
-	}
-
-	boolean isRequired()
-	{
-		List<ConstraintDescriptor<?>> constraints = findNotNullConstraints(NOT_NULL_ANNOTATIONS);
 
+		List<ConstraintDescriptor<?>> constraints = config.getRequiredConstraints(getProperty());
 		if (constraints.isEmpty())
 		{
 			return false;
 		}
 
-		Set<Class<?>> validatorGroups = new HashSet<>();
-		validatorGroups.addAll(Arrays.asList(getGroups()));
+		HashSet<Class<?>> groups = new HashSet<Class<?>>(Arrays.asList(getGroups()));
 
 		for (ConstraintDescriptor<?> constraint : constraints)
 		{
-			if (canApplyToDefaultGroup(constraint) && validatorGroups.isEmpty())
+			if (AnnotationUtils.canApplyToDefaultGroup(constraint) && groups.size() == 0)
 			{
 				return true;
 			}
 
 			for (Class<?> constraintGroup : constraint.getGroups())
 			{
-				if (validatorGroups.contains(constraintGroup))
+				if (groups.contains(constraintGroup))
 				{
 					return true;
 				}
@@ -259,14 +216,6 @@ public class PropertyValidator<T> extends Behavior implements IValidator<T>
 		return false;
 	}
 
-	private boolean canApplyToDefaultGroup(ConstraintDescriptor<?> constraint)
-	{
-		Set<Class<?>> groups = constraint.getGroups();
-		//the constraint can be applied to default group either if its group array is empty
-		//or if it contains javax.validation.groups.Default
-		return groups.size() == 0 || groups.contains(Default.class);
-	}
-
 	@Override
 	@SuppressWarnings({ "rawtypes", "unchecked" })
 	public void onComponentTag(Component component, ComponentTag tag)
diff --git a/wicket-bean-validation/src/test/java/org/apache/wicket/bean/validation/PropertyValidatorRequiredTest.java b/wicket-bean-validation/src/test/java/org/apache/wicket/bean/validation/PropertyValidatorRequiredTest.java
index 4e56c9b..3a5342a 100644
--- a/wicket-bean-validation/src/test/java/org/apache/wicket/bean/validation/PropertyValidatorRequiredTest.java
+++ b/wicket-bean-validation/src/test/java/org/apache/wicket/bean/validation/PropertyValidatorRequiredTest.java
@@ -1,5 +1,15 @@
 package org.apache.wicket.bean.validation;
 
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.util.Arrays;
+import java.util.List;
+
+import javax.validation.Valid;
+import javax.validation.constraints.NotNull;
+
 import org.apache.wicket.MarkupContainer;
 import org.apache.wicket.markup.IMarkupResourceStreamProvider;
 import org.apache.wicket.markup.html.WebPage;
@@ -15,14 +25,6 @@ import org.apache.wicket.util.tester.WicketTesterExtension;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.RegisterExtension;
 
-import javax.validation.Valid;
-import javax.validation.constraints.NotNull;
-import java.util.Arrays;
-import java.util.List;
-
-import static org.junit.jupiter.api.Assertions.assertFalse;
-import static org.junit.jupiter.api.Assertions.assertTrue;
-
 public class PropertyValidatorRequiredTest
 {
 	@RegisterExtension
@@ -84,9 +86,8 @@ public class PropertyValidatorRequiredTest
 	public void testResolveComposedConstraints() throws Exception
 	{
 		Property property = new Property(DefaultPropertyResolverTest.BeanWithPassword.class, "password");
-		PropertyValidator<DefaultPropertyResolverTest.BeanWithPassword> propertyValidator = new PropertyValidator<>(property);
 
-		assertTrue(propertyValidator.isRequired());
+		assertEquals(1, new BeanValidationConfiguration().getRequiredConstraints(property).size());
 	}
 
 	public static class TestApplication extends MockApplication
diff --git a/wicket-examples/pom.xml b/wicket-examples/pom.xml
index faf9962..65e6bfd 100644
--- a/wicket-examples/pom.xml
+++ b/wicket-examples/pom.xml
@@ -56,11 +56,6 @@
 				<groupId>org.codelibs</groupId>
 				<artifactId>jhighlight</artifactId>
 				<version>1.0.3</version>
-				
-			</dependency>
-			<dependency>
-				<groupId>org.hibernate.validator</groupId>
-				<artifactId>hibernate-validator</artifactId>
 			</dependency>
 		</dependencies>
 	</dependencyManagement>
@@ -145,7 +140,6 @@
 		<dependency>
 			<groupId>org.hibernate.validator</groupId>
 			<artifactId>hibernate-validator</artifactId>
-			<version>6.0.13.Final</version>
 			<scope>compile</scope>
 		</dependency>
 		<dependency>
diff --git a/wicket-examples/src/main/java/org/apache/wicket/examples/bean/validation/BeanValidationApplication.java b/wicket-examples/src/main/java/org/apache/wicket/examples/bean/validation/BeanValidationApplication.java
index a2fd84c..222b629 100644
--- a/wicket-examples/src/main/java/org/apache/wicket/examples/bean/validation/BeanValidationApplication.java
+++ b/wicket-examples/src/main/java/org/apache/wicket/examples/bean/validation/BeanValidationApplication.java
@@ -16,8 +16,14 @@
  */
 package org.apache.wicket.examples.bean.validation;
 
+import java.util.Collections;
+import java.util.List;
+
+import javax.validation.metadata.ConstraintDescriptor;
+
 import org.apache.wicket.Page;
 import org.apache.wicket.bean.validation.BeanValidationConfiguration;
+import org.apache.wicket.bean.validation.Property;
 import org.apache.wicket.examples.WicketExampleApplication;
 
 public class BeanValidationApplication extends WicketExampleApplication
@@ -31,6 +37,15 @@ public class BeanValidationApplication extends WicketExampleApplication
 	@Override
 	protected void init()
 	{
-		new BeanValidationConfiguration().configure(this);
+		new BeanValidationConfiguration()
+		{
+			/**
+			 * Let bean-validation handle required constraints.
+			 */
+			@Override
+			public List<ConstraintDescriptor<?>> getRequiredConstraints(Property property) {
+				return Collections.emptyList();
+			}
+		}.configure(this);
 	}
 }
diff --git a/wicket-examples/src/main/java/org/apache/wicket/examples/bean/validation/BeanValidationPage.java b/wicket-examples/src/main/java/org/apache/wicket/examples/bean/validation/BeanValidationPage.java
index 3173867..435489f 100644
--- a/wicket-examples/src/main/java/org/apache/wicket/examples/bean/validation/BeanValidationPage.java
+++ b/wicket-examples/src/main/java/org/apache/wicket/examples/bean/validation/BeanValidationPage.java
@@ -54,7 +54,7 @@ public class BeanValidationPage extends WicketExamplePage
 		form.add(new TextField<>("name", new PropertyModel<String>(this, "person.name")).add(new PropertyValidator<>()));
 		form.add(new TextField<>("phone", new PropertyModel<String>(this, "person.phone")).add(new PropertyValidator<>()));
 		form.add(new TextField<>("email", new PropertyModel<String>(this, "person.email")).add(new PropertyValidator<>()));
-		LocalDateTextField dateField = new LocalDateTextField("birthdate", new PropertyModel<>(this, "person.birthdate"), FormatStyle.SHORT);
+		LocalDateTextField dateField = new LocalDateTextField("birthdate", new PropertyModel<>(this, "person.birthdate"), FormatStyle.MEDIUM);
 		form.add(dateField.add(new PropertyValidator<>()));
 		form.add(new Label("pattern", new PropertyModel<>(dateField, "textFormat")));
 		form.add(new TextField<>("password", new PropertyModel<String>(this, "person.password")).add(new PropertyValidator<>()));
diff --git a/wicket-examples/src/main/java/org/apache/wicket/examples/bean/validation/constraint/ValidPasswordValidator.java b/wicket-examples/src/main/java/org/apache/wicket/examples/bean/validation/constraint/ValidPasswordValidator.java
index 1b68dd3..f392501 100644
--- a/wicket-examples/src/main/java/org/apache/wicket/examples/bean/validation/constraint/ValidPasswordValidator.java
+++ b/wicket-examples/src/main/java/org/apache/wicket/examples/bean/validation/constraint/ValidPasswordValidator.java
@@ -38,7 +38,11 @@ public class ValidPasswordValidator implements ConstraintValidator<ValidPassword
 	{
 		boolean validationResult = true;
 
-		if (!CONTENT.matcher(value).matches())
+		if (value == null)
+		{
+			validationResult = false; 
+		}
+		else if (!CONTENT.matcher(value).matches())
 		{
 			validationResult = false;
 		}