Maven: Switch to the optimized dependency graph format

Rework the implementation to use DependencyGraphBuilder together with MavenDependencyHandler to produce the analyzer result. Adapt test classes as necessary. Rather than changing the expected test results, convert results to the classic format before they are compared with the expectations. This check is safer, as it makes sure that the content of the results has not changed, although they are now written in a different format. Add a helper function for this conversion to the test-utils module. Signed-off-by: Oliver Heger <oliver.heger@bosch.io>
oss-review-toolkit · May 14, 2021 · 1926422 · 1926422
1 parent 58fcd57
commit 1926422
Show file tree

Hide file tree

Showing 4 changed files with 47 additions and 93 deletions.
diff --git a/analyzer/src/funTest/kotlin/GitRepoFunTest.kt b/analyzer/src/funTest/kotlin/GitRepoFunTest.kt
@@ -66,7 +66,7 @@ class GitRepoFunTest : StringSpec() {
         // Disabled on Azure Windows because it fails for unknown reasons.
         "Analyzer correctly reports VcsInfo for git-repo projects".config(enabled = !Ci.isAzureWindows) {
             val ortResult = Analyzer(DEFAULT_ANALYZER_CONFIGURATION).analyze(outputDir)
-            val actualResult = ortResult.toYaml()
+            val actualResult = ortResult.withResolvedScopes().toYaml()
             val expectedResult = patchExpectedResult(
                 File("src/funTest/assets/projects/external/git-repo-expected-output.yml"),
                 revision = REPO_REV,

diff --git a/analyzer/src/funTest/kotlin/managers/MavenFunTest.kt b/analyzer/src/funTest/kotlin/managers/MavenFunTest.kt
@@ -49,7 +49,7 @@ class MavenFunTest : StringSpec() {
             val pomFile = projectDir.resolve("pom.xml")
             val expectedResult = projectDir.parentFile.resolve("jgnash-expected-output.yml").readText()
 
-            val result = createMaven().resolveSingleProject(pomFile)
+            val result = createMaven().resolveSingleProject(pomFile, resolveScopes = true)
 
             result.toYaml() shouldBe expectedResult
         }
@@ -65,12 +65,12 @@ class MavenFunTest : StringSpec() {
             // jgnash-core depends on jgnash-resources, so we also have to pass the pom.xml of jgnash-resources to
             // resolveDependencies so that it is available in the Maven.projectsByIdentifier cache. Otherwise resolution
             // of transitive dependencies would not work.
-            val result = createMaven().resolveDependencies(listOf(pomFileCore, pomFileResources))
-                .projectResults[pomFileCore]
+            val managerResult = createMaven().resolveDependencies(listOf(pomFileCore, pomFileResources))
+            val result = managerResult.projectResults[pomFileCore]
 
             result.shouldNotBeNull()
             result should haveSize(1)
-            result.single().toYaml() shouldBe expectedResult
+            managerResult.resolveScopes(result.single()).toYaml() shouldBe expectedResult
         }
 
         "Root project dependencies are detected correctly" {
@@ -81,7 +81,7 @@ class MavenFunTest : StringSpec() {
                 revision = vcsRevision
             )
 
-            val result = createMaven().resolveSingleProject(pomFile)
+            val result = createMaven().resolveSingleProject(pomFile, resolveScopes = true)
 
             result.toYaml() shouldBe expectedResult
         }
@@ -98,11 +98,12 @@ class MavenFunTest : StringSpec() {
             // app depends on lib, so we also have to pass the pom.xml of lib to resolveDependencies so that it is
             // available in the Maven.projectsByIdentifier cache. Otherwise resolution of transitive dependencies would
             // not work.
-            val result = createMaven().resolveDependencies(listOf(pomFileApp, pomFileLib)).projectResults[pomFileApp]
+            val managerResult = createMaven().resolveDependencies(listOf(pomFileApp, pomFileLib))
+            val result = managerResult.projectResults[pomFileApp]
 
             result.shouldNotBeNull()
             result should haveSize(1)
-            result.single().toYaml() shouldBe expectedResult
+            managerResult.resolveScopes(result.single()).toYaml() shouldBe expectedResult
         }
 
         "External dependencies are detected correctly" {
@@ -113,7 +114,7 @@ class MavenFunTest : StringSpec() {
                 revision = vcsRevision
             )
 
-            val result = createMaven().resolveSingleProject(pomFile)
+            val result = createMaven().resolveSingleProject(pomFile, resolveScopes = true)
 
             result.toYaml() shouldBe expectedResult
         }
@@ -132,7 +133,7 @@ class MavenFunTest : StringSpec() {
                 revision = vcsRevision
             )
 
-            val result = createMaven().resolveSingleProject(pomFile)
+            val result = createMaven().resolveSingleProject(pomFile, resolveScopes = true)
 
             result.toYaml() shouldBe expectedResult
         }
@@ -146,7 +147,7 @@ class MavenFunTest : StringSpec() {
                 revision = vcsRevision
             )
 
-            val result = createMaven().resolveSingleProject(pomFile)
+            val result = createMaven().resolveSingleProject(pomFile, resolveScopes = true)
 
             patchActualResult(result.toYaml(), patchStartAndEndTime = true) shouldBe expectedResult
         }

diff --git a/analyzer/src/funTest/kotlin/managers/SbtFunTest.kt b/analyzer/src/funTest/kotlin/managers/SbtFunTest.kt
@@ -46,7 +46,7 @@ class SbtFunTest : StringSpec({
 
         val ortResult = Analyzer(DEFAULT_ANALYZER_CONFIGURATION).analyze(projectDir, listOf(Sbt.Factory()))
 
-        val actualResult = ortResult.toYaml()
+        val actualResult = ortResult.withResolvedScopes().toYaml()
         val expectedResult = patchExpectedResult(expectedOutputFile)
 
         patchActualResult(actualResult, patchStartAndEndTime = true) shouldBe expectedResult
@@ -64,7 +64,7 @@ class SbtFunTest : StringSpec({
 
         val ortResult = Analyzer(DEFAULT_ANALYZER_CONFIGURATION).analyze(projectDir, listOf(Sbt.Factory()))
 
-        val actualResult = ortResult.toYaml()
+        val actualResult = ortResult.withResolvedScopes().toYaml()
         val expectedResult = patchExpectedResult(expectedOutputFile)
 
         patchActualResult(actualResult, patchStartAndEndTime = true) shouldBe expectedResult
@@ -85,7 +85,7 @@ class SbtFunTest : StringSpec({
 
         val ortResult = Analyzer(DEFAULT_ANALYZER_CONFIGURATION).analyze(projectDir, listOf(Sbt.Factory()))
 
-        val actualResult = ortResult.toYaml()
+        val actualResult = ortResult.withResolvedScopes().toYaml()
         val expectedResult = patchExpectedResult(
             expectedOutputFile,
             url = vcsUrl,

diff --git a/analyzer/src/main/kotlin/managers/Maven.kt b/analyzer/src/main/kotlin/managers/Maven.kt
@@ -21,7 +21,6 @@ package org.ossreviewtoolkit.analyzer.managers
 
 import java.io.File
 
-import org.apache.maven.project.ProjectBuildingException
 import org.apache.maven.project.ProjectBuildingResult
 
 import org.eclipse.aether.artifact.Artifact
@@ -31,24 +30,20 @@ import org.eclipse.aether.repository.WorkspaceRepository
 
 import org.ossreviewtoolkit.analyzer.AbstractPackageManagerFactory
 import org.ossreviewtoolkit.analyzer.PackageManager
+import org.ossreviewtoolkit.analyzer.PackageManagerResult
+import org.ossreviewtoolkit.analyzer.managers.utils.DependencyGraphBuilder
 import org.ossreviewtoolkit.analyzer.managers.utils.MavenSupport
 import org.ossreviewtoolkit.analyzer.managers.utils.identifier
 import org.ossreviewtoolkit.downloader.VersionControlSystem
+import org.ossreviewtoolkit.model.DependencyGraph
 import org.ossreviewtoolkit.model.Identifier
-import org.ossreviewtoolkit.model.Package
-import org.ossreviewtoolkit.model.PackageLinkage
-import org.ossreviewtoolkit.model.PackageReference
 import org.ossreviewtoolkit.model.Project
 import org.ossreviewtoolkit.model.ProjectAnalyzerResult
-import org.ossreviewtoolkit.model.Scope
 import org.ossreviewtoolkit.model.Severity
 import org.ossreviewtoolkit.model.config.AnalyzerConfiguration
 import org.ossreviewtoolkit.model.config.RepositoryConfiguration
 import org.ossreviewtoolkit.model.createAndLogIssue
-import org.ossreviewtoolkit.utils.collectMessagesAsString
-import org.ossreviewtoolkit.utils.log
 import org.ossreviewtoolkit.utils.searchUpwardsForSubdirectory
-import org.ossreviewtoolkit.utils.showStackTrace
 
 /**
  * The [Maven](https://maven.apache.org/) package manager for Java.
@@ -88,6 +83,9 @@ class Maven(
 
     private val localProjectBuildingResults = mutableMapOf<String, ProjectBuildingResult>()
 
+    /** The builder for the shared dependency graph. */
+    private lateinit var graphBuilder: DependencyGraphBuilder<DependencyNode>
+
     private var sbtMode = false
 
     /**
@@ -97,22 +95,29 @@ class Maven(
 
     override fun beforeResolution(definitionFiles: List<File>) {
         localProjectBuildingResults += mvn.prepareMavenProjects(definitionFiles)
+
+        val localProjects = localProjectBuildingResults.mapValues { it.value.project }
+        val dependencyHandler = MavenDependencyHandler(managerName, mvn, localProjects, sbtMode)
+        graphBuilder = DependencyGraphBuilder(dependencyHandler)
     }
 
+    override fun createPackageManagerResult(projectResults: Map<File, List<ProjectAnalyzerResult>>):
+            PackageManagerResult =
+        PackageManagerResult(projectResults, graphBuilder.build(), graphBuilder.packages().toSortedSet())
+
     override fun resolveDependencies(definitionFile: File): List<ProjectAnalyzerResult> {
         val workingDir = definitionFile.parentFile
         val projectBuildingResult = mvn.buildMavenProject(definitionFile)
         val mavenProject = projectBuildingResult.project
-        val packagesById = mutableMapOf<String, Package>()
-        val scopesByName = mutableMapOf<String, Scope>()
-
-        projectBuildingResult.dependencyResolutionResult.dependencyGraph.children.forEach { node ->
-            val scopeName = node.dependency.scope
-            val scope = scopesByName.getOrPut(scopeName) {
-                Scope(scopeName, sortedSetOf())
-            }
+        val projectId = Identifier(
+            type = managerName,
+            namespace = mavenProject.groupId,
+            name = mavenProject.artifactId,
+            version = mavenProject.version
+        )
 
-            scope.dependencies += parseDependency(node, packagesById)
+        projectBuildingResult.dependencies().forEach { node ->
+            graphBuilder.addDependency(DependencyGraph.qualifyScope(projectId, node.dependency.scope), node)
         }
 
         val declaredLicenses = MavenSupport.parseLicenses(mavenProject)
@@ -133,23 +138,18 @@ class Maven(
         val vcsFallbackUrls = listOfNotNull(browsableScmUrl, homepageUrl).toTypedArray()
 
         val project = Project(
-            id = Identifier(
-                type = managerName,
-                namespace = mavenProject.groupId,
-                name = mavenProject.artifactId,
-                version = mavenProject.version
-            ),
+            id = projectId,
             definitionFilePath = VersionControlSystem.getPathInfo(definitionFile).path,
             authors = MavenSupport.parseAuthors(mavenProject),
             declaredLicenses = declaredLicenses,
             declaredLicensesProcessed = declaredLicensesProcessed,
             vcs = vcsFromPackage,
             vcsProcessed = processProjectVcs(projectDir, vcsFromPackage, *vcsFallbackUrls),
             homepageUrl = homepageUrl.orEmpty(),
-            scopeDependencies = scopesByName.values.toSortedSet()
+            scopeNames = projectBuildingResult.dependencies().mapTo(sortedSetOf()) { it.dependency.scope }
         )
 
-        val packages = packagesById.values.toSortedSet()
+        val packages = graphBuilder.packages().toSortedSet()
         val issues = packages.mapNotNull { pkg ->
             if (pkg.description == "POM was created by Sonatype Nexus") {
                 createAndLogIssue(
@@ -162,59 +162,12 @@ class Maven(
             }
         }
 
-        return listOf(ProjectAnalyzerResult(project, packages, issues))
-    }
-
-    private fun parseDependency(node: DependencyNode, packages: MutableMap<String, Package>): PackageReference {
-        val identifier = node.artifact.identifier()
-
-        try {
-            val dependencies = node.children.mapNotNull { child ->
-                val toolsJarCoordinates = listOf("com.sun:tools:", "jdk.tools:jdk.tools:")
-                if (toolsJarCoordinates.any { child.artifact.identifier().startsWith(it) }) {
-                    log.info { "Omitting the Java < 1.9 system dependency on 'tools.jar'." }
-                    null
-                } else {
-                    parseDependency(child, packages)
-                }
-            }.toSortedSet()
-
-            val localProjects = localProjectBuildingResults.mapValues { it.value.project }
-
-            return if (localProjects.contains(identifier)) {
-                val id = Identifier(
-                    type = managerName,
-                    namespace = node.artifact.groupId,
-                    name = node.artifact.artifactId,
-                    version = node.artifact.version
-                )
-
-                log.info { "Dependency '${id.toCoordinates()}' refers to a local project." }
-
-                PackageReference(id, PackageLinkage.PROJECT_DYNAMIC, dependencies)
-            } else {
-                val pkg = packages.getOrPut(identifier) {
-                    // TODO: Omit the "localProjects" argument here once SBT is implemented independently of Maven as at
-                    //       this point we know already that "identifier" is not a local project.
-                    mvn.parsePackage(node.artifact, node.repositories, localProjects, sbtMode)
-                }
-
-                pkg.toReference(dependencies = dependencies)
-            }
-        } catch (e: ProjectBuildingException) {
-            e.showStackTrace()
-
-            return PackageReference(
-                Identifier(managerName, node.artifact.groupId, node.artifact.artifactId, node.artifact.version),
-                dependencies = sortedSetOf(),
-                issues = listOf(
-                    createAndLogIssue(
-                        source = managerName,
-                        message = "Could not get package information for dependency '$identifier': " +
-                                e.collectMessagesAsString()
-                    )
-                )
-            )
-        }
+        return listOf(ProjectAnalyzerResult(project, sortedSetOf(), issues))
     }
 }
+
+/**
+ * Convenience function to obtain all the [DependencyNode]s from this [ProjectBuildingResult].
+ */
+private fun ProjectBuildingResult.dependencies(): List<DependencyNode> =
+    dependencyResolutionResult.dependencyGraph.children