From c142db995e1619c9e8571987c49af8daeddffb76 Mon Sep 17 00:00:00 2001 From: Scott Leberknight <174812+sleberknight@users.noreply.github.com> Date: Sat, 27 Jul 2024 15:50:45 -0400 Subject: [PATCH 1/2] Add new KiwiConstraintViolations utilities * Add asMap, asSingleValuedMap, asMultiValuedMap, and asMultimap methods to convert a Set of ConstraintViolation into a JDK or Guava Multimap. Overloads provide the ability to customize the translation from a property Path to the map key. * Add pathStringOf, a pure convenience method to eliminate boilerplate. * Clean up a few minor grammatical errors in javadoc. Closes #1169 Closes #1170 --- .../validation/KiwiConstraintViolations.java | 209 ++++++++++- .../KiwiConstraintViolationsTest.java | 335 +++++++++++++++++- 2 files changed, 533 insertions(+), 11 deletions(-) diff --git a/src/main/java/org/kiwiproject/validation/KiwiConstraintViolations.java b/src/main/java/org/kiwiproject/validation/KiwiConstraintViolations.java index 79a83198..92b86280 100644 --- a/src/main/java/org/kiwiproject/validation/KiwiConstraintViolations.java +++ b/src/main/java/org/kiwiproject/validation/KiwiConstraintViolations.java @@ -1,12 +1,18 @@ package org.kiwiproject.validation; import static com.google.common.base.Preconditions.checkArgument; +import static java.util.stream.Collectors.groupingBy; import static java.util.stream.Collectors.joining; +import static java.util.stream.Collectors.toMap; +import static java.util.stream.Collectors.toSet; import static java.util.stream.Collectors.toUnmodifiableMap; import static org.kiwiproject.base.KiwiPreconditions.checkArgumentNotNull; import static org.kiwiproject.collect.KiwiSets.isNotNullOrEmpty; import static org.kiwiproject.collect.KiwiSets.isNullOrEmpty; +import static org.kiwiproject.stream.KiwiMultimapCollectors.toLinkedHashMultimap; +import com.google.common.collect.Maps; +import com.google.common.collect.Multimap; import jakarta.validation.ConstraintViolation; import jakarta.validation.Path; import lombok.experimental.UtilityClass; @@ -37,6 +43,205 @@ @UtilityClass public class KiwiConstraintViolations { + /** + * Convert the set of {@link ConstraintViolation} to a map keyed by the property path. + *

+ * The map's values are the single {@link ConstraintViolation} associated with each property. + *

