From 72312523bc6570c1405dd3832e913030207c4b0c Mon Sep 17 00:00:00 2001 From: Thomas Turrell-Croft Date: Thu, 30 Mar 2023 21:22:18 +0100 Subject: [PATCH] LocaleValidator should handle any invalid locale --- .../internal/validators/LocaleValidator.java | 10 ++-- .../learning/xapi/model/ActivityTests.java | 30 ++++++++++- .../validators/LocaleValidatorTests.java | 53 ++++++++++--------- 3 files changed, 62 insertions(+), 31 deletions(-) diff --git a/xapi-model/src/main/java/dev/learning/xapi/model/validation/internal/validators/LocaleValidator.java b/xapi-model/src/main/java/dev/learning/xapi/model/validation/internal/validators/LocaleValidator.java index 85b5dcd0..e371fcd3 100644 --- a/xapi-model/src/main/java/dev/learning/xapi/model/validation/internal/validators/LocaleValidator.java +++ b/xapi-model/src/main/java/dev/learning/xapi/model/validation/internal/validators/LocaleValidator.java @@ -12,6 +12,11 @@ /** * The Locale being validated must have a ISO3 Language and Country. + *

+ * There is no way to reliably test a locale that was instantiated with + * {@link Locale.forLanguageTag}. {@link Locale.forLanguageTag} treats most invalid locales as + * undetermined (und). + *

* * @author István Rátkai (Selindek) * @author Thomas Turrell-Croft @@ -36,10 +41,7 @@ public boolean isValid(Locale locale, ConstraintValidatorContext context) { final var blar = Locale.forLanguageTag(locale.toString()); try { - blar.getISO3Language(); - blar.getISO3Country(); - - return true; + return !blar.getISO3Language().equals(""); } catch (final MissingResourceException e2) { return false; diff --git a/xapi-model/src/test/java/dev/learning/xapi/model/ActivityTests.java b/xapi-model/src/test/java/dev/learning/xapi/model/ActivityTests.java index b1a97760..2482c34a 100644 --- a/xapi-model/src/test/java/dev/learning/xapi/model/ActivityTests.java +++ b/xapi-model/src/test/java/dev/learning/xapi/model/ActivityTests.java @@ -5,13 +5,18 @@ package dev.learning.xapi.model; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; import com.fasterxml.jackson.databind.ObjectMapper; +import jakarta.validation.ConstraintViolation; +import jakarta.validation.Validation; +import jakarta.validation.Validator; import java.io.IOException; import java.net.URI; import java.util.Locale; +import java.util.Set; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import org.springframework.util.ResourceUtils; @@ -27,6 +32,8 @@ class ActivityTests { private final ObjectMapper objectMapper = new ObjectMapper().findAndRegisterModules(); + private final Validator validator = Validation.buildDefaultValidatorFactory().getValidator(); + @Test void whenDeserializingActivityThenResultIsInstanceOfActivity() throws Exception { @@ -138,12 +145,31 @@ void whenActivityIsConstructedWithIdThenIdIsExpected() { void whenActivityIsBuiltWithStringIdThenIdIsExpected() { // When Activity Is Built With String Id - final var activity = - Activity.builder().id("http://www.example.co.uk/exampleactivity").build(); + final var activity = Activity.builder().id("http://www.example.co.uk/exampleactivity").build(); // Then Id Is Expected assertThat(activity.getId(), is(URI.create("http://www.example.co.uk/exampleactivity"))); } + @Test + void whenValidatingActivityWithInvalidDisplayThenConstraintViolationsSizeIsOne() + throws Exception { + + // This test uses Jackson to read the JSON because we need to test that Jackson does not use + // Locale.forLanguageTag. Currently forLanguageTag is very permissive and will treat most + // invalid strings as und. + final var json = + "{\"objectType\":\"Activity\",\"id\":\"https://example.com/activity/simplestatement\",\"definition\":{\"name\":{\"a12345678\":\"Simple Statement\"}}}"; + + final var activity = objectMapper.readValue(json, Activity.class); + + // When Validating Activity With Invalid Display + final Set> constraintViolations = validator.validate(activity); + + // Then ConstraintViolations Size Is One + assertThat(constraintViolations, hasSize(1)); + + } + } diff --git a/xapi-model/src/test/java/dev/learning/xapi/model/validation/internal/validators/LocaleValidatorTests.java b/xapi-model/src/test/java/dev/learning/xapi/model/validation/internal/validators/LocaleValidatorTests.java index 31dc80a7..0ba53d65 100644 --- a/xapi-model/src/test/java/dev/learning/xapi/model/validation/internal/validators/LocaleValidatorTests.java +++ b/xapi-model/src/test/java/dev/learning/xapi/model/validation/internal/validators/LocaleValidatorTests.java @@ -34,9 +34,9 @@ void whenValueIsNullThenResultIsTrue() { } @Test - void whenCallingIsValidOnLocaleWithUKKeyThenResultIsTrue() { + void whenCallingIsValidOnUKLocaleThenResultIsTrue() { - // When Calling Is Valid On Locale With UK Key + // When Calling Is Valid On UK Locale final var result = validator.isValid(Locale.UK, null); // Then Result Is True @@ -44,9 +44,9 @@ void whenCallingIsValidOnLocaleWithUKKeyThenResultIsTrue() { } @Test - void whenCallingIsValidOnLocaleWithENKeyThenResultIsTrue() { + void whenCallingIsValidOnENLocaleThenResultIsTrue() { - // When Calling Is Valid On Locale With EN Key + // When Calling Is Valid On EN Locale final var result = validator.isValid(Locale.ENGLISH, null); // Then Result Is True @@ -54,51 +54,54 @@ void whenCallingIsValidOnLocaleWithENKeyThenResultIsTrue() { } @Test - void whenCallingIsValidOnLocaleWithENAndUKKeysThenResultIsTrue() { + void whenCallingIsValidOnUSLocaleThenResultIsTrue() { - // When Calling Is Valid On Locale With EN And UK Keys - final var result = validator.isValid(Locale.UK, null); + // When Calling Is Valid On US Locale + final var result = validator.isValid(Locale.US, null); // Then Result Is True assertTrue(result); } - @Test - void whenCallingIsValidOnLocaleWithENAndUnknownKeysThenResultIsFalse() { + @ParameterizedTest + @ValueSource(strings = {"und", "zh-CHS", "zh-CN", "zh-Hans", "zh-Hant", "zh-HK"}) + void whenCallingIsValidOnLocaleWithValidLanguageTagThenResultIsTrue(String arg) { - // When Calling Is Valid On Locale With EN And Unknown Keys - final var result = validator.isValid(Locale.forLanguageTag("unknown"), null); + // When Calling Is Valid On Locale With Valid Language Tag + final var result = validator.isValid(Locale.forLanguageTag(arg), null); - // Then Result Is False - assertFalse(result); + // Then Result Is True + assertTrue(result); } - @Test - void whenCallingIsValidOnLocaleWithChineseSimplifiedKeyUsingForLangugeTagThenResultIsTrue() { + @ParameterizedTest + @ValueSource(strings = {"unknown"}) + void whenCallingIsValidOnLocaleWithInvalidLanguageTagThenResultIsFalse(String arg) { - // When Calling Is Valid On Locale With Chinese Simplified Key - final var result = validator.isValid(Locale.forLanguageTag("zh-CHS"), null); + // When Calling Is Valid On Locale With Invalid Language Tag + final var result = validator.isValid(Locale.forLanguageTag(arg), null); - // Then Result Is True - assertTrue(result); + // Then Result Is False + assertFalse(result); } @ParameterizedTest @ValueSource(strings = {"und", "zh-CHS", "zh-CN", "zh-Hans", "zh-Hant", "zh-HK"}) - void whenCallingIsValidOnLocaleWithValidKeyThenResultIsTrue(String arg) { + void whenCallingIsValidOnLocaleWithValidLanguageThenResultIsTrue(String arg) { - // When Calling Is Valid On Locale With Valid Key + // when Calling Is Valid On Locale With Valid Language final var result = validator.isValid(new Locale(arg), null); // Then Result Is True assertTrue(result); } - @Test - void whenCallingIsValidOnLocaleWithUnknownKeyThenResultIsFalse() { + @ParameterizedTest + @ValueSource(strings = {"unknown", "a12345678", "123456789", "12345678", "1234567"}) + void whenCallingIsValidOnLocaleWithInvalidLanguageThenResultIsFalse(String arg) { - // When Calling Is Valid On Locale With Unknown Key - final var result = validator.isValid(new Locale("unknown"), null); + // When Calling Is Valid On Locale With Invalid Language + final var result = validator.isValid(new Locale(arg), null); // Then Result Is False assertFalse(result);