refactor(validation): cleanup DatasetFieldValueValidator URL validation

IQSS#8531
poikilotherm · Mar 24, 2022 · 9589d3f · 9589d3f
1 parent 85dcbbf
commit 9589d3f
Show file tree

Hide file tree

Showing 2 changed files with 38 additions and 41 deletions.
diff --git a/src/main/java/edu/harvard/iq/dataverse/DatasetFieldValueValidator.java b/src/main/java/edu/harvard/iq/dataverse/DatasetFieldValueValidator.java
@@ -17,6 +17,7 @@
 import javax.validation.ConstraintValidatorContext;
 
 import edu.harvard.iq.dataverse.validation.EMailValidator;
+import edu.harvard.iq.dataverse.validation.URLValidator;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.commons.validator.routines.UrlValidator;
 
@@ -167,20 +168,11 @@ public boolean isValid(DatasetFieldValue value, ConstraintValidatorContext conte
         // Note, length validation for FieldType.TEXT was removed to accommodate migrated data that is greater than 255 chars.
 
         if (fieldType.equals(FieldType.URL) && !lengthOnly) {
-
-            String[] schemes = {"http","https", "ftp"};
-            UrlValidator urlValidator = new UrlValidator(schemes);
-
-            try {
-                if (urlValidator.isValid(value.getValue())) {
-                } else {
-                    context.buildConstraintViolationWithTemplate(dsfType.getDisplayName() + " " + value.getValue() + "  is not a valid URL.").addConstraintViolation();
-                    return false;
-                }
-            } catch (NullPointerException npe) {
+            boolean isValidUrl = URLValidator.isURLValid(value.getValue());
+            if (!isValidUrl) {
+                context.buildConstraintViolationWithTemplate(dsfType.getDisplayName() + " " + value.getValue() + "  {url.invalid}").addConstraintViolation();
                 return false;
             }
-
         }
 
         if (fieldType.equals(FieldType.EMAIL) && !lengthOnly) {

diff --git a/src/test/java/edu/harvard/iq/dataverse/DatasetFieldValueValidatorTest.java b/src/test/java/edu/harvard/iq/dataverse/DatasetFieldValueValidatorTest.java
@@ -13,6 +13,8 @@
 import javax.validation.Validator;
 
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
 import org.mockito.Mockito;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
@@ -107,33 +109,6 @@ public void testIsValid() {
         value.setValue("12.14");
         result = instance.isValid(value, ctx);
         assertEquals(false, result);
-
-        //URL
-        dft.setFieldType(DatasetFieldType.FieldType.URL); 
-        value.setValue("https://www.google.com");
-        result = instance.isValid(value, ctx);
-        assertEquals(true, result);
-
-        value.setValue("http://google.com");
-        result = instance.isValid(value, ctx);
-        assertEquals(true, result);
-
-        value.setValue("https://do-not-exist-123-123.com/"); // does not exist
-        result = instance.isValid(value, ctx);
-        assertEquals(true, result);   
-
-        value.setValue("ftp://somesite.com");
-        result = instance.isValid(value, ctx);
-        assertEquals(true, result);   
-
-        value.setValue("google.com");
-        result = instance.isValid(value, ctx);
-        assertEquals(false, result);
-
-        value.setValue("[email protected]:IQSS/dataverse.git");
-        result = instance.isValid(value, ctx);
-        assertEquals(false, result);
-
     }
 
     @Test
@@ -184,6 +159,38 @@ public void testIsValidAuthorIdentifierGnd() {
 
     final Validator validator = Validation.buildDefaultValidatorFactory().getValidator();
 
+    @ParameterizedTest
+    @CsvSource(
+        {
+            "true, https://www.google.com",
+            "true, http://google.com",
+            "true, https://do-not-exist-123-123.com/",
+            "true, ftp://somesite.com",
+            "false, google.com",
+            "false, [email protected]:IQSS/dataverse.git"
+        }
+    )
+    public void testInvalidURL(boolean expected, String url) {
+        // given
+        String fieldName = "testField";
+
+        DatasetField field = new DatasetField();
+        field.setDatasetFieldType(new DatasetFieldType(fieldName, DatasetFieldType.FieldType.URL, false));
+        DatasetFieldValue sut = new DatasetFieldValue(field);
+        sut.setValue(url);
+
+        // when
+        Set<ConstraintViolation<DatasetFieldValue>> violations = validator.validate(sut);
+
+        // then
+        assertEquals(expected, violations.size() < 1);
+        violations.stream().findFirst().ifPresent(c -> {
+            assertTrue(c.getMessage().startsWith(fieldName + " " + url + " "));
+            assertTrue(c.getMessage().contains("not"));
+            assertTrue(c.getMessage().contains("URL"));
+        });
+    }
+
     @Test
     public void testInvalidEmail() {
         // given
@@ -205,7 +212,5 @@ public void testInvalidEmail() {
             assertTrue(c.getMessage().contains("not"));
             assertTrue(c.getMessage().contains("email"));
         });
-
     }
-
 }