Skip to content

Commit

Permalink
fix modulepath scanning #506
Browse files Browse the repository at this point in the history
In a modular Java project, where the main code is supplied from the modulepath instead of the classpath, the class scanning was unreliable. In fact the behavior was sporadic at first sight. To elaborate, consider the following structure:

```
src
|-- main
|    |-- java
|         |-- module-info.java
|         |-- com/root/First.java
|         |-- com/otherroot/Second.java
|-- test
      |-- java
           |-- com/root/ArchUnitTest.java
```

In this scenario `com.otherroot.Second` would be found by ArchUnit's scanning, but surprisingly `com.root.First` would not be found. The reason for this behavior was an internal fallback to `ClassLoader.getResources(..)` to locate the class (e.g. `/com/otherroot/Second.class`), but this only worked, if the respective resource path had not been overwritten via `--patch-module`. Surprisingly `--patch-module` causes the original resource path to be hidden (compare http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-July/054228.html).
Thus in this example case `ArchUnitTest` could find `com.otherroot.Second`, as long as only `com/root` was patched. I.e. Maven would add `--patch-module example_module=com/root` to patch the test class into the module. But `ClassLoader.getResources(..)` would then not find `/com/root/First.class` anymore, since `target/classes/com/root` had been overwritten with `target/test-classes/com/root`. On the other hand `ClassLoader.getResources(..)` would still look into `target/classes/com/otherroot`, since that path had not been patched.

To fix this behavior I have tried to eliminate the root cause, namely to make the fallback to `ClassLoader.getResources(..)` unnecessary for classes on the module path. So far in the first step only the configured classpath and the system modulepath have been read in `ModuleLocationResolver`. I have extended this to also parse the configured modulepath (through the system property `jdk.module.path`) and scan the content of these modules analogously to the system modules.

It provided quite challenging to add a test for this behavior, since ArchUnit itself is not written as Java Modules and Gradle's support for Java Modules has proven to still lack some usability (i.e. I could not find a way without tinkering around with compiler and JVM args to get this working in a modular way, but this also might be because of the somewhat suboptimal ArchUnit build). To make the setup easier I have added the `org.javamodularity.moduleplugin` (compare https://github.com/java9-modularity/gradle-modules-plugin) to a separate Gradle module, which has the only purpose to try to scan a class from the module path that has been overridden by a `--patch-module` (since the test resides in the same package).
However, this test only has any significance if run with Gradle, since IntelliJ seems to simply dump all classes on the classpath to run tests, no matter if the code base is written as Java Modules. Thus I've added a simple assertion to the test to make sure that the respective `main` class is actually not present on the classpath, since the test would then just always be successful, even if modulepath scanning would be broken. This assertion is a little brittle, because it uses a lot of Reflection, but I wanted to reuse ArchUnit's main code to resolve the classpath and I didn't want to make these internal parts visible for external use, so I decided this is still the best solution, considering all tradeoffs.

In any case modulepath scanning probably still doesn't work in all scenarios now (e.g. if `--patch-module` and other options are supplied), but it should be good enough for most standard use cases. And since this issue has only been brought up over 3 years after ArchUnit + Java 9 have been out there together, I would guess that exotic combinations (like "I want to use `--patch-module` to patch in test code that I want to scan with ArchUnit" or similar) are not that common. Besides that it is always possible to just dump all classes on the classpath together to execute the ArchUnit test and avoid the modulepath issues altogether (like Gradle or IntelliJ seem to do by default anyway to execute unit tests).

Resolves: #497
  • Loading branch information
codecholeric authored Jan 25, 2021
2 parents 6657d82 + 9235749 commit 85bb0e9
Show file tree
Hide file tree
Showing 6 changed files with 105 additions and 2 deletions.
31 changes: 31 additions & 0 deletions archunit-java-modules-test/build.gradle
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
plugins {
id 'org.javamodularity.moduleplugin' version '1.7.0'
}

ext.moduleName = 'com.tngtech.archunit.javamodulestest'

sourceCompatibility = JavaVersion.VERSION_1_9
targetCompatibility = JavaVersion.VERSION_1_9

dependencies {
testImplementation project(path: ':archunit', configuration: 'shadow')
testImplementation dependency.log4j_slf4j

testImplementation dependency.junit5JupiterApi

testRuntimeOnly dependency.junitPlatform
testRuntimeOnly dependency.junit5JupiterEngine
}

test {
useJUnitPlatform()
}

def addArchUnitModuleOptions = {
moduleOptions {
addModules = ['com.tngtech.archunit']
addReads = ['example_module': 'com.tngtech.archunit']
}
}

[compileTestJava, test]*.with(addArchUnitModuleOptions)
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
package com.tngtech.archunit.example;

public class SomeClass {
}
3 changes: 3 additions & 0 deletions archunit-java-modules-test/src/main/java/module-info.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
module example_module {
exports com.tngtech.archunit.example;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
package com.tngtech.archunit.example;

import java.lang.reflect.Method;
import java.net.URL;
import java.net.URLClassLoader;

import com.tngtech.archunit.core.domain.JavaClasses;
import com.tngtech.archunit.core.importer.ClassFileImporter;
import org.junit.jupiter.api.Test;

import static java.util.stream.StreamSupport.stream;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.fail;

public class ModulePathTest {

@Test
public void can_find_class_on_the_modulepath() throws Exception {
assertNotOnClasspath(SomeClass.class);

JavaClasses classes = new ClassFileImporter().importPackages("com.tngtech.archunit.example");

assertNotNull(classes.get(SomeClass.class));
}

@SuppressWarnings("unchecked")
private void assertNotOnClasspath(Class<?> clazz) throws Exception {
Class<?> urlSource = Class.forName("com.tngtech.archunit.core.importer.UrlSource$From");
Method classPathSystemProperties = urlSource.getDeclaredMethod("classPathSystemProperties");
classPathSystemProperties.setAccessible(true);
Iterable<URL> classpathUrls = (Iterable<URL>) classPathSystemProperties.invoke(null);
URL[] classpath = stream(classpathUrls.spliterator(), false).toArray(URL[]::new);
URLClassLoader classpathClassLoader = new URLClassLoader(classpath, null);
try {
classpathClassLoader.loadClass(clazz.getName());
fail("Class " + clazz.getName() + " can be loaded from the classpath -> the test can't reliably show us, if this class can be found on the modulepath. "
+ "The reason for this might be a wrongly configured test environment, like IntelliJ does "
+ "(throwing everything on the classpath instead of setting up the modulepath). "
+ "Check if this test runs correctly via Gradle!");
} catch (ClassNotFoundException ignored) {
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,26 +15,48 @@
*/
package com.tngtech.archunit.core.importer;

import java.io.File;
import java.lang.module.ModuleFinder;
import java.lang.module.ModuleReference;
import java.net.MalformedURLException;
import java.net.URI;
import java.net.URL;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.List;
import java.util.Set;
import java.util.stream.Stream;

import com.google.common.base.Splitter;

import static com.google.common.base.Strings.nullToEmpty;
import static com.google.common.collect.Iterables.concat;
import static java.util.stream.Collectors.toList;

class ModuleLocationResolver implements LocationResolver {
@Override
public UrlSource resolveClassPath() {
Iterable<URL> classpath = UrlSource.From.classPathSystemProperties();
Iterable<URL> modulepath = ModuleFinder.ofSystem().findAll().stream()
Set<ModuleReference> systemModuleReferences = ModuleFinder.ofSystem().findAll();
Set<ModuleReference> configuredModuleReferences = ModuleFinder.of(modulepath()).findAll();
Iterable<URL> modulepath = Stream.concat(systemModuleReferences.stream(), configuredModuleReferences.stream())
.flatMap(moduleReference -> moduleReference.location().stream())
.map(this::toUrl)
.collect(toList());

return UrlSource.From.iterable(concat(classpath, modulepath));
}

private Path[] modulepath() {
String modulepathProperty = nullToEmpty(System.getProperty("jdk.module.path"));
List<String> modulepath = Splitter.on(File.pathSeparatorChar).omitEmptyStrings().splitToList(modulepathProperty);
Path[] result = new Path[modulepath.size()];
for (int i = 0; i < modulepath.size(); i++) {
result[i] = Paths.get(modulepath.get(i));
}
return result;
}

private URL toUrl(URI uri) {
try {
return uri.toURL();
Expand Down
2 changes: 1 addition & 1 deletion settings.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ plugins {
id "com.gradle.enterprise" version "3.1.1"
}

include 'archunit', 'archunit-example', 'archunit-integration-test',
include 'archunit', 'archunit-example', 'archunit-integration-test', 'archunit-java-modules-test',
'archunit-junit', 'archunit-junit4', 'archunit-junit5-api','archunit-junit5-engine-api','archunit-junit5-engine', 'archunit-junit5',
'archunit-example:example-plain', 'archunit-example:example-junit4', 'archunit-example:example-junit5', 'docs'

Expand Down

0 comments on commit 85bb0e9

Please sign in to comment.