fix missing optional dependencies in .wtpmodules, cleanup test and introduce a test using MavenEmbedder (currently disabled due to a couple of issues, but it should replace the command line test)

git-svn-id: https://svn.apache.org/repos/asf/maven/components/trunk@330946 13f79535-47bb-0310-9956-ffa450edef68

maven-plugins/maven-eclipse-plugin/pom.xml

@@ -26,6 +26,12 @@
       <artifactId>maven-artifact-manager</artifactId>
       <version>2.0</version>
       <scope>test</scope>
+    </dependency>    
+    <dependency>
+      <groupId>org.apache.maven</groupId>
+      <artifactId>maven-embedder</artifactId>
+      <version>2.0</version>
+      <scope>test</scope>
     </dependency>
     <dependency>
       <groupId>org.codehaus.plexus</groupId>

maven-plugins/maven-eclipse-plugin/src/main/java/org/apache/maven/plugin/eclipse/EclipseClasspathWriter.java

@@ -22,7 +22,6 @@ import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Iterator;
 import java.util.List;
-import java.util.Set;
 
 import org.apache.maven.artifact.Artifact;
 import org.apache.maven.artifact.factory.ArtifactFactory;
@@ -128,31 +127,8 @@ public class EclipseClasspathWriter
 
         List artifacts = project.getTestArtifacts();
 
-        // @todo direct optional artifacts are not included in the list returned by project.getTestArtifacts()
+        EclipseUtils.fixMissingOptionalArtifacts( artifacts, project.getDependencyArtifacts(), localRepository,
-        // .classpath should include ANY direct dependency, and optional dependencies are required to compile
+                                                  artifactResolver, remoteArtifactRepositories, log );
-        Set depArtifacts = project.getDependencyArtifacts();
-        for ( Iterator it = depArtifacts.iterator(); it.hasNext(); )
-        {
-            Artifact artifact = (Artifact) it.next();
-            if ( artifact.isOptional() && !artifacts.contains( artifact ) )
-            {
-                try
-                {
-                    artifactResolver.resolve( artifact, remoteArtifactRepositories, localRepository );
-                }
-                catch ( ArtifactResolutionException e )
-                {
-                    log.error( "Unable to resolve optional artifact " + artifact.getId() );
-                    continue;
-                }
-                catch ( ArtifactNotFoundException e )
-                {
-                    log.error( "Unable to resolve optional artifact " + artifact.getId() );
-                    continue;
-                }
-                artifacts.add( artifact );
-            }
-        }
 
         EclipseUtils.fixSystemScopeArtifacts( artifacts, project.getDependencies() );
 

maven-plugins/maven-eclipse-plugin/src/main/java/org/apache/maven/plugin/eclipse/EclipsePlugin.java

@@ -353,7 +353,8 @@ public class EclipsePlugin
         new EclipseSettingsWriter( getLog() ).write( projectBaseDir, outputDir, project );
 
         new EclipseWtpmodulesWriter( getLog() ).write( outputDir, project, reactorArtifacts, sourceDirs,
-                                                       localRepository );
+                                                       localRepository,artifactResolver,
+                                                       remoteArtifactRepositories );
 
         getLog().info( Messages.getString( "EclipsePlugin.wrote", //$NON-NLS-1$
                                            new Object[]{project.getArtifactId(), outputDir.getAbsolutePath()} ) );

maven-plugins/maven-eclipse-plugin/src/main/java/org/apache/maven/plugin/eclipse/EclipseUtils.java

@@ -25,6 +25,10 @@ import java.util.Set;
 import java.util.TreeSet;
 
 import org.apache.maven.artifact.Artifact;
+import org.apache.maven.artifact.repository.ArtifactRepository;
+import org.apache.maven.artifact.resolver.ArtifactNotFoundException;
+import org.apache.maven.artifact.resolver.ArtifactResolutionException;
+import org.apache.maven.artifact.resolver.ArtifactResolver;
 import org.apache.maven.model.Dependency;
 import org.apache.maven.model.Plugin;
 import org.apache.maven.model.Resource;
@@ -262,6 +266,10 @@ public class EclipseUtils
         return referencedProjects;
     }
 
