[MINVOKER-297] NPE when non-existing Maven Home

pom.xml

@@ -343,6 +343,14 @@ under the License.
           <artifactId>maven-invoker-plugin</artifactId>
           <version>3.2.2</version>
         </plugin>
+        <plugin>
+          <artifactId>maven-surefire-plugin</artifactId>
+          <configuration>
+            <systemPropertyVariables>
+              <maven.home>${maven.home}</maven.home>
+            </systemPropertyVariables>
+          </configuration>
+        </plugin>
       </plugins>
     </pluginManagement>
 

src/main/java/org/apache/maven/plugins/invoker/AbstractInvokerMojo.java

@@ -761,6 +761,8 @@ public void execute()
             return;
         }
 
+        setupActualMavenVersion();
+
         handleScriptRunnerWithScriptClassPath();
 
         Collection<String> collectedProjects = new LinkedHashSet<>();
@@ -830,6 +832,25 @@ else if ( lastModifiedRecursive( projectsDirectory ) <= lastModifiedRecursive( c
 
     }
 
+    private void setupActualMavenVersion() throws MojoExecutionException
+    {
+        if ( mavenHome != null )
+        {
+            try
+            {
+                actualMavenVersion = SelectorUtils.getMavenVersion( mavenHome );
+            }
+            catch ( IOException e )
+            {
+                throw new MojoExecutionException( e.getMessage(), e );
+            }
+        }
+        else
+        {
+            actualMavenVersion = SelectorUtils.getMavenVersion();
+        }
+    }
+
     /**
      * Find the latest lastModified recursively within a directory structure.
      *
@@ -912,6 +933,7 @@ private void handleScriptRunnerWithScriptClassPath()
         scriptRunner = new ScriptRunner( );
         scriptRunner.setScriptEncoding( encoding );
         scriptRunner.setGlobalVariable( "localRepositoryPath", localRepositoryPath );
+        scriptRunner.setGlobalVariable( "mavenVersion", actualMavenVersion );
         if ( scriptVariables != null )
         {
             scriptVariables.forEach( ( key, value ) -> scriptRunner.setGlobalVariable( key, value ) );
@@ -1267,16 +1289,6 @@ private void runBuilds( final File projectsDir, List<BuildJob> buildJobs, int ru
 
         final File mergedSettingsFile = mergeSettings( interpolatedSettingsFile );
 
-        if ( mavenHome != null )
-        {
-            actualMavenVersion = SelectorUtils.getMavenVersion( mavenHome );
-        }
-        else
-        {
-            actualMavenVersion = SelectorUtils.getMavenVersion();
-        }
-        scriptRunner.setGlobalVariable( "mavenVersion", actualMavenVersion );
-
         final CharSequence actualJreVersion;
         // @todo if ( javaVersions ) ... to be picked up from toolchains
         if ( javaHome != null )

src/main/java/org/apache/maven/plugins/invoker/SelectorUtils.java

@@ -20,7 +20,9 @@
  */
 
 import java.io.File;
+import java.io.FileNotFoundException;
 import java.io.IOException;
+import java.io.InputStream;
 import java.net.MalformedURLException;
 import java.net.URL;
 import java.util.ArrayList;
@@ -109,7 +111,8 @@ static String getMavenVersion()
             // if this ever changes, we will have to revisit this code.
             Properties properties = new Properties();
             // CHECKSTYLE_OFF: LineLength
-            properties.load( MavenProject.class.getClassLoader().getResourceAsStream( "META-INF/maven/org.apache.maven/maven-core/pom.properties" ) );
+            properties.load( MavenProject.class.getClassLoader()
+                                 .getResourceAsStream( "META-INF/maven/org.apache.maven/maven-core/pom.properties" ) );
             // CHECKSTYLE_ON: LineLength
             return StringUtils.trim( properties.getProperty( "version" ) );
         }
@@ -119,33 +122,35 @@ static String getMavenVersion()
         }
     }
 
