Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not use sorted sets for package references in Scope and PackageReferences #7010

Merged
merged 10 commits into from
May 23, 2023
Merged
4 changes: 2 additions & 2 deletions analyzer/src/main/kotlin/managers/GoDep.kt
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ class GoDep(

val projects = parseProjects(workingDir, gopath)
val packages = mutableSetOf<Package>()
val packageRefs = mutableListOf<PackageReference>()
val packageRefs = mutableSetOf<PackageReference>()

for (project in projects) {
// parseProjects() made sure that all entries contain these keys
Expand Down Expand Up @@ -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)
Expand Down
7 changes: 3 additions & 4 deletions analyzer/src/main/kotlin/managers/utils/Graph.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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)
}

Expand All @@ -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) }
}

/**
Expand Down
18 changes: 9 additions & 9 deletions analyzer/src/test/kotlin/AnalyzerResultBuilderTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Expand Down Expand Up @@ -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)
)
)
)
Expand All @@ -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)
)
)
Expand Down
6 changes: 3 additions & 3 deletions evaluator/src/test/kotlin/TestData.kt
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ val allPackages = setOf(

val scopeExcluded = Scope(
name = "compile",
dependencies = sortedSetOf(
dependencies = setOf(
packageExcluded.toReference()
)
)
Expand All @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions model/src/main/kotlin/DependencyGraph.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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.")
}

Expand Down Expand Up @@ -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)
}

Expand Down
4 changes: 1 addition & 3 deletions model/src/main/kotlin/Package.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
10 changes: 6 additions & 4 deletions model/src/main/kotlin/PackageReference.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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].
Expand Down Expand Up @@ -116,10 +118,10 @@ data class PackageReference(
* return the modified [PackageReference]. The tree is traversed depth-first (post-order).
*/
fun traverse(transform: (PackageReference) -> PackageReference): PackageReference {
val transformedDependencies = dependencies.map {
val transformedDependencies = dependencies.mapTo(mutableSetOf()) {
it.traverse(transform)
}
return transform(copy(dependencies = transformedDependencies.toSortedSet()))
return transform(copy(dependencies = transformedDependencies))
}

override fun <T> visitDependencies(block: (Sequence<DependencyNode>) -> T): T = block(dependencies.asSequence())
Expand Down
7 changes: 5 additions & 2 deletions model/src/main/kotlin/Scope.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand Down
5 changes: 5 additions & 0 deletions model/src/main/kotlin/utils/SortedSetConverters.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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 })
}
Expand Down
6 changes: 4 additions & 2 deletions model/src/test/kotlin/AbstractDependencyNavigatorTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import io.kotest.matchers.collections.haveSize
import io.kotest.matchers.collections.shouldContainExactlyInAnyOrder
import io.kotest.matchers.maps.containExactly as containExactlyEntries
import io.kotest.matchers.sequences.beEmpty as beEmptySequence
import io.kotest.matchers.sequences.containExactly as containSequenceExactly
import io.kotest.matchers.sequences.containAllInAnyOrder
import io.kotest.matchers.should
import io.kotest.matchers.shouldBe
import io.kotest.matchers.shouldNot
Expand Down Expand Up @@ -72,7 +72,9 @@ abstract class AbstractDependencyNavigatorTest : WordSpec() {

"directDependencies" should {
"return the direct dependencies of a project" {
navigator.directDependencies(testProject, "test").map { it.id } should containSequenceExactly(
val scopeDependencies = navigator.directDependencies(testProject, "test").map { it.id }

scopeDependencies should containAllInAnyOrder(
Identifier("Maven:org.scalacheck:scalacheck_2.12:1.13.5"),
Identifier("Maven:org.scalatest:scalatest_2.12:3.0.4")
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,8 @@ private fun createProject(
/**
* Create a [Scope] with the given [name] and a synthetic dependency.
*/
private fun createScope(name: String): Scope {
val dependencies = sortedSetOf(PackageReference(Identifier.EMPTY.copy(name = "dep$name")))
return Scope(name, dependencies)
}
private fun createScope(name: String) =
Scope(
name = name,
dependencies = setOf(PackageReference(Identifier.EMPTY.copy(name = "dep$name")))
)
4 changes: 2 additions & 2 deletions model/src/test/kotlin/DependencyGraphTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ private fun id(group: String, artifact: String, version: String): Identifier =
*/
private fun scopeDependencies(scopes: Set<Scope>, name: String): String = buildString {
scopes.find { it.name == name }?.let { scope ->
scope.dependencies.forEach { dumpDependencies(it) }
scope.dependencies.sorted().forEach { dumpDependencies(it) }
fviernau marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand All @@ -253,7 +253,7 @@ private fun StringBuilder.dumpDependencies(ref: PackageReference) {
append(ref.id)
if (ref.dependencies.isNotEmpty()) {
append('<')
ref.dependencies.forEach { dumpDependencies(it) }
ref.dependencies.sorted().forEach { dumpDependencies(it) }
append('>')
}
}
10 changes: 5 additions & 5 deletions model/src/test/kotlin/DependencyTreeNavigatorTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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"))
)
Expand All @@ -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")
}
Expand All @@ -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")
Expand All @@ -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()
Expand Down
2 changes: 1 addition & 1 deletion model/src/test/kotlin/PackageReferenceTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
6 changes: 3 additions & 3 deletions model/src/test/kotlin/config/ExcludesTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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, "")
Expand Down
2 changes: 1 addition & 1 deletion model/src/test/kotlin/licenses/TestData.kt
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ val allPackages = setOf(

val scope = Scope(
name = "compile",
dependencies = sortedSetOf(
dependencies = setOf(
packageWithoutLicense.toReference(),
packageWithConcludedLicense.toReference(),
packageWithDeclaredLicense.toReference(),
Expand Down
Loading