Skip to content

Commit

Permalink
Fix classpath scanning #363
Browse files Browse the repository at this point in the history
Unfortunately #347 broke the possibility to use a manifest `Class-Path` attribute to specify the classpath. Since this is a feature that many tools use to avoid "command line too long" problems (e.g. on Windows) when the classpath grows in size, I have added support for this to the new way of scanning the classpath.
I have also added back the old way to resolve files through the context classloader, since I could not measure a real performance impact and using both sources seems safer in case there is some other classpath quirk I have overlooked. The set will remove duplicates anyway, before starting to import the classes.
  • Loading branch information
codecholeric authored May 23, 2020
2 parents 15e5df6 + 0c3eb53 commit 6a2de78
Show file tree
Hide file tree
Showing 9 changed files with 456 additions and 30 deletions.
6 changes: 5 additions & 1 deletion .github/workflows/build.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
name: CI

on: [push, pull_request]
on:
push:
branches:
- master
pull_request:

jobs:
build:
Expand Down
7 changes: 6 additions & 1 deletion .github/workflows/gradle-wrapper-validation.yml
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
name: "Validate Gradle Wrapper"
on: [push, pull_request]

on:
push:
branches:
- master
pull_request:

jobs:
validation:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,9 +140,9 @@ private String encodeIllegalCharacters(String relativeURI) {
}

