Bugfix - Build-info ignores duplicate artifacts checksum (#579)

Author: Or-Geva

Diff for: build-info-api/src/main/java/org/jfrog/build/api/Artifact.java

@@ -17,6 +17,7 @@
 package org.jfrog.build.api;
 
 import com.thoughtworks.xstream.annotations.XStreamAlias;
+import org.apache.commons.lang.StringUtils;
 
 /**
  * Contains the build deployed artifact information
@@ -59,10 +60,7 @@ public boolean equals(Object o) {
         }
 
         Artifact artifact = (Artifact) o;
-        if (name != null ? !name.equals(artifact.name) : artifact.name != null) {
-            return false;
-        }
-        return true;
+        return StringUtils.equals(name, artifact.name) && StringUtils.equals(remotePath, artifact.remotePath);
     }
 
     @Override

Diff for: build-info-api/src/main/java/org/jfrog/build/api/BaseBuildFileBean.java

@@ -111,6 +111,7 @@ public int hashCode() {
         result = 31 * result + (sha1 != null ? sha1.hashCode() : 0);
         result = 31 * result + (sha256 != null ? sha256.hashCode() : 0);
         result = 31 * result + (md5 != null ? md5.hashCode() : 0);
+        result = 31 * result + (remotePath != null ? remotePath.hashCode() : 0);
         return result;
     }
 }

Diff for: build-info-api/src/main/java/org/jfrog/build/api/Dependency.java

@@ -18,6 +18,7 @@
 
 import com.thoughtworks.xstream.annotations.XStreamAlias;
 import org.apache.commons.lang.ArrayUtils;
+import org.apache.commons.lang.StringUtils;
 
 import java.util.Arrays;
 import java.util.Objects;
@@ -121,7 +122,10 @@ public boolean equals(Object o) {
         }
         Dependency that = (Dependency) o;
 
-        if (!Objects.equals(id, that.id)) {
+        if (!StringUtils.equals(id, that.id)) {
+            return false;
+        }
+        if (!StringUtils.equals(remotePath, that.remotePath)) {
             return false;
         }
         if (!Objects.equals(scopes, that.scopes)) {
@@ -132,6 +136,6 @@ public boolean equals(Object o) {
 
     @Override
     public int hashCode() {
-        return Objects.hash(id, scopes, Arrays.deepHashCode(requestedBy));
+        return Objects.hash(id, scopes, Arrays.deepHashCode(requestedBy), remotePath);
     }
 }

Diff for: build-info-extractor/src/main/java/org/jfrog/build/extractor/clientConfiguration/util/DependenciesDownloaderHelper.java

@@ -21,9 +21,22 @@
 import org.jfrog.filespecs.FileSpec;
 import org.jfrog.filespecs.entities.FilesGroup;
 
-import java.io.*;
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileNotFoundException;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.PrintWriter;
+import java.io.SequenceInputStream;
+import java.io.StringWriter;
 import java.security.NoSuchAlgorithmException;
-import java.util.*;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
 import java.util.regex.Pattern;
 
 import static org.jfrog.build.extractor.clientConfiguration.client.artifactory.services.Upload.MD5_HEADER_NAME;
@@ -62,7 +75,7 @@ public DependenciesDownloaderHelper(ArtifactoryManager artifactoryManager, Strin
 
     /**
      * Download dependencies by the provided spec using the provided in the constructor client.
-     * returns list of downloaded artifacts
+     * returns a distinct list of downloaded artifacts
      *
      * @param downloadSpec the download spec
      * @return list of downloaded artifacts
@@ -72,7 +85,7 @@ public List<Dependency> downloadDependencies(FileSpec downloadSpec) throws IOExc
         ArtifactorySearcher searcher = new ArtifactorySearcher(downloader.getArtifactoryManager(), log);
         Set<DownloadableArtifact> downloadableArtifacts;
         List<AqlSearchResult.SearchEntry> searchResults;
-        List<Dependency> resolvedDependencies = new ArrayList<>();
+        HashSet<Dependency> resolvedDependencies = new HashSet<>();
 
         for (FilesGroup file : downloadSpec.getFiles()) {
             log.debug("Downloading dependencies using spec: \n" + file.toString());
@@ -84,7 +97,7 @@ public List<Dependency> downloadDependencies(FileSpec downloadSpec) throws IOExc
             }
             resolvedDependencies.addAll(downloadDependencies(downloadableArtifacts));
         }
-        return resolvedDependencies;
+        return new ArrayList<>(resolvedDependencies);
     }
 
     private void replaceTargetPlaceholders(String searchPattern, Set<DownloadableArtifact> downloadableArtifacts, String target) {

Diff for: build-info-extractor/src/test/java/org/jfrog/build/extractor/clientConfiguration/util/DownloadTest.java

@@ -7,6 +7,8 @@
 import org.jfrog.build.api.dependency.pattern.PatternType;
 import org.jfrog.build.api.util.FileChecksumCalculator;
 import org.jfrog.build.extractor.clientConfiguration.deploy.DeployDetails;
+import org.jfrog.filespecs.FileSpec;
+import org.jfrog.filespecs.entities.FilesGroup;
 import org.testng.Assert;
 import org.testng.annotations.AfterClass;
 import org.testng.annotations.BeforeClass;
@@ -18,9 +20,13 @@
 import java.io.RandomAccessFile;
 import java.security.NoSuchAlgorithmException;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 
-import static org.jfrog.build.extractor.clientConfiguration.util.DependenciesDownloaderHelper.*;
+import static org.jfrog.build.extractor.clientConfiguration.util.DependenciesDownloaderHelper.ArtifactMetaData;
+import static org.jfrog.build.extractor.clientConfiguration.util.DependenciesDownloaderHelper.MD5_ALGORITHM_NAME;
+import static org.jfrog.build.extractor.clientConfiguration.util.DependenciesDownloaderHelper.MIN_SIZE_FOR_CONCURRENT_DOWNLOAD;
+import static org.jfrog.build.extractor.clientConfiguration.util.DependenciesDownloaderHelper.SHA1_ALGORITHM_NAME;
 
 /**
  * Integration tests for the DependenciesDownloader classes.
@@ -112,6 +118,36 @@ public void testDownloadArtifactWithoutContentLength(Map<String, String> uploade
         Assert.assertEquals((new File(targetDirPath + fileName)).length(), fileSize);
     }
 
+    public void testDownloadArtifactFromDifferentPath() throws IOException {
+        String targetDirPath = tempWorkspace.getPath() + File.separatorChar + "testDownloaddupArtifactFromDifferentPath" + File.separatorChar;
+        FileSpec fileSpec = new FileSpec();
+        // Upload one file to different locations in Artifactory.
+        try {
+            File file = createRandomFile(tempWorkspace.getPath() + File.pathSeparatorChar + "file", 1);
+            for (int i = 0; i < 3; i++) {
+                String filePath = TEST_REPO_PATH + "/" + i + "/file";
+                DeployDetails deployDetails = new DeployDetails.Builder()
+                        .file(file)
+                        .artifactPath(filePath)
+                        .targetRepository(localRepo1)
+                        .explode(false)
+                        .packageType(DeployDetails.PackageType.GENERIC)
+                        .build();
+                FilesGroup fg = new FilesGroup();
+                fg.setPattern(localRepo1 + "/" + filePath);
+                fg.setTarget(targetDirPath);
+                fileSpec.addFilesGroup(fg);
+                // Upload artifact
+                artifactoryManager.upload(deployDetails);
+            }
+            DependenciesDownloaderHelper helper = new DependenciesDownloaderHelper(artifactoryManager, tempWorkspace.getPath(), log);
+            List<Dependency> dependencies = helper.downloadDependencies(fileSpec);
+            Assert.assertEquals(dependencies.size(), 3);
+        } finally {
+            FileUtils.deleteDirectory(tempWorkspace);
+        }
+    }
+
     /**
      * Create and upload files to Artifactory.
      * The test files are created according to the data provided in testFilesMap