[SUREFIRE-1679] Prevent classpath caching from causing pollution

Previously, classpath caching was performed statically. This resulted
in the classpath cached by one project for a particular provider
being used by a subsequent project. As a result any customizations to
the classpath, such as removing duplicate artifacts, would leak out
and pollute the classpath used by subsequent projects.

This commit prevents the pollution by making the classpath cache
instance-scoped so that the cache is only used by a single mojo and,
therefore, a single project.

maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/AbstractSurefireMojo.java

@@ -159,6 +159,8 @@ public abstract class AbstractSurefireMojo
 
     private final ProviderDetector providerDetector = new ProviderDetector();
 
+    private final ClasspathCache classpathCache = new ClasspathCache();
+
     /**
      * Note: use the legacy system property <em>disableXmlReport</em> set to {@code true} to disable the report.
      */
@@ -1787,10 +1789,10 @@ private StartupConfiguration newStartupConfigWithClasspath(
     {
         Classpath testClasspath = testClasspathWrapper.toClasspath();
 
-        Classpath providerClasspath = ClasspathCache.getCachedClassPath( providerName );
+        Classpath providerClasspath = classpathCache.getCachedClassPath( providerName );
         if ( providerClasspath == null )
         {
-            providerClasspath = ClasspathCache.setCachedClasspath( providerName, providerArtifacts );
+            providerClasspath = classpathCache.setCachedClasspath( providerName, providerArtifacts );
         }
 
         getConsoleLogger().debug( testClasspath.getLogMessage( "test classpath:" ) );
@@ -1868,10 +1870,10 @@ private StartupConfiguration newStartupConfigWithModularPath(
     {
         Classpath testClasspath = testClasspathWrapper.toClasspath();
 
-        Classpath providerClasspath = ClasspathCache.getCachedClassPath( providerName );
+        Classpath providerClasspath = classpathCache.getCachedClassPath( providerName );
         if ( providerClasspath == null )
         {
-            providerClasspath = ClasspathCache.setCachedClasspath( providerName, providerArtifacts );
+            providerClasspath = classpathCache.setCachedClasspath( providerName, providerArtifacts );
         }
 
         ResolvePathsRequest<String> req = ResolvePathsRequest.ofStrings( testClasspath.getClassPath() )
@@ -2629,7 +2631,7 @@ private void showMap( Map<?, ?> map, String setting )
 
     private Classpath getArtifactClasspath( Artifact surefireArtifact )
     {
-        Classpath existing = ClasspathCache.getCachedClassPath( surefireArtifact.getArtifactId() );
+        Classpath existing = classpathCache.getCachedClassPath( surefireArtifact.getArtifactId() );
         if ( existing == null )
         {
             List<String> items = new ArrayList<>();
@@ -2643,7 +2645,7 @@ private Classpath getArtifactClasspath( Artifact surefireArtifact )
                 items.add( artifact.getFile().getAbsolutePath() );
             }
             existing = new Classpath( items );
-            ClasspathCache.setCachedClasspath( surefireArtifact.getArtifactId(), existing );
+            classpathCache.setCachedClasspath( surefireArtifact.getArtifactId(), existing );
         }
         return existing;
     }
@@ -3839,4 +3841,31 @@ else if ( forkMode.equals( FORK_NEVER ) || forkMode.equals( FORK_ONCE )
             throw new IllegalArgumentException( "Fork mode " + forkMode + " is not a legal value" );
         }
     }
+
+    private static final class ClasspathCache
+    {
+        private final Map<String, Classpath> classpaths = new HashMap<>( 4 );
+
+        private Classpath getCachedClassPath( @Nonnull String artifactId )
+        {
+            return classpaths.get( artifactId );
+        }
+
+        private void setCachedClasspath( @Nonnull String key, @Nonnull Classpath classpath )
+        {
+            classpaths.put( key, classpath );
+        }
+
+        private Classpath setCachedClasspath( @Nonnull String key, @Nonnull Set<Artifact> artifacts )
+        {
+            Collection<String> files = new ArrayList<>();
+            for ( Artifact artifact : artifacts )
+            {
+                files.add( artifact.getFile().getAbsolutePath() );
+            }
+            Classpath classpath = new Classpath( files );
+            setCachedClasspath( key, classpath );
+            return classpath;
+        }
+    }
 }

maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/ClasspathCache.java

@@ -32,6 +32,7 @@
 /**
  * @author Kristian Rosenvold
  */
+@Deprecated
 public class ClasspathCache
 {
     private static final ConcurrentHashMap<String, Classpath> CLASSPATHS =

maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/AbstractSurefireMojoTest.java

@@ -335,6 +335,67 @@ public void shouldHaveStartupConfigForNonModularClasspath()
                 .isEqualTo( "org.asf.Provider" );
     }
 
+    @Test
+    public void providerClasspathCachingIsNotSharedAcrossMojoInstances() throws Exception {
+        ProviderInfo providerInfo = mock( ProviderInfo.class );
+        when( providerInfo.getProviderName() ).thenReturn( "test-provider" );
+        Artifact provider = new DefaultArtifact( "com.example", "provider", createFromVersion( "1" ), "runtime", "jar", "", handler );
+        provider.setFile( mockFile( "original-test-provider.jar" ) );
+        HashSet<Artifact> providerClasspath = new HashSet<Artifact>( asList( provider ) );
+        when( providerInfo.getProviderClasspath()).thenReturn( providerClasspath );
+
+        StartupConfiguration startupConfiguration = startupConfigurationForProvider(providerInfo);
+        assertThat( startupConfiguration.getClasspathConfiguration().getProviderClasspath().getClassPath() )
+                .containsExactly( "original-test-provider.jar" );
+
+        provider.setFile( mockFile( "modified-test-provider.jar" ) );
+        startupConfiguration = startupConfigurationForProvider(providerInfo);
+        assertThat( startupConfiguration.getClasspathConfiguration().getProviderClasspath().getClassPath() )
+                .containsExactly( "modified-test-provider.jar" );
+    }
+
+    private StartupConfiguration startupConfigurationForProvider(ProviderInfo providerInfo) throws Exception {
+        AbstractSurefireMojo mojo = spy( new Mojo() );
+
+        Logger logger = mock( Logger.class );
+        when( logger.isDebugEnabled() ).thenReturn( true );
+        doNothing().when( logger ).debug( anyString() );
+        when( mojo.getConsoleLogger() ).thenReturn( new PluginConsoleLogger( logger ) );
+
+        File classesDir = mockFile( "classes" );
+        File testClassesDir = mockFile( "test-classes" );
+        TestClassPath testClassPath = new TestClassPath( new ArrayList<Artifact>(), classesDir, testClassesDir, new String[0] );
+
+        Artifact common = new DefaultArtifact( "org.apache.maven.surefire", "maven-surefire-common",
+                createFromVersion( "1" ), "runtime", "jar", "", handler );
+        common.setFile( mockFile( "maven-surefire-common.jar" ) );
+
+        Artifact ext = new DefaultArtifact( "org.apache.maven.surefire", "surefire-extensions-api",
+                createFromVersion( "1" ), "runtime", "jar", "", handler );
+        ext.setFile( mockFile( "surefire-extensions-api.jar" ) );
+
+        Artifact api = new DefaultArtifact( "org.apache.maven.surefire", "surefire-api",
+                createFromVersion( "1" ), "runtime", "jar", "", handler );
+        api.setFile( mockFile( "surefire-api.jar" ) );
+
+        Artifact loggerApi = new DefaultArtifact( "org.apache.maven.surefire", "surefire-logger-api",
+                createFromVersion( "1" ), "runtime", "jar", "", handler );
+        loggerApi.setFile( mockFile( "surefire-logger-api.jar" ) );
+
+        Map<String, Artifact> providerArtifactsMap = new HashMap<>();
+        providerArtifactsMap.put( "org.apache.maven.surefire:maven-surefire-common", common );
+        providerArtifactsMap.put( "org.apache.maven.surefire:surefire-extensions-api", ext );
+        providerArtifactsMap.put( "org.apache.maven.surefire:surefire-api", api );
+        providerArtifactsMap.put( "org.apache.maven.surefire:surefire-logger-api", loggerApi );
+
+        when( mojo.getPluginArtifactMap() ).thenReturn( providerArtifactsMap );
+
+        doReturn( 1 ).when( mojo, "getEffectiveForkCount" );
+
+        StartupConfiguration startupConfiguration = invokeMethod( mojo, "createStartupConfiguration", providerInfo, false, null, null, null, testClassPath);
+        return startupConfiguration;
+    }
+
     @Test
     public void shouldExistTmpDirectory() throws IOException
     {