Skip to content

Commit

Permalink
GH-802 - Allow transitive application module dependency resolution.
Browse files Browse the repository at this point in the history
ApplicationModule now exposes both getDirectDependencies(…) and getAllDependencies(…), the former as alias for the now deprecated getDependencies(…) for symmetry reasons. The latter recursively resolves transitive dependencies.

We now optimize the dependency analysis by skipping types residing java and javax packages as they're not relevant to our dependency arrangement model. A few additional optimizations in ApplicationModuleDependencies to avoid iterating over each establishing dependency if all we need to look at is the general module dependency arrangement.

Improve performance of ApplicationModule.contains(…) checks by checking whether the given type can even live inside the package space of the module.
  • Loading branch information
odrotbohm committed Sep 6, 2024
1 parent d5ab8b6 commit f09f3aa
Show file tree
Hide file tree
Showing 11 changed files with 295 additions and 121 deletions.

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/
package org.springframework.modulith.core;

import java.util.Collection;
import java.util.HashSet;
import java.util.List;
import java.util.function.Function;
Expand All @@ -30,22 +31,23 @@
public class ApplicationModuleDependencies {

private final List<ApplicationModuleDependency> dependencies;
private final ApplicationModules modules;
private final Collection<ApplicationModule> modules;

/**
* Creates a new {@link ApplicationModuleDependencies} for the given {@link List} of
* {@link ApplicationModuleDependency} and {@link ApplicationModules}.
*
* @param dependencies must not be {@literal null}.
* @param modules must not be {@literal null}.
*/
private ApplicationModuleDependencies(List<ApplicationModuleDependency> dependencies, ApplicationModules modules) {
private ApplicationModuleDependencies(List<ApplicationModuleDependency> dependencies) {

Assert.notNull(dependencies, "ApplicationModuleDependency list must not be null!");
Assert.notNull(modules, "ApplicationModules must not be null!");

this.dependencies = dependencies;
this.modules = modules;
this.modules = dependencies.stream()
.map(ApplicationModuleDependency::getTargetModule)
.distinct()
.toList();
}

/**
Expand All @@ -56,10 +58,8 @@ private ApplicationModuleDependencies(List<ApplicationModuleDependency> dependen
* @param modules must not be {@literal null}.
* @return will never be {@literal null}.
*/
static ApplicationModuleDependencies of(List<ApplicationModuleDependency> dependencies,
ApplicationModules modules) {

return new ApplicationModuleDependencies(dependencies, modules);
static ApplicationModuleDependencies of(List<ApplicationModuleDependency> dependencies) {
return new ApplicationModuleDependencies(dependencies);
}

/**
Expand All @@ -72,9 +72,7 @@ public boolean contains(ApplicationModule module) {

Assert.notNull(module, "ApplicationModule must not be null!");

return dependencies.stream()
.map(ApplicationModuleDependency::getTargetModule)
.anyMatch(module::equals);
return modules.contains(module);
}

/**
Expand All @@ -87,8 +85,7 @@ public boolean containsModuleNamed(String name) {

Assert.hasText(name, "Module name must not be null or empty!");

return dependencies.stream()
.map(ApplicationModuleDependency::getTargetModule)
return modules.stream()
.map(ApplicationModule::getName)
.anyMatch(name::equals);
}
Expand Down Expand Up @@ -119,13 +116,22 @@ public Stream<ApplicationModuleDependency> uniqueStream(Function<ApplicationModu
.filter(it -> seenTargets.add(extractor.apply(it)));
}

/**
* Returns a new {@link ApplicationModuleDependencies} instance containing only the dependencies of the given
* {@link DependencyType}.
*
* @param type must not be {@literal null}.
* @return
*/
public ApplicationModuleDependencies withType(DependencyType type) {

Assert.notNull(type, "DependencyType must not be null!");

var filtered = dependencies.stream()
.filter(it -> it.getDependencyType().equals(type))
.toList();

return ApplicationModuleDependencies.of(filtered, modules);
return ApplicationModuleDependencies.of(filtered);
}

/**
Expand All @@ -134,6 +140,45 @@ public ApplicationModuleDependencies withType(DependencyType type) {
* @return will never be {@literal null}.
*/
public boolean isEmpty() {
return dependencies.isEmpty();
return modules.isEmpty();
}

/**
* Returns whether the dependencies contain the type with the given fully-qualified name.
*
* @param type must not be {@literal null} or empty.
* @return
* @since 1.3
*/
public boolean contains(String type) {

Assert.hasText(type, "Type must not be null or empty!");

return uniqueModules().anyMatch(it -> it.contains(type));
}

/**
* Returns all unique {@link ApplicationModule}s involved in the dependencies.
*
* @return will never be {@literal null}.
*/
public Stream<ApplicationModule> uniqueModules() {
return modules.stream();
}

/**
* Returns the {@link ApplicationModule} containing the given type.
*
* @param name must not be {@literal null} or empty.
* @return will never be {@literal null}.
*/
public ApplicationModule getModuleByType(String name) {

Assert.hasText(name, "Name must not be null or empty!");

return uniqueModules()
.filter(it -> it.contains(name))
.findFirst()
.orElse(null);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -778,8 +778,7 @@ private static List<String> topologicallySortModules(ApplicationModules modules)

graph.addVertex(project);

project.getDependencies(modules).stream() //
.map(ApplicationModuleDependency::getTargetModule) //
project.getDirectDependencies(modules).uniqueModules() //
.forEach(dependency -> {
graph.addVertex(dependency);
graph.addEdge(project, dependency);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package org.springframework.modulith.core;

import org.springframework.util.Assert;
import org.springframework.util.ClassUtils;

/**
* The name of a Java package. Packages are sortable comparing their individual segments and deeper packages sorted
Expand All @@ -32,16 +33,29 @@ class PackageName implements Comparable<PackageName> {
/**
* Creates a new {@link PackageName} with the given name.
*
* @param name must not be {@literal null} or empty.
* @param name must not be {@literal null}.
*/
public PackageName(String name) {

Assert.hasText(name, "Name must not be null or empty!");
Assert.notNull(name, "Name must not be null!");

this.name = name;
this.segments = name.split("\\.");
}

/**
* Creates a new {@link PackageName} for the given fully-qualified type name.
*
* @param fullyQualifiedName must not be {@literal null} or empty.
* @return will never be {@literal null}.
*/
public static PackageName ofType(String fullyQualifiedName) {

Assert.notNull(fullyQualifiedName, "Type name must not be null!");

return new PackageName(ClassUtils.getPackageName(fullyQualifiedName));
}

/**
* Returns the length of the package name.
*
Expand Down Expand Up @@ -118,6 +132,19 @@ boolean isParentPackageOf(PackageName reference) {
return reference.name.startsWith(name + ".");
}

/**
* Returns whether the package name contains the given one, i.e. if the given one either is the current one or a
* sub-package of it.
*
* @param reference must not be {@literal null}.
*/
boolean contains(PackageName reference) {

Assert.notNull(reference, "Reference package name must not be null!");

return this.equals(reference) || reference.isSubPackageOf(this);
}

/**
* Returns whether the current {@link PackageName} is the name of a sub-package with the given name.
*
Expand All @@ -131,6 +158,10 @@ boolean isSubPackageOf(PackageName reference) {
return name.startsWith(reference.name + ".");
}

boolean isEmpty() {
return length() == 0;
}

/*
* (non-Javadoc)
* @see java.lang.Comparable#compareTo(java.lang.Object)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import static org.springframework.modulith.core.SyntacticSugar.*;

import java.lang.annotation.Annotation;
import java.util.function.Predicate;

import org.springframework.lang.Nullable;
import org.springframework.modulith.PackageInfo;
Expand Down Expand Up @@ -79,6 +80,14 @@ public static boolean areRulesPresent() {
}
}

static class JavaTypes {

static Predicate<JavaClass> IS_CORE_JAVA_TYPE = it -> it.getName().startsWith("java.")
|| it.getName().startsWith("javax.");

static Predicate<JavaClass> IS_NOT_CORE_JAVA_TYPE = Predicate.not(IS_CORE_JAVA_TYPE);
}

static class JavaXTypes {

private static final String BASE_PACKAGE = "jakarta";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ private static Map<String, Object> toInfo(ApplicationModule module, ApplicationM
json.put("namedInterfaces", toNamedInterfaces(module.getNamedInterfaces()));
}

json.put("dependencies", module.getDependencies(modules).stream() //
json.put("dependencies", module.getDirectDependencies(modules).stream() //
.collect(Collectors.groupingBy(ApplicationModuleDependency::getTargetModule, MAPPER))
.entrySet() //
.stream() //
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,4 +44,15 @@ void sortsPackagesByNameAndDepth() {
.map(it -> it.getLocalName("com")))
.containsExactly("acme.b", "acme.a.second", "acme.a.first.one", "acme.a.first", "acme.a", "acme");
}

@Test // GH-802
void caculatesNestingCorrectly() {

var comAcme = new PackageName("com.acme");
var comAcmeA = new PackageName("com.acme.a");

assertThat(comAcme.contains(comAcme)).isTrue();
assertThat(comAcme.contains(comAcmeA)).isTrue();
assertThat(comAcmeA.contains(comAcme)).isFalse();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,7 @@ public String toAsciidoctor(String source) {
*/
public String renderBeanReferences(ApplicationModule module) {

var bullets = module.getDependencies(modules, DependencyType.USES_COMPONENT)
var bullets = module.getDirectDependencies(modules, DependencyType.USES_COMPONENT)
.uniqueStream(ApplicationModuleDependency::getTargetType)
.map(it -> "%s (in %s)".formatted(toInlineCode(it.getTargetType()), it.getTargetModule().getDisplayName()))
.map(this::toBulletPoint)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@

import org.springframework.lang.Nullable;
import org.springframework.modulith.core.ApplicationModule;
import org.springframework.modulith.core.ApplicationModuleDependency;
import org.springframework.modulith.core.ApplicationModules;
import org.springframework.modulith.core.DependencyDepth;
import org.springframework.modulith.core.DependencyType;
Expand Down Expand Up @@ -442,8 +441,7 @@ private void addDependencies(ApplicationModule module, Component component, Diag

DEPENDENCY_DESCRIPTIONS.entrySet().stream().forEach(entry -> {

module.getDependencies(modules, entry.getKey()).stream() //
.map(ApplicationModuleDependency::getTargetModule) //
module.getDirectDependencies(modules, entry.getKey()).uniqueModules() //
.map(it -> getComponents(options).get(it)) //
.map(it -> component.uses(it, entry.getValue())) //
.filter(it -> it != null) //
Expand Down Expand Up @@ -475,8 +473,7 @@ private void addComponentsToView(ApplicationModule module, ComponentView view, D
Supplier<Stream<ApplicationModule>> bootstrapDependencies = () -> module.getBootstrapDependencies(modules,
options.dependencyDepth);
Supplier<Stream<ApplicationModule>> otherDependencies = () -> options.getDependencyTypes()
.flatMap(it -> module.getDependencies(modules, it).stream()
.map(ApplicationModuleDependency::getTargetModule));
.flatMap(it -> module.getDirectDependencies(modules, it).uniqueModules());

Supplier<Stream<ApplicationModule>> dependencies = () -> Stream.concat(bootstrapDependencies.get(),
otherDependencies.get());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,10 +110,10 @@ void configrationPropertiesTypesEstablishSimpleDependency() {

assertThat(modules.getModuleByName("moduleD")).hasValueSatisfying(it -> {

assertThat(it.getDependencies(modules, DependencyType.DEFAULT))
assertThat(it.getDirectDependencies(modules, DependencyType.DEFAULT))
.matches(inner -> inner.containsModuleNamed("moduleC"));

assertThat(it.getDependencies(modules, DependencyType.USES_COMPONENT))
assertThat(it.getDirectDependencies(modules, DependencyType.USES_COMPONENT))
.matches(ApplicationModuleDependencies::isEmpty);
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ void moduleBListensToModuleA() {
ApplicationModule moduleA = modules.getModuleByName("moduleA").orElseThrow(IllegalStateException::new);

assertThat(module).hasValueSatisfying(it -> {
assertThat(it.getDependencies(modules, DependencyType.EVENT_LISTENER).contains(moduleA)).isTrue();
assertThat(it.getDirectDependencies(modules, DependencyType.EVENT_LISTENER).contains(moduleA)).isTrue();
});
}

Expand Down Expand Up @@ -261,6 +261,17 @@ void detectsContributedApplicationModules() {
}
}

@Test // GH-802
void detectsFullDependencyChain() {

assertThat(modules.getModuleByName("moduleC")).hasValueSatisfying(it -> {

assertThat(it.getAllDependencies(modules).uniqueModules())
.extracting(ApplicationModule::getName)
.containsExactlyInAnyOrder("moduleB", "moduleA");
});
}

private static void verifyNamedInterfaces(NamedInterfaces interfaces, String name, Class<?>... types) {

Stream.of(types).forEach(type -> {
Expand Down

0 comments on commit f09f3aa

Please sign in to comment.