BI-558 - Improve requiredBy field to support path to root (#443)

Author: yahavi

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

@@ -17,8 +17,10 @@
 package org.jfrog.build.api;
 
 import com.thoughtworks.xstream.annotations.XStreamAlias;
+import org.apache.commons.lang.ArrayUtils;
 
-import java.util.List;
+import java.util.Arrays;
+import java.util.Objects;
 import java.util.Set;
 
 /**
@@ -33,7 +35,14 @@ public class Dependency extends BaseBuildFileBean {
 
     private String id;
     private Set<String> scopes;
-    private List<String> requiredBy;
+
+    /*
+     * [["parentIdA", "a1", "a2",... "moduleId"],
+     * ["parentIdB", "b1", "b2",... "moduleId"],
+     * ["parentIdC", "c1", "c2",... "moduleId"],
+     * ...]
+     */
+    private String[][] requiredBy;
 
     /**
      * Returns the ID of the dependency
@@ -72,24 +81,33 @@ public void setScopes(Set<String> scopes) {
     }
 
     /**
-     * Returns an ID list of dependencies that directly depend on this dependency. Used for building the module's
-     * transitive dependency graph. Can be left empty if a root dependency.
+     * Returns path-to-root dependency lists that directly depend on this dependency.
+     * Used for building the module's transitive dependency graph. Can be left empty if a root dependency.
      *
-     * @return Required dependency IDs list
+     * @return dependency path-to-root lists.
      */
-    public List<String> getRequiredBy() {
+    public String[][] getRequiredBy() {
         return requiredBy;
     }
 
     /**
-     * Sets an ID list of dependencies that directly depend on this dependency.
+     * Sets path-to-root dependency lists that directly depend on this dependency.
      *
-     * @param requiredBy Required dependency IDs list
+     * @param requiredBy dependency path-to-root lists
      */
-    public void setRequiredBy(List<String> requiredBy) {
+    public void setRequiredBy(String[][] requiredBy) {
         this.requiredBy = requiredBy;
     }
 
+    /**
+     * Adds an ID of another dependency required by this one to the required dependencies list.
+     *
+     * @param pathToModuleRoot - path from parent dependency to the module ID, modules separated
+     */
+    public void addRequiredBy(String[] pathToModuleRoot) {
+        this.requiredBy = (String[][]) ArrayUtils.add(requiredBy, pathToModuleRoot);
+    }
+
     @Override
     public boolean equals(Object o) {
         if (this == o) {
@@ -103,24 +121,17 @@ public boolean equals(Object o) {
         }
         Dependency that = (Dependency) o;
 
-        if (id != null ? !id.equals(that.id) : that.id != null) {
-            return false;
-        }
-        if (requiredBy != null ? !requiredBy.equals(that.requiredBy) : that.requiredBy != null) {
+        if (!Objects.equals(id, that.id)) {
             return false;
         }
-        if (scopes != null ? !scopes.equals(that.scopes) : that.scopes != null) {
+        if (!Objects.equals(scopes, that.scopes)) {
             return false;
         }
-        return true;
+        return Arrays.deepEquals(requiredBy, that.requiredBy);
     }
 
     @Override
     public int hashCode() {
-        int result = super.hashCode();
-        result = 31 * result + (id != null ? id.hashCode() : 0);
-        result = 31 * result + (scopes != null ? scopes.hashCode() : 0);
-        result = 31 * result + (requiredBy != null ? requiredBy.hashCode() : 0);
-        return result;
+        return Objects.hash(id, scopes, Arrays.deepHashCode(requiredBy));
     }
 }

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

@@ -16,9 +16,12 @@
 
 package org.jfrog.build.api.builder;
 
+import org.apache.commons.lang.ArrayUtils;
 import org.jfrog.build.api.Dependency;
 
-import java.util.*;
+import java.util.HashSet;
+import java.util.Properties;
+import java.util.Set;
 
 /**
  * A builder for the dependency class
@@ -35,7 +38,7 @@ public class DependencyBuilder {
     private String md5;
     private String localPath;
     private String remotePath;
-    private List<String> requiredBy;
+    private String[][] requiredBy;
     private Properties properties;
 
     /**
@@ -161,27 +164,24 @@ public DependencyBuilder remotePath(String remotePath) {
     }
 
     /**
-     * Sets an ID list of other dependencies required by this one
+     * Sets path-to-root dependency lists that directly depend on this dependency.
      *
-     * @param requiredBy Required dependency IDs list
+     * @param requiredBy dependency path-to-root lists
      * @return Builder instance
      */
-    public DependencyBuilder requiredBy(List<String> requiredBy) {
+    public DependencyBuilder requiredBy(String[][] requiredBy) {
         this.requiredBy = requiredBy;
         return this;
     }
 
     /**
-     * Adds an ID of another dependency required by this one to the required dependencies list
+     * Adds an ID of another dependency that requires this one to the requiredBy dependencies list.
      *
-     * @param requiredBy Required dependency ID
+     * @param pathToModuleRoot - path from parent dependency to the module ID, modules separated
      * @return Builder instance
      */
-    public DependencyBuilder addRequiredBy(String requiredBy) {
-        if (this.requiredBy == null) {
-            this.requiredBy = new ArrayList<>();
-        }
-        this.requiredBy.add(requiredBy);
+    public DependencyBuilder addRequiredBy(String[] pathToModuleRoot) {
+        this.requiredBy = (String[][]) ArrayUtils.add(requiredBy, pathToModuleRoot);
         return this;
     }
 

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

@@ -16,14 +16,15 @@
 
 package org.jfrog.build.api;
 
+import org.apache.commons.lang.ArrayUtils;
 import org.jfrog.build.api.util.CommonUtils;
-import org.testng.Assert;
 import org.testng.annotations.Test;
 
-import java.util.*;
+import java.util.HashMap;
+import java.util.Properties;
+import java.util.Set;
 
-import static org.testng.Assert.assertEquals;
-import static org.testng.Assert.assertNull;
+import static org.testng.Assert.*;
 
 /**
  * Tests the behavior of the dependency class
@@ -58,7 +59,7 @@ public void testSetters() {
         String sha1 = "pop";
         String sha256 = "lol";
         String md5 = "shmop";
-        List<String> requiredBy = Collections.singletonList("pitzi");
+        String[][] requiredBy = new String[][]{{"pitzi"}};
 
         Dependency dependency = new Dependency();
         dependency.setId(id);
@@ -78,6 +79,33 @@ public void testSetters() {
         assertEquals(dependency.getRequiredBy(), requiredBy, "Unexpected dependency required by.");
     }
 
+    public void testAddRequiredBy() {
+        // Create a dependency
+        Dependency dependency = new Dependency();
+        assertTrue(ArrayUtils.isEmpty(dependency.getRequiredBy()));
+
+        // Add required by A
+        String[] parentA = new String[]{"a", "b", "c"};
+        dependency.addRequiredBy(parentA);
+        assertEquals(ArrayUtils.getLength(dependency.getRequiredBy()), 1);
+        assertEquals(dependency.getRequiredBy()[0], parentA);
+
+        // Add required by B
+        String[] parentB = new String[]{"b", "c", "d", "e"};
+        dependency.addRequiredBy(parentB);
+        assertEquals(ArrayUtils.getLength(dependency.getRequiredBy()), 2);
+        assertEquals(dependency.getRequiredBy()[0], parentA);
+        assertEquals(dependency.getRequiredBy()[1], parentB);
+
+        // Add required by C
+        String[] parentC = new String[]{"c", "d"};
+        dependency.addRequiredBy(parentC);
+        assertEquals(ArrayUtils.getLength(dependency.getRequiredBy()), 3);
+        assertEquals(dependency.getRequiredBy()[0], parentA);
+        assertEquals(dependency.getRequiredBy()[1], parentB);
+        assertEquals(dependency.getRequiredBy()[2], parentC);
+    }
+
     public void testEqualsAndHash() {
         //Properties are not included in equals\hash
         Properties properties = new Properties();
@@ -88,7 +116,7 @@ public void testEqualsAndHash() {
         dependency1.setSha1("111");
         dependency1.setSha256("11111");
         dependency1.setMd5("1111");
-        dependency1.setRequiredBy(Collections.singletonList("11111"));
+        dependency1.setRequiredBy(new String[][]{{"11111"}});
         dependency1.setScopes(CommonUtils.newHashSet("1", "11"));
         dependency1.setProperties(properties);
 
@@ -98,7 +126,7 @@ public void testEqualsAndHash() {
         dependency2.setSha1("111");
         dependency2.setSha256("11111");
         dependency2.setMd5("1111");
-        dependency2.setRequiredBy(Collections.singletonList("11111"));
+        dependency2.setRequiredBy(new String[][]{{"11111"}});
         dependency2.setScopes(CommonUtils.newHashSet("1", "11"));
         dependency2.setProperties(properties);
 
@@ -108,21 +136,21 @@ public void testEqualsAndHash() {
         dependency3.setSha1("333");
         dependency3.setSha256("33333");
         dependency3.setMd5("3333");
-        dependency3.setRequiredBy(Collections.singletonList("33333"));
+        dependency3.setRequiredBy(new String[][]{{"33333"}});
         dependency3.setScopes(CommonUtils.newHashSet("333333", "3333333"));
         dependency3.setProperties(properties);
 
-        Assert.assertEquals(dependency1, dependency2, "Expected equals == true for equivalent artifacts");
-        Assert.assertTrue(!dependency1.equals(dependency3), "Expected equals == false for non-equivalent artifacts");
+        assertEquals(dependency1, dependency2, "Expected equals == true for equivalent artifacts");
+        assertNotEquals(dependency3, dependency1, "Expected equals == false for non-equivalent artifacts");
 
         HashMap<Dependency, String> testMap = new HashMap<>();
         testMap.put(dependency1, "1");
         testMap.put(dependency2, "2");
-        Assert.assertEquals(testMap.size(), 1, "Expected same hashcode for equal artifacts");
-        Assert.assertEquals(testMap.values().iterator().next(), "2", "Expected insertion of equivalent artifact to" +
+        assertEquals(testMap.size(), 1, "Expected same hashcode for equal artifacts");
+        assertEquals(testMap.values().iterator().next(), "2", "Expected insertion of equivalent artifact to" +
                 " map to replace old one");
 
         testMap.put(dependency3, "3");
-        Assert.assertEquals(testMap.size(), 2, "Expected artifact to not overwrite existing non-equivalent artifact");
+        assertEquals(testMap.size(), 2, "Expected artifact to not overwrite existing non-equivalent artifact");
     }
 }

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

@@ -16,12 +16,11 @@
 
 package org.jfrog.build.api.builder;
 
+import org.apache.commons.lang.ArrayUtils;
 import org.jfrog.build.api.Dependency;
 import org.jfrog.build.api.util.CommonUtils;
 import org.testng.annotations.Test;
 
-import java.util.Collections;
-import java.util.List;
 import java.util.Properties;
 import java.util.Set;
 
@@ -61,7 +60,7 @@ public void testBuilderSetters() {
         String sha1 = "pop";
         String sha256 = "lol";
         String md5 = "shmop";
-        List<String> requiredBy = Collections.singletonList("pitzi");
+        String[][] requiredBy = new String[][]{{"pitzi"}};
         Properties properties = new Properties();
 
         Dependency dependency = new DependencyBuilder().id(id).type(type).scopes(scopes).sha1(sha1).md5(md5).sha256(sha256)
@@ -82,18 +81,22 @@ public void testBuilderSetters() {
      * Validates the dependency values after using the builder add methods
      */
     public void testBuilderAddMethods() {
-        String requiredBy = "requiredMoo";
-        String propertyKey = "key";
-        String propertyValue = "value";
+        String propertyKey = "keyA";
+        String propertyValue = "valueA";
 
-        Dependency dependency = new DependencyBuilder().addRequiredBy(requiredBy).
-                addProperty(propertyKey, propertyValue).build();
-        List<String> requiredByList = dependency.getRequiredBy();
-        assertFalse(requiredByList.isEmpty(), "A dependency requirement should have been added.");
-        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,
-                "Unexpected dependency property value.");
+        Dependency dependency = new DependencyBuilder().addProperty(propertyKey, propertyValue).build();
+        assertTrue(dependency.getProperties().containsKey(propertyKey), "A dependency property should have been added.");
+        assertEquals(dependency.getProperties().get(propertyKey), propertyValue, "Unexpected dependency property value.");
+    }
+
+    public void testBuilderAddRequiredBy() {
+        String[] requiredByA = {"A", "to", "module", "root", "moduleID"};
+        String[] requiredByB = {"B", "C", "to", "module", "root", "moduleID"};
+
+        Dependency dependency = new DependencyBuilder().addRequiredBy(requiredByA).addRequiredBy(requiredByB).build();
+        String[][] requiredByList = dependency.getRequiredBy();
+        assertEquals(ArrayUtils.getLength(requiredByList), 2);
+        assertEquals(requiredByList[0], requiredByA, "Unexpected dependency requirement.");
+        assertEquals(requiredByList[1], requiredByB, "Unexpected dependency requirement.");
     }
 }