GH-317 - Detect violations from types located in root packages.

odrotbohm · odrotbohm · commit 9148a57b02b9 · 2023-10-16T07:57:16.000+02:00
We now create artificial root application modules for all root packages to detect violations (for example, types located in root packages referring to module-internal types).
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
@@ -317,6 +317,28 @@ public Violations detectDependencies(ApplicationModules modules) {
 				.reduce(Violations.NONE, Violations::and);
 	}
 
+	/**
+	 * Returns whether the module is considered a root one, i.e., it is an artificial one created for each base package
+	 * configured.
+	 * 
+	 * @return whether the module is considered a root one.
+	 * @since 1.1
+	 */
+	public boolean isRootModule() {
+		return false;
+	}
+
+	/**
+	 * Returns whether the module has a base package with the given name.
+	 *
+	 * @param candidate must not be {@literal null} or empty.
+	 * @return whether the module has a base package with the given name.
+	 * @since 1.1
+	 */
+	boolean hasBasePackage(String candidate) {
+		return basePackage.getName().equals(candidate);
+	}
+
 	/*
 	 * (non-Javadoc)
 	 * @see java.lang.Object#toString()
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
@@ -22,6 +22,7 @@
 import java.util.*;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.function.Function;
+import java.util.function.Supplier;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 import java.util.stream.StreamSupport;
@@ -31,16 +32,13 @@
 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.core.Types.JMoleculesTypes;
 import org.springframework.util.Assert;
 import org.springframework.util.ClassUtils;
+import org.springframework.util.function.SingletonSupplier;
 
 import com.tngtech.archunit.base.DescribedPredicate;
 import com.tngtech.archunit.core.domain.JavaClass;
@@ -86,6 +84,7 @@ public class ApplicationModules implements Iterable<ApplicationModule> {
 	private final Map<String, 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;
 
@@ -112,6 +111,10 @@ protected ApplicationModules(ModulithMetadata metadata, Collection<String> packa
 				.map(it -> JavaPackage.of(classes, it).toSingle()) //
 				.toList();
 
+		this.rootModules = SingletonSupplier.of(() -> rootPackages.stream()
+				.map(ApplicationModules::rootModuleFor)
+				.toList());
+
 		this.sharedModules = Collections.emptySet();
 
 		this.orderedNames = JGRAPHT_PRESENT //
@@ -121,32 +124,35 @@ protected ApplicationModules(ModulithMetadata metadata, Collection<String> packa
 
 	/**
 	 * Creates a new {@link ApplicationModules} for the given {@link ModulithMetadata}, {@link ApplicationModule}s,
-	 * {@link JavaClasses}, {@link JavaPackage}s, shared {@link ApplicationModule}s, ordered module names and verified
-	 * flag.
+	 * {@link JavaClasses}, {@link JavaPackage}s, root and shared {@link ApplicationModule}s, ordered module names and
+	 * verified flag.
 	 *
 	 * @param metadata must not be {@literal null}.
 	 * @param modules must not be {@literal null}.
 	 * @param allClasses must not be {@literal null}.
 	 * @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 verified
 	 */
 	private ApplicationModules(ModulithMetadata metadata, Map<String, ApplicationModule> modules, JavaClasses classes,
-			List<JavaPackage> rootPackages, Set<ApplicationModule> sharedModules, List<String> orderedNames,
-			boolean verified) {
+			List<JavaPackage> rootPackages, Supplier<List<ApplicationModule>> rootModules,
+			Set<ApplicationModule> sharedModules, List<String> orderedNames, boolean verified) {
 
 		Assert.notNull(metadata, "ModulithMetadata must not be null!");
 		Assert.notNull(modules, "Application modules must not be null!");
 		Assert.notNull(classes, "JavaClasses must not be null!");
 		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!");
 
 		this.metadata = metadata;
 		this.modules = modules;
 		this.allClasses = classes;
 		this.rootPackages = rootPackages;
+		this.rootModules = rootModules;
 		this.sharedModules = sharedModules;
 		this.orderedNames = orderedNames;
 		this.verified = verified;
@@ -296,7 +302,7 @@ public Optional<ApplicationModule> getModuleByType(JavaClass type) {
 
 		Assert.notNull(type, "Type must not be null!");
 
-		return modules.values().stream() //
+		return allModules() //
 				.filter(it -> it.contains(type)) //
 				.findFirst();
 	}
@@ -311,7 +317,7 @@ public Optional<ApplicationModule> getModuleByType(String candidate) {
 
 		Assert.hasText(candidate, "Candidate must not be null or empty!");
 
-		return modules.values().stream() //
+		return allModules() //
 				.filter(it -> it.contains(candidate)) //
 				.findFirst();
 	}
@@ -328,15 +334,20 @@ public Optional<ApplicationModule> getModuleByType(Class<?> candidate) {
 
 	/**
 	 * Returns the {@link ApplicationModule} containing the given package.
-	 * 
+	 *
 	 * @param name must not be {@literal null} or empty.
 	 * @return will never be {@literal null}.
 	 */
 	public Optional<ApplicationModule> getModuleForPackage(String name) {
 
 		return modules.values().stream() //
 				.filter(it -> it.containsPackage(name)) //
-				.findFirst();
+				.findFirst()
+				.or(() -> {
+					return rootModules.get().stream()
+							.filter(it -> it.hasBasePackage(name))
+							.findFirst();
+				});
 	}
 
 	/**
@@ -383,7 +394,7 @@ public Violations detectViolations() {
 			}
 		}
 
-		return modules.values().stream() //
+		return Stream.concat(rootModules.get().stream(), modules.values().stream()) //
 				.map(it -> it.detectDependencies(this)) //
 				.reduce(violations, Violations::and);
 	}
@@ -458,7 +469,8 @@ public String toString() {
 	}
 
 	private ApplicationModules withSharedModules(Set<ApplicationModule> sharedModules) {
-		return new ApplicationModules(metadata, modules, allClasses, rootPackages, sharedModules, orderedNames, verified);
+		return new ApplicationModules(metadata, modules, allClasses, rootPackages, rootModules, sharedModules, orderedNames,
+				verified);
 	}
 
 	private FailureReport assertNoCyclesFor(JavaPackage rootPackage) {
@@ -506,6 +518,16 @@ private ApplicationModule getRequiredModule(String moduleName) {
 		return module;
 	}
 
+	/**
+	 * Returns of all {@link ApplicationModule}s, including root ones (last).
+	 *
+	 * @return will never be {@literal null}.
+	 * @since 1.1
+	 */
+	private Stream<ApplicationModule> allModules() {
+		return Stream.concat(modules.values().stream(), rootModules.get().stream());
+	}
+
 	/**
 	 * Creates a new {@link ApplicationModules} instance for the given {@link CacheKey}.
 	 *
@@ -532,6 +554,29 @@ private static ApplicationModules of(CacheKey key) {
 		return modules.withSharedModules(sharedModules);
 	}
 
+	/**
+	 * Creates a special root {@link ApplicationModule} for the given {@link JavaPackage}.
+	 *
+	 * @param javaPackage must not be {@literal null}.
+	 * @return will never be {@literal null}.
+	 * @since 1.1
+	 */
+	private static ApplicationModule rootModuleFor(JavaPackage javaPackage) {
+
+		return new ApplicationModule(javaPackage, true) {
+
+			@Override
+			public String getName() {
+				return "root:" + super.getName();
+			}
+
+			@Override
+			public boolean isRootModule() {
+				return true;
+			}
+		};
+	}
+
 	public static class Filters {
 
 		public static DescribedPredicate<JavaClass> withoutModules(String... names) {
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
@@ -20,7 +20,6 @@
 import java.util.List;
 import java.util.stream.Collector;
 import java.util.stream.Collectors;
-import java.util.stream.Stream;
 
 import org.springframework.util.Assert;
 
@@ -70,6 +69,19 @@ public String getMessage() {
 				.collect(Collectors.joining("\n- ", "- ", ""));
 	}
 
+	/**
+	 * Returns all violations' messages.
+	 *
+	 * @return will never be {@literal null}.
+	 * @since 1.1
+	 */
+	public List<String> getMessages() {
+
+		return exceptions.stream() //
+				.map(RuntimeException::getMessage)
+				.toList();
+	}
+
 	/**
 	 * Returns whether there are violations available.
 	 *
diff --git a/spring-modulith-integration-test/src/main/java/com/acme/myproject/CentralConfiguration.java b/spring-modulith-integration-test/src/main/java/com/acme/myproject/CentralConfiguration.java
@@ -0,0 +1,29 @@
+/*
+ * Copyright 2023 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;
+
+import org.springframework.stereotype.Component;
+
+import com.acme.myproject.moduleB.internal.InternalComponentB;
+
+/**
+ *
+ * @author Oliver Drotbohm
+ */
+@Component
+class CentralConfiguration {
+	private InternalComponentB referenceToModuleInternalComponent;
+}
diff --git a/spring-modulith-integration-test/src/test/java/com/acme/myproject/ModulithTest.java b/spring-modulith-integration-test/src/test/java/com/acme/myproject/ModulithTest.java
@@ -17,6 +17,7 @@
 
 import static org.assertj.core.api.Assertions.*;
 
+import org.assertj.core.api.InstanceOfAssertFactories;
 import org.junit.jupiter.api.Test;
 import org.springframework.modulith.core.ApplicationModule;
 import org.springframework.modulith.core.ApplicationModuleDependencies;
@@ -56,7 +57,22 @@ void verifyModules() {
 
 	@Test
 	void verifyModulesWithoutInvalid() {
-		ApplicationModules.of(Application.class, DEFAULT_EXCLUSIONS.or(Filters.withoutModule("invalid"))).verify();
+
+		assertThatExceptionOfType(Violations.class).isThrownBy(() -> {
+
+			ApplicationModules
+					.of(Application.class, DEFAULT_EXCLUSIONS.or(Filters.withoutModule("invalid")))
+					.verify();
+
+		}).satisfies(it -> {
+
+			assertThat(it.getMessages())
+					.hasSize(1)
+					.element(0, as(InstanceOfAssertFactories.STRING))
+					.contains("root:com.acme.myproject")
+					.contains(InternalComponentB.class.getName());
+		});
+
 	}
 
 	@Test
diff --git a/spring-modulith-test/src/main/java/org/springframework/modulith/test/ModuleContextCustomizerFactory.java b/spring-modulith-test/src/main/java/org/springframework/modulith/test/ModuleContextCustomizerFactory.java
@@ -18,6 +18,7 @@
 import java.util.Arrays;
 import java.util.List;
 import java.util.Objects;
+import java.util.function.Predicate;
 import java.util.function.Supplier;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
@@ -191,14 +192,14 @@ public int hashCode() {
 	 * @since 1.1
 	 */
 	private static class ModuleTestExecutionBeanDefinitionSelector implements BeanDefinitionRegistryPostProcessor {
-		
+
 		private static final Logger LOGGER = LoggerFactory.getLogger(ModuleTestExecutionBeanDefinitionSelector.class);
 
 		private final ModuleTestExecution execution;
 
 		/**
 		 * Creates a new {@link ModuleTestExecutionBeanDefinitionSelector} for the given {@link ModuleTestExecution}.
-		 * 
+		 *
 		 * @param execution must not be {@literal null}.
 		 */
 		private ModuleTestExecutionBeanDefinitionSelector(ModuleTestExecution execution) {
@@ -208,7 +209,7 @@ private ModuleTestExecutionBeanDefinitionSelector(ModuleTestExecution execution)
 			this.execution = execution;
 		}
 
-		/* 
+		/*
 		 * (non-Javadoc)
 		 * @see org.springframework.beans.factory.support.BeanDefinitionRegistryPostProcessor#postProcessBeanDefinitionRegistry(org.springframework.beans.factory.support.BeanDefinitionRegistry)
 		 */
@@ -224,7 +225,8 @@ public void postProcessBeanDefinitionRegistry(BeanDefinitionRegistry registry) t
 			for (String name : registry.getBeanDefinitionNames()) {
 
 				var type = factory.getType(name, false);
-				var module = modules.getModuleByType(type);
+				var module = modules.getModuleByType(type)
+						.filter(Predicate.not(ApplicationModule::isRootModule));
 
 				// Not a module type -> pass
 				if (module.isEmpty()) {
@@ -239,15 +241,17 @@ public void postProcessBeanDefinitionRegistry(BeanDefinitionRegistry registry) t
 						.filter(packagesIncludedInTestRun::contains).isPresent()) {
 					continue;
 				}
-				
-				LOGGER.trace("Dropping bean definition {} for type {} as it is not included in an application module to be bootstrapped!", name, type.getName());
+
+				LOGGER.trace(
+						"Dropping bean definition {} for type {} as it is not included in an application module to be bootstrapped!",
+						name, type.getName());
 
 				// Remove bean definition from bootstrap
 				registry.removeBeanDefinition(name);
 			}
 		}
 
-		/* 
+		/*
 		 * (non-Javadoc)
 		 * @see org.springframework.beans.factory.config.BeanFactoryPostProcessor#postProcessBeanFactory(org.springframework.beans.factory.config.ConfigurableListableBeanFactory)
 		 */
