Skip to content

Commit b7bf88a

Browse files
authored
fix tests not behaving consistently with Android TNG#347
It seems like Gradle Android support treats Java and Android libraries differently. When run from Gradle `ClassLoader.getResources(..)` would return a package entry only from the library JAR, but not the Android JAR. When run from Android Studio it seems like plain file paths and no archives are used, so `ClassLoader.getResources(..)` returns URLs from both libraries. In the beginning we only used `ClassLoader.getResources(..)`, but then the first bug occurring was that JARs containing a package would not be detected if the folder entry for the package was missing (e.g. look for `java.io` but the entry `/java/io` from the JAR is missing, even though `/java/io/File.class` is there, then `ClassLoader.getResources("/java/io/")` would not give a result). As a workaround we would check if the requested resource was no class file and we got no result and then in turn look through all the JARs on the classpath to return all entries starting with the respective package prefix. Unfortunately this did not work for Android, because `ClassLoader.getResources(..)` provided a partial result, just missing some entries that should be present on the classpath. The only consistent solution seems to be to immediately do the scanning through the classpath ourselves and completely drop the usage of `ClassLoader.getResources(..)`. I did some performance analysis on Hibernate Core which seemed fine (similar times). What was needed though is a cache for the entries or the build time would explode. This will not make a difference if every location is only scanned once, but as soon as we repeatedly import classes from the classpath the scanning of all archives would cost massive performance (in the end the `ClassLoader` caches all these entries internally as well). I have compared the heap usage with and without the cache on Hibernate core and could not notice a significant increase. Thus it seems okay to go this way and keeping the resource entries in the cache for the lifetime of ArchUnit seems okay.
2 parents 06f94b2 + 7f74b59 commit b7bf88a

File tree

3 files changed

+37
-52
lines changed

3 files changed

+37
-52
lines changed

archunit/src/jdk9main/java/com/tngtech/archunit/core/importer/ModuleLocationFactory.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ public boolean isArchive() {
100100
}
101101