void checkScheme(String scheme, NormalizedUri uri) {
checkArgument(scheme.equals(uri.getScheme()),
"URI %s of %s must have scheme %s, but has %s",
uri, getClass().getSimpleName(), scheme, uri.getScheme());
String actualScheme = uri.getScheme();
checkArgument(scheme.equals(actualScheme),
"URI %s of Location must have scheme %s, but has %s", uri, scheme, actualScheme);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,21 @@
*/
package com.tngtech.archunit.core.importer;

import java.io.IOException;
import java.net.URL;
import java.util.Collection;
import java.util.HashSet;
import java.util.List;
import java.util.Set;

import com.google.common.collect.ImmutableSet;
import com.tngtech.archunit.PublicAPI;
import com.tngtech.archunit.base.ArchUnitException.LocationException;
import com.tngtech.archunit.core.InitialConfiguration;

import static com.google.common.collect.Sets.newHashSet;
import static com.tngtech.archunit.PublicAPI.Usage.ACCESS;
import static com.tngtech.archunit.base.ClassLoaders.getCurrentClassLoader;
import static java.util.Collections.list;

/**
* Represents a set of {@link Location locations} of Java class files. Also offers methods to derive concrete locations (i.e. URIs) from
Expand Down Expand Up @@ -106,7 +111,7 @@ private static String asResourceName(String qualifiedName) {
private static Set<Location> getLocationsOf(String resourceName) {
UrlSource classpath = locationResolver.get().resolveClassPath();
NormalizedResourceName normalizedResourceName = NormalizedResourceName.from(resourceName);
return ImmutableSet.copyOf(getResourceLocations(normalizedResourceName, classpath));
return ImmutableSet.copyOf(getResourceLocations(getCurrentClassLoader(Locations.class), normalizedResourceName, classpath));
}

/**
Expand All @@ -117,8 +122,8 @@ private static Set<Location> getLocationsOf(String resourceName) {
* does not behave correctly for older Java versions,
* because the folder entry {@code /java/io} is missing from {@code rt.jar}.
*/
private static Collection<Location> getResourceLocations(NormalizedResourceName resourceName, Iterable<URL> classpath) {
Set<Location> result = new HashSet<>();
private static Collection<Location> getResourceLocations(ClassLoader loader, NormalizedResourceName resourceName, Iterable<URL> classpath) {
Set<Location> result = newHashSet(Locations.of(getResources(loader, resourceName)));
for (Location location : Locations.of(classpath)) {
if (containsEntryWithPrefix(location, resourceName)) {
result.add(location.append(resourceName.toString()));
Expand All @@ -127,6 +132,14 @@ private static Collection<Location> getResourceLocations(NormalizedResourceName
return result;
}

private static List<URL> getResources(ClassLoader loader, NormalizedResourceName resourceName) {
try {
return list(loader.getResources(resourceName.toString()));
} catch (IOException e) {
throw new LocationException(e);
}
}

private static boolean containsEntryWithPrefix(Location location, NormalizedResourceName searchedJarEntryPrefix) {
for (NormalizedResourceName name : location.iterateEntries()) {
if (name.startsWith(searchedJarEntryPrefix)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,17 @@
package com.tngtech.archunit.core.importer;

import java.io.File;
import java.io.IOException;
import java.net.JarURLConnection;
import java.net.MalformedURLException;
import java.net.URI;
import java.net.URISyntaxException;
import java.net.URL;
import java.nio.file.InvalidPathException;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Set;
Expand All @@ -31,13 +35,19 @@
import com.google.common.base.Splitter;
import com.google.common.collect.FluentIterable;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Sets;
import com.tngtech.archunit.Internal;
import com.tngtech.archunit.base.ArchUnitException.LocationException;
import com.tngtech.archunit.base.Optional;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import static com.google.common.base.Strings.nullToEmpty;
import static com.google.common.collect.Iterables.concat;
import static com.tngtech.archunit.core.importer.Location.toURI;
import static java.util.Collections.emptySet;
import static java.util.jar.Attributes.Name.CLASS_PATH;

interface UrlSource extends Iterable<URL> {
@Internal
Expand Down Expand Up @@ -68,10 +78,106 @@ private static Iterable<URL> unique(Iterable<URL> urls) {
}

static UrlSource classPathSystemProperties() {
return iterable(ImmutableList.<URL>builder()
List<URL> directlySpecifiedAsProperties = ImmutableList.<URL>builder()
.addAll(findUrlsForClassPathProperty(BOOT_CLASS_PATH_PROPERTY_NAME))
.addAll(findUrlsForClassPathProperty(CLASS_PATH_PROPERTY_NAME))
.build());
.build();
Iterable<URL> transitivelySpecifiedThroughManifest = readClasspathEntriesFromManifests(directlySpecifiedAsProperties);
return iterable(concat(directlySpecifiedAsProperties, transitivelySpecifiedThroughManifest));
}

private static Iterable<URL> readClasspathEntriesFromManifests(List<URL> urls) {
Set<URI> result = new HashSet<>();
readClasspathUriEntriesFromManifests(result, FluentIterable.from(urls).transform(URL_TO_URI));
return FluentIterable.from(result).transform(URI_TO_URL);
}

// Use URI because of better equals / hashcode
private static void readClasspathUriEntriesFromManifests(Set<URI> result, Iterable<URI> urls) {
for (URI url : urls) {
if (url.getScheme().equals("jar")) {
Set<URI> manifestUris = readClasspathEntriesFromManifest(url);
Set<URI> unknownSoFar = ImmutableSet.copyOf(Sets.difference(manifestUris, result));
result.addAll(unknownSoFar);
readClasspathUriEntriesFromManifests(result, unknownSoFar);
}
}
}

private static Set<URI> readClasspathEntriesFromManifest(URI url) {
Optional<Path> jarPath = findParentPathOf(url);
if (!jarPath.isPresent()) {
return emptySet();
}

Set<URI> result = new HashSet<>();
for (String classpathEntry : Splitter.on(" ").omitEmptyStrings().split(readManifestClasspath(url))) {
result.addAll(parseManifestClasspathEntry(jarPath.get(), classpathEntry).asSet());
}
return result;
}

private static Optional<Path> findParentPathOf(URI uri) {
try {
return Optional.fromNullable(Paths.get(ensureFileUrl(uri).toURI()).getParent());
} catch (Exception e) {
LOG.warn("Could not find parent folder for " + uri, e);
return Optional.absent();
}
}

private static URL ensureFileUrl(URI url) throws IOException {
return ((JarURLConnection) url.toURL().openConnection()).getJarFileURL();
}

private static String readManifestClasspath(URI uri) {
try {
String result = (String) ((JarURLConnection) uri.toURL().openConnection()).getMainAttributes().get(CLASS_PATH);
return nullToEmpty(result);
} catch (Exception e) {
return "";
}
}

private static Optional<URI> parseManifestClasspathEntry(Path parent, String classpathEntry) {
if (isUrl(classpathEntry)) {
return parseUrl(parent, classpathEntry);
} else {
return parsePath(parent, classpathEntry);
}
}

private static boolean isUrl(String classpathEntry) {
return classpathEntry.startsWith("file:") || classpathEntry.startsWith("jar:");
}

private static Optional<URI> parseUrl(Path parent, String classpathUrlEntry) {
try {
return Optional.of(convertToJarUrlIfNecessary(parent.toUri().resolve(URI.create(classpathUrlEntry).getSchemeSpecificPart())));
} catch (Exception e) {
LOG.warn("Cannot parse URL classpath entry " + classpathUrlEntry, e);
return Optional.absent();
}
}

private static Optional<URI> parsePath(Path parent, String classpathFilePathEntry) {
try {
Path path = Paths.get(classpathFilePathEntry);
if (!path.isAbsolute()) {
path = parent.resolve(path);
}
return Optional.of(convertToJarUrlIfNecessary(path.toUri()));
} catch (Exception e) {
LOG.warn("Cannot parse file path classpath entry " + classpathFilePathEntry, e);
return Optional.absent();
}
}

private static URI convertToJarUrlIfNecessary(URI uri) {
if (uri.toString().endsWith(".jar")) {
return URI.create("jar:" + uri + "!/");
}
return uri;
}

private static List<URL> findUrlsForClassPathProperty(String propertyName) {
Expand All @@ -85,7 +191,7 @@ private static List<URL> findUrlsForClassPathProperty(String propertyName) {
}

private static Optional<URL> parseClassPathEntry(String path) {
return path.endsWith(".jar") ? newJarUri(path) : newFileUri(path);
return path.endsWith(".jar") ? newJarUrl(path) : newFileUri(path);
}

private static Optional<URL> newFileUri(String path) {
Expand Down Expand Up @@ -118,7 +224,7 @@ private static Optional<URL> tryResolvePathFromUrl(String path) {
}
}

private static Optional<URL> newJarUri(String path) {
private static Optional<URL> newJarUrl(String path) {
Optional<URL> fileUri = newFileUri(path);

try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,29 +4,45 @@
import java.io.IOException;
import java.lang.annotation.Annotation;
import java.lang.reflect.Field;
import java.net.URL;
import java.net.URLClassLoader;
import java.util.List;

import com.google.common.base.Joiner;
import com.google.common.base.Splitter;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.tngtech.archunit.Slow;
import com.tngtech.archunit.core.domain.JavaClass;
import com.tngtech.archunit.core.domain.JavaClasses;
import com.tngtech.archunit.core.domain.JavaPackage;
import com.tngtech.archunit.testutil.ContextClassLoaderRule;
import com.tngtech.archunit.testutil.SystemPropertiesRule;
import com.tngtech.archunit.testutil.TransientCopyRule;
import org.junit.Assert;
import org.junit.Rule;
import org.junit.Test;
import org.junit.experimental.categories.Category;
import org.junit.rules.TemporaryFolder;

import static com.tngtech.archunit.core.domain.SourceTest.urlOf;
import static com.tngtech.archunit.core.importer.ClassFileImporterTest.jarFileOf;
import static com.tngtech.archunit.core.importer.ImportOption.Predefined.DO_NOT_INCLUDE_TESTS;
import static com.tngtech.archunit.core.importer.UrlSourceTest.JAVA_CLASS_PATH_PROP;
import static com.tngtech.archunit.testutil.Assertions.assertThat;
import static com.tngtech.archunit.testutil.Assertions.assertThatClasses;
import static java.util.jar.Attributes.Name.CLASS_PATH;

@Category(Slow.class)
public class ClassFileImporterSlowTest {
@Rule
public final TransientCopyRule copyRule = new TransientCopyRule();
@Rule
public final TemporaryFolder temporaryFolder = new TemporaryFolder();
@Rule
public final SystemPropertiesRule systemPropertiesRule = new SystemPropertiesRule();
@Rule
public final ContextClassLoaderRule contextClassLoaderRule = new ContextClassLoaderRule();

@Test
public void imports_the_classpath() {
Expand Down Expand Up @@ -102,6 +118,34 @@ public void imports_duplicate_classes() throws IOException {
assertThat(classes.get(JavaClass.class)).isNotNull();
}

@Test
public void imports_classes_from_classpath_specified_in_manifest_file() {
String manifestClasspath =
Joiner.on(" ").join(Splitter.on(File.pathSeparator).omitEmptyStrings().split(System.getProperty(JAVA_CLASS_PATH_PROP)));
String jarPath = new TestJarFile()
.withManifestAttribute(CLASS_PATH, manifestClasspath)
.create()
.getName();

System.clearProperty(JAVA_CLASS_PATH_PROP);
// Ensure we cannot load the class through the fallback via the Classloader
Thread.currentThread().setContextClassLoader(new URLClassLoader(new URL[0], null));
verifyCantLoadWithCurrentClasspath(getClass());
System.setProperty(JAVA_CLASS_PATH_PROP, jarPath);

JavaClasses javaClasses = new ClassFileImporter().importPackages(getClass().getPackage().getName());

assertThatClasses(javaClasses).contain(getClass());
}

private void verifyCantLoadWithCurrentClasspath(Class<?> clazz) {
try {
new ClassFileImporter().importClass(clazz);
Assert.fail(String.format("Should not have been able to load class %s with the current classpath", clazz.getName()));
} catch (RuntimeException ignored) {
}
}

@Test
public void creates_JavaPackages() {
JavaClasses javaClasses = importJavaBase();
Expand Down
Loading

0 comments on commit 6a2de78

Please sign in to comment.