+ * WARNING: + * An {@link IllegalStateException} is thrown if there is more than one violation associated + * with any key. Therefore, this method should only be used if you are sure there can only + * be at most one violation per property. Otherwise, use either {@link #asMultiValuedMap(Set)} + * or {@link #asSingleValuedMap(Set)}. + * + * @param violations set of non-null but possibly empty violations + * @param the type of the root bean that was validated + * @return a map whose keys are the property path of the violations, and values are the violations + * @throws IllegalStateException if there is more than one violation associated with any key + * @see #asSingleValuedMap(Set) + * @see #asMultiValuedMap(Set) + */ + public static Map> asMap(Set> violations) { + return asMap(violations, Path::toString); + } + + /** + * Convert the set of {@link ConstraintViolation} to a map keyed by the property path. + * The property path is determined by the {@code pathTransformer}. + *

+ * The map's values are the single {@link ConstraintViolation} associated with each property. + *

+ * WARNING: + * An {@link IllegalStateException} is thrown if there is more than one violation associated + * with any key. Therefore, this method should only be used if you are sure there can only + * be at most one violation per property. Otherwise, use either {@link #asMultiValuedMap(Set)} + * or {@link #asSingleValuedMap(Set)}. + * + * @param violations set of non-null but possibly empty violations + * @param pathTransformer function to convert a Path into a String + * @param the type of the root bean that was validated + * @return a map whose keys are the property path of the violations, and values are the violations + * @throws IllegalStateException if there is more than one violation associated with any key + * @see #asSingleValuedMap(Set) + * @see #asMultiValuedMap(Set) + */ + public static Map> asMap(Set> violations, + Function pathTransformer) { + return violations.stream().collect(toMap( + violation -> pathTransformer.apply(violation.getPropertyPath()), + violation -> violation)); + } + + /** + * Convert the set of {@link ConstraintViolation} to a map keyed by the property path. + *

+ * The map's values are the last {@link ConstraintViolation} associated with each property. + * The definition of "last" depends on the iteration order of the provided set of violations, which + * may be non-deterministic if the set does not have a well-defined traversal order. + *

+ * WARNING: + * If there is more than one violation associated with any key, the last violation, as + * determined by the set traversal order, becomes they key. If you need to retain all violations + * associated with each key, use {@link #asMultiValuedMap(Set)}. + * + * @param violations set of non-null but possibly empty violations + * @param the type of the root bean that was validated + * @return a map whose keys are the property path of the violations, and values are the violations + * @see #asMultiValuedMap(Set) + */ + public static Map> asSingleValuedMap(Set> violations) { + return asSingleValuedMap(violations, Path::toString); + } + + /** + * Convert the set of {@link ConstraintViolation} to a map keyed by the property path. + * The property path is determined by the {@code pathTransformer}. + *

+ * The map's values are the last {@link ConstraintViolation} associated with each property. + * The definition of "last" depends on the iteration order of the provided set of violations, which + * may be non-deterministic if the set does not have a well-defined traversal order. + *

+ * WARNING: + * If there is more than one violation associated with any key, the last violation, as + * determined by the set traversal order, becomes they key. If you need to retain all violations + * associated with each key, use {@link #asMultiValuedMap(Set)}. + * + * @param violations set of non-null but possibly empty violations + * @param pathTransformer function to convert a Path into a String + * @param the type of the root bean that was validated + * @return a map whose keys are the property path of the violations, and values are the violations + * @see #asMultiValuedMap(Set) + */ + public static Map> asSingleValuedMap(Set> violations, + Function pathTransformer) { + return violations.stream().collect(toMap( + violation -> pathTransformer.apply(violation.getPropertyPath()), + violation -> violation, + (violation1, violation2) -> violation2)); + } + + /** + * Convert the set of {@link ConstraintViolation} to a map keyed by the property path. + *

+ * The map's values are the set of {@link ConstraintViolation} associated with each property. + * + * @param violations set of non-null but possibly empty violations + * @param the type of the root bean that was validated + * @return a map whose keys are the property path of the violations, and values are a Set containing + * violations for the corresponding property + */ + public static Map>> asMultiValuedMap(Set> violations) { + return asMultiValuedMap(violations, Path::toString); + } + + /** + * Convert the set of {@link ConstraintViolation} to a map keyed by the property path. + * The property path is determined by the {@code pathTransformer}. + *

+ * The map's values are the set of {@link ConstraintViolation} associated with each property. + * + * @param violations set of non-null but possibly empty violations + * @param pathTransformer function to convert a Path into a String + * @param the type of the root bean that was validated + * @return a map whose keys are the property path of the violations, and values are a Set containing + * violations for the corresponding property + */ + public static Map>> asMultiValuedMap(Set> violations, + Function pathTransformer) { + return violations.stream().collect( + groupingBy(violation -> pathTransformer.apply(violation.getPropertyPath()), toSet())); + } + + /** + * Convert the set of {@link ConstraintViolation} to a {@link Multimap} keyed by the property path. + * + * @param violations set of non-null but possibly empty violations + * @param the type of the root bean that was validated + * @return a {@link Multimap} whose keys are the property path of the violations, and values contain + * the violations for the corresponding property + * @implNote The returned value is a {@link com.google.common.collect.LinkedHashMultimap}; the iteration + * order of the values for each key is always the order in which the values were added, and there + * cannot be duplicate values for a key. + */ + public static Multimap> asMultimap(Set> violations) { + return asMultimap(violations, Path::toString); + } + + /** + * Convert the set of {@link ConstraintViolation} to a {@link Multimap} keyed by the property path. + * + * @param violations set of non-null but possibly empty violations + * @param pathTransformer function to convert a Path into a String + * @param the type of the root bean that was validated + * @return a {@link Multimap} whose keys are the property path of the violations, and values contain + * the violations for the corresponding property + * @implNote The returned value is a {@link com.google.common.collect.LinkedHashMultimap}; the iteration + * order of the values for each key is always the order in which the values were added, and there + * cannot be duplicate values for a key. + */ + public static Multimap> asMultimap(Set> violations, + Function pathTransformer) { + return violations.stream() + .map(violation -> Maps.immutableEntry(pathTransformer.apply(violation.getPropertyPath()), violation)) + .collect(toLinkedHashMultimap()); + } + + /** + * Convenience method to get the property path of the {@link ConstraintViolation} as a String. + *

+ * Please refer to the Implementation Note for details on the structure of the returned values + * and warnings about that structure. + * + * @param violation the constraint violation + * @param the type of the root bean that was validated + * @return the property path of the violation, as a String + * @implNote This uses {@link ConstraintViolation#getPropertyPath()} to obtain a {@link Path} + * and then calls {@link Path#toString()} to get the final value. Therefore, the issues on + * {@link Path#toString()} with regard to the structure of the return value apply here as well. + * However, in many years of usage, the implementation (in Hibernate Validator anyway) has + * always returned the same expected result, and is generally what you expect. + *

+ * The main exception is iterable types, such as Set, that don't have a consistent traversal + * order. For example, if you have a property named "nicknames" declared as + * {@code Set<@NotBlank String> nicknames}, the property path for violation errors + * look like {@code "nicknames[]."}. + *

+ * Maps look similar to Sets. For example, in the Hibernate Validator reference + * documentation, one example shows the property path of a constraint violation + * on a Map as {@code "fuelConsumption[HIGHWAY]."}, and similarly on + * a Map value as {@code "fuelConsumption[]."}. + *

+ * Indexed properties such as a List look more reasonable. For example, suppose a property + * named "passwordHints" is declared as {@code List<@NotNull @Valid Hint> passwordHints}, + * and that {@code Hint} contains a String property named {@code text}. The property + * path for violation errors includes the zero-based index as well as the path. For + * example, if the second password hint is not valid, the property path is + * {@code passwordHints[1].text}. + */ + public static String pathStringOf(ConstraintViolation violation) { + return violation.getPropertyPath().toString(); + } + /** * Given a non-empty set of violations, produce a single string containing all violation messages * separated by commas. If the given set is empty (or null), then throw IllegalArgumentException. @@ -192,7 +397,7 @@ public static List simpleCombinedErrorMessages(Set - * For example contactInfo.email.address using ":" as the path separator would result in Contact Info:Email:Address. + * For example, contactInfo.email.address using ":" as the path separator would result in Contact Info:Email:Address. * * @param propertyPath the property path from a {@link ConstraintViolation} * @param pathSeparator the separator to use between path elements diff --git a/src/test/java/org/kiwiproject/validation/KiwiConstraintViolationsTest.java b/src/test/java/org/kiwiproject/validation/KiwiConstraintViolationsTest.java index a28187c3..122eed85 100644 --- a/src/test/java/org/kiwiproject/validation/KiwiConstraintViolationsTest.java +++ b/src/test/java/org/kiwiproject/validation/KiwiConstraintViolationsTest.java @@ -1,11 +1,16 @@ package org.kiwiproject.validation; +import static java.util.Comparator.comparing; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; +import static org.assertj.core.api.Assertions.assertThatIllegalStateException; import static org.assertj.core.api.Assertions.entry; +import static org.junit.jupiter.api.Assertions.assertAll; import static org.kiwiproject.base.KiwiStrings.f; import static org.kiwiproject.collect.KiwiLists.first; import static org.kiwiproject.collect.KiwiLists.second; +import static org.kiwiproject.collect.KiwiLists.third; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -16,11 +21,13 @@ import jakarta.validation.constraints.Email; import jakarta.validation.constraints.Max; import jakarta.validation.constraints.Min; +import jakarta.validation.constraints.NotBlank; import jakarta.validation.constraints.NotEmpty; import jakarta.validation.constraints.NotNull; import jakarta.validation.constraints.Pattern; import lombok.AllArgsConstructor; import lombok.Getter; +import org.assertj.guava.api.Assertions; import org.hibernate.validator.constraints.Length; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; @@ -31,6 +38,7 @@ import org.junit.jupiter.params.provider.NullAndEmptySource; import java.time.LocalDate; +import java.util.List; import java.util.Objects; import java.util.Set; import java.util.function.Function; @@ -490,7 +498,7 @@ void shouldBuildMap() { entry("Contact Info // Email // Address", emailViolation.getMessage()) ); - // We cannot rely on a deterministic violation order (since it is a Set), so just check + // We cannot rely on a deterministic violation order (since it is a Set), so check // that the message contains both expected messages and that there is a comma separator var fullNameErrorMessage = errorMessages.get("Full Name"); assertThat(fullNameErrorMessage) @@ -500,6 +508,314 @@ void shouldBuildMap() { } } + @Nested + class AsMap { + + @Test + void shouldReturnEmptyMap_WhenGivenEmptySet() { + assertThat(KiwiConstraintViolations.asMap(Set.of())).isEmpty(); + } + + @Test + void shouldThrowIllegalState_WhenMoreThanOneViolation_ForSomeProperty() { + var person = new Person("@", 42, null, new ContactInfo(new EmailAddress("bob"))); + var violations = validator.validate(person); + + assertThatIllegalStateException() + .isThrownBy(() -> KiwiConstraintViolations.asMap(violations)); + } + + @Test + void shouldCollectViolationsIntoMapKeyedByPropertyPath() { + var person = new Person("X", 42, null, new ContactInfo(new EmailAddress("bob"))); + var violations = validator.validate(person); + + var violationMap = KiwiConstraintViolations.asMap(violations); + + assertAll( + () -> assertThat(violationMap).containsOnlyKeys("fullName", "birthDate", "contactInfo.email.address"), + + () -> assertThat(violationMap).extractingByKey("fullName") + .extracting("message") + .isEqualTo("length must be between 2 and 2147483647"), + + () -> assertThat(violationMap).extractingByKey("birthDate") + .extracting("message") + .isEqualTo("must not be null"), + + () -> assertThat(violationMap).extractingByKey("contactInfo.email.address") + .extracting("message") + .isEqualTo("must be a well-formed email address") + ); + } + + @Test + void shouldCollectViolationsIntoMap_UsingCustomPathTransformerFunction() { + var person = new Person("X", 42, null, new ContactInfo(new EmailAddress("bob"))); + var violations = validator.validate(person); + + var violationMap = KiwiConstraintViolations.asMap(violations, KiwiConstraintViolations::humanize); + + assertThat(violationMap).containsOnlyKeys("Full Name", "Birth Date", "Contact Info / Email / Address"); + } + } + + @Nested + class AsSingleValuedMap { + + @Test + void shouldReturnEmptyMap_WhenGivenEmptySet() { + assertThat(KiwiConstraintViolations.asSingleValuedMap(Set.of())).isEmpty(); + } + + @Test + void shouldNotThrowIllegalState_WhenMoreThanOneViolation_ForSomeProperty() { + var person = new Person("@", 42, null, new ContactInfo(new EmailAddress("bob"))); + var violations = validator.validate(person); + + assertThatCode(() -> KiwiConstraintViolations.asSingleValuedMap(violations)) + .doesNotThrowAnyException(); + } + + @Test + void shouldCollectViolationsIntoMapKeyedByPropertyPath() { + var person = new Person("!", 42, null, new ContactInfo(new EmailAddress("bob"))); + var violations = validator.validate(person); + + var violationMap = KiwiConstraintViolations.asSingleValuedMap(violations); + + assertAll( + () -> assertThat(violationMap).containsOnlyKeys("fullName", "birthDate", "contactInfo.email.address"), + + () -> assertThat(violationMap).extractingByKey("fullName") + .extracting("message") + .describedAs("Order is non-deterministic, so must be one of the messages") + .isIn("must not be empty", + "length must be between 2 and 2147483647", + "must include only alphabetic characters (upper or lower case)"), + + () -> assertThat(violationMap).extractingByKey("birthDate") + .extracting("message") + .isEqualTo("must not be null"), + + () -> assertThat(violationMap).extractingByKey("contactInfo.email.address") + .extracting("message") + .isEqualTo("must be a well-formed email address") + ); + } + + @Test + void shouldCollectViolationsIntoMap_UsingCustomPathTransformerFunction() { + var person = new Person("!", 42, null, new ContactInfo(new EmailAddress("bob"))); + var violations = validator.validate(person); + + var violationMap = KiwiConstraintViolations.asSingleValuedMap(violations, KiwiConstraintViolations::humanize); + + assertThat(violationMap).containsOnlyKeys("Full Name", "Birth Date", "Contact Info / Email / Address"); + } + } + + @Nested + class AsMultiValuedMap { + + @Test + void shouldReturnEmptyMap_WhenGivenEmptySet() { + assertThat(KiwiConstraintViolations.asMultiValuedMap(Set.of())).isEmpty(); + } + + @Test + void shouldCollectViolationsIntoMapKeyedByPropertyPath() { + var person = new Person("!", 42, null, new ContactInfo(new EmailAddress("bob"))); + var violations = validator.validate(person); + + var violationMap = KiwiConstraintViolations.asMultiValuedMap(violations); + + assertAll( + () -> assertThat(violationMap).containsOnlyKeys("fullName", "birthDate", "contactInfo.email.address"), + + () -> assertThat(violationMap.get("fullName")) + .hasSize(2) + .extracting("message") + .contains( + "length must be between 2 and 2147483647", + "must include only alphabetic characters (upper or lower case)"), + + () -> assertThat(violationMap.get("birthDate")) + .hasSize(1) + .extracting("message") + .contains("must not be null"), + + () -> assertThat(violationMap.get("contactInfo.email.address")) + .hasSize(1) + .extracting("message") + .contains("must be a well-formed email address") + ); + } + + @Test + void shouldCollectViolationsIntoMap_UsingCustomPathTransformerFunction() { + var person = new Person("!", 42, null, new ContactInfo(new EmailAddress("bob"))); + var violations = validator.validate(person); + + var violationMap = KiwiConstraintViolations.asMultiValuedMap(violations, KiwiConstraintViolations::humanize); + + assertThat(violationMap).containsOnlyKeys("Full Name", "Birth Date", "Contact Info / Email / Address"); + } + } + + @Nested + class AsMultimap { + + @Test + void shouldReturnEmptyMap_WhenGivenEmptySet() { + assertThat(KiwiConstraintViolations.asMultimap(Set.of()).size()).isZero(); + } + + @Test + void shouldCollectViolationsIntoMultimapKeyedByPropertyPath() { + var person = new Person("!", 42, null, new ContactInfo(new EmailAddress("bob"))); + var violations = validator.validate(person); + + var violationMultimap = KiwiConstraintViolations.asMultimap(violations); + + assertAll( + () -> Assertions.assertThat(violationMultimap) + .containsKeys("fullName", "birthDate", "contactInfo.email.address"), + + () -> assertThat(violationMultimap.get("fullName")) + .extracting("message") + .hasSize(2) + .contains( + "length must be between 2 and 2147483647", + "must include only alphabetic characters (upper or lower case)"), + + () -> assertThat(violationMultimap.get("birthDate")) + .extracting("message") + .hasSize(1) + .contains("must not be null"), + + () -> assertThat(violationMultimap.get("contactInfo.email.address")) + .extracting("message") + .hasSize(1) + .contains("must be a well-formed email address") + ); + } + + @Test + void shouldCollectViolationsIntoMultimap_UsingCustomPathTransformerFunction() { + var person = new Person("!", 42, null, new ContactInfo(new EmailAddress("bob"))); + var violations = validator.validate(person); + + var violationMultimap = KiwiConstraintViolations.asMultimap(violations, KiwiConstraintViolations::humanize); + + Assertions.assertThat(violationMultimap) + .containsKeys("Full Name", "Birth Date", "Contact Info / Email / Address"); + } + } + + @Nested + class PathStringOf { + + @Test + void shouldGetPathForSimpleProperties() { + assertAll( + () -> assertPersonPropertyPathString("fullName", "X"), + () -> assertPersonPropertyPathString("age", -1), + () -> assertPersonPropertyPathString("age", 151), + () -> assertPersonPropertyPathString("birthDate", null), + () -> assertPersonPropertyPathString("contactInfo.email", null), + () -> assertPersonPropertyPathString("contactInfo.email.address", null), + () -> assertPersonPropertyPathString("contactInfo.email.address", "") + ); + } + + private void assertPersonPropertyPathString(String propertyName, Object value) { + var violations = validator.validateValue(Person.class, propertyName, value); + assertThat(violations) + .describedAs("Expected only one violation on %s but found %d", propertyName, violations.size()) + .hasSize(1); + + var violation = violations.iterator().next(); + assertThat(KiwiConstraintViolations.pathStringOf(violation)).isEqualTo(propertyName); + } + + @Test + void shouldGetPathForIndexedProperties() { + var passwordHints = List.of(new Hint(""), new Hint(null)); + var nicknames = Set.of("bobby", "booby jo"); + var user = new User("bob_jones", "monkey-123-456", passwordHints, nicknames); + var violations = validateAndFilterByPropertyPathContains(user, "passwordHints", 2); + + assertAll( + () -> assertThat(KiwiConstraintViolations.pathStringOf(first(violations))) + .isEqualTo("passwordHints[0].text"), + + () -> assertThat(KiwiConstraintViolations.pathStringOf(second(violations))) + .isEqualTo("passwordHints[1].text") + ); + } + + @Test + void shouldGetPathForIterableProperties() { + var passwordHints = List.of(new Hint("bananas")); + var nicknames = Set.of("", " ", " "); + var user = new User("bob_jones", "monkey-123-456", passwordHints, nicknames); + var violations = validateAndFilterByPropertyPathContains(user, "nicknames", 3); + + var expectedPropertyPath = "nicknames[]."; + assertAll( + () -> assertThat(KiwiConstraintViolations.pathStringOf(first(violations))) + .isEqualTo(expectedPropertyPath), + + () -> assertThat(KiwiConstraintViolations.pathStringOf(second(violations))) + .isEqualTo(expectedPropertyPath), + + () -> assertThat(KiwiConstraintViolations.pathStringOf(third(violations))) + .isEqualTo(expectedPropertyPath) + ); + } + + private List> validateAndFilterByPropertyPathContains( + User user, + String pathSubstring, + int expectedViolations) { + + var violations = validator.validate(user).stream() + .filter(violation -> { + var path = violation.getPropertyPath().toString(); + return path.contains(pathSubstring); + }) + .sorted(comparing(violation -> violation.getPropertyPath().toString())) + .toList(); + + assertThat(violations) + .describedAs("Precondition failed: expected %d violations on %s", expectedViolations, pathSubstring) + .hasSize(expectedViolations); + + return violations; + } + + record User( + @NotBlank + @Length(min = 6) + String userName, + + @NotBlank + @Length(min = 12) + String password, + + @NotEmpty + List<@NotNull @Valid Hint> passwordHints, + + @NotNull + Set<@NotBlank String> nicknames + ) { + } + + record Hint(@NotBlank String text) { + } + } + private ConstraintViolation firstViolation(T object, String property) { return validator.validateProperty(object, property).iterator().next(); } @@ -508,17 +824,17 @@ private ConstraintViolation firstViolation(T object, String property) { @AllArgsConstructor private static class Person { - @NotEmpty - @Length(min = 2) + @NotEmpty(message = "must not be empty") + @Length(min = 2, message = "length must be between 2 and 2147483647") @Pattern(regexp = "\\p{Alpha}+", message = "must include only alphabetic characters (upper or lower case)") private final String fullName; - @NotNull - @Min(0) - @Max(150) + @NotNull(message = "must not be null") + @Min(value = 0, message = "must be greater than zero") + @Max(value = 150, message = "must be less than 150") private final Integer age; - @NotNull + @NotNull(message = "must not be null") private final LocalDate birthDate; @Valid @@ -529,6 +845,7 @@ private static class Person { @AllArgsConstructor private static class ContactInfo { + @NotNull(message = "must not be null") @Valid private final EmailAddress email; } @@ -537,8 +854,8 @@ private static class ContactInfo { @AllArgsConstructor private static class EmailAddress { - @NotEmpty - @Email + @NotEmpty(message = "must not be blank") + @Email(message = "must be a well-formed email address") private final String address; } } From ce556e2b081917f5862c2215ec617b96e913e19b Mon Sep 17 00:00:00 2001 From: Scott Leberknight <174812+sleberknight@users.noreply.github.com> Date: Sun, 28 Jul 2024 22:32:18 -0400 Subject: [PATCH 2/2] Make the returned Maps unmodifiable, and update the javadocs --- .../validation/KiwiConstraintViolations.java | 34 +++++++------ .../KiwiConstraintViolationsTest.java | 48 +++++++++++++++---- 2 files changed, 58 insertions(+), 24 deletions(-) diff --git a/src/main/java/org/kiwiproject/validation/KiwiConstraintViolations.java b/src/main/java/org/kiwiproject/validation/KiwiConstraintViolations.java index 92b86280..118d217c 100644 --- a/src/main/java/org/kiwiproject/validation/KiwiConstraintViolations.java +++ b/src/main/java/org/kiwiproject/validation/KiwiConstraintViolations.java @@ -1,16 +1,17 @@ package org.kiwiproject.validation; import static com.google.common.base.Preconditions.checkArgument; +import static java.util.stream.Collectors.collectingAndThen; import static java.util.stream.Collectors.groupingBy; import static java.util.stream.Collectors.joining; -import static java.util.stream.Collectors.toMap; -import static java.util.stream.Collectors.toSet; import static java.util.stream.Collectors.toUnmodifiableMap; +import static java.util.stream.Collectors.toUnmodifiableSet; import static org.kiwiproject.base.KiwiPreconditions.checkArgumentNotNull; import static org.kiwiproject.collect.KiwiSets.isNotNullOrEmpty; import static org.kiwiproject.collect.KiwiSets.isNullOrEmpty; import static org.kiwiproject.stream.KiwiMultimapCollectors.toLinkedHashMultimap; +import com.google.common.collect.ImmutableMultimap; import com.google.common.collect.Maps; import com.google.common.collect.Multimap; import jakarta.validation.ConstraintViolation; @@ -20,6 +21,7 @@ import org.apache.commons.text.WordUtils; import java.util.Arrays; +import java.util.Collections; import java.util.List; import java.util.Map; import java.util.Objects; @@ -44,7 +46,7 @@ public class KiwiConstraintViolations { /** - * Convert the set of {@link ConstraintViolation} to a map keyed by the property path. + * Convert the set of {@link ConstraintViolation} to an unmodifiable map keyed by the property path. *

* The map's values are the single {@link ConstraintViolation} associated with each property. *

@@ -66,7 +68,7 @@ public static Map> asMap(Set * The map's values are the single {@link ConstraintViolation} associated with each property. @@ -87,13 +89,13 @@ public static Map> asMap(Set Map> asMap(Set> violations, Function pathTransformer) { - return violations.stream().collect(toMap( + return violations.stream().collect(toUnmodifiableMap( violation -> pathTransformer.apply(violation.getPropertyPath()), violation -> violation)); } /** - * Convert the set of {@link ConstraintViolation} to a map keyed by the property path. + * Convert the set of {@link ConstraintViolation} to an unmodifiable map keyed by the property path. *

* The map's values are the last {@link ConstraintViolation} associated with each property. * The definition of "last" depends on the iteration order of the provided set of violations, which @@ -114,7 +116,7 @@ public static Map> asSingleValuedMap(Set * The map's values are the last {@link ConstraintViolation} associated with each property. @@ -134,14 +136,14 @@ public static Map> asSingleValuedMap(Set Map> asSingleValuedMap(Set> violations, Function pathTransformer) { - return violations.stream().collect(toMap( + return violations.stream().collect(toUnmodifiableMap( violation -> pathTransformer.apply(violation.getPropertyPath()), violation -> violation, (violation1, violation2) -> violation2)); } /** - * Convert the set of {@link ConstraintViolation} to a map keyed by the property path. + * Convert the set of {@link ConstraintViolation} to an unmodifiable map keyed by the property path. *

* The map's values are the set of {@link ConstraintViolation} associated with each property. * @@ -155,10 +157,10 @@ public static Map>> asMultiValuedMap(Set< } /** - * Convert the set of {@link ConstraintViolation} to a map keyed by the property path. + * Convert the set of {@link ConstraintViolation} to an unmodifiable map keyed by the property path. * The property path is determined by the {@code pathTransformer}. *

- * The map's values are the set of {@link ConstraintViolation} associated with each property. + * The map's values are unmodifiable sets of {@link ConstraintViolation} associated with each property. * * @param violations set of non-null but possibly empty violations * @param pathTransformer function to convert a Path into a String @@ -169,11 +171,13 @@ public static Map>> asMultiValuedMap(Set< public static Map>> asMultiValuedMap(Set> violations, Function pathTransformer) { return violations.stream().collect( - groupingBy(violation -> pathTransformer.apply(violation.getPropertyPath()), toSet())); + collectingAndThen( + groupingBy(violation -> pathTransformer.apply(violation.getPropertyPath()), toUnmodifiableSet()), + Collections::unmodifiableMap)); } /** - * Convert the set of {@link ConstraintViolation} to a {@link Multimap} keyed by the property path. + * Convert the set of {@link ConstraintViolation} to an unmodifiable {@link Multimap} keyed by the property path. * * @param violations set of non-null but possibly empty violations * @param the type of the root bean that was validated @@ -188,7 +192,7 @@ public static Multimap> asMultimap(Set Multimap> asMultimap(Set pathTransformer) { return violations.stream() .map(violation -> Maps.immutableEntry(pathTransformer.apply(violation.getPropertyPath()), violation)) - .collect(toLinkedHashMultimap()); + .collect(collectingAndThen(toLinkedHashMultimap(), ImmutableMultimap::copyOf)); } /** diff --git a/src/test/java/org/kiwiproject/validation/KiwiConstraintViolationsTest.java b/src/test/java/org/kiwiproject/validation/KiwiConstraintViolationsTest.java index 122eed85..ac1f22df 100644 --- a/src/test/java/org/kiwiproject/validation/KiwiConstraintViolationsTest.java +++ b/src/test/java/org/kiwiproject/validation/KiwiConstraintViolationsTest.java @@ -14,6 +14,7 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import com.google.common.collect.ImmutableMultimap; import jakarta.validation.ConstraintViolation; import jakarta.validation.Path; import jakarta.validation.Valid; @@ -513,7 +514,9 @@ class AsMap { @Test void shouldReturnEmptyMap_WhenGivenEmptySet() { - assertThat(KiwiConstraintViolations.asMap(Set.of())).isEmpty(); + assertThat(KiwiConstraintViolations.asMap(Set.of())) + .isUnmodifiable() + .isEmpty(); } @Test @@ -533,6 +536,7 @@ void shouldCollectViolationsIntoMapKeyedByPropertyPath() { var violationMap = KiwiConstraintViolations.asMap(violations); assertAll( + () -> assertThat(violationMap).isUnmodifiable(), () -> assertThat(violationMap).containsOnlyKeys("fullName", "birthDate", "contactInfo.email.address"), () -> assertThat(violationMap).extractingByKey("fullName") @@ -556,7 +560,10 @@ void shouldCollectViolationsIntoMap_UsingCustomPathTransformerFunction() { var violationMap = KiwiConstraintViolations.asMap(violations, KiwiConstraintViolations::humanize); - assertThat(violationMap).containsOnlyKeys("Full Name", "Birth Date", "Contact Info / Email / Address"); + assertAll( + () -> assertThat(violationMap).isUnmodifiable(), + () -> assertThat(violationMap).containsOnlyKeys("Full Name", "Birth Date", "Contact Info / Email / Address") + ); } } @@ -565,7 +572,9 @@ class AsSingleValuedMap { @Test void shouldReturnEmptyMap_WhenGivenEmptySet() { - assertThat(KiwiConstraintViolations.asSingleValuedMap(Set.of())).isEmpty(); + assertThat(KiwiConstraintViolations.asSingleValuedMap(Set.of())) + .isUnmodifiable() + .isEmpty(); } @Test @@ -585,6 +594,8 @@ void shouldCollectViolationsIntoMapKeyedByPropertyPath() { var violationMap = KiwiConstraintViolations.asSingleValuedMap(violations); assertAll( + () -> assertThat(violationMap).isUnmodifiable(), + () -> assertThat(violationMap).containsOnlyKeys("fullName", "birthDate", "contactInfo.email.address"), () -> assertThat(violationMap).extractingByKey("fullName") @@ -611,7 +622,10 @@ void shouldCollectViolationsIntoMap_UsingCustomPathTransformerFunction() { var violationMap = KiwiConstraintViolations.asSingleValuedMap(violations, KiwiConstraintViolations::humanize); - assertThat(violationMap).containsOnlyKeys("Full Name", "Birth Date", "Contact Info / Email / Address"); + assertAll( + () -> assertThat(violationMap).isUnmodifiable(), + () -> assertThat(violationMap).containsOnlyKeys("Full Name", "Birth Date", "Contact Info / Email / Address") + ); } } @@ -620,7 +634,9 @@ class AsMultiValuedMap { @Test void shouldReturnEmptyMap_WhenGivenEmptySet() { - assertThat(KiwiConstraintViolations.asMultiValuedMap(Set.of())).isEmpty(); + assertThat(KiwiConstraintViolations.asMultiValuedMap(Set.of())) + .isUnmodifiable() + .isEmpty(); } @Test @@ -631,6 +647,8 @@ void shouldCollectViolationsIntoMapKeyedByPropertyPath() { var violationMap = KiwiConstraintViolations.asMultiValuedMap(violations); assertAll( + () -> assertThat(violationMap).isUnmodifiable(), + () -> assertThat(violationMap).containsOnlyKeys("fullName", "birthDate", "contactInfo.email.address"), () -> assertThat(violationMap.get("fullName")) @@ -659,7 +677,10 @@ void shouldCollectViolationsIntoMap_UsingCustomPathTransformerFunction() { var violationMap = KiwiConstraintViolations.asMultiValuedMap(violations, KiwiConstraintViolations::humanize); - assertThat(violationMap).containsOnlyKeys("Full Name", "Birth Date", "Contact Info / Email / Address"); + assertAll( + () -> assertThat(violationMap).isUnmodifiable(), + () -> assertThat(violationMap).containsOnlyKeys("Full Name", "Birth Date", "Contact Info / Email / Address") + ); } } @@ -668,7 +689,11 @@ class AsMultimap { @Test void shouldReturnEmptyMap_WhenGivenEmptySet() { - assertThat(KiwiConstraintViolations.asMultimap(Set.of()).size()).isZero(); + var multimap = KiwiConstraintViolations.asMultimap(Set.of()); + assertAll( + () -> assertThat(multimap).isInstanceOf(ImmutableMultimap.class), + () -> assertThat(multimap.size()).isZero() + ); } @Test @@ -679,6 +704,8 @@ void shouldCollectViolationsIntoMultimapKeyedByPropertyPath() { var violationMultimap = KiwiConstraintViolations.asMultimap(violations); assertAll( + () -> assertThat(violationMultimap).isInstanceOf(ImmutableMultimap.class), + () -> Assertions.assertThat(violationMultimap) .containsKeys("fullName", "birthDate", "contactInfo.email.address"), @@ -708,8 +735,11 @@ void shouldCollectViolationsIntoMultimap_UsingCustomPathTransformerFunction() { var violationMultimap = KiwiConstraintViolations.asMultimap(violations, KiwiConstraintViolations::humanize); - Assertions.assertThat(violationMultimap) - .containsKeys("Full Name", "Birth Date", "Contact Info / Email / Address"); + assertAll( + () -> assertThat(violationMultimap).isInstanceOf(ImmutableMultimap.class), + () -> Assertions.assertThat(violationMultimap) + .containsKeys("Full Name", "Birth Date", "Contact Info / Email / Address") + ); } }