refactor(Scope/PackageReference): Do not use sorted sets for pkg refs

Only sort on serialization for human readability and reproducibility. Signed-off-by: Frank Viernau <[email protected]>
oss-review-toolkit · May 17, 2023 · 3363a22 · 3363a22
1 parent 7807e14
commit 3363a22
Show file tree

Hide file tree

Showing 33 changed files with 110 additions and 116 deletions.
diff --git a/analyzer/src/main/kotlin/managers/GoDep.kt b/analyzer/src/main/kotlin/managers/GoDep.kt
@@ -156,7 +156,7 @@ class GoDep(
             packageRefs += pkg.toReference(linkage = PackageLinkage.STATIC, issues = issues)
         }
 
-        val scope = Scope("default", packageRefs.toSortedSet())
+        val scope = Scope("default", packageRefs)
 
         val projectName = runCatching {
             val uri = URI(projectVcs.url)

diff --git a/analyzer/src/main/kotlin/managers/utils/Graph.kt b/analyzer/src/main/kotlin/managers/utils/Graph.kt
@@ -20,7 +20,6 @@
 package org.ossreviewtoolkit.analyzer.managers.utils
 
 import java.util.LinkedList
-import java.util.SortedSet
 
 import org.apache.logging.log4j.kotlin.Logging
 
@@ -122,9 +121,9 @@ internal class Graph private constructor(private val nodeMap: MutableMap<Identif
      * Convert this [Graph] to a set of [PackageReference]s that spawn the dependency trees of the direct dependencies
      * of the given [root] package. The graph must not contain any cycles, so [breakCycles] should be called before.
      */
-    fun toPackageReferenceForest(root: Identifier): SortedSet<PackageReference> {
+    fun toPackageReferenceForest(root: Identifier): Set<PackageReference> {
         fun getPackageReference(id: Identifier): PackageReference {
-            val dependencies = nodeMap.getValue(id).mapTo(sortedSetOf()) {
+            val dependencies = nodeMap.getValue(id).mapTo(mutableSetOf()) {
                 getPackageReference(it)
             }
 
@@ -135,7 +134,7 @@ internal class Graph private constructor(private val nodeMap: MutableMap<Identif
             )
         }
 
-        return dependencies(root).mapTo(sortedSetOf()) { getPackageReference(it) }
+        return dependencies(root).mapTo(mutableSetOf()) { getPackageReference(it) }
     }
 
     /**

diff --git a/analyzer/src/test/kotlin/AnalyzerResultBuilderTest.kt b/analyzer/src/test/kotlin/AnalyzerResultBuilderTest.kt
@@ -66,11 +66,11 @@ class AnalyzerResultBuilderTest : WordSpec() {
 
     private val pkgRef1 = package1.toReference(issues = listOf(issue1))
     private val pkgRef2 = package2.toReference(
-        dependencies = sortedSetOf(package3.toReference(issues = listOf(issue2)))
+        dependencies = setOf(package3.toReference(issues = listOf(issue2)))
     )
 
-    private val scope1 = Scope("scope-1", sortedSetOf(pkgRef1))
-    private val scope2 = Scope("scope-2", sortedSetOf(pkgRef2))
+    private val scope1 = Scope("scope-1", setOf(pkgRef1))
+    private val scope2 = Scope("scope-2", setOf(pkgRef2))
 
     private val project1 = Project.EMPTY.copy(
         id = Identifier("type-1", "namespace-1", "project-1", "version-1"),
@@ -339,11 +339,11 @@ class AnalyzerResultBuilderTest : WordSpec() {
 
                 val scope = Scope(
                     name = "scope",
-                    dependencies = sortedSetOf(
+                    dependencies = setOf(
                         packageManagerDependency,
                         PackageReference(
                             id = pkgRef1.id,
-                            dependencies = sortedSetOf(packageManagerDependency)
+                            dependencies = setOf(packageManagerDependency)
                         )
                     )
                 )
@@ -370,19 +370,19 @@ class AnalyzerResultBuilderTest : WordSpec() {
                         project.scopes shouldContainExactly sortedSetOf(
                             Scope(
                                 name = "scope",
-                                dependencies = sortedSetOf(
+                                dependencies = setOf(
                                     PackageReference(
                                         id = project1.id,
-                                        dependencies = sortedSetOf(
+                                        dependencies = setOf(
                                             PackageReference(id = package1.id)
                                         )
                                     ),
                                     PackageReference(
                                         id = package1.id,
-                                        dependencies = sortedSetOf(
+                                        dependencies = setOf(
                                             PackageReference(
                                                 id = project1.id,
-                                                dependencies = sortedSetOf(
+                                                dependencies = setOf(
                                                     PackageReference(id = package1.id)
                                                 )
                                             )

diff --git a/evaluator/src/test/kotlin/TestData.kt b/evaluator/src/test/kotlin/TestData.kt
@@ -137,7 +137,7 @@ val allPackages = setOf(
 
 val scopeExcluded = Scope(
     name = "compile",
-    dependencies = sortedSetOf(
+    dependencies = setOf(
         packageExcluded.toReference()
     )
 )
@@ -153,11 +153,11 @@ val packageRefStaticallyLinked = packageStaticallyLinked.toReference(PackageLink
 
 val scopeIncluded = Scope(
     name = "compile",
-    dependencies = sortedSetOf(
+    dependencies = setOf(
         packageWithoutLicense.toReference(),
         packageWithNotPresentLicense.toReference(),
         packageWithOnlyConcludedLicense.toReference(),
-        packageWithOnlyDeclaredLicense.toReference(dependencies = sortedSetOf(packageDependency.toReference())),
+        packageWithOnlyDeclaredLicense.toReference(dependencies = setOf(packageDependency.toReference())),
         packageWithConcludedAndDeclaredLicense.toReference(),
         packageRefDynamicallyLinked,
         packageRefStaticallyLinked,

diff --git a/model/src/main/kotlin/DependencyGraph.kt b/model/src/main/kotlin/DependencyGraph.kt
@@ -170,7 +170,7 @@ data class DependencyGraph(
      */
     private fun createScopesFor(map: Map<String, List<RootDependencyIndex>>, unqualify: Boolean): Set<Scope> =
         map.mapTo(mutableSetOf()) { entry ->
-            val dependencies = entry.value.mapTo(sortedSetOf()) { index ->
+            val dependencies = entry.value.mapTo(mutableSetOf()) { index ->
                 referenceMapping[index.toKey()] ?: error("Could not resolve dependency index $index.")
             }
 
@@ -201,7 +201,7 @@ data class DependencyGraph(
     ): PackageReference {
         val indexKey = RootDependencyIndex.generateKey(node.pkg, node.fragment)
         return refMapping.getOrPut(indexKey) {
-            val refDependencies = dependencies[node].orEmpty().mapTo(sortedSetOf()) {
+            val refDependencies = dependencies[node].orEmpty().mapTo(mutableSetOf()) {
                 constructReferenceTree(it, refMapping)
             }
 

diff --git a/model/src/main/kotlin/Package.kt b/model/src/main/kotlin/Package.kt
@@ -22,8 +22,6 @@ package org.ossreviewtoolkit.model
 import com.fasterxml.jackson.annotation.JsonInclude
 import com.fasterxml.jackson.databind.annotation.JsonSerialize
 
-import java.util.SortedSet
-
 import org.ossreviewtoolkit.model.utils.toPurl
 import org.ossreviewtoolkit.utils.common.StringSortedSetConverter
 import org.ossreviewtoolkit.utils.ort.DeclaredLicenseProcessor
@@ -186,7 +184,7 @@ data class Package(
      */
     fun toReference(
         linkage: PackageLinkage? = null,
-        dependencies: SortedSet<PackageReference>? = null,
+        dependencies: Set<PackageReference>? = null,
         issues: List<Issue>? = null
     ): PackageReference {
         var ref = PackageReference(id)

diff --git a/model/src/main/kotlin/PackageReference.kt b/model/src/main/kotlin/PackageReference.kt
@@ -20,12 +20,13 @@
 package org.ossreviewtoolkit.model
 
 import com.fasterxml.jackson.annotation.JsonInclude
+import com.fasterxml.jackson.databind.annotation.JsonSerialize
 
 import java.util.Deque
 import java.util.LinkedList
-import java.util.SortedSet
 
 import org.ossreviewtoolkit.model.utils.PackageLinkageValueFilter
+import org.ossreviewtoolkit.model.utils.PackageReferenceSortedSetConverter
 
 /**
  * A human-readable reference to a software [Package]. Each package reference itself refers to other package
@@ -51,7 +52,8 @@ data class PackageReference(
      * The set of [references to packages][PackageReference] this package depends on. Note that this list depends on the
      * [scope][Scope] in which this package is referenced.
      */
-    val dependencies: SortedSet<PackageReference> = sortedSetOf(),
+    @JsonSerialize(converter = PackageReferenceSortedSetConverter::class)
+    val dependencies: Set<PackageReference> = emptySet(),
 
     /**
      * A list of [Issue]s that occurred handling this [PackageReference].

diff --git a/model/src/main/kotlin/Scope.kt b/model/src/main/kotlin/Scope.kt
@@ -19,7 +19,9 @@
 
 package org.ossreviewtoolkit.model
 
-import java.util.SortedSet
+import com.fasterxml.jackson.databind.annotation.JsonSerialize
+
+import org.ossreviewtoolkit.model.utils.PackageReferenceSortedSetConverter
 
 /**
  * The scope class puts package dependencies into context.
@@ -39,7 +41,8 @@ data class Scope(
      * dependencies would not be test dependencies of the test dependencies, but compile dependencies of test
      * dependencies.
      */
-    val dependencies: SortedSet<PackageReference> = sortedSetOf()
+    @JsonSerialize(converter = PackageReferenceSortedSetConverter::class)
+    val dependencies: Set<PackageReference> = emptySet()
 ) : Comparable<Scope> {
     /**
      * Return the set of package [Identifier]s in this [Scope], up to and including a depth of [maxDepth] where counting

diff --git a/model/src/main/kotlin/utils/SortedSetConverters.kt b/model/src/main/kotlin/utils/SortedSetConverters.kt
@@ -28,6 +28,7 @@ import java.util.SortedSet
 import org.ossreviewtoolkit.model.CopyrightFinding
 import org.ossreviewtoolkit.model.LicenseFinding
 import org.ossreviewtoolkit.model.Package
+import org.ossreviewtoolkit.model.PackageReference
 import org.ossreviewtoolkit.model.Project
 import org.ossreviewtoolkit.model.Scope
 
@@ -39,6 +40,10 @@ class LicenseFindingSortedSetConverter : StdConverter<Set<LicenseFinding>, Sorte
     override fun convert(value: Set<LicenseFinding>) = value.toSortedSet(LicenseFinding.COMPARATOR)
 }
 
+class PackageReferenceSortedSetConverter : StdConverter<Set<PackageReference>, SortedSet<PackageReference>>() {
+    override fun convert(value: Set<PackageReference>) = value.toSortedSet(compareBy { it.id })
+}
+
 class PackageSortedSetConverter : StdConverter<Set<Package>, SortedSet<Package>>() {
     override fun convert(value: Set<Package>) = value.toSortedSet(compareBy { it.id })
 }

diff --git a/model/src/test/kotlin/CompatibilityDependencyNavigatorTest.kt b/model/src/test/kotlin/CompatibilityDependencyNavigatorTest.kt
@@ -256,5 +256,5 @@ private fun createProject(
 private fun createScope(name: String) =
     Scope(
         name = name,
-        dependencies = sortedSetOf(PackageReference(Identifier.EMPTY.copy(name = "dep$name")))
+        dependencies = setOf(PackageReference(Identifier.EMPTY.copy(name = "dep$name")))
     )
diff --git a/model/src/test/kotlin/DependencyTreeNavigatorTest.kt b/model/src/test/kotlin/DependencyTreeNavigatorTest.kt
@@ -35,7 +35,7 @@ class DependencyTreeNavigatorTest : AbstractDependencyNavigatorTest() {
                 // various corner cases.
                 val scope = Scope(
                     name = "test",
-                    dependencies = sortedSetOf(
+                    dependencies = setOf(
                         pkg("A"),
                         pkg("B") {
                             pkg("A")
@@ -92,7 +92,7 @@ class DependencyTreeNavigatorTest : AbstractDependencyNavigatorTest() {
             "return 1 if the scope contains only direct dependencies" {
                 val scope = Scope(
                     name = "test",
-                    dependencies = sortedSetOf(
+                    dependencies = setOf(
                         PackageReference(id = Identifier("a")),
                         PackageReference(id = Identifier("b"))
                     )
@@ -105,7 +105,7 @@ class DependencyTreeNavigatorTest : AbstractDependencyNavigatorTest() {
             "return 2 if the scope contains a tree of height 2" {
                 val scope = Scope(
                     name = "test",
-                    dependencies = sortedSetOf(
+                    dependencies = setOf(
                         pkg("a") {
                             pkg("a1")
                         }
@@ -119,7 +119,7 @@ class DependencyTreeNavigatorTest : AbstractDependencyNavigatorTest() {
             "return 3 if it contains a tree of height 3" {
                 val scope = Scope(
                     name = "test",
-                    dependencies = sortedSetOf(
+                    dependencies = setOf(
                         pkg("a") {
                             pkg("a1") {
                                 pkg("a11")
@@ -146,7 +146,7 @@ private const val RESULT_WITH_ISSUES_FILE = "src/test/assets/result-with-issues-
 
 private class PackageRefBuilder(id: String) {
     private val id = Identifier(id)
-    private val dependencies = sortedSetOf<PackageReference>()
+    private val dependencies = mutableSetOf<PackageReference>()
 
     fun pkg(id: String, block: PackageRefBuilder.() -> Unit = {}) {
         dependencies += PackageRefBuilder(id).apply { block() }.build()

diff --git a/model/src/test/kotlin/PackageReferenceTest.kt b/model/src/test/kotlin/PackageReferenceTest.kt
@@ -30,7 +30,7 @@ import io.kotest.matchers.string.endWith
 class PackageReferenceTest : WordSpec() {
     companion object {
         fun pkgRefFromIdStr(id: String, vararg dependencies: PackageReference) =
-            PackageReference(Identifier(id), dependencies = dependencies.toSortedSet())
+            PackageReference(Identifier(id), dependencies = dependencies.toSet())
     }
 
     private val node111 = pkgRefFromIdStr("::node1_1_1")

diff --git a/model/src/test/kotlin/config/ExcludesTest.kt b/model/src/test/kotlin/config/ExcludesTest.kt
@@ -56,9 +56,9 @@ class ExcludesTest : WordSpec() {
     private val pathExclude3 = PathExclude("**/*.ext", PathExcludeReason.BUILD_TOOL_OF, "")
     private val pathExclude4 = PathExclude("**/file.ext", PathExcludeReason.BUILD_TOOL_OF, "")
 
-    private val scope1 = Scope("scope1", sortedSetOf(PackageReference(id)))
-    private val scope2 = Scope("scope2", sortedSetOf(PackageReference(id)))
-    private val scopeProject1 = Scope("scopeProject1", sortedSetOf(PackageReference(project1.id)))
+    private val scope1 = Scope("scope1", setOf(PackageReference(id)))
+    private val scope2 = Scope("scope2", setOf(PackageReference(id)))
+    private val scopeProject1 = Scope("scopeProject1", setOf(PackageReference(project1.id)))
 
     private val scopeExclude1 = ScopeExclude("scope1", ScopeExcludeReason.PROVIDED_DEPENDENCY_OF, "")
     private val scopeExclude2 = ScopeExclude("scope2", ScopeExcludeReason.PROVIDED_DEPENDENCY_OF, "")

diff --git a/model/src/test/kotlin/licenses/TestData.kt b/model/src/test/kotlin/licenses/TestData.kt
@@ -117,7 +117,7 @@ val allPackages = setOf(
 
 val scope = Scope(
     name = "compile",
-    dependencies = sortedSetOf(
+    dependencies = setOf(
         packageWithoutLicense.toReference(),
         packageWithConcludedLicense.toReference(),
         packageWithDeclaredLicense.toReference(),