refactor(validation): cleanup UserNameValidator #8531

- Reuse existing constraints instead of creating our own validator
- Provide programmatic method to use the same from code like OAuth2 etc
- Add much more tests, also for actual bean validation

src/main/java/ValidationMessages.properties

@@ -2,7 +2,7 @@ user.firstName=Please enter your first name.
 user.lastName=Please enter your last name.
 user.invalidEmail=Please enter a valid email address.
 user.enterUsername=Please enter a username.
-user.usernameLength=Username must be between 2 and 60 characters.
+user.usernameLength=Username must be between {min} and {max} characters.
 user.illegalCharacters=Found an illegal character(s). Valid characters are a-Z, 0-9, '_', '-', and '.'.
 
 user.enterNickname=Please enter a nickname.

src/main/java/edu/harvard/iq/dataverse/authorization/providers/builtin/BuiltinUser.java

@@ -17,8 +17,8 @@
 import javax.persistence.OneToOne;
 import javax.persistence.Table;
 import javax.persistence.Transient;
+import javax.validation.constraints.NotBlank;
 import javax.validation.constraints.Size;
-import org.hibernate.validator.constraints.NotBlank;
 
 /**
  *
@@ -41,10 +41,8 @@ public class BuiltinUser implements Serializable {
     @Id
     @GeneratedValue(strategy = GenerationType.IDENTITY)
     private Long id;
-
-    @NotBlank(message = "{user.enterUsername}")
-    @Size(min=2, max=60, message = "{user.usernameLength}")
-    @ValidateUserName(message = "{user.illegalCharacters}")
+
+    @ValidateUserName
     @Column(nullable = false, unique=true)  
     private String userName;
 

src/main/java/edu/harvard/iq/dataverse/validation/UserNameValidator.java

@@ -1,61 +1,46 @@
-/*
- * To change this license header, choose License Headers in Project Properties.
- * To change this template file, choose Tools | Templates
- * and open the template in the editor.
- */
 package edu.harvard.iq.dataverse.validation;
 
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
-import javax.validation.ConstraintValidator;
-import javax.validation.ConstraintValidatorContext;
 
 /**
- *
- * @author sarahferry
- * Modeled after PasswordValidator and EMailValidator
+ * This class is not implementing the ConstraintValidatorMatcher interface, as this is not necessary any more.
+ * It serves as the storage for the annotation configuration and convenient interface to a programmatic
+ * validation of usernames.
  */
