From 1cc57192345a031d231f3ad327077dbaf7860d9d Mon Sep 17 00:00:00 2001 From: Oliver Drotbohm Date: Sat, 25 Jan 2025 13:54:43 +0100 Subject: [PATCH] GH-1041 - Revamp JavaPackage's sub-package traversal to retain empty intermediate packages. --- .../modulith/core/JavaPackage.java | 69 ++++++------ .../modulith/core/PackageName.java | 104 +++++++++++++++++- .../modulith/core/JavaPackageUnitTests.java | 12 ++ .../modulith/core/PackageNameUnitTests.java | 16 +-- .../modulith/core/TestUtils.java | 24 ++-- .../many/intermediate/packages/Marker.java | 21 ++++ 6 files changed, 189 insertions(+), 57 deletions(-) create mode 100644 spring-modulith-core/src/test/java/with/many/intermediate/packages/Marker.java diff --git a/spring-modulith-core/src/main/java/org/springframework/modulith/core/JavaPackage.java b/spring-modulith-core/src/main/java/org/springframework/modulith/core/JavaPackage.java index 0f91ff2c..0517d213 100644 --- a/spring-modulith-core/src/main/java/org/springframework/modulith/core/JavaPackage.java +++ b/spring-modulith-core/src/main/java/org/springframework/modulith/core/JavaPackage.java @@ -59,6 +59,7 @@ public class JavaPackage implements DescribedIterable, Comparable> directSubPackages; + private final Supplier subPackages; /** * Creates a new {@link JavaPackage} for the given {@link Classes}, name and whether to include all sub-packages. @@ -75,13 +76,13 @@ public class JavaPackage implements DescribedIterable, Comparable packageClasses.stream() // - .map(it -> it.getPackageName()) // - .filter(Predicate.not(name::hasName)) // - .map(it -> extractDirectSubPackage(it)) // - .distinct() // - .map(it -> of(classes, it)) // - .collect(Collectors.toSet())); + + this.directSubPackages = () -> detectSubPackages() + .filter(this::isDirectParentOf) + .collect(Collectors.toUnmodifiableSet()); + + this.subPackages = SingletonSupplier.of(() -> detectSubPackages() + .collect(collectingAndThen(toUnmodifiableList(), JavaPackages::new))); } /** @@ -92,7 +93,7 @@ public class JavaPackage implements DescribedIterable, Comparable, Comparable new JavaPackage(classes, new PackageName(it), true)) - .collect(collectingAndThen(toUnmodifiableList(), JavaPackages::new)); + return subPackages.get(); } /** @@ -390,6 +385,21 @@ public class JavaPackage implements DescribedIterable, Comparable, Comparable detectSubPackages() { + + return packageClasses.stream() // + .map(JavaClass::getPackageName) + .filter(Predicate.not(name::hasName)) + .map(PackageName::of) + .flatMap(name::expandUntil) + .distinct() + .map(it -> new JavaPackage(classes, it, true)); + } + /* * (non-Javadoc) * @see java.lang.Iterable#iterator() @@ -461,24 +482,6 @@ public class JavaPackage implements DescribedIterable, Comparable reverse() { return (left, right) -> -left.compareTo(right); } diff --git a/spring-modulith-core/src/main/java/org/springframework/modulith/core/PackageName.java b/spring-modulith-core/src/main/java/org/springframework/modulith/core/PackageName.java index f12ecb0c..71d08bc8 100644 --- a/spring-modulith-core/src/main/java/org/springframework/modulith/core/PackageName.java +++ b/spring-modulith-core/src/main/java/org/springframework/modulith/core/PackageName.java @@ -15,6 +15,13 @@ */ package org.springframework.modulith.core; +import java.util.Arrays; +import java.util.HashMap; +import java.util.Map; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; @@ -27,6 +34,8 @@ import org.springframework.util.ClassUtils; */ class PackageName implements Comparable { + private static final Map PACKAGE_NAMES = new HashMap<>(); + private final String name; private final String[] segments; @@ -35,12 +44,23 @@ class PackageName implements Comparable { * * @param name must not be {@literal null}. */ - public PackageName(String name) { + private PackageName(String name) { + this(name, name.split("\\.")); + } + + /** + * Creates a new {@link PackageName} with the given name and segments. + * + * @param name must not be {@literal null}. + * @param segments must not be {@literal null}. + */ + private PackageName(String name, String[] segments) { Assert.notNull(name, "Name must not be null!"); + Assert.notNull(segments, "Segments must not be null!"); this.name = name; - this.segments = name.split("\\."); + this.segments = segments; } /** @@ -49,11 +69,41 @@ class PackageName implements Comparable { * @param fullyQualifiedName must not be {@literal null} or empty. * @return will never be {@literal null}. */ - public static PackageName ofType(String fullyQualifiedName) { + static PackageName ofType(String fullyQualifiedName) { Assert.notNull(fullyQualifiedName, "Type name must not be null!"); - return new PackageName(ClassUtils.getPackageName(fullyQualifiedName)); + return PackageName.of(ClassUtils.getPackageName(fullyQualifiedName)); + } + + /** + * Returns the {@link PackageName} with the given name. + * + * @param name must not be {@literal null}. + * @return will never be {@literal null}. + * @since 1.4 + */ + static PackageName of(String name) { + + Assert.notNull(name, "Name must not be null!"); + + return PACKAGE_NAMES.computeIfAbsent(name, PackageName::new); + } + + /** + * Returns the {@link PackageName} for the given segments. + * + * @param segments must not be {@literal null}. + * @return will never be {@literal null}. + * @since 1.4 + */ + static PackageName of(String[] segments) { + + Assert.notNull(segments, "Segments must not be null!"); + + var name = Stream.of(segments).collect(Collectors.joining(".")); + + return PACKAGE_NAMES.computeIfAbsent(name, it -> new PackageName(name, segments)); } /** @@ -132,6 +182,10 @@ class PackageName implements Comparable { return reference.name.startsWith(name + "."); } + boolean isDirectParentOf(PackageName reference) { + return this.equals(getParent()); + } + /** * 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. @@ -185,6 +239,48 @@ class PackageName implements Comparable { return segments.length - o.segments.length; } + /** + * Returns the names of sub-packages of the current one until the given reference {@link PackageName}. + * + * @param reference must not be {@literal null}. + * @return will never be {@literal null}. + * @since 1.4 + */ + Stream expandUntil(PackageName reference) { + + Assert.notNull(reference, "Reference must not be null!"); + + if (!reference.isSubPackageOf(this) || !reference.hasParent()) { + return Stream.empty(); + } + + if (isDirectParentOf(reference)) { + return Stream.of(reference); + } + + return Stream.concat(expandUntil(reference.getParent()), Stream.of(reference)); + } + + /** + * Returns whether the current {@link PackageName} has a parent. + * + * @since 1.4 + */ + boolean hasParent() { + return segments.length > 1; + } + + /** + * Returns the parent {@link PackageName}. + * + * @return can be {@literal null}. + * @since 1.4 + */ + @Nullable + PackageName getParent() { + return PackageName.of(Arrays.copyOf(segments, segments.length - 1)); + } + /* * (non-Javadoc) * @see java.lang.Object#toString() diff --git a/spring-modulith-core/src/test/java/org/springframework/modulith/core/JavaPackageUnitTests.java b/spring-modulith-core/src/test/java/org/springframework/modulith/core/JavaPackageUnitTests.java index 2060d114..c6d97d88 100644 --- a/spring-modulith-core/src/test/java/org/springframework/modulith/core/JavaPackageUnitTests.java +++ b/spring-modulith-core/src/test/java/org/springframework/modulith/core/JavaPackageUnitTests.java @@ -116,4 +116,16 @@ class JavaPackageUnitTests { assertThat(first.equals(second)).isTrue(); assertThat(second.equals(first)).isTrue(); } + + @Test // GH-1039 + void detectsIntermediateSubPackages() { + + var packages = TestUtils.getPackage("with").getSubPackages(); + + assertThat(packages) + .extracting(JavaPackage::getName) + .containsExactly("with.many", + "with.many.intermediate", + "with.many.intermediate.packages"); + } } diff --git a/spring-modulith-core/src/test/java/org/springframework/modulith/core/PackageNameUnitTests.java b/spring-modulith-core/src/test/java/org/springframework/modulith/core/PackageNameUnitTests.java index 71a6cb58..86bc72b3 100644 --- a/spring-modulith-core/src/test/java/org/springframework/modulith/core/PackageNameUnitTests.java +++ b/spring-modulith-core/src/test/java/org/springframework/modulith/core/PackageNameUnitTests.java @@ -31,12 +31,12 @@ class PackageNameUnitTests { @Test // GH-578 void sortsPackagesByNameAndDepth() { - var comAcme = new PackageName("com.acme"); - var comAcmeA = new PackageName("com.acme.a"); - var comAcmeAFirst = new PackageName("com.acme.a.first"); - var comAcmeAFirstOne = new PackageName("com.acme.a.first.one"); - var comAcmeASecond = new PackageName("com.acme.a.second"); - var comAcmeB = new PackageName("com.acme.b"); + var comAcme = PackageName.of("com.acme"); + var comAcmeA = PackageName.of("com.acme.a"); + var comAcmeAFirst = PackageName.of("com.acme.a.first"); + var comAcmeAFirstOne = PackageName.of("com.acme.a.first.one"); + var comAcmeASecond = PackageName.of("com.acme.a.second"); + var comAcmeB = PackageName.of("com.acme.b"); assertThat(List.of(comAcmeAFirstOne, comAcmeB, comAcmeASecond, comAcmeAFirst, comAcme, comAcmeA) .stream() @@ -48,8 +48,8 @@ class PackageNameUnitTests { @Test // GH-802 void caculatesNestingCorrectly() { - var comAcme = new PackageName("com.acme"); - var comAcmeA = new PackageName("com.acme.a"); + var comAcme = PackageName.of("com.acme"); + var comAcmeA = PackageName.of("com.acme.a"); assertThat(comAcme.contains(comAcme)).isTrue(); assertThat(comAcme.contains(comAcmeA)).isTrue(); diff --git a/spring-modulith-core/src/test/java/org/springframework/modulith/core/TestUtils.java b/spring-modulith-core/src/test/java/org/springframework/modulith/core/TestUtils.java index 483209eb..a6b2eedf 100644 --- a/spring-modulith-core/src/test/java/org/springframework/modulith/core/TestUtils.java +++ b/spring-modulith-core/src/test/java/org/springframework/modulith/core/TestUtils.java @@ -85,10 +85,22 @@ public class TestUtils { .that(resideInAPackage(packageType.getPackage().getName() + ".."))); } + public static Classes getClasses(String packageName) { + + Assert.hasText(packageName, "Package name must not be null or empty!"); + + return Classes.of(new ClassFileImporter() + .importPackages(packageName)); + } + public static JavaPackage getPackage(Class packageType) { return JavaPackage.of(TestUtils.getClasses(packageType), packageType.getPackageName()); } + public static JavaPackage getPackage(String name) { + return JavaPackage.of(getClasses(name), name); + } + public static ApplicationModule getApplicationModule(String packageName) { var pkg = getPackage(packageName); @@ -97,18 +109,6 @@ public class TestUtils { return new ApplicationModule(source); } - private static JavaPackage getPackage(String name) { - return JavaPackage.of(getClasses(name), name); - } - - private static Classes getClasses(String packageName) { - - Assert.hasText(packageName, "Package name must not be null or empty!"); - - return Classes.of(new ClassFileImporter() - .importPackages(packageName)); - } - private static ApplicationModules of(ModulithMetadata metadata, DescribedPredicate ignores) { return new ApplicationModules(metadata, ignores, new ImportOption.OnlyIncludeTests()) {}; } diff --git a/spring-modulith-core/src/test/java/with/many/intermediate/packages/Marker.java b/spring-modulith-core/src/test/java/with/many/intermediate/packages/Marker.java new file mode 100644 index 00000000..0e526972 --- /dev/null +++ b/spring-modulith-core/src/test/java/with/many/intermediate/packages/Marker.java @@ -0,0 +1,21 @@ +/* + * Copyright 2025 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 with.many.intermediate.packages; + +/** + * @author Oliver Drotbohm + */ +class Marker {}