BI-196 - Change Dependency scopes to be held in a set, instead of a list

Author: danf

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

@@ -19,6 +19,7 @@
 import com.thoughtworks.xstream.annotations.XStreamAlias;
 
 import java.util.List;
+import java.util.Set;
 
 /**
  * Contains the build module dependency information
@@ -31,7 +32,7 @@ public class Dependency extends BaseBuildFileBean {
     public static final String SCOPE_BUILD = "_build_";
 
     private String id;
-    private List<String> scopes;
+    private Set<String> scopes;
     private List<String> requiredBy;
 
     /**
@@ -57,7 +58,7 @@ public void setId(String id) {
      *
      * @return Dependency scope list
      */
-    public List<String> getScopes() {
+    public Set<String> getScopes() {
         return scopes;
     }
 
@@ -66,7 +67,7 @@ public List<String> getScopes() {
      *
      * @param scopes Dependency scope list
      */
-    public void setScopes(List<String> scopes) {
+    public void setScopes(Set<String> scopes) {
         this.scopes = scopes;
     }
 

build-info-api/src/main/java/org/jfrog/build/api/builder/BuildInfoMavenBuilder.java

@@ -26,6 +26,7 @@
 import java.util.Date;
 import java.util.List;
 import java.util.Properties;
+import java.util.Set;
 
 /**
  * A temporary builder for the build class specifically for Maven extractor.
@@ -383,18 +384,14 @@ private void mergeModuleDependencies(Module existingModule, Module moduleToMerge
             return;
         }
 
-        for (Dependency dependencyToMarge : dependenciesToMerge) {
-            Dependency foundDependency = findDependency(existingDependencies, dependencyToMarge.getId());
+        for (Dependency dependencyToMerge : dependenciesToMerge) {
+            Dependency foundDependency = findDependency(existingDependencies, dependencyToMerge.getId());
             if (foundDependency == null) {
-                existingDependencies.add(dependencyToMarge);
+                existingDependencies.add(dependencyToMerge);
             } else {
-                List<String> existingScopes = foundDependency.getScopes();
-                List<String> scopesToMerge = dependencyToMarge.getScopes();
-                for (String scopeToMerge : scopesToMerge) {
-                    if (!existingScopes.contains(scopeToMerge)) {
-                        existingScopes.add(scopeToMerge);
-                    }
-                }
+                Set<String> mergedDependencies = foundDependency.getScopes();
+                mergedDependencies.addAll(dependencyToMerge.getScopes());
+                foundDependency.setScopes(mergedDependencies);
             }
         }
     }

build-info-api/src/main/java/org/jfrog/build/api/builder/DependencyBuilder.java

@@ -21,6 +21,7 @@
 
 import java.util.List;
 import java.util.Properties;
+import java.util.Set;
 
 /**
  * A builder for the dependency class
@@ -31,7 +32,7 @@ public class DependencyBuilder {
 
     private String id;
     private String type;
-    private List<String> scopes;
+    private Set<String> scopes;
     private String sha1;
     private String md5;
     private List<String> requiredBy;
@@ -82,7 +83,7 @@ public DependencyBuilder type(String type) {
      * @param scopes Dependency scope list
      * @return Builder instance
      */
-    public DependencyBuilder scopes(List<String> scopes) {
+    public DependencyBuilder scopes(Set<String> scopes) {
         this.scopes = scopes;
         return this;
     }

build-info-api/src/test/java/org/jfrog/build/api/DependencyTest.java

@@ -18,12 +18,14 @@
 
 import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
+import com.google.common.collect.Sets;
 import org.testng.Assert;
 import org.testng.annotations.Test;
 
 import java.util.HashMap;
 import java.util.List;
 import java.util.Properties;
+import java.util.Set;
 
 import static org.testng.Assert.assertEquals;
 import static org.testng.Assert.assertNull;
@@ -56,7 +58,7 @@ public void testEmptyConstructor() {
     public void testSetters() {
         String id = "moo";
         String type = "bob";
-        List<String> scopes = Lists.newArrayList("mitzi");
+        Set<String> scopes = Sets.newHashSet("mitzi");
         String sha1 = "pop";
         String md5 = "shmop";
         List<String> requiredBy = Lists.newArrayList("pitzi");
@@ -87,7 +89,7 @@ public void testEqualsAndHash() {
         dependency1.setSha1("111");
         dependency1.setMd5("1111");
         dependency1.setRequiredBy(Lists.newArrayList("11111"));
-        dependency1.setScopes(Lists.newArrayList("1", "11"));
+        dependency1.setScopes(Sets.newHashSet("1", "11"));
         dependency1.setProperties(properties);
 
         Dependency dependency2 = new Dependency();
@@ -96,7 +98,7 @@ public void testEqualsAndHash() {
         dependency2.setSha1("111");
         dependency2.setMd5("1111");
         dependency2.setRequiredBy(Lists.newArrayList("11111"));
-        dependency2.setScopes(Lists.newArrayList("1", "11"));
+        dependency2.setScopes(Sets.newHashSet("1", "11"));
         dependency2.setProperties(properties);
 
         Dependency dependency3 = new Dependency();
@@ -105,7 +107,7 @@ public void testEqualsAndHash() {
         dependency3.setSha1("333");
         dependency3.setMd5("3333");
         dependency3.setRequiredBy(Lists.newArrayList("33333"));
-        dependency3.setScopes(Lists.newArrayList("333333", "3333333"));
+        dependency3.setScopes(Sets.newHashSet("333333", "3333333"));
         dependency3.setProperties(properties);
 
         Assert.assertEquals(dependency1, dependency2, "Expected equals == true for equivalent artifacts");

build-info-api/src/test/java/org/jfrog/build/api/builder/BuildInfoMavenBuilderTest.java

@@ -1,6 +1,7 @@
 package org.jfrog.build.api.builder;
 
 import com.google.common.collect.Lists;
+import com.google.common.collect.Sets;
 import org.jfrog.build.api.*;
 import org.jfrog.build.api.release.PromotionStatus;
 import org.testng.annotations.Test;
@@ -182,12 +183,12 @@ public void testDuplicateModuleArtifacts() {
      */
     public void testDuplicateModuleDependencies() {
         ModuleBuilder module1 = new ModuleBuilder().id("id");
-        module1.addDependency(new DependencyBuilder().id("dep1").scopes(Lists.newArrayList("compile")).build());
-        module1.addDependency(new DependencyBuilder().id("dep2").scopes(Lists.newArrayList("compile")).build());
+        module1.addDependency(new DependencyBuilder().id("dep1").scopes(Sets.newHashSet("compile")).build());
+        module1.addDependency(new DependencyBuilder().id("dep2").scopes(Sets.newHashSet("compile")).build());
 
         ModuleBuilder module2 = new ModuleBuilder().id("id");
-        module2.addDependency(new DependencyBuilder().id("dep1").scopes(Lists.newArrayList("compile", "test")).build());
-        module2.addDependency(new DependencyBuilder().id("dep2").scopes(Lists.newArrayList("compile", "test")).build());
+        module2.addDependency(new DependencyBuilder().id("dep1").scopes(Sets.newHashSet("compile", "test")).build());
+        module2.addDependency(new DependencyBuilder().id("dep2").scopes(Sets.newHashSet("compile", "test")).build());
 
         BuildInfoMavenBuilder builder = new BuildInfoMavenBuilder("test").number("4").started("test");
         builder.addModule(module1.build());

build-info-api/src/test/java/org/jfrog/build/api/builder/DependencyBuilderTest.java

@@ -17,11 +17,13 @@
 package org.jfrog.build.api.builder;
 
 import com.google.common.collect.Lists;
+import com.google.common.collect.Sets;
 import org.jfrog.build.api.Dependency;
 import org.testng.annotations.Test;
 
 import java.util.List;
 import java.util.Properties;
+import java.util.Set;
 
 import static org.testng.Assert.*;
 
@@ -54,7 +56,7 @@ public void testDefaultBuild() {
     public void testBuilderSetters() {
         String id = "moo";
         String type = "bob";
-        List<String> scopes = Lists.newArrayList("mitzi");
+        Set<String> scopes = Sets.newHashSet("mitzi");
         String sha1 = "pop";
         String md5 = "shmop";
         List<String> requiredBy = Lists.newArrayList("pitzi");
@@ -85,7 +87,7 @@ public void testBuilderAddMethods() {
                 addProperty(propertyKey, propertyValue).build();
         List<String> requiredByList = dependency.getRequiredBy();
         assertFalse(requiredByList.isEmpty(), "A dependency requirement should have been added.");
-        assertEquals(requiredByList.get(0), requiredBy, "Unexpected dependency requirement.");
+        assertEquals(requiredByList.iterator().next(), requiredBy, "Unexpected dependency requirement.");
         assertTrue(dependency.getProperties().containsKey(propertyKey),
                 "A dependency property should have been added.");
         assertEquals(dependency.getProperties().get(propertyKey), propertyValue,

build-info-extractor-gradle/src/main/groovy/org/jfrog/gradle/plugin/artifactory/extractor/GradleBuildInfoExtractor.java

@@ -19,6 +19,7 @@
 import com.google.common.base.Function;
 import com.google.common.base.Predicate;
 import com.google.common.collect.Iterables;
+import com.google.common.collect.Sets;
 import org.apache.commons.lang.StringUtils;
 import org.gradle.api.Project;
 import org.gradle.api.Task;
@@ -32,12 +33,12 @@
 import org.jfrog.build.api.builder.*;
 import org.jfrog.build.api.release.Promotion;
 import org.jfrog.build.api.util.FileChecksumCalculator;
-import org.jfrog.build.extractor.clientConfiguration.ArtifactoryClientConfiguration;
 import org.jfrog.build.client.DeployDetails;
-import org.jfrog.build.extractor.clientConfiguration.IncludeExcludePatterns;
-import org.jfrog.build.extractor.clientConfiguration.PatternMatcher;
 import org.jfrog.build.extractor.BuildInfoExtractor;
 import org.jfrog.build.extractor.BuildInfoExtractorUtils;
+import org.jfrog.build.extractor.clientConfiguration.ArtifactoryClientConfiguration;
+import org.jfrog.build.extractor.clientConfiguration.IncludeExcludePatterns;
+import org.jfrog.build.extractor.clientConfiguration.PatternMatcher;
 import org.jfrog.gradle.plugin.artifactory.ArtifactoryPluginUtil;
 import org.jfrog.gradle.plugin.artifactory.task.BuildInfoBaseTask;
 
@@ -321,18 +322,16 @@ public boolean apply(@Nullable Dependency input) {
                     // if it's already in the dependencies list just add the current scope
                     if (any(dependencies, idEqualsPredicate)) {
                         Dependency existingDependency = find(dependencies, idEqualsPredicate);
-                        List<String> existingScopes = existingDependency.getScopes();
-                        String configScope = configuration.getName();
-                        if (!existingScopes.contains(configScope)) {
-                            existingScopes.add(configScope);
-                        }
+                        Set<String> existingScopes = existingDependency.getScopes();
+                        existingScopes.add(configuration.getName());
+                        existingDependency.setScopes(existingScopes);
                     } else {
                         Map<String, String> checksums = FileChecksumCalculator.calculateChecksums(file, MD5, SHA1);
                         DependencyBuilder dependencyBuilder = new DependencyBuilder()
                                 .type(getTypeString(artifact.getType(),
                                         artifact.getClassifier(), artifact.getExtension()))
                                 .id(depId)
-                                .scopes(newArrayList(configuration.getName())).
+                                .scopes(Sets.newHashSet(configuration.getName())).
                                         md5(checksums.get(MD5)).sha1(checksums.get(SHA1));
                         dependencies.add(dependencyBuilder.build());
                     }

build-info-extractor-ivy/src/main/java/org/jfrog/build/extractor/trigger/ArtifactoryBuildInfoTrigger.java

@@ -3,6 +3,7 @@
 import com.google.common.base.Predicate;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
+import com.google.common.collect.Sets;
 import org.apache.commons.lang.StringUtils;
 import org.apache.ivy.ant.IvyTask;
 import org.apache.ivy.core.IvyContext;
@@ -27,11 +28,11 @@
 import org.jfrog.build.api.builder.DependencyBuilder;
 import org.jfrog.build.api.builder.ModuleBuilder;
 import org.jfrog.build.api.util.FileChecksumCalculator;
-import org.jfrog.build.extractor.clientConfiguration.ArtifactoryClientConfiguration;
 import org.jfrog.build.client.DeployDetails;
+import org.jfrog.build.context.BuildContext;
+import org.jfrog.build.extractor.clientConfiguration.ArtifactoryClientConfiguration;
 import org.jfrog.build.extractor.clientConfiguration.IncludeExcludePatterns;
 import org.jfrog.build.extractor.clientConfiguration.PatternMatcher;
-import org.jfrog.build.context.BuildContext;
 import org.jfrog.build.util.IvyResolverHelper;
 
 import java.io.File;
@@ -114,7 +115,7 @@ private void collectDependencyInformation(IvyEvent event) {
                     Dependency dependency = findDependencyInList(id, type, moduleDependencies);
                     if (dependency == null) {
                         DependencyBuilder dependencyBuilder = new DependencyBuilder();
-                        dependencyBuilder.type(type).scopes(Lists.newArrayList(configuration));
+                        dependencyBuilder.type(type).scopes(Sets.newHashSet(configuration));
                         String idString = getModuleIdString(id.getOrganisation(),
                                 id.getName(), id.getRevision());
                         dependencyBuilder.id(idString);

build-info-extractor-maven3/src/main/java/org/jfrog/build/extractor/maven/BuildInfoRecorder.java

@@ -40,7 +40,7 @@
 import org.jfrog.build.api.builder.DependencyBuilder;
 import org.jfrog.build.api.builder.ModuleBuilder;
 import org.jfrog.build.api.util.FileChecksumCalculator;
-import org.jfrog.build.client.*;
+import org.jfrog.build.client.DeployDetails;
 import org.jfrog.build.extractor.BuildInfoExtractor;
 import org.jfrog.build.extractor.BuildInfoExtractorUtils;
 import org.jfrog.build.extractor.clientConfiguration.ArtifactoryClientConfiguration;
@@ -49,6 +49,7 @@
 import org.jfrog.build.extractor.clientConfiguration.PatternMatcher;
 import org.jfrog.build.extractor.maven.resolver.ResolutionHelper;
 import org.xml.sax.InputSource;
+
 import javax.xml.xpath.*;
 import java.io.File;
 import java.io.FileInputStream;
@@ -105,7 +106,8 @@ public void setConfiguration(ArtifactoryClientConfiguration conf) {
     /**
      * The repository listeners (either ArtifactoryEclipseRepositoryListener or ArtifactorySonatypeRepositoryListener) invoke this method
      * with each artifact being resolved by Maven.
-     * @param artifact  The artifact being resolved by Maven.
+     *
+     * @param artifact The artifact being resolved by Maven.
      */
     public void artifactResolved(Artifact artifact) {
         if (artifact != null) {
@@ -410,13 +412,14 @@ private void extractModuleDependencies(MavenProject project) {
 
     /**
      * Merge the dependencies taken from the MavenProject object with those collected inside the resolvedArtifacts collection.
-     * @param projectDependencies   The artifacts taken from the MavenProject object.
+     *
+     * @param projectDependencies The artifacts taken from the MavenProject object.
      */
     private void mergeProjectDependencies(Set<Artifact> projectDependencies) {
         // Go over all the artifacts taken from the MavenProject object, and replace their equals method, so that we are
         // able to merge them together with the artifacts inside the resolvedArtifacts set:
         Set<Artifact> dependecies = Sets.newHashSet();
-        for(Artifact artifact : projectDependencies) {
+        for (Artifact artifact : projectDependencies) {
             String classifier = artifact.getClassifier();
             classifier = classifier == null ? "" : classifier;
             DefaultArtifact art = new DefaultArtifact(artifact.getGroupId(), artifact.getArtifactId(), artifact.getVersion(),
@@ -595,7 +598,7 @@ private void addDependenciesToCurrentModule(ModuleBuilder module) {
                             dependency.getClassifier(), getExtension(depFile)));
             String scopes = dependency.getScope();
             if (StringUtils.isNotBlank(scopes)) {
-                dependencyBuilder.scopes(Lists.newArrayList(scopes));
+                dependencyBuilder.scopes(Sets.newHashSet(scopes));
             }
             setDependencyChecksums(depFile, dependencyBuilder);
             module.addDependency(dependencyBuilder.build());