GH-36 - Better dependency verification for named interfaces.

We now verify detected dependencies against optionally explicitly defined ones. Those can refer to named interfaces (via the "moduleName :: namedInterfaceName" syntax). If such an allowed dependency is defined, all dependencies towards other named interfaces (also the unnamed one) are rejected.

Some Javadoc polishing.
This commit is contained in:
Oliver Drotbohm
2022-08-09 16:04:22 +02:00
parent fb23c452cd
commit 7e890fd705
12 changed files with 270 additions and 73 deletions

View File

@@ -22,20 +22,29 @@ import java.lang.annotation.Target;
/**
* Annotation to customize information of a {@link Modulith} module.
*
*
* @author Oliver Drotbohm
*/
@Target({ ElementType.PACKAGE, ElementType.ANNOTATION_TYPE })
@Retention(RetentionPolicy.RUNTIME)
public @interface ApplicationModule {
/**
* The human readable name of the module to be used for display and documentation purposes.
*
* @return
*/
String displayName() default "";
/**
* List the names of modules that the module is allowed to depend on. Shared modules defined in {@link Modulith} will
* be allowed, too.
*
* be allowed, too. Names listed are local ones, unless the application has configured
* {@link Modulithic#useFullyQualifiedModuleNames()} to {@literal true}. Explicit references to
* {@link NamedInterface}s need to be separated by a double colon {@code ::}, e.g. {@code module::API} if
* {@code module} is the logical module name and {@code API} is the name of the named interface.
*
* @return
* @see NamedInterface
*/
String[] allowedDependencies() default {};
}

View File

@@ -21,8 +21,11 @@ import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;
import org.springframework.core.annotation.AliasFor;
/**
* Annotation to mark a package as named interface of a {@link ApplicationModule} (either implicit or explicitly annotated).
* Annotation to mark a package as named interface of a {@link ApplicationModule} (either implicit or explicitly
* annotated).
*
* @author Oliver Drotbohm
*/
@@ -32,9 +35,20 @@ import java.lang.annotation.Target;
public @interface NamedInterface {
/**
* The name of the interface.
* The name(s) of the named interface. Declaring multiple values here is useful in case named interfaces are defined
* based on types and a particular type is supposed to be part of multiple named interfaces.
*
* @return
*/
String[] value();
@AliasFor("name")
String[] value() default {};
/**
* The name(s) of the named interface. Declaring multiple values here is useful in case named interfaces are defined
* based on types and a particular type is supposed to be part of multiple named interfaces.
*
* @return
*/
@AliasFor("value")
String[] name() default {};
}

View File

