GH-872 - Introduce proper identifiers for ApplicationModules.

Introduced ApplicationModuleIdentifier that ensures that identifiers chosen for application modules do not contain a double colon (::) as we need those as separator characters for manual dependency declarations referring to named interfaces.
This commit is contained in:
Oliver Drotbohm
2024-10-13 12:04:27 +02:00
parent 5733a27ff4
commit 68185a7aef
10 changed files with 194 additions and 43 deletions

View File

@@ -115,6 +115,15 @@ class AnnotationModulithMetadata implements ModulithMetadata {
return Arrays.stream(annotation.sharedModules());
}
/*
* (non-Javadoc)
* @see org.springframework.modulith.core.ModulithMetadata#getSharedModuleIdentifiers()
*/
@Override
public Stream<ApplicationModuleIdentifier> getSharedModuleIdentifiers() {
return getSharedModuleNames().map(ApplicationModuleIdentifier::of);
}
/*
* (non-Javadoc)
* @see org.springframework.modulith.model.ModulithMetadata#getSystemName()

View File

@@ -143,9 +143,21 @@ public class ApplicationModule implements Comparable<ApplicationModule> {
* Returns the logical name of the module.
*
* @return will never be {@literal null} or empty.
* @deprecated since 1.3, use {@link #getIdentifier()} instead.
*/
@Deprecated
public String getName() {
return source.getModuleName();
return getIdentifier().toString();
}
/**
* Returns the logical identifier of the module.
*
* @return will never be {@literal null}.
* @since 1.3
*/
public ApplicationModuleIdentifier getIdentifier() {
return ApplicationModuleIdentifier.of(source.getModuleName());
}
/**
@@ -306,7 +318,7 @@ public class ApplicationModule implements Comparable<ApplicationModule> {
return getType(type.getName())
.map(it -> ArchitecturallyEvidentType.of(it, getSpringBeansInternal()))
.orElseThrow(() -> new IllegalArgumentException("Couldn't find type %s in module %s!".formatted(
FormattableType.of(type).getAbbreviatedFullName(this), getName())));
FormattableType.of(type).getAbbreviatedFullName(this), getIdentifier())));
}
/**
@@ -404,11 +416,11 @@ public class ApplicationModule implements Comparable<ApplicationModule> {
if (modules != null) {
modules.getParentOf(this).ifPresent(it -> {
builder.append("> Parent module: ").append(it.getName()).append("\n");
builder.append("> Parent module: ").append(it.getIdentifier()).append("\n");
});
}
builder.append("> Logical name: ").append(getName()).append('\n');
builder.append("> Logical name: ").append(getIdentifier()).append('\n');
builder.append("> Base package: ").append(basePackage.getName()).append('\n');
builder.append("> Excluded packages: ");
@@ -438,8 +450,12 @@ public class ApplicationModule implements Comparable<ApplicationModule> {
List<ApplicationModule> dependencies = getBootstrapDependencies(modules).toList();
builder.append("> Direct module dependencies: ");
builder.append(dependencies.isEmpty() ? "none"
: dependencies.stream().map(ApplicationModule::getName).collect(Collectors.joining(", ")));
builder.append(dependencies.isEmpty()
? "none"
: dependencies.stream()
.map(ApplicationModule::getIdentifier)
.map(ApplicationModuleIdentifier::toString)
.collect(Collectors.joining(", ")));
builder.append('\n');
}
@@ -739,7 +755,7 @@ public class ApplicationModule implements Comparable<ApplicationModule> {
}
private String getQualifiedName(NamedInterface namedInterface) {
return namedInterface.getQualifiedName(getName());
return namedInterface.getQualifiedName(getIdentifier());
}
private Collection<ApplicationModule> doGetNestedModules(ApplicationModules modules, boolean recursive) {
@@ -877,7 +893,7 @@ public class ApplicationModule implements Comparable<ApplicationModule> {
var target = modules.getModuleByName(targetModuleName)
.orElseThrow(() -> new IllegalArgumentException(
INVALID_EXPLICIT_MODULE_DEPENDENCY.formatted(source.getName(), targetModuleName)));
INVALID_EXPLICIT_MODULE_DEPENDENCY.formatted(source.getIdentifier(), targetModuleName)));
if (WILDCARD.equals(namedInterfaceName)) {
return new DeclaredDependency(target, null);
@@ -888,7 +904,8 @@ public class ApplicationModule implements Comparable<ApplicationModule> {
? namedInterfaces.getUnnamedInterface()
: namedInterfaces.getByName(namedInterfaceName)
.orElseThrow(() -> new IllegalArgumentException(
INVALID_NAMED_INTERFACE_DECLARATION.formatted(namedInterfaceName, source.getName(), identifier)));
INVALID_NAMED_INTERFACE_DECLARATION.formatted(namedInterfaceName, source.getIdentifier(),
identifier)));
return new DeclaredDependency(target, namedInterface);
}
@@ -943,7 +960,7 @@ public class ApplicationModule implements Comparable<ApplicationModule> {
@Override
public String toString() {
var result = target.getName();
var result = target.getIdentifier().toString();
if (namedInterface == null) {
return result + " :: " + WILDCARD;
@@ -1230,14 +1247,14 @@ public class ApplicationModule implements Comparable<ApplicationModule> {
.toList();
var targetString = targetNamedInterfaces.isEmpty()
? "module '%s'".formatted(targetModule.getName())
? "module '%s'".formatted(targetModule.getIdentifier())
: "named interface(s) '%s'".formatted(
targetNamedInterfaces.stream()
.map(targetModule::getQualifiedName)
.collect(Collectors.joining(", ")));
var message = "Module '%s' depends on %s via %s -> %s. Allowed targets: %s." //
.formatted(originModule.getName(), targetString, source.getName(), target.getName(),
.formatted(originModule.getIdentifier(), targetString, source.getName(), target.getName(),
declaredDependencies.toString());
return violations.and(new Violation(message));
@@ -1256,7 +1273,7 @@ public class ApplicationModule implements Comparable<ApplicationModule> {
if (!targetModule.isExposed(target)) {
var violationText = INTERNAL_REFERENCE
.formatted(originModule.getName(), target.getName(), targetModule.getName());
.formatted(originModule.getIdentifier(), target.getName(), targetModule.getIdentifier());
return violations.and(new Violation(violationText + lineSeparator() + description));
}
@@ -1266,7 +1283,7 @@ public class ApplicationModule implements Comparable<ApplicationModule> {
if (!haveSameParentOrDirectParentRelationship(originModule, targetModule, modules)) {
var violationText = INVALID_SUB_MODULE_REFERENCE
.formatted(originModule.getName(), targetModule.getName(),
.formatted(originModule.getIdentifier(), targetModule.getIdentifier(),
FormattableType.of(source).getAbbreviatedFullName(originModule),
FormattableType.of(target).getAbbreviatedFullName(targetModule));

View File

@@ -85,7 +85,8 @@ public class ApplicationModuleDependencies {
Assert.hasText(name, "Module name must not be null or empty!");
return modules.stream()
.map(ApplicationModule::getName)
.map(ApplicationModule::getIdentifier)
.map(Object::toString)
.anyMatch(name::equals);
}

View File

@@ -0,0 +1,99 @@
/*
* Copyright 2024 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.springframework.modulith.core;
import java.util.Objects;
import org.springframework.util.Assert;
/**
* An identifier of an {@link ApplicationModule}.
*
* @author Oliver Drotbohm
* @since 1.3
*/
public class ApplicationModuleIdentifier implements Comparable<ApplicationModuleIdentifier> {
private final String identifier;
/**
* Creates a new {@link ApplicationModuleIdentifier} for the given {@link String}.
*
* @param identifier must not be {@literal null} .
*/
private ApplicationModuleIdentifier(String identifier) {
Assert.hasText(identifier, "Identitifier must not be null or empty!");
Assert.isTrue(!identifier.contains("::"), "Identifier must not contain a double-colon!");
this.identifier = identifier;
}
/**
* Returns the {@link ApplicationModuleIdentifier} for the given source {@link String}.
*
* @param identifier must not be {@literal null}, empty or contain a double colon ({@code ::}).
* @return will never be {@literal null}.
*/
public static ApplicationModuleIdentifier of(String identifier) {
return new ApplicationModuleIdentifier(identifier);
}
/*
* (non-Javadoc)
* @see java.lang.Comparable#compareTo(java.lang.Object)
*/
@Override
public int compareTo(ApplicationModuleIdentifier o) {
return identifier.compareTo(o.identifier);
}
/*
* (non-Javadoc)
* @see java.lang.Object#toString()
*/
@Override
public String toString() {
return identifier;
}
/*
* (non-Javadoc)
* @see java.lang.Object#equals(java.lang.Object)
*/
@Override
public boolean equals(Object obj) {
if (obj == this) {
return true;
}
if (!(obj instanceof ApplicationModuleIdentifier that)) {
return false;
}
return Objects.equals(this.identifier, that.identifier);
}
/*
* (non-Javadoc)
* @see java.lang.Object#hashCode()
*/
@Override
public int hashCode() {
return Objects.hash(identifier);
}
}

