Optimize artifact lookup in DefaultProjectDependencyAnalyzer

src/main/java/org/apache/maven/shared/dependency/analyzer/DefaultProjectDependencyAnalyzer.java

@@ -69,6 +69,7 @@ public ProjectDependencyAnalysis analyze(MavenProject project, Collection<String
         try {
             ClassesPatterns excludedClassesPatterns = new ClassesPatterns(excludedClasses);
             Map<Artifact, Set<String>> artifactClassMap = buildArtifactClassMap(project, excludedClassesPatterns);
+            Map<String, Artifact> classToArtifactMap = buildClassToArtifactMap(artifactClassMap);
 
             Set<DependencyUsage> mainDependencyClasses = new HashSet<>();
             for (MainDependencyClassesProvider provider : mainDependencyClassesProviders) {
@@ -87,11 +88,12 @@ public ProjectDependencyAnalysis analyze(MavenProject project, Collection<String
             Set<DependencyUsage> testOnlyDependencyClasses =
                     buildTestOnlyDependencyClasses(mainDependencyClasses, testDependencyClasses);
 
-            Map<Artifact, Set<DependencyUsage>> usedArtifacts = buildUsedArtifacts(artifactClassMap, dependencyClasses);
-            Set<Artifact> mainUsedArtifacts =
-                    buildUsedArtifacts(artifactClassMap, mainDependencyClasses).keySet();
+            Map<Artifact, Set<DependencyUsage>> usedArtifacts =
+                    buildUsedArtifacts(classToArtifactMap, dependencyClasses);
+            Set<Artifact> mainUsedArtifacts = buildUsedArtifacts(classToArtifactMap, mainDependencyClasses)
+                    .keySet();
 
-            Set<Artifact> testArtifacts = buildUsedArtifacts(artifactClassMap, testOnlyDependencyClasses)
+            Set<Artifact> testArtifacts = buildUsedArtifacts(classToArtifactMap, testOnlyDependencyClasses)
                     .keySet();
             Set<Artifact> testOnlyArtifacts = removeAll(testArtifacts, mainUsedArtifacts);
 
@@ -124,7 +126,8 @@ public ProjectDependencyAnalysis analyze(MavenProject project, Collection<String
     }
 
     /**
-     * This method defines a new way to remove the artifacts by using the conflict id. We don't care about the version
+     * This method defines a new way to remove the artifacts by using the conflict
+     * id. We don't care about the version
      * here because there can be only 1 for a given artifact anyway.
      *
      * @param start  initial set
@@ -156,7 +159,7 @@ private static Set<Artifact> getTestArtifactsWithNonTestScope(Set<Artifact> test
         Set<Artifact> nonTestScopeArtifacts = new LinkedHashSet<>();
 
         for (Artifact artifact : testOnlyArtifacts) {
-            if (artifact.getScope().equals("compile")) {
+            if (Artifact.SCOPE_COMPILE.equals(artifact.getScope())) {
                 nonTestScopeArtifacts.add(artifact);
             }
         }
@@ -225,20 +228,15 @@ private static Set<Artifact> buildDeclaredArtifacts(MavenProject project) {
         return declaredArtifacts;
     }
 
-    private static Map<Artifact, Set<DependencyUsage>> buildUsedArtifacts(
-            Map<Artifact, Set<String>> artifactClassMap, Set<DependencyUsage> dependencyClasses) {
+    static Map<Artifact, Set<DependencyUsage>> buildUsedArtifacts(
+            Map<String, Artifact> classToArtifactMap, Set<DependencyUsage> dependencyClasses) {
         Map<Artifact, Set<DependencyUsage>> usedArtifacts = new HashMap<>();
 
         for (DependencyUsage classUsage : dependencyClasses) {
-            Artifact artifact = findArtifactForClassName(artifactClassMap, classUsage.getDependencyClass());
+            Artifact artifact = classToArtifactMap.get(classUsage.getDependencyClass());
 
             if (artifact != null && !includedInJDK(artifact)) {
-                Set<DependencyUsage> classesFromArtifact = usedArtifacts.get(artifact);
-                if (classesFromArtifact == null) {
-                    classesFromArtifact = new HashSet<>();
-                    usedArtifacts.put(artifact, classesFromArtifact);
-                }
-                classesFromArtifact.add(classUsage);
+                usedArtifacts.computeIfAbsent(artifact, k -> new HashSet<>()).add(classUsage);
             }
         }
 
@@ -247,7 +245,7 @@ private static Map<Artifact, Set<DependencyUsage>> buildUsedArtifacts(
 
     // MSHARED-47 an uncommon case where a commonly used
     // third party dependency was added to the JDK
-    private static boolean includedInJDK(Artifact artifact) {
+    static boolean includedInJDK(Artifact artifact) {
         if ("xml-apis".equals(artifact.getGroupId())) {
             if ("xml-apis".equals(artifact.getArtifactId())) {
                 return true;
@@ -260,13 +258,16 @@ private static boolean includedInJDK(Artifact artifact) {
         return false;
     }
 
-    private static Artifact findArtifactForClassName(Map<Artifact, Set<String>> artifactClassMap, String className) {
+    static Map<String, Artifact> buildClassToArtifactMap(Map<Artifact, Set<String>> artifactClassMap) {
+        Map<String, Artifact> classToArtifactMap = new HashMap<>();
+
         for (Map.Entry<Artifact, Set<String>> entry : artifactClassMap.entrySet()) {
-            if (entry.getValue().contains(className)) {
-                return entry.getKey();
+            Artifact artifact = entry.getKey();
+            for (String className : entry.getValue()) {
+                classToArtifactMap.putIfAbsent(className, artifact);
             }
         }
 
-        return null;
+        return classToArtifactMap;
     }
 }

src/test/java/org/apache/maven/shared/dependency/analyzer/DefaultProjectDependencyAnalyzerTest.java

@@ -0,0 +1,143 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.shared.dependency.analyzer;
+
+import java.util.Collections;
+import java.util.LinkedHashMap;
+import java.util.Map;
+import java.util.Set;
+import java.util.HashSet;
+import java.util.Arrays;
+
+import org.apache.maven.artifact.Artifact;
+import org.apache.maven.artifact.DefaultArtifact;
+import org.apache.maven.artifact.versioning.VersionRange;
+import org.junit.jupiter.api.Test;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+/**
+ * Tests <code>DefaultProjectDependencyAnalyzer</code>.
+ *
+ * @see DefaultProjectDependencyAnalyzer
+ */
+class DefaultProjectDependencyAnalyzerTest {
+
+    @Test
+    void testBuildClassToArtifactMap() {
+        Artifact artifact1 = aTestArtifact("artifact1");
+        Artifact artifact2 = aTestArtifact("artifact2");
+
+        Map<Artifact, Set<String>> artifactClassMap = new LinkedHashMap<>();
+        artifactClassMap.put(artifact1, Collections.singleton("class1"));
+        artifactClassMap.put(artifact2, Collections.singleton("class2"));
+
+        Map<String, Artifact> result = DefaultProjectDependencyAnalyzer.buildClassToArtifactMap(artifactClassMap);
+
+        assertThat(result).hasSize(2);
+        assertThat(result.get("class1")).isEqualTo(artifact1);
+        assertThat(result.get("class2")).isEqualTo(artifact2);
+    }
+
+    @Test
+    void testBuildClassToArtifactMapWithDuplicates() {
+        Artifact artifact1 = aTestArtifact("artifact1");
+        Artifact artifact2 = aTestArtifact("artifact2");
+
+        Map<Artifact, Set<String>> artifactClassMap = new LinkedHashMap<>();
+        artifactClassMap.put(artifact1, Collections.singleton("duplicateClass"));
+        artifactClassMap.put(artifact2, Collections.singleton("duplicateClass"));
+
+        Map<String, Artifact> result = DefaultProjectDependencyAnalyzer.buildClassToArtifactMap(artifactClassMap);
+
+        assertThat(result).hasSize(1);
+        // Should favor the first artifact encountered
+        assertThat(result.get("duplicateClass")).isEqualTo(artifact1);
+    }
+
+    @Test
+    void testBuildClassToArtifactMapWithMultipleClasses() {
+        Artifact artifact1 = aTestArtifact("artifact1");
+
+        Map<Artifact, Set<String>> artifactClassMap = new LinkedHashMap<>();
+        artifactClassMap.put(artifact1, new HashSet<>(Arrays.asList("class1", "class2")));
+
+        Map<String, Artifact> result = DefaultProjectDependencyAnalyzer.buildClassToArtifactMap(artifactClassMap);
+
+        assertThat(result).hasSize(2);
+        assertThat(result.get("class1")).isEqualTo(artifact1);
+        assertThat(result.get("class2")).isEqualTo(artifact1);
+    }
+
+    @Test
+    void testBuildUsedArtifacts() {
+        Artifact artifact1 = aTestArtifact("artifact1");
+        Map<String, Artifact> classToArtifactMap = Collections.singletonMap("class1", artifact1);
+        Set<DependencyUsage> dependencyClasses = Collections.singleton(new DependencyUsage("class1", "main"));
+
+        Map<Artifact, Set<DependencyUsage>> result = DefaultProjectDependencyAnalyzer.buildUsedArtifacts(classToArtifactMap, dependencyClasses);
+
+        assertThat(result).hasSize(1);
+        assertThat(result.get(artifact1)).hasSize(1);
+        assertThat(result.get(artifact1).iterator().next().getDependencyClass()).isEqualTo("class1");
+    }
+
+    @Test
+    void testBuildUsedArtifactsWithMultipleClasses() {
+        Artifact artifact1 = aTestArtifact("artifact1");
+        Map<String, Artifact> classToArtifactMap = Collections.singletonMap("class1", artifact1);
+        Set<DependencyUsage> dependencyClasses = new HashSet<>(Arrays.asList(
+                new DependencyUsage("class1", "main"),
+                new DependencyUsage("class1", "test")
+        ));
+
+        Map<Artifact, Set<DependencyUsage>> result = DefaultProjectDependencyAnalyzer.buildUsedArtifacts(classToArtifactMap, dependencyClasses);
+
+        assertThat(result).hasSize(1);
+        assertThat(result.get(artifact1)).hasSize(2);
+    }
+
+    @Test
+    void testBuildUsedArtifactsWithJDKExcluded() {
+        Artifact artifact1 = aTestArtifact("xml-apis", "xml-apis");
+        Map<String, Artifact> classToArtifactMap = Collections.singletonMap("class1", artifact1);
+        Set<DependencyUsage> dependencyClasses = Collections.singleton(new DependencyUsage("class1", "main"));
+
+        Map<Artifact, Set<DependencyUsage>> result = DefaultProjectDependencyAnalyzer.buildUsedArtifacts(classToArtifactMap, dependencyClasses);
+
+        // Being in JDK, it should be excluded from used artifacts
+        assertThat(result).isEmpty();
+    }
+
+    @Test
+    void testIncludedInJDK() {
+        assertThat(DefaultProjectDependencyAnalyzer.includedInJDK(aTestArtifact("xml-apis", "xml-apis"))).isTrue();
+        assertThat(DefaultProjectDependencyAnalyzer.includedInJDK(aTestArtifact("xerces", "xmlParserAPIs"))).isTrue();
+        assertThat(DefaultProjectDependencyAnalyzer.includedInJDK(aTestArtifact("groupId", "artifactId"))).isFalse();
+    }
+
+    private Artifact aTestArtifact(String artifactId) {
+        return aTestArtifact("groupId", artifactId);
+    }
+
+    private Artifact aTestArtifact(String groupId, String artifactId) {
+        return new DefaultArtifact(
+                groupId, artifactId, VersionRange.createFromVersion("1.0"), "compile", "jar", "", null);
+    }
+}