From a7e033172b585493636d861cd52268b78ced0a11 Mon Sep 17 00:00:00 2001 From: Oliver Drotbohm Date: Wed, 4 Jan 2023 23:38:28 +0100 Subject: [PATCH] GH-102 - ApplicationModules now allows traversing modules in topological order. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We now optionally integrate with the JGraphT library to calculate the topological order of modules based on their dependency structure. That order is then exposed in ApplicationModules' iteration and via ….getComparator(). Renamed FormattableJavaClass to FormattableType and allow it to be created from a plain Class as well. --- spring-modulith-core/pom.xml | 7 + .../modulith/model/ApplicationModule.java | 2 +- .../modulith/model/ApplicationModules.java | 130 ++++++++++++++++-- .../model/ArchitecturallyEvidentType.java | 2 +- .../modulith/model/DependencyType.java | 10 +- ...ableJavaClass.java => FormatableType.java} | 53 +++++-- .../modulith/model/JavaAccessSource.java | 4 +- .../modulith/docs/Asciidoctor.java | 4 +- spring-modulith-integration-test/pom.xml | 7 + ...=> ApplicationModulesIntegrationTest.java} | 18 ++- .../observability/DefaultObservedModule.java | 6 +- .../modulith/test/ModuleTestExecution.java | 2 +- 12 files changed, 205 insertions(+), 40 deletions(-) rename spring-modulith-core/src/main/java/org/springframework/modulith/model/{FormatableJavaClass.java => FormatableType.java} (69%) rename spring-modulith-integration-test/src/test/java/org/springframework/modulith/model/{ModulesIntegrationTest.java => ApplicationModulesIntegrationTest.java} (88%) diff --git a/spring-modulith-core/pom.xml b/spring-modulith-core/pom.xml index 9de659ab..4abd2bb4 100644 --- a/spring-modulith-core/pom.xml +++ b/spring-modulith-core/pom.xml @@ -29,6 +29,13 @@ ${archunit.version} + + org.jgrapht + jgrapht-core + 1.4.0 + true + + org.springframework spring-core diff --git a/spring-modulith-core/src/main/java/org/springframework/modulith/model/ApplicationModule.java b/spring-modulith-core/src/main/java/org/springframework/modulith/model/ApplicationModule.java index a5a193c5..ca64556e 100644 --- a/spring-modulith-core/src/main/java/org/springframework/modulith/model/ApplicationModule.java +++ b/spring-modulith-core/src/main/java/org/springframework/modulith/model/ApplicationModule.java @@ -684,7 +684,7 @@ public class ApplicationModule { */ @Override public String toString() { - return type.format(FormatableJavaClass.of(source), FormatableJavaClass.of(target)); + return type.format(FormatableType.of(source), FormatableType.of(target)); } static QualifiedDependency fromCodeUnitParameter(JavaCodeUnit codeUnit, JavaClass parameter) { diff --git a/spring-modulith-core/src/main/java/org/springframework/modulith/model/ApplicationModules.java b/spring-modulith-core/src/main/java/org/springframework/modulith/model/ApplicationModules.java index 3863f188..af7eceed 100644 --- a/spring-modulith-core/src/main/java/org/springframework/modulith/model/ApplicationModules.java +++ b/spring-modulith-core/src/main/java/org/springframework/modulith/model/ApplicationModules.java @@ -29,13 +29,23 @@ import java.util.*; import java.util.function.Function; import java.util.stream.Collectors; import java.util.stream.Stream; +import java.util.stream.StreamSupport; +import org.jgrapht.Graph; +import org.jgrapht.graph.DefaultDirectedGraph; +import org.jgrapht.graph.DefaultEdge; +import org.jgrapht.traverse.TopologicalOrderIterator; import org.jmolecules.archunit.JMoleculesDddRules; +import org.springframework.core.Ordered; +import org.springframework.core.annotation.AnnotationAwareOrderComparator; +import org.springframework.core.annotation.Order; import org.springframework.core.io.support.SpringFactoriesLoader; +import org.springframework.lang.Nullable; import org.springframework.modulith.Modulith; import org.springframework.modulith.Modulithic; import org.springframework.modulith.model.Types.JMoleculesTypes; import org.springframework.util.Assert; +import org.springframework.util.ClassUtils; import com.tngtech.archunit.base.DescribedPredicate; import com.tngtech.archunit.core.domain.JavaClass; @@ -56,6 +66,8 @@ public class ApplicationModules implements Iterable { private static final Map CACHE = new HashMap<>(); private static final ApplicationModuleDetectionStrategy DETECTION_STRATEGY; private static final ImportOption IMPORT_OPTION = new ImportOption.DoNotIncludeTests(); + private static final boolean JGRAPHT_PRESENT = ClassUtils.isPresent("org.jgrapht.Graph", + ApplicationModules.class.getClassLoader()); static { @@ -79,6 +91,7 @@ public class ApplicationModules implements Iterable { private final JavaClasses allClasses; private final List rootPackages; private final @With(AccessLevel.PRIVATE) @Getter Set sharedModules; + private final List orderedNames; private boolean verified; @@ -104,6 +117,10 @@ public class ApplicationModules implements Iterable { .toList(); this.sharedModules = Collections.emptySet(); + + this.orderedNames = JGRAPHT_PRESENT // + ? TopologicalSorter.topologicallySortModules(this) // + : modules.values().stream().map(ApplicationModule::getName).toList(); } /** @@ -272,6 +289,16 @@ public class ApplicationModules implements Iterable { .findFirst(); } + /** + * Returns the {@link ApplicationModule} containing the given type. + * + * @param candidate must not be {@literal null}. + * @return will never be {@literal null}. + */ + public Optional getModuleByType(Class candidate) { + return getModuleByType(candidate.getName()); + } + public Optional getModuleForPackage(String name) { return modules.values().stream() // @@ -328,23 +355,13 @@ public class ApplicationModules implements Iterable { .reduce(violations, Violations::and); } - private FailureReport assertNoCyclesFor(JavaPackage rootPackage) { - - EvaluationResult result = SlicesRuleDefinition.slices() // - .matching(rootPackage.getName().concat(".(*)..")) // - .should().beFreeOfCycles() // - .evaluate(allClasses.that(resideInAPackage(rootPackage.getName().concat("..")))); - - return result.getFailureReport(); - } - /** * Returns all {@link ApplicationModule}s. * * @return will never be {@literal null}. */ public Stream stream() { - return modules.values().stream(); + return StreamSupport.stream(this.spliterator(), false); } /** @@ -356,13 +373,71 @@ public class ApplicationModules implements Iterable { return metadata.getSystemName(); } + /** + * Returns a {@link Comparator} that will sort objects based on their types' application module. In other words, + * objects of types in more fundamental modules will be ordered before ones residing in downstream modules. For + * example, if module A depends on B, objects of types residing in B will be ordered before ones in A. For objects + * residing in the same module, standard Spring-based ordering (via {@link Order} or {@link Ordered}) will be applied. + * + * @return will never be {@literal null}. + */ + public Comparator getComparator() { + + return (left, right) -> { + + var leftIndex = getModuleIndexFor(left); + + if (leftIndex == null) { + return 1; + } + + var rightIndex = getModuleIndexFor(right); + + if (rightIndex == null) { + return -1; + } + + var result = leftIndex - rightIndex; + + return result != 0 ? result : AnnotationAwareOrderComparator.INSTANCE.compare(left, right); + }; + } + /* * (non-Javadoc) * @see java.lang.Iterable#iterator() */ @Override public Iterator iterator() { - return modules.values().iterator(); + return orderedNames.stream().map(this::getRequiredModule).iterator(); + } + + private FailureReport assertNoCyclesFor(JavaPackage rootPackage) { + + EvaluationResult result = SlicesRuleDefinition.slices() // + .matching(rootPackage.getName().concat(".(*)..")) // + .should().beFreeOfCycles() // + .evaluate(allClasses.that(resideInAPackage(rootPackage.getName().concat("..")))); + + return result.getFailureReport(); + } + + /** + * Returns the index of the module that contains the type of the given object or 1 if the given object is + * {@literal null} or its type does not reside in any module. + * + * @param object can be {@literal null}. + * @return + */ + private Integer getModuleIndexFor(@Nullable Object object) { + + return Optional.ofNullable(object) + .map(it -> Class.class.isInstance(it) ? Class.class.cast(it) : it.getClass()) + .map(Class::getName) + .flatMap(this::getModuleByType) + .map(ApplicationModule::getName) + .map(orderedNames::indexOf) + .orElse(null); } /** @@ -445,4 +520,35 @@ public class ApplicationModules implements Iterable { return ModulithMetadata.of(basePackage); } } + + /** + * Dedicated class to be able to only optionally depend on the JGraphT library. + * + * @author Oliver Drotbohm + */ + private static class TopologicalSorter { + + private static List topologicallySortModules(ApplicationModules modules) { + + Graph graph = new DefaultDirectedGraph<>(DefaultEdge.class); + + modules.modules.forEach((__, project) -> { + + graph.addVertex(project); + + project.getDependencies(modules).stream() // + .map(ApplicationModuleDependency::getTargetModule) // + .forEach(dependency -> { + graph.addVertex(dependency); + graph.addEdge(project, dependency); + }); + }); + + var names = new ArrayList(); + + new TopologicalOrderIterator<>(graph).forEachRemaining(it -> names.add(0, it.getName())); + + return names; + } + } } diff --git a/spring-modulith-core/src/main/java/org/springframework/modulith/model/ArchitecturallyEvidentType.java b/spring-modulith-core/src/main/java/org/springframework/modulith/model/ArchitecturallyEvidentType.java index 23b4d3ae..1115d60f 100644 --- a/spring-modulith-core/src/main/java/org/springframework/modulith/model/ArchitecturallyEvidentType.java +++ b/spring-modulith-core/src/main/java/org/springframework/modulith/model/ArchitecturallyEvidentType.java @@ -92,7 +92,7 @@ public abstract class ArchitecturallyEvidentType { * @return will never be {@literal null}. */ String getAbbreviatedFullName() { - return FormatableJavaClass.of(getType()).getAbbreviatedFullName(); + return FormatableType.of(getType()).getAbbreviatedFullName(); } /** diff --git a/spring-modulith-core/src/main/java/org/springframework/modulith/model/DependencyType.java b/spring-modulith-core/src/main/java/org/springframework/modulith/model/DependencyType.java index a8da5f92..514df37a 100644 --- a/spring-modulith-core/src/main/java/org/springframework/modulith/model/DependencyType.java +++ b/spring-modulith-core/src/main/java/org/springframework/modulith/model/DependencyType.java @@ -47,7 +47,7 @@ public enum DependencyType { * @see org.springframework.modulith.model.Module.DependencyType#format(org.springframework.modulith.model.FormatableJavaClass, org.springframework.modulith.model.FormatableJavaClass) */ @Override - public String format(FormatableJavaClass source, FormatableJavaClass target) { + public String format(FormatableType source, FormatableType target) { return String.format("Component %s using %s", source.getAbbreviatedFullName(), target.getAbbreviatedFullName()); } }, @@ -62,7 +62,7 @@ public enum DependencyType { * @see org.springframework.modulith.model.Module.DependencyType#format(org.springframework.modulith.model.FormatableJavaClass, org.springframework.modulith.model.FormatableJavaClass) */ @Override - public String format(FormatableJavaClass source, FormatableJavaClass target) { + public String format(FormatableType source, FormatableType target) { return String.format("Entity %s depending on %s", source.getAbbreviatedFullName(), target.getAbbreviatedFullName()); } @@ -79,7 +79,7 @@ public enum DependencyType { * @see org.springframework.modulith.model.Module.DependencyType#format(org.springframework.modulith.model.FormatableJavaClass, org.springframework.modulith.model.FormatableJavaClass) */ @Override - public String format(FormatableJavaClass source, FormatableJavaClass target) { + public String format(FormatableType source, FormatableType target) { return String.format("%s listening to events of type %s", source.getAbbreviatedFullName(), target.getAbbreviatedFullName()); } @@ -101,7 +101,7 @@ public enum DependencyType { * @see org.springframework.modulith.model.Module.DependencyType#format(org.springframework.modulith.model.FormatableJavaClass, org.springframework.modulith.model.FormatableJavaClass) */ @Override - public String format(FormatableJavaClass source, FormatableJavaClass target) { + public String format(FormatableType source, FormatableType target) { return String.format("%s depending on %s", source.getAbbreviatedFullName(), target.getAbbreviatedFullName()); } }; @@ -135,7 +135,7 @@ public enum DependencyType { return forParameter(dependency.getTargetClass()); } - public abstract String format(FormatableJavaClass source, FormatableJavaClass target); + public abstract String format(FormatableType source, FormatableType target); /** * Returns all {@link DependencyType}s except the given ones. diff --git a/spring-modulith-core/src/main/java/org/springframework/modulith/model/FormatableJavaClass.java b/spring-modulith-core/src/main/java/org/springframework/modulith/model/FormatableType.java similarity index 69% rename from spring-modulith-core/src/main/java/org/springframework/modulith/model/FormatableJavaClass.java rename to spring-modulith-core/src/main/java/org/springframework/modulith/model/FormatableType.java index a8eee053..adb66f7f 100644 --- a/spring-modulith-core/src/main/java/org/springframework/modulith/model/FormatableJavaClass.java +++ b/spring-modulith-core/src/main/java/org/springframework/modulith/model/FormatableType.java @@ -38,26 +38,50 @@ import com.tngtech.archunit.thirdparty.com.google.common.base.Suppliers; * @author Oliver Drotbohm */ @RequiredArgsConstructor(access = AccessLevel.PRIVATE) -public class FormatableJavaClass { +public class FormatableType { - private static final Map CACHE = new ConcurrentHashMap<>(); + private static final Map CACHE = new ConcurrentHashMap<>(); - private final JavaClass type; + private final String type; private final Supplier abbreviatedName; - public static FormatableJavaClass of(JavaClass type) { - return CACHE.computeIfAbsent(type, FormatableJavaClass::new); - } - - private FormatableJavaClass(JavaClass type) { + /** + * Creates a new {@link FormatableType} for the given {@link JavaClass}. + * + * @param type must not be {@literal null}. + * @return will never be {@literal null}. + */ + public static FormatableType of(JavaClass type) { Assert.notNull(type, "JavaClass must not be null!"); + return CACHE.computeIfAbsent(type.getName(), FormatableType::new); + } + + /** + * Creates a new {@link FormatableType} for the given {@link Class}. + * + * @param type must not be {@literal null}. + * @return will never be {@literal null}. + */ + public static FormatableType of(Class type) { + return CACHE.computeIfAbsent(type.getName(), FormatableType::new); + } + + /** + * Creates a new {@link FormatableType} for the given fully-qualified type name. + * + * @param type must not be {@literal null} or empty. + */ + private FormatableType(String type) { + + Assert.hasText(type, "Type must not be null or empty!"); + this.type = type; this.abbreviatedName = Suppliers.memoize(() -> { String abbreviatedPackage = Stream // - .of(type.getPackageName().split("\\.")) // + .of(ClassUtils.getPackageName(type).split("\\.")) // .map(it -> it.substring(0, 1)) // .collect(Collectors.joining(".")); @@ -76,6 +100,13 @@ public class FormatableJavaClass { return abbreviatedName.get(); } + /** + * Returns the abbreviated full name of the type abbreviating only the part of the given {@link ApplicationModule}'s + * base package. + * + * @param module can be {@literal null}. + * @return will never be {@literal null}. + */ public String getAbbreviatedFullName(@Nullable ApplicationModule module) { if (module == null) { @@ -88,7 +119,7 @@ public class FormatableJavaClass { return getAbbreviatedFullName(); } - String typePackageName = type.getPackageName(); + String typePackageName = ClassUtils.getPackageName(type); if (basePackageName.equals(typePackageName)) { return getAbbreviatedFullName(); @@ -110,7 +141,7 @@ public class FormatableJavaClass { * @return will never be {@literal null}. */ public String getFullName() { - return type.getName().replace("$", "."); + return type.replace("$", "."); } private static String abbreviate(String source) { diff --git a/spring-modulith-core/src/main/java/org/springframework/modulith/model/JavaAccessSource.java b/spring-modulith-core/src/main/java/org/springframework/modulith/model/JavaAccessSource.java index 61bd33a5..5679e5c9 100644 --- a/spring-modulith-core/src/main/java/org/springframework/modulith/model/JavaAccessSource.java +++ b/spring-modulith-core/src/main/java/org/springframework/modulith/model/JavaAccessSource.java @@ -30,7 +30,7 @@ class JavaAccessSource implements Source { private final static Pattern LAMBDA_EXTRACTOR = Pattern.compile("lambda\\$(.*)\\$.*"); - private final FormatableJavaClass type; + private final FormatableType type; private final JavaCodeUnit method; private final String name; @@ -41,7 +41,7 @@ class JavaAccessSource implements Source { */ public JavaAccessSource(JavaAccess access) { - this.type = FormatableJavaClass.of(access.getOriginOwner()); + this.type = FormatableType.of(access.getOriginOwner()); this.method = access.getOrigin(); String name = method.getName(); diff --git a/spring-modulith-docs/src/main/java/org/springframework/modulith/docs/Asciidoctor.java b/spring-modulith-docs/src/main/java/org/springframework/modulith/docs/Asciidoctor.java index 5c5debe9..8a658410 100644 --- a/spring-modulith-docs/src/main/java/org/springframework/modulith/docs/Asciidoctor.java +++ b/spring-modulith-docs/src/main/java/org/springframework/modulith/docs/Asciidoctor.java @@ -34,7 +34,7 @@ import org.springframework.modulith.model.ApplicationModules; import org.springframework.modulith.model.ArchitecturallyEvidentType; import org.springframework.modulith.model.DependencyType; import org.springframework.modulith.model.EventType; -import org.springframework.modulith.model.FormatableJavaClass; +import org.springframework.modulith.model.FormatableType; import org.springframework.modulith.model.Source; import org.springframework.modulith.model.SpringBean; import org.springframework.util.Assert; @@ -259,7 +259,7 @@ class Asciidoctor { ApplicationModule module = modules.getModuleByType(source).orElse(null); String typeAndMethod = toCode( - toTypeAndMethod(FormatableJavaClass.of(source).getAbbreviatedFullName(module), methodSignature)); + toTypeAndMethod(FormatableType.of(source).getAbbreviatedFullName(module), methodSignature)); if (module == null || !source.getModifiers().contains(JavaModifier.PUBLIC) diff --git a/spring-modulith-integration-test/pom.xml b/spring-modulith-integration-test/pom.xml index 8a1a1331..6b8ba029 100644 --- a/spring-modulith-integration-test/pom.xml +++ b/spring-modulith-integration-test/pom.xml @@ -57,6 +57,13 @@ spring-boot-starter-test test + + + org.jgrapht + jgrapht-core + 1.4.0 + test + diff --git a/spring-modulith-integration-test/src/test/java/org/springframework/modulith/model/ModulesIntegrationTest.java b/spring-modulith-integration-test/src/test/java/org/springframework/modulith/model/ApplicationModulesIntegrationTest.java similarity index 88% rename from spring-modulith-integration-test/src/test/java/org/springframework/modulith/model/ModulesIntegrationTest.java rename to spring-modulith-integration-test/src/test/java/org/springframework/modulith/model/ApplicationModulesIntegrationTest.java index 6510946f..cc989503 100644 --- a/spring-modulith-integration-test/src/test/java/org/springframework/modulith/model/ModulesIntegrationTest.java +++ b/spring-modulith-integration-test/src/test/java/org/springframework/modulith/model/ApplicationModulesIntegrationTest.java @@ -17,9 +17,9 @@ package org.springframework.modulith.model; import static org.assertj.core.api.Assertions.*; +import java.util.ArrayList; import java.util.List; import java.util.Optional; -import java.util.stream.Collectors; import java.util.stream.Stream; import org.junit.jupiter.api.Test; @@ -27,13 +27,15 @@ import org.junit.jupiter.api.Test; import com.acme.myproject.Application; import com.acme.myproject.complex.internal.FirstTypeBasedPort; import com.acme.myproject.complex.internal.SecondTypeBasePort; +import com.acme.myproject.moduleA.ServiceComponentA; import com.acme.myproject.moduleA.SomeConfigurationA.SomeAtBeanComponentA; +import com.acme.myproject.moduleB.ServiceComponentB; /** * @author Oliver Drotbohm * @author Peter Gafert */ -class ModulesIntegrationTest { +class ApplicationModulesIntegrationTest { ApplicationModules modules = ApplicationModules.of(Application.class); @@ -150,6 +152,18 @@ class ModulesIntegrationTest { modules.stream().map(ApplicationModule::getName).toList()); } + @Test // #102 + void ordersTypesByModuleDependencies() { + + // Non-module type first, B depending on A + var source = new ArrayList<>(List.of(String.class, ServiceComponentB.class, ServiceComponentA.class)); + + source.sort(ApplicationModules.of(Application.class).getComparator()); + + // Expect A before B before non-module type. + assertThat(source).containsExactly(ServiceComponentA.class, ServiceComponentB.class, String.class); + } + private static void verifyNamedInterfaces(NamedInterfaces interfaces, String name, Class... types) { Stream.of(types).forEach(type -> { diff --git a/spring-modulith-observability/src/main/java/org/springframework/modulith/observability/DefaultObservedModule.java b/spring-modulith-observability/src/main/java/org/springframework/modulith/observability/DefaultObservedModule.java index 096f4c73..c8cf4d9d 100644 --- a/spring-modulith-observability/src/main/java/org/springframework/modulith/observability/DefaultObservedModule.java +++ b/spring-modulith-observability/src/main/java/org/springframework/modulith/observability/DefaultObservedModule.java @@ -23,7 +23,7 @@ import java.util.Arrays; import org.aopalliance.intercept.MethodInvocation; import org.springframework.aop.ProxyMethodInvocation; import org.springframework.aop.framework.Advised; -import org.springframework.modulith.model.FormatableJavaClass; +import org.springframework.modulith.model.FormatableType; import org.springframework.modulith.model.ApplicationModule; import org.springframework.modulith.model.ApplicationModules; import org.springframework.modulith.model.SpringBean; @@ -132,8 +132,8 @@ class DefaultObservedModule implements ObservedModule { private static String toString(Class type, Method method, ApplicationModule module) { String typeName = module.getType(type.getName()) - .map(FormatableJavaClass::of) - .map(FormatableJavaClass::getAbbreviatedFullName) + .map(FormatableType::of) + .map(FormatableType::getAbbreviatedFullName) .orElseGet(() -> type.getName()); return String.format("%s.%s(…)", typeName, method.getName()); diff --git a/spring-modulith-test/src/main/java/org/springframework/modulith/test/ModuleTestExecution.java b/spring-modulith-test/src/main/java/org/springframework/modulith/test/ModuleTestExecution.java index 290c30c5..b83840e8 100644 --- a/spring-modulith-test/src/main/java/org/springframework/modulith/test/ModuleTestExecution.java +++ b/spring-modulith-test/src/main/java/org/springframework/modulith/test/ModuleTestExecution.java @@ -126,7 +126,7 @@ public class ModuleTestExecution implements Iterable { || basePackages.get().stream().anyMatch(it -> it.contains(className)); if (result) { - LOG.debug("Including class {}.", className); + LOG.trace("Including class {}.", className); } return !result;