-    static String getMavenVersion( File mavenHome )
+    static String getMavenVersion( File mavenHome ) throws IOException
     {
         File mavenLib = new File( mavenHome, "lib" );
         File[] jarFiles = mavenLib.listFiles( ( dir, name ) -> name.endsWith( ".jar" ) );
 
+        if ( jarFiles == null )
+        {
+            throw new IllegalArgumentException( "Invalid Maven home installation directory: " + mavenHome );
+        }
+
         for ( File file : jarFiles )
         {
             try
             {
-                @SuppressWarnings( "deprecation" )
-                URL url =
-                    new URL( "jar:" + file.toURL().toExternalForm()
-                        + "!/META-INF/maven/org.apache.maven/maven-core/pom.properties" );
-
-                Properties properties = new Properties();
-                properties.load( url.openStream() );
-                String version = StringUtils.trim( properties.getProperty( "version" ) );
-                if ( version != null )
+                URL url = new URL( "jar:" + file.toURI().toURL().toExternalForm()
+                                 + "!/META-INF/maven/org.apache.maven/maven-core/pom.properties" );
+
+                try ( InputStream in = url.openStream() )
                 {
-                    return version;
+                    Properties properties = new Properties();
+                    properties.load( in );
+                    String version = StringUtils.trim( properties.getProperty( "version" ) );
+                    if ( version != null )
+                    {
+                        return version;
+                    }
                 }
             }
-            catch ( MalformedURLException e )
-            {
-                // ignore
-            }
-            catch ( IOException e )
+            catch ( FileNotFoundException | MalformedURLException e )
             {
                 // ignore
             }
@@ -251,7 +256,7 @@ static List<Integer> parseVersion( String version )
 
     static int compareVersions( List<Integer> version1, List<Integer> version2 )
     {
-        for ( Iterator<Integer> it1 = version1.iterator(), it2 = version2.iterator();; )
+        for ( Iterator<Integer> it1 = version1.iterator(), it2 = version2.iterator(); ; )
         {
             if ( !it1.hasNext() )
             {

src/test/java/org/apache/maven/plugins/invoker/SelectorUtilsTest.java

@@ -19,13 +19,8 @@
  * under the License.
  */
 
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.when;
-import static org.mockito.Mockito.isA;
-
+import java.io.File;
+import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
@@ -36,6 +31,15 @@
 import org.apache.maven.toolchain.ToolchainPrivate;
 import org.junit.Test;
 
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatCode;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.isA;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
 /**
  * Tests {@link SelectorUtils}.
  *
@@ -120,26 +124,45 @@ public void testIsMatchingToolchain() throws Exception
         when( jdkMismatch.getType() ).thenReturn( "jdk" );
 
         when( toolchainPrivateManager.getToolchainPrivates( "jdk" ) )
-                .thenReturn( new ToolchainPrivate[] { jdkMatching } );
+            .thenReturn( new ToolchainPrivate[] {jdkMatching} );
         assertTrue( SelectorUtils.isToolchain( toolchainPrivateManager, Collections.singleton( openJdk9 ) ) );
 
         when( toolchainPrivateManager.getToolchainPrivates( "jdk" ) )
-                .thenReturn( new ToolchainPrivate[] { jdkMismatch } );
+            .thenReturn( new ToolchainPrivate[] {jdkMismatch} );
         assertFalse( SelectorUtils.isToolchain( toolchainPrivateManager, Collections.singleton( openJdk9 ) ) );
 
         when( toolchainPrivateManager.getToolchainPrivates( "jdk" ) )
-                .thenReturn( new ToolchainPrivate[] { jdkMatching, jdkMismatch, jdkMatching } );
+            .thenReturn( new ToolchainPrivate[] {jdkMatching, jdkMismatch, jdkMatching} );
         assertTrue( SelectorUtils.isToolchain( toolchainPrivateManager, Collections.singleton( openJdk9 ) ) );
 
         when( toolchainPrivateManager.getToolchainPrivates( "jdk" ) )
-                .thenReturn( new ToolchainPrivate[0] );
+            .thenReturn( new ToolchainPrivate[0] );
         assertFalse( SelectorUtils.isToolchain( toolchainPrivateManager, Collections.singleton( openJdk9 ) ) );
 
         when( toolchainPrivateManager.getToolchainPrivates( "jdk" ) )
-                .thenReturn( new ToolchainPrivate[] { jdkMatching } );
+            .thenReturn( new ToolchainPrivate[] {jdkMatching} );
         when( toolchainPrivateManager.getToolchainPrivates( "maven" ) )
-                .thenReturn( new ToolchainPrivate[0] );
+            .thenReturn( new ToolchainPrivate[0] );
         assertFalse( SelectorUtils.isToolchain( toolchainPrivateManager, Arrays.asList( openJdk9, maven360 ) ) );
     }
 
+    @Test
+    public void mavenVersionForNotExistingMavenHomeThrowException()
+    {
+        File mavenHome = new File( "not-existing-path" );
+
+        assertThatCode( () -> SelectorUtils.getMavenVersion( mavenHome ) )
+            .isExactlyInstanceOf( IllegalArgumentException.class )
+            .hasMessage( "Invalid Maven home installation directory: not-existing-path" );
+    }
+
+    @Test
+    public void mavenVersionFromMavenHome() throws IOException
+    {
+        File mavenHome = new File( System.getProperty( "maven.home" ) );
+
+        String mavenVersion = SelectorUtils.getMavenVersion( mavenHome );
+
+        assertThat( mavenVersion ).isNotBlank();
+    }
 }