Skip to content

Commit

Permalink
Fail on duplicate modules on the module path
Browse files Browse the repository at this point in the history
When resolving the module path, no module id is allowed to exist
multiple times.

The location manager used to drop duplicates silently which led to
hard to understand error messages when a plugin using plexus-java
dropped a dependency without any notice.

This change enforces uniqueness of module names on the module path
and will add an exception to the PathResolveResult in that case.
  • Loading branch information
hgschmie authored and rfscholte committed Sep 18, 2023
1 parent 605780b commit 3a1f22b
Show file tree
Hide file tree
Showing 4 changed files with 78 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,12 @@ public String extract(Path path) throws IOException {
.put(
entry.getKey(),
moduleNameSources.get(entry.getValue().name()));
} else {
result.getPathExceptions()
.put(
entry.getKey(),
new IllegalStateException(
"Module '" + entry.getValue().name() + "' is already on the module path!"));
}
} else {
result.getClasspathElements().add(entry.getKey());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,36 @@ void testIdenticalModuleNames() throws Exception {
ResolvePathsRequest<Path> request =
ResolvePathsRequest.ofPaths(Arrays.asList(pj1, pj2)).setMainModuleDescriptor(mockModuleInfoJava);

when(asmParser.getModuleDescriptor(pj1))
.thenReturn(JavaModuleDescriptor.newModule("plexus.java").build());
when(asmParser.getModuleDescriptor(pj2))
.thenReturn(JavaModuleDescriptor.newModule("plexus.java").build());

ResolvePathsResult<Path> result = locationManager.resolvePaths(request);

assertThat(result.getMainModuleDescriptor()).isEqualTo(descriptor);
assertThat(result.getPathElements()).hasSize(2);
assertThat(result.getModulepathElements()).hasSize(1);
assertThat(result.getModulepathElements()).containsKey(pj1);
assertThat(result.getModulepathElements()).doesNotContainKey(pj2);
assertThat(result.getClasspathElements()).isEmpty();
// duplicate is flagged as an error
assertThat(result.getPathExceptions()).containsOnlyKeys(pj2);
assertThat(result.getPathExceptions().get(pj2))
.isInstanceOf(IllegalStateException.class)
.hasMessageContaining("Module 'plexus.java' is already on the module path!");
}

@Test
public void testIdenticalAutomaticModuleNames() throws Exception {
Path pj1 = Paths.get("src/test/resources/jar.empty/plexus-java-1.0.0-SNAPSHOT.jar");
Path pj2 = Paths.get("src/test/resources/jar.empty.2/plexus-java-2.0.0-SNAPSHOT.jar");
JavaModuleDescriptor descriptor =
JavaModuleDescriptor.newModule("base").requires("plexus.java").build();
when(qdoxParser.fromSourcePath(any(Path.class))).thenReturn(descriptor);
ResolvePathsRequest<Path> request =
ResolvePathsRequest.ofPaths(Arrays.asList(pj1, pj2)).setMainModuleDescriptor(mockModuleInfoJava);

when(asmParser.getModuleDescriptor(pj1))
.thenReturn(
JavaModuleDescriptor.newAutomaticModule("plexus.java").build());
Expand All @@ -173,8 +203,42 @@ void testIdenticalModuleNames() throws Exception {
assertThat(result.getPathElements()).hasSize(2);
assertThat(result.getModulepathElements()).containsOnlyKeys(pj1);
assertThat(result.getModulepathElements()).doesNotContainKey(pj2);
assertThat(result.getClasspathElements()).hasSize(0);
assertThat(result.getPathExceptions()).hasSize(0);
assertThat(result.getClasspathElements()).isEmpty();
// duplicate is flagged as an error
assertThat(result.getPathExceptions()).containsOnlyKeys(pj2);
assertThat(result.getPathExceptions().get(pj2))
.isInstanceOf(IllegalStateException.class)
.hasMessageContaining("Module 'plexus.java' is already on the module path!");
}

@Test
public void testMainJarModuleAndTestJarAutomatic() throws Exception {
Path pj1 = Paths.get("src/test/resources/jar.tests/plexus-java-1.0.0-SNAPSHOT.jar");
Path pj2 = Paths.get("src/test/resources/jar.tests/plexus-java-1.0.0-SNAPSHOT-tests.jar");
JavaModuleDescriptor descriptor =
JavaModuleDescriptor.newModule("base").requires("plexus.java").build();
when(qdoxParser.fromSourcePath(any(Path.class))).thenReturn(descriptor);
ResolvePathsRequest<Path> request =
ResolvePathsRequest.ofPaths(Arrays.asList(pj1, pj2)).setMainModuleDescriptor(mockModuleInfoJava);

when(asmParser.getModuleDescriptor(pj1))
.thenReturn(JavaModuleDescriptor.newModule("plexus.java").build());
when(asmParser.getModuleDescriptor(pj2)).thenReturn(null);

ResolvePathsResult<Path> result = locationManager.resolvePaths(request);

assertThat(result.getMainModuleDescriptor()).isEqualTo(descriptor);
assertThat(result.getPathElements()).hasSize(2);
assertThat(result.getModulepathElements()).hasSize(1);
assertThat(result.getModulepathElements()).containsKey(pj1);
assertThat(result.getModulepathElements()).doesNotContainKey(pj2);
assertThat(result.getClasspathElements()).isEmpty();

// duplicate is flagged as an error
assertThat(result.getPathExceptions()).containsOnlyKeys(pj2);
assertThat(result.getPathExceptions().get(pj2))
.isInstanceOf(IllegalStateException.class)
.hasMessageContaining("Module 'plexus.java' is already on the module path!");
}

@Test
Expand Down Expand Up @@ -473,9 +537,13 @@ void testDuplicateModule() throws Exception {
ResolvePathsResult<Path> result = locationManager.resolvePaths(request);
assertThat(result.getPathElements()).hasSize(2);
assertThat(result.getModulepathElements()).containsOnlyKeys(moduleB);
// with current default the duplicate will be ignored
assertThat(result.getClasspathElements()).hasSize(0);
assertThat(result.getPathExceptions()).hasSize(0);
assertThat(result.getPathExceptions()).hasSize(1);
// duplicate (module B / module C) is flagged as an error
assertThat(result.getPathExceptions()).containsOnlyKeys(moduleC);
assertThat(result.getPathExceptions().get(moduleC))
.isInstanceOf(IllegalStateException.class)
.hasMessageContaining("Module 'anonymous' is already on the module path!");
}

@Test
Expand Down
Binary file not shown.
Binary file not shown.

0 comments on commit 3a1f22b

Please sign in to comment.