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

ScanSummary: Do not use sorted set for licenseFindings #7004

Merged
merged 4 commits into from
May 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion evaluator/src/test/kotlin/ProjectSourceRuleTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ private fun createOrtResult(
url = "https://github.com/oss-review-toolkit/example.git",
revision = "0000000000000000000000000000000000000000"
)
val licenseFindings = detectedLicensesForFilePath.flatMapTo(sortedSetOf()) { (filePath, licenses) ->
val licenseFindings = detectedLicensesForFilePath.flatMapTo(mutableSetOf()) { (filePath, licenses) ->
licenses.map { license ->
LicenseFinding(license, TextLocation(filePath, startLine = 1, endLine = 2))
}
Expand Down
2 changes: 1 addition & 1 deletion evaluator/src/test/kotlin/TestData.kt
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ val ortResult = OrtResult(
provenance = UnknownProvenance,
scanner = ScannerDetails.EMPTY,
summary = ScanSummary.EMPTY.copy(
licenseFindings = sortedSetOf(
licenseFindings = setOf(
LicenseFinding("LicenseRef-a", TextLocation("LICENSE", 1)),
LicenseFinding("LicenseRef-b", TextLocation("LICENSE", 2))
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ private operator fun ScanSummary.minus(other: ScanSummary?): ScanSummary {
if (other == null) return this

return copy(
licenseFindings = (licenseFindings - other.licenseFindings).toSortedSet(),
licenseFindings = licenseFindings - other.licenseFindings,
copyrightFindings = copyrightFindings - other.copyrightFindings
)
}
Expand Down
6 changes: 2 additions & 4 deletions model/src/main/kotlin/LicenseFinding.kt
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,9 @@ data class LicenseFinding(
* 100.0 means that the scanner is 100% confident that the finding is correct.
*/
val score: Float? = null
) : Comparable<LicenseFinding> {
) {
companion object {
private val COMPARATOR = compareBy<LicenseFinding>({ it.license.toString() }, { it.location })
val COMPARATOR = compareBy<LicenseFinding>({ it.license.toString() }, { it.location })
.thenByDescending { it.score }

/**
Expand All @@ -73,8 +73,6 @@ data class LicenseFinding(
}

constructor(license: String, location: TextLocation, score: Float? = null) : this(license.toSpdx(), location, score)

override fun compareTo(other: LicenseFinding) = COMPARATOR.compare(this, other)
}

/**
Expand Down
14 changes: 6 additions & 8 deletions model/src/main/kotlin/ScanSummary.kt
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,10 @@ import com.fasterxml.jackson.annotation.JsonProperty
import com.fasterxml.jackson.databind.annotation.JsonSerialize

import java.time.Instant
import java.util.SortedSet

import org.ossreviewtoolkit.model.config.LicenseFilePatterns
import org.ossreviewtoolkit.model.utils.CopyrightFindingSortedSetConverter
import org.ossreviewtoolkit.model.utils.LicenseFindingSortedSetConverter
import org.ossreviewtoolkit.model.utils.RootLicenseMatcher
import org.ossreviewtoolkit.model.utils.SnippetFinding
import org.ossreviewtoolkit.model.utils.SnippetFindingSortedSetConverter
Expand Down Expand Up @@ -63,7 +63,8 @@ data class ScanSummary(
*/
@JsonInclude(JsonInclude.Include.NON_EMPTY)
@JsonProperty("licenses")
val licenseFindings: SortedSet<LicenseFinding> = sortedSetOf(),
@JsonSerialize(converter = LicenseFindingSortedSetConverter::class)
val licenseFindings: Set<LicenseFinding> = emptySet(),

/**
* The detected copyright findings.
Expand Down Expand Up @@ -119,12 +120,9 @@ data class ScanSummary(

fun TextLocation.matchesPath() = this.path.startsWith("$path/") || this.path in applicableLicenseFiles

val licenseFindings = licenseFindings.filter { it.location.matchesPath() }.toSortedSet()
val copyrightFindings = copyrightFindings.filterTo(mutableSetOf()) { it.location.matchesPath() }

return copy(
licenseFindings = licenseFindings,
copyrightFindings = copyrightFindings
licenseFindings = licenseFindings.filterTo(mutableSetOf()) { it.location.matchesPath() },
copyrightFindings = copyrightFindings.filterTo(mutableSetOf()) { it.location.matchesPath() }
)
}

Expand All @@ -136,7 +134,7 @@ data class ScanSummary(
val matcher = FileMatcher(ignorePatterns)

return copy(
licenseFindings = licenseFindings.filterTo(sortedSetOf()) { !matcher.matches(it.location.path) },
licenseFindings = licenseFindings.filterTo(mutableSetOf()) { !matcher.matches(it.location.path) },
copyrightFindings = copyrightFindings.filterTo(mutableSetOf()) { !matcher.matches(it.location.path) }
)
}
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 @@ -26,13 +26,18 @@ import com.fasterxml.jackson.databind.util.StdConverter
import java.util.SortedSet

import org.ossreviewtoolkit.model.CopyrightFinding
import org.ossreviewtoolkit.model.LicenseFinding
import org.ossreviewtoolkit.model.Package
import org.ossreviewtoolkit.model.Project

class CopyrightFindingSortedSetConverter : StdConverter<Set<CopyrightFinding>, SortedSet<CopyrightFinding>>() {
override fun convert(value: Set<CopyrightFinding>) = value.toSortedSet(CopyrightFinding.COMPARATOR)
}

class LicenseFindingSortedSetConverter : StdConverter<Set<LicenseFinding>, SortedSet<LicenseFinding>>() {
override fun convert(value: Set<LicenseFinding>) = value.toSortedSet(LicenseFinding.COMPARATOR)
}

class PackageSortedSetConverter : StdConverter<Set<Package>, SortedSet<Package>>() {
override fun convert(value: Set<Package>) = value.toSortedSet(compareBy { it.id })
}
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 @@ -49,7 +49,7 @@ val concludedLicense = "LicenseRef-a AND LicenseRef-b".toSpdx()
val declaredLicenses = setOf("LicenseRef-a", "LicenseRef-b")
val declaredLicensesProcessed = DeclaredLicenseProcessor.process(declaredLicenses)

val licenseFindings = sortedSetOf(
val licenseFindings = setOf(
LicenseFinding("LicenseRef-a", TextLocation("LICENSE", 1)),
LicenseFinding("LicenseRef-b", TextLocation("LICENSE", 2))
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import org.ossreviewtoolkit.model.CuratedPackage
import org.ossreviewtoolkit.model.DependencyNode
import org.ossreviewtoolkit.model.Identifier
import org.ossreviewtoolkit.model.Issue
import org.ossreviewtoolkit.model.LicenseFinding
import org.ossreviewtoolkit.model.PackageLinkage
import org.ossreviewtoolkit.model.Project
import org.ossreviewtoolkit.model.Provenance
Expand Down Expand Up @@ -635,8 +636,9 @@ internal class EvaluatedModelMapper(private val input: ReporterInput) {
) {
val pathExcludes = getPathExcludes(id, scanResult.provenance)
val licenseFindingCurations = getLicenseFindingCurations(id, scanResult.provenance)
// Sort the curated findings here to avoid the need to sort in the web-app each time it is loaded.
val curatedFindings = curationsMatcher.applyAll(scanResult.summary.licenseFindings, licenseFindingCurations)
.mapNotNullTo(mutableSetOf()) { it.curatedFinding }
.mapNotNull { it.curatedFinding }.toSortedSet(LicenseFinding.COMPARATOR)
val matchResult = findingsMatcher.match(curatedFindings, scanResult.summary.copyrightFindings)
val matchedFindings = matchResult.matchedFindings.entries.groupBy { it.key.license }.mapValues { entry ->
val licenseFindings = entry.value.map { it.key }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -469,7 +469,7 @@ internal fun OrtResult.deduplicateProjectScanResults(targetProjects: Set<Identif
fun TextLocation.isExcluded() = "$repositoryPath$path" !in excludePaths

val copyrightFindings = summary.copyrightFindings.filterTo(mutableSetOf()) { it.location.isExcluded() }
val licenseFindings = summary.licenseFindings.filterTo(sortedSetOf()) { it.location.isExcluded() }
val licenseFindings = summary.licenseFindings.filterTo(mutableSetOf()) { it.location.isExcluded() }

scanResult.copy(
summary = summary.copy(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ private fun scanResults(
vcsInfo: VcsInfo,
findingsPaths: Collection<String>
): List<ScanResult> {
val licenseFindings = findingsPaths.mapTo(sortedSetOf()) { LicenseFinding("MIT", TextLocation(it, 1)) }
val licenseFindings = findingsPaths.mapTo(mutableSetOf()) { LicenseFinding("MIT", TextLocation(it, 1)) }
val copyrightFindings = findingsPaths.mapTo(mutableSetOf()) { CopyrightFinding("(c)", TextLocation(it, 1)) }

return listOf(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -454,7 +454,7 @@ private fun createOrtResult(): OrtResult {
),
summary = ScanSummary.EMPTY.copy(
packageVerificationCode = "0000000000000000000000000000000000000000",
licenseFindings = sortedSetOf(
licenseFindings = setOf(
LicenseFinding(
license = "Apache-2.0",
location = TextLocation("LICENSE", 1)
Expand Down Expand Up @@ -491,7 +491,7 @@ private fun createOrtResult(): OrtResult {
startTime = Instant.EPOCH,
endTime = Instant.EPOCH,
packageVerificationCode = "0000000000000000000000000000000000000000",
licenseFindings = sortedSetOf(
licenseFindings = setOf(
LicenseFinding(
license = "BSD-2-Clause",
location = TextLocation("LICENSE", 1)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ private val ortResult = OrtResult(
scanner = ScannerDetails.EMPTY,
summary = ScanSummary.EMPTY.copy(
packageVerificationCode = "0000000000000000000000000000000000000000",
licenseFindings = sortedSetOf(
licenseFindings = setOf(
LicenseFinding(
license = "Apache-2.0",
location = TextLocation("LICENSE", 1)
Expand Down Expand Up @@ -308,7 +308,7 @@ private val ortResult = OrtResult(
scanner = ScannerDetails.EMPTY,
summary = ScanSummary.EMPTY.copy(
packageVerificationCode = "0000000000000000000000000000000000000000",
licenseFindings = sortedSetOf(
licenseFindings = setOf(
LicenseFinding(
license = "BSD-2-Clause",
location = TextLocation("LICENSE", 1)
Expand Down
12 changes: 6 additions & 6 deletions reporter/src/testFixtures/kotlin/TestData.kt
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ val ORT_RESULT = OrtResult(
provenance = UnknownProvenance,
scanner = ScannerDetails(name = "scanner", version = "1.0", configuration = ""),
summary = ScanSummary.EMPTY.copy(
licenseFindings = sortedSetOf(
licenseFindings = setOf(
LicenseFinding(
license = "MIT",
location = TextLocation("project-with-findings/file", 1)
Expand All @@ -254,7 +254,7 @@ val ORT_RESULT = OrtResult(
provenance = UnknownProvenance,
scanner = ScannerDetails(name = "scanner", version = "1.0", configuration = ""),
summary = ScanSummary.EMPTY.copy(
licenseFindings = sortedSetOf(
licenseFindings = setOf(
LicenseFinding(
license = "MIT",
location = TextLocation("file", 1)
Expand All @@ -279,7 +279,7 @@ val ORT_RESULT = OrtResult(
),
scanner = ScannerDetails(name = "scanner", version = "1.0", configuration = ""),
summary = ScanSummary.EMPTY.copy(
licenseFindings = sortedSetOf(
licenseFindings = setOf(
LicenseFinding(
license = "MIT",
location = TextLocation("LICENSE", 1)
Expand Down Expand Up @@ -312,7 +312,7 @@ val ORT_RESULT = OrtResult(
),
scanner = ScannerDetails(name = "scanner", version = "1.0", configuration = ""),
summary = ScanSummary.EMPTY.copy(
licenseFindings = sortedSetOf(
licenseFindings = setOf(
LicenseFinding(
license = "MIT",
location = TextLocation("LICENSE", 1)
Expand Down Expand Up @@ -353,7 +353,7 @@ val ORT_RESULT = OrtResult(
),
scanner = ScannerDetails(name = "scanner", version = "1.0", configuration = ""),
summary = ScanSummary.EMPTY.copy(
licenseFindings = sortedSetOf(
licenseFindings = setOf(
LicenseFinding(
license = "MIT",
location = TextLocation("file1", 1)
Expand Down Expand Up @@ -386,7 +386,7 @@ val ORT_RESULT = OrtResult(
),
scanner = ScannerDetails(name = "scanner", version = "1.0", configuration = ""),
summary = ScanSummary.EMPTY.copy(
licenseFindings = sortedSetOf(
licenseFindings = setOf(
LicenseFinding(
license = "BSD-3-Clause",
location = TextLocation("LICENSE", 1)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ private class DummyScanner : PathScannerWrapper {
override val criteria = ScannerCriteria.forDetails(details)

override fun scanPath(path: File, context: ScanContext): ScanSummary {
val licenseFindings = path.listFiles().orEmpty().mapTo(sortedSetOf()) { file ->
val licenseFindings = path.listFiles().orEmpty().mapTo(mutableSetOf()) { file ->
LicenseFinding(
license = SpdxConstants.NONE,
location = TextLocation(file.relativeTo(path).invariantSeparatorsPath, TextLocation.UNKNOWN_LINE)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ private fun createScanResult(
provenance,
scannerDetails,
ScanSummary.EMPTY.copy(
licenseFindings = sortedSetOf(
licenseFindings = setOf(
LicenseFinding(license, TextLocation("file.txt", 1, 2))
)
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,11 +103,11 @@ abstract class AbstractStorageFunTest(vararg listeners: TestListener) : WordSpec
startTime = Instant.EPOCH + Duration.ofMinutes(1),
endTime = Instant.EPOCH + Duration.ofMinutes(2),
packageVerificationCode = "packageVerificationCode",
licenseFindings = sortedSetOf(
licenseFindings = setOf(
LicenseFinding("license-1.1", DUMMY_TEXT_LOCATION),
LicenseFinding("license-1.2", DUMMY_TEXT_LOCATION)
),
issues = mutableListOf(
issues = listOf(
Issue(source = "source-1", message = "error-1"),
Issue(source = "source-2", message = "error-2")
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ class ClearlyDefinedStorageFunTest : StringSpec({
startTime = Instant.parse("2020-02-14T00:36:14.000335513Z"),
endTime = Instant.parse("2020-02-14T00:36:37.000492119Z"),
packageVerificationCode = SpdxConstants.NONE,
licenseFindings = sortedSetOf(
licenseFindings = setOf(
LicenseFinding(
license = "MIT",
location = TextLocation(
Expand Down
2 changes: 1 addition & 1 deletion scanner/src/main/kotlin/Scanner.kt
Original file line number Diff line number Diff line change
Expand Up @@ -709,7 +709,7 @@ fun ScanResult.toNestedProvenanceScanResult(nestedProvenance: NestedProvenance):
copy(
provenance = provenance,
summary = summary.copy(
licenseFindings = licenseFindingsByProvenance[provenance].orEmpty().toSortedSet(),
licenseFindings = licenseFindingsByProvenance[provenance].orEmpty().toSet(),
copyrightFindings = copyrightFindingsByProvenance[provenance].orEmpty().toSet()
)
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
package org.ossreviewtoolkit.scanner.provenance

import java.time.Instant
import java.util.SortedSet

import org.ossreviewtoolkit.model.CopyrightFinding
import org.ossreviewtoolkit.model.KnownProvenance
Expand Down Expand Up @@ -107,12 +106,12 @@ data class NestedProvenanceScanResult(
}
}

private fun Map<KnownProvenance, List<ScanResult>>.mergeLicenseFindings(): SortedSet<LicenseFinding> {
private fun Map<KnownProvenance, List<ScanResult>>.mergeLicenseFindings(): Set<LicenseFinding> {
val findingsByPath = mapKeys { getPath(it.key) }.mapValues { (_, scanResults) ->
scanResults.flatMap { it.summary.licenseFindings }
}

val findings = findingsByPath.flatMapTo(sortedSetOf()) { (path, findings) ->
val findings = findingsByPath.flatMapTo(mutableSetOf()) { (path, findings) ->
val prefix = if (path.isEmpty()) path else "$path/"
findings.map { it.copy(location = it.location.copy(path = "$prefix${it.location.path}")) }
}
Expand Down
2 changes: 1 addition & 1 deletion scanner/src/main/kotlin/scanners/Askalono.kt
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ class Askalono internal constructor(
}

private fun generateSummary(startTime: Instant, endTime: Instant, scanPath: File, result: String): ScanSummary {
val licenseFindings = sortedSetOf<LicenseFinding>()
val licenseFindings = mutableSetOf<LicenseFinding>()

result.lines().forEach { line ->
val root = jsonMapper.readTree(line)
Expand Down
2 changes: 1 addition & 1 deletion scanner/src/main/kotlin/scanners/BoyterLc.kt
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ class BoyterLc internal constructor(
}

private fun generateSummary(startTime: Instant, endTime: Instant, scanPath: File, resultFile: File): ScanSummary {
val licenseFindings = sortedSetOf<LicenseFinding>()
val licenseFindings = mutableSetOf<LicenseFinding>()
val result = resultFile.readTree()

result.flatMapTo(licenseFindings) { file ->
Expand Down
2 changes: 1 addition & 1 deletion scanner/src/main/kotlin/scanners/Licensee.kt
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ class Licensee internal constructor(
}

private fun generateSummary(startTime: Instant, endTime: Instant, scanPath: File, result: String): ScanSummary {
val licenseFindings = sortedSetOf<LicenseFinding>()
val licenseFindings = mutableSetOf<LicenseFinding>()

val json = jsonMapper.readTree(result)
val matchedFiles = json["matched_files"]
Expand Down
2 changes: 1 addition & 1 deletion scanner/src/main/kotlin/scanners/fossid/FossId.kt
Original file line number Diff line number Diff line change
Expand Up @@ -859,7 +859,7 @@ class FossId internal constructor(
startTime = startTime,
endTime = Instant.now(),
packageVerificationCode = "",
licenseFindings = licenseFindings.toSortedSet(),
licenseFindings = licenseFindings,
copyrightFindings = copyrightFindings,
snippetFindings = snippetFindings,
issues = issues
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ internal fun generateSummary(
startTime = startTime,
endTime = endTime,
packageVerificationCode = verificationCode,
licenseFindings = getLicenseFindings(result, detectedLicenseMapping, parseExpressions).toSortedSet(),
licenseFindings = getLicenseFindings(result, detectedLicenseMapping, parseExpressions),
copyrightFindings = getCopyrightFindings(result),
issues = issues + getIssues(result)
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ internal fun generateSummary(
startTime = startTime,
endTime = endTime,
packageVerificationCode = verificationCode,
licenseFindings = licenseFindings.toSortedSet(),
licenseFindings = licenseFindings,
copyrightFindings = copyrightFindings,
snippetFindings = snippetFindings
)
Expand Down
Loading