Skip to content

Commit

Permalink
restore the old way of resolving classes from the classpath
Browse files Browse the repository at this point in the history
I measured this and found no noticeable performance impact. Thus I think it is safer to still use the current Classloader to resolve class files as well. In the end we will ignore duplicates in the set of locations anyway, and if the lookup time has no real impact, we are still safer in case there is some other quirk (like the manifest classpath that I forgot about when I refactored this to fix the Android problem).

Signed-off-by: Peter Gafert <peter.gafert@tngtech.com>
  • Loading branch information
codecholeric committed May 23, 2020
1 parent bc353f4 commit 8f882df
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 5 deletions.
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 @@ -4,6 +4,8 @@
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;
Expand All @@ -14,6 +16,7 @@
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;
Expand All @@ -38,6 +41,8 @@ public class ClassFileImporterSlowTest {
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 @@ -115,13 +120,16 @@ public void imports_duplicate_classes() throws IOException {

@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 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);

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package com.tngtech.archunit.testutil;

import org.junit.rules.ExternalResource;

public class ContextClassLoaderRule extends ExternalResource {
private ClassLoader contextClassLoader;

@Override
public void before() {
contextClassLoader = Thread.currentThread().getContextClassLoader();
}

@Override
public void after() {
Thread.currentThread().setContextClassLoader(contextClassLoader);
}
}

0 comments on commit 8f882df

Please sign in to comment.