@@ -23,10 +23,12 @@ import static org.springframework.modulith.model.Types.JavaXTypes.*;
import static org.springframework.modulith.model.Types.SpringDataTypes.*;
import static org.springframework.modulith.model.Types.SpringTypes.*;
import lombok.AccessLevel;
import lombok.EqualsAndHashCode;
import lombok.Getter;
import lombok.NonNull;
import lombok.RequiredArgsConstructor;
import lombok.Value;
import java.util.Arrays;
import java.util.Collection;
@@ -197,7 +199,7 @@ public class ApplicationModule {
}
public boolean contains(@Nullable Class<?> type) {
return (type != null) && getType(type.getName()).isPresent();
return type != null && getType(type.getName()).isPresent();
}
/**
@@ -298,23 +300,25 @@ public class ApplicationModule {
* @param modules must not be {@literal null}.
* @return
*/
List<ApplicationModule> getAllowedDependencies(ApplicationModules modules) {
ApplicationModuleDependencies getAllowedDependencies(ApplicationModules modules) {
Assert.notNull(modules, "Modules must not be null!");
List<String> allowedDependencyNames = information.getAllowedDependencies();
var allowedDependencyNames = information.getAllowedDependencies();
if (allowedDependencyNames.isEmpty()) {
return Collections.emptyList();
return new ApplicationModuleDependencies(Collections.emptyList());
}
Stream<ApplicationModule> explicitlyDeclaredModules = allowedDependencyNames.stream() //
.map(modules::getModuleByName) //
.flatMap(it -> it.map(Stream::of).orElse(Stream.empty()));
var explicitlyDeclaredModules = allowedDependencyNames.stream() //
.map(it -> ApplicationModuleDependency.of(it, modules));
return Stream.concat(explicitlyDeclaredModules, modules.getSharedModules().stream()) //
var sharedDependencies = modules.getSharedModules().stream()
.map(ApplicationModuleDependency::to);
return Stream.concat(explicitlyDeclaredModules, sharedDependencies) //
.distinct() //
.collect(Collectors.toList());
.collect(Collectors.collectingAndThen(Collectors.toList(), ApplicationModuleDependencies::new));
}
/**
@@ -444,6 +448,106 @@ public class ApplicationModule {
ALL;
}
@Value
@RequiredArgsConstructor(access = AccessLevel.PRIVATE)
static class ApplicationModuleDependency {
@NonNull ApplicationModule target;
@NonNull NamedInterface namedInterface;
/**
* Creates an {@link ApplicationModuleDependency} to the module and optionally named interface defined by the given
* identifier.
*
* @param identifier must not be {@literal null} or empty. Follows the
* {@code ${moduleName}(::${namedInterfaceName})} pattern.
* @param modules must not be {@literal null}.
* @return will never be {@literal null}.
* @throws IllegalArgumentException in case the given identifier is invalid, i.e. does not refer to an existing
* module or named interface.
*/
public static ApplicationModuleDependency of(String identifier, ApplicationModules modules) {
Assert.hasText(identifier, "Module dependency identifier must not be null or empty!");
var segments = identifier.split("::");
var moduleName = segments[0].trim();
var namedInterfacename = segments.length > 1 ? segments[1].trim() : null;
var module = modules.getModuleByName(moduleName)
.orElseThrow(() -> new IllegalArgumentException("No module named %s found!".formatted(moduleName)));
var namedInterface = namedInterfacename == null
? module.getNamedInterfaces().getUnnamedInterface()
: module.getNamedInterfaces().getByName(segments[1])
.orElseThrow(
() -> new IllegalArgumentException(
"No named interface named %s found!".formatted(namedInterfacename)));
return new ApplicationModuleDependency(module, namedInterface);
}
/**
* Creates a new {@link ApplicationModuleDependency} to the unnamed interface of the given
* {@link ApplicationModule}.
*
* @param module must not be {@literal null}.
* @return
*/
public static ApplicationModuleDependency to(ApplicationModule module) {
return new ApplicationModuleDependency(module, module.getNamedInterfaces().getUnnamedInterface());
}
public boolean contains(JavaClass type) {
return namedInterface.contains(type);
}
@Override
public String toString() {
return namedInterface.isUnnamed() //
? target.getName() //
: target.getName() + "::" + namedInterface.getName();
}
}
/**
* A collection wrapper for {@link ApplicationModuleDependency} instances.
*
* @author Oliver Drotbohm
*/
@Value
static class ApplicationModuleDependencies {
List<ApplicationModuleDependency> dependencies;
/**
* Returns whether any of the dependencies contains the given {@link JavaClass}.
*
* @param type must not be {@literal null}.
* @return
*/
public boolean contains(JavaClass type) {
Assert.notNull(type, "JavaClass must not be null!");
return dependencies.stream() //
.anyMatch(it -> it.contains(type));
}
public boolean isEmpty() {
return dependencies.isEmpty();
}
@Override
public String toString() {
return dependencies.stream() //
.map(ApplicationModuleDependency::toString)
.collect(Collectors.joining(", "));
}
}
@EqualsAndHashCode
@RequiredArgsConstructor
static class ModuleDependency {
@@ -468,30 +572,31 @@ public class ApplicationModule {
Violations isValidDependencyWithin(ApplicationModules modules) {
ApplicationModule originModule = getExistingModuleOf(origin, modules);
ApplicationModule targetModule = getExistingModuleOf(target, modules);
var originModule = getExistingModuleOf(origin, modules);
var targetModule = getExistingModuleOf(target, modules);
List<ApplicationModule> allowedTargets = originModule.getAllowedDependencies(modules);
ApplicationModuleDependencies allowedTargets = originModule.getAllowedDependencies(modules);
Violations violations = Violations.NONE;
if (!allowedTargets.isEmpty() && !allowedTargets.contains(targetModule)) {
// Check explicitly defined allowed targets
String allowedTargetsString = allowedTargets.stream() //
.map(ApplicationModule::getName) //
.collect(Collectors.joining(", "));
if (!allowedTargets.isEmpty() && !allowedTargets.contains(target)) {
String message = String.format("Module '%s' depends on module '%s' via %s -> %s. Allowed target modules: %s.",
originModule.getName(), targetModule.getName(), origin.getName(), target.getName(), allowedTargetsString);
var message = "Module '%s' depends on module '%s' via %s -> %s. Allowed targets: %s." //
.formatted(originModule.getName(), targetModule.getName(), origin.getName(), target.getName(),
allowedTargets.toString());
violations = violations.and(new IllegalStateException(message));
return violations.and(new IllegalStateException(message));
}
// No explicitly allowed dependencies - check for general access
if (!targetModule.isExposed(target)) {
String violationText = String.format("Module '%s' depends on non-exposed type %s within module '%s'!",
originModule.getName(), target.getName(), targetModule.getName());
var violationText = "Module '%s' depends on non-exposed type %s within module '%s'!"
.formatted(originModule.getName(), target.getName(), targetModule.getName());
violations = violations.and(new IllegalStateException(violationText + lineSeparator() + description));
return violations.and(new IllegalStateException(violationText + lineSeparator() + description));
}
return violations;
@@ -533,7 +638,7 @@ public class ApplicationModule {
Set<JavaConstructor> constructors = source.getConstructors();
return constructors.stream() //
.filter(it -> (constructors.size() == 1) || isInjectionPoint(it)) //
.filter(it -> constructors.size() == 1 || isInjectionPoint(it)) //
.flatMap(it -> it.getRawParameterTypes().stream() //
.map(parameter -> new InjectionModuleDependency(source, parameter, it)));
}

View File

@@ -17,8 +17,6 @@ package org.springframework.modulith.model;
import java.util.stream.Stream;
import org.springframework.modulith.ApplicationModule;
/**
* Strategy interface to customize which packages are considered module base packages.
*

View File

@@ -30,6 +30,8 @@ import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import org.springframework.core.annotation.AnnotatedElementUtils;
import com.tngtech.archunit.base.DescribedIterable;
import com.tngtech.archunit.base.DescribedPredicate;
import com.tngtech.archunit.core.domain.JavaClass;
@@ -143,7 +145,8 @@ public class JavaPackage implements DescribedIterable<JavaClass> {
return packageClasses.that(JavaClass.Predicates.simpleName(PACKAGE_INFO_NAME) //
.and(CanBeAnnotated.Predicates.annotatedWith(annotationType))) //
.toOptional() //
.map(it -> it.getAnnotationOfType(annotationType));
.map(it -> it.reflect())
.map(it -> AnnotatedElementUtils.getMergedAnnotation(it, annotationType));
}
/*

View File

@@ -50,7 +50,7 @@ public abstract class NamedInterface implements Iterable<JavaClass> {
public static List<PackageBasedNamedInterface> of(JavaPackage javaPackage) {
String[] name = javaPackage.getAnnotation(org.springframework.modulith.NamedInterface.class) //
.map(it -> it.value()) //
.map(it -> it.name()) //
.orElseThrow(() -> new IllegalArgumentException(
String.format("Couldn't find NamedInterface annotation on package %s!", javaPackage)));

View File

@@ -26,6 +26,7 @@ import java.util.Optional;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import org.springframework.core.annotation.AnnotatedElementUtils;
import org.springframework.modulith.model.NamedInterface.TypeBasedNamedInterface;
import org.springframework.util.LinkedMultiValueMap;
import org.springframework.util.MultiValueMap;
@@ -46,7 +47,7 @@ public class NamedInterfaces implements Iterable<NamedInterface> {
return NamedInterfaces.ofAnnotatedPackages(basePackage) //
.and(NamedInterfaces.ofAnnotatedTypes(basePackage)) //
.orUnnamed(basePackage);
.and(NamedInterface.unnamed(basePackage));
}
public static NamedInterfaces of(List<NamedInterface> interfaces) {
@@ -73,10 +74,10 @@ public class NamedInterfaces implements Iterable<NamedInterface> {
return;
}
org.springframework.modulith.NamedInterface annotation = it
.getAnnotationOfType(org.springframework.modulith.NamedInterface.class);
org.springframework.modulith.NamedInterface annotation = AnnotatedElementUtils
.getMergedAnnotation(it.reflect(), org.springframework.modulith.NamedInterface.class);
for (String name : annotation.value()) {
for (String name : annotation.name()) {
mappings.add(name, it);
}
});
@@ -86,6 +87,15 @@ public class NamedInterfaces implements Iterable<NamedInterface> {
.collect(Collectors.toList());
}
private NamedInterfaces and(NamedInterface namedInterface) {
List<NamedInterface> result = new ArrayList<>(namedInterfaces.size() + 1);
result.addAll(namedInterfaces);
result.add(namedInterface);
return new NamedInterfaces(result);
}
public boolean hasExplicitInterfaces() {
return namedInterfaces.size() > 1 || !namedInterfaces.get(0).isUnnamed();
}
@@ -123,16 +133,23 @@ public class NamedInterfaces implements Iterable<NamedInterface> {
return new NamedInterfaces(namedInterfaces);
}
public NamedInterfaces orUnnamed(JavaPackage basePackage) {
return namedInterfaces.isEmpty() //
? of(Collections.singletonList(NamedInterface.unnamed(basePackage))) //
: this;
}
public Optional<NamedInterface> getByName(String name) {
return namedInterfaces.stream().filter(it -> it.getName().equals(name)).findFirst();
}
/**
* Returns the unnamed {@link NamedInterface} of the module.
*
* @return will never be {@literal null}.
*/
public NamedInterface getUnnamedInterface() {
return namedInterfaces.stream() //
.filter(NamedInterface::isUnnamed) //
.findFirst() //
.orElseThrow(() -> new IllegalStateException("No unnamed interface found!"));
}
/*
* (non-Javadoc)
* @see java.lang.Iterable#iterator()

View File

@@ -0,0 +1,34 @@
/*
* Copyright 2018-2022 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 com.acme.myproject.invalid3;
import com.acme.myproject.complex.spi.ComplexSpiComponent;
/**
* @author Oliver Drotbohm
*/
public class InvalidModuleDependency {
/**
* The dependency is invalid as the module declaration only allows dependencies to the named interface API within the
* complex module.
*
* @param dependency
*/
public InvalidModuleDependency(ComplexSpiComponent dependency) {
}
}

View File

@@ -0,0 +1,4 @@
@ApplicationModule(allowedDependencies = "complex::API")
package com.acme.myproject.invalid3;
import org.springframework.modulith.ApplicationModule;

View File

@@ -36,7 +36,7 @@ import com.tngtech.archunit.core.domain.JavaClass;
class ModulithTest {
static final DescribedPredicate<JavaClass> DEFAULT_EXCLUSIONS = Filters.withoutModules("cycleA", "cycleB", "invalid2",
"fieldinjected");
"invalid3", "fieldinjected");
@Test
void verifyModules() {
@@ -60,7 +60,8 @@ class ModulithTest {
void detectsCycleBetweenModules() {
assertThatExceptionOfType(Violations.class) //
.isThrownBy(() -> ApplicationModules.of(Application.class, Filters.withoutModules("invalid", "invalid2")).verify()) //
.isThrownBy(
() -> ApplicationModules.of(Application.class, Filters.withoutModules("invalid", "invalid2")).verify()) //
// mentions modules
.withMessageContaining("cycleA") //

View File

@@ -17,10 +17,11 @@ package com.acme.myproject.complex;
import static org.assertj.core.api.Assertions.*;
import java.util.List;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.modulith.model.NamedInterface;
import org.springframework.modulith.model.NamedInterfaces;
import org.springframework.modulith.test.ModuleTestExecution;
import com.acme.myproject.NonVerifyingModuleTest;
@@ -36,9 +37,14 @@ class ComplexTest {
@Test
void exposesNamedInterfaces() {
NamedInterfaces interfaces = moduleTest.getModule().getNamedInterfaces();
var interfaces = moduleTest.getModule().getNamedInterfaces();
var expectedNames = List.of("API", "SPI", "Port 1", "Port 2", "Port 3");
assertThat(interfaces.stream().map(NamedInterface::getName)) //
.containsExactlyInAnyOrder("API", "SPI", "Port 1", "Port 2", "Port 3");
assertThat(interfaces) //
.extracting(NamedInterface::getName) //
.hasSize(expectedNames.size() + 1)
.containsAll(expectedNames);
assertThatNoException().isThrownBy(() -> interfaces.getUnnamedInterface());
}
}

View File

@@ -17,6 +17,7 @@ package org.springframework.modulith.model;
import static org.assertj.core.api.Assertions.*;
import java.util.List;
import java.util.Optional;
import java.util.stream.Collectors;
import java.util.stream.Stream;
@@ -45,9 +46,7 @@ class ModulesIntegrationTest {
@Test
void exposesModulesForPrimaryPackages() {
Optional<ApplicationModule> module = modules.getModuleByName("moduleB");
assertThat(module).hasValueSatisfying(it -> {
assertThat(modules.getModuleByName("moduleB")).hasValueSatisfying(it -> {
assertThat(it.getBootstrapDependencies(modules)).anySatisfy(dep -> {
assertThat(dep.getName()).isEqualTo("moduleA");
});
@@ -55,17 +54,15 @@ class ModulesIntegrationTest {
}
@Test
public void usesExplicitlyAnnotatedDisplayName() {
void usesExplicitlyAnnotatedDisplayName() {
Optional<ApplicationModule> module = modules.getModuleByName("moduleC");
assertThat(module).hasValueSatisfying(it -> {
assertThat(modules.getModuleByName("moduleC")).hasValueSatisfying(it -> {
assertThat(it.getDisplayName()).isEqualTo("MyModule C");
});
}
@Test
public void rejectsDependencyIntoInternalPackage() {
void rejectsDependencyIntoInternalPackage() {
Optional<ApplicationModule> module = modules.getModuleByName("invalid");
@@ -76,16 +73,16 @@ class ModulesIntegrationTest {
}
@Test
public void complexModuleExposesNamedInterfaces() {
void complexModuleExposesNamedInterfaces() {
Optional<ApplicationModule> module = modules.getModuleByName("complex");
assertThat(modules.getModuleByName("complex")).hasValueSatisfying(it -> {
assertThat(module).hasValueSatisfying(it -> {
var interfaces = it.getNamedInterfaces();
var reference = List.of("API", "SPI", "Port 1", "Port 2", "Port 3");
NamedInterfaces interfaces = it.getNamedInterfaces();
assertThat(interfaces.stream().map(NamedInterface::getName)) //
.containsExactlyInAnyOrder("API", "SPI", "Port 1", "Port 2", "Port 3");
assertThat(interfaces).extracting(NamedInterface::getName) //
.hasSize(reference.size() + 1) //
.containsAll(reference);
verifyNamedInterfaces(interfaces, "Port 1", FirstTypeBasedPort.class, SecondTypeBasePort.class);
verifyNamedInterfaces(interfaces, "Port 2", FirstTypeBasedPort.class, SecondTypeBasePort.class);
@@ -93,17 +90,18 @@ class ModulesIntegrationTest {
});
}
private static void verifyNamedInterfaces(NamedInterfaces interfaces, String name, Class<?>... types) {
@Test
void detectsReferenceToUndeclaredNamedInterface() {
Optional<NamedInterface> byName = interfaces.getByName(name);
Stream.of(types).forEach(type -> {
assertThat(byName).hasValueSatisfying(named -> named.contains(type));
assertThat(modules.getModuleByName("invalid3")).hasValueSatisfying(it -> {
assertThatExceptionOfType(Violations.class).isThrownBy(() -> it.verifyDependencies(modules))
.withMessageContaining("Allowed targets")
.withMessageContaining("complex::API");
});
}
@Test
public void discoversAtBeanComponent() {
void discoversAtBeanComponent() {
Optional<ApplicationModule> module = modules.getModuleByName("moduleA");
@@ -113,7 +111,7 @@ class ModulesIntegrationTest {
}
@Test
public void moduleBListensToModuleA() {
void moduleBListensToModuleA() {
Optional<ApplicationModule> module = modules.getModuleByName("moduleB");
ApplicationModule moduleA = modules.getModuleByName("moduleA").orElseThrow(IllegalStateException::new);
@@ -125,7 +123,7 @@ class ModulesIntegrationTest {
}
@Test
public void rejectsNotExplicitlyListedDependency() {
void rejectsNotExplicitlyListedDependency() {
Optional<ApplicationModule> moduleByName = modules.getModuleByName("invalid2");
@@ -150,6 +148,14 @@ class ModulesIntegrationTest {
ApplicationModules fromPackage = ApplicationModules.of(Application.class.getPackage().getName());
assertThat(fromPackage.stream().map(ApplicationModule::getName)) //
.containsExactlyInAnyOrderElementsOf(modules.stream().map(ApplicationModule::getName).collect(Collectors.toList()));
.containsExactlyInAnyOrderElementsOf(
modules.stream().map(ApplicationModule::getName).collect(Collectors.toList()));
}
private static void verifyNamedInterfaces(NamedInterfaces interfaces, String name, Class<?>... types) {
Stream.of(types).forEach(type -> {
assertThat(interfaces.getByName(name)).hasValueSatisfying(named -> named.contains(type));
});
}
}