WICKET-6499 Support Bean Validation 2.0 #246

theigl · 2017-11-19T10:43:09Z

This PR adds support for Bean Validation 2.0 to wicket-bean-validation.

It extends scanning of @NotNull annotations to @NotEmpty and @NotBlank and supports registering further annotations that should be treated as not-null constraints to guard against future changes.

See https://issues.apache.org/jira/browse/WICKET-6499

solomax · 2017-11-19T10:57:12Z

wicket-bean-validation/src/main/java/org/apache/wicket/bean/validation/PropertyValidator.java


-		HashSet<Class<?>> validatorGroups = new HashSet<Class<?>>();
+		HashSet<Class<?>> validatorGroups = new HashSet<>();
 		validatorGroups.addAll(Arrays.asList(getGroups()));


I would use Set here

solomax · 2017-11-19T10:57:17Z

wicket-bean-validation/src/main/java/org/apache/wicket/bean/validation/PropertyValidator.java


-		for (NotNull constraint : constraints)
+		for (Annotation constraint : constraints.keySet())
 		{


entrySet() can be used here to avoid double search

theigl · 2017-11-19T11:12:07Z

@solomax: Thanks for the quick feedback. I adjusted the PR accordingly.

solomax · 2017-11-19T11:14:23Z

wicket-bean-validation/src/main/java/org/apache/wicket/bean/validation/PropertyValidator.java

 		{
-			if (canApplyToDefaultGroup(constraint) && validatorGroups.isEmpty())
+			final Annotation annotation = entry.getKey();
+			ConstraintDescriptor<?> constraintDescriptor = constraints.get(annotation);


you can use entry.getValue() here

Sorry, totally overlooked that. Changed.

theigl · 2017-11-20T08:53:31Z

...lidation/src/main/java/org/apache/wicket/bean/validation/PropertyValidator_fr.properties.xml

 	<entry key="javax.validation.constraints.Max.message">Le champ '${label}' doit être inférieur ou égal à ${value}</entry>
 	<entry key="javax.validation.constraints.Min.message">Le champ '${label}' doit être supérieur ou égal à ${value}</entry>
 	<entry key="javax.validation.constraints.NotNull.message">Le champ '${label}' ne doit pas être null</entry>
+	<entry key="javax.validation.constraints.NotBlank.message">Le champ '${label}' ne peut pas être vide</entry>


A couple of other annotations have been added to BV 2.0 as well:

new built-in constraints: @Email, @notempty, @notblank, @positive, @PositiveOrZero, @Negative, @NegativeOrZero, @PastOrPresent and @FutureOrPresent

@solomax: Should we add translations for these new constraints?

+1 for new labels, I'll add _ru localization ...

Added properties for English and French. My French is a bit rusty, but the translations should be ok.

We should probably copy the properties from the HV implementation:

https://github.com/hibernate/hibernate-validator/tree/master/engine/src/main/resources/org/hibernate/validator

It is APL licensed so, I guess, we can copy
I would better use existing ones, but not sure how is this possible :(

The existing ones could probably be loaded via a custom IStringResourceLoader but this is out of the scope of this PR.

@solomax: What else do you want me to do for this PR so it can be merged?

OK with me :)

svenmeier · 2017-11-21T22:29:33Z

Actually I'm wondering whether we should just add another method to ITagModifier (possibly with better names):

public interface ITagModifier<T extends Annotation>
{
  void modify(FormComponent<?> component, T annotation);
	
  void modify(FormComponent<?> component, ComponentTag tag, T annotation);
}

This way we can use the identical lookup BeanValidationConfiguration#getTagModifier() to alter the component itself from #onConfigure() and the component tag from #onComponentTag().

All that's required for the new annotations is then:

register(Size.class, new SizeTagModifier());
register(NotNull.class, new RequiredTagModifier<NotNull>());
register(NotEmpty.class, new RequiredTagModifier<NotEmpty>());
register(NotBlank.class, new RequiredTagModifier<NotBlank>());

theigl · 2017-11-22T18:18:23Z

@svenmeier: This makes a lot of sense. I'm not sure however how much time I'll find the next weeks to work on this.

I suggest the following approach:

I'll look into this this week and try to rework the code as you suggested.
If It takes too much time and I can't finish it, I vote for merging a very simple version of this PR that only applies the changes required for making wicket-bean-validation work with BV 2.0. I'll remove the API changes I introduced in BeanValidationConfiguration and hard-code the list of built-in annotations that imply a not-null constraint. This way, we do not introduce any new methods that we would later remove if your solution is implemented.

WDYT?

svenmeier · 2017-11-22T20:30:35Z

That's a very good approach, thank you.

…API changes

theigl · 2017-11-23T09:59:43Z

I pushed the simplified implementation without API changes in 145e12e.

martin-g · 2017-11-24T08:01:51Z

wicket-bean-validation/src/main/java/org/apache/wicket/bean/validation/PropertyValidator.java

-import java.util.List;
-import java.util.Set;
+import java.lang.annotation.Annotation;
+import java.util.*;


No * imports please

solomax · 2017-11-27T13:11:55Z

I believe this can be closed
@theigl could you please close this PR?

theigl force-pushed the WICKET-6499-bean-validation-2x branch 2 times, most recently from 40824aa to fd07047 Compare November 19, 2017 10:49

solomax reviewed Nov 19, 2017

View reviewed changes

theigl force-pushed the WICKET-6499-bean-validation-2x branch from fd07047 to 46b4634 Compare November 19, 2017 11:09

solomax reviewed Nov 19, 2017

View reviewed changes

theigl force-pushed the WICKET-6499-bean-validation-2x branch 2 times, most recently from 36fbc18 to 65e9d88 Compare November 20, 2017 08:50

theigl commented Nov 20, 2017

View reviewed changes

WICKET-6499 Support Bean Validation 2.0

a6e3268

theigl force-pushed the WICKET-6499-bean-validation-2x branch from 65e9d88 to a6e3268 Compare November 20, 2017 09:09

WICKET-6499 Use List instead of Map to collect constraints

7cc4de2

WICKET-6499 Use constant for list of not-null constraints and revert …

145e12e

…API changes

theigl force-pushed the WICKET-6499-bean-validation-2x branch from 7579651 to 145e12e Compare November 23, 2017 09:58

martin-g approved these changes Nov 24, 2017

View reviewed changes

WICKET-6499 Use explicit imports instead of "*"

a5bfb45

theigl closed this Nov 27, 2017

theigl deleted the WICKET-6499-bean-validation-2x branch November 27, 2017 13:58

WICKET-6499 Support Bean Validation 2.0 #246

WICKET-6499 Support Bean Validation 2.0 #246

Uh oh!

Conversation

theigl commented Nov 19, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

theigl commented Nov 19, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

svenmeier commented Nov 21, 2017

Uh oh!

theigl commented Nov 22, 2017

Uh oh!

svenmeier commented Nov 22, 2017

Uh oh!

theigl commented Nov 23, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

solomax commented Nov 27, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants