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

refactor(model)!: Simplify the CuratedPackage and its creation #9679

Merged
merged 1 commit into from
Jan 2, 2025
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
4 changes: 2 additions & 2 deletions model/src/main/kotlin/CuratedPackage.kt
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
package org.ossreviewtoolkit.model

/**
* A [Package] including the [PackageCurationResult]s that were applied to it, in order to be able to trace back how the
* A [Package] including the [PackageCurationData] that was applied to it, in order to be able to trace back how the
* original metadata of the package was modified by applying [PackageCuration]s.
*/
data class CuratedPackage(
Expand All @@ -32,5 +32,5 @@ data class CuratedPackage(
/**
* The curations in the order they were applied.
*/
val curations: List<PackageCurationResult> = emptyList()
val curations: List<PackageCurationData> = emptyList()
)
23 changes: 0 additions & 23 deletions model/src/main/kotlin/Package.kt
Original file line number Diff line number Diff line change
Expand Up @@ -164,29 +164,6 @@ data class Package(
sourceCodeOrigins?.requireNotEmptyNoDuplicates()
}

/**
* Compares this package with [other] and creates a [PackageCurationData] containing the values from this package
* which are different in [other]. All equal values are set to null. Only the fields present in
* [PackageCurationData] are compared.
*/
fun diff(other: Package): PackageCurationData {
require(id == other.id) {
"Cannot diff packages with different ids: '${id.toCoordinates()}' vs. '${other.id.toCoordinates()}'"
}

return PackageCurationData(
authors = authors.takeIf { it != other.authors },
description = description.takeIf { it != other.description },
homepageUrl = homepageUrl.takeIf { it != other.homepageUrl },
binaryArtifact = binaryArtifact.takeIf { it != other.binaryArtifact },
sourceArtifact = sourceArtifact.takeIf { it != other.sourceArtifact },
vcs = vcsProcessed.takeIf { it != other.vcsProcessed }?.toCuration(),
isMetadataOnly = isMetadataOnly.takeIf { it != other.isMetadataOnly },
isModified = isModified.takeIf { it != other.isModified },
sourceCodeOrigins = sourceCodeOrigins.takeIf { it != other.sourceCodeOrigins }
)
}

/**
* Create a [CuratedPackage] from this package with an empty list of applied curations.
*/
Expand Down
16 changes: 2 additions & 14 deletions model/src/main/kotlin/PackageCurationData.kt
Original file line number Diff line number Diff line change
Expand Up @@ -159,19 +159,7 @@ data class PackageCurationData(
sourceCodeOrigins = sourceCodeOrigins ?: original.sourceCodeOrigins
)

val declaredLicenseMappingDiff = buildMap {
val previous = targetPackage.getDeclaredLicenseMapping().toList()
val current = declaredLicenseMapping.toList()

putAll(previous - current)
}

val curations = targetPackage.curations + PackageCurationResult(
base = original.diff(pkg).copy(declaredLicenseMapping = declaredLicenseMappingDiff),
curation = this
)

return CuratedPackage(pkg, curations)
return CuratedPackage(pkg, targetPackage.curations + this)
}

/**
Expand Down Expand Up @@ -203,5 +191,5 @@ data class PackageCurationData(

private fun CuratedPackage.getDeclaredLicenseMapping(): Map<String, SpdxExpression> =
buildMap {
curations.forEach { putAll(it.curation.declaredLicenseMapping) }
curations.forEach { putAll(it.declaredLicenseMapping) }
}
35 changes: 0 additions & 35 deletions model/src/main/kotlin/PackageCurationResult.kt

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ class DefaultLicenseInfoProvider(val ortResult: OrtResult) : LicenseInfoProvider
ortResult.getPackage(id)?.let { (pkg, curations) ->
ConcludedLicenseInfo(
concludedLicense = pkg.concludedLicense,
appliedCurations = curations.filter { it.curation.concludedLicense != null }
appliedCurations = curations.filter { it.concludedLicense != null }
)
} ?: ConcludedLicenseInfo(concludedLicense = null, appliedCurations = emptyList())

Expand All @@ -67,7 +67,7 @@ class DefaultLicenseInfoProvider(val ortResult: OrtResult) : LicenseInfoProvider
authors = pkg.authors,
licenses = pkg.declaredLicenses,
processed = pkg.declaredLicensesProcessed,
appliedCurations = curations.filter { it.curation.declaredLicenseMapping.isNotEmpty() }
appliedCurations = curations.filter { it.declaredLicenseMapping.isNotEmpty() }
)
} ?: DeclaredLicenseInfo(
authors = emptySet(),
Expand Down
10 changes: 5 additions & 5 deletions model/src/main/kotlin/licenses/LicenseInfo.kt
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ package org.ossreviewtoolkit.model.licenses
import org.ossreviewtoolkit.model.CopyrightFinding
import org.ossreviewtoolkit.model.Identifier
import org.ossreviewtoolkit.model.LicenseFinding
import org.ossreviewtoolkit.model.PackageCurationResult
import org.ossreviewtoolkit.model.PackageCurationData
import org.ossreviewtoolkit.model.Provenance
import org.ossreviewtoolkit.model.Repository
import org.ossreviewtoolkit.model.config.LicenseFindingCuration
Expand Down Expand Up @@ -66,9 +66,9 @@ data class ConcludedLicenseInfo(
val concludedLicense: SpdxExpression?,

/**
* The list of [PackageCurationResult]s that modified the concluded license.
* The list of [PackageCurationData] that modified the concluded license.
*/
val appliedCurations: List<PackageCurationResult>
val appliedCurations: List<PackageCurationData>
)

