diff --git a/spring-modulith-core/src/main/java/org/springframework/modulith/core/ApplicationModule.java b/spring-modulith-core/src/main/java/org/springframework/modulith/core/ApplicationModule.java index d32f1f6f..c104efa4 100644 --- a/spring-modulith-core/src/main/java/org/springframework/modulith/core/ApplicationModule.java +++ b/spring-modulith-core/src/main/java/org/springframework/modulith/core/ApplicationModule.java @@ -37,6 +37,7 @@ import java.util.stream.Stream; import org.springframework.lang.Nullable; import org.springframework.modulith.core.Types.JMoleculesTypes; import org.springframework.modulith.core.Types.SpringTypes; +import org.springframework.modulith.core.Violations.Violation; import org.springframework.util.Assert; import org.springframework.util.StringUtils; import org.springframework.util.function.SingletonSupplier; @@ -994,7 +995,7 @@ public class ApplicationModule { .formatted(originModule.getName(), targetModule.getName(), source.getName(), target.getName(), declaredDependencies.toString()); - return violations.and(new IllegalStateException(message)); + return violations.and(new Violation(message)); } // No explicitly allowed dependencies - check for general access @@ -1008,7 +1009,7 @@ public class ApplicationModule { var violationText = "Module '%s' depends on non-exposed type %s within module '%s'!" .formatted(originModule.getName(), target.getName(), targetModule.getName()); - return violations.and(new IllegalStateException(violationText + lineSeparator() + description)); + return violations.and(new Violation(violationText + lineSeparator() + description)); } return violations; @@ -1184,7 +1185,7 @@ public class ApplicationModule { ApplicationModule module = getExistingModuleOf(source.getOwner(), modules); - violations = violations.and(new IllegalStateException( + violations = violations.and(new Violation( String.format("Module %s uses field injection in %s. Prefer constructor injection instead!", module.getDisplayName(), source.getFullName()))); } diff --git a/spring-modulith-core/src/main/java/org/springframework/modulith/core/ApplicationModules.java b/spring-modulith-core/src/main/java/org/springframework/modulith/core/ApplicationModules.java index a9b436d5..0a2e644d 100644 --- a/spring-modulith-core/src/main/java/org/springframework/modulith/core/ApplicationModules.java +++ b/spring-modulith-core/src/main/java/org/springframework/modulith/core/ApplicationModules.java @@ -40,6 +40,7 @@ import org.springframework.core.annotation.AnnotationAwareOrderComparator; import org.springframework.core.io.support.SpringFactoriesLoader; import org.springframework.lang.Nullable; import org.springframework.modulith.core.Types.JMoleculesTypes; +import org.springframework.modulith.core.Violations.Violation; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; import org.springframework.util.function.SingletonSupplier; @@ -401,7 +402,7 @@ public class ApplicationModules implements Iterable { Violations violations = rootPackages.stream() // .map(this::assertNoCyclesFor) // .flatMap(it -> it.getDetails().stream()) // - .map(IllegalStateException::new) // + .map(Violation::new) // .collect(Violations.toViolations()); if (JMoleculesTypes.areRulesPresent()) { diff --git a/spring-modulith-core/src/main/java/org/springframework/modulith/core/Violations.java b/spring-modulith-core/src/main/java/org/springframework/modulith/core/Violations.java index 60b4f96e..45e0195b 100644 --- a/spring-modulith-core/src/main/java/org/springframework/modulith/core/Violations.java +++ b/spring-modulith-core/src/main/java/org/springframework/modulith/core/Violations.java @@ -34,18 +34,18 @@ public class Violations extends RuntimeException { public static Violations NONE = new Violations(Collections.emptyList()); - private final List exceptions; + private final List violations; /** - * Creates a new {@link Violations} from the given {@link RuntimeException}s. + * Creates a new {@link Violations} from the given {@link ArchitecturalViolation}s. * - * @param exceptions must not be {@literal null}. + * @param violations must not be {@literal null}. */ - private Violations(List exceptions) { + private Violations(List violations) { - Assert.notNull(exceptions, "Exceptions must not be null!"); + Assert.notNull(violations, "Exceptions must not be null!"); - this.exceptions = exceptions; + this.violations = violations; } /** @@ -54,7 +54,7 @@ public class Violations extends RuntimeException { * * @return will never be {@literal null}. */ - static Collector toViolations() { + static Collector toViolations() { return Collectors.collectingAndThen(Collectors.toList(), Violations::new); } @@ -65,8 +65,8 @@ public class Violations extends RuntimeException { @Override public String getMessage() { - return exceptions.stream() // - .map(RuntimeException::getMessage) // + return violations.stream() // + .map(Violation::message) // .collect(Collectors.joining("\n- ", "- ", "")); } @@ -78,8 +78,8 @@ public class Violations extends RuntimeException { */ public List getMessages() { - return exceptions.stream() // - .map(RuntimeException::getMessage) + return violations.stream() // + .map(Violation::message) .toList(); } @@ -89,7 +89,7 @@ public class Violations extends RuntimeException { * @return */ public boolean hasViolations() { - return !exceptions.isEmpty(); + return !violations.isEmpty(); } /** @@ -108,36 +108,45 @@ public class Violations extends RuntimeException { * @param exception must not be {@literal null}. * @return */ - Violations and(RuntimeException exception) { + Violations and(Violation violation) { - Assert.notNull(exception, "Exception must not be null!"); + Assert.notNull(violation, "Exception must not be null!"); - List newExceptions = new ArrayList<>(exceptions.size() + 1); - newExceptions.addAll(exceptions); - newExceptions.add(exception); + if (violations.isEmpty()) { + return new Violations(List.of(violation)); + } + + if (violations.contains(violation)) { + return this; + } + + List newExceptions = new ArrayList<>(violations.size() + 1); + newExceptions.addAll(violations); + newExceptions.add(violation); return new Violations(newExceptions); } Violations and(Violations other) { - List newExceptions = new ArrayList<>(exceptions.size() + other.exceptions.size()); - newExceptions.addAll(exceptions); - newExceptions.addAll(other.exceptions); + if (violations.isEmpty()) { + return new Violations(other.violations); + } + + var newExceptions = new ArrayList<>(violations); + + for (Violation candidate : other.violations) { + if (!violations.contains(candidate)) { + newExceptions.add(candidate); + } + } return new Violations(newExceptions); } Violations and(String violation) { - return and(new ArchitecturalViolation(violation)); + return and(new Violation(violation)); } - private static class ArchitecturalViolation extends RuntimeException { - - private static final long serialVersionUID = 3587887036508024142L; - - public ArchitecturalViolation(String message) { - super(message); - } - } + static record Violation(String message) {} } diff --git a/spring-modulith-core/src/test/java/org/springframework/modulith/core/ViolationsUnitTests.java b/spring-modulith-core/src/test/java/org/springframework/modulith/core/ViolationsUnitTests.java index ec322e02..a86aa280 100644 --- a/spring-modulith-core/src/test/java/org/springframework/modulith/core/ViolationsUnitTests.java +++ b/spring-modulith-core/src/test/java/org/springframework/modulith/core/ViolationsUnitTests.java @@ -30,10 +30,25 @@ class ViolationsUnitTests { void combinesExceptionMessages() { Violations violations = Violations.NONE // - .and(new IllegalArgumentException("First")) // - .and(new IllegalArgumentException("Second")); + .and("First") // + .and("Second"); assertThat(violations.getMessage()) // .isEqualTo("- First\n- Second"); } + + @Test + void deduplicatesViolationsWithTheSameMessage() { + + var violations = Violations.NONE + .and("First") + .and("First") + .and("Second") + .and(Violations.NONE.and("First") + .and("Second") + .and("Third")); + + assertThat(violations.getMessages()) + .containsExactlyInAnyOrder("First", "Second", "Third"); + } }