From f81a2cc88b3c763a73dfc14df4fba1648fe0aef6 Mon Sep 17 00:00:00 2001 From: Frank Viernau Date: Mon, 15 May 2023 12:44:29 +0200 Subject: [PATCH 1/5] refactor(ScanOssResultParser): Consistently use sets for the findings This simplifies an upcoming change. Signed-off-by: Frank Viernau --- .../src/main/kotlin/scanners/scanoss/ScanOssResultParser.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scanner/src/main/kotlin/scanners/scanoss/ScanOssResultParser.kt b/scanner/src/main/kotlin/scanners/scanoss/ScanOssResultParser.kt index b796e0244fba..3c6e644fb8ea 100644 --- a/scanner/src/main/kotlin/scanners/scanoss/ScanOssResultParser.kt +++ b/scanner/src/main/kotlin/scanners/scanoss/ScanOssResultParser.kt @@ -68,8 +68,8 @@ internal fun generateSummary( result: FullScanResponse, detectedLicenseMapping: Map ): ScanSummary { - val licenseFindings = mutableListOf() - val copyrightFindings = mutableListOf() + val licenseFindings = mutableSetOf() + val copyrightFindings = mutableSetOf() val snippetFindings = mutableSetOf() result.forEach { (_, scanResponses) -> From 0aff5579cb4a4a3fbf019506056d8a70cb7d2720 Mon Sep 17 00:00:00 2001 From: Frank Viernau Date: Mon, 15 May 2023 13:29:28 +0200 Subject: [PATCH 2/5] refactor(ScanCodeResultParser): Return sets from getters for findings This simplifies upcoming changes. Signed-off-by: Frank Viernau --- .../main/kotlin/scanners/scancode/ScanCodeResultParser.kt | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/scanner/src/main/kotlin/scanners/scancode/ScanCodeResultParser.kt b/scanner/src/main/kotlin/scanners/scancode/ScanCodeResultParser.kt index 289836c11c8e..093472d76c79 100644 --- a/scanner/src/main/kotlin/scanners/scancode/ScanCodeResultParser.kt +++ b/scanner/src/main/kotlin/scanners/scancode/ScanCodeResultParser.kt @@ -190,7 +190,7 @@ private fun getLicenseFindings( result: JsonNode, detectedLicenseMapping: Map, parseExpressions: Boolean -): List { +): Set { val licenseFindings = mutableListOf() val input = getInputPath(result) @@ -227,7 +227,7 @@ private fun getLicenseFindings( } } - return associateLicensesWithExceptions(licenseFindings) + return associateLicensesWithExceptions(licenseFindings).toSet() } /** @@ -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 { - val copyrightFindings = mutableListOf() +private fun getCopyrightFindings(result: JsonNode): Set { + val copyrightFindings = mutableSetOf() val input = getInputPath(result) From 8c2cd670f49b47c97c4095b6e28eda36f00a5630 Mon Sep 17 00:00:00 2001 From: Frank Viernau Date: Mon, 15 May 2023 14:04:57 +0200 Subject: [PATCH 3/5] refactor(FossId): Use sets for the findings This simplifies an upcoming change. Signed-off-by: Frank Viernau --- .../src/main/kotlin/scanners/fossid/FossIdScanResults.kt | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/scanner/src/main/kotlin/scanners/fossid/FossIdScanResults.kt b/scanner/src/main/kotlin/scanners/fossid/FossIdScanResults.kt index fc420014ea72..cb8d9e71a2da 100644 --- a/scanner/src/main/kotlin/scanners/fossid/FossIdScanResults.kt +++ b/scanner/src/main/kotlin/scanners/fossid/FossIdScanResults.kt @@ -46,8 +46,8 @@ internal data class RawResults( * A data class to hold FossID mapped results. */ internal data class FindingsContainer( - val licenseFindings: MutableList, - val copyrightFindings: MutableList + val licenseFindings: MutableSet, + val copyrightFindings: MutableSet ) /** @@ -58,8 +58,8 @@ internal fun List.mapSummary( issues: MutableList, detectedLicenseMapping: Map ): FindingsContainer { - val licenseFindings = mutableListOf() - val copyrightFindings = mutableListOf() + val licenseFindings = mutableSetOf() + val copyrightFindings = mutableSetOf() val files = filterNot { it.getFileName() in ignoredFiles } files.forEach { summarizable -> From a6654533d81c357f235539c06b10e02735ff7a43 Mon Sep 17 00:00:00 2001 From: Frank Viernau Date: Mon, 15 May 2023 13:39:27 +0200 Subject: [PATCH 4/5] refactor(ScanSummary): Do not use sorted set for `copyrightFindings` Only sort on serialization for human readability and reproducibility. While that allows to remove the implementation of the comparable interface, still keep the comparator constant for use in the converter and any place where `CopyrightFinding`s shall be shown to users. Signed-off-by: Frank Viernau --- .../kotlin/commands/SubtractScanResultsCommand.kt | 2 +- model/src/main/kotlin/CopyrightFinding.kt | 6 ++---- model/src/main/kotlin/ScanSummary.kt | 8 +++++--- model/src/main/kotlin/utils/SortedSetConverters.kt | 5 +++++ .../src/main/kotlin/FreemarkerTemplateProcessor.kt | 2 +- .../test/kotlin/FreeMarkerTemplateProcessorTest.kt | 2 +- .../opossum/src/test/kotlin/OpossumReporterTest.kt | 4 ++-- .../funTest/kotlin/SpdxDocumentReporterFunTest.kt | 4 ++-- reporter/src/testFixtures/kotlin/TestData.kt | 12 ++++++------ scanner/src/main/kotlin/Scanner.kt | 2 +- scanner/src/main/kotlin/scanners/fossid/FossId.kt | 2 +- .../kotlin/scanners/scancode/ScanCodeResultParser.kt | 2 +- .../kotlin/scanners/scanoss/ScanOssResultParser.kt | 2 +- .../provenance/NestedProvenanceScanResultTest.kt | 6 +++--- 14 files changed, 32 insertions(+), 27 deletions(-) diff --git a/helper-cli/src/main/kotlin/commands/SubtractScanResultsCommand.kt b/helper-cli/src/main/kotlin/commands/SubtractScanResultsCommand.kt index 8ee5a273f6b7..649cfb443205 100644 --- a/helper-cli/src/main/kotlin/commands/SubtractScanResultsCommand.kt +++ b/helper-cli/src/main/kotlin/commands/SubtractScanResultsCommand.kt @@ -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 ) } diff --git a/model/src/main/kotlin/CopyrightFinding.kt b/model/src/main/kotlin/CopyrightFinding.kt index 5ce19f575410..5974ab390fdf 100644 --- a/model/src/main/kotlin/CopyrightFinding.kt +++ b/model/src/main/kotlin/CopyrightFinding.kt @@ -32,10 +32,8 @@ data class CopyrightFinding( * The text location where the copyright statement was found. */ val location: TextLocation -) : Comparable { +) { companion object { - private val COMPARATOR = compareBy({ it.statement }, { it.location }) + val COMPARATOR = compareBy({ it.statement }, { it.location }) } - - override fun compareTo(other: CopyrightFinding) = COMPARATOR.compare(this, other) } diff --git a/model/src/main/kotlin/ScanSummary.kt b/model/src/main/kotlin/ScanSummary.kt index 817fad93cab4..4bee0426e8b5 100644 --- a/model/src/main/kotlin/ScanSummary.kt +++ b/model/src/main/kotlin/ScanSummary.kt @@ -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 @@ -69,7 +70,8 @@ data class ScanSummary( */ @JsonInclude(JsonInclude.Include.NON_EMPTY) @JsonProperty("copyrights") - val copyrightFindings: SortedSet = sortedSetOf(), + @JsonSerialize(converter = CopyrightFindingSortedSetConverter::class) + val copyrightFindings: Set = emptySet(), /** * The detected snippet findings. @@ -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, @@ -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) } ) } } diff --git a/model/src/main/kotlin/utils/SortedSetConverters.kt b/model/src/main/kotlin/utils/SortedSetConverters.kt index 8f75d682c7db..f0cd41ae6841 100644 --- a/model/src/main/kotlin/utils/SortedSetConverters.kt +++ b/model/src/main/kotlin/utils/SortedSetConverters.kt @@ -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, SortedSet>() { + override fun convert(value: Set) = value.toSortedSet(CopyrightFinding.COMPARATOR) +} + class PackageSortedSetConverter : StdConverter, SortedSet>() { override fun convert(value: Set) = value.toSortedSet(compareBy { it.id }) } diff --git a/plugins/reporters/freemarker/src/main/kotlin/FreemarkerTemplateProcessor.kt b/plugins/reporters/freemarker/src/main/kotlin/FreemarkerTemplateProcessor.kt index 529297cee8d5..853d7b937638 100644 --- a/plugins/reporters/freemarker/src/main/kotlin/FreemarkerTemplateProcessor.kt +++ b/plugins/reporters/freemarker/src/main/kotlin/FreemarkerTemplateProcessor.kt @@ -468,7 +468,7 @@ internal fun OrtResult.deduplicateProjectScanResults(targetProjects: Set ): List { 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( diff --git a/plugins/reporters/opossum/src/test/kotlin/OpossumReporterTest.kt b/plugins/reporters/opossum/src/test/kotlin/OpossumReporterTest.kt index 2a0f35e27eee..1f750131c7ec 100644 --- a/plugins/reporters/opossum/src/test/kotlin/OpossumReporterTest.kt +++ b/plugins/reporters/opossum/src/test/kotlin/OpossumReporterTest.kt @@ -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) @@ -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) diff --git a/plugins/reporters/spdx/src/funTest/kotlin/SpdxDocumentReporterFunTest.kt b/plugins/reporters/spdx/src/funTest/kotlin/SpdxDocumentReporterFunTest.kt index 0b912303b5bc..e360726042a3 100644 --- a/plugins/reporters/spdx/src/funTest/kotlin/SpdxDocumentReporterFunTest.kt +++ b/plugins/reporters/spdx/src/funTest/kotlin/SpdxDocumentReporterFunTest.kt @@ -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) @@ -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) diff --git a/reporter/src/testFixtures/kotlin/TestData.kt b/reporter/src/testFixtures/kotlin/TestData.kt index 6632cd668284..4cb7437de8a4 100644 --- a/reporter/src/testFixtures/kotlin/TestData.kt +++ b/reporter/src/testFixtures/kotlin/TestData.kt @@ -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) @@ -260,7 +260,7 @@ val ORT_RESULT = OrtResult( location = TextLocation("file", 1) ) ), - copyrightFindings = sortedSetOf( + copyrightFindings = setOf( CopyrightFinding( statement = "Copyright 1", location = TextLocation("file", 1) @@ -289,7 +289,7 @@ val ORT_RESULT = OrtResult( location = TextLocation("file", 1) ) ), - copyrightFindings = sortedSetOf( + copyrightFindings = setOf( CopyrightFinding( statement = "Copyright 1", location = TextLocation("LICENSE", 1) @@ -326,7 +326,7 @@ val ORT_RESULT = OrtResult( location = TextLocation("file", 50) ) ), - copyrightFindings = sortedSetOf( + copyrightFindings = setOf( CopyrightFinding( statement = "Copyright 1", location = TextLocation("LICENSE", 1) @@ -363,7 +363,7 @@ val ORT_RESULT = OrtResult( location = TextLocation("file2", 1) ) ), - copyrightFindings = sortedSetOf( + copyrightFindings = setOf( CopyrightFinding( statement = "Copyright 1", location = TextLocation("file1", 1) @@ -392,7 +392,7 @@ val ORT_RESULT = OrtResult( location = TextLocation("LICENSE", 1) ) ), - copyrightFindings = sortedSetOf( + copyrightFindings = setOf( CopyrightFinding( statement = "Copyright 1", location = TextLocation("LICENSE", 1) diff --git a/scanner/src/main/kotlin/Scanner.kt b/scanner/src/main/kotlin/Scanner.kt index 5b63a6fc2e43..a7f22c386f86 100644 --- a/scanner/src/main/kotlin/Scanner.kt +++ b/scanner/src/main/kotlin/Scanner.kt @@ -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() ) ) ) diff --git a/scanner/src/main/kotlin/scanners/fossid/FossId.kt b/scanner/src/main/kotlin/scanners/fossid/FossId.kt index c30b89746ce8..c01969a388a9 100644 --- a/scanner/src/main/kotlin/scanners/fossid/FossId.kt +++ b/scanner/src/main/kotlin/scanners/fossid/FossId.kt @@ -860,7 +860,7 @@ class FossId internal constructor( endTime = Instant.now(), packageVerificationCode = "", licenseFindings = licenseFindings.toSortedSet(), - copyrightFindings = copyrightFindings.toSortedSet(), + copyrightFindings = copyrightFindings, snippetFindings = snippetFindings, issues = issues ) diff --git a/scanner/src/main/kotlin/scanners/scancode/ScanCodeResultParser.kt b/scanner/src/main/kotlin/scanners/scancode/ScanCodeResultParser.kt index 093472d76c79..c5e755e1d21e 100644 --- a/scanner/src/main/kotlin/scanners/scancode/ScanCodeResultParser.kt +++ b/scanner/src/main/kotlin/scanners/scancode/ScanCodeResultParser.kt @@ -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) ) } diff --git a/scanner/src/main/kotlin/scanners/scanoss/ScanOssResultParser.kt b/scanner/src/main/kotlin/scanners/scanoss/ScanOssResultParser.kt index 3c6e644fb8ea..2866cb8ff690 100644 --- a/scanner/src/main/kotlin/scanners/scanoss/ScanOssResultParser.kt +++ b/scanner/src/main/kotlin/scanners/scanoss/ScanOssResultParser.kt @@ -97,7 +97,7 @@ internal fun generateSummary( endTime = endTime, packageVerificationCode = verificationCode, licenseFindings = licenseFindings.toSortedSet(), - copyrightFindings = copyrightFindings.toSortedSet(), + copyrightFindings = copyrightFindings, snippetFindings = snippetFindings ) } diff --git a/scanner/src/test/kotlin/provenance/NestedProvenanceScanResultTest.kt b/scanner/src/test/kotlin/provenance/NestedProvenanceScanResultTest.kt index 729814184dd3..c6cd65135eb0 100644 --- a/scanner/src/test/kotlin/provenance/NestedProvenanceScanResultTest.kt +++ b/scanner/src/test/kotlin/provenance/NestedProvenanceScanResultTest.kt @@ -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)) ) @@ -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)) ) @@ -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)) ) From b790ed27a2d603344bf83d23d360fe8c8d518525 Mon Sep 17 00:00:00 2001 From: Frank Viernau Date: Mon, 15 May 2023 14:07:12 +0200 Subject: [PATCH 5/5] refactor: Return a set from `mergeCopyrightFindings()` Returning a sorted set is no more necessary, because `ScanSummary` has been changed in a preceeding change to use a set for the copyright findings. Signed-off-by: Frank Viernau --- .../src/main/kotlin/provenance/NestedProvenanceScanResult.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scanner/src/main/kotlin/provenance/NestedProvenanceScanResult.kt b/scanner/src/main/kotlin/provenance/NestedProvenanceScanResult.kt index a892a687b38b..f7462751c00e 100644 --- a/scanner/src/main/kotlin/provenance/NestedProvenanceScanResult.kt +++ b/scanner/src/main/kotlin/provenance/NestedProvenanceScanResult.kt @@ -120,12 +120,12 @@ data class NestedProvenanceScanResult( return findings } - private fun Map>.mergeCopyrightFindings(): SortedSet { + private fun Map>.mergeCopyrightFindings(): Set { 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}")) } }