From 51c78e000ee1dae41a0ac56095ce658fcb6b9457 Mon Sep 17 00:00:00 2001 From: Frank Viernau Date: Mon, 6 Feb 2023 15:54:00 +0100 Subject: [PATCH] feat(OrtResult)!: Associate package curations with provider IDs Extend `ResolvedConfiguration` to associate the curations with the ID of the package curation provider to enable tracability of curations back to the provider. The separate `ResolvedConfiguration.provider` list is introduced to align with the idea of adding provider metadata, as outlined in [^1] and also mentioned in [^2]. This implementation also is a first step towards use cases involving: 1. Replacing the curations for a given provider ID with the given ones. 2. Re-resolve curations only for a particular provider ID. Both are left as TODO for future changes to limit the size of this change, while for 1. a TODO comment is left in the code. Fixes #5668. [^1] https://github.com/oss-review-toolkit/ort/issues/6188#issuecomment-1383833350 [^2]: #5668 Signed-off-by: Frank Viernau --- .../src/test/assets/ort-analyzer-result.yml | 6 +- .../external/git-repo-expected-output.yml | 6 +- ...-multi-project-example-expected-output.yml | 6 +- .../pnpm-workspaces-expected-output.yml | 6 +- .../sbt-http4s-template-expected-output.yml | 6 +- analyzer/src/main/kotlin/Analyzer.kt | 12 ++- ...dencies-expected-result-with-curations.yml | 20 +++-- .../funTest/assets/semver4j-ort-result.yml | 6 +- model/src/main/kotlin/OrtResult.kt | 11 ++- .../src/main/kotlin/ResolvedConfiguration.kt | 76 +++++++++++++++++-- .../analyzer-result-with-dependency-graph.yml | 22 +++--- ...radle-all-dependencies-expected-result.yml | 6 +- .../assets/result-with-issues-graph-old.yml | 6 +- .../test/assets/result-with-issues-graph.yml | 6 +- .../test/assets/result-with-issues-scopes.yml | 6 +- .../sbt-multi-project-example-graph-old.yml | 6 +- .../sbt-multi-project-example-graph.yml | 6 +- .../assets/gradle-all-dependencies-result.yml | 6 +- .../static-html-reporter-test-input.yml | 20 +++-- .../src/funTest/assets/analyzer-result.yml | 6 +- ...my-expected-output-for-analyzer-result.yml | 6 +- 21 files changed, 201 insertions(+), 50 deletions(-) diff --git a/advisor/src/test/assets/ort-analyzer-result.yml b/advisor/src/test/assets/ort-analyzer-result.yml index 83ec52b0704c7..49c8942361a5a 100644 --- a/advisor/src/test/assets/ort-analyzer-result.yml +++ b/advisor/src/test/assets/ort-analyzer-result.yml @@ -571,4 +571,8 @@ analyzer: scanner: null advisor: null evaluator: null -resolved_configuration: {} +resolved_configuration: + package_curations: + providers: + - "DefaultDir" + data: {} diff --git a/analyzer/src/funTest/assets/projects/external/git-repo-expected-output.yml b/analyzer/src/funTest/assets/projects/external/git-repo-expected-output.yml index 6117dc225dcfb..5f908a5b51c06 100644 --- a/analyzer/src/funTest/assets/projects/external/git-repo-expected-output.yml +++ b/analyzer/src/funTest/assets/projects/external/git-repo-expected-output.yml @@ -4120,4 +4120,8 @@ analyzer: scanner: null advisor: null evaluator: null -resolved_configuration: {} +resolved_configuration: + package_curations: + providers: + - "DefaultDir" + data: {} diff --git a/analyzer/src/funTest/assets/projects/external/sbt-multi-project-example-expected-output.yml b/analyzer/src/funTest/assets/projects/external/sbt-multi-project-example-expected-output.yml index f644b31254117..9ec3daf4a1d06 100644 --- a/analyzer/src/funTest/assets/projects/external/sbt-multi-project-example-expected-output.yml +++ b/analyzer/src/funTest/assets/projects/external/sbt-multi-project-example-expected-output.yml @@ -1295,4 +1295,8 @@ analyzer: scanner: null advisor: null evaluator: null -resolved_configuration: {} +resolved_configuration: + package_curations: + providers: + - "DefaultDir" + data: {} diff --git a/analyzer/src/funTest/assets/projects/synthetic/pnpm-workspaces-expected-output.yml b/analyzer/src/funTest/assets/projects/synthetic/pnpm-workspaces-expected-output.yml index eddf211baf102..90c661f323858 100644 --- a/analyzer/src/funTest/assets/projects/synthetic/pnpm-workspaces-expected-output.yml +++ b/analyzer/src/funTest/assets/projects/synthetic/pnpm-workspaces-expected-output.yml @@ -788,4 +788,8 @@ analyzer: scanner: null advisor: null evaluator: null -resolved_configuration: {} +resolved_configuration: + package_curations: + providers: + - "DefaultDir" + data: {} diff --git a/analyzer/src/funTest/assets/projects/synthetic/sbt-http4s-template-expected-output.yml b/analyzer/src/funTest/assets/projects/synthetic/sbt-http4s-template-expected-output.yml index b930415adbc0a..e3e4fdfa7328c 100644 --- a/analyzer/src/funTest/assets/projects/synthetic/sbt-http4s-template-expected-output.yml +++ b/analyzer/src/funTest/assets/projects/synthetic/sbt-http4s-template-expected-output.yml @@ -1721,4 +1721,8 @@ analyzer: scanner: null advisor: null evaluator: null -resolved_configuration: {} +resolved_configuration: + package_curations: + providers: + - "DefaultDir" + data: {} diff --git a/analyzer/src/main/kotlin/Analyzer.kt b/analyzer/src/main/kotlin/Analyzer.kt index 28be04f4a7780..04e66cff50bac 100644 --- a/analyzer/src/main/kotlin/Analyzer.kt +++ b/analyzer/src/main/kotlin/Analyzer.kt @@ -46,6 +46,7 @@ import org.ossreviewtoolkit.model.AnalyzerResult import org.ossreviewtoolkit.model.AnalyzerRun import org.ossreviewtoolkit.model.OrtResult import org.ossreviewtoolkit.model.PackageCuration +import org.ossreviewtoolkit.model.PackageCurations import org.ossreviewtoolkit.model.Repository import org.ossreviewtoolkit.model.ResolvedConfiguration import org.ossreviewtoolkit.model.config.AnalyzerConfiguration @@ -357,17 +358,22 @@ private fun resolveConfiguration( curationProviders: List> ): ResolvedConfiguration { val packageIds = analyzerResult.packages.mapTo(mutableSetOf()) { it.id } - val packageCurations = mutableListOf() + val packageCurations = mutableMapOf>() curationProviders.forEach { (id, curationProvider) -> val (curations, duration) = measureTimedValue { curationProvider.getCurationsFor(packageIds).values.flatten().distinct() } - packageCurations += curations + packageCurations[id] = curations Analyzer.logger().info { "Getting ${curations.size} package curation(s) from provider '$id' took $duration." } } - return ResolvedConfiguration(packageCurations) + return ResolvedConfiguration( + packageCurations = PackageCurations( + providers = curationProviders.map { (id, _) -> id }, + data = packageCurations + ) + ) } diff --git a/cli/src/funTest/assets/gradle-all-dependencies-expected-result-with-curations.yml b/cli/src/funTest/assets/gradle-all-dependencies-expected-result-with-curations.yml index c926d74bebb5a..57af5f2dc1240 100644 --- a/cli/src/funTest/assets/gradle-all-dependencies-expected-result-with-curations.yml +++ b/cli/src/funTest/assets/gradle-all-dependencies-expected-result-with-curations.yml @@ -462,11 +462,15 @@ advisor: null evaluator: null resolved_configuration: package_curations: - - id: "Maven:org.hamcrest::" - curations: - comment: "Use the actual homepage instead of the GitHub page." - homepage_url: "http://hamcrest.org/JavaHamcrest/" - - id: "Maven:org.hamcrest:hamcrest-core:" - curations: - comment: "Fix description." - description: "Curated description." + providers: + - "DefaultDir" + data: + DefaultDir: + - id: "Maven:org.hamcrest::" + curations: + comment: "Use the actual homepage instead of the GitHub page." + homepage_url: "http://hamcrest.org/JavaHamcrest/" + - id: "Maven:org.hamcrest:hamcrest-core:" + curations: + comment: "Fix description." + description: "Curated description." diff --git a/cli/src/funTest/assets/semver4j-ort-result.yml b/cli/src/funTest/assets/semver4j-ort-result.yml index 03cb00f38742a..0dc99fd410e3a 100644 --- a/cli/src/funTest/assets/semver4j-ort-result.yml +++ b/cli/src/funTest/assets/semver4j-ort-result.yml @@ -345,4 +345,8 @@ advisor: severity: "5.5" has_issues: false evaluator: null -resolved_configuration: {} +resolved_configuration: + package_curations: + providers: + - "DefaultDir" + data: {} diff --git a/model/src/main/kotlin/OrtResult.kt b/model/src/main/kotlin/OrtResult.kt index f9c4150931cba..978415c41bcc0 100644 --- a/model/src/main/kotlin/OrtResult.kt +++ b/model/src/main/kotlin/OrtResult.kt @@ -134,7 +134,7 @@ data class OrtResult( val packages = analyzer?.result?.packages.orEmpty().associateBy { it.id } val curatedPackages = applyPackageCurations( packages.values, - resolvedConfiguration.packageCurations + resolvedConfiguration.getAllPackageCurations() ).associateBy { it.metadata.id } val allDependencies = packages.keys.toMutableSet() @@ -326,9 +326,16 @@ data class OrtResult( packageIds.any { curation.isApplicable(it) } } + // TODO: Implement replacing curations for a particular provider ID, without loosing the curations from the + // repository configuration if applicable. + val providerId = "Replaced" + return copy( resolvedConfiguration = resolvedConfiguration.copy( - packageCurations = applicableCurations + packageCurations = PackageCurations( + providers = listOf(providerId), + data = mapOf(providerId to applicableCurations) + ) ) ) } diff --git a/model/src/main/kotlin/ResolvedConfiguration.kt b/model/src/main/kotlin/ResolvedConfiguration.kt index 2c5707d690f9d..1813794028181 100644 --- a/model/src/main/kotlin/ResolvedConfiguration.kt +++ b/model/src/main/kotlin/ResolvedConfiguration.kt @@ -19,7 +19,12 @@ package org.ossreviewtoolkit.model +import com.fasterxml.jackson.annotation.JsonIgnore import com.fasterxml.jackson.annotation.JsonInclude +import com.fasterxml.jackson.databind.annotation.JsonSerialize +import com.fasterxml.jackson.databind.util.StdConverter + +import org.ossreviewtoolkit.utils.common.getDuplicates /** * A container which holds all resolved data which augments ORT's automatically obtained data, like @@ -28,10 +33,71 @@ import com.fasterxml.jackson.annotation.JsonInclude * TODO: Add further data. */ data class ResolvedConfiguration( + @JsonInclude(value = JsonInclude.Include.CUSTOM, valueFilter = PackageCurationsFilter::class) + val packageCurations: PackageCurations = PackageCurations(), +) { + /** + * Return all [PackageCuration]s contained in this [ResolvedConfiguration] in highest-priority-first order. + */ + @JsonIgnore + fun getAllPackageCurations(): List = + packageCurations.providers.flatMap { id -> + packageCurations.data[id].orEmpty() + } +} + +@JsonSerialize(converter = PackageCurationsConverter::class) +data class PackageCurations( /** - * All package curations applicable to the packages contained in the enclosing [OrtResult] ordered - * highest-priority-first. + * All enabled providers ordered highest-priority-first. */ - @JsonInclude(JsonInclude.Include.NON_EMPTY) - val packageCurations: List = emptyList() -) + val providers: List = emptyList(), + + /** + * All package curations applicable to the packages contained in the enclosing [OrtResult]. + */ + val data: Map> = emptyMap() +) { + @JsonIgnore + fun isEmpty(): Boolean = + providers.isEmpty() && data.isEmpty() + + init { + val duplicateProviders = providers.getDuplicates() + require(duplicateProviders.isEmpty()) { + "The list 'providers' contains the following duplicates, which is not allowed: " + + "${duplicateProviders.joinToStringSingleQuoted()}." + } + + val invalidProviderReferences = data.keys.filter { it !in providers } + require(invalidProviderReferences.isEmpty()) { + "The following keys in 'data' reference a non-existing provider, which is not allowed: " + + "${invalidProviderReferences.joinToStringSingleQuoted()}." + } + } +} + +@Suppress("EqualsOrHashCode", "EqualsWithHashCodeExist") // The class is not supposed to be used with hashing. +private class PackageCurationsFilter { + override fun equals(other: Any?): Boolean = + other is PackageCurations && other.isEmpty() +} + +/** + * Sorts the keys of [PackageCurations.data] alphabetically and the values of [PackageCurations.data] by + * their [id][PackageCuration.id]. Drops entries for provider without curations and removes duplicate curations for + * each provider. + */ +private class PackageCurationsConverter : StdConverter() { + override fun convert(value: PackageCurations): PackageCurations { + val sortedData = value.data.filter { (_, curations) -> + curations.isNotEmpty() + }.mapValues { (_, curations) -> + curations.distinct().sortedBy { curation -> curation.id } + }.toList().sortedBy { (provider, _) -> provider }.toMap() + + return value.copy(data = sortedData) + } +} + +private fun Collection.joinToStringSingleQuoted() = joinToString(prefix = "'", separator = "','", postfix = "'") diff --git a/model/src/test/assets/analyzer-result-with-dependency-graph.yml b/model/src/test/assets/analyzer-result-with-dependency-graph.yml index e916e8b40db67..8e40c59bbfeb3 100644 --- a/model/src/test/assets/analyzer-result-with-dependency-graph.yml +++ b/model/src/test/assets/analyzer-result-with-dependency-graph.yml @@ -177,15 +177,19 @@ advisor: null evaluator: null resolved_configuration: package_curations: - - id: "Maven:junit:junit:4.12" - curations: - comment: "ScanCode claims to find some NOASSERTION and Apache-2.0 in the FAQ\ - \ and the pom.xml, both are false-positives." - concluded_license: "EPL-1.0" - vcs: - type: "Git" - url: "https://github.com/junit-team/junit4.git" - revision: "r4.12" + providers: + - "DefaultDir" + data: + DefaultDir: + - id: "Maven:junit:junit:4.12" + curations: + comment: "ScanCode claims to find some NOASSERTION and Apache-2.0 in the FAQ\ + \ and the pom.xml, both are false-positives." + concluded_license: "EPL-1.0" + vcs: + type: "Git" + url: "https://github.com/junit-team/junit4.git" + revision: "r4.12" labels: applicationCategory: "BT05 Client application" projectName: "Semver4jNewDependencyGraphShared" diff --git a/model/src/test/assets/gradle-all-dependencies-expected-result.yml b/model/src/test/assets/gradle-all-dependencies-expected-result.yml index d35abbcd985f8..25368f425a421 100644 --- a/model/src/test/assets/gradle-all-dependencies-expected-result.yml +++ b/model/src/test/assets/gradle-all-dependencies-expected-result.yml @@ -459,4 +459,8 @@ analyzer: scanner: null advisor: null evaluator: null -resolved_configuration: {} +resolved_configuration: + package_curations: + providers: + - "DefaultDir" + data: {} diff --git a/model/src/test/assets/result-with-issues-graph-old.yml b/model/src/test/assets/result-with-issues-graph-old.yml index ef3aaefbce45d..c1857791b4b39 100644 --- a/model/src/test/assets/result-with-issues-graph-old.yml +++ b/model/src/test/assets/result-with-issues-graph-old.yml @@ -1310,4 +1310,8 @@ analyzer: scanner: null advisor: null evaluator: null -resolved_configuration: {} +resolved_configuration: + package_curations: + providers: + - "DefaultDir" + data: {} diff --git a/model/src/test/assets/result-with-issues-graph.yml b/model/src/test/assets/result-with-issues-graph.yml index 92d48970c4afc..eba1b62d16dea 100644 --- a/model/src/test/assets/result-with-issues-graph.yml +++ b/model/src/test/assets/result-with-issues-graph.yml @@ -1339,4 +1339,8 @@ analyzer: scanner: null advisor: null evaluator: null -resolved_configuration: {} +resolved_configuration: + package_curations: + providers: + - "DefaultDir" + data: {} diff --git a/model/src/test/assets/result-with-issues-scopes.yml b/model/src/test/assets/result-with-issues-scopes.yml index 017d1bb947410..b9bf1e12e8e5c 100644 --- a/model/src/test/assets/result-with-issues-scopes.yml +++ b/model/src/test/assets/result-with-issues-scopes.yml @@ -1306,4 +1306,8 @@ analyzer: scanner: null advisor: null evaluator: null -resolved_configuration: {} +resolved_configuration: + package_curations: + providers: + - "DefaultDir" + data: {} diff --git a/model/src/test/assets/sbt-multi-project-example-graph-old.yml b/model/src/test/assets/sbt-multi-project-example-graph-old.yml index 0ce69b7f1f26f..73a1936234f0e 100644 --- a/model/src/test/assets/sbt-multi-project-example-graph-old.yml +++ b/model/src/test/assets/sbt-multi-project-example-graph-old.yml @@ -1300,4 +1300,8 @@ analyzer: scanner: null advisor: null evaluator: null -resolved_configuration: {} +resolved_configuration: + package_curations: + providers: + - "DefaultDir" + data: {} diff --git a/model/src/test/assets/sbt-multi-project-example-graph.yml b/model/src/test/assets/sbt-multi-project-example-graph.yml index 647a4a6b88fd6..262010236a70a 100644 --- a/model/src/test/assets/sbt-multi-project-example-graph.yml +++ b/model/src/test/assets/sbt-multi-project-example-graph.yml @@ -1329,4 +1329,8 @@ analyzer: scanner: null advisor: null evaluator: null -resolved_configuration: {} +resolved_configuration: + package_curations: + providers: + - "DefaultDir" + data: {} diff --git a/reporter/src/funTest/assets/gradle-all-dependencies-result.yml b/reporter/src/funTest/assets/gradle-all-dependencies-result.yml index e61ec84ed901a..6b131da2b85e0 100644 --- a/reporter/src/funTest/assets/gradle-all-dependencies-result.yml +++ b/reporter/src/funTest/assets/gradle-all-dependencies-result.yml @@ -536,4 +536,8 @@ analyzer: scanner: null advisor: null evaluator: null -resolved_configuration: {} +resolved_configuration: + package_curations: + providers: + - "DefaultDir" + data: {} diff --git a/reporter/src/funTest/assets/static-html-reporter-test-input.yml b/reporter/src/funTest/assets/static-html-reporter-test-input.yml index db6e0a1436776..4cc50b2201a4a 100644 --- a/reporter/src/funTest/assets/static-html-reporter-test-input.yml +++ b/reporter/src/funTest/assets/static-html-reporter-test-input.yml @@ -639,14 +639,18 @@ evaluator: \ that overflow:scroll is working as expected.\n```" resolved_configuration: package_curations: - - id: "Maven:com.foobar:foobar:1.0" - curations: - comment: "Foobar is an imaginary dependency and offers a license choice" - concluded_license: "GPL-2.0-only OR MIT" - - id: "Maven:com.h2database:h2:1.4.200" - curations: - comment: "H2 database offers a license choice" - concluded_license: "MPL-2.0 OR EPL-1.0" + providers: + - "DefaultDir" + data: + DefaultDir: + - id: "Maven:com.foobar:foobar:1.0" + curations: + comment: "Foobar is an imaginary dependency and offers a license choice" + concluded_license: "GPL-2.0-only OR MIT" + - id: "Maven:com.h2database:h2:1.4.200" + curations: + comment: "H2 database offers a license choice" + concluded_license: "MPL-2.0 OR EPL-1.0" labels: job_parameters.JOB_PARAM_1: "label job param 1" job_parameters.JOB_PARAM_2: "label job param 2" diff --git a/scanner/src/funTest/assets/analyzer-result.yml b/scanner/src/funTest/assets/analyzer-result.yml index c470e7b827480..f23057885ea7b 100644 --- a/scanner/src/funTest/assets/analyzer-result.yml +++ b/scanner/src/funTest/assets/analyzer-result.yml @@ -184,4 +184,8 @@ analyzer: scanner: null advisor: null evaluator: null -resolved_configuration: {} +resolved_configuration: + package_curations: + providers: + - "DefaultDir" + data: {} diff --git a/scanner/src/funTest/assets/dummy-expected-output-for-analyzer-result.yml b/scanner/src/funTest/assets/dummy-expected-output-for-analyzer-result.yml index f607210ef6c89..5923cef4b8144 100644 --- a/scanner/src/funTest/assets/dummy-expected-output-for-analyzer-result.yml +++ b/scanner/src/funTest/assets/dummy-expected-output-for-analyzer-result.yml @@ -369,4 +369,8 @@ scanner: has_issues: true advisor: null evaluator: null -resolved_configuration: {} +resolved_configuration: + package_curations: + providers: + - "DefaultDir" + data: {}