View File

@@ -81,12 +81,12 @@ public class ApplicationModules implements Iterable<ApplicationModule> {
}
private final ModulithMetadata metadata;
private final Map<String, ApplicationModule> modules;
private final Map<ApplicationModuleIdentifier, ApplicationModule> modules;
private final JavaClasses allClasses;
private final List<JavaPackage> rootPackages;
private final Supplier<List<ApplicationModule>> rootModules;
private final Set<ApplicationModule> sharedModules;
private final List<String> orderedNames;
private final List<ApplicationModuleIdentifier> orderedNames;
private boolean verified;
@@ -155,7 +155,7 @@ public class ApplicationModules implements Iterable<ApplicationModule> {
JavaPackages.onlySubPackagesOf(it.getModuleBasePackage(),
sources.stream().map(ApplicationModuleSource::getModuleBasePackage).toList())); //
})
.collect(toMap(ApplicationModule::getName, Function.identity()));
.collect(toMap(ApplicationModule::getIdentifier, Function.identity()));
this.rootPackages = Stream.concat(packages.stream(), contributions.getRootPackages()) //
.distinct()
@@ -168,8 +168,8 @@ public class ApplicationModules implements Iterable<ApplicationModule> {
this.sharedModules = Collections.emptySet();
Supplier<List<String>> fallback = () -> modules.values().stream()
.map(ApplicationModule::getName)
Supplier<List<ApplicationModuleIdentifier>> fallback = () -> modules.values().stream()
.map(ApplicationModule::getIdentifier)
.sorted()
.toList();
@@ -188,12 +188,12 @@ public class ApplicationModules implements Iterable<ApplicationModule> {
* @param rootPackages must not be {@literal null}.
* @param rootModules must not be {@literal null}.
* @param sharedModules must not be {@literal null}.
* @param orderedNames must not be {@literal null}.
* @param orderedIdentifiers must not be {@literal null}.
* @param verified
*/
private ApplicationModules(ModulithMetadata metadata, Map<String, ApplicationModule> modules, JavaClasses allClasses,
List<JavaPackage> rootPackages, Supplier<List<ApplicationModule>> rootModules,
Set<ApplicationModule> sharedModules, List<String> orderedNames, boolean verified) {
private ApplicationModules(ModulithMetadata metadata, Map<ApplicationModuleIdentifier, ApplicationModule> modules,
JavaClasses allClasses, List<JavaPackage> rootPackages, Supplier<List<ApplicationModule>> rootModules,
Set<ApplicationModule> sharedModules, List<ApplicationModuleIdentifier> orderedIdentifiers, boolean verified) {
Assert.notNull(metadata, "ModulithMetadata must not be null!");
Assert.notNull(modules, "Application modules must not be null!");
@@ -201,7 +201,7 @@ public class ApplicationModules implements Iterable<ApplicationModule> {
Assert.notNull(rootPackages, "Root JavaPackages must not be null!");
Assert.notNull(rootModules, "Root modules must not be null!");
Assert.notNull(sharedModules, "Shared ApplicationModules must not be null!");
Assert.notNull(orderedNames, "Ordered application module names must not be null!");
Assert.notNull(orderedIdentifiers, "Ordered application module identifiers must not be null!");
this.metadata = metadata;
this.modules = modules;
@@ -209,7 +209,7 @@ public class ApplicationModules implements Iterable<ApplicationModule> {
this.rootPackages = rootPackages;
this.rootModules = rootModules;
this.sharedModules = sharedModules;
this.orderedNames = orderedNames;
this.orderedNames = orderedIdentifiers;
this.verified = verified;
}
@@ -370,7 +370,7 @@ public class ApplicationModules implements Iterable<ApplicationModule> {
Assert.hasText(name, "Module name must not be null or empty!");
return Optional.ofNullable(modules.get(name));
return Optional.ofNullable(modules.get(ApplicationModuleIdentifier.of(name)));
}
/**
@@ -601,7 +601,7 @@ public class ApplicationModules implements Iterable<ApplicationModule> {
.map(it -> Class.class.isInstance(it) ? Class.class.cast(it) : it.getClass())
.map(Class::getName)
.flatMap(this::getModuleByType)
.map(ApplicationModule::getName)
.map(ApplicationModule::getIdentifier)
.map(orderedNames::indexOf)
.orElse(null);
}
@@ -612,12 +612,12 @@ public class ApplicationModules implements Iterable<ApplicationModule> {
* @param moduleName must not be {@literal null}.
* @return
*/
private ApplicationModule getRequiredModule(String moduleName) {
private ApplicationModule getRequiredModule(ApplicationModuleIdentifier identifier) {
var module = modules.get(moduleName);
var module = modules.get(identifier);
if (module == null) {
throw new IllegalArgumentException(String.format("Module %s does not exist!", moduleName));
throw new IllegalArgumentException(String.format("Module %s does not exist!", identifier));
}
return module;
@@ -648,7 +648,7 @@ public class ApplicationModules implements Iterable<ApplicationModule> {
var metadata = key.getMetadata();
var modules = new ApplicationModules(metadata, key.getIgnored(), key.getOptions());
var sharedModules = metadata.getSharedModuleNames() //
var sharedModules = metadata.getSharedModuleIdentifiers() //
.map(modules::getRequiredModule) //
.collect(Collectors.toSet());
@@ -764,7 +764,7 @@ public class ApplicationModules implements Iterable<ApplicationModule> {
private static class TopologicalSorter {
@Nullable
private static List<String> topologicallySortModules(ApplicationModules modules) {
private static List<ApplicationModuleIdentifier> topologicallySortModules(ApplicationModules modules) {
Graph<ApplicationModule, DefaultEdge> graph = new DefaultDirectedGraph<>(DefaultEdge.class);
@@ -782,12 +782,12 @@ public class ApplicationModules implements Iterable<ApplicationModule> {
});
});
var names = new ArrayList<String>();
var names = new ArrayList<ApplicationModuleIdentifier>();
var iterator = new TopologicalOrderIterator<>(graph);
try {
iterator.forEachRemaining(it -> names.add(0, it.getName()));
iterator.forEachRemaining(it -> names.add(0, it.getIdentifier()));
return names;
} catch (IllegalArgumentException o_O) {
@@ -807,7 +807,8 @@ public class ApplicationModules implements Iterable<ApplicationModule> {
return getModuleByType(javaClass)
.filter(Predicate.not(ApplicationModule::isOpen))
.map(ApplicationModule::getName)
.map(ApplicationModule::getIdentifier)
.map(ApplicationModuleIdentifier::toString)
.map(SliceIdentifier::of)
.orElse(SliceIdentifier.ignore());
}

View File

@@ -103,9 +103,20 @@ public interface ModulithMetadata {
* Returns the name of shared modules, i.e. modules that are supposed to always be included in bootstraps.
*
* @return will never be {@literal null}.
* @deprecated since 1.3, use {@link #getSharedModuleIdentifiers()} instead.
*/
@Deprecated
Stream<String> getSharedModuleNames();
/**
* Returns the {@link ApplicationModuleIdentifier}s of shared modules, i.e. modules that are supposed to always be
* included in bootstraps.
*
* @return will never be {@literal null}.
* @since 1.3
*/
Stream<ApplicationModuleIdentifier> getSharedModuleIdentifiers();
/**
* Returns the name of the system.
*

View File

@@ -133,7 +133,7 @@ public class NamedInterface implements Iterable<JavaClass> {
return name;
}
String getQualifiedName(String qualifier) {
String getQualifiedName(ApplicationModuleIdentifier qualifier) {
return qualifier + " :: " + name;
}

View File

@@ -132,6 +132,15 @@ class SpringBootModulithMetadata implements ModulithMetadata {
return Stream.empty();
}
/*
* (non-Javadoc)
* @see org.springframework.modulith.core.ModulithMetadata#getSharedModuleIdentifiers()
*/
@Override
public Stream<ApplicationModuleIdentifier> getSharedModuleIdentifiers() {
return Stream.empty();
}
/*
* (non-Javadoc)
* @see org.springframework.modulith.model.ModulithMetadata#getSystemName()

View File

@@ -32,6 +32,7 @@ import java.util.stream.Stream;
import org.springframework.modulith.core.ApplicationModule;
import org.springframework.modulith.core.ApplicationModuleDependency;
import org.springframework.modulith.core.ApplicationModuleIdentifier;
import org.springframework.modulith.core.ApplicationModules;
import org.springframework.modulith.core.DependencyType;
import org.springframework.modulith.core.NamedInterface;
@@ -119,11 +120,11 @@ public class ApplicationModulesExporter {
return Json.toString(toMap(Details.FULL));
}
private Map<String, Object> toMap(Details details) {
private Map<ApplicationModuleIdentifier, Object> toMap(Details details) {
return modules.stream()
.collect(
Collectors.toMap(ApplicationModule::getName, it -> toInfo(it, modules, details), (l, r) -> r,
Collectors.toMap(ApplicationModule::getIdentifier, it -> toInfo(it, modules, details), (l, r) -> r,
LinkedHashMap::new));
}
@@ -157,7 +158,7 @@ public class ApplicationModulesExporter {
private static Map<String, Object> toInfo(Entry<ApplicationModule, ? extends Set<DependencyType>> types) {
return Map.of( //
"target", types.getKey().getName(), //
"target", types.getKey().getIdentifier().toString(), //
"types", types.getValue() //
);
}

View File

@@ -32,7 +32,8 @@ class ApplicationModulesUnitTests {
void discoversComplexModuleArrangement() {
assertThat(modules)
.extracting(ApplicationModule::getName)
.extracting(ApplicationModule::getIdentifier)
.extracting(Object::toString)
.containsExactlyInAnyOrder(
"invalid",
"jmolecules",
@@ -55,11 +56,13 @@ class ApplicationModulesUnitTests {
assertThat(ni.getParentModule(modules)).isEmpty();
assertThat(ni.getDirectlyNestedModules(modules))
.extracting(ApplicationModule::getName)
.extracting(ApplicationModule::getIdentifier)
.extracting(Object::toString)
.containsExactlyInAnyOrder("ni.nested");
assertThat(ni.getNestedModules(modules))
.extracting(ApplicationModule::getName)
.extracting(ApplicationModule::getIdentifier)
.extracting(Object::toString)
.containsExactlyInAnyOrder("ni.nested", "ni.nested.b.first", "ni.nested.b.second");
}