Skip to content

Commit

Permalink
fix packages not being imported for Spring Boot JARs without director…
Browse files Browse the repository at this point in the history
…y entries

I realized that the legacy behavior actually had an approach that could fix this, i.e. there we would not only read the classpath from the system properties, but also look through the whole context for any derivations of URLClassLoader and take those URLs. The Spring Boot ClassLoader actually gives us the hierarchical URLs in this form, i.e. it would return `jar:file:/some/path.jar!/BOOT-INF/classes!/` instead of just the root path. By fixing the search logic a little to also only consider the last separator and not the first one we can actually use the custom URL handling to again stream the entries from a Spring Boot JAR in a consistent way and find class files even if there is no directory entry for the respective package.
If this change looks reasonable I would refactor the commits of the PR though to adjust the scanning behavior already in a pre-commit. Because I actually don't remember anymore why I decided to drop support for URLClassLoader analysis for any JRE > 9, AFAIR I thought it wouldn't make any difference and the URLClassLoader would lose meaning after JDK 8, but in fact it's still out there widely and this here shows clearly that it does make a difference to also include it.
  • Loading branch information
codecholeric committed Aug 6, 2023
1 parent 6a1fed1 commit d7714c4
Show file tree
Hide file tree
Showing 6 changed files with 28 additions and 53 deletions.
1 change: 1 addition & 0 deletions archunit-3rd-party-test/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,6 @@ dependencies {
dependency.addGuava { dependencyNotation, config -> testImplementation(dependencyNotation, config) }
testImplementation dependency.log4j_slf4j
testImplementation dependency.junit4
testImplementation dependency.junit_dataprovider
testImplementation dependency.assertj
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,21 @@

import java.io.File;
import java.io.IOException;
import java.net.URI;
import java.net.URL;
import java.util.Arrays;
import java.util.Iterator;
import java.util.function.Function;
import java.util.jar.JarFile;
import java.util.stream.Stream;

import com.tngtech.archunit.core.importer.testexamples.SomeEnum;
import com.tngtech.archunit.testutil.SystemPropertiesRule;
import com.tngtech.java.junit.dataprovider.DataProvider;
import com.tngtech.java.junit.dataprovider.DataProviderRunner;
import com.tngtech.java.junit.dataprovider.UseDataProvider;
import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.springframework.boot.loader.LaunchedURLClassLoader;
import org.springframework.boot.loader.archive.Archive;
import org.springframework.boot.loader.archive.JarFileArchive;
Expand All @@ -24,23 +28,32 @@
import static com.tngtech.archunit.core.importer.LocationTest.classFileEntry;
import static com.tngtech.archunit.core.importer.LocationTest.urlOfClass;
import static com.tngtech.archunit.core.importer.LocationsTest.unchecked;
import static com.tngtech.archunit.core.importer.UrlSourceTest.JAVA_CLASS_PATH_PROP;
import static org.assertj.core.api.Assertions.assertThat;

@RunWith(DataProviderRunner.class)
public class SpringLocationsTest {
/**
* Spring Boot configures some system properties that we want to reset afterward (e.g. custom URL stream handler)
*/
@Rule
public final SystemPropertiesRule systemPropertiesRule = new SystemPropertiesRule();

@Test
public void finds_locations_of_packages_from_Spring_Boot_ClassLoader_for_JARs_with_directory_entries() throws Exception {
try (JarFile jarFile = new TestJarFile()
.withDirectoryEntries()
@DataProvider
public static Object[][] springBootJars() {
Function<Function<TestJarFile, TestJarFile>, TestJarFile> createSpringBootJar = setUpJarFile -> setUpJarFile.apply(new TestJarFile())
.withNestedClassFilesDirectory("BOOT-INF/classes")
.withEntry(classFileEntry(SomeEnum.class).toAbsolutePath())
.create()) {
.withEntry(classFileEntry(SomeEnum.class).toAbsolutePath());

return com.tngtech.java.junit.dataprovider.DataProviders.testForEach(
createSpringBootJar.apply(TestJarFile::withDirectoryEntries),
createSpringBootJar.apply(TestJarFile::withoutDirectoryEntries)
);
}

@Test
@UseDataProvider("springBootJars")
public void finds_locations_of_packages_from_Spring_Boot_ClassLoader_for_JARs(TestJarFile jarFileToTest) throws Exception {
try (JarFile jarFile = jarFileToTest.create()) {

configureSpringBootContextClassLoaderKnowingOnly(jarFile);

Expand All @@ -60,43 +73,6 @@ public void finds_locations_of_packages_from_Spring_Boot_ClassLoader_for_JARs_wi
}
}

/**
* This is not "desired" behavior, but just to document the state as it is right now. I.e. if there was a
* Spring Boot JAR that only has a {@code BOOT-INF/classes/} directory entry, but no further directory entries
* for the packages contained, then we wouldn't find the classes in this package right now.
* Since we don't know if this really happens out there in the wild we keep it like this for now and see if
* this problem will ever occur (because to solve it is likely not trivial at all and might in the end require
* a custom extension point for frameworks).
* Note that we don't need to test the case where {@code BOOT-INF/classes/} has no directory entry,
* because in that case deriving the nested archive already fails with an exception. So it's a valid assumption
* that this directory entry always exists out in the wild.
*/
@Test
public void does_not_find_locations_of_packages_from_Spring_Boot_ClassLoader_for_JARs_without_directory_entries() throws Exception {
try (JarFile jarFile = new TestJarFile()
.withoutDirectoryEntries()
.withNestedClassFilesDirectory("BOOT-INF/classes")
.withEntry(classFileEntry(SomeEnum.class).toAbsolutePath())
.create()) {

configureSpringBootContextClassLoaderKnowingOnly(jarFile);

// Also configure classpath to match, so we principally would have a chance to find the class file there
System.setProperty(JAVA_CLASS_PATH_PROP, uriOf(jarFile));

// We still don't find any class files, because reading the JAR from the classpath gives us
// resource entries of the form `BOOT-INF/classes/...` which don't match the expected package name
// `com.tngtech...`. Without telling ArchUnit somehow that there is a nested archive
// within `BOOT-INF/classes` there doesn't seem to be any robust solution for this
// (unless the classpath would contain a URL of the form `jar:file:/.../some.jar!/BOOT-INF/classes!/`
// with two separators, then the customized URL handling would kick in. But realistic usage AFAIS just adds
// the base JAR URL and then lets the Spring Boot `JarLauncher` handle the nested archive logic).
Stream<Location> locationsInMatchingJarFile = Locations.ofPackage(SomeEnum.class.getPackage().getName()).stream()
.filter(it -> it.contains(jarFile.getName()));
assertThat(locationsInMatchingJarFile).isEmpty();
}
}

private static void configureSpringBootContextClassLoaderKnowingOnly(JarFile jarFile) throws IOException {
// This hooks in Spring Boot's own JAR URL protocol handler which knows how to handle URLs with
// multiple separators (e.g. "jar:file:/dir/some.jar!/BOOT-INF/classes!/pkg/some.class")
Expand All @@ -116,8 +92,4 @@ private static JarFileArchive getNestedJarFileArchive(JarFileArchive jarFileArch
Iterator<Archive> archiveCandidates = jarFileArchive.getNestedArchives(entry -> entry.getName().equals(path), entry -> true);
return (JarFileArchive) getOnlyElement(archiveCandidates);
}

private static String uriOf(JarFile jarFile) {
return URI.create("jar:" + new File(jarFile.getName()).toURI()).toString();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,11 @@
import static java.util.stream.Collectors.toList;

class ModuleLocationResolver implements LocationResolver {
private final FromClasspathAndUrlClassLoaders standardResolver = new FromClasspathAndUrlClassLoaders();

@Override
public UrlSource resolveClassPath() {
Iterable<URL> classpath = UrlSource.From.classPathSystemProperties();
Iterable<URL> classpath = standardResolver.resolveClassPath();
Set<ModuleReference> systemModuleReferences = ModuleFinder.ofSystem().findAll();
Set<ModuleReference> configuredModuleReferences = ModuleFinder.of(modulepath()).findAll();
Iterable<URL> modulepath = Stream.concat(systemModuleReferences.stream(), configuredModuleReferences.stream())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public void plugInLocationFactories(InitialConfiguration<Set<Location.Factory>>

@Override
public void plugInLocationResolver(InitialConfiguration<LocationResolver> locationResolver) {
locationResolver.set(new LocationResolver.Legacy());
locationResolver.set(new LocationResolver.FromClasspathAndUrlClassLoaders());
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ private Optional<JarFile> getJarFile() {
// Note: We can't use a composed JAR URL like `jar:file:/path/to/file.jar!/com/example`, because opening the connection
// fails with an exception if the directory entry for this path is missing (which is possible, even if there is
// a class `com.example.SomeClass` in the JAR file).
String baseUri = uri.toString().replaceAll("!/.*", "!/");
String baseUri = uri.toString().replaceAll("(.*!/).*", "$1");
JarURLConnection jarUrlConnection = (JarURLConnection) new URL(baseUri).openConnection();
return Optional.of(jarUrlConnection.getJarFile());
} catch (IOException e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ interface LocationResolver {
UrlSource resolveClassPath();

@Internal
class Legacy implements LocationResolver {
class FromClasspathAndUrlClassLoaders implements LocationResolver {
@Override
public UrlSource resolveClassPath() {
ImmutableList.Builder<URL> result = ImmutableList.builder();
Expand Down

0 comments on commit d7714c4

Please sign in to comment.