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/23 19:06:13 UTC

[wicket] branch master updated: WICKET-6656 allow bean validation to handle required annotations

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

svenmeier pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/wicket.git


The following commit(s) were added to refs/heads/master by this push:
     new b5bf661  WICKET-6656 allow bean validation to handle required annotations
b5bf661 is described below

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

    WICKET-6656 allow bean validation to handle required annotations
    
    and updated dependencies; this closes #353
---
 .../validation/BeanValidationConfiguration.java    |  44 ++++++++-
 .../bean/validation/BeanValidationContext.java     |  14 +++
 .../wicket/bean/validation/PropertyValidator.java  | 110 +++++++--------------
 wicket-examples/pom.xml                            |   6 --
 .../constraint/ValidPasswordValidator.java         |   6 +-
 5 files changed, 97 insertions(+), 83 deletions(-)

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..7971c84 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,16 @@ public class BeanValidationConfiguration implements BeanValidationContext
 		}
 		return null;
 	}
+
+	/**
+	 * By default {@link NotNull} and {@link NotEmpty} constraints make a component required.
+	 * 
+	 * @param constraint
+	 *            constraint 
+	 */
+	@Override
+	public boolean isRequiredConstraint(ConstraintDescriptor<?> constraint)
+	{
+		return REQUIRED_ANNOTATIONS.contains(constraint.getAnnotation().annotationType());
+	}
 }
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..8f1f8b9 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
@@ -3,6 +3,7 @@ package org.apache.wicket.bean.validation;
 import java.lang.annotation.Annotation;
 
 import javax.validation.Validator;
+import javax.validation.metadata.ConstraintDescriptor;
 
 import org.apache.wicket.markup.html.form.FormComponent;
 
@@ -36,6 +37,19 @@ public interface BeanValidationContext extends IPropertyResolver
 	 */
 	IViolationTranslator getViolationTranslator();
 
+	/**
+	 * Resolve the property for a component.
+	 * 
+	 * @param component component
+	 */
 	@Override
 	Property resolveProperty(FormComponent<?> component);
+	
+	/**
+	 * Does the given constraint make a component required.
+	 * 
+	 * @param constraint constraint
+	 * @return <code>true</code> if required
+	 */
+	boolean isRequiredConstraint(ConstraintDescriptor<?> constraint);
 }
\ 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..437c04a 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,17 +1,12 @@
 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;
 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;
 
@@ -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,55 +185,38 @@ public class PropertyValidator<T> extends Behavior implements IValidator<T>
 		}
 	}
 
-	private List<ConstraintDescriptor<?>> findNotNullConstraints(List<Class<? extends Annotation>> notNullAnnotationTypes)
+	/**
+	 * Should this property make the owning component required.
+	 * 
+	 * @return <code>true</code> if required
+	 * 
+	 * @see BeanValidationContext#isRequiredConstraint(ConstraintDescriptor)
+	 */
+	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);
+		HashSet<Class<?>> groups = new HashSet<Class<?>>(Arrays.asList(getGroups()));
 
+		Iterator<ConstraintDescriptor<?>> it = new ConstraintIterator(config.getValidator(), getProperty());
 		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);
-
-		if (constraints.isEmpty())
-		{
-			return false;
-		}
-
-		Set<Class<?>> validatorGroups = new HashSet<>();
-		validatorGroups.addAll(Arrays.asList(getGroups()));
-
-		for (ConstraintDescriptor<?> constraint : constraints)
-		{
-			if (canApplyToDefaultGroup(constraint) && validatorGroups.isEmpty())
-			{
-				return true;
-			}
-
-			for (Class<?> constraintGroup : constraint.getGroups())
+			ConstraintDescriptor<?> constraint = it.next();
+			
+			if (config.isRequiredConstraint(constraint))
 			{
-				if (validatorGroups.contains(constraintGroup))
+				if (canApplyToDefaultGroup(constraint) && groups.size() == 0)
 				{
 					return true;
 				}
+		
+				for (Class<?> constraintGroup : constraint.getGroups())
+				{
+					if (groups.contains(constraintGroup))
+					{
+						return true;
+					}
+				}
 			}
 		}
 
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/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;
 		}