/**
Expand All @@ -91,9 +91,9 @@ data class DeclaredLicenseInfo(
val processed: ProcessedDeclaredLicense,

/**
* The list of [PackageCurationResult]s that modified the declared license.
* The list of [PackageCurationData] that modified the declared license.
*/
val appliedCurations: List<PackageCurationResult>
val appliedCurations: List<PackageCurationData>
)

/**
Expand Down
33 changes: 10 additions & 23 deletions model/src/test/kotlin/PackageCurationTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,8 @@ import io.kotest.assertions.assertSoftly
import io.kotest.assertions.throwables.shouldThrow
import io.kotest.core.spec.style.WordSpec
import io.kotest.matchers.collections.containExactly
import io.kotest.matchers.collections.shouldContainExactly
import io.kotest.matchers.collections.shouldHaveSize
import io.kotest.matchers.maps.beEmpty
import io.kotest.matchers.maps.shouldContainExactly
import io.kotest.matchers.should
import io.kotest.matchers.shouldBe

Expand Down Expand Up @@ -89,8 +88,7 @@ class PackageCurationTest : WordSpec({
}

curatedPkg.curations shouldHaveSize 1
curatedPkg.curations.first().base shouldBe pkg.diff(curatedPkg.metadata)
curatedPkg.curations.first().curation shouldBe curation.data
curatedPkg.curations.first() shouldBe curation.data
}

"change only curated fields" {
Expand Down Expand Up @@ -142,8 +140,7 @@ class PackageCurationTest : WordSpec({
}

curatedPkg.curations shouldHaveSize 1
curatedPkg.curations.first().base shouldBe pkg.diff(curatedPkg.metadata)
curatedPkg.curations.first().curation shouldBe curation.data
curatedPkg.curations.first() shouldBe curation.data
}

"be able to empty VCS information" {
Expand Down Expand Up @@ -283,24 +280,18 @@ class PackageCurationTest : WordSpec({

result1.metadata.description shouldBe "description 1"
result1.curations shouldHaveSize 1
result1.curations[0].base shouldBe PackageCurationData(description = "")
result1.curations[0].curation shouldBe curation1.data
result1.curations[0] shouldBe curation1.data

result2.metadata.description shouldBe "description 2"
result2.curations shouldHaveSize 2
result2.curations[0].base shouldBe PackageCurationData(description = "")
result2.curations[0].curation shouldBe curation1.data
result2.curations[1].base shouldBe PackageCurationData(description = "description 1")
result2.curations[1].curation shouldBe curation2.data
result2.curations[0] shouldBe curation1.data
result2.curations[1] shouldBe curation2.data

result3.metadata.description shouldBe "description 3"
result3.curations shouldHaveSize 3
result3.curations[0].base shouldBe PackageCurationData(description = "")
result3.curations[0].curation shouldBe curation1.data
result3.curations[1].base shouldBe PackageCurationData(description = "description 1")
result3.curations[1].curation shouldBe curation2.data
result3.curations[2].base shouldBe PackageCurationData(description = "description 2")
result3.curations[2].curation shouldBe curation3.data
result3.curations[0] shouldBe curation1.data
result3.curations[1] shouldBe curation2.data
result3.curations[2] shouldBe curation3.data
}
}

Expand All @@ -326,11 +317,7 @@ class PackageCurationTest : WordSpec({
result3.metadata.declaredLicensesProcessed.spdxExpression shouldBe
"Apache-2.0 AND BSD-3-Clause AND CC-BY-2.0".toSpdx()

result3.curations[0].base.declaredLicenseMapping should beEmpty()
result3.curations[1].base.declaredLicenseMapping should beEmpty()
result3.curations[2].base.declaredLicenseMapping.shouldContainExactly(
mapOf("license c" to "CC-BY-1.0".toSpdx())
)
result3.curations shouldContainExactly listOf(curation1, curation2, curation3).map { it.data }
}
}

Expand Down
119 changes: 0 additions & 119 deletions model/src/test/kotlin/PackageTest.kt

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -692,10 +692,8 @@ packages:
revision: ""
path: ""
curations:
- base: {}
curation:
comment: "Foobar is an imaginary dependency and offers a license choice"
concluded_license: "GPL-2.0-only OR MIT"
- comment: "Foobar is an imaginary dependency and offers a license choice"
concluded_license: "GPL-2.0-only OR MIT"
paths:
- 3
levels:
Expand Down Expand Up @@ -744,10 +742,8 @@ packages:
revision: ""
path: ""
curations:
- base: {}
curation:
comment: "H2 database offers a license choice"
concluded_license: "MPL-2.0 OR EPL-1.0"
- comment: "H2 database offers a license choice"
concluded_license: "MPL-2.0 OR EPL-1.0"
paths:
- 4
levels:
Expand Down
Loading
Loading