-
-public class UserNameValidator implements ConstraintValidator<ValidateUserName, String> {
-    @Override
-    public void initialize(ValidateUserName constraintAnnotation) {
-
-    }
+public class UserNameValidator {
 
-    // note: while the ConstraintValidatorContext is not used in this method, 
-    // it is required in order to impelement the ConstraintValidator interface
-    @Override
-    public boolean isValid(String value, ConstraintValidatorContext context) {
-        return isUserNameValid(value);
-    }
+    public static final int MIN_CHARS = 2;
+    public static final int MAX_CHARS = 60;
+
+    // NOTE: the size is checked by either the @Size annotation of @ValidateUserName or programmatically below!
+    public static final String USERNAME_PATTERN = "[a-zA-Z0-9\\_\\-\\.]*";
+
+    /*
+     * If you would like to support accents or chinese characters in usernames, choose one of the below.
+     *
+     * With support for accents:
+    private static final String USERNAME_PATTERN = "[a-zA-Z0-9\\_\\-\\.À-ÿ\\u00C0-\\u017F]";
+     *
+     * With support for chinese characters:
+    private static final String USERNAME_PATTERN = "[a-zA-Z0-9\\_\\-\\.\\x{4e00}-\\x{9fa5}]";
+     */
+
+    private static final Matcher usernameMatcher = Pattern.compile(USERNAME_PATTERN).matcher("");
 
     /**
-     * Here we will validate the username
+     * Validate a username against the pattern in {@link #USERNAME_PATTERN}
+     * and check for length: min {@link #MIN_CHARS} chars, max {@link #MAX_CHARS} chars.
+     * Nullsafe - null is considered invalid (as is empty).
      *
-     * @param username
-     * @return boolean 
+     * @param username The username to validate
+     * @return true if matching, false otherwise
      */
     public static boolean isUserNameValid(final String username) {
-        if (username == null) {
-            return false;
-        }
-        //TODO: What other characters do we need to support?
-        String validCharacters = "[a-zA-Z0-9\\_\\-\\.";
-        /*
-         * if you would like to support accents or chinese characters, uncomment this
-         *
-        //support accents
-        validCharacters += "À-ÿ\\u00C0-\\u017F";
-        //support chinese characters
-        validCharacters += "\\x{4e00}-\\x{9fa5}";
-        *
-        */
-        //end
-        validCharacters += "]";
-        validCharacters += "{2,60}"; //must be between 2 and 60 characters for user name
-        Pattern p = Pattern.compile(validCharacters);
-        Matcher m = p.matcher(username);
-        return m.matches();
+        return username != null &&
+               username.length() >= MIN_CHARS &&
+               username.length() <= MAX_CHARS &&
+               usernameMatcher.reset(username).matches();
     }
 
 }

src/main/java/edu/harvard/iq/dataverse/validation/ValidateUserName.java

@@ -12,6 +12,9 @@
 import java.lang.annotation.Target;
 import javax.validation.Constraint;
 import javax.validation.Payload;
+import javax.validation.constraints.NotBlank;
+import javax.validation.constraints.Size;
+import javax.validation.constraints.Pattern;
 
 /**
  *
@@ -20,14 +23,19 @@
 
 @Target({FIELD})
 @Retention(RUNTIME)
-@Constraint(validatedBy = {UserNameValidator.class})
-@Documented
-public @interface  ValidateUserName {
-  String message() default "Failed Validation Username";
+// This is empty by intention - we are just recombining existing bean validation here.
+// The class UserNameValidator is just for convenience and historic reasons.
+@Constraint(validatedBy = {})
 
-  Class<?>[] groups() default {};
+@NotBlank(message = "{user.enterUsername}")
+@Size(min = UserNameValidator.MIN_CHARS, max = UserNameValidator.MAX_CHARS, message = "{user.usernameLength}")
+@Pattern(regexp = UserNameValidator.USERNAME_PATTERN, message = "{user.illegalCharacters}")
 
-  Class<? extends Payload>[] payload() default {};
+@Documented
+public @interface ValidateUserName {
+    String message() default "";
 
+    Class<?>[] groups() default {};
+
+    Class<? extends Payload>[] payload() default {};
 }
-

src/test/java/edu/harvard/iq/dataverse/validation/UserNameValidatorTest.java

@@ -1,76 +1,82 @@
-/*
- * To change this license header, choose License Headers in Project Properties.
- * To change this template file, choose Tools | Templates
- * and open the template in the editor.
- */
 package edu.harvard.iq.dataverse.validation;
 
-import static org.junit.Assert.assertEquals;
+import java.util.Set;
+import java.util.stream.Stream;
 
-import java.util.Arrays;
-import java.util.Collection;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
 
-import edu.harvard.iq.dataverse.validation.UserNameValidator;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
-import org.junit.runners.Parameterized.Parameters;
+import javax.validation.ConstraintViolation;
+import javax.validation.Validation;
+import javax.validation.Validator;
 
-/**
- *
- * @author sarahferry
- * @author alexscheitlin
- */
-@RunWith(Parameterized.class)
-public class UserNameValidatorTest {
-
-    public boolean isValid;
-    public String userName;
-
-    public UserNameValidatorTest(boolean isValid, String userName) {
-        this.isValid = isValid;
-        this.userName = userName;
-    }
+import static org.junit.jupiter.api.Assertions.assertEquals;
 
-    @Parameters
-    public static Collection<Object[]> parameters() {
-        return Arrays.asList(new Object[][] {
+public class UserNameValidatorTest {
+
+    public static Stream<Arguments> usernameExamples() {
+        return Stream.of(
             // good usernames
-            { true, "sarah" },
-            { true, ".-_5Arah_-." },
-
+            Arguments.of(true, "sarah"),
+            Arguments.of(true, ".-_5Arah_-."),
             // dont allow accents
-            { false, "àèìòùÀÈÌÒÙáéíóúýÁÉÍÓÚÝâêîôûÂÊÎÔÛãñõÃÑÕäëïöüÿÄËÏÖÜŸçÇßØøÅåÆæœ" },
-
+            Arguments.of(false, "àèìòùÀÈÌÒÙáéíóúýÁÉÍÓÚÝâêîôûÂÊÎÔÛãñõÃÑÕäëïöüÿÄËÏÖÜŸçÇßØøÅåÆæœ"),
             // dont allow chinese characters
-            { false, "谁日吧爸好" },
-
+            Arguments.of(false, "谁日吧爸好"),
             // dont allow middle white space
-            { false, "sarah f" },
-
+            Arguments.of(false, "sarah f"),
             // dont allow leading white space
-            { false, " sarah" },
-
+            Arguments.of(false, " sarah"),
             // dont allow trailing white space
-            { false, "sarah " },
-
+            Arguments.of(false, "sarah "),
             // dont allow symbols
-            { false, "sarah!" },
-            { false, "sarah?" },
-            { false, "sarah:(" },
-            { false, "💲🅰️®️🅰️🚧" },
-
+            Arguments.of(false, "sarah!"),
+            Arguments.of(false, "sarah?"),
+            Arguments.of(false, "sarah:("),
+            Arguments.of(false, "💲🅰️®️🅰️🚧"),
             // only allow between 2 and 60 characters
-            { false, "q" },
-            { true, "q2" },
-            { false, "q2jsalfhjopiwurtiosfhkdhasjkdhfgkfhkfrhnefcn4cqonroclmooi4oiqwhrfq4jrlqhaskdalwehrlwhflhklasdjfglq0kkajfelirhilwhakjgv" },
-            { false, "" },
-            { false, null }
-        });
+            Arguments.of(false, "q"),
+            Arguments.of(true, "q2"),
+            Arguments.of(false, "q2jsalfhjopiwurtiosfhkdhasjkdhfgkfhkfrhnefcn4cqonroclmooi4oiqwhrfq4jrlqhaskdalwehrlwhflhklasdjfglq0kkajfelirhilwhakjgv"),
+            Arguments.of(false, "q2jsalfhjopiwurtiosfhkdhasj!1! !#fhkfrhnefcn4cqonroclmooi4oiqwhrfq4jrlqhaskdalwehrlwhflhklasdjfglq0kkajfelirhilwhakjgv"),
+            Arguments.of(false, ""),
+            Arguments.of(false, null)
+        );
     }
-
-    @Test
-    public void testIsUserNameValid() {
-        assertEquals(isValid, UserNameValidator.isUserNameValid(userName));
+
+    @ParameterizedTest
+    @MethodSource("usernameExamples")
+    public void testUsernames(boolean expected, String username) {
+        assertEquals(expected, UserNameValidator.isUserNameValid(username));
+    }
+
+    /**
+     * This is a simple test class, as we defined the annotation for fields only.
+     */
+    private final class SubjectClass {
+        @ValidateUserName
+        String username;
+
+        SubjectClass(String username) {
+            this.username = username;
+        }
+    }
+
+    private final Validator validator = Validation.buildDefaultValidatorFactory().getValidator();
+
+    @ParameterizedTest
+    @MethodSource("usernameExamples")
+    public void testConstraint(boolean expected, String username) {
+        // given
+        UserNameValidatorTest.SubjectClass sut = new UserNameValidatorTest.SubjectClass(username);
+
+        //when
+        Set<ConstraintViolation<UserNameValidatorTest.SubjectClass>> violations = validator.validate(sut);
+
+        // then
+        assertEquals(expected, violations.size() < 1);
+        // TODO: one might add tests for the two violations (size and illegal chars) here...
     }
+
 }