From 3054d380b46791289d95d775790a0cf36d10ea20 Mon Sep 17 00:00:00 2001 From: Frank Viernau Date: Fri, 24 May 2024 17:38:26 +0200 Subject: [PATCH 1/3] test(model): Make formatting of `fragments` construction consistent Signed-off-by: Frank Viernau --- model/src/test/kotlin/DependencyGraphTest.kt | 22 +++++++++----------- 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/model/src/test/kotlin/DependencyGraphTest.kt b/model/src/test/kotlin/DependencyGraphTest.kt index e4efa125bf5f..4c64fc46dfac 100644 --- a/model/src/test/kotlin/DependencyGraphTest.kt +++ b/model/src/test/kotlin/DependencyGraphTest.kt @@ -35,12 +35,11 @@ class DependencyGraphTest : WordSpec({ id("org.apache.commons", "commons-collections4", "4.4"), id(group = "org.junit", artifact = "junit", version = "5") ) - val fragments = - sortedSetOf( - DependencyReference(0), - DependencyReference(1), - DependencyReference(2) - ) + val fragments = sortedSetOf( + DependencyReference(0), + DependencyReference(1), + DependencyReference(2) + ) val scopeMap = mapOf( "p1:scope1" to listOf(RootDependencyIndex(0), RootDependencyIndex(1)), "p2:scope2" to listOf(RootDependencyIndex(1), RootDependencyIndex(2)) @@ -63,12 +62,11 @@ class DependencyGraphTest : WordSpec({ id("org.apache.commons", "commons-collections4", "4.4"), id("org.junit", "junit", "5") ) - val fragments = - sortedSetOf( - DependencyReference(0), - DependencyReference(1), - DependencyReference(2) - ) + val fragments = sortedSetOf( + DependencyReference(0), + DependencyReference(1), + DependencyReference(2) + ) val scopeMap = mapOf( qualifiedScopeName to listOf(RootDependencyIndex(0), RootDependencyIndex(1)), DependencyGraph.qualifyScope(qualifier, "scope2") to listOf( From 093f6b63a096d1998d2270b368a0388977b372e6 Mon Sep 17 00:00:00 2001 From: Frank Viernau Date: Fri, 24 May 2024 17:28:04 +0200 Subject: [PATCH 2/3] refactor(model): Use named arguments in a couple of constructor calls Improve readability, and while at it make use of the default value for scope roots in two of the calls. Signed-off-by: Frank Viernau --- .../test/kotlin/AnalyzerResultBuilderTest.kt | 23 +++++++++---------- .../kotlin/utils/DependencyGraphBuilder.kt | 9 ++++---- model/src/test/kotlin/DependencyGraphTest.kt | 7 +++++- model/src/test/kotlin/ProjectTest.kt | 6 ++++- 4 files changed, 26 insertions(+), 19 deletions(-) diff --git a/analyzer/src/test/kotlin/AnalyzerResultBuilderTest.kt b/analyzer/src/test/kotlin/AnalyzerResultBuilderTest.kt index a7428a769b36..f791cd33e8ad 100644 --- a/analyzer/src/test/kotlin/AnalyzerResultBuilderTest.kt +++ b/analyzer/src/test/kotlin/AnalyzerResultBuilderTest.kt @@ -103,18 +103,17 @@ class AnalyzerResultBuilderTest : WordSpec() { DependencyGraph.qualifyScope(project2, "scope-3") to listOf(RootDependencyIndex(0)) ) - private val graph1 = - DependencyGraph( - dependencies1, - sortedSetOf(DependencyGraph.DEPENDENCY_REFERENCE_COMPARATOR, depRef1, depRef2), - scopeMapping1 - ) - private val graph2 = - DependencyGraph( - dependencies2, - sortedSetOf(DependencyGraph.DEPENDENCY_REFERENCE_COMPARATOR, depRef3), - scopeMapping2 - ) + private val graph1 = DependencyGraph( + packages = dependencies1, + scopeRoots = sortedSetOf(DependencyGraph.DEPENDENCY_REFERENCE_COMPARATOR, depRef1, depRef2), + scopes = scopeMapping1 + ) + + private val graph2 = DependencyGraph( + packages = dependencies2, + scopeRoots = sortedSetOf(DependencyGraph.DEPENDENCY_REFERENCE_COMPARATOR, depRef3), + scopes = scopeMapping2 + ) private val analyzerResult1 = ProjectAnalyzerResult( project1, setOf(package1), listOf(issue3, issue4) diff --git a/model/src/main/kotlin/utils/DependencyGraphBuilder.kt b/model/src/main/kotlin/utils/DependencyGraphBuilder.kt index a6e71b70da8f..562a1a342b00 100644 --- a/model/src/main/kotlin/utils/DependencyGraphBuilder.kt +++ b/model/src/main/kotlin/utils/DependencyGraphBuilder.kt @@ -171,11 +171,10 @@ class DependencyGraphBuilder( val (nodes, edges) = references.toGraph(indexMapping) return DependencyGraph( - sortedDependencyIds, - sortedSetOf(), - constructSortedScopeMappings(scopeMapping, indexMapping), - nodes, - edges.removeCycles() + packages = sortedDependencyIds, + scopes = constructSortedScopeMappings(scopeMapping, indexMapping), + nodes = nodes, + edges = edges.removeCycles() ) } diff --git a/model/src/test/kotlin/DependencyGraphTest.kt b/model/src/test/kotlin/DependencyGraphTest.kt index 4c64fc46dfac..0144fbf67f5a 100644 --- a/model/src/test/kotlin/DependencyGraphTest.kt +++ b/model/src/test/kotlin/DependencyGraphTest.kt @@ -154,7 +154,12 @@ class DependencyGraphTest : WordSpec({ "s2" to listOf(RootDependencyIndex(2, fragment = 1)) ) - val graph = DependencyGraph(ids, sortedSetOf(), scopeMap, nodes, edges) + val graph = DependencyGraph( + packages = ids, + scopes = scopeMap, + nodes = nodes, + edges = edges + ) val scopes = graph.createScopes() scopeDependencies(scopes, "s1") shouldBe "${ids[2]}<${ids[1]}${ids[0]}>" diff --git a/model/src/test/kotlin/ProjectTest.kt b/model/src/test/kotlin/ProjectTest.kt index 334e9629e2c7..ca0adf223c34 100644 --- a/model/src/test/kotlin/ProjectTest.kt +++ b/model/src/test/kotlin/ProjectTest.kt @@ -77,7 +77,11 @@ private fun createDependencyGraph(qualified: Boolean = false): DependencyGraph { plainScopeMapping } - return DependencyGraph(dependencies, sortedSetOf(exampleRef, csvRef), scopeMapping) + return DependencyGraph( + packages = dependencies, + scopeRoots = sortedSetOf(exampleRef, csvRef), + scopes = scopeMapping + ) } class ProjectTest : WordSpec({ From a523522505c893df8cbdf356c8828a8c9de34df9 Mon Sep 17 00:00:00 2001 From: Frank Viernau Date: Fri, 24 May 2024 18:03:20 +0200 Subject: [PATCH 3/3] refactor(model): Stop using `SortedSet` for `scopeRoots` Only sort on serialization for human readability and reproducibility. Also, remove the now unused comparator, which was used only by tests before. Signed-off-by: Frank Viernau --- analyzer/src/test/kotlin/AnalyzerResultBuilderTest.kt | 4 ++-- model/src/main/kotlin/DependencyGraph.kt | 11 ++--------- model/src/test/kotlin/DependencyGraphTest.kt | 10 +++++----- model/src/test/kotlin/ProjectTest.kt | 2 +- 4 files changed, 10 insertions(+), 17 deletions(-) diff --git a/analyzer/src/test/kotlin/AnalyzerResultBuilderTest.kt b/analyzer/src/test/kotlin/AnalyzerResultBuilderTest.kt index f791cd33e8ad..9268f2539741 100644 --- a/analyzer/src/test/kotlin/AnalyzerResultBuilderTest.kt +++ b/analyzer/src/test/kotlin/AnalyzerResultBuilderTest.kt @@ -105,13 +105,13 @@ class AnalyzerResultBuilderTest : WordSpec() { private val graph1 = DependencyGraph( packages = dependencies1, - scopeRoots = sortedSetOf(DependencyGraph.DEPENDENCY_REFERENCE_COMPARATOR, depRef1, depRef2), + scopeRoots = setOf(depRef1, depRef2), scopes = scopeMapping1 ) private val graph2 = DependencyGraph( packages = dependencies2, - scopeRoots = sortedSetOf(DependencyGraph.DEPENDENCY_REFERENCE_COMPARATOR, depRef3), + scopeRoots = setOf(depRef3), scopes = scopeMapping2 ) diff --git a/model/src/main/kotlin/DependencyGraph.kt b/model/src/main/kotlin/DependencyGraph.kt index 2eec2456ca54..b073fa06ca3c 100644 --- a/model/src/main/kotlin/DependencyGraph.kt +++ b/model/src/main/kotlin/DependencyGraph.kt @@ -23,8 +23,6 @@ import com.fasterxml.jackson.annotation.JsonIgnore import com.fasterxml.jackson.annotation.JsonInclude 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 @@ -86,7 +84,8 @@ data class DependencyGraph( * declared by scopes that cannot be reached via other paths in the dependency graph. Note that this property * exists for backwards compatibility only; it is replaced by the lists of nodes and edges. */ - val scopeRoots: SortedSet = sortedSetOf(), + @JsonSerialize(converter = DependencyReferenceSortedSetConverter::class) + val scopeRoots: Set = emptySet(), /** * A mapping from scope names to the direct dependencies of the scopes. Based on this information, the set of @@ -109,12 +108,6 @@ data class DependencyGraph( val edges: Set? = null ) { companion object { - /** - * A comparator for [DependencyReference] objects. Note that the concrete order does not really matter, it - * just has to be well-defined. - */ - val DEPENDENCY_REFERENCE_COMPARATOR = compareBy({ it.pkg }, { it.fragment }) - /** * Return a name for the given [scope][scopeName] that is qualified with parts of the identifier of the given * [project]. This is used to ensure that the scope names are unique when constructing a dependency graph from diff --git a/model/src/test/kotlin/DependencyGraphTest.kt b/model/src/test/kotlin/DependencyGraphTest.kt index 0144fbf67f5a..030018cf7ec1 100644 --- a/model/src/test/kotlin/DependencyGraphTest.kt +++ b/model/src/test/kotlin/DependencyGraphTest.kt @@ -35,7 +35,7 @@ class DependencyGraphTest : WordSpec({ id("org.apache.commons", "commons-collections4", "4.4"), id(group = "org.junit", artifact = "junit", version = "5") ) - val fragments = sortedSetOf( + val fragments = setOf( DependencyReference(0), DependencyReference(1), DependencyReference(2) @@ -62,7 +62,7 @@ class DependencyGraphTest : WordSpec({ id("org.apache.commons", "commons-collections4", "4.4"), id("org.junit", "junit", "5") ) - val fragments = sortedSetOf( + val fragments = setOf( DependencyReference(0), DependencyReference(1), DependencyReference(2) @@ -93,7 +93,7 @@ class DependencyGraphTest : WordSpec({ val refCollections = DependencyReference(1) val refConfig = DependencyReference(2, dependencies = setOf(refLang, refCollections)) val refCsv = DependencyReference(3, dependencies = setOf(refConfig)) - val fragments = sortedSetOf(DependencyGraph.DEPENDENCY_REFERENCE_COMPARATOR, refCsv) + val fragments = setOf(refCsv) val scopeMap = mapOf("s" to listOf(RootDependencyIndex(3))) val graph = DependencyGraph(ids, fragments, scopeMap) val scopes = graph.createScopes() @@ -115,7 +115,7 @@ class DependencyGraphTest : WordSpec({ val refConfig1 = DependencyReference(2, dependencies = setOf(refLang, refCollections1)) val refConfig2 = DependencyReference(2, fragment = 1, dependencies = setOf(refLang, refCollections2)) - val fragments = sortedSetOf(refConfig1, refConfig2) + val fragments = setOf(refConfig1, refConfig2) val scopeMap = mapOf( "s1" to listOf(RootDependencyIndex(2)), "s2" to listOf(RootDependencyIndex(2, fragment = 1)) @@ -174,7 +174,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 = setOf(refLang)) - val trees = sortedSetOf(refCol) + val trees = setOf(refCol) val scopeMap = mapOf("s" to listOf(RootDependencyIndex(1))) val graph = DependencyGraph(ids, trees, scopeMap) diff --git a/model/src/test/kotlin/ProjectTest.kt b/model/src/test/kotlin/ProjectTest.kt index ca0adf223c34..a08feda73993 100644 --- a/model/src/test/kotlin/ProjectTest.kt +++ b/model/src/test/kotlin/ProjectTest.kt @@ -79,7 +79,7 @@ private fun createDependencyGraph(qualified: Boolean = false): DependencyGraph { return DependencyGraph( packages = dependencies, - scopeRoots = sortedSetOf(exampleRef, csvRef), + scopeRoots = setOf(exampleRef, csvRef), scopes = scopeMapping ) }