Skip to content

Commit

Permalink
feat(OrtResult)!: Associate package curations with provider IDs
Browse files Browse the repository at this point in the history
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] #6188 (comment)
[^2]: #5668

Signed-off-by: Frank Viernau <frank_viernau@epam.com>
  • Loading branch information
fviernau committed Feb 8, 2023
1 parent 8d1bcbe commit 51c78e0
Show file tree
Hide file tree
Showing 21 changed files with 201 additions and 50 deletions.
6 changes: 5 additions & 1 deletion advisor/src/test/assets/ort-analyzer-result.yml
Original file line number Diff line number Diff line change
Expand Up @@ -571,4 +571,8 @@ analyzer:
scanner: null
advisor: null
evaluator: null
resolved_configuration: {}
resolved_configuration:
package_curations:
providers:
- "DefaultDir"
data: {}
Original file line number Diff line number Diff line change
Expand Up @@ -4120,4 +4120,8 @@ analyzer:
scanner: null
advisor: null
evaluator: null
resolved_configuration: {}
resolved_configuration:
package_curations:
providers:
- "DefaultDir"
data: {}
Original file line number Diff line number Diff line change
Expand Up @@ -1295,4 +1295,8 @@ analyzer:
scanner: null
advisor: null
evaluator: null
resolved_configuration: {}
resolved_configuration:
package_curations:
providers:
- "DefaultDir"
data: {}
Original file line number Diff line number Diff line change
Expand Up @@ -788,4 +788,8 @@ analyzer:
scanner: null
advisor: null
evaluator: null
resolved_configuration: {}
resolved_configuration:
package_curations:
providers:
- "DefaultDir"
data: {}
Original file line number Diff line number Diff line change
Expand Up @@ -1721,4 +1721,8 @@ analyzer:
scanner: null
advisor: null
evaluator: null
resolved_configuration: {}
resolved_configuration:
package_curations:
providers:
- "DefaultDir"
data: {}
12 changes: 9 additions & 3 deletions analyzer/src/main/kotlin/Analyzer.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -357,17 +358,22 @@ private fun resolveConfiguration(
curationProviders: List<Pair<String, PackageCurationProvider>>
): ResolvedConfiguration {
val packageIds = analyzerResult.packages.mapTo(mutableSetOf()) { it.id }
val packageCurations = mutableListOf<PackageCuration>()
val packageCurations = mutableMapOf<String, List<PackageCuration>>()

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
)
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -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."
6 changes: 5 additions & 1 deletion cli/src/funTest/assets/semver4j-ort-result.yml
Original file line number Diff line number Diff line change
Expand Up @@ -345,4 +345,8 @@ advisor:
severity: "5.5"
has_issues: false
evaluator: null
resolved_configuration: {}
resolved_configuration:
package_curations:
providers:
- "DefaultDir"
data: {}
11 changes: 9 additions & 2 deletions model/src/main/kotlin/OrtResult.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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)
)
)
)
}
Expand Down
76 changes: 71 additions & 5 deletions model/src/main/kotlin/ResolvedConfiguration.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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<PackageCuration> =
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<PackageCuration> = emptyList()
)
val providers: List<String> = emptyList(),

/**
* All package curations applicable to the packages contained in the enclosing [OrtResult].
*/
val data: Map<String, List<PackageCuration>> = 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<PackageCurations, PackageCurations>() {
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<Any>.joinToStringSingleQuoted() = joinToString(prefix = "'", separator = "','", postfix = "'")
22 changes: 13 additions & 9 deletions model/src/test/assets/analyzer-result-with-dependency-graph.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Original file line number Diff line number Diff line change
Expand Up @@ -459,4 +459,8 @@ analyzer:
scanner: null
advisor: null
evaluator: null
resolved_configuration: {}
resolved_configuration:
package_curations:
providers:
- "DefaultDir"
data: {}
6 changes: 5 additions & 1 deletion model/src/test/assets/result-with-issues-graph-old.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1310,4 +1310,8 @@ analyzer:
scanner: null
advisor: null
evaluator: null
resolved_configuration: {}
resolved_configuration:
package_curations:
providers:
- "DefaultDir"
data: {}
6 changes: 5 additions & 1 deletion model/src/test/assets/result-with-issues-graph.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1339,4 +1339,8 @@ analyzer:
scanner: null
advisor: null
evaluator: null
resolved_configuration: {}
resolved_configuration:
package_curations:
providers:
- "DefaultDir"
data: {}
6 changes: 5 additions & 1 deletion model/src/test/assets/result-with-issues-scopes.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1306,4 +1306,8 @@ analyzer:
scanner: null
advisor: null
evaluator: null
resolved_configuration: {}
resolved_configuration:
package_curations:
providers:
- "DefaultDir"
data: {}
Original file line number Diff line number Diff line change
Expand Up @@ -1300,4 +1300,8 @@ analyzer:
scanner: null
advisor: null
evaluator: null
resolved_configuration: {}
resolved_configuration:
package_curations:
providers:
- "DefaultDir"
data: {}
6 changes: 5 additions & 1 deletion model/src/test/assets/sbt-multi-project-example-graph.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1329,4 +1329,8 @@ analyzer:
scanner: null
advisor: null
evaluator: null
resolved_configuration: {}
resolved_configuration:
package_curations:
providers:
- "DefaultDir"
data: {}
Original file line number Diff line number Diff line change
Expand Up @@ -536,4 +536,8 @@ analyzer:
scanner: null
advisor: null
evaluator: null
resolved_configuration: {}
resolved_configuration:
package_curations:
providers:
- "DefaultDir"
data: {}
20 changes: 12 additions & 8 deletions reporter/src/funTest/assets/static-html-reporter-test-input.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
6 changes: 5 additions & 1 deletion scanner/src/funTest/assets/analyzer-result.yml
Original file line number Diff line number Diff line change
Expand Up @@ -184,4 +184,8 @@ analyzer:
scanner: null
advisor: null
evaluator: null
resolved_configuration: {}
resolved_configuration:
package_curations:
providers:
- "DefaultDir"
data: {}
Original file line number Diff line number Diff line change
Expand Up @@ -369,4 +369,8 @@ scanner:
has_issues: true
advisor: null
evaluator: null
resolved_configuration: {}
resolved_configuration:
package_curations:
providers:
- "DefaultDir"
data: {}

0 comments on commit 51c78e0

Please sign in to comment.