102102
@Override
103-
Iterable<NormalizedResourceName> iterateEntries() {
103+
Iterable<NormalizedResourceName> iterateEntriesInternal() {
104104
return doWithModuleReader(moduleReference, moduleReader -> moduleReader.list()
105105
.filter(resourceName::isStartOf)
106106
.map(NormalizedResourceName::from)

archunit/src/main/java/com/tngtech/archunit/core/importer/Location.java

+27-7
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,14 @@
3131
import java.util.List;
3232
import java.util.Objects;
3333
import java.util.Set;
34+
import java.util.concurrent.Callable;
35+
import java.util.concurrent.ExecutionException;
3436
import java.util.jar.JarEntry;
3537
import java.util.jar.JarFile;
3638
import java.util.regex.Pattern;
3739

40+
import com.google.common.cache.Cache;
41+
import com.google.common.cache.CacheBuilder;
3842
import com.google.common.collect.ImmutableList;
3943
import com.tngtech.archunit.PublicAPI;
4044
import com.tngtech.archunit.base.ArchUnitException.LocationException;
@@ -63,6 +67,14 @@ public abstract class Location {
6367
ImportPlugin.Loader.loadForCurrentPlatform().plugInLocationFactories(factories);
6468
}
6569

70+
private static final Cache<NormalizedUri, Iterable<NormalizedResourceName>> ENTRY_CACHE = CacheBuilder.newBuilder().build();
71+
private final Callable<Iterable<NormalizedResourceName>> readResourceEntries = new Callable<Iterable<NormalizedResourceName>>() {
72+
@Override
73+
public Iterable<NormalizedResourceName> call() {
74+
return iterateEntriesInternal();
75+
}
76+
};
77+
6678
final NormalizedUri uri;
6779

6880
Location(NormalizedUri uri) {
@@ -133,6 +145,19 @@ void checkScheme(String scheme, NormalizedUri uri) {
133145
uri, getClass().getSimpleName(), scheme, uri.getScheme());
134146
}
135147

148+
/**
149+
* @return An iterable containing all class file names under this location, e.g. relative file names, Jar entry names, ...
150+
*/
151+
final Iterable<NormalizedResourceName> iterateEntries() {
152+
try {
153+
return ENTRY_CACHE.get(uri, readResourceEntries);
154+
} catch (ExecutionException e) {
155+
throw new LocationException(e);
156+
}
157+
}
158+
159+
abstract Iterable<NormalizedResourceName> iterateEntriesInternal();
160+
136161
@Override
137162
public int hashCode() {
138163
return Objects.hash(uri);
@@ -189,11 +214,6 @@ static URI toURI(URL url) {
189214
}
190215
}
191216

192-
/**
193-
* @return An iterable containing all class file names under this location, e.g. relative file names, Jar entry names, ...
194-
*/
195-
abstract Iterable<NormalizedResourceName> iterateEntries();
196-
197217
interface Factory {
198218
boolean supports(String scheme);
199219

@@ -270,7 +290,7 @@ public boolean isArchive() {
270290
}
271291

272292
@Override
273-
Iterable<NormalizedResourceName> iterateEntries() {
293+
Iterable<NormalizedResourceName> iterateEntriesInternal() {
274294
File file = getFileOfJar();
275295
if (!file.exists()) {
276296
return emptySet();
@@ -341,7 +361,7 @@ public boolean isArchive() {
341361
}
342362

343363
@Override
344-
Iterable<NormalizedResourceName> iterateEntries() {
364+
Iterable<NormalizedResourceName> iterateEntriesInternal() {
345365
try {
346366
return getAllFilesBeneath(uri);
347367
} catch (IOException e) {

archunit/src/main/java/com/tngtech/archunit/core/importer/Locations.java

+9-44
Original file line numberDiff line numberDiff line change
@@ -15,23 +15,16 @@
1515
*/
1616
package com.tngtech.archunit.core.importer;
1717

18-
import java.io.IOException;
1918
import java.net.URL;
2019
import java.util.Collection;
2120
import java.util.HashSet;
2221
import java.util.Set;
2322

24-
import com.google.common.base.Predicate;
25-
import com.google.common.collect.FluentIterable;
2623
import com.google.common.collect.ImmutableSet;
2724
import com.tngtech.archunit.PublicAPI;
28-
import com.tngtech.archunit.base.ArchUnitException.LocationException;
2925
import com.tngtech.archunit.core.InitialConfiguration;
3026

31-
import static com.google.common.collect.Sets.newHashSet;
3227
import static com.tngtech.archunit.PublicAPI.Usage.ACCESS;
33-
import static com.tngtech.archunit.base.ClassLoaders.getCurrentClassLoader;
34-
import static java.util.Collections.list;
3528

3629
/**
3730
* Represents a set of {@link Location locations} of Java class files. Also offers methods to derive concrete locations (i.e. URIs) from
@@ -113,37 +106,20 @@ private static String asResourceName(String qualifiedName) {
113106
private static Set<Location> getLocationsOf(String resourceName) {
114107
UrlSource classpath = locationResolver.get().resolveClassPath();
115108
NormalizedResourceName normalizedResourceName = NormalizedResourceName.from(resourceName);
116-
return ImmutableSet.copyOf(getResourceLocations(getCurrentClassLoader(Locations.class), normalizedResourceName, classpath));
117-
}
118-
119-
private static Collection<Location> getResourceLocations(ClassLoader loader, NormalizedResourceName resourceName, Iterable<URL> classpath) {
120-
try {
121-
Set<Location> result = newHashSet(Locations.of(list(loader.getResources(resourceName.toString()))));
122-
if (result.isEmpty() && !resourceName.belongsToClassFile()) {
123-
return findMissedClassesDueToLackOfPackageEntry(classpath, resourceName);
124-
}
125-
return result;
126-
} catch (IOException e) {
127-
throw new LocationException(e);
128-
}
109+
return ImmutableSet.copyOf(getResourceLocations(normalizedResourceName, classpath));
129110
}
130111

131112
/**
132-
* Unfortunately the behavior with archives is not completely consistent. Originally,
133-
* {@link Locations#ofPackage(String)} simply asked all {@link java.net.URLClassLoader}s, for
134-
* {@link ClassLoader#getResources(String)} of the respective resource name.
135-
* E.g.
136-
* <pre><code>importPackage("org.junit") -> getResources("/org/junit")</code></pre>
137-
* However, this only works, if all respective archives contain an entry for the folder, which is not always the
138-
* case. Consider the standard JRE "rt.jar", which does not contain an entry "/java/io", but nonetheless
139-
* entries like "/java/io/File.class". Thus an import of "java.io", relying on
140-
* {@link ClassLoader#getResources(String)}, would not import <code>java.io.File</code>.
113+
* We cannot use {@link ClassLoader#getResources(String)} here, because this behaves inconsistently
114+
* if JAR files are missing package folder entries.<br>
115+
* E.g. loading the package via
116+
* <pre><code>importPackage("java.io") -> classLoader.getResources("/java/io")</code></pre>
117+
* does not behave correctly for older Java versions,
118+
* because the folder entry {@code /java/io} is missing from {@code rt.jar}.
141119
*/
142-
private static Collection<Location> findMissedClassesDueToLackOfPackageEntry(
143-
Iterable<URL> classpath, NormalizedResourceName resourceName) {
144-
120+
private static Collection<Location> getResourceLocations(NormalizedResourceName resourceName, Iterable<URL> classpath) {
145121
Set<Location> result = new HashSet<>();
146-
for (Location location : archiveLocationsOf(classpath)) {
122+
for (Location location : Locations.of(classpath)) {
147123
if (containsEntryWithPrefix(location, resourceName)) {
148124
result.add(location.append(resourceName.toString()));
149125
}
@@ -159,15 +135,4 @@ private static boolean containsEntryWithPrefix(Location location, NormalizedReso
159135
}
160136
return false;
161137
}
162-
163-
private static Set<Location> archiveLocationsOf(Iterable<URL> urls) {
164-
return FluentIterable.from(Locations.of(urls))
165-
.filter(new Predicate<Location>() {
166-
@Override
167-
public boolean apply(Location input) {
168-
return input.isArchive();
169-
}
170-
}).toSet();
171-
}
172-
173138
}

0 commit comments

Comments
 (0)