Merge pull request #43 from frengor/gradle-transitive

Improve transitive dependencies resolution

core/src/main/java/com/alessiodp/libby/Library.java

@@ -5,12 +5,13 @@
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
-
 import java.util.Base64;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.LinkedList;
 
+import static com.alessiodp.libby.Util.craftPartialPath;
+import static com.alessiodp.libby.Util.craftPath;
 import static com.alessiodp.libby.Util.hexStringToByteArray;
 import static com.alessiodp.libby.Util.replaceWithDots;
 import static java.util.Objects.requireNonNull;
@@ -146,13 +147,8 @@ private Library(@Nullable Collection<String> urls,
         this.checksum = checksum;
         this.relocations = relocations != null ? Collections.unmodifiableList(new LinkedList<>(relocations)) : Collections.emptyList();
 
-        this.partialPath = this.groupId.replace('.', '/') + '/' + this.artifactId + '/' + version + '/';
-        String path = this.partialPath + this.artifactId + '-' + version;
-        if (hasClassifier()) {
-            path += '-' + classifier;
-        }
-
-        this.path = path + ".jar";
+        this.partialPath = craftPartialPath(this.artifactId, this.groupId, version);
+        this.path = craftPath(this.partialPath, this.artifactId, this.version, this.classifier);
 
         this.repositories = repositories != null ? Collections.unmodifiableList(new LinkedList<>(repositories)) : Collections.emptyList();
         relocatedPath = hasRelocations() ? path + "-relocated-" + Math.abs(this.relocations.hashCode()) + ".jar" : null;
@@ -228,7 +224,7 @@ public String getClassifier() {
      * @return true if library has classifier, false otherwise
      */
     public boolean hasClassifier() {
-        return classifier != null;
+        return classifier != null && !classifier.isEmpty();
     }
 
     /**

core/src/main/java/com/alessiodp/libby/LibraryManager.java

@@ -424,11 +424,7 @@ protected String getURLFromMetadata(@NotNull InputStream inputStream, @NotNull L
             version = version.substring(0, version.length() - "-SNAPSHOT".length());
         }
 
-        String url = library.getPartialPath() + library.getArtifactId() + '-' + version + '-' + timestamp + '-' + buildNumber;
-        if (library.hasClassifier()) {
-            url += '-' + library.getClassifier();
-        }
-        return url + ".jar";
+        return Util.craftPath(library.getPartialPath(), library.getArtifactId(), version + '-' + timestamp + '-' + buildNumber, library.getClassifier());
     }
 
     /**

core/src/main/java/com/alessiodp/libby/Util.java

@@ -1,6 +1,7 @@
 package com.alessiodp.libby;
 
 import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
 
 /**
  * Libby's utility class.
@@ -28,7 +29,7 @@ public static String replaceWithDots(@NotNull String str) {
      * @param string The string to convert
      * @return The byte array
      */
-    public static byte[] hexStringToByteArray(String string) {
+    public static byte[] hexStringToByteArray(@NotNull String string) {
         int len = string.length();
         byte[] data = new byte[len / 2];
         for (int i = 0; i < len; i += 2) {
@@ -37,4 +38,37 @@ public static byte[] hexStringToByteArray(String string) {
         }
         return data;
     }
+
+    /**
+     * Constructs the partial path of a {@link Library} given its artifactId, groupId and version.
+     *
+     * @param artifactId The artifactId of the library.
+     * @param groupId The groupId of the library.
+     * @param version The version of the library.
+     * @return The partial path of the library.
+     * @see Library#getPartialPath()
+     */
+    @NotNull
+    public static String craftPartialPath(@NotNull String artifactId, @NotNull String groupId, @NotNull String version) {
+        return groupId.replace('.', '/') + '/' + artifactId + '/' + version + '/';
+    }
+
+    /**
+     * Constructs the path of a {@link Library} given its partialPath, artifactId, version and classifier.
+     *
+     * @param partialPath The partialPath of the library.
+     * @param artifactId The artifactId of the library.
+     * @param version The version of the library.
+     * @param classifier The classifier of the library. May be null.
+     * @return The path of the library.
+     * @see Library#getPath()
+     */
+    @NotNull
+    public static String craftPath(@NotNull String partialPath, @NotNull String artifactId, @NotNull String version, @Nullable String classifier) {
+        String path = partialPath + artifactId + '-' + version;
+        if (classifier != null && !classifier.isEmpty()) {
+            path += '-' + classifier;
+        }
+        return path + ".jar";
+    }
 }

core/src/main/java/com/alessiodp/libby/transitive/TransitiveDependencyCollector.java

@@ -8,6 +8,7 @@
 import org.eclipse.aether.artifact.DefaultArtifact;
 import org.eclipse.aether.collection.CollectRequest;
 import org.eclipse.aether.graph.Dependency;
+import org.eclipse.aether.repository.ArtifactRepository;
 import org.eclipse.aether.repository.LocalRepository;
 import org.eclipse.aether.repository.RemoteRepository;
 import org.eclipse.aether.resolution.ArtifactResult;
@@ -18,13 +19,17 @@
 import org.eclipse.aether.util.artifact.JavaScopes;
 import org.eclipse.aether.util.filter.ScopeDependencyFilter;
 import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
 
 import java.nio.file.Path;
+import java.util.AbstractMap.SimpleEntry;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
+import java.util.Map.Entry;
 import java.util.Properties;
+import java.util.concurrent.atomic.AtomicInteger;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
@@ -35,6 +40,13 @@
  */
 class TransitiveDependencyCollector {
 
+    /**
+     * Counter used to generate ids for repositories
+     *
+     * @see #newDefaultRepository(String)
+     */
+    private static final AtomicInteger ID_GENERATOR = new AtomicInteger();
+
     /**
      * Maven repository system
      *
@@ -72,7 +84,7 @@ public TransitiveDependencyCollector(@NotNull Path saveDirectory) {
      */
     @NotNull
     public static RemoteRepository newDefaultRepository(@NotNull String url) {
-        return new RemoteRepository.Builder(url, "default", url).build();
+        return new RemoteRepository.Builder("repo" + ID_GENERATOR.getAndIncrement(), "default", url).build();
     }
 
     /**
@@ -83,19 +95,30 @@ public static RemoteRepository newDefaultRepository(@NotNull String url) {
      * @param version      Maven dependency version
      * @param classifier   Maven artifact classifier. May be null
      * @param repositories Maven repositories that would be used for dependency resolution
-     * @return Transitive dependencies, exception otherwise
+     * @return Transitive dependencies paired with their repository url, exception otherwise
      * @throws DependencyResolutionException thrown if dependency doesn't exist on provided repositories
      */
     @NotNull
-    public Collection<Artifact> findTransitiveDependencies(@NotNull String groupId, @NotNull String artifactId, @NotNull String version, @NotNull String classifier, @NotNull List<RemoteRepository> repositories) throws DependencyResolutionException {
+    public Collection<Entry<Artifact, @Nullable String>> findTransitiveDependencies(@NotNull String groupId, @NotNull String artifactId, @NotNull String version, @NotNull String classifier, @NotNull List<RemoteRepository> repositories) throws DependencyResolutionException {
         Artifact artifact = new DefaultArtifact(groupId, artifactId, classifier, "jar", version);
 
         CollectRequest collectRequest = new CollectRequest(new Dependency(artifact, JavaScopes.COMPILE), repositories);
         DependencyRequest dependencyRequest = new DependencyRequest(collectRequest, new ScopeDependencyFilter(Arrays.asList(JavaScopes.COMPILE, JavaScopes.RUNTIME), Collections.emptyList()));
 
         DependencyResult dependencyResult = repositorySystem.resolveDependencies(repositorySystemSession, dependencyRequest);
 
-        return dependencyResult.getArtifactResults().stream().filter(ArtifactResult::isResolved).map(ArtifactResult::getArtifact).collect(Collectors.toList());
+        return dependencyResult.getArtifactResults()
+                .stream()
+                .filter(ArtifactResult::isResolved)
+                .map(artifactResult -> {
+                    ArtifactRepository repo = artifactResult.getRepository();
+                    String url = null;
+                    if (repo instanceof RemoteRepository) {
+                        url = ((RemoteRepository) repo).getUrl();
+                    }
+                    return new SimpleEntry<>(artifactResult.getArtifact(), url);
+                })
+                .collect(Collectors.toList());
     }
 
     /**
@@ -106,12 +129,12 @@ public Collection<Artifact> findTransitiveDependencies(@NotNull String groupId,
      * @param version      Maven artifact version
      * @param classifier   Maven artifact classifier. May be null
      * @param repositories Maven repositories for transitive dependencies search
-     * @return Transitive dependencies, exception otherwise
+     * @return Transitive dependencies paired with their repository url, exception otherwise
      * @throws DependencyResolutionException thrown if dependency doesn't exist on provided repositories
      * @see #findTransitiveDependencies(String, String, String, String, List)
      */
     @NotNull
-    public Collection<Artifact> findTransitiveDependencies(@NotNull String groupId, @NotNull String artifactId, @NotNull String version, @NotNull String classifier, @NotNull Stream<String> repositories) throws DependencyResolutionException {
+    public Collection<Entry<Artifact, @Nullable String>> findTransitiveDependencies(@NotNull String groupId, @NotNull String artifactId, @NotNull String version, @NotNull String classifier, @NotNull Stream<String> repositories) throws DependencyResolutionException {
         return findTransitiveDependencies(groupId, artifactId, version, classifier, repositories.map(TransitiveDependencyCollector::newDefaultRepository).collect(Collectors.toList()));
     }
 

core/src/main/java/com/alessiodp/libby/transitive/TransitiveDependencyHelper.java

@@ -2,8 +2,10 @@
 
 import com.alessiodp.libby.Library;
 import com.alessiodp.libby.LibraryManager;
+import com.alessiodp.libby.Util;
 import com.alessiodp.libby.classloader.IsolatedClassLoader;
 import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
 
 import java.io.IOException;
 import java.lang.reflect.Constructor;
@@ -14,6 +16,7 @@
 import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
+import java.util.Map.Entry;
 import java.util.Set;
 import java.util.stream.Stream;
 
@@ -46,7 +49,7 @@ public class TransitiveDependencyHelper {
     /**
      * Reflected getter methods of Artifact class
      */
-    private final Method artifactGetGroupIdMethod, artifactGetArtifactIdMethod, artifactGetVersionMethod, artifactGetClassifierMethod;
+    private final Method artifactGetGroupIdMethod, artifactGetArtifactIdMethod, artifactGetVersionMethod, artifactGetBaseVersionMethod, artifactGetClassifierMethod;
 
     /**
      * LibraryManager instance, used in {@link #findTransitiveLibraries(Library)}
@@ -95,6 +98,8 @@ public TransitiveDependencyHelper(@NotNull LibraryManager libraryManager, @NotNu
             artifactGetArtifactIdMethod = artifactClass.getMethod("getArtifactId");
             // org.eclipse.aether.artifact.Artifact#getVersion()
             artifactGetVersionMethod = artifactClass.getMethod("getVersion");
+            // org.eclipse.aether.artifact.Artifact#getBaseVersion()
+            artifactGetBaseVersionMethod = artifactClass.getMethod("getBaseVersion");
             // org.eclipse.aether.artifact.Artifact#getClassifier()
             artifactGetClassifierMethod = artifactClass.getMethod("getClassifier");
         } catch (ReflectiveOperationException e) {
@@ -111,7 +116,7 @@ public TransitiveDependencyHelper(@NotNull LibraryManager libraryManager, @NotNu
      * </p>
      * <p>
      * Note: The method merges the repositories from both the library manager and the given library
-     * for dependency resolution. And clones all relocations into transitive libraries.
+     * for dependency resolution. It also clones all relocations into transitive libraries.
      * </p>
      *
      * @param library The primary library for which transitive dependencies need to be found.
@@ -132,16 +137,20 @@ public Collection<Library> findTransitiveLibraries(@NotNull Library library) {
 
         Stream<String> repositories = Stream.of(globalRepositories, libraryRepositories).flatMap(Collection::stream);
         try {
-            Collection<?> artifacts = (Collection<?>) resolveTransitiveDependenciesMethod.invoke(transitiveDependencyCollectorObject,
+            Collection<?> resolvedArtifacts = (Collection<?>) resolveTransitiveDependenciesMethod.invoke(transitiveDependencyCollectorObject,
                 library.getGroupId(),
                 library.getArtifactId(),
                 library.getVersion(),
                 library.getClassifier(),
                 repositories);
-            for (Object artifact : artifacts) {
+            for (Object resolved : resolvedArtifacts) {
+                Entry<?, ?> resolvedEntry = (Entry<?, ?>) resolved;
+                Object artifact = resolvedEntry.getKey();
+                @Nullable String repository = (String) resolvedEntry.getValue();
+
                 String groupId = (String) artifactGetGroupIdMethod.invoke(artifact);
                 String artifactId = (String) artifactGetArtifactIdMethod.invoke(artifact);
-                String version = (String) artifactGetVersionMethod.invoke(artifact);
+                String baseVersion = (String) artifactGetBaseVersionMethod.invoke(artifact);
                 String classifier = (String) artifactGetClassifierMethod.invoke(artifact);
 
                 if (library.getGroupId().equals(groupId) && library.getArtifactId().equals(artifactId))
@@ -153,7 +162,7 @@ public Collection<Library> findTransitiveLibraries(@NotNull Library library) {
                 Library.Builder libraryBuilder = Library.builder()
                                                         .groupId(groupId)
                                                         .artifactId(artifactId)
-                                                        .version(version)
+                                                        .version(baseVersion)
                                                         .isolatedLoad(library.isIsolatedLoad())
                                                         .loaderId(library.getLoaderId());
 
@@ -162,7 +171,30 @@ public Collection<Library> findTransitiveLibraries(@NotNull Library library) {
                 }
 
                 library.getRelocations().forEach(libraryBuilder::relocate);
-                library.getRepositories().forEach(libraryBuilder::repository);
+
+                if (repository != null) {
+                    // Construct direct download URL
+
+                    // Add ending "/" if missing
+                    if (!repository.endsWith("/")) {
+                        repository = repository + '/';
+                    }
+
+                    // TODO Uncomment the line below once LibraryManager#resolveLibrary stops resolving snapshots
+                    //      for every repository before trying direct URLs
+                    // Make sure the repository is added as fallback if the dependency isn't found at the constructed URL
+                    // libraryBuilder.repository(repository);
+
+                    // For snapshots, getVersion() returns version-timestamp-buildNumber instead of version-SNAPSHOT
+                    String version = (String) artifactGetVersionMethod.invoke(artifact);
+
+                    String partialPath = Util.craftPartialPath(artifactId, groupId, baseVersion);
+                    String path = Util.craftPath(partialPath, artifactId, version, classifier);
+
+                    libraryBuilder.url(repository + path);
+                } else {
+                    library.getRepositories().forEach(libraryBuilder::repository);
+                }
 
                 transitiveLibraries.add(libraryBuilder.build());
             }

core/src/test/java/com/alessiodp/libby/transitive/TransitiveDownloadingTest.java

@@ -28,6 +28,15 @@ public class TransitiveDownloadingTest {
             .resolveTransitiveDependencies(true)
             .excludeTransitiveDependency(EXCLUDED_LIBRARY.getGroupId(), EXCLUDED_LIBRARY.getArtifactId())
             .build();
+    private static final Library BUNGEECORD = Library.builder()
+            .groupId("net{}md-5")
+            .artifactId("bungeecord-api")
+            .version("1.20-R0.2-SNAPSHOT")
+            .repository("https://oss.sonatype.org/content/repositories/snapshots")
+            .isolatedLoad(true)
+            .loaderId("bungeecord")
+            .resolveTransitiveDependencies(true)
+            .build();
 
     private LibraryManagerMock libraryManager;
 
@@ -49,6 +58,13 @@ public void transitiveLoad() {
         checkDownloadedDependencies();
     }
 
+    @Test
+    public void snapshotLibraryTransitiveLoad() throws Exception {
+        libraryManager.loadLibrary(BUNGEECORD);
+
+        assertNotNull(libraryManager.getIsolatedClassLoaderById("bungeecord").loadClass("net.md_5.bungee.api.ProxyServer"));
+    }
+
     @Test
     public void transitiveWithExcludedLoad() {
         libraryManager.loadLibrary(MAVEN_RESOLVER_SUPPLIER_WITH_EXCLUDED);