Fixing #654: NPE in SetMojo when a dependency version is null

mojohaus · Aug 28, 2022 · 22ef02f · 22ef02f
1 parent 44431b8
commit 22ef02f
Show file tree

Hide file tree

Showing 6 changed files with 77 additions and 33 deletions.
diff --git a/pom.xml b/pom.xml
@@ -81,6 +81,9 @@
       <email>antoon.johansson@gmail.com</email>
       <timezone>+1</timezone>
     </contributor>
+    <contributor>
+      <name>Andrzej Jarmoniuk</name>
+    </contributor>
   </contributors>
 
   <prerequisites>

diff --git a/src/main/java/org/codehaus/mojo/versions/SetMojo.java b/src/main/java/org/codehaus/mojo/versions/SetMojo.java
@@ -433,10 +433,12 @@ private void applyChange( MavenProject project, SortedMap<String, Model> reactor
         addChange( groupId, artifactId, oldVersion, newVersion );
         // now fake out the triggering change
 
-        final Map.Entry<String, Model> current = PomHelper.getModelEntry( reactor, groupId, artifactId );
-        current.getValue().setVersion( newVersion );
-
-        addFile( files, project, current.getKey() );
+        Map.Entry<String, Model> current = PomHelper.getModelEntry( reactor, groupId, artifactId );
+        if ( current != null )
+        {
+            current.getValue().setVersion( newVersion );
+            addFile( files, project, current.getKey() );
+        }
 
         for ( Map.Entry<String, Model> sourceEntry : reactor.entrySet() )
         {

diff --git a/src/main/java/org/codehaus/mojo/versions/api/PomHelper.java b/src/main/java/org/codehaus/mojo/versions/api/PomHelper.java
@@ -816,7 +816,7 @@ else if ( inMatchScope && matchTargetRegex.matcher( path ).matches() )
                 {
                     if ( "groupId".equals( elementName ) )
                     {
-                        haveGroupId = groupId.equals( pom.getElementText().trim() );
+                        haveGroupId =  pom.getElementText().trim().equals( groupId );
                         path = stack.pop();
                     }
                     else if ( "artifactId".equals( elementName ) )
@@ -1539,12 +1539,14 @@ public static Map<String, Model> getChildModels( Map<String, Model> reactor, Str
      */
     public static Model getModel( Map<String, Model> reactor, String groupId, String artifactId )
     {
-        Map.Entry<String, Model> entry = getModelEntry( reactor, groupId, artifactId );
-        return entry == null ? null : entry.getValue();
+        return reactor.values().stream().filter(
+                model -> ( groupId == null || groupId.equals( getGroupId( model ) ) ) && artifactId.equals(
+                        getArtifactId( model ) ) ).findAny().orElse( null );
     }
 
     /**
-     * Returns the model that has the specified groupId and artifactId or <code>null</code> if no such model exists.
+     * Returns the model that has the specified groupId (if specified)
+     * and artifactId or <code>null</code> if no such model exists.
      *
      * @param reactor    The map of models keyed by path.
      * @param groupId    The groupId to match.
@@ -1554,15 +1556,9 @@ public static Model getModel( Map<String, Model> reactor, String groupId, String
     public static Map.Entry<String, Model> getModelEntry( Map<String, Model> reactor, String groupId,
                                                           String artifactId )
     {
-        for ( Map.Entry<String, Model> entry : reactor.entrySet() )
-        {
-            Model model = entry.getValue();
-            if ( groupId.equals( getGroupId( model ) ) && artifactId.equals( getArtifactId( model ) ) )
-            {
-                return entry;
-            }
-        }
-        return null;
+        return reactor.entrySet().stream().filter(
+                e -> ( groupId == null || groupId.equals( PomHelper.getGroupId( e.getValue() ) ) ) && artifactId.equals(
+                        PomHelper.getArtifactId( e.getValue() ) ) ).findAny().orElse( null );
     }
 
     /**
@@ -1578,15 +1574,12 @@ public static int getReactorParentCount( Map<String, Model> reactor, Model model
         {
             return 0;
         }
-        else
+        Model parentModel = getModel( reactor, model.getParent().getGroupId(), model.getParent().getArtifactId() );
+        if ( parentModel == null )
         {
-            Model parentModel = getModel( reactor, model.getParent().getGroupId(), model.getParent().getArtifactId() );
-            if ( parentModel != null )
-            {
-                return getReactorParentCount( reactor, parentModel ) + 1;
-            }
             return 0;
         }
+        return getReactorParentCount( reactor, parentModel ) + 1;
     }
 
     /**

diff --git a/src/main/java/org/codehaus/mojo/versions/utils/RegexUtils.java b/src/main/java/org/codehaus/mojo/versions/utils/RegexUtils.java
@@ -97,16 +97,18 @@ public static String quote( String s )
     public static int getWildcardScore( String wildcardRule )
     {
         int score = 0;
-        for ( int i = 0; i < wildcardRule.length(); i++ )
+        if ( wildcardRule != null )
         {
-            char c = wildcardRule.charAt( i );
-            if ( c == '?' )
-            {
-                score++;
-            }
-            else if ( c == '*' )
+            for ( char c : wildcardRule.toCharArray() )
             {
-                score += 1000;
+                if ( c == '?' )
+                {
+                    score++;
+                }
+                else if ( c == '*' )
+                {
+                    score += 1000;
+                }
             }
         }
         return score;

diff --git a/src/test/java/org/codehaus/mojo/versions/SetMojoTest.java b/src/test/java/org/codehaus/mojo/versions/SetMojoTest.java
@@ -4,14 +4,14 @@
 import org.apache.maven.plugin.MojoExecutionException;
 import org.apache.maven.plugin.MojoFailureException;
 import org.apache.maven.project.MavenProject;
+import org.codehaus.mojo.versions.utils.BaseMojoTestCase;
 import org.junit.Test;
 
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.is;
-import static org.junit.Assert.fail;
 
-public class SetMojoTest
+public class SetMojoTest extends BaseMojoTestCase
 {
     @Test
     public void testGetIncrementedVersion() throws MojoExecutionException
@@ -92,4 +92,10 @@ public void testNextSnapshotIndexWithoutNextSnapshot() throws MojoFailureExcepti
         }
     }
 
+    @Test
+    public void testVersionlessDependency() throws Exception
+    {
+        SetMojo myMojo = createMojo( "set", "src/test/resources/org/codehaus/mojo/set/versionless-01/pom.xml" );
+        myMojo.execute();
+    }
 }
diff --git a/src/test/resources/org/codehaus/mojo/set/versionless-01/pom.xml b/src/test/resources/org/codehaus/mojo/set/versionless-01/pom.xml
@@ -0,0 +1,38 @@
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+    <modelVersion>4.0.0</modelVersion>
+    <groupId>default-group</groupId>
+    <artifactId>default-artifact</artifactId>
+    <version>1.0</version>
+    <packaging>pom</packaging>
+
+    <dependencyManagement>
+        <dependencies>
+            <dependency>
+                <groupId>localhost</groupId>
+                <artifactId>dummy-api</artifactId>
+                <version>1.1</version>
+            </dependency>
+        </dependencies>
+    </dependencyManagement>
+
+    <dependencies>
+        <dependency>
+            <groupId>localhost</groupId>
+            <artifactId>dummy-api</artifactId>
+        </dependency>
+    </dependencies>
+
+    <build>
+        <plugins>
+            <plugin>
+                <artifactId>versions-maven-plugin</artifactId>
+                <configuration>
+                    <updateBuildOutputTimestampPolicy>onchange</updateBuildOutputTimestampPolicy>
+                    <artifactId>dummy-api</artifactId>
+                    <newVersion>2.0</newVersion>
+                </configuration>
+            </plugin>
+        </plugins>
+    </build>
+</project>