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 copyrightFindings #7000

Merged
merged 5 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
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ private operator fun ScanSummary.minus(other: ScanSummary?): ScanSummary {

return copy(
licenseFindings = (licenseFindings - other.licenseFindings).toSortedSet(),
copyrightFindings = (copyrightFindings - other.copyrightFindings).toSortedSet()
copyrightFindings = copyrightFindings - other.copyrightFindings
)
}

Expand Down
6 changes: 2 additions & 4 deletions model/src/main/kotlin/CopyrightFinding.kt
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,8 @@ data class CopyrightFinding(
* The text location where the copyright statement was found.
*/
val location: TextLocation
) : Comparable<CopyrightFinding> {
) {
companion object {
private val COMPARATOR = compareBy<CopyrightFinding>({ it.statement }, { it.location })
val COMPARATOR = compareBy<CopyrightFinding>({ it.statement }, { it.location })
fviernau marked this conversation as resolved.
Show resolved Hide resolved
}

override fun compareTo(other: CopyrightFinding) = COMPARATOR.compare(this, other)
}
8 changes: 5 additions & 3 deletions model/src/main/kotlin/ScanSummary.kt
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ 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.RootLicenseMatcher
import org.ossreviewtoolkit.model.utils.SnippetFinding
import org.ossreviewtoolkit.model.utils.SnippetFindingSortedSetConverter
Expand Down Expand Up @@ -69,7 +70,8 @@ data class ScanSummary(
*/
@JsonInclude(JsonInclude.Include.NON_EMPTY)
@JsonProperty("copyrights")
val copyrightFindings: SortedSet<CopyrightFinding> = sortedSetOf(),
@JsonSerialize(converter = CopyrightFindingSortedSetConverter::class)
val copyrightFindings: Set<CopyrightFinding> = emptySet(),

/**
* The detected snippet findings.
Expand Down Expand Up @@ -118,7 +120,7 @@ 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.filter { it.location.matchesPath() }.toSortedSet()
val copyrightFindings = copyrightFindings.filterTo(mutableSetOf()) { it.location.matchesPath() }

return copy(
licenseFindings = licenseFindings,
Expand All @@ -135,7 +137,7 @@ data class ScanSummary(

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

import java.util.SortedSet

import org.ossreviewtoolkit.model.CopyrightFinding
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 PackageSortedSetConverter : StdConverter<Set<Package>, SortedSet<Package>>() {
override fun convert(value: Set<Package>) = value.toSortedSet(compareBy { it.id })
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,7 @@ internal fun OrtResult.deduplicateProjectScanResults(targetProjects: Set<Identif
val repositoryPath = getRepositoryPath(scanResult.provenance as RepositoryProvenance)
fun TextLocation.isExcluded() = "$repositoryPath$path" !in excludePaths

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

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

return listOf(
ScanResult(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -460,7 +460,7 @@ private fun createOrtResult(): OrtResult {
location = TextLocation("LICENSE", 1)
)
),
copyrightFindings = sortedSetOf(
copyrightFindings = setOf(
CopyrightFinding(
statement = "Copyright 2020 Some copyright holder in source artifact",
location = TextLocation("some/file", 1)
Expand Down Expand Up @@ -497,7 +497,7 @@ private fun createOrtResult(): OrtResult {
location = TextLocation("LICENSE", 1)
)
),
copyrightFindings = sortedSetOf(
copyrightFindings = setOf(
CopyrightFinding(
statement = "Copyright 2020 Some copyright holder in VCS",
location = TextLocation("some/file", 1)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ private val ortResult = OrtResult(
location = TextLocation("LICENSE", 1)
)
),
copyrightFindings = sortedSetOf(
copyrightFindings = setOf(
CopyrightFinding(
statement = "Copyright 2020 Some copyright holder in source artifact",
location = TextLocation("some/file", 1)
Expand Down Expand Up @@ -314,7 +314,7 @@ private val ortResult = OrtResult(
location = TextLocation("LICENSE", 1)
)
),
copyrightFindings = sortedSetOf(
copyrightFindings = setOf(
CopyrightFinding(
statement = "Copyright 2020 Some copyright holder in VCS",
location = TextLocation("some/file", 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 @@ -233,7 +233,7 @@ val ORT_RESULT = OrtResult(
location = TextLocation("project-with-findings/file", 1)
)
),
copyrightFindings = sortedSetOf(
copyrightFindings = setOf(
CopyrightFinding(
statement = "Copyright 1",
location = TextLocation("project-with-findings/file", 1)
Expand All @@ -260,7 +260,7 @@ val ORT_RESULT = OrtResult(
location = TextLocation("file", 1)
)
),
copyrightFindings = sortedSetOf(
copyrightFindings = setOf(
CopyrightFinding(
statement = "Copyright 1",
location = TextLocation("file", 1)
Expand Down Expand Up @@ -289,7 +289,7 @@ val ORT_RESULT = OrtResult(
location = TextLocation("file", 1)
)
),
copyrightFindings = sortedSetOf(
copyrightFindings = setOf(
CopyrightFinding(
statement = "Copyright 1",
location = TextLocation("LICENSE", 1)
Expand Down Expand Up @@ -326,7 +326,7 @@ val ORT_RESULT = OrtResult(
location = TextLocation("file", 50)
)
),
copyrightFindings = sortedSetOf(
copyrightFindings = setOf(
CopyrightFinding(
statement = "Copyright 1",
location = TextLocation("LICENSE", 1)
Expand Down Expand Up @@ -363,7 +363,7 @@ val ORT_RESULT = OrtResult(
location = TextLocation("file2", 1)
)
),
copyrightFindings = sortedSetOf(
copyrightFindings = setOf(
CopyrightFinding(
statement = "Copyright 1",
location = TextLocation("file1", 1)
Expand Down Expand Up @@ -392,7 +392,7 @@ val ORT_RESULT = OrtResult(
location = TextLocation("LICENSE", 1)
)
),
copyrightFindings = sortedSetOf(
copyrightFindings = setOf(
CopyrightFinding(
statement = "Copyright 1",
location = TextLocation("LICENSE", 1)
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 @@ -710,7 +710,7 @@ fun ScanResult.toNestedProvenanceScanResult(nestedProvenance: NestedProvenance):
provenance = provenance,
summary = summary.copy(
licenseFindings = licenseFindingsByProvenance[provenance].orEmpty().toSortedSet(),
copyrightFindings = copyrightFindingsByProvenance[provenance].orEmpty().toSortedSet()
copyrightFindings = copyrightFindingsByProvenance[provenance].orEmpty().toSet()
)
)
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,12 +120,12 @@ data class NestedProvenanceScanResult(
return findings
}

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

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/fossid/FossId.kt
Original file line number Diff line number Diff line change
Expand Up @@ -860,7 +860,7 @@ class FossId internal constructor(
endTime = Instant.now(),
packageVerificationCode = "",
licenseFindings = licenseFindings.toSortedSet(),
copyrightFindings = copyrightFindings.toSortedSet(),
copyrightFindings = copyrightFindings,
snippetFindings = snippetFindings,
issues = issues
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@ internal data class RawResults(
* A data class to hold FossID mapped results.
*/
internal data class FindingsContainer(
val licenseFindings: MutableList<LicenseFinding>,
val copyrightFindings: MutableList<CopyrightFinding>
val licenseFindings: MutableSet<LicenseFinding>,
val copyrightFindings: MutableSet<CopyrightFinding>
)

/**
Expand All @@ -58,8 +58,8 @@ internal fun <T : Summarizable> List<T>.mapSummary(
issues: MutableList<Issue>,
detectedLicenseMapping: Map<String, String>
): FindingsContainer {
val licenseFindings = mutableListOf<LicenseFinding>()
val copyrightFindings = mutableListOf<CopyrightFinding>()
val licenseFindings = mutableSetOf<LicenseFinding>()
val copyrightFindings = mutableSetOf<CopyrightFinding>()

val files = filterNot { it.getFileName() in ignoredFiles }
files.forEach { summarizable ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ internal fun generateSummary(
endTime = endTime,
packageVerificationCode = verificationCode,
licenseFindings = getLicenseFindings(result, detectedLicenseMapping, parseExpressions).toSortedSet(),
copyrightFindings = getCopyrightFindings(result).toSortedSet(),
copyrightFindings = getCopyrightFindings(result),
issues = issues + getIssues(result)
)
}
Expand Down Expand Up @@ -190,7 +190,7 @@ private fun getLicenseFindings(
result: JsonNode,
detectedLicenseMapping: Map<String, String>,
parseExpressions: Boolean
): List<LicenseFinding> {
): Set<LicenseFinding> {
val licenseFindings = mutableListOf<LicenseFinding>()

val input = getInputPath(result)
Expand Down Expand Up @@ -227,7 +227,7 @@ private fun getLicenseFindings(
}
}

return associateLicensesWithExceptions(licenseFindings)
return associateLicensesWithExceptions(licenseFindings).toSet()
}

/**
Expand Down Expand Up @@ -265,8 +265,8 @@ internal fun replaceLicenseKeys(licenseExpression: String, replacements: Collect
/**
* Get the copyright findings from the given [result].
*/
private fun getCopyrightFindings(result: JsonNode): List<CopyrightFinding> {
val copyrightFindings = mutableListOf<CopyrightFinding>()
private fun getCopyrightFindings(result: JsonNode): Set<CopyrightFinding> {
val copyrightFindings = mutableSetOf<CopyrightFinding>()

val input = getInputPath(result)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,8 @@ internal fun generateSummary(
result: FullScanResponse,
detectedLicenseMapping: Map<String, String>
): ScanSummary {
val licenseFindings = mutableListOf<LicenseFinding>()
val copyrightFindings = mutableListOf<CopyrightFinding>()
val licenseFindings = mutableSetOf<LicenseFinding>()
val copyrightFindings = mutableSetOf<CopyrightFinding>()
val snippetFindings = mutableSetOf<SnippetFinding>()

result.forEach { (_, scanResponses) ->
Expand Down Expand Up @@ -97,7 +97,7 @@ internal fun generateSummary(
endTime = endTime,
packageVerificationCode = verificationCode,
licenseFindings = licenseFindings.toSortedSet(),
copyrightFindings = copyrightFindings.toSortedSet(),
copyrightFindings = copyrightFindings,
snippetFindings = snippetFindings
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ private val scanResultRoot = ScanResult(
LicenseFinding("Apache-2.0", TextLocation("file", 1)),
LicenseFinding("Apache-2.0", TextLocation("submodules/file", 1))
),
copyrightFindings = sortedSetOf(
copyrightFindings = setOf(
CopyrightFinding("Copyright", TextLocation("file", 1)),
CopyrightFinding("Copyright", TextLocation("submodules/file", 1))
)
Expand All @@ -136,7 +136,7 @@ private val scanResultSubmoduleA = ScanResult(
LicenseFinding("Apache-2.0", TextLocation("fileA", 1)),
LicenseFinding("Apache-2.0", TextLocation("dir/fileA", 1))
),
copyrightFindings = sortedSetOf(
copyrightFindings = setOf(
CopyrightFinding("Copyright", TextLocation("fileA", 1)),
CopyrightFinding("Copyright", TextLocation("dir/fileA", 1))
)
Expand All @@ -151,7 +151,7 @@ private val scanResultSubmoduleB = ScanResult(
LicenseFinding("Apache-2.0", TextLocation("fileB", 1)),
LicenseFinding("Apache-2.0", TextLocation("dir/fileB", 1))
),
copyrightFindings = sortedSetOf(
copyrightFindings = setOf(
CopyrightFinding("Copyright", TextLocation("fileB", 1)),
CopyrightFinding("Copyright", TextLocation("dir/fileB", 1))
)
Expand Down