[MDEP-689] Fixes ignored dependency filtering in go-offline goal (#417)

pom.xml

@@ -59,6 +59,9 @@ under the License.
     <contributor>
       <name>Maarten Mulders</name>
     </contributor>
+    <contributor>
+      <name>Lisa Hardy</name>
+    </contributor>
   </contributors>
 
   <prerequisites>

src/it/projects/mdep-689-apply-filtering-go-offline-goal/invoker.properties

@@ -0,0 +1,18 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+# 
+#   http://www.apache.org/licenses/LICENSE-2.0
+# 
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+invoker.goals = clean ${project.groupId}:${project.artifactId}:${project.version}:go-offline 

src/it/projects/mdep-689-apply-filtering-go-offline-goal/pom.xml

@@ -0,0 +1,109 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<!--
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements.  See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership.  The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License.  You may obtain a copy of the License at
+
+  http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied.  See the License for the
+specific language governing permissions and limitations
+under the License.
+-->
+
+<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>org.apache.maven.plugins.dependency</groupId>
+	<artifactId>mdep-739-go-offline-respect-classifiers</artifactId>
+	<version>1.0.0-SNAPSHOT</version>
+	<description>Test that dependency:go-offline applies exclude filters</description>
+	<dependencies>
+		<dependency> <!-- excluded by excludeGroupId -->
+			<groupId>skip.this.groupid</groupId>
+			<artifactId>dummy-artifact</artifactId>
+			<version>1.0</version>
+		</dependency>
+		<dependency> <!-- excluded by excludeGroupId -->
+			<groupId>skip.this.groupid.too</groupId>
+			<artifactId>dummy-artifact</artifactId>
+			<version>1.0</version>
+		</dependency>
+		<dependency> <!-- excluded by excludeArtifactId -->
+			<groupId>org.junit.jupiter</groupId>
+			<artifactId>skip-this-artifact</artifactId>
+			<version>1.0</version>
+		</dependency>
+		<dependency> <!-- excluded by excludeArtifactId -->
+			<groupId>org.junit.jupiter</groupId>
+			<artifactId>skip-this-artifact-too</artifactId>
+			<version>1.0-SNAPSHOT</version>
+		</dependency>
+		<dependency> <!-- excluded by excludeClassifier -->
+			<groupId>ch.qos.logback</groupId>
+			<artifactId>logback-access</artifactId>
+			<version>1.4.7</version>
+			<classifier>skipThisClassifier</classifier>
+		</dependency>
+		<dependency> <!-- excluded by excludeClassifier -->
+			<groupId>ch.qos.logback</groupId>
+			<artifactId>logback-core</artifactId>
+			<version>1.5.6</version>
+			<classifier>skipThisClassifierToo</classifier>
+		</dependency>
+		<dependency><!-- should not be excluded -->
+			<groupId>org.slf4j</groupId>
+			<artifactId>slf4j-api</artifactId>
+			<version>2.0.13</version>
+			<scope>compile</scope>
+		</dependency>
+		<dependency> <!-- excluded by excludeScope -->
+			<groupId>ch.qos.logback</groupId>
+			<artifactId>logback-classic</artifactId>
+			<version>1.5.4</version>
+			<scope>system</scope>
+			<!-- it doesn't matter what jar this points to, needs a systemPath to compile -->
+			<systemPath>${project.build.directory}/local-repo/junit/junit/4.13.2/junit-4.13.2.jar</systemPath>
+		</dependency>
+		<dependency><!-- should not be excluded -->
+			<groupId>org.slf4j</groupId>
+			<artifactId>slf4j-simple</artifactId>
+			<version>2.0.13</version>
+		</dependency>
+		<dependency> <!-- excluded by excludeType -->
+			<groupId>ch.qos.logback</groupId>
+			<artifactId>logback-examples</artifactId>
+			<version>1.4.7</version>
+			<type>ear</type>
+		</dependency>
+	</dependencies>
+	<build>
+    <pluginManagement>
+      <plugins>
+        <plugin>
+          <artifactId>maven-dependency-plugin</artifactId>
+          <version>@project.version@</version>
+          <configuration>
+            <excludeGroupIds>skip.this.groupid,skip.this.groupid.too</excludeGroupIds>
+            <excludeArtifactIds>skip-this-artifact,skip-this-artifact-too</excludeArtifactIds>
+            <!-- the maven-common-artifact-filters library does not support comma delimited
+            scope exclusions.  only one at a time.  the other filters support comma delimited lists-->
+            <!-- <excludeScope>compile,system</excludeScope> -->
+            <excludeScope>system</excludeScope>
+            <excludeTypes>ear</excludeTypes>
+            <excludeClassifiers>skipThisClassifier,skipThisClassifierToo</excludeClassifiers>
+          </configuration>
+        </plugin>
+      </plugins>
+    </pluginManagement>
+  </build>
+</project>

src/it/projects/mdep-689-apply-filtering-go-offline-goal/verify.groovy

@@ -0,0 +1,50 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * 
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+File file = new File( basedir, "build.log" )
+assert file.exists()
+
+String buildLog = file.getText( "UTF-8" )
+
+// Make sure non-exluded explicit dependencies are resolved
+assert buildLog.contains( 'Resolved dependency: slf4j-simple-2.0.13.jar' )
+assert buildLog.contains( 'Resolved dependency: slf4j-api-2.0.13.jar' )
+
+// Did group excludes work?
+assert !buildLog.contains( 'Resolving artifact skip.this.groupid' )
+
+// Did artifact excludes work?
+assert !buildLog.contains( 'Resolving artifact org.junit.jupiter:skip-this-artifact' )
+
+// Did scope exludes work?
+assert !buildLog.contains( 'Resolving artifact ch.qos.logback:logback-classic' )
+
+// Did type excludes work?
+assert !buildLog.contains( 'Resolving artifact ch.qos.logback:logback-examples' )
+
+// Did classifier excludes work?
+assert !buildLog.contains( 'Resolving artifact ch.qos.logback:logback-core:jar:skipThisClassifierToo' )
+assert !buildLog.contains( 'Resolving artifact ch.qos.logback:logback-core' )
+
+assert !buildLog.contains( 'Resolving artifact ch.qos.logback:logback-access:jar:skipThisClassifier' )
+assert !buildLog.contains( 'Resolving artifact ch.qos.logback:logback-access' )
+
+
+
+return true

src/main/java/org/apache/maven/plugins/dependency/fromDependencies/AbstractDependencyFilterMojo.java

@@ -136,7 +136,9 @@ public abstract class AbstractDependencyFilterMojo extends AbstractDependencyMoj
 
     /**
      * Scope threshold to exclude, if no value is defined for include.
-     * An empty string indicates no dependencies (default).<br>
+     * An empty string indicates no dependencies (default).  Unlike the other
+     * exclusion parameters, this property does not support a comma-delimited
+     * list of scope exclusions. Just one scope may be excluded at a time.<br>
      * The scope threshold value being interpreted is the scope as
      * Maven filters for creating a classpath, not as specified in the pom. In summary:
      * <ul>

src/main/java/org/apache/maven/plugins/dependency/resolvers/AbstractResolveMojo.java

@@ -22,6 +22,13 @@
 
 import org.apache.maven.plugins.annotations.Parameter;
 import org.apache.maven.plugins.dependency.fromDependencies.AbstractDependencyFilterMojo;
+import org.apache.maven.plugins.dependency.utils.DependencyUtil;
+import org.apache.maven.shared.artifact.filter.collection.ArtifactIdFilter;
+import org.apache.maven.shared.artifact.filter.collection.ClassifierFilter;
+import org.apache.maven.shared.artifact.filter.collection.FilterArtifacts;
+import org.apache.maven.shared.artifact.filter.collection.GroupIdFilter;
+import org.apache.maven.shared.artifact.filter.collection.ScopeFilter;
+import org.apache.maven.shared.artifact.filter.collection.TypeFilter;
 
 /**
  * @author <a href="mailto:[email protected]">Brian Fox</a>
@@ -52,4 +59,38 @@ public abstract class AbstractResolveMojo extends AbstractDependencyFilterMojo {
      */
     @Parameter(property = "excludeReactor", defaultValue = "true")
     protected boolean excludeReactor;
+
+    /**
+     * @return {@link FilterArtifacts}
+     */
+    protected FilterArtifacts getArtifactsFilter() {
+        final FilterArtifacts filter = new FilterArtifacts();
+
+        if (excludeReactor) {
+
+            filter.addFilter(new ExcludeReactorProjectsArtifactFilter(reactorProjects, getLog()));
+        }
+
+        filter.addFilter(new ScopeFilter(
+                DependencyUtil.cleanToBeTokenizedString(this.includeScope),
+                DependencyUtil.cleanToBeTokenizedString(this.excludeScope)));
+
+        filter.addFilter(new TypeFilter(
+                DependencyUtil.cleanToBeTokenizedString(this.includeTypes),
+                DependencyUtil.cleanToBeTokenizedString(this.excludeTypes)));
+
+        filter.addFilter(new ClassifierFilter(
+                DependencyUtil.cleanToBeTokenizedString(this.includeClassifiers),
+                DependencyUtil.cleanToBeTokenizedString(this.excludeClassifiers)));
+
+        filter.addFilter(new GroupIdFilter(
+                DependencyUtil.cleanToBeTokenizedString(this.includeGroupIds),
+                DependencyUtil.cleanToBeTokenizedString(this.excludeGroupIds)));
+
+        filter.addFilter(new ArtifactIdFilter(
+                DependencyUtil.cleanToBeTokenizedString(this.includeArtifactIds),
+                DependencyUtil.cleanToBeTokenizedString(this.excludeArtifactIds)));
+
+        return filter;
+    }
 }

src/main/java/org/apache/maven/plugins/dependency/resolvers/GoOfflineMojo.java

@@ -25,13 +25,17 @@
 import java.util.stream.Collectors;
 
 import org.apache.maven.artifact.Artifact;
+import org.apache.maven.artifact.DefaultArtifact;
+import org.apache.maven.artifact.handler.DefaultArtifactHandler;
 import org.apache.maven.artifact.repository.ArtifactRepository;
 import org.apache.maven.model.Dependency;
 import org.apache.maven.plugin.MojoExecutionException;
 import org.apache.maven.plugins.annotations.Mojo;
 import org.apache.maven.plugins.dependency.utils.DependencyUtil;
 import org.apache.maven.project.ProjectBuildingRequest;
+import org.apache.maven.shared.artifact.filter.collection.ArtifactFilterException;
 import org.apache.maven.shared.artifact.filter.collection.ArtifactsFilter;
+import org.apache.maven.shared.artifact.filter.collection.FilterArtifacts;
 import org.apache.maven.shared.artifact.filter.resolve.TransformableFilter;
 import org.apache.maven.shared.transfer.artifact.resolve.ArtifactResult;
 import org.apache.maven.shared.transfer.dependencies.DefaultDependableCoordinate;
@@ -43,6 +47,7 @@
  *
  * @author <a href="mailto:[email protected]">Brian Fox</a>
  * @author Maarten Mulders
+ * @author Lisa Hardy
  * @since 2.0
  */
 @Mojo(name = "go-offline", threadSafe = true)
@@ -71,7 +76,7 @@ protected void doExecute() throws MojoExecutionException {
                 }
             }
 
-        } catch (DependencyResolverException e) {
+        } catch (DependencyResolverException | ArtifactFilterException e) {
             throw new MojoExecutionException(e.getMessage(), e);
         }
     }
@@ -81,10 +86,13 @@ protected void doExecute() throws MojoExecutionException {
      *
      * @return set of resolved dependency artifacts.
      * @throws DependencyResolverException in case of an error while resolving the artifacts.
+     * @throws ArtifactFilterException
      */
-    protected Set<Artifact> resolveDependencyArtifacts() throws DependencyResolverException {
+    protected Set<Artifact> resolveDependencyArtifacts() throws DependencyResolverException, ArtifactFilterException {
         Collection<Dependency> dependencies = getProject().getDependencies();
 
+        dependencies = filterDependencies(dependencies);
+
         Set<DependableCoordinate> dependableCoordinates = dependencies.stream()
                 .map(this::createDependendableCoordinateFromDependency)
                 .collect(Collectors.toSet());
@@ -133,8 +141,9 @@ private TransformableFilter getTransformableFilter() {
      *
      * @return set of resolved plugin artifacts.
      * @throws DependencyResolverException in case of an error while resolving the artifacts.
+     * @throws ArtifactFilterException
      */
-    protected Set<Artifact> resolvePluginArtifacts() throws DependencyResolverException {
+    protected Set<Artifact> resolvePluginArtifacts() throws DependencyResolverException, ArtifactFilterException {
 
         Set<Artifact> plugins = getProject().getPluginArtifacts();
         Set<Artifact> reports = getProject().getReportArtifacts();
@@ -143,6 +152,9 @@ protected Set<Artifact> resolvePluginArtifacts() throws DependencyResolverExcept
         artifacts.addAll(reports);
         artifacts.addAll(plugins);
 
+        final FilterArtifacts filter = getArtifactsFilter();
+        artifacts = filter.filter(artifacts);
+
         Set<DependableCoordinate> dependableCoordinates = artifacts.stream()
                 .map(this::createDependendableCoordinateFromArtifact)
                 .collect(Collectors.toSet());
@@ -152,6 +164,16 @@ protected Set<Artifact> resolvePluginArtifacts() throws DependencyResolverExcept
         return resolveDependableCoordinate(buildingRequest, dependableCoordinates, "plugins");
     }
 
+    private Collection<Dependency> filterDependencies(Collection<Dependency> deps) throws ArtifactFilterException {
+
+        Set<Artifact> artifacts = createArtifactSetFromDependencies(deps);
+
+        final FilterArtifacts filter = getArtifactsFilter();
+        artifacts = filter.filter(artifacts);
+
+        return createDependencySetFromArtifacts(artifacts);
+    }
+
     private DependableCoordinate createDependendableCoordinateFromArtifact(final Artifact artifact) {
         final DefaultDependableCoordinate result = new DefaultDependableCoordinate();
         result.setGroupId(artifact.getGroupId());
@@ -174,6 +196,39 @@ private DependableCoordinate createDependendableCoordinateFromDependency(final D
         return result;
     }
 
+    private Set<Artifact> createArtifactSetFromDependencies(Collection<Dependency> deps) {
+        Set<Artifact> artifacts = new HashSet<>();
+        for (Dependency dep : deps) {
+            DefaultArtifactHandler handler = new DefaultArtifactHandler(dep.getType());
+            artifacts.add(new DefaultArtifact(
+                    dep.getGroupId(),
+                    dep.getArtifactId(),
+                    dep.getVersion(),
+                    dep.getScope(),
+                    dep.getType(),
+                    dep.getClassifier(),
+                    handler));
+        }
+        return artifacts;
+    }
+
+    private Collection<Dependency> createDependencySetFromArtifacts(Set<Artifact> artifacts) {
+        Set<Dependency> dependencies = new HashSet<>();
+
+        for (Artifact artifact : artifacts) {
+            Dependency d = new Dependency();
+            d.setGroupId(artifact.getGroupId());
+            d.setArtifactId(artifact.getArtifactId());
+            d.setVersion(artifact.getVersion());
+            d.setType(artifact.getType());
+            d.setClassifier(artifact.getClassifier());
+            d.setScope(artifact.getScope());
+            dependencies.add(d);
+        }
+
+        return dependencies;
+    }
+
     @Override
     protected ArtifactsFilter getMarkedArtifactFilter() {
         return null;