From 9148a57b02b9b61089ebf608175ccb446033d015 Mon Sep 17 00:00:00 2001 From: Oliver Drotbohm Date: Sun, 15 Oct 2023 19:34:25 +0200 Subject: [PATCH] GH-317 - Detect violations from types located in root packages. 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). --- .../modulith/core/ApplicationModule.java | 22 ++++++ .../modulith/core/ApplicationModules.java | 73 +++++++++++++++---- .../modulith/core/Violations.java | 14 +++- .../acme/myproject/CentralConfiguration.java | 29 ++++++++ .../java/com/acme/myproject/ModulithTest.java | 18 ++++- .../test/ModuleContextCustomizerFactory.java | 18 +++-- 6 files changed, 151 insertions(+), 23 deletions(-) create mode 100644 spring-modulith-integration-test/src/main/java/com/acme/myproject/CentralConfiguration.java 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 index a7146aae..0c62e7f3 100644 --- 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 class ApplicationModule { .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 index 3310d107..4f63f5d9 100644 --- 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 static java.util.stream.Collectors.*; 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.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.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 { private final Map modules; private final JavaClasses allClasses; private final List rootPackages; + private final Supplier> rootModules; private final Set sharedModules; private final List orderedNames; @@ -112,6 +111,10 @@ public class ApplicationModules implements Iterable { .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,25 +124,27 @@ public class ApplicationModules implements Iterable { /** * 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 modules, JavaClasses classes, - List rootPackages, Set sharedModules, List orderedNames, - boolean verified) { + List rootPackages, Supplier> rootModules, + Set sharedModules, List 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!"); @@ -147,6 +152,7 @@ public class ApplicationModules implements Iterable { 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 class ApplicationModules implements Iterable { 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 class ApplicationModules implements Iterable { Assert.hasText(candidate, "Candidate must not be null or empty!"); - return modules.values().stream() // + return allModules() // .filter(it -> it.contains(candidate)) // .findFirst(); } @@ -328,7 +334,7 @@ public class ApplicationModules implements Iterable { /** * Returns the {@link ApplicationModule} containing the given package. - * + * * @param name must not be {@literal null} or empty. * @return will never be {@literal null}. */ @@ -336,7 +342,12 @@ public class ApplicationModules implements Iterable { 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 class ApplicationModules implements Iterable { } } - 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 class ApplicationModules implements Iterable { } private ApplicationModules withSharedModules(Set 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 @@ public class ApplicationModules implements Iterable { return module; } + /** + * Returns of all {@link ApplicationModule}s, including root ones (last). + * + * @return will never be {@literal null}. + * @since 1.1 + */ + private Stream 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 @@ public class ApplicationModules implements Iterable { 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 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 index 92aa32db..92da73e0 100644 --- 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.Collections; 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 class Violations extends RuntimeException { .collect(Collectors.joining("\n- ", "- ", "")); } + /** + * Returns all violations' messages. + * + * @return will never be {@literal null}. + * @since 1.1 + */ + public List 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 new file mode 100644 index 00000000..fca6669e --- /dev/null +++ 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 index f2eb38c5..e73ba184 100644 --- 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 @@ package com.acme.myproject; 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 @@ class ModulithTest { @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 index 6f94c986..af470d45 100644 --- 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 @@ package org.springframework.modulith.test; 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 @@ class ModuleContextCustomizerFactory implements ContextCustomizerFactory { * @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 @@ class ModuleContextCustomizerFactory implements ContextCustomizerFactory { this.execution = execution; } - /* + /* * (non-Javadoc) * @see org.springframework.beans.factory.support.BeanDefinitionRegistryPostProcessor#postProcessBeanDefinitionRegistry(org.springframework.beans.factory.support.BeanDefinitionRegistry) */ @@ -224,7 +225,8 @@ class ModuleContextCustomizerFactory implements ContextCustomizerFactory { 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 @@ class ModuleContextCustomizerFactory implements ContextCustomizerFactory { .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) */