From 8f882df93e00a36e471f01107638caee290b6be9 Mon Sep 17 00:00:00 2001 From: Peter Gafert Date: Sat, 23 May 2020 20:54:05 +0200 Subject: [PATCH] restore the old way of resolving classes from the classpath 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 --- .../archunit/core/importer/Locations.java | 21 +++++++++++++++---- .../importer/ClassFileImporterSlowTest.java | 10 ++++++++- .../testutil/ContextClassLoaderRule.java | 17 +++++++++++++++ 3 files changed, 43 insertions(+), 5 deletions(-) create mode 100644 archunit/src/test/java/com/tngtech/archunit/testutil/ContextClassLoaderRule.java diff --git a/archunit/src/main/java/com/tngtech/archunit/core/importer/Locations.java b/archunit/src/main/java/com/tngtech/archunit/core/importer/Locations.java index e8575e30d7..b428e8ac2f 100644 --- a/archunit/src/main/java/com/tngtech/archunit/core/importer/Locations.java +++ b/archunit/src/main/java/com/tngtech/archunit/core/importer/Locations.java @@ -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 @@ -106,7 +111,7 @@ private static String asResourceName(String qualifiedName) { private static Set 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)); } /** @@ -117,8 +122,8 @@ private static Set 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 getResourceLocations(NormalizedResourceName resourceName, Iterable classpath) { - Set result = new HashSet<>(); + private static Collection getResourceLocations(ClassLoader loader, NormalizedResourceName resourceName, Iterable classpath) { + Set result = newHashSet(Locations.of(getResources(loader, resourceName))); for (Location location : Locations.of(classpath)) { if (containsEntryWithPrefix(location, resourceName)) { result.add(location.append(resourceName.toString())); @@ -127,6 +132,14 @@ private static Collection getResourceLocations(NormalizedResourceName return result; } + private static List 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)) { diff --git a/archunit/src/test/java/com/tngtech/archunit/core/importer/ClassFileImporterSlowTest.java b/archunit/src/test/java/com/tngtech/archunit/core/importer/ClassFileImporterSlowTest.java index 756201fbd1..1f265413af 100644 --- a/archunit/src/test/java/com/tngtech/archunit/core/importer/ClassFileImporterSlowTest.java +++ b/archunit/src/test/java/com/tngtech/archunit/core/importer/ClassFileImporterSlowTest.java @@ -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; @@ -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; @@ -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() { @@ -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); diff --git a/archunit/src/test/java/com/tngtech/archunit/testutil/ContextClassLoaderRule.java b/archunit/src/test/java/com/tngtech/archunit/testutil/ContextClassLoaderRule.java new file mode 100644 index 0000000000..6bdab426a4 --- /dev/null +++ b/archunit/src/test/java/com/tngtech/archunit/testutil/ContextClassLoaderRule.java @@ -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); + } +}