+    /**
+     * @todo MNG-1379 Wrong path for artifacts with system scope
+     * Artifacts with a system scope have a wrong path in mvn 2.0. This is a temporary workaround.
+     */
     public static void fixSystemScopeArtifacts( Collection artifacts, Collection dependencies )
     {
         // fix path for system dependencies.Artifact.getFile() returns a wrong path in mvn 2.0
@@ -287,4 +295,35 @@ public class EclipseUtils
         }
     }
 
+    /**
+     * @todo MNG-1384 optional dependencies not resolved while compiling from a master project 
+     * Direct optional artifacts are not included in the list returned by project.getTestArtifacts()
+     * .classpath should include ANY direct dependency, and optional dependencies are required to compile
+     */
+    public static void fixMissingOptionalArtifacts(                                                   Collection artifacts, Collection depArtifacts, ArtifactRepository localRepository,
+                                                   ArtifactResolver artifactResolver, List remoteArtifactRepositories,Log log )
+    {
+        for ( Iterator it = depArtifacts.iterator(); it.hasNext(); )
+        {
+            Artifact artifact = (Artifact) it.next();
+            if ( artifact.isOptional() && !artifacts.contains( artifact ) )
+            {
+                try
+                {
+                    artifactResolver.resolve( artifact, remoteArtifactRepositories, localRepository );
+                }
+                catch ( ArtifactResolutionException e )
+                {
+                    log.error( "Unable to resolve optional artifact " + artifact.getId() );
+                    continue;
+                }
+                catch ( ArtifactNotFoundException e )
+                {
+                    log.error( "Unable to resolve optional artifact " + artifact.getId() );
+                    continue;
+                }
+                artifacts.add( artifact );
+            }
+        }
+    }
 }

maven-plugins/maven-eclipse-plugin/src/main/java/org/apache/maven/plugin/eclipse/EclipseWtpmodulesWriter.java

@@ -25,6 +25,7 @@ import java.util.Set;
 
 import org.apache.maven.artifact.Artifact;
 import org.apache.maven.artifact.repository.ArtifactRepository;
+import org.apache.maven.artifact.resolver.ArtifactResolver;
 import org.apache.maven.artifact.resolver.filter.ScopeArtifactFilter;
 import org.apache.maven.plugin.MojoExecutionException;
 import org.apache.maven.plugin.logging.Log;
@@ -51,7 +52,8 @@ public class EclipseWtpmodulesWriter
     }
 
     protected void write( File basedir, MavenProject project, List referencedReactorArtifacts,
-                         EclipseSourceDir[] sourceDirs, ArtifactRepository localRepository )
+                         EclipseSourceDir[] sourceDirs, ArtifactRepository localRepository,
+                         ArtifactResolver artifactResolver, List remoteArtifactRepositories )
         throws MojoExecutionException
     {
         FileWriter w;
@@ -151,7 +153,8 @@ public class EclipseWtpmodulesWriter
         String target = "/"; //$NON-NLS-1$
         if ( "war".equals( project.getPackaging() ) ) //$NON-NLS-1$
         {
-            writeWarSpecificResources( writer, basedir, project, referencedReactorArtifacts, localRepository );
+            writeWarSpecificResources( writer, basedir, project, referencedReactorArtifacts, localRepository,
+                                       artifactResolver, remoteArtifactRepositories );
 
             target = "/WEB-INF/classes"; //$NON-NLS-1$
         }
@@ -177,7 +180,8 @@ public class EclipseWtpmodulesWriter
     }
 
     private void writeWarSpecificResources( XMLWriter writer, File basedir, MavenProject project,
-                                           List referencedReactorArtifacts, ArtifactRepository localRepository )
+                                           List referencedReactorArtifacts, ArtifactRepository localRepository,
+                                           ArtifactResolver artifactResolver, List remoteArtifactRepositories )
     {
 
         String warSourceDirectory = EclipseUtils.getPluginSetting( project, "maven-war-plugin", //$NON-NLS-1$
@@ -191,6 +195,10 @@ public class EclipseWtpmodulesWriter
         writer.endElement();
 
         Set artifacts = project.getArtifacts();
+
+        EclipseUtils.fixMissingOptionalArtifacts( artifacts, project.getDependencyArtifacts(), localRepository,
+                                                  artifactResolver, remoteArtifactRepositories, log );
+
         EclipseUtils.fixSystemScopeArtifacts( artifacts, project.getDependencies() );
 
         ScopeArtifactFilter scopeFilter = new ScopeArtifactFilter( Artifact.SCOPE_RUNTIME );

maven-plugins/maven-eclipse-plugin/src/test/java/org/apache/maven/plugin/eclipse/AbstractEclipsePluginTestCase.java

@@ -203,6 +203,7 @@ public abstract class AbstractEclipsePluginTestCase
             buf.append( "Unexpected \"" );
             buf.append( substring );
             buf.append( "\" found" );
+            fail( buf.toString() );
         }
     }
 

maven-plugins/maven-eclipse-plugin/src/test/java/org/apache/maven/plugin/eclipse/EclipsePluginMasterProjectTest.java

@@ -19,9 +19,20 @@ package org.apache.maven.plugin.eclipse;
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.FileWriter;
+import java.io.IOException;
 import java.io.InputStream;
 import java.io.Writer;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Properties;
 
+import org.apache.maven.cli.ConsoleDownloadMonitor;
+import org.apache.maven.embedder.MavenEmbedder;
+import org.apache.maven.embedder.MavenEmbedderConsoleLogger;
+import org.apache.maven.embedder.PlexusLoggerAdapter;
+import org.apache.maven.monitor.event.DefaultEventMonitor;
+import org.apache.maven.monitor.event.EventMonitor;
 import org.apache.maven.settings.MavenSettingsBuilder;
 import org.apache.maven.settings.Settings;
 import org.codehaus.plexus.util.IOUtil;
@@ -67,26 +78,76 @@ import org.codehaus.plexus.util.xml.XMLWriter;
 public class EclipsePluginMasterProjectTest
     extends AbstractEclipsePluginTestCase
 {
+
+    protected File basedir;
+
+    protected MavenEmbedder maven;
+
+    protected List projectList = new ArrayList();
+
+    protected void setUp()
+        throws Exception
+    {
+        this.basedir = getTestFile( "src/test/projects/master-test" );
+
+        this.maven = new MavenEmbedder();
+        this.maven.setClassLoader( Thread.currentThread().getContextClassLoader() );
+        this.maven.setLogger( new MavenEmbedderConsoleLogger() );
+        this.maven.start();
+
+        projectList.add( maven.readProjectWithDependencies( new File( basedir, "pom.xml" ) ) );
+
+        super.setUp();
+    }
+
+    protected void tearDown()
+        throws Exception
+    {
+        maven.stop();
+        super.tearDown();
+    }
+
+    /**
+     * Currently disabled because:
+     * <ul>
+     *   <li>the reactor build is not run by the embedder</li>
+     *   <li>the embedder doesn't support custom settings</li>
+     * </ul>
+     * @throws Exception
+     */
+    public void disabledTestMasterProjectWithEmbedder()
+        throws Exception
+    {
+        EventMonitor eventMonitor = new DefaultEventMonitor( new PlexusLoggerAdapter( new MavenEmbedderConsoleLogger() ) );
+
+        this.maven.execute( projectList, Arrays.asList( new String[] {
+            "org.apache.maven.plugins:maven-eclipse-plugin:clean",
+            "org.apache.maven.plugins:maven-eclipse-plugin:eclipse" } ), eventMonitor, new ConsoleDownloadMonitor(),
+                            new Properties(), this.basedir );
+
+        compareFiles();
+    }
+
+    /**
+     * Test using a command line. Should be replaced by the embedder test.
+     */
     public void testMasterProject()
         throws Exception
     {
-        File basedir = getTestFile( "src/test/projects/master-test" );
-
         executeMaven2CommandLine( basedir );
+        compareFiles();
+    }
 
+    private void compareFiles()
+        throws Exception
+    {
         assertFileEquals( null, new File( basedir, "module-1/project" ), new File( basedir, "module-1/.project" ) );
         assertFileEquals( null, new File( basedir, "module-1/classpath" ), new File( basedir, "module-1/.classpath" ) );
         assertFileEquals( null, new File( basedir, "module-1/wtpmodules" ), new File( basedir, "module-1/.wtpmodules" ) );
-
-        // the real test: this should include any sort of direct/transitive dependency handled by mvn
         assertFileEquals( null, new File( basedir, "module-2/project" ), new File( basedir, "module-2/.project" ) );
 
-        // manual check, easier to handle
         checkModule2Classpath( new File( basedir, "module-2/.classpath" ) );
-
-        // manual check, easier to handle
         checkModule2Wtpmodules( new File( basedir, "module-2/.wtpmodules" ) );
-
     }
 
     private void checkModule2Classpath( File file )
@@ -101,58 +162,63 @@ public class EclipsePluginMasterProjectTest
         assertContains( "Invalid classpath", classpath, "/direct-test" );
         assertContains( "Invalid classpath", classpath, "/direct-sysdep" );
         assertContains( "Invalid classpath", classpath, "/direct-optional" );
+        assertContains( "Invalid classpath", classpath, "/direct-provided" );
 
-        // referenced project: no deps!
+        // referenced project: not required, but it's not a problem to have them included
         assertContains( "Invalid classpath", classpath, "/module-1" );
-        assertDoesNotContain( "Invalid classpath", classpath, "/refproject-compile" );
+        // assertDoesNotContain( "Invalid classpath", classpath, "/refproject-compile" );
+        // assertDoesNotContain( "Invalid classpath", classpath, "/refproject-sysdep" );
         assertDoesNotContain( "Invalid classpath", classpath, "/refproject-test" );
-        assertDoesNotContain( "Invalid classpath", classpath, "/refproject-sysdep" );
         assertDoesNotContain( "Invalid classpath", classpath, "/refproject-optional" );
+        assertDoesNotContain( "Invalid classpath", classpath, "/refproject-provided" );
 
         // transitive dependencies from referenced projects
         assertContains( "Invalid classpath", classpath, "/deps-direct-compile" );
         assertDoesNotContain( "Invalid classpath", classpath, "/deps-direct-test" );
-        assertDoesNotContain( "Invalid classpath", classpath, "/deps-direct-system" );
         assertDoesNotContain( "Invalid classpath", classpath, "/deps-direct-optional" );
+        // @todo should this be included? see MNG-514
+        assertDoesNotContain( "Invalid classpath", classpath, "/deps-direct-provided" );
 
         // transitive dependencies from referenced projects
-        assertDoesNotContain( "Invalid classpath", classpath, "/deps-refproject-compile" );
+        assertContains( "Invalid classpath", classpath, "/deps-refproject-compile" );
         assertDoesNotContain( "Invalid classpath", classpath, "/deps-refproject-test" );
-        assertDoesNotContain( "Invalid classpath", classpath, "/deps-refproject-system" );
         assertDoesNotContain( "Invalid classpath", classpath, "/deps-refproject-optional" );
+        assertDoesNotContain( "Invalid classpath", classpath, "/deps-refproject-provided" );
     }
 
     private void checkModule2Wtpmodules( File file )
         throws Exception
     {
         InputStream fis = new FileInputStream( file );
-        String classpath = IOUtil.toString( fis );
+        String wtpmodules = IOUtil.toString( fis );
         IOUtil.close( fis );
 
-        // direct dependencies: include all
+        // direct dependencies: include only runtime (also optional) dependencies
-        assertContains( "Invalid wtpmodules", classpath, "/direct-compile" );
+        assertContains( "Invalid wtpmodules", wtpmodules, "/direct-compile" );
-        assertDoesNotContain( "Invalid wtpmodules", classpath, "/direct-test" );
+        assertDoesNotContain( "Invalid wtpmodules", wtpmodules, "/direct-test" );
-        assertContains( "Invalid wtpmodules", classpath, "/direct-system" );
+        assertContains( "Invalid wtpmodules", wtpmodules, "/direct-sysdep" );
-        assertContains( "Invalid wtpmodules", classpath, "/direct-optional" );
+        assertContains( "Invalid wtpmodules", wtpmodules, "/direct-optional" );
+        assertDoesNotContain( "Invalid wtpmodules", wtpmodules, "/direct-provided" );
 
         // referenced project: only runtime deps
-        assertContains( "Invalid wtpmodules", classpath, "/module-1" );
+        assertContains( "Invalid wtpmodules", wtpmodules, "/module-1" );
-        assertContains( "Invalid wtpmodules", classpath, "/refproject-compile" );
+        assertContains( "Invalid wtpmodules", wtpmodules, "/refproject-compile" );
-        assertDoesNotContain( "Invalid wtpmodules", classpath, "/refproject-test" );
+        assertContains( "Invalid wtpmodules", wtpmodules, "/refproject-sysdep" );
-        assertDoesNotContain( "Invalid wtpmodules", classpath, "/refproject-system" );
+        assertDoesNotContain( "Invalid wtpmodules", wtpmodules, "/refproject-test" );
-        assertDoesNotContain( "Invalid wtpmodules", classpath, "/refproject-optional" );
+        assertDoesNotContain( "Invalid wtpmodules", wtpmodules, "/refproject-optional" );
+        assertDoesNotContain( "Invalid wtpmodules", wtpmodules, "/refproject-provided" );
 
         // transitive dependencies from referenced projects
-        assertContains( "Invalid wtpmodules", classpath, "/deps-direct-compile" );
+        assertContains( "Invalid wtpmodules", wtpmodules, "/deps-direct-compile" );
-        assertDoesNotContain( "Invalid wtpmodules", classpath, "/deps-direct-test" );
+        assertDoesNotContain( "Invalid wtpmodules", wtpmodules, "/deps-direct-test" );
-        assertDoesNotContain( "Invalid wtpmodules", classpath, "/deps-direct-system" );
+        assertDoesNotContain( "Invalid wtpmodules", wtpmodules, "/deps-direct-optional" );
-        assertDoesNotContain( "Invalid wtpmodules", classpath, "/deps-direct-optional" );
+        assertDoesNotContain( "Invalid wtpmodules", wtpmodules, "/deps-direct-provided" );
 
         // transitive dependencies from referenced projects
-        assertDoesNotContain( "Invalid wtpmodules", classpath, "/deps-refproject-compile" );
+        assertContains( "Invalid wtpmodules", wtpmodules, "/deps-refproject-compile" );
-        assertDoesNotContain( "Invalid wtpmodules", classpath, "/deps-refproject-test" );
+        assertDoesNotContain( "Invalid wtpmodules", wtpmodules, "/deps-refproject-test" );
-        assertDoesNotContain( "Invalid wtpmodules", classpath, "/deps-refproject-system" );
+        assertDoesNotContain( "Invalid wtpmodules", wtpmodules, "/deps-refproject-optional" );
-        assertDoesNotContain( "Invalid wtpmodules", classpath, "/deps-refproject-optional" );
+        assertDoesNotContain( "Invalid wtpmodules", wtpmodules, "/deps-refproject-provided" );
     }
 
     /**
@@ -191,6 +257,25 @@ public class EclipsePluginMasterProjectTest
         MavenSettingsBuilder settingsBuilder = (MavenSettingsBuilder) lookup( MavenSettingsBuilder.ROLE );
         Settings defaultSettings = settingsBuilder.buildSettings();
 
+        String settingsPath = createTestSettings( defaultSettings );
+
+        Commandline cmd = new Commandline();
+
+        cmd.setWorkingDirectory( workingDir.getAbsolutePath() );
+
+        cmd.setExecutable( "mvn" );
+        cmd.createArgument().setValue( "-s" + settingsPath );
+        cmd.createArgument().setValue( "-e" );
+
+        cmd.createArgument().setValue( "eclipse:clean" );
+        cmd.createArgument().setValue( "eclipse:eclipse" );
+
+        return cmd;
+    }
+
+    private String createTestSettings( Settings defaultSettings )
+        throws IOException
+    {
         // prepare a temporary settings.xml
         File settings = File.createTempFile( "settings", ".xml" );
         settings.deleteOnExit();
@@ -226,19 +311,9 @@ public class EclipsePluginMasterProjectTest
 
         writer.endElement();
         IOUtil.close( w );
+        settings.deleteOnExit();
 
-        Commandline cmd = new Commandline();
+        return settings.getAbsolutePath();
-
-        cmd.setWorkingDirectory( workingDir.getAbsolutePath() );
-
-        cmd.setExecutable( "mvn" );
-        cmd.createArgument().setValue( "-s" + settings.getAbsolutePath() );
-        cmd.createArgument().setValue( "-e" );
-
-        cmd.createArgument().setValue( "eclipse:clean" );
-        cmd.createArgument().setValue( "eclipse:eclipse" );
-
-        return cmd;
     }
 
 }