You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by "efegokdemir (via GitHub)" <gi...@apache.org> on 2026/09/24 06:24:13 UTC

[PR] Avoid null-pointer failure before EAR execution context setup [maven-ear-plugin]

efegokdemir opened a new pull request, #563:
URL: https://github.com/apache/maven-ear-plugin/pull/563

   ## Summary
   
   Configured EAR modules can attempt artifact resolution before their execution context is initialized. That currently produces an opaque `NullPointerException` when the module accesses the artifact repository.
   
   This adds an explicit `MojoFailureException` with the module identity and a regression test for the uninitialized-context path.
   
   Fixes #514
   
   ## Testing
   
   - `mvn -DskipITs -Dspotless.check.skip=true -Dspotless.apply.skip=true test` — passed (49 tests).
   - The focused regression failed on the unchanged implementation with the reported `NullPointerException` and passed with this change.
   - `git diff --check` — passed.
   - The repository's Spotless check was attempted but is blocked locally by a Palantir formatter/Java incompatibility (`NoSuchFieldError` in `com.sun.tools.javac.tree`).
   
   ## Notes
   
   This contribution was developed with AI assistance. I reviewed the complete diff and verified the failure before and after the fix. No competing open PR for #514 was found.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Avoid null-pointer failure before EAR execution context setup [maven-ear-plugin]

Posted by "elharo (via GitHub)" <gi...@apache.org>.
elharo commented on code in PR #563:
URL: https://github.com/apache/maven-ear-plugin/pull/563#discussion_r4360015544


##########
src/test/java/org/apache/maven/plugins/ear/EarModuleTest.java:
##########
@@ -40,4 +45,22 @@ void testCleanArchivePath() {
         assertEquals("", AbstractEarModule.cleanArchivePath(""));
         assertNull(AbstractEarModule.cleanArchivePath(null));
     }
+
+    @Test
+    void testResolveArtifactRequiresExecutionContext() throws Exception {
+        JarModule module = new JarModule();
+        setField(module, "groupId", "groupId");
+        setField(module, "artifactId", "artifactId");
+
+        MojoFailureException exception = assertThrows(
+                MojoFailureException.class, () -> module.resolveArtifact(Collections.emptySet()));
+
+        assertEquals("Ear execution context not initialized for module jar:groupId:artifactId", exception.getMessage());
+    }
+
+    private static void setField(Object target, String name, Object value) throws Exception {

Review Comment:
   If you're using reflection to test something, you're doing it wrong. Always test through public APIs and observable behavior. 



##########
src/main/java/org/apache/maven/plugins/ear/AbstractEarModule.java:
##########
@@ -155,6 +155,9 @@ public void resolveArtifact(Set<Artifact> artifacts) throws EarPluginException,
                 throw new MojoFailureException(
                         "Could not resolve artifact[" + groupId + ":" + artifactId + ":" + getType() + "]");
             }
+            if (earExecutionContext == null) {
+                throw new MojoFailureException("Ear execution context not initialized for module " + this);

Review Comment:
   EAR



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Avoid null-pointer failure before EAR execution context setup [maven-ear-plugin]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #563:
URL: https://github.com/apache/maven-ear-plugin/pull/563#discussion_r4360162669


##########
src/test/java/org/apache/maven/plugins/ear/EarModuleTest.java:
##########
@@ -40,4 +45,22 @@ void testCleanArchivePath() {
         assertEquals("", AbstractEarModule.cleanArchivePath(""));
         assertNull(AbstractEarModule.cleanArchivePath(null));
     }
+
+    @Test
+    void testResolveArtifactRequiresExecutionContext() throws Exception {
+        JarModule module = new JarModule();
+        setField(module, "groupId", "groupId");
+        setField(module, "artifactId", "artifactId");
+
+        MojoFailureException exception = assertThrows(
+                MojoFailureException.class, () -> module.resolveArtifact(Collections.emptySet()));
+
+        assertEquals("Ear execution context not initialized for module jar:groupId:artifactId", exception.getMessage());
+    }
+
+    private static void setField(Object target, String name, Object value) throws Exception {
+        Field field = AbstractEarModule.class.getDeclaredField(name);
+        field.setAccessible(true);
+        field.set(target, value);
+    }

Review Comment:
   The test uses reflection to mutate private fields, which is brittle and can break on refactors (e.g., renames) unrelated to behavior. Prefer constructing a module with the needed identity via public/protected APIs (setters/constructor) if available, or create a minimal test subclass in the test that exposes a supported way to set `groupId`/`artifactId` for testing.



##########
src/main/java/org/apache/maven/plugins/ear/AbstractEarModule.java:
##########
@@ -155,6 +155,9 @@ public void resolveArtifact(Set<Artifact> artifacts) throws EarPluginException,
                 throw new MojoFailureException(
                         "Could not resolve artifact[" + groupId + ":" + artifactId + ":" + getType() + "]");
             }
+            if (earExecutionContext == null) {
+                throw new MojoFailureException("Ear execution context not initialized for module " + this);
+            }

Review Comment:
   This null check occurs after `getType()` is used earlier in the method (see the error message construction preceding this hunk). If `getType()` depends on `earExecutionContext` (directly or indirectly), the method can still throw a `NullPointerException` before reaching this guard. Move the `earExecutionContext == null` check earlier (before any calls that might consult the context), or ensure `getType()` is context-independent.



##########
src/test/java/org/apache/maven/plugins/ear/EarModuleTest.java:
##########
@@ -40,4 +45,22 @@ void testCleanArchivePath() {
         assertEquals("", AbstractEarModule.cleanArchivePath(""));
         assertNull(AbstractEarModule.cleanArchivePath(null));
     }
+
+    @Test
+    void testResolveArtifactRequiresExecutionContext() throws Exception {
+        JarModule module = new JarModule();
+        setField(module, "groupId", "groupId");
+        setField(module, "artifactId", "artifactId");
+
+        MojoFailureException exception = assertThrows(
+                MojoFailureException.class, () -> module.resolveArtifact(Collections.emptySet()));
+
+        assertEquals("Ear execution context not initialized for module jar:groupId:artifactId", exception.getMessage());

Review Comment:
   The assertion hard-codes the full exception message, which couples the test to `toString()` formatting and exact wording. Consider asserting on stable parts (e.g., `assertTrue(message.contains(\"Ear execution context not initialized\"))` and `contains(\"jar:groupId:artifactId\")`) or switching to `assertThrows` + message substring checks, to keep the test focused on behavior rather than exact formatting.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org