You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2020/05/19 18:00:04 UTC

[GitHub] [maven-dependency-plugin] bimargulies-google commented on a change in pull request #53: Moved GetMojo logic to an abstract class. This allows new mojos to im…

bimargulies-google commented on a change in pull request #53:
URL: https://github.com/apache/maven-dependency-plugin/pull/53#discussion_r427494413



##########
File path: src/main/java/org/apache/maven/plugins/dependency/AbstractGetMojo.java
##########
@@ -0,0 +1,355 @@
+package org.apache.maven.plugins.dependency;
+
+/*
+ * 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.
+ */
+
+import org.apache.maven.artifact.handler.ArtifactHandler;
+import org.apache.maven.artifact.handler.manager.ArtifactHandlerManager;
+import org.apache.maven.artifact.repository.ArtifactRepository;
+import org.apache.maven.artifact.repository.ArtifactRepositoryPolicy;
+import org.apache.maven.artifact.repository.MavenArtifactRepository;
+import org.apache.maven.artifact.repository.layout.ArtifactRepositoryLayout;
+import org.apache.maven.execution.MavenSession;
+import org.apache.maven.plugin.AbstractMojo;
+import org.apache.maven.plugin.MojoExecutionException;
+import org.apache.maven.plugin.MojoFailureException;
+import org.apache.maven.plugins.annotations.Component;
+import org.apache.maven.plugins.annotations.Parameter;
+import org.apache.maven.project.DefaultProjectBuildingRequest;
+import org.apache.maven.project.ProjectBuildingRequest;
+import org.apache.maven.repository.RepositorySystem;
+import org.apache.maven.settings.Settings;
+import org.apache.maven.shared.transfer.artifact.ArtifactCoordinate;
+import org.apache.maven.shared.transfer.artifact.DefaultArtifactCoordinate;
+import org.apache.maven.shared.transfer.artifact.resolve.ArtifactResolver;
+import org.apache.maven.shared.transfer.artifact.resolve.ArtifactResolverException;
+import org.apache.maven.shared.transfer.dependencies.DefaultDependableCoordinate;
+import org.apache.maven.shared.transfer.dependencies.DependableCoordinate;
+import org.apache.maven.shared.transfer.dependencies.resolve.DependencyResolver;
+import org.apache.maven.shared.transfer.dependencies.resolve.DependencyResolverException;
+import org.codehaus.plexus.util.StringUtils;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+public abstract class AbstractGetMojo
+        extends AbstractMojo
+{
+    private static final Pattern ALT_REPO_SYNTAX_PATTERN = Pattern.compile( "(.+)::(.*)::(.+)" );
+
+    @Parameter( defaultValue = "${session}", required = true, readonly = true )
+    private MavenSession session;
+
+    /**
+     *
+     */
+    @Component
+    private ArtifactResolver artifactResolver;
+
+    /**
+     *
+     */
+    @Component
+    private DependencyResolver dependencyResolver;
+
+    @Component
+    private ArtifactHandlerManager artifactHandlerManager;
+
+    /**
+     * Map that contains the layouts.
+     */
+    @Component( role = ArtifactRepositoryLayout.class )
+    private Map<String, ArtifactRepositoryLayout> repositoryLayouts;
+
+    /**
+     * The repository system.
+     */
+    @Component
+    private RepositorySystem repositorySystem;
+
+    private DefaultDependableCoordinate coordinate = new DefaultDependableCoordinate();
+
+    /**
+     * The groupId of the artifact to download. Ignored if {@link #artifact} is used.
+     */
+    @Parameter( property = "groupId" )
+    private String groupId;
+
+    /**
+     * The artifactId of the artifact to download. Ignored if {@link #artifact} is used.
+     */
+    @Parameter( property = "artifactId" )
+    private String artifactId;
+
+    /**
+     * The version of the artifact to download. Ignored if {@link #artifact} is used.
+     */
+    @Parameter( property = "version" )
+    private String version;
+
+    /**
+     * The classifier of the artifact to download. Ignored if {@link #artifact} is used.
+     *
+     * @since 2.3
+     */
+    @Parameter( property = "classifier" )
+    private String classifier;
+
+    /**
+     * The packaging of the artifact to download. Ignored if {@link #artifact} is used.
+     */
+    @Parameter( property = "packaging", defaultValue = "jar" )
+    private String packaging = "jar";
+
+    /**
+     * Repositories in the format id::[layout]::url or just url, separated by comma. ie.
+     * central::default::https://repo.maven.apache.org/maven2,myrepo::::https://repo.acme.com,https://repo.acme2.com
+     */
+    @Parameter( property = "remoteRepositories" )
+    private String remoteRepositories;
+
+    /**
+     * A string of the form groupId:artifactId:version[:packaging[:classifier]].
+     */
+    @Parameter( property = "artifact" )
+    private String artifact;
+
+    /**
+     *
+     */
+    @Parameter( defaultValue = "${project.remoteArtifactRepositories}", readonly = true, required = true )
+    private List<ArtifactRepository> pomRemoteRepositories;
+
+    /**
+     * Download transitively, retrieving the specified artifact and all of its dependencies.
+     */
+    @Parameter( property = "transitive", defaultValue = "true" )
+    private boolean transitive = true;
+
+    /**
+     * Skip plugin execution completely.
+     *
+     * @since 2.7
+     */
+    @Parameter( property = "mdep.skip", defaultValue = "false" )
+    private boolean skip;
+
+    /**
+     * @throws MojoExecutionException {@link MojoExecutionException}

Review comment:
       Needs text stating that each derived class will provide this.

##########
File path: src/main/java/org/apache/maven/plugins/dependency/AbstractGetMojo.java
##########
@@ -0,0 +1,355 @@
+package org.apache.maven.plugins.dependency;
+
+/*
+ * 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.
+ */
+
+import org.apache.maven.artifact.handler.ArtifactHandler;
+import org.apache.maven.artifact.handler.manager.ArtifactHandlerManager;
+import org.apache.maven.artifact.repository.ArtifactRepository;
+import org.apache.maven.artifact.repository.ArtifactRepositoryPolicy;
+import org.apache.maven.artifact.repository.MavenArtifactRepository;
+import org.apache.maven.artifact.repository.layout.ArtifactRepositoryLayout;
+import org.apache.maven.execution.MavenSession;
+import org.apache.maven.plugin.AbstractMojo;
+import org.apache.maven.plugin.MojoExecutionException;
+import org.apache.maven.plugin.MojoFailureException;
+import org.apache.maven.plugins.annotations.Component;
+import org.apache.maven.plugins.annotations.Parameter;
+import org.apache.maven.project.DefaultProjectBuildingRequest;
+import org.apache.maven.project.ProjectBuildingRequest;
+import org.apache.maven.repository.RepositorySystem;
+import org.apache.maven.settings.Settings;
+import org.apache.maven.shared.transfer.artifact.ArtifactCoordinate;
+import org.apache.maven.shared.transfer.artifact.DefaultArtifactCoordinate;
+import org.apache.maven.shared.transfer.artifact.resolve.ArtifactResolver;
+import org.apache.maven.shared.transfer.artifact.resolve.ArtifactResolverException;
+import org.apache.maven.shared.transfer.dependencies.DefaultDependableCoordinate;
+import org.apache.maven.shared.transfer.dependencies.DependableCoordinate;
+import org.apache.maven.shared.transfer.dependencies.resolve.DependencyResolver;
+import org.apache.maven.shared.transfer.dependencies.resolve.DependencyResolverException;
+import org.codehaus.plexus.util.StringUtils;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+

Review comment:
       Add javadoc comment here that explains what this class is good for.

##########
File path: src/main/java/org/apache/maven/plugins/dependency/AbstractGetMojo.java
##########
@@ -0,0 +1,355 @@
+package org.apache.maven.plugins.dependency;
+
+/*
+ * 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.
+ */
+
+import org.apache.maven.artifact.handler.ArtifactHandler;
+import org.apache.maven.artifact.handler.manager.ArtifactHandlerManager;
+import org.apache.maven.artifact.repository.ArtifactRepository;
+import org.apache.maven.artifact.repository.ArtifactRepositoryPolicy;
+import org.apache.maven.artifact.repository.MavenArtifactRepository;
+import org.apache.maven.artifact.repository.layout.ArtifactRepositoryLayout;
+import org.apache.maven.execution.MavenSession;
+import org.apache.maven.plugin.AbstractMojo;
+import org.apache.maven.plugin.MojoExecutionException;
+import org.apache.maven.plugin.MojoFailureException;
+import org.apache.maven.plugins.annotations.Component;
+import org.apache.maven.plugins.annotations.Parameter;
+import org.apache.maven.project.DefaultProjectBuildingRequest;
+import org.apache.maven.project.ProjectBuildingRequest;
+import org.apache.maven.repository.RepositorySystem;
+import org.apache.maven.settings.Settings;
+import org.apache.maven.shared.transfer.artifact.ArtifactCoordinate;
+import org.apache.maven.shared.transfer.artifact.DefaultArtifactCoordinate;
+import org.apache.maven.shared.transfer.artifact.resolve.ArtifactResolver;
+import org.apache.maven.shared.transfer.artifact.resolve.ArtifactResolverException;
+import org.apache.maven.shared.transfer.dependencies.DefaultDependableCoordinate;
+import org.apache.maven.shared.transfer.dependencies.DependableCoordinate;
+import org.apache.maven.shared.transfer.dependencies.resolve.DependencyResolver;
+import org.apache.maven.shared.transfer.dependencies.resolve.DependencyResolverException;
+import org.codehaus.plexus.util.StringUtils;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+public abstract class AbstractGetMojo
+        extends AbstractMojo
+{
+    private static final Pattern ALT_REPO_SYNTAX_PATTERN = Pattern.compile( "(.+)::(.*)::(.+)" );
+
+    @Parameter( defaultValue = "${session}", required = true, readonly = true )
+    private MavenSession session;
+
+    /**
+     *
+     */
+    @Component
+    private ArtifactResolver artifactResolver;
+
+    /**
+     *
+     */
+    @Component
+    private DependencyResolver dependencyResolver;
+
+    @Component
+    private ArtifactHandlerManager artifactHandlerManager;
+
+    /**
+     * Map that contains the layouts.
+     */
+    @Component( role = ArtifactRepositoryLayout.class )
+    private Map<String, ArtifactRepositoryLayout> repositoryLayouts;
+
+    /**
+     * The repository system.
+     */
+    @Component
+    private RepositorySystem repositorySystem;
+
+    private DefaultDependableCoordinate coordinate = new DefaultDependableCoordinate();
+
+    /**
+     * The groupId of the artifact to download. Ignored if {@link #artifact} is used.
+     */
+    @Parameter( property = "groupId" )
+    private String groupId;
+
+    /**
+     * The artifactId of the artifact to download. Ignored if {@link #artifact} is used.
+     */
+    @Parameter( property = "artifactId" )
+    private String artifactId;
+
+    /**
+     * The version of the artifact to download. Ignored if {@link #artifact} is used.
+     */
+    @Parameter( property = "version" )
+    private String version;
+
+    /**
+     * The classifier of the artifact to download. Ignored if {@link #artifact} is used.
+     *
+     * @since 2.3
+     */
+    @Parameter( property = "classifier" )
+    private String classifier;
+
+    /**
+     * The packaging of the artifact to download. Ignored if {@link #artifact} is used.
+     */
+    @Parameter( property = "packaging", defaultValue = "jar" )
+    private String packaging = "jar";
+
+    /**
+     * Repositories in the format id::[layout]::url or just url, separated by comma. ie.
+     * central::default::https://repo.maven.apache.org/maven2,myrepo::::https://repo.acme.com,https://repo.acme2.com
+     */
+    @Parameter( property = "remoteRepositories" )
+    private String remoteRepositories;
+
+    /**
+     * A string of the form groupId:artifactId:version[:packaging[:classifier]].
+     */
+    @Parameter( property = "artifact" )
+    private String artifact;
+
+    /**
+     *
+     */
+    @Parameter( defaultValue = "${project.remoteArtifactRepositories}", readonly = true, required = true )
+    private List<ArtifactRepository> pomRemoteRepositories;
+
+    /**
+     * Download transitively, retrieving the specified artifact and all of its dependencies.
+     */
+    @Parameter( property = "transitive", defaultValue = "true" )
+    private boolean transitive = true;
+
+    /**
+     * Skip plugin execution completely.
+     *
+     * @since 2.7
+     */
+    @Parameter( property = "mdep.skip", defaultValue = "false" )
+    private boolean skip;
+
+    /**
+     * @throws MojoExecutionException {@link MojoExecutionException}
+     * @throws MojoFailureException {@link MojoFailureException}
+     */
+    protected abstract void doExecute()
+            throws MojoExecutionException, MojoFailureException;
+
+    @Override
+    public void execute()
+            throws MojoExecutionException, MojoFailureException
+    {
+        doExecute();
+    }
+
+    public ProjectBuildingRequest buildBuildingRequest()

Review comment:
       Need javadoc comment on here, should explain that the expectation is that derived classes will call this.

##########
File path: src/main/java/org/apache/maven/plugins/dependency/GetMojo.java
##########
@@ -58,292 +58,38 @@
  */
 @Mojo( name = "get", requiresProject = false, threadSafe = true )
 public class GetMojo
-    extends AbstractMojo
+        extends AbstractGetMojo
 {
-    private static final Pattern ALT_REPO_SYNTAX_PATTERN = Pattern.compile( "(.+)::(.*)::(.+)" );
 
-    @Parameter( defaultValue = "${session}", required = true, readonly = true )
-    private MavenSession session;
-
-    /**
-     *
-     */
-    @Component
-    private ArtifactResolver artifactResolver;
-
-    /**
-     *
-     */
-    @Component
-    private DependencyResolver dependencyResolver;
-
-    @Component
-    private ArtifactHandlerManager artifactHandlerManager;
-
-    /**
-     * Map that contains the layouts.
-     */
-    @Component( role = ArtifactRepositoryLayout.class )
-    private Map<String, ArtifactRepositoryLayout> repositoryLayouts;
-
-    /**
-     * The repository system.
-     */
-    @Component
-    private RepositorySystem repositorySystem;
-
-    private DefaultDependableCoordinate coordinate = new DefaultDependableCoordinate();
-
-    /**
-     * The groupId of the artifact to download. Ignored if {@link #artifact} is used.
-     */
-    @Parameter( property = "groupId" )
-    private String groupId;
-
-    /**
-     * The artifactId of the artifact to download. Ignored if {@link #artifact} is used.
-     */
-    @Parameter( property = "artifactId" )
-    private String artifactId;
-
-    /**
-     * The version of the artifact to download. Ignored if {@link #artifact} is used.
-     */
-    @Parameter( property = "version" )
-    private String version;
-
-    /**
-     * The classifier of the artifact to download. Ignored if {@link #artifact} is used.
-     *
-     * @since 2.3
-     */
-    @Parameter( property = "classifier" )
-    private String classifier;
-
-    /**
-     * The packaging of the artifact to download. Ignored if {@link #artifact} is used.
-     */
-    @Parameter( property = "packaging", defaultValue = "jar" )
-    private String packaging = "jar";
-
-    /**
-     * Repositories in the format id::[layout]::url or just url, separated by comma. ie.
-     * central::default::https://repo.maven.apache.org/maven2,myrepo::::https://repo.acme.com,https://repo.acme2.com
-     */
-    @Parameter( property = "remoteRepositories" )
-    private String remoteRepositories;
-
-    /**
-     * A string of the form groupId:artifactId:version[:packaging[:classifier]].
-     */
-    @Parameter( property = "artifact" )
-    private String artifact;
-
-    /**
-     *
-     */
-    @Parameter( defaultValue = "${project.remoteArtifactRepositories}", readonly = true, required = true )
-    private List<ArtifactRepository> pomRemoteRepositories;
-
-    /**
-     * Download transitively, retrieving the specified artifact and all of its dependencies.
-     */
-    @Parameter( property = "transitive", defaultValue = "true" )
-    private boolean transitive = true;
-
-    /**
-     * Skip plugin execution completely.
-     *
-     * @since 2.7
-     */
-    @Parameter( property = "mdep.skip", defaultValue = "false" )
-    private boolean skip;
 
     @Override
-    public void execute()
-        throws MojoExecutionException, MojoFailureException
+    protected void doExecute()
+            throws MojoExecutionException, MojoFailureException
     {
+
         if ( isSkip() )

Review comment:
       Skip logic can be in base class.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org