refactor(validation): simplify ValidateURL IQSS#8531

- Simplify the code for URLValidator
- Make it nullsafe
- Make allowed schemes configurable from annotation
- Rewrite tests to JUnit5, more examples and test with real subject
  classes
- Move message string to validation bundle

src/main/java/ValidationMessages.properties

@@ -44,3 +44,4 @@ guestbook.name=Enter a name for the guestbook
 guestbook.response.nameLength=Please limit response to 255 characters
 
 email.invalid=is not a valid email address.
+url.invalid=is not a valid URL.

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

@@ -10,40 +10,40 @@
  */
 public class URLValidator implements ConstraintValidator<ValidateURL, String> {
 
+    private String[] allowedSchemes;
     @Override
     public void initialize(ValidateURL constraintAnnotation) {
-
+        this.allowedSchemes = constraintAnnotation.schemes();
     }
 
     @Override
     public boolean isValid(String value, ConstraintValidatorContext context) {
-
-        boolean valid = isURLValid(value);
-        if (context != null && !valid) {
-            context.buildConstraintViolationWithTemplate(value + "  " + BundleUtil.getStringFromBundle("url.invalid")).addConstraintViolation();
-        }
-        return valid;
+        return isURLValid(value, this.allowedSchemes);
     }
-
+
+    /**
+     * Check if a URL is valid in a nullsafe way. (null = valid to allow optional values).
+     * Empty values are no valid URLs. This variant allows default schemes HTTP, HTTPS and FTP.
+     *
+     * @param value The URL to validate
+     * @return true when valid (null is also valid) or false
+     */
     public static boolean isURLValid(String value) {
-        if (value == null || value.isEmpty()) {
-            return true;
-        }
-
-        String[] schemes = {"http","https", "ftp"};
+        // default schemes == ValidateURL schemes() default
+        return isURLValid(value, new String[]{"http", "https", "ftp"});
+    }
+
+    /**
+     * Check if a URL is valid in a nullsafe way. (null = valid to allow optional values).
+     * Empty values are no valid URLs. This variant allows any schemes you hand over.
+     *
+     * @param value The URL to validate
+     * @param schemes The list of allowed schemes
+     * @return true when valid (null is also valid) or false
+     */
+    public static boolean isURLValid(String value, String[] schemes) {
         UrlValidator urlValidator = new UrlValidator(schemes);
-
-        try {
-            if (urlValidator.isValid(value)) {
-            } else {
-                return false;
-            }
-        } catch (NullPointerException npe) {
-            return false;
-        }
-
-        return true;
-
+        return value == null || urlValidator.isValid(value);
     }
 
 

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

@@ -14,8 +14,8 @@
 @Constraint(validatedBy = {URLValidator.class})
 @Documented
 public @interface ValidateURL {
-
-    String message() default "Failed Validation for Validate URL";
+    String message() default "'${validatedValue}' {url.invalid}";
+    String[] schemes() default {"http", "https", "ftp"};
 
     Class<?>[] groups() default {};
 

src/main/java/propertyFiles/Bundle.properties

@@ -2292,9 +2292,6 @@ dataset.file.uploadWarning=upload warning
 dataset.file.uploadWorked=upload worked
 dataset.file.upload.popup.explanation.tip=For more information, please refer to the <a href="{0}/{1}/user/dataset-management.html#duplicate-files" title="Duplicate Files - Dataverse User Guide" target="_blank" rel="noopener">Duplicate Files section of the User Guide</a>.
 
-#URLValidator.java
-url.invalid=is not a valid URL.
-
 #HarvestingClientsPage.java
 harvest.start.error=Sorry, harvest could not be started for the selected harvesting client configuration (unknown server error).
 harvest.delete.error=Selected harvesting client cannot be deleted; unknown exception:

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

@@ -1,56 +1,107 @@
-
 package edu.harvard.iq.dataverse.validation;
 
-import static org.junit.Assert.assertEquals;
+import javax.validation.ConstraintViolation;
+import javax.validation.Validation;
+import javax.validation.Validator;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
 
-import javax.validation.ConstraintValidatorContext;
+import java.util.Set;
+import java.util.stream.Stream;
 
-import edu.harvard.iq.dataverse.validation.URLValidator;
-import org.hibernate.validator.internal.engine.constraintvalidation.ConstraintValidatorContextImpl;
-import org.hibernate.validator.internal.engine.path.PathImpl;
-import javax.validation.Validation;
-import javax.validation.ValidatorFactory;
-import org.junit.Test;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 
 /**
  *
  * @author skraffmi
  */
 public class URLValidatorTest {
-    ValidatorFactory validatorFactory = Validation.buildDefaultValidatorFactory();
 
-
-    @Test
-    public void testIsURLValid() {
-        assertEquals(true, URLValidator.isURLValid(null));
-        assertEquals(true, URLValidator.isURLValid(""));
-        assertEquals(true, URLValidator.isURLValid("https://twitter.com/"));
-
-        assertEquals(false, URLValidator.isURLValid("cnn.com"));
-
+    final Validator validator = Validation.buildDefaultValidatorFactory().getValidator();
+
+    public static Stream<Arguments> stdUrlExamples() {
+        return Stream.of(
+            Arguments.of(true, null),
+            Arguments.of(false, ""),
+            Arguments.of(true, "https://twitter.com"),
+            Arguments.of(true, "http://foobar.com:9101"),
+            Arguments.of(true, "ftp://user@foobar.com"),
+            Arguments.of(false, "cnn.com"),
+            Arguments.of(false, "smb://user@foobar.com")
+        );
     }
-
-    @Test
-    public void testIsValidWithUnspecifiedContext() {
-        String value = "https://twitter.com/";
-        ConstraintValidatorContext context = null;
-        assertEquals(true, new URLValidator().isValid(value, context));
+
+    @ParameterizedTest
+    @MethodSource("stdUrlExamples")
+    public void testIsURLValid(boolean expected, String url) {
+        assertEquals(expected, URLValidator.isURLValid(url));
     }
-
-    @Test
-    public void testIsValidWithContextAndValidURL() {
-        String value = "https://twitter.com/";
-        ConstraintValidatorContext context = new ConstraintValidatorContextImpl(validatorFactory.getClockProvider(), PathImpl.createPathFromString(""),null, null);
-
-        assertEquals(true, new URLValidator().isValid(value, context));
+
+    /**
+     * This is a simple test class, as we defined the annotation for fields only.
+     */
+    private final class SubjectClass {
+        @ValidateURL
+        String url;
+
+        SubjectClass(String url) {
+            this.url = url;
+        }
     }
-
-    @Test
-    public void testIsValidWithContextButInvalidURL() {
-        String value = "cnn.com";
-        ConstraintValidatorContext context = new ConstraintValidatorContextImpl(validatorFactory.getClockProvider(), PathImpl.createPathFromString(""),null, null);
-
-        assertEquals(false, new URLValidator().isValid(value, context));
+
+    @ParameterizedTest
+    @MethodSource("stdUrlExamples")
+    public void testConstraint(boolean expected, String url) {
+        // given
+        URLValidatorTest.SubjectClass sut = new URLValidatorTest.SubjectClass(url);
+
+        //when
+        Set<ConstraintViolation<URLValidatorTest.SubjectClass>> violations = validator.validate(sut);
+
+        // then
+        assertEquals(expected ? 0 : 1, violations.size());
+        violations.stream().findFirst().ifPresent( c -> {
+            assertTrue(c.getMessage().contains(url)); });
+    }
+
+    public static Stream<Arguments> fancyUrlExamples() {
+        return Stream.of(
+            Arguments.of(true, null),
+            Arguments.of(false, ""),
+            Arguments.of(false, "https://twitter.com"),
+            Arguments.of(true, "http://foobar.com:9101"),
+            Arguments.of(false, "ftp://user@foobar.com"),
+            Arguments.of(false, "cnn.com"),
+            Arguments.of(true, "smb://user@foobar.com")
+        );
+    }
+
+    /**
+     * This is a simple test class like above, but with a scheme given
+     */
+    private final class SubjectSchemeClass {
+        @ValidateURL(schemes = {"http", "smb"})
+        String url;
+
+        SubjectSchemeClass(String url) {
+            this.url = url;
+        }
+    }
+
+    @ParameterizedTest
+    @MethodSource("fancyUrlExamples")
+    public void testConstraintWithSchemes(boolean expected, String url) {
+        // given
+        URLValidatorTest.SubjectSchemeClass sut = new URLValidatorTest.SubjectSchemeClass(url);
+
+        //when
+        Set<ConstraintViolation<URLValidatorTest.SubjectSchemeClass>> violations = validator.validate(sut);
+
+        // then
+        assertEquals(expected ? 0 : 1, violations.size());
+        violations.stream().findFirst().ifPresent( c -> {
+            assertTrue(c.getMessage().contains(url)); });
     }
-
 }