Skip to content

Commit

Permalink
refactor(model): Do not use SortedSet for dependencies
Browse files Browse the repository at this point in the history
Only sort on serialization for human readability and reproducibility.

Signed-off-by: Frank Viernau <frank_viernau@epam.com>
  • Loading branch information
fviernau committed May 24, 2024
1 parent 64f7aae commit d8fb1d0
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 12 deletions.
4 changes: 3 additions & 1 deletion model/src/main/kotlin/DependencyGraph.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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

/**
Expand Down Expand Up @@ -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<DependencyReference> = sortedSetOf(),
@JsonSerialize(contentConverter = DependencyReferenceSortedSetConverter::class)
val dependencies: Set<DependencyReference> = emptySet(),

/**
* The type of linkage used for the referred package from its dependent package. As most of our supported
Expand Down
4 changes: 2 additions & 2 deletions model/src/main/kotlin/utils/DependencyGraphBuilder.kt
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,7 @@ class DependencyGraphBuilder<D>(
transitive: Boolean,
processed: Set<D>
): DependencyReference? {
val transitiveDependencies = dependencyHandler.dependenciesFor(dependency).mapNotNull {
val transitiveDependencies = dependencyHandler.dependenciesFor(dependency).mapNotNullTo(mutableSetOf()) {
addDependencyToGraph(scopeName, it, transitive = true, processed)
}

Expand All @@ -387,7 +387,7 @@ class DependencyGraphBuilder<D>(
val ref = DependencyReference(
pkg = index.root,
fragment = index.fragment,
dependencies = transitiveDependencies.toSortedSet(),
dependencies = transitiveDependencies,
linkage = dependencyHandler.linkageFor(dependency),
issues = issues
)
Expand Down
6 changes: 6 additions & 0 deletions model/src/main/kotlin/utils/SortedCollectionConverters.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.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
Expand All @@ -49,6 +50,11 @@ class DependencyGraphEdgeSortedSetConverter : StdConverter<Set<DependencyGraphEd
override fun convert(value: Set<DependencyGraphEdge>) = value.toSortedSet(compareBy({ it.from }, { it.to }))
}

class DependencyReferenceSortedSetConverter : StdConverter<Set<DependencyReference>, SortedSet<DependencyReference>>() {
override fun convert(value: Set<DependencyReference>) =
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<FileList>, Set<FileList>>() {
override fun convert(value: Set<FileList>) = value.sortedBy { it.provenance.getSortKey() }.toSet()
Expand Down
12 changes: 6 additions & 6 deletions model/src/test/kotlin/DependencyGraphTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)),
Expand Down Expand Up @@ -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)))

Expand Down
6 changes: 3 additions & 3 deletions model/src/test/kotlin/ProjectTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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)),
Expand Down

0 comments on commit d8fb1d0

Please sign in to comment.