Merge pull request #12 from sse-labs/feature/performance-improve

Some performance and API improvements

pom.xml

@@ -5,8 +5,8 @@
     <modelVersion>4.0.0</modelVersion>
 
     <groupId>org.tudo.sse</groupId>
-    <artifactId>Maven-Central-Research-Interface</artifactId>
-    <version>1.0-SNAPSHOT</version>
+    <artifactId>marin</artifactId>
+    <version>1.0.0-SNAPSHOT</version>
 
     <properties>
         <maven.compiler.source>11</maven.compiler.source>

src/main/java/org/tudo/sse/model/Artifact.java

@@ -55,9 +55,6 @@ public Artifact(JarInformation jarInformation) {
         pomInformation = null;
     }
 
-    public IndexInformation getIndexInformation() {
-        return indexInformation;
-    }
 
     public ArtifactIdent getIdent() {
         return ident;
@@ -67,6 +64,14 @@ public ArtifactIdent getRelocation() {
         return relocation;
     }
 
+    public IndexInformation getIndexInformation() {
+        return indexInformation;
+    }
+
+    public boolean hasIndexInformation() {
+        return indexInformation != null;
+    }
+
     public void setIndexInformation(IndexInformation indexInformation) {
         this.indexInformation = indexInformation;
     }
@@ -75,6 +80,10 @@ public PomInformation getPomInformation() {
         return pomInformation;
     }
 
+    public boolean hasPomInformation() {
+        return pomInformation != null;
+    }
+
     public void setPomInformation(PomInformation pomInformation) {
         this.pomInformation = pomInformation;
     }
@@ -83,6 +92,10 @@ public JarInformation getJarInformation() {
         return jarInformation;
     }
 
+    public boolean hasJarInformation(){
+        return jarInformation != null;
+    }
+
     public void setJarInformation(JarInformation jarInformation) {
         this.jarInformation = jarInformation;
     }

src/main/java/org/tudo/sse/model/ArtifactIdent.java

@@ -21,6 +21,12 @@ public class ArtifactIdent {
      * The specific artifact identifier.
      */
     private String artifactID;
+
+    /**
+     * The GA tuple of this artifact
+     */
+    private String GA;
+
     /**
      * The version of the artifact.
      */
@@ -35,6 +41,7 @@ public class ArtifactIdent {
     public ArtifactIdent(String groupID, String artifactID, String version) {
         this.artifactID = artifactID;
         this.groupID = groupID;
+        this.GA = groupID + ":" + artifactID;
         this.version = version;
         this.repository = "https://repo1.maven.org/maven2/";
     }
@@ -44,6 +51,7 @@ public ArtifactIdent(ArtifactIdent toCopy) {
         this.artifactID = toCopy.artifactID;
         this.version = toCopy.version;
         this.repository = toCopy.repository;
+        this.GA = toCopy.getGA();
     }
 
     /**
@@ -60,6 +68,7 @@ public String getGroupID() {
      */
     public void setGroupID(String groupID) {
         this.groupID = groupID;
+        this.GA = this.groupID + ":" + this.artifactID;
     }
 
     /**
@@ -76,8 +85,16 @@ public String getArtifactID() {
      */
     public void setArtifactID(String artifactID) {
         this.artifactID = artifactID;
+        this.GA = this.groupID + ":" + this.artifactID;
     }
 
+    /**
+     * Gets GA tuple for this artifact.
+     * @return GA tuple separated by colon
+     */
+    public String getGA(){
+        return this.GA;
+    }
     /**
      * Gets the version.
      * @return version

src/main/java/org/tudo/sse/model/pom/Dependency.java

@@ -1,6 +1,8 @@
 package org.tudo.sse.model.pom;
 
 import java.util.List;
+import java.util.Set;
+
 import org.tudo.sse.model.ArtifactIdent;
 
 /**
@@ -9,7 +11,7 @@
 public class Dependency {
     private ArtifactIdent ident;
     private String scope;
-    private final List<String> exclusions;
+    private final Set<String> exclusions;
     private final boolean optional;
     private boolean isResolved;
     private boolean isVersionRange;
@@ -23,7 +25,7 @@ public class Dependency {
      * @param optional if the dependency can be left out of resolution
      * @param exclusions list of dependencies to exclude from transitive resolution
      */
-    public Dependency(ArtifactIdent ident, String scope, boolean isResolved, boolean isVersionRange, boolean optional, List<String> exclusions) {
+    public Dependency(ArtifactIdent ident, String scope, boolean isResolved, boolean isVersionRange, boolean optional, Set<String> exclusions) {
         this.ident = ident;
         this.scope = scope;
         this.isResolved = isResolved;
@@ -115,9 +117,9 @@ public boolean isOptional() {
 
     /**
      * Gets the list of dependencies to exclude from resolution
-     * @return a list of identifiers to exclude from transitive dependency resolution
+     * @return a set of G:A identifiers to exclude from transitive dependency resolution
      */
-    public List<String> getExclusions() {
+    public Set<String> getExclusions() {
         return exclusions;
     }
 }

src/main/java/org/tudo/sse/resolution/PomResolver.java

@@ -220,14 +220,14 @@ public List<Artifact> resolveArtifacts(List<ArtifactIdent> idents) {
     public Artifact resolveArtifact(ArtifactIdent identifier) throws FileNotFoundException, IOException, PomResolutionException {
         Map<ArtifactIdent, Artifact> alrEncountered = new HashMap<>();
         if(output) {
-            InputStream inputStream = MavenRepo.openPomFileInputStream(identifier);
+            try(InputStream inputStream = MavenRepo.openPomFileInputStream(identifier)){
+                byte[] pomBytes = inputStream.readAllBytes();
 
-            byte[] pomBytes = inputStream.readAllBytes();
-
-            Path filePath = pathToDirectory.resolve(identifier.getGroupID() + "-" + identifier.getArtifactID() + "-" + identifier.getVersion() + ".xml");
-            if(!Files.exists(filePath)) {
-                Files.createFile(filePath);
-                Files.write(filePath, pomBytes);
+                Path filePath = pathToDirectory.resolve(identifier.getGroupID() + "-" + identifier.getArtifactID() + "-" + identifier.getVersion() + ".xml");
+                if(!Files.exists(filePath)) {
+                    Files.createFile(filePath);
+                    Files.write(filePath, pomBytes);
+                }
             }
         }
 
@@ -346,14 +346,14 @@ public List<org.tudo.sse.model.pom.Dependency> getDependencies(List<Dependency>
 
             ArtifactIdent currentID = new ArtifactIdent(dependency.getGroupId(), dependency.getArtifactId(), dependency.getVersion());
             String scope = null;
-            List<String> exclusions = null;
+            Set<String> exclusions = null;
 
             if(dependency.getScope() != null) {
                 scope = dependency.getScope();
             }
 
             if(dependency.getExclusions() != null) {
-                exclusions = new ArrayList<>();
+                exclusions = new HashSet<>();
                 for(Exclusion exclusion : dependency.getExclusions()) {
                     exclusions.add(exclusion.getGroupId() + ":" + exclusion.getArtifactId());
                 }
@@ -797,19 +797,33 @@ private static boolean isVersionRange(String version) {
         return version.contains("[") || version.contains("]") || version.contains("(") || version.contains(")") || version.contains(",");
     }
 
+    /**
+     * This method resolves all the transitive dependencies of a given artifact. This is done recursively working with resolveTransitives and the recursiveHandler.
+     *
+     * @param toResolve the current artifact transitive dependencies are being resolved for.
+     * @throws IOException thrown when there's an issue opening the pom for an artifact
+     */
+    public void resolveAllTransitives(Artifact toResolve) throws IOException {
+        resolveAllTransitives(toResolve, new HashMap<>(), null);
+    }
+
     /**
      * This method resolves all the transitive dependencies of a given artifact. This is done recursively working with resolveTransitives and the recursiveHandler.
      *
      * @param current the current artifact transitive dependencies are being resolved for.
      * @param alrEncountered a map of dependencies that have already been resolved to avoid double resolutions
-     * @param exclusions a list of artifacts to not include in resolution
+     * @param exclusions a Set of G:A values to not include in resolution
      * @throws IOException thrown when there's an issue opening the pom for an artifact
      */
-    public void resolveAllTransitives(Artifact current, Map<ArtifactIdent, Artifact> alrEncountered, List<String> exclusions) throws IOException {
+    public void resolveAllTransitives(Artifact current, Map<ArtifactIdent, Artifact> alrEncountered, Set<String> exclusions) throws IOException {
         List<Artifact> transitives = new ArrayList<>();
         for (org.tudo.sse.model.pom.Dependency dependency : current.getPomInformation().getResolvedDependencies()) {
-            if(exclusions == null || !checkExclusions(dependency.getIdent().getGroupID() + ":" + dependency.getIdent().getArtifactID(), exclusions)) {
-                if ((dependency.getScope().equals("compile") || dependency.getScope().equals("runtime")) && !dependency.isOptional() && dependency.isResolved() && !alrEncountered.containsKey(dependency.getIdent())) {
+            if(exclusions == null || !exclusions.contains(dependency.getIdent().getGA())) {
+
+                boolean dependencyRelevant = (dependency.getScope().equals("compile") || dependency.getScope().equals("runtime")) &&
+                        !dependency.isOptional() && dependency.isResolved();
+
+                if (dependencyRelevant && !alrEncountered.containsKey(dependency.getIdent())) {
                     alrEncountered.put(dependency.getIdent(), null);
                     Artifact transitive = resolveTransitives(current, dependency, alrEncountered, dependency.getExclusions());
                     if (transitive != null) {
@@ -821,7 +835,7 @@ public void resolveAllTransitives(Artifact current, Map<ArtifactIdent, Artifact>
                         }
                         transitives.add(transitive);
                     }
-                } else if((dependency.getScope().equals("compile") || dependency.getScope().equals("runtime")) && !dependency.isOptional() && alrEncountered.get(dependency.getIdent()) != null) {
+                } else if(dependencyRelevant && alrEncountered.get(dependency.getIdent()) != null) {
                     transitives.add(alrEncountered.get(dependency.getIdent()));
                 }
             }
@@ -830,7 +844,7 @@ public void resolveAllTransitives(Artifact current, Map<ArtifactIdent, Artifact>
         current.getPomInformation().setAllTransitiveDependencies(transitives);
     }
 
-    private Artifact resolveTransitives(Artifact current, org.tudo.sse.model.pom.Dependency toResolve, Map<ArtifactIdent, Artifact> alrEncountered, List<String> exclusions) throws IOException {
+    private Artifact resolveTransitives(Artifact current, org.tudo.sse.model.pom.Dependency toResolve, Map<ArtifactIdent, Artifact> alrEncountered, Set<String> exclusions) throws IOException {
         try {
             return recursiveResolver(toResolve.getIdent(), alrEncountered, exclusions);
         } catch(FileNotFoundException | PomResolutionException e) {
@@ -842,23 +856,14 @@ private Artifact resolveTransitives(Artifact current, org.tudo.sse.model.pom.Dep
         }
     }
 
-    private Artifact recursiveResolver(ArtifactIdent identifier, Map<ArtifactIdent, Artifact> alrEncountered, List<String> exclusions) throws FileNotFoundException, IOException, PomResolutionException {
+    private Artifact recursiveResolver(ArtifactIdent identifier, Map<ArtifactIdent, Artifact> alrEncountered, Set<String> exclusions) throws FileNotFoundException, IOException, PomResolutionException {
         Artifact current = processArtifact(identifier);
         current.getPomInformation().setResolvedDependencies(resolveDependencies(current.getPomInformation()));
         resolveAllTransitives(current, alrEncountered, exclusions);
         return current;
     }
 
-    private boolean checkExclusions(String toCheck, List<String> exclusions) {
-        for(String exclusion : exclusions) {
-            if(exclusion.equals(toCheck)) {
-                return true;
-            }
-        }
-        return false;
-    }
-
-    private Artifact resolveFromSecondaryRepo(List<String> repos, org.tudo.sse.model.pom.Dependency toResolve, Map<ArtifactIdent, Artifact> alrEncountered, List<String> exclusions) {
+    private Artifact resolveFromSecondaryRepo(List<String> repos, org.tudo.sse.model.pom.Dependency toResolve, Map<ArtifactIdent, Artifact> alrEncountered, Set<String> exclusions) {
         int i = 0;
         Artifact toReturn = null;
         while(i < repos.size() && toReturn == null) {
@@ -879,35 +884,56 @@ private Artifact resolveFromSecondaryRepo(List<String> repos, org.tudo.sse.model
      * @param toResolve the artifact for which to resolve the effective transitive dependencies
      */
     public void resolveEffectiveTransitives(Artifact toResolve) {
+        // Can only resolve dependencies if POM info is present.
+        // IMPROVE: Do we want to load POM info on demand?
+        if(!toResolve.hasPomInformation()){
+            throw new IllegalStateException("Cannot resolve dependencies for " + toResolve.ident.getCoordinates() + " : No POM information loaded");
+        }
+
         List<Artifact> allTransitive = toResolve.getPomInformation().getAllTransitiveDependencies();
+
+        // If "normal" transitive dependencies have not been computed yet, we try to do that on demand
+        if(allTransitive == null){
+            try {
+                this.resolveAllTransitives(toResolve, new HashMap<>(), null);
+                allTransitive = toResolve.getPomInformation().getAllTransitiveDependencies();
+            } catch (IOException iox){
+                throw new IllegalStateException("Failed to compute transitive dependencies on demand", iox);
+            }
+        }
+
+        // If "normal" transitive dependencies are still null, we cannot do anything and abort
+        if(allTransitive == null){
+            throw new RuntimeException("Unable to compute transitive dependencies for " + toResolve.ident.getCoordinates());
+        }
+
+
         Map<String, ArtifactIdent> foundGA = new HashMap<>();
         Map<String, List<ArtifactIdent>> conflicts = new HashMap<>();
-        Queue<List<Artifact>> toProcess = new LinkedList<>();
         List<Artifact> toReturn = new ArrayList<>();
 
-        toProcess.add(allTransitive);
+        Queue<Artifact> toProcess = new LinkedList<>(allTransitive);
 
         while(!toProcess.isEmpty()) {
-            List<Artifact> currentList = toProcess.peek();
-            toProcess.remove();
-            for(Artifact artifact : currentList) {
-                String key = artifact.getIdent().getGroupID() + ":" + artifact.getIdent().getArtifactID();
-                if(!foundGA.containsKey(key)) {
-                    foundGA.put(key, artifact.getIdent());
-                    if(!artifact.getPomInformation().getAllTransitiveDependencies().isEmpty()) {
-                        toProcess.add(artifact.getPomInformation().getAllTransitiveDependencies());
-                    }
+            Artifact artifact = toProcess.poll();
+
+            String key = artifact.getIdent().getGroupID() + ":" + artifact.getIdent().getArtifactID();
+            if(!foundGA.containsKey(key)) {
+                foundGA.put(key, artifact.getIdent());
+                if(!artifact.getPomInformation().getAllTransitiveDependencies().isEmpty()) {
+                    toProcess.addAll(artifact.getPomInformation().getAllTransitiveDependencies());
+                }
+            } else {
+                if(conflicts.containsKey(key)) {
+                    conflicts.get(key).add(artifact.getIdent());
                 } else {
-                    if(conflicts.containsKey(key)) {
-                        conflicts.get(key).add(artifact.getIdent());
-                    } else {
-                        List<ArtifactIdent> temp = new ArrayList<>();
-                        temp.add(foundGA.get(key));
-                        temp.add(artifact.getIdent());
-                        conflicts.put(key, temp);
-                    }
+                    List<ArtifactIdent> temp = new ArrayList<>();
+                    temp.add(foundGA.get(key));
+                    temp.add(artifact.getIdent());
+                    conflicts.put(key, temp);
                 }
             }
+
         }
 
         for(Map.Entry<String,ArtifactIdent> entry : foundGA.entrySet()) {

src/test/java/org/tudo/sse/MavenCentralAnalysisTest.java

@@ -286,7 +286,7 @@ public void analyzeArtifact(Artifact current) {
         };
 
         jarUseCase.resolveJar = true;
-        assertDoesNotThrow( () -> jarUseCase.runAnalysis(new String[]{"-st", "0:1000"}));
+        assertDoesNotThrow( () -> jarUseCase.runAnalysis(new String[]{"-st", "0:300"}));
 
         MavenCentralAnalysis pomUseCase = new MavenCentralAnalysis() {
             public final Set<License> uniqueLicenses = new HashSet<>();
@@ -306,7 +306,7 @@ public void analyzeArtifact(Artifact toAnalyze) {
         };
 
         pomUseCase.resolvePom = true;
-        assertDoesNotThrow( () -> pomUseCase.runAnalysis(new String[]{"-st", "0:1000"}));
+        assertDoesNotThrow( () -> pomUseCase.runAnalysis(new String[]{"-st", "0:300"}));
 
         MavenCentralAnalysis indexUseCase = new MavenCentralAnalysis() {
             public final Set<Artifact> hasJavadocs = new HashSet<>();
@@ -325,7 +325,7 @@ public void analyzeArtifact(Artifact toAnalyze) {
         };
 
         indexUseCase.resolveIndex = true;
-        assertDoesNotThrow( () -> indexUseCase.runAnalysis(new String[]{"-st", "0:1000"}));
+        assertDoesNotThrow( () -> indexUseCase.runAnalysis(new String[]{"-st", "0:300"}));
     }
 
 }