[MNG-8197] Use default classifier when Eclipse Aether specifies none (#1621)

* Use default classifier when Eclipse Aether specifies none. The important change in this commit is in the implementation of `getClassifier()` methods. However, this commit opportunistically refactors the code with a base class shared by `DefaultDependency` and `DefaultDependencyCoordinate` implementations. * Preserve the type declared in the dependency (e.g. "modular-jar") when wrapping the object from Maven model.
2024-08-11 00:20:04 +02:00 · 2024-08-11 00:20:04 +02:00 · 473b5374f0
parent 0f6d555073
commit 473b5374f0
6 changed files with 150 additions and 112 deletions
--- a/api/maven-api-core/src/main/java/org/apache/maven/api/services/DependencyResolverRequest.java
+++ b/api/maven-api-core/src/main/java/org/apache/maven/api/services/DependencyResolverRequest.java
@ -323,7 +323,7 @@ public interface DependencyResolverRequest {
         * @return {@code this} for method call chaining
         */
        @Nonnull
-        public DependencyResolverRequestBuilder pathTypeFilter(@Nonnull Collection<PathType> desiredTypes) {
+        public DependencyResolverRequestBuilder pathTypeFilter(@Nonnull Collection<? extends PathType> desiredTypes) {
            return pathTypeFilter(desiredTypes::contains);
        }

--- a/maven-api-impl/src/main/java/org/apache/maven/internal/impl/AbstractSession.java
+++ b/maven-api-impl/src/main/java/org/apache/maven/internal/impl/AbstractSession.java
@ -88,6 +88,7 @@ import org.apache.maven.api.services.VersionResolver;
 import org.eclipse.aether.DefaultRepositorySystemSession;
 import org.eclipse.aether.RepositorySystem;
 import org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.artifact.ArtifactType;

 import static org.apache.maven.internal.impl.Utils.map;
 import static org.apache.maven.internal.impl.Utils.nonNull;
@ -281,28 +282,33 @@ public abstract class AbstractSession implements InternalSession {
    }

    @Nonnull
+    @Override
    public RepositorySystemSession getSession() {
        return session;
    }

    @Nonnull
+    @Override
    public RepositorySystem getRepositorySystem() {
        return repositorySystem;
    }

+    @Override
    public org.eclipse.aether.graph.Dependency toDependency(DependencyCoordinate dependency, boolean managed) {
        org.eclipse.aether.graph.Dependency dep;
-        if (dependency instanceof DefaultDependencyCoordinate) {
-            dep = ((DefaultDependencyCoordinate) dependency).getDependency();
+        if (dependency instanceof AetherDependencyWrapper wrapper) {
+            dep = wrapper.dependency;
        } else {
+            Type type = dependency.getType();
            dep = new org.eclipse.aether.graph.Dependency(
                    new org.eclipse.aether.artifact.DefaultArtifact(
                            dependency.getGroupId(),
                            dependency.getArtifactId(),
                            dependency.getClassifier(),
-                            dependency.getType().getExtension(),
+                            type.getExtension(),
                            dependency.getVersion().toString(),
-                            null),
+                            Map.of("type", type.id()),
+                            (ArtifactType) null),
                    dependency.getScope().id(),
                    dependency.getOptional(),
                    map(dependency.getExclusions(), this::toExclusion));
--- a/maven-api-impl/src/main/java/org/apache/maven/internal/impl/AetherDependencyWrapper.java
+++ b/maven-api-impl/src/main/java/org/apache/maven/internal/impl/AetherDependencyWrapper.java
@ -0,0 +1,127 @@
+/*
+ * 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.
+ */
+package org.apache.maven.internal.impl;
+
+import java.util.Objects;
+
+import org.apache.maven.api.DependencyScope;
+import org.apache.maven.api.Type;
+import org.apache.maven.api.annotations.Nonnull;
+import org.eclipse.aether.artifact.ArtifactProperties;
+import org.eclipse.aether.graph.Dependency;
+
+/**
+ * Base class of {@code Dependency} or {@code DependencyCoordinate} implementations as a wrapper around
+ * an Eclipse Aether object. This class implements the methods that are common to {@code Dependency} and
+ * {@code DependencyCoordinate}, even if this class does not implement directly any of those interfaces.
+ * Having matching method signatures is sufficient, even if there is no {@code @Override} annotations.
+ *
+ * <p>The fact that this class is wrapping an Eclipse Aether object is an implementation details that may
+ * change in any future Maven version. For now, one purpose of this class is to have a single type to check
+ * for unwrapping the Eclipse Aether object.</p>
+ */
+abstract class AetherDependencyWrapper {
+    /**
+     * The session to install / deploy / resolve artifacts and dependencies.
+     */
+    final InternalSession session;
+
+    /**
+     * The wrapped Eclipse Aether dependency.
+     */
+    final Dependency dependency;
+
+    /**
+     * Creates a new wrapper for the given dependency.
+     *
+     * @param dependency the Eclipse Aether dependency to wrap
+     */
+    AetherDependencyWrapper(@Nonnull InternalSession session, @Nonnull Dependency dependency) {
+        this.session = Objects.requireNonNull(session, "session");
+        this.dependency = Objects.requireNonNull(dependency, "dependency");
+    }
+
+    /**
+     * {@return the group identifier of the wrapped dependency}.
+     * The default implementation delegates to the Eclipse Aether artifact.
+     */
+    public String getGroupId() {
+        return dependency.getArtifact().getGroupId();
+    }
+
+    /**
+     * {@return the artifact identifier of the wrapped dependency}.
+     * The default implementation delegates to the Eclipse Aether artifact.
+     */
+    public String getArtifactId() {
+        return dependency.getArtifact().getArtifactId();
+    }
+
+    /**
+     * {@return the file extension of the wrapped dependency}.
+     * The default implementation delegates to the Eclipse Aether artifact.
+     */
+    public String getExtension() {
+        return dependency.getArtifact().getExtension();
+    }
+
+    /**
+     * {@return the type of the wrapped dependency}.
+     * The default implementation infers the type from the properties associated to the Eclipse Aether artifact.
+     */
+    public Type getType() {
+        String type = dependency.getArtifact().getProperty(ArtifactProperties.TYPE, getExtension());
+        return session.requireType(type);
+    }
+
+    /**
+     * {@return the classifier ("jar", "test-jar", …) of the wrapped dependency}.
+     * The default implementation first delegates to the Eclipse Aether artifact.
+     * If the latter does not provide a non-empty classifier,
+     * then the default value is determined by {@linkplain #getType() type}.
+     */
+    @Nonnull
+    public String getClassifier() {
+        String classifier = dependency.getArtifact().getClassifier();
+        if (classifier.isEmpty()) {
+            classifier = getType().getClassifier();
+            if (classifier == null) {
+                classifier = "";
+            }
+        }
+        return classifier;
+    }
+
+    /**
+     * {@return the scope (compile, test, …) of this dependency}.
+     */
+    @Nonnull
+    public DependencyScope getScope() {
+        return session.requireDependencyScope(dependency.getScope());
+    }
+
+    /**
+     * {@return a string representation of this dependency}.
+     * This is for debugging purposes only and may change in any future version.
+     */
+    @Override
+    public String toString() {
+        return dependency.toString();
+    }
+}
--- a/maven-api-impl/src/main/java/org/apache/maven/internal/impl/DefaultDependency.java
+++ b/maven-api-impl/src/main/java/org/apache/maven/internal/impl/DefaultDependency.java
@ -23,25 +23,16 @@ import java.util.Objects;
 import org.apache.maven.api.Artifact;
 import org.apache.maven.api.Dependency;
 import org.apache.maven.api.DependencyCoordinate;
-import org.apache.maven.api.DependencyScope;
-import org.apache.maven.api.Type;
 import org.apache.maven.api.Version;
 import org.apache.maven.api.annotations.Nonnull;
-import org.apache.maven.api.annotations.Nullable;
-import org.eclipse.aether.artifact.ArtifactProperties;

-import static org.apache.maven.internal.impl.Utils.nonNull;
+public class DefaultDependency extends AetherDependencyWrapper implements Dependency {

-public class DefaultDependency implements Dependency {
-
-    private final InternalSession session;
-    private final org.eclipse.aether.graph.Dependency dependency;
    private final String key;

    public DefaultDependency(
            @Nonnull InternalSession session, @Nonnull org.eclipse.aether.graph.Dependency dependency) {
-        this.session = nonNull(session, "session");
-        this.dependency = nonNull(dependency, "dependency");
+        super(session, dependency);
        this.key = getGroupId()
                + ':'
                + getArtifactId()
@ -57,26 +48,6 @@ public class DefaultDependency implements Dependency {
        return key;
    }

-    @Nonnull
-    public org.eclipse.aether.graph.Dependency getDependency() {
-        return dependency;
-    }
-
-    @Override
-    public String getGroupId() {
-        return dependency.getArtifact().getGroupId();
-    }
-
-    @Override
-    public String getArtifactId() {
-        return dependency.getArtifact().getArtifactId();
-    }
-
-    @Override
-    public String getClassifier() {
-        return dependency.getArtifact().getClassifier();
-    }
-
    @Override
    public Version getVersion() {
        return session.parseVersion(dependency.getArtifact().getVersion());
@ -87,31 +58,11 @@ public class DefaultDependency implements Dependency {
        return session.parseVersion(dependency.getArtifact().getBaseVersion());
    }

-    @Override
-    public String getExtension() {
-        return dependency.getArtifact().getExtension();
-    }
-
-    @Override
-    public Type getType() {
-        String type = dependency
-                .getArtifact()
-                .getProperty(ArtifactProperties.TYPE, dependency.getArtifact().getExtension());
-        return session.requireType(type);
-    }
-
    @Override
    public boolean isSnapshot() {
        return DefaultModelVersionParser.checkSnapshot(dependency.getArtifact().getVersion());
    }

-    @Nonnull
-    @Override
-    public DependencyScope getScope() {
-        return session.requireDependencyScope(dependency.getScope());
-    }
-
-    @Nullable
    @Override
    public boolean isOptional() {
        return dependency.isOptional();
@ -132,9 +83,4 @@ public class DefaultDependency implements Dependency {
    public int hashCode() {
        return key.hashCode();
    }
-
-    @Override
-    public String toString() {
-        return dependency.toString();
-    }
 }
--- a/maven-api-impl/src/main/java/org/apache/maven/internal/impl/DefaultDependencyCoordinate.java
+++ b/maven-api-impl/src/main/java/org/apache/maven/internal/impl/DefaultDependencyCoordinate.java
@ -21,44 +21,15 @@ package org.apache.maven.internal.impl;
 import java.util.Collection;

 import org.apache.maven.api.DependencyCoordinate;
-import org.apache.maven.api.DependencyScope;
 import org.apache.maven.api.Exclusion;
-import org.apache.maven.api.Type;
 import org.apache.maven.api.VersionConstraint;
 import org.apache.maven.api.annotations.Nonnull;
 import org.apache.maven.api.annotations.Nullable;
-import org.eclipse.aether.artifact.ArtifactProperties;
-
-import static org.apache.maven.internal.impl.Utils.nonNull;
-
-public class DefaultDependencyCoordinate implements DependencyCoordinate {
-    private final InternalSession session;
-    private final org.eclipse.aether.graph.Dependency dependency;

+public class DefaultDependencyCoordinate extends AetherDependencyWrapper implements DependencyCoordinate {
    public DefaultDependencyCoordinate(
            @Nonnull InternalSession session, @Nonnull org.eclipse.aether.graph.Dependency dependency) {
-        this.session = nonNull(session, "session");
-        this.dependency = nonNull(dependency, "dependency");
-    }
-
-    @Nonnull
-    public org.eclipse.aether.graph.Dependency getDependency() {
-        return dependency;
-    }
-
-    @Override
-    public String getGroupId() {
-        return dependency.getArtifact().getGroupId();
-    }
-
-    @Override
-    public String getArtifactId() {
-        return dependency.getArtifact().getArtifactId();
-    }
-
-    @Override
-    public String getClassifier() {
-        return dependency.getArtifact().getClassifier();
+        super(session, dependency);
    }

    @Override
@ -66,25 +37,6 @@ public class DefaultDependencyCoordinate implements DependencyCoordinate {
        return session.parseVersionConstraint(dependency.getArtifact().getVersion());
    }

-    @Override
-    public String getExtension() {
-        return dependency.getArtifact().getExtension();
-    }
-
-    @Override
-    public Type getType() {
-        String type = dependency
-                .getArtifact()
-                .getProperty(ArtifactProperties.TYPE, dependency.getArtifact().getExtension());
-        return session.requireType(type);
-    }
-
-    @Nonnull
-    @Override
-    public DependencyScope getScope() {
-        return session.requireDependencyScope(dependency.getScope());
-    }
-
    @Nullable
    @Override
    public Boolean getOptional() {
--- a/maven-core/src/main/java/org/apache/maven/internal/impl/DefaultProject.java
+++ b/maven-core/src/main/java/org/apache/maven/internal/impl/DefaultProject.java
@ -177,7 +177,14 @@ public class DefaultProject implements Project {

            @Override
            public String getClassifier() {
-                return dependency.getClassifier();
+                String classifier = dependency.getClassifier();
+                if (classifier == null || classifier.isEmpty()) {
+                    classifier = getType().getClassifier();
+                    if (classifier == null) {
+                        classifier = "";
+                    }
+                }
+                return classifier;
            }

            @Override