Copilot commented on code in PR #1677:
URL:
https://github.com/apache/maven-dependency-plugin/pull/1677#discussion_r3745147241
##########
src/test/java/org/apache/maven/plugins/dependency/TestGetMojo.java:
##########
@@ -119,13 +164,10 @@ void testTransitive(GetMojo mojo) throws Exception {
@InjectMojo(goal = "get")
@MojoParameter(
name = "remoteRepositories",
- value =
-
"central::default::https://repo.maven.apache.org/maven2,central::::https://repo.maven.apache.org/maven2,https://repo.maven.apache.org/maven2";)
+ value = "central::default::https://repo.maven.apache.org/maven2,";
+ + "central::::https://repo.maven.apache.org/maven2,";
+ + "https://repo.maven.apache.org/maven2";)
Review Comment:
The test uses `@MojoParameter(... value = \"...,...,...\")` (a single
string) while the mojo field is now `List<String>`. If the test harness binds
that annotation value as a single list element (rather than splitting on
commas), the test may not reflect real-world behavior and could mask a
production regression. To make the behavior unambiguous, either (a) change the
mojo parameter back to `String` as documented, or (b) update the test to set a
true multi-element list (e.g., via `setRemoteRepositories(...)`/reflection) and
add coverage for comma-separated input handling if you intend to keep
supporting it.
##########
src/main/java/org/apache/maven/plugins/dependency/GetMojo.java:
##########
@@ -20,80 +20,40 @@
import javax.inject.Inject;
-import java.util.ArrayList;
import java.util.List;
-import java.util.Map;
-import java.util.regex.Matcher;
-import java.util.regex.Pattern;
-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.Mojo;
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.apache.maven.plugins.dependency.utils.ParamArtifact;
+import org.apache.maven.plugins.dependency.utils.ResolverUtil;
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.repository.RemoteRepository;
+import org.eclipse.aether.resolution.ArtifactDescriptorException;
+import org.eclipse.aether.resolution.ArtifactResolutionException;
+import org.eclipse.aether.resolution.DependencyResolutionException;
/**
* Resolves a single artifact, eventually transitively, from the specified
remote repositories. Caveat: will always
* check the central repository defined in the super pom. You could use a
mirror entry in your <code>settings.xml</code>
*/
@Mojo(name = "get", requiresProject = false, threadSafe = true)
public class GetMojo extends AbstractMojo {
- private static final Pattern ALT_REPO_SYNTAX_PATTERN =
Pattern.compile("(.+)::(.*)::(.+)");
- private final MavenSession session;
+ private final ResolverUtil resolverUtil;
- private final ArtifactResolver artifactResolver;
-
- private final DependencyResolver dependencyResolver;
-
- private final ArtifactHandlerManager artifactHandlerManager;
+ private final ParamArtifact paramArtifact = new ParamArtifact();
/**
- * Map that contains the layouts.
- */
- private final Map<String, ArtifactRepositoryLayout> repositoryLayouts;
-
- /**
- * The repository system.
- */
- private final RepositorySystem repositorySystem;
-
- private final DefaultDependableCoordinate coordinate = new
DefaultDependableCoordinate();
-
- /**
- * Repositories in the format id::[layout]::url or just url, separated by
comma. i.e.
- *
central::default::https://repo.maven.apache.org/maven2,myrepo::::https://repo.acme.com,https://repo.acme2.com.
+ * Repositories in the format {@code id::[layout::]url} or just URLs,
separated by comma. That is,
Review Comment:
Changing `remoteRepositories` from a single comma-separated `String` to
`List<String>` is a breaking behavior/API change for plugin users. Maven
parameter binding typically treats a POM `<remoteRepositories>` list
differently from a `-DremoteRepositories=...` system property, and your Javadoc
still documents comma-separated syntax; this risks `-DremoteRepositories`
becoming a single list element containing commas. Consider reverting the
parameter type back to `String` (and parse/split/trim internally), or
explicitly support both by flattening/splitting any comma-separated entries
inside the provided `List<String>` and updating the parameter documentation
accordingly.
##########
src/main/java/org/apache/maven/plugins/dependency/GetMojo.java:
##########
@@ -20,80 +20,40 @@
import javax.inject.Inject;
-import java.util.ArrayList;
import java.util.List;
-import java.util.Map;
-import java.util.regex.Matcher;
-import java.util.regex.Pattern;
-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.Mojo;
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.apache.maven.plugins.dependency.utils.ParamArtifact;
+import org.apache.maven.plugins.dependency.utils.ResolverUtil;
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.repository.RemoteRepository;
+import org.eclipse.aether.resolution.ArtifactDescriptorException;
+import org.eclipse.aether.resolution.ArtifactResolutionException;
+import org.eclipse.aether.resolution.DependencyResolutionException;
/**
* Resolves a single artifact, eventually transitively, from the specified
remote repositories. Caveat: will always
* check the central repository defined in the super pom. You could use a
mirror entry in your <code>settings.xml</code>
*/
@Mojo(name = "get", requiresProject = false, threadSafe = true)
public class GetMojo extends AbstractMojo {
- private static final Pattern ALT_REPO_SYNTAX_PATTERN =
Pattern.compile("(.+)::(.*)::(.+)");
- private final MavenSession session;
+ private final ResolverUtil resolverUtil;
- private final ArtifactResolver artifactResolver;
-
- private final DependencyResolver dependencyResolver;
-
- private final ArtifactHandlerManager artifactHandlerManager;
+ private final ParamArtifact paramArtifact = new ParamArtifact();
/**
- * Map that contains the layouts.
- */
- private final Map<String, ArtifactRepositoryLayout> repositoryLayouts;
-
- /**
- * The repository system.
- */
- private final RepositorySystem repositorySystem;
-
- private final DefaultDependableCoordinate coordinate = new
DefaultDependableCoordinate();
-
- /**
- * Repositories in the format id::[layout]::url or just url, separated by
comma. i.e.
- *
central::default::https://repo.maven.apache.org/maven2,myrepo::::https://repo.acme.com,https://repo.acme2.com.
+ * Repositories in the format {@code id::[layout::]url} or just URLs,
separated by comma. That is,
+ * <code>
+ *
central::default::https://repo.maven.apache.org/maven2,myrepo::https://repo.acme.com,https://repo.acme2.com
+ * </code>
*/
@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;
+ private List<String> remoteRepositories;
Review Comment:
Changing `remoteRepositories` from a single comma-separated `String` to
`List<String>` is a breaking behavior/API change for plugin users. Maven
parameter binding typically treats a POM `<remoteRepositories>` list
differently from a `-DremoteRepositories=...` system property, and your Javadoc
still documents comma-separated syntax; this risks `-DremoteRepositories`
becoming a single list element containing commas. Consider reverting the
parameter type back to `String` (and parse/split/trim internally), or
explicitly support both by flattening/splitting any comma-separated entries
inside the provided `List<String>` and updating the parameter documentation
accordingly.
##########
src/test/java/org/apache/maven/plugins/dependency/TestGetMojo.java:
##########
@@ -218,20 +285,8 @@ void testRemoteRepositoriesNonProxyHosts(GetMojo mojo)
throws Exception {
}
}
- /**
- * Points the mojo at an empty local repository, so that the tests above
depend on the transfer actually
- * happening rather than on what an earlier run left behind in the shared
one.
- */
- private void useIsolatedLocalRepository() {
- DefaultRepositorySystemSession repositorySession =
- new
DefaultRepositorySystemSession(session.getRepositorySession());
-
repositorySession.setLocalRepositoryManager(repositorySystem.newLocalRepositoryManager(
- repositorySession, new
LocalRepository(isolatedLocalRepository.toFile())));
- when(session.getRepositorySession()).thenReturn(repositorySession);
-
- DefaultProjectBuildingRequest pbr = new
DefaultProjectBuildingRequest();
- pbr.setRepositorySession(repositorySession);
- when(session.getProjectBuildingRequest()).thenReturn(pbr);
+ private void setRemoteRepositories(GetMojo mojo, String... repositories)
throws Exception {
+ setVariableValueToObject(mojo, "remoteRepositories",
Arrays.asList(repositories));
}
Review Comment:
The test uses `@MojoParameter(... value = \"...,...,...\")` (a single
string) while the mojo field is now `List<String>`. If the test harness binds
that annotation value as a single list element (rather than splitting on
commas), the test may not reflect real-world behavior and could mask a
production regression. To make the behavior unambiguous, either (a) change the
mojo parameter back to `String` as documented, or (b) update the test to set a
true multi-element list (e.g., via `setRemoteRepositories(...)`/reflection) and
add coverage for comma-separated input handling if you intend to keep
supporting it.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
