From 5012c7959ff287e0e3229dce3daffc53607d753b Mon Sep 17 00:00:00 2001 From: Frank Viernau Date: Fri, 24 May 2024 12:36:03 +0200 Subject: [PATCH] refactor(model): Do not use `SortedSet` for `dependencies` Only sort on serialization for human readability and reproducibility. Signed-off-by: Frank Viernau --- model/src/main/kotlin/DependencyGraph.kt | 4 +++- .../src/main/kotlin/utils/DependencyGraphBuilder.kt | 4 ++-- .../main/kotlin/utils/SortedCollectionConverters.kt | 6 ++++++ model/src/test/kotlin/DependencyGraphTest.kt | 12 ++++++------ model/src/test/kotlin/ProjectTest.kt | 6 +++--- 5 files changed, 20 insertions(+), 12 deletions(-) diff --git a/model/src/main/kotlin/DependencyGraph.kt b/model/src/main/kotlin/DependencyGraph.kt index 21b67054b896b..2eec2456ca542 100644 --- a/model/src/main/kotlin/DependencyGraph.kt +++ b/model/src/main/kotlin/DependencyGraph.kt @@ -26,6 +26,7 @@ import com.fasterxml.jackson.databind.annotation.JsonSerialize import java.util.SortedSet import org.ossreviewtoolkit.model.utils.DependencyGraphEdgeSortedSetConverter +import org.ossreviewtoolkit.model.utils.DependencyReferenceSortedSetConverter import org.ossreviewtoolkit.model.utils.PackageLinkageValueFilter /** @@ -317,7 +318,8 @@ class DependencyReference( /** * A set with the references to the dependencies of this dependency. That way a tree-like structure is established. */ - val dependencies: SortedSet = sortedSetOf(), + @JsonSerialize(contentConverter = DependencyReferenceSortedSetConverter::class) + val dependencies: Set = emptySet(), /** * The type of linkage used for the referred package from its dependent package. As most of our supported diff --git a/model/src/main/kotlin/utils/DependencyGraphBuilder.kt b/model/src/main/kotlin/utils/DependencyGraphBuilder.kt index ba05caa199747..9bc61595c1da0 100644 --- a/model/src/main/kotlin/utils/DependencyGraphBuilder.kt +++ b/model/src/main/kotlin/utils/DependencyGraphBuilder.kt @@ -370,7 +370,7 @@ class DependencyGraphBuilder( transitive: Boolean, processed: Set ): DependencyReference? { - val transitiveDependencies = dependencyHandler.dependenciesFor(dependency).mapNotNull { + val transitiveDependencies = dependencyHandler.dependenciesFor(dependency).mapNotNullTo(mutableSetOf()) { addDependencyToGraph(scopeName, it, transitive = true, processed) } @@ -387,7 +387,7 @@ class DependencyGraphBuilder( val ref = DependencyReference( pkg = index.root, fragment = index.fragment, - dependencies = transitiveDependencies.toSortedSet(), + dependencies = transitiveDependencies, linkage = dependencyHandler.linkageFor(dependency), issues = issues ) diff --git a/model/src/main/kotlin/utils/SortedCollectionConverters.kt b/model/src/main/kotlin/utils/SortedCollectionConverters.kt index 3cdee6a32ac47..8e403bd0f0952 100644 --- a/model/src/main/kotlin/utils/SortedCollectionConverters.kt +++ b/model/src/main/kotlin/utils/SortedCollectionConverters.kt @@ -28,6 +28,7 @@ import java.util.SortedSet import org.ossreviewtoolkit.model.ArtifactProvenance import org.ossreviewtoolkit.model.CopyrightFinding import org.ossreviewtoolkit.model.DependencyGraphEdge +import org.ossreviewtoolkit.model.DependencyReference import org.ossreviewtoolkit.model.FileList import org.ossreviewtoolkit.model.Identifier import org.ossreviewtoolkit.model.LicenseFinding @@ -49,6 +50,11 @@ class DependencyGraphEdgeSortedSetConverter : StdConverter) = value.toSortedSet(compareBy({ it.from }, { it.to })) } +class DependencyReferenceSortedSetConverter : StdConverter, SortedSet>() { + override fun convert(value: Set) = + value.toSortedSet(compareBy({ it.pkg.toString() }, { it.fragment })) +} + /** Do not convert to SortedSet in order to not require a comparator consistent with equals */ class FileListSortedSetConverter : StdConverter, Set>() { override fun convert(value: Set) = value.sortedBy { it.provenance.getSortKey() }.toSet() diff --git a/model/src/test/kotlin/DependencyGraphTest.kt b/model/src/test/kotlin/DependencyGraphTest.kt index fcc8ed3eeeb44..e4efa125bf5f2 100644 --- a/model/src/test/kotlin/DependencyGraphTest.kt +++ b/model/src/test/kotlin/DependencyGraphTest.kt @@ -93,8 +93,8 @@ class DependencyGraphTest : WordSpec({ ) val refLang = DependencyReference(0) val refCollections = DependencyReference(1) - val refConfig = DependencyReference(2, dependencies = sortedSetOf(refLang, refCollections)) - val refCsv = DependencyReference(3, dependencies = sortedSetOf(refConfig)) + val refConfig = DependencyReference(2, dependencies = setOf(refLang, refCollections)) + val refCsv = DependencyReference(3, dependencies = setOf(refConfig)) val fragments = sortedSetOf(DependencyGraph.DEPENDENCY_REFERENCE_COMPARATOR, refCsv) val scopeMap = mapOf("s" to listOf(RootDependencyIndex(3))) val graph = DependencyGraph(ids, fragments, scopeMap) @@ -113,10 +113,10 @@ class DependencyGraphTest : WordSpec({ val refLogging = DependencyReference(3) val refLang = DependencyReference(0) val refCollections1 = DependencyReference(1) - val refCollections2 = DependencyReference(1, fragment = 1, dependencies = sortedSetOf(refLogging)) - val refConfig1 = DependencyReference(2, dependencies = sortedSetOf(refLang, refCollections1)) + val refCollections2 = DependencyReference(1, fragment = 1, dependencies = setOf(refLogging)) + val refConfig1 = DependencyReference(2, dependencies = setOf(refLang, refCollections1)) val refConfig2 = - DependencyReference(2, fragment = 1, dependencies = sortedSetOf(refLang, refCollections2)) + DependencyReference(2, fragment = 1, dependencies = setOf(refLang, refCollections2)) val fragments = sortedSetOf(refConfig1, refConfig2) val scopeMap = mapOf( "s1" to listOf(RootDependencyIndex(2)), @@ -170,7 +170,7 @@ class DependencyGraphTest : WordSpec({ ) val issue = Issue(source = "analyzer", message = "Could not analyze :-(") val refLang = DependencyReference(0, linkage = PackageLinkage.PROJECT_DYNAMIC) - val refCol = DependencyReference(1, issues = listOf(issue), dependencies = sortedSetOf(refLang)) + val refCol = DependencyReference(1, issues = listOf(issue), dependencies = setOf(refLang)) val trees = sortedSetOf(refCol) val scopeMap = mapOf("s" to listOf(RootDependencyIndex(1))) diff --git a/model/src/test/kotlin/ProjectTest.kt b/model/src/test/kotlin/ProjectTest.kt index 902d37fc7ce8e..334e9629e2c77 100644 --- a/model/src/test/kotlin/ProjectTest.kt +++ b/model/src/test/kotlin/ProjectTest.kt @@ -59,10 +59,10 @@ private fun createDependencyGraph(qualified: Boolean = false): DependencyGraph { exampleId ) val langRef = DependencyReference(0) - val textRef = DependencyReference(1, dependencies = sortedSetOf(langRef)) + val textRef = DependencyReference(1, dependencies = setOf(langRef)) val strutsRef = DependencyReference(2) - val csvRef = DependencyReference(3, dependencies = sortedSetOf(langRef)) - val exampleRef = DependencyReference(4, dependencies = sortedSetOf(textRef, strutsRef)) + val csvRef = DependencyReference(3, dependencies = setOf(langRef)) + val exampleRef = DependencyReference(4, dependencies = setOf(textRef, strutsRef)) val plainScopeMapping = mapOf( "default" to listOf(RootDependencyIndex(4)),