From 01e40fc8303670db1840c463a8f973dd9d17f83a Mon Sep 17 00:00:00 2001 From: elharo Date: Thu, 30 Jul 2026 13:53:08 +0000 Subject: [PATCH 1/6] MNG-7531: Prevent NPE in inheritance assembler when project directory is root path or artifactId is null --- .../model/DefaultInheritanceAssembler.java | 3 +- .../DefaultInheritanceAssemblerTest.java | 106 +++++++++--------- 2 files changed, 57 insertions(+), 52 deletions(-) diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultInheritanceAssembler.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultInheritanceAssembler.java index aeacef7b530a..b6d7230b4f41 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultInheritanceAssembler.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultInheritanceAssembler.java @@ -105,7 +105,8 @@ private String getChildPathAdjustment(Model child, Model parent, String childDir * repository. In other words, modules where artifactId != moduleDirName will see different effective URLs * depending on how the model was constructed (from filesystem or from repository). */ - if (child.getProjectDirectory() != null) { + if (child.getProjectDirectory() != null + && child.getProjectDirectory().getFileName() != null) { childName = child.getProjectDirectory().getFileName().toString(); } diff --git a/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultInheritanceAssemblerTest.java b/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultInheritanceAssemblerTest.java index e8ee2d37a111..ca260b35ef59 100644 --- a/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultInheritanceAssemblerTest.java +++ b/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultInheritanceAssemblerTest.java @@ -18,81 +18,85 @@ */ package org.apache.maven.impl.model; -import java.nio.file.Files; -import java.nio.file.Path; import java.nio.file.Paths; +import java.util.List; import org.apache.maven.api.model.Model; -import org.apache.maven.api.services.xml.XmlReaderRequest; -import org.apache.maven.api.services.xml.XmlWriterRequest; -import org.apache.maven.impl.DefaultModelXmlFactory; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import org.xmlunit.builder.DiffBuilder; -import org.xmlunit.diff.Diff; -import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertNotNull; class DefaultInheritanceAssemblerTest { - private DefaultModelXmlFactory xmlFactory; - private DefaultInheritanceAssembler assembler; @BeforeEach void setUp() { - xmlFactory = new DefaultModelXmlFactory(); assembler = new DefaultInheritanceAssembler(); } - private Path getPom(String name) { - return Paths.get("../../compat/maven-model-builder/src/test/resources/poms/inheritance/" + name + ".xml"); - } - - private Model getModel(String name) throws Exception { - return xmlFactory.read(XmlReaderRequest.builder().path(getPom(name)).build()); - } - @Test - void testPluginConfiguration() throws Exception { - testInheritance("plugin-configuration"); - } + void testAssembleWithNullArtifactIdDoesNotThrowNpe() { + Model parent = Model.newBuilder() + .modelVersion("4.0.0") + .groupId("test") + .artifactId("parent") + .version("1.0") + .build(); - public void testInheritance(String baseName) throws Exception { - testInheritance(baseName, false); - testInheritance(baseName, true); - } + Model child = Model.newBuilder() + .modelVersion("4.0.0") + .groupId("test") + .version("1.0") + .build(); - public void testInheritance(String baseName, boolean fromRepo) throws Exception { - Model parent = getModel(baseName + "-parent"); - Model child = getModel(baseName + "-child"); + assertNotNull(assembler); + assertDoesNotThrow(() -> assembler.assembleModelInheritance(child, parent, null, null)); + } - if (!fromRepo) { - // when model is built from disk, pomFile is set - // (has consequences in inheritance algorithm since getProjectDirectory() returns non-null) - parent = parent.withPomFile(getPom(baseName + "-parent").toAbsolutePath()); - child = child.withPomFile(getPom(baseName + "-child").toAbsolutePath()); - } + @Test + void testAssembleWithRootProjectDirectoryDoesNotThrowNpe() { + Model parent = Model.newBuilder() + .modelVersion("4.0.0") + .groupId("test") + .artifactId("parent") + .version("1.0") + .build(); - Model assembled = assembler.assembleModelInheritance(child, parent, null, null); + // child has pomFile at root, so getProjectDirectory() returns root path + // and getFileName() on root path returns null + Model child = Model.newBuilder() + .modelVersion("4.0.0") + .groupId("test") + .artifactId("child") + .version("1.0") + .pomFile(Paths.get("/pom.xml")) + .build(); - // write baseName + "-actual" - Path actual = Paths.get( - "target/test-classes/poms/inheritance/" + baseName + (fromRepo ? "-build" : "-repo") + "-actual.xml"); - Files.createDirectories(actual.getParent()); - xmlFactory.write(XmlWriterRequest.builder() - .content(assembled) - .path(actual) - .build()); + assertNotNull(assembler); + assertDoesNotThrow(() -> assembler.assembleModelInheritance(child, parent, null, null)); + } - // check with getPom( baseName + "-expected" ) - Path expected = getPom(baseName + "-expected"); + @Test + void testAssembleWithNullArtifactIdAndRootProjectDirectoryDoesNotThrowNpe() { + Model parent = Model.newBuilder() + .modelVersion("4.0.0") + .groupId("test") + .artifactId("parent") + .version("1.0") + .modules(List.of("../child/pom.xml")) + .build(); - Diff diff = DiffBuilder.compare(expected.toFile()) - .withTest(actual.toFile()) - .ignoreComments() - .ignoreWhitespace() + Model child = Model.newBuilder() + .modelVersion("4.0.0") + .groupId("test") + .version("1.0") + .pomFile(Paths.get("/pom.xml")) .build(); - assertFalse(diff.hasDifferences(), "XML files should be identical: " + diff.toString()); + + assertNotNull(assembler); + assertDoesNotThrow(() -> assembler.assembleModelInheritance(child, parent, null, null)); } } From d47349604409ccbcd6a63d07e287d019e5bcdb10 Mon Sep 17 00:00:00 2001 From: Elliotte Rusty Harold Date: Thu, 30 Jul 2026 14:02:16 +0000 Subject: [PATCH 2/6] Remove assertNotNull assertions from tests Removed assertNotNull checks for assembler in tests. --- .../maven/impl/model/DefaultInheritanceAssemblerTest.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultInheritanceAssemblerTest.java b/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultInheritanceAssemblerTest.java index ca260b35ef59..10564a98c657 100644 --- a/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultInheritanceAssemblerTest.java +++ b/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultInheritanceAssemblerTest.java @@ -26,7 +26,6 @@ import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; -import static org.junit.jupiter.api.Assertions.assertNotNull; class DefaultInheritanceAssemblerTest { @@ -52,7 +51,6 @@ void testAssembleWithNullArtifactIdDoesNotThrowNpe() { .version("1.0") .build(); - assertNotNull(assembler); assertDoesNotThrow(() -> assembler.assembleModelInheritance(child, parent, null, null)); } @@ -75,7 +73,6 @@ void testAssembleWithRootProjectDirectoryDoesNotThrowNpe() { .pomFile(Paths.get("/pom.xml")) .build(); - assertNotNull(assembler); assertDoesNotThrow(() -> assembler.assembleModelInheritance(child, parent, null, null)); } @@ -96,7 +93,6 @@ void testAssembleWithNullArtifactIdAndRootProjectDirectoryDoesNotThrowNpe() { .pomFile(Paths.get("/pom.xml")) .build(); - assertNotNull(assembler); assertDoesNotThrow(() -> assembler.assembleModelInheritance(child, parent, null, null)); } } From 2cda0992baeb5acd4fa498549410e948485f89e5 Mon Sep 17 00:00:00 2001 From: elharo Date: Thu, 30 Jul 2026 14:24:02 +0000 Subject: [PATCH 3/6] Restore XML-based inheritance tests alongside new NPE tests Restore XML-based inheritance tests alongside new NPE tests --- .../DefaultInheritanceAssemblerTest.java | 115 +++++++++++++++++- 1 file changed, 113 insertions(+), 2 deletions(-) diff --git a/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultInheritanceAssemblerTest.java b/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultInheritanceAssemblerTest.java index 10564a98c657..26ad975232a7 100644 --- a/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultInheritanceAssemblerTest.java +++ b/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultInheritanceAssemblerTest.java @@ -18,24 +18,137 @@ */ package org.apache.maven.impl.model; +import java.nio.file.Files; +import java.nio.file.Path; import java.nio.file.Paths; import java.util.List; import org.apache.maven.api.model.Model; +import org.apache.maven.api.services.xml.XmlReaderRequest; +import org.apache.maven.api.services.xml.XmlWriterRequest; +import org.apache.maven.impl.DefaultModelXmlFactory; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.xmlunit.builder.DiffBuilder; +import org.xmlunit.diff.Diff; import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; class DefaultInheritanceAssemblerTest { + private DefaultModelXmlFactory xmlFactory; + private DefaultInheritanceAssembler assembler; @BeforeEach void setUp() { + xmlFactory = new DefaultModelXmlFactory(); assembler = new DefaultInheritanceAssembler(); } + private Path getPom(String name) { + return Paths.get("../../compat/maven-model-builder/src/test/resources/poms/inheritance/" + name + ".xml"); + } + + private Model getModel(String name) throws Exception { + return xmlFactory.read(XmlReaderRequest.builder().path(getPom(name)).build()); + } + + @Test + void testPluginConfiguration() throws Exception { + testInheritance("plugin-configuration"); + } + + @Test + void testUrls() throws Exception { + testInheritance("urls"); + } + + @Test + void testFlatUrls() throws Exception { + testInheritance("flat-urls"); + } + + @Test + void testNoAppendUrls() throws Exception { + testInheritance("no-append-urls"); + } + + @Test + void testNoAppendUrls2() throws Exception { + testInheritance("no-append-urls2"); + } + + @Test + void testNoAppendUrls3() throws Exception { + testInheritance("no-append-urls3"); + } + + @Test + void testWithEmptyUrl() throws Exception { + testInheritance("empty-urls", false); + } + + @Test + void testModulePathNotArtifactId() throws Exception { + Model parent = getModel("module-path-not-artifactId-parent"); + Model child = getModel("module-path-not-artifactId-child"); + + Model assembled = assembler.assembleModelInheritance(child, parent, null, null); + + Path actual = Paths.get("target/test-classes/poms/inheritance/module-path-not-artifactId-actual.xml"); + Files.createDirectories(actual.getParent()); + xmlFactory.write(XmlWriterRequest.builder() + .content(assembled) + .path(actual) + .build()); + + Path expected = getPom("module-path-not-artifactId-expected"); + + Diff diff = DiffBuilder.compare(expected.toFile()) + .withTest(actual.toFile()) + .ignoreComments() + .ignoreWhitespace() + .build(); + assertFalse(diff.hasDifferences(), "XML files should be identical: " + diff.toString()); + } + + public void testInheritance(String baseName) throws Exception { + testInheritance(baseName, false); + testInheritance(baseName, true); + } + + public void testInheritance(String baseName, boolean fromRepo) throws Exception { + Model parent = getModel(baseName + "-parent"); + Model child = getModel(baseName + "-child"); + + if (!fromRepo) { + parent = parent.withPomFile(getPom(baseName + "-parent").toAbsolutePath()); + child = child.withPomFile(getPom(baseName + "-child").toAbsolutePath()); + } + + Model assembled = assembler.assembleModelInheritance(child, parent, null, null); + + Path actual = Paths.get( + "target/test-classes/poms/inheritance/" + baseName + (fromRepo ? "-build" : "-repo") + "-actual.xml"); + Files.createDirectories(actual.getParent()); + xmlFactory.write(XmlWriterRequest.builder() + .content(assembled) + .path(actual) + .build()); + + Path expected = getPom(baseName + "-expected"); + + Diff diff = DiffBuilder.compare(expected.toFile()) + .withTest(actual.toFile()) + .ignoreComments() + .ignoreWhitespace() + .build(); + assertFalse(diff.hasDifferences(), "XML files should be identical: " + diff.toString()); + } + @Test void testAssembleWithNullArtifactIdDoesNotThrowNpe() { Model parent = Model.newBuilder() @@ -63,8 +176,6 @@ void testAssembleWithRootProjectDirectoryDoesNotThrowNpe() { .version("1.0") .build(); - // child has pomFile at root, so getProjectDirectory() returns root path - // and getFileName() on root path returns null Model child = Model.newBuilder() .modelVersion("4.0.0") .groupId("test") From 79f0bb3deb27a6509cb2341bd22977775634b4d6 Mon Sep 17 00:00:00 2001 From: elharo Date: Thu, 30 Jul 2026 14:36:05 +0000 Subject: [PATCH 4/6] Apply spotless formatting --- .../apache/maven/impl/model/DefaultInheritanceAssemblerTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultInheritanceAssemblerTest.java b/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultInheritanceAssemblerTest.java index 26ad975232a7..17abd5f4ef5f 100644 --- a/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultInheritanceAssemblerTest.java +++ b/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultInheritanceAssemblerTest.java @@ -34,7 +34,6 @@ import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertNotNull; class DefaultInheritanceAssemblerTest { From c8118de1b812b10dd6ec1b14bc42ba1536de7329 Mon Sep 17 00:00:00 2001 From: elharo Date: Thu, 30 Jul 2026 14:38:33 +0000 Subject: [PATCH 5/6] Restore Javadoc comments on ported tests --- .../model/DefaultInheritanceAssemblerTest.java | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultInheritanceAssemblerTest.java b/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultInheritanceAssemblerTest.java index 17abd5f4ef5f..e20b028c6cd9 100644 --- a/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultInheritanceAssemblerTest.java +++ b/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultInheritanceAssemblerTest.java @@ -60,26 +60,42 @@ void testPluginConfiguration() throws Exception { testInheritance("plugin-configuration"); } + /** + * Check most classical urls inheritance: directory structure where parent POM in parent directory + * and child directory == artifactId + */ @Test void testUrls() throws Exception { testInheritance("urls"); } + /** + * Flat directory structure: parent & child POMs in sibling directories, child directory == artifactId. + */ @Test void testFlatUrls() throws Exception { testInheritance("flat-urls"); } + /** + * MNG-5951 MNG-6059 child.x.y.inherit.append.path="false" test + */ @Test void testNoAppendUrls() throws Exception { testInheritance("no-append-urls"); } + /** + * MNG-5951 special case test: inherit with partial override + */ @Test void testNoAppendUrls2() throws Exception { testInheritance("no-append-urls2"); } + /** + * MNG-5951 special case test: child.x.y.inherit.append.path="true" in child should not reset content + */ @Test void testNoAppendUrls3() throws Exception { testInheritance("no-append-urls3"); From 20643c16a9e8f96e48dccd224ffcfc7323968c14 Mon Sep 17 00:00:00 2001 From: elharo Date: Thu, 30 Jul 2026 14:54:16 +0000 Subject: [PATCH 6/6] Restore inline code comments in testInheritance method --- .../maven/impl/model/DefaultInheritanceAssemblerTest.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultInheritanceAssemblerTest.java b/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultInheritanceAssemblerTest.java index e20b028c6cd9..b7e32d3c89ff 100644 --- a/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultInheritanceAssemblerTest.java +++ b/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultInheritanceAssemblerTest.java @@ -140,12 +140,15 @@ public void testInheritance(String baseName, boolean fromRepo) throws Exception Model child = getModel(baseName + "-child"); if (!fromRepo) { + // when model is built from disk, pomFile is set + // (has consequences in inheritance algorithm since getProjectDirectory() returns non-null) parent = parent.withPomFile(getPom(baseName + "-parent").toAbsolutePath()); child = child.withPomFile(getPom(baseName + "-child").toAbsolutePath()); } Model assembled = assembler.assembleModelInheritance(child, parent, null, null); + // write baseName + "-actual" Path actual = Paths.get( "target/test-classes/poms/inheritance/" + baseName + (fromRepo ? "-build" : "-repo") + "-actual.xml"); Files.createDirectories(actual.getParent()); @@ -154,6 +157,7 @@ public void testInheritance(String baseName, boolean fromRepo) throws Exception .path(actual) .build()); + // check with getPom( baseName + "-expected" ) Path expected = getPom(baseName + "-expected"); Diff diff = DiffBuilder.compare(expected.toFile())