diff --git a/src/main/java/edu/harvard/iq/dataverse/DatasetFieldValueValidator.java b/src/main/java/edu/harvard/iq/dataverse/DatasetFieldValueValidator.java index 7cd823211bf..8b807f78bca 100644 --- 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 index a637848851e..ceaa69ade4e 100644 --- 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("git@github.com: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, git@github.com: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> 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")); }); - } - }