From 4de3dbbf2aff40a6bad7ec7105cb4fe77ca19117 Mon Sep 17 00:00:00 2001 From: alexzurbonsen Date: Tue, 19 Nov 2024 19:37:22 +0100 Subject: [PATCH 1/2] chore(opossum reporter): migrate serialization to kotlinx Migrates the OpossumReporter plugin away from Jackson serialization to use kotlinx serialization. To get properly typed data structures for serialization the previous untyped Map<*, *> are substituted by properly typed data classes. This leads to the creation of an OpossumInputCreator class that holds intermediate data structures needed for processing of the Reporter input. Signed-off-by: alexzurbonsen --- plugins/reporters/opossum/build.gradle.kts | 7 +- .../funTest/kotlin/OpossumReporterFunTest.kt | 34 +- .../src/main/kotlin/OpossumReporter.kt | 359 +++++++++++------- .../src/test/kotlin/OpossumReporterTest.kt | 74 ++-- 4 files changed, 278 insertions(+), 196 deletions(-) diff --git a/plugins/reporters/opossum/build.gradle.kts b/plugins/reporters/opossum/build.gradle.kts index d62c71a1f0a53..6a3d6ed117033 100644 --- a/plugins/reporters/opossum/build.gradle.kts +++ b/plugins/reporters/opossum/build.gradle.kts @@ -20,6 +20,9 @@ plugins { // Apply precompiled plugins. id("ort-plugin-conventions") + + // Apply third-party plugins. + alias(libs.plugins.kotlinSerialization) } dependencies { @@ -33,6 +36,6 @@ dependencies { implementation(projects.utils.ortUtils) implementation(projects.utils.spdxUtils) - implementation(libs.jackson.annotations) - implementation(libs.jackson.databind) + implementation(libs.kotlinx.serialization.core) + implementation(libs.kotlinx.serialization.json) } diff --git a/plugins/reporters/opossum/src/funTest/kotlin/OpossumReporterFunTest.kt b/plugins/reporters/opossum/src/funTest/kotlin/OpossumReporterFunTest.kt index ea7f36c05b6b1..ec4ab47fddf2b 100644 --- a/plugins/reporters/opossum/src/funTest/kotlin/OpossumReporterFunTest.kt +++ b/plugins/reporters/opossum/src/funTest/kotlin/OpossumReporterFunTest.kt @@ -19,14 +19,17 @@ package org.ossreviewtoolkit.plugins.reporters.opossum -import com.fasterxml.jackson.databind.json.JsonMapper +import kotlinx.serialization.json.Json import io.kotest.core.TestConfiguration import io.kotest.core.spec.style.WordSpec import io.kotest.engine.spec.tempdir -import io.kotest.matchers.sequences.shouldContain +import io.kotest.matchers.collections.shouldContain import io.kotest.matchers.shouldBe import io.kotest.matchers.string.shouldContain +import kotlinx.serialization.json.jsonArray +import kotlinx.serialization.json.jsonObject +import kotlinx.serialization.json.jsonPrimitive import org.ossreviewtoolkit.model.OrtResult import org.ossreviewtoolkit.plugins.api.PluginConfig @@ -45,15 +48,24 @@ class OpossumReporterFunTest : WordSpec({ } "create a parseable result and contain some expected values" { - with(JsonMapper().readTree(reportStr)) { - isObject shouldBe true - get("metadata").get("projectId").asText() shouldBe "0" - get("attributionBreakpoints").size() shouldBe 4 - get("externalAttributionSources").size() shouldBe 7 - get("resourcesToAttributions").fieldNames().asSequence() shouldContain - "/analyzer/src/funTest/assets/projects/synthetic/gradle/lib/build.gradle/" + - "compile/org.apache.commons/commons-text@1.1/dependencies/org.apache.commons/commons-lang3@3.5" - } + val json = Json.parseToJsonElement(reportStr).jsonObject + + json["metadata"]?.jsonObject?.get("projectId")?.jsonPrimitive?.content shouldBe "0" + json["resources"]?.jsonObject?.size shouldBe 2 + json["attributionBreakpoints"]?.jsonArray?.size shouldBe 4 + json["externalAttributionSources"]?.jsonObject?.size shouldBe 7 + json["frequentLicenses"]?.jsonArray?.size shouldBe 666 + json["resourcesToAttributions"]?.jsonObject?.size shouldBe 13 + + val resourceRoots = json["resources"]?.jsonObject?.keys ?: emptySet() + resourceRoots shouldBe setOf("analyzer", "project") + val projectResource = json["resources"]?.jsonObject?.get("project") + projectResource?.jsonObject?.size shouldBe 5 + projectResource?.jsonObject?.get("file.kt")?.jsonPrimitive?.content shouldBe "1" + + val resourcePaths = json["resourcesToAttributions"]?.jsonObject?.keys ?: emptySet() + resourcePaths shouldContain "/analyzer/src/funTest/assets/projects/synthetic/gradle/lib/build.gradle/" + + "compile/org.apache.commons/commons-text@1.1/dependencies/org.apache.commons/commons-lang3@3.5" } } }) diff --git a/plugins/reporters/opossum/src/main/kotlin/OpossumReporter.kt b/plugins/reporters/opossum/src/main/kotlin/OpossumReporter.kt index 6206c5e7aa22b..1ce4992e8f8c8 100644 --- a/plugins/reporters/opossum/src/main/kotlin/OpossumReporter.kt +++ b/plugins/reporters/opossum/src/main/kotlin/OpossumReporter.kt @@ -19,13 +19,21 @@ package org.ossreviewtoolkit.plugins.reporters.opossum -import com.fasterxml.jackson.annotation.JsonInclude.Include -import com.fasterxml.jackson.databind.json.JsonMapper +import kotlinx.serialization.KSerializer +import kotlinx.serialization.builtins.MapSerializer +import kotlinx.serialization.builtins.serializer +import kotlinx.serialization.encoding.Decoder +import kotlinx.serialization.encoding.Encoder +import kotlinx.serialization.Serializable +import kotlinx.serialization.Transient +import kotlinx.serialization.descriptors.PrimitiveKind +import kotlinx.serialization.descriptors.PrimitiveSerialDescriptor +import kotlinx.serialization.descriptors.SerialDescriptor +import kotlinx.serialization.descriptors.buildClassSerialDescriptor +import kotlinx.serialization.json.Json import java.io.File import java.time.LocalDateTime -import java.util.SortedMap -import java.util.SortedSet import java.util.UUID import kotlin.math.min @@ -96,50 +104,78 @@ class OpossumReporter( override val descriptor: PluginDescriptor = OpossumReporterFactory.descriptor, private val config: OpossumReporterConfig ) : Reporter { + @Serializable internal data class OpossumSignal( - val source: String, - val id: Identifier? = null, - val url: String? = null, - val license: SpdxExpression? = null, - val copyright: String? = null, - val comment: String? = null, - val preselected: Boolean = false, - val followUp: Boolean = false, - val excludeFromNotice: Boolean = false, - val uuid: UUID = UUID.randomUUID() + @Transient + val uuid: UUID = UUID.randomUUID(), + val source: OpossumSignalSource, + val attributionConfidence: Int = 80, + val packageType: String?, + val packageNamespace: String?, + val packageName: String?, + val packageVersion: String?, + val copyright: String?, + val licenseName: String?, + val url: String?, + val preSelected: Boolean, + val followUp: OpossumFollowUp?, + val excludeFromNotice: Boolean, + val comment: String? ) { - fun toJson(): Map<*, *> = - sortedMapOf( - uuid.toString() to sortedMapOf( - "source" to sortedMapOf( - "name" to source, - "documentConfidence" to 80 - ), - "attributionConfidence" to 80, - "packageType" to id?.getPurlType(), - "packageNamespace" to id?.namespace, - "packageName" to id?.name, - "packageVersion" to id?.version, - "copyright" to copyright, - "licenseName" to license?.toString(), - "url" to url, - "preSelected" to preselected, - "followUp" to "FOLLOW_UP".takeIf { followUp }, - "excludeFromNotice" to excludeFromNotice, - "comment" to comment + companion object { + fun create( + source: String, + id: Identifier? = null, + url: String? = null, + license: SpdxExpression? = null, + copyright: String? = null, + comment: String? = null, + preSelected: Boolean = false, + followUp: Boolean = false, + excludeFromNotice: Boolean = false + ): OpossumSignal { + return OpossumSignal( + source = OpossumSignalSource(name = source), + packageType = id?.getPurlType(), + packageNamespace = id?.namespace, + packageName = id?.name, + packageVersion = id?.version, + copyright = copyright, + licenseName = license?.toString(), + url = url, + preSelected = preSelected, + followUp = OpossumFollowUp.FOLLOW_UP.takeIf { followUp }, + excludeFromNotice = excludeFromNotice, + comment = comment, ) - ) + } + } fun matches(other: OpossumSignal): Boolean = source == other.source - && id == other.id - && url == other.url - && license == other.license + && packageType == other.packageType + && packageNamespace == other.packageNamespace + && packageName == other.packageName + && packageVersion == other.packageVersion && copyright == other.copyright + && licenseName == other.licenseName + && url == other.url + && preSelected == other.preSelected && comment == other.comment - && preselected == other.preselected } + @Serializable + internal data class OpossumSignalSource( + val name: String, + val documentConfidence: Int = 80, + ) + + @Serializable + internal enum class OpossumFollowUp { + FOLLOW_UP, + } + + @Serializable(with = OpossumResourcesSerializer::class) internal data class OpossumResources( val tree: MutableMap = mutableMapOf() ) { @@ -176,26 +212,18 @@ class OpossumReporter( return head !in tree || tree.getValue(head).isPathAFile(tail) } - fun toJson(): Map<*, *> = tree.mapValues { (_, v) -> if (v.isFile()) 1 else v.toJson() } - fun toFileList(): Set = tree.flatMapTo(mutableSetOf()) { (key, value) -> value.toFileList().map { resolvePath(key, it, isDirectory = false) } }.plus("/") } + @Serializable internal data class OpossumFrequentLicense( val shortName: String, val fullName: String?, val defaultText: String? ) : Comparable { - fun toJson(): Map<*, *> = - sortedMapOf( - "shortName" to shortName, - "fullName" to fullName, - "defaultText" to defaultText - ) - override fun compareTo(other: OpossumFrequentLicense) = compareValuesBy( this, @@ -206,46 +234,88 @@ class OpossumReporter( ) } + @Serializable internal data class OpossumExternalAttributionSource( val name: String, val priority: Int ) + @Serializable internal data class OpossumInput( - val resources: OpossumResources = OpossumResources(), - val signals: MutableList = mutableListOf(), - val pathToSignal: SortedMap> = sortedMapOf(), - val packageToRoot: SortedMap> = sortedMapOf(), - val attributionBreakpoints: SortedSet = sortedSetOf(), - val filesWithChildren: SortedSet = sortedSetOf(), - val frequentLicenses: SortedSet = sortedSetOf(), - val baseUrlsForSources: SortedMap = sortedMapOf(), - val externalAttributionSources: SortedMap = sortedMapOf() + val metadata: OpossumInputMetadata = OpossumInputMetadata(), + val resources: OpossumResources, + val externalAttributions: Map<@Serializable(UUIDSerializer::class) UUID, OpossumSignal>, + val resourcesToAttributions: Map>, + val attributionBreakpoints: Set, + val filesWithChildren: Set, + val frequentLicenses: Set, + val baseUrlsForSources: Map, + val externalAttributionSources: Map ) { - fun toJson(): Map<*, *> = - sortedMapOf( - "metadata" to sortedMapOf( - "projectId" to "0", - "fileCreationDate" to LocalDateTime.now().toString() - ), - "resources" to resources.toJson(), - "externalAttributions" to signals - .map { it.toJson() } - .flatMap { it.toList() } - .toMap(), - "resourcesToAttributions" to pathToSignal.mapKeys { - val trailingSlash = if (resources.isPathAFile(it.key)) "" else "/" - resolvePath("${it.key}$trailingSlash") - }, - "attributionBreakpoints" to attributionBreakpoints, - "filesWithChildren" to filesWithChildren, - "frequentLicenses" to frequentLicenses.map { it.toJson() }, - "baseUrlsForSources" to baseUrlsForSources, - "externalAttributionSources" to externalAttributionSources - ) + internal fun getSignalsForFile(file: String): List = + resourcesToAttributions[file].orEmpty().mapNotNull { uuid -> externalAttributions[uuid] } + } + + internal class OpossumInputCreator { + private val resources: OpossumResources = OpossumResources() + private val signals: MutableList = mutableListOf() + private val pathToSignals: MutableMap> = mutableMapOf() + private val packageToRoot: MutableMap> = mutableMapOf() + private val attributionBreakpoints: MutableSet = mutableSetOf() + private val filesWithChildren: MutableSet = mutableSetOf() + private val frequentLicenses: MutableSet = mutableSetOf() + private val baseUrlsForSources: MutableMap = mutableMapOf() + private val externalAttributionSources: MutableMap = mutableMapOf() + + internal fun create(input: ReporterInput, maxDepth: Int = Int.MAX_VALUE): OpossumInput { + addBaseUrl("/", input.ortResult.repository.vcs) + + SpdxLicense.entries.forEach { + val licenseText = input.licenseTextProvider.getLicenseText(it.id) + frequentLicenses += OpossumFrequentLicense(it.id, it.fullName, licenseText) + } + + input.ortResult.getProjects().forEach { project -> + addProject(project, input.ortResult) + } + + if (input.ortResult.getExcludes().scopes.isEmpty()) { + addPackagesThatAreRootless(input.ortResult.getPackages()) + } - fun getSignalsForFile(file: String): List = - pathToSignal[file].orEmpty().mapNotNull { uuid -> signals.find { it.uuid == uuid } } + input.ortResult.analyzer?.result?.issues.orEmpty().forEach { (id, issues) -> + issues.forEach { issue -> + val source = addExternalAttributionSource( + key = "ORT-Analyzer-Issues", + name = "ORT-Analyzer Issues", + priority = ISSUE_PRIORITY + ) + + addIssue(issue, id, source) + } + } + + input.ortResult.getScanResults().forEach { (id, results) -> + addScannerResults(id, results, maxDepth) + } + + val externalAttributions = signals.associateBy { it.uuid } + val resourcesToAttributions = pathToSignals.mapKeys { + val trailingSlash = if (resources.isPathAFile(it.key)) "" else "/" + resolvePath("${it.key}$trailingSlash") + } + + return OpossumInput( + resources = resources, + externalAttributions = externalAttributions, + resourcesToAttributions = resourcesToAttributions, + attributionBreakpoints = attributionBreakpoints, + filesWithChildren = filesWithChildren, + baseUrlsForSources = baseUrlsForSources, + externalAttributionSources = externalAttributionSources, + frequentLicenses = frequentLicenses + ) + } private fun addAttributionBreakpoint(breakpoint: String) { attributionBreakpoints += resolvePath(breakpoint, isDirectory = true) @@ -255,7 +325,7 @@ class OpossumReporter( filesWithChildren += resolvePath(fileWithChildren, isDirectory = true) } - fun addBaseUrl(path: String, vcs: VcsInfo) { + private fun addBaseUrl(path: String, vcs: VcsInfo) { val idFromPath = resolvePath(path, isDirectory = true) if (idFromPath in baseUrlsForSources) return @@ -271,7 +341,7 @@ class OpossumReporter( } } - fun addExternalAttributionSource(key: String, name: String, priority: Int): String { + private fun addExternalAttributionSource(key: String, name: String, priority: Int): String { if (key !in externalAttributionSources) { externalAttributionSources[key] = OpossumExternalAttributionSource(name, priority) } @@ -280,7 +350,7 @@ class OpossumReporter( } private fun addPackageRoot(id: Identifier, path: String, level: Int = 0, vcs: VcsInfo = VcsInfo.EMPTY) { - val mapOfId = packageToRoot.getOrPut(id) { sortedMapOf() } + val mapOfId = packageToRoot.getOrPut(id) { mutableMapOf() } val oldLevel = mapOfId.getOrDefault(path, level) mapOfId[path] = min(level, oldLevel) @@ -288,7 +358,7 @@ class OpossumReporter( addBaseUrl(path, vcs) } - private fun addSignal(signal: OpossumSignal, paths: SortedSet) { + private fun addSignal(signal: OpossumSignal, paths: Set) { if (paths.isEmpty()) return val matchingSignal = signals.find { it.matches(signal) } @@ -301,21 +371,24 @@ class OpossumReporter( } paths.forEach { path -> - logger.debug { "add signal ${signal.id} of source ${signal.source} to $path" } + logger.debug { + "add signal ${signal.packageName} ${signal.packageVersion} with namespace " + + "${signal.packageNamespace} of of source ${signal.source} to $path" + } + resources.addResource(path) - pathToSignal.getOrPut(resolvePath(path)) { sortedSetOf() } += uuidOfSignal + pathToSignals.getOrPut(resolvePath(path)) { mutableSetOf() } += uuidOfSignal } } private fun signalFromPkg(pkg: Package): OpossumSignal { val source = addExternalAttributionSource("ORT-Package", "ORT-Package", 180) - - return OpossumSignal( + return OpossumSignal.create( source, id = pkg.id, url = pkg.homepageUrl, license = pkg.concludedLicense ?: pkg.declaredLicensesProcessed.spdxExpression, - preselected = true + preSelected = true ) } @@ -330,7 +403,7 @@ class OpossumReporter( ?: Package.EMPTY.copy(id = dependencyId) addPackageRoot(dependencyId, dependencyPath, level, dependencyPackage.vcsProcessed) - addSignal(signalFromPkg(dependencyPackage), sortedSetOf(dependencyPath)) + addSignal(signalFromPkg(dependencyPackage), setOf(dependencyPath)) val dependencies = dependency.getDependencies() @@ -352,7 +425,7 @@ class OpossumReporter( } } - fun addProject(project: Project, ortResult: OrtResult, relRoot: String = "/") { + private fun addProject(project: Project, ortResult: OrtResult, relRoot: String = "/") { val projectId = project.id val definitionFilePath = resolvePath(relRoot, project.definitionFilePath) logger.debug { "$definitionFilePath - $projectId - Project" } @@ -362,16 +435,16 @@ class OpossumReporter( addFileWithChildren(definitionFilePath) val source = addExternalAttributionSource("ORT-Project", "ORT-Project", 200) - val signalFromProject = OpossumSignal( + val signalFromProject = OpossumSignal.create( source, id = projectId, url = project.homepageUrl, license = project.declaredLicensesProcessed.spdxExpression, copyright = project.authors.joinToString(separator = "\n"), - preselected = true + preSelected = true ) - addSignal(signalFromProject, sortedSetOf(definitionFilePath)) + addSignal(signalFromProject, setOf(definitionFilePath)) val scopeNames = ortResult.dependencyNavigator.scopeNames(project).filterNot { ortResult.getExcludes().isScopeExcluded(it) @@ -424,7 +497,7 @@ class OpossumReporter( .distinct() .reduceRightOrNull { left, right -> left and right } - val pathSignal = OpossumSignal( + val pathSignal = OpossumSignal.create( source, copyright = copyright, license = license @@ -432,7 +505,7 @@ class OpossumReporter( addSignal( pathSignal, - rootsBelowMaxDepth.map { resolvePath(it, pathFromFinding) }.toSortedSet() + rootsBelowMaxDepth.map { resolvePath(it, pathFromFinding) }.toSet() ) } } @@ -449,12 +522,12 @@ class OpossumReporter( .distinct() .reduceRightOrNull { left, right -> left and right } - val rootSignal = OpossumSignal( + val rootSignal = OpossumSignal.create( source, copyright = copyright, license = license ) - addSignal(rootSignal, rootsAboveMaxDepth.toSortedSet()) + addSignal(rootSignal, rootsAboveMaxDepth.toSet()) } val issueSource = addExternalAttributionSource( @@ -466,11 +539,11 @@ class OpossumReporter( result.summary.issues.forEach { addIssue(it, id, issueSource) } } - fun addScannerResults(id: Identifier, results: List, maxDepth: Int) { + private fun addScannerResults(id: Identifier, results: List, maxDepth: Int) { results.forEach { addScannerResult(id, it, maxDepth) } } - fun addIssue(issue: Issue, id: Identifier, source: String) { + private fun addIssue(issue: Issue, id: Identifier, source: String) { val roots = packageToRoot[id] val paths = if (roots.isNullOrEmpty()) { @@ -481,16 +554,16 @@ class OpossumReporter( } val signal = - OpossumSignal(source, comment = issue.toString(), followUp = true, excludeFromNotice = true) - addSignal(signal, paths.map { resolvePath(it) }.toSortedSet()) + OpossumSignal.create(source, comment = issue.toString(), followUp = true, excludeFromNotice = true) + addSignal(signal, paths.map { resolvePath(it) }.toSet()) } - fun addPackagesThatAreRootless(analyzerResultPackages: Set) { + private fun addPackagesThatAreRootless(analyzerResultPackages: Set) { val rootlessPackages = analyzerResultPackages.filter { packageToRoot[it.metadata.id] == null } rootlessPackages.forEach { val path = resolvePath("/lost+found", it.metadata.id.toPurl()) - addSignal(signalFromPkg(it.metadata), sortedSetOf(path)) + addSignal(signalFromPkg(it.metadata), setOf(path)) addPackageRoot(it.metadata.id, path, Int.MAX_VALUE, it.metadata.vcsProcessed) } @@ -500,53 +573,31 @@ class OpossumReporter( } } + @Serializable + internal data class OpossumInputMetadata( + val projectId: String = "0", + val fileCreationDate: String = LocalDateTime.now().toString() + ) + private fun writeReport(outputFile: File, opossumInput: OpossumInput) { val jsonFile = createOrtTempDir().resolve("input.json") - JsonMapper().setSerializationInclusion(Include.NON_NULL).writeValue(jsonFile, opossumInput.toJson()) + val json = Json { + explicitNulls = false + encodeDefaults = true + } + jsonFile.writeText(json.encodeToString(OpossumInput.serializer(), opossumInput)) + jsonFile.packZip(outputFile) jsonFile.delete() } - internal fun generateOpossumInput(input: ReporterInput, maxDepth: Int = Int.MAX_VALUE): OpossumInput { - val opossumInput = OpossumInput() - - opossumInput.addBaseUrl("/", input.ortResult.repository.vcs) - - SpdxLicense.entries.forEach { - val licenseText = input.licenseTextProvider.getLicenseText(it.id) - opossumInput.frequentLicenses += OpossumFrequentLicense(it.id, it.fullName, licenseText) - } - - input.ortResult.getProjects().forEach { project -> - opossumInput.addProject(project, input.ortResult) - } - - if (input.ortResult.getExcludes().scopes.isEmpty()) { - opossumInput.addPackagesThatAreRootless(input.ortResult.getPackages()) - } - - input.ortResult.analyzer?.result?.issues.orEmpty().forEach { (id, issues) -> - issues.forEach { issue -> - val source = opossumInput.addExternalAttributionSource( - key = "ORT-Analyzer-Issues", - name = "ORT-Analyzer Issues", - priority = ISSUE_PRIORITY - ) - - opossumInput.addIssue(issue, id, source) - } - } - - input.ortResult.getScanResults().forEach { (id, results) -> - opossumInput.addScannerResults(id, results, maxDepth) - } - - return opossumInput + internal fun createOpossumInput(input: ReporterInput, maxDepth: Int = Int.MAX_VALUE): OpossumInput { + return OpossumInputCreator().create(input, maxDepth) } override fun generateReport(input: ReporterInput, outputDir: File): List> { val reportFileResult = runCatching { - val opossumInput = generateOpossumInput(input, config.maxDepth) + val opossumInput = createOpossumInput(input, config.maxDepth) outputDir.resolve("report.opossum").also { writeReport(it, opossumInput) @@ -563,3 +614,31 @@ private fun DependencyNode.getDependencies(): List = this += dependencyNodes.map { it.getStableReference() } } } + +private object UUIDSerializer : KSerializer { + override val descriptor = PrimitiveSerialDescriptor("UUID", PrimitiveKind.STRING) + + override fun serialize(encoder: Encoder, value: UUID) { + encoder.encodeString(value.toString()) + } + + override fun deserialize(decoder: Decoder): UUID { + return UUID.fromString(decoder.decodeString()) + } +} + +private object OpossumResourcesSerializer : KSerializer { + override val descriptor: SerialDescriptor = buildClassSerialDescriptor("Resource") + + override fun serialize(encoder: Encoder, value: OpossumReporter.OpossumResources) { + if (value.isFile()) { + encoder.encodeInt(1) + } else { + encoder.encodeSerializableValue(MapSerializer(String.serializer(), this), value.tree) + } + } + + override fun deserialize(decoder: Decoder): OpossumReporter.OpossumResources { + throw NotImplementedError("Deserialization of OpossumResources is not supported.") + } +} diff --git a/plugins/reporters/opossum/src/test/kotlin/OpossumReporterTest.kt b/plugins/reporters/opossum/src/test/kotlin/OpossumReporterTest.kt index c46ffeefdbb38..ed58fcfa6fad9 100644 --- a/plugins/reporters/opossum/src/test/kotlin/OpossumReporterTest.kt +++ b/plugins/reporters/opossum/src/test/kotlin/OpossumReporterTest.kt @@ -21,13 +21,11 @@ package org.ossreviewtoolkit.plugins.reporters.opossum import io.kotest.core.spec.style.WordSpec import io.kotest.inspectors.forAll -import io.kotest.matchers.collections.containExactlyInAnyOrder import io.kotest.matchers.collections.shouldContain import io.kotest.matchers.collections.shouldHaveSize import io.kotest.matchers.collections.shouldNotContain import io.kotest.matchers.nulls.beNull import io.kotest.matchers.nulls.shouldNotBeNull -import io.kotest.matchers.should import io.kotest.matchers.shouldBe import io.kotest.matchers.shouldNot import io.kotest.matchers.string.shouldContain @@ -101,18 +99,23 @@ class OpossumReporterTest : WordSpec({ } } - "generateOpossumInput()" should { + "createOpossumInput()" should { val result = createOrtResult() - val opossumInput = OpossumReporterFactory().create(PluginConfig()).generateOpossumInput(ReporterInput(result)) + val opossumInput = OpossumReporterFactory().create(PluginConfig()).createOpossumInput(ReporterInput(result)) - "create input that is somehow valid" { + "create input that has all expected top level entries" { opossumInput shouldNotBeNull { + metadata shouldNot beNull() resources shouldNot beNull() - signals shouldNot beNull() - pathToSignal shouldNot beNull() - packageToRoot shouldNot beNull() + externalAttributions shouldNot beNull() + resourcesToAttributions shouldNot beNull() attributionBreakpoints shouldNot beNull() + frequentLicenses shouldNot beNull() + filesWithChildren shouldNot beNull() + baseUrlsForSources shouldNot beNull() + externalAttributionSources shouldNot beNull() } + (opossumInput.metadata.projectId).toInt() shouldBe 0 } val fileList = opossumInput.resources.toFileList() @@ -130,22 +133,24 @@ class OpossumReporterTest : WordSpec({ } "create a file list that contains files from other lists" { - opossumInput.pathToSignal.keys.forAll { path -> fileList shouldContain resolvePath(path) } + opossumInput.resourcesToAttributions.keys.forAll { path -> + val pathWithoutTrailingSlash = if (path != "/") path.removeSuffix("/") else "/" + fileList shouldContain resolvePath(pathWithoutTrailingSlash) + } opossumInput.attributionBreakpoints.map { it.replace(Regex("/$"), "") }.forAll { path -> fileList shouldContain resolvePath(path) } - - opossumInput.packageToRoot.values.forAll { levelForPath -> - levelForPath.keys.forAll { path -> - fileList shouldContain resolvePath(path) - } - } } "create a result that contains all packages in its signals" { result.getPackages().forAll { pkg -> - opossumInput.signals.find { it.id == pkg.metadata.id } shouldNot beNull() + opossumInput.externalAttributions.values.find { + it.packageType == pkg.metadata.id.type.lowercase() + && it.packageName == pkg.metadata.id.name + && it.packageVersion == pkg.metadata.id.version + && it.packageNamespace == pkg.metadata.id.namespace + } shouldNot beNull() } } @@ -154,8 +159,8 @@ class OpossumReporterTest : WordSpec({ "/pom.xml/compile/first-package-group/first-package@0.0.1/LICENSE" ) signals shouldHaveSize 2 - signals.find { it.source == "ORT-Scanner-SCANNER@1.2.3" } shouldNotBeNull { - license.toString() shouldBe "Apache-2.0" + signals.find { it.source.name == "ORT-Scanner-SCANNER@1.2.3" } shouldNotBeNull { + licenseName shouldBe "Apache-2.0" } } @@ -165,37 +170,20 @@ class OpossumReporterTest : WordSpec({ ) signals shouldHaveSize 2 - signals.find { it.source == "ORT-Scanner-SCANNER@1.2.3" } shouldNotBeNull { + signals.find { it.source.name == "ORT-Scanner-SCANNER@1.2.3" } shouldNotBeNull { copyright shouldContain "Copyright 2020 Some copyright holder in source artifact" copyright shouldContain "Copyright 2020 Some other copyright holder in source artifact" } } "create signals with all uuids being assigned" { - opossumInput.pathToSignal.values.forAll { signal -> - signal.forAll { uuid -> - opossumInput.signals.find { it.uuid == uuid } shouldNot beNull() + opossumInput.resourcesToAttributions.values.forAll { signals -> + signals.forAll { uuid -> + opossumInput.externalAttributions[uuid] shouldNot beNull() } } } - "create an opossumInput JSON with expected top level entries" { - val opossumInputJson = opossumInput.toJson() - - opossumInputJson.keys should containExactlyInAnyOrder( - "attributionBreakpoints", - "baseUrlsForSources", - "externalAttributionSources", - "externalAttributions", - "filesWithChildren", - "frequentLicenses", - "metadata", - "resources", - "resourcesToAttributions" - ) - (opossumInputJson["metadata"] as Map<*, *>).keys shouldContain "projectId" - } - "create frequentLicenses" { opossumInput.frequentLicenses.map { it.shortName } shouldContain "MIT" } @@ -213,18 +201,18 @@ class OpossumReporterTest : WordSpec({ "create issues containing all issues" { val issuesFromFirstPackage = - opossumInput.getSignalsForFile("/pom.xml/compile/first-package-group/first-package@0.0.1") + opossumInput.getSignalsForFile("/pom.xml/compile/first-package-group/first-package@0.0.1/") .filter { it.comment?.contains(Regex("Source-.*Message-")) == true } issuesFromFirstPackage shouldHaveSize 4 issuesFromFirstPackage.forAll { - it.followUp shouldBe true + it.followUp shouldBe OpossumReporter.OpossumFollowUp.FOLLOW_UP it.excludeFromNotice shouldBe true } val issuesAttachedToFallbackPath = opossumInput.getSignalsForFile("/") issuesAttachedToFallbackPath shouldHaveSize 1 issuesAttachedToFallbackPath.forAll { - it.followUp shouldBe true + it.followUp shouldBe OpossumReporter.OpossumFollowUp.FOLLOW_UP it.excludeFromNotice shouldBe true it.comment shouldContain Regex("Source-.*Message-") } @@ -234,7 +222,7 @@ class OpossumReporterTest : WordSpec({ "generateOpossumInput() with excluded scopes" should { val result = createOrtResult().setScopeExcludes("devDependencies") val opossumInputWithExcludedScopes = - OpossumReporterFactory().create(PluginConfig()).generateOpossumInput(ReporterInput(result)) + OpossumReporterFactory().create(PluginConfig()).createOpossumInput(ReporterInput(result)) val fileListWithExcludedScopes = opossumInputWithExcludedScopes.resources.toFileList() "exclude scopes" { From 4bf27e227e8d0372f7f9c9b58caba85f8f9edb6e Mon Sep 17 00:00:00 2001 From: alexzurbonsen Date: Thu, 21 Nov 2024 20:47:21 +0100 Subject: [PATCH 2/2] refactor(opossum reporter): adjust file structure Put public method at the top, put caller above callees etc. Signed-off-by: alexzurbonsen --- .../src/main/kotlin/OpossumReporter.kt | 292 +++++++++--------- 1 file changed, 150 insertions(+), 142 deletions(-) diff --git a/plugins/reporters/opossum/src/main/kotlin/OpossumReporter.kt b/plugins/reporters/opossum/src/main/kotlin/OpossumReporter.kt index 1ce4992e8f8c8..bd924630bcbc0 100644 --- a/plugins/reporters/opossum/src/main/kotlin/OpossumReporter.kt +++ b/plugins/reporters/opossum/src/main/kotlin/OpossumReporter.kt @@ -104,142 +104,35 @@ class OpossumReporter( override val descriptor: PluginDescriptor = OpossumReporterFactory.descriptor, private val config: OpossumReporterConfig ) : Reporter { - @Serializable - internal data class OpossumSignal( - @Transient - val uuid: UUID = UUID.randomUUID(), - val source: OpossumSignalSource, - val attributionConfidence: Int = 80, - val packageType: String?, - val packageNamespace: String?, - val packageName: String?, - val packageVersion: String?, - val copyright: String?, - val licenseName: String?, - val url: String?, - val preSelected: Boolean, - val followUp: OpossumFollowUp?, - val excludeFromNotice: Boolean, - val comment: String? - ) { - companion object { - fun create( - source: String, - id: Identifier? = null, - url: String? = null, - license: SpdxExpression? = null, - copyright: String? = null, - comment: String? = null, - preSelected: Boolean = false, - followUp: Boolean = false, - excludeFromNotice: Boolean = false - ): OpossumSignal { - return OpossumSignal( - source = OpossumSignalSource(name = source), - packageType = id?.getPurlType(), - packageNamespace = id?.namespace, - packageName = id?.name, - packageVersion = id?.version, - copyright = copyright, - licenseName = license?.toString(), - url = url, - preSelected = preSelected, - followUp = OpossumFollowUp.FOLLOW_UP.takeIf { followUp }, - excludeFromNotice = excludeFromNotice, - comment = comment, - ) + override fun generateReport(input: ReporterInput, outputDir: File): List> { + val reportFileResult = runCatching { + val opossumInput = createOpossumInput(input, config.maxDepth) + + outputDir.resolve("report.opossum").also { + writeReport(it, opossumInput) } } - fun matches(other: OpossumSignal): Boolean = - source == other.source - && packageType == other.packageType - && packageNamespace == other.packageNamespace - && packageName == other.packageName - && packageVersion == other.packageVersion - && copyright == other.copyright - && licenseName == other.licenseName - && url == other.url - && preSelected == other.preSelected - && comment == other.comment + return listOf(reportFileResult) } - @Serializable - internal data class OpossumSignalSource( - val name: String, - val documentConfidence: Int = 80, - ) - - @Serializable - internal enum class OpossumFollowUp { - FOLLOW_UP, + internal fun createOpossumInput(input: ReporterInput, maxDepth: Int = Int.MAX_VALUE): OpossumInput { + return OpossumInputCreator().create(input, maxDepth) } - @Serializable(with = OpossumResourcesSerializer::class) - internal data class OpossumResources( - val tree: MutableMap = mutableMapOf() - ) { - private fun addResource(pathPieces: List) { - if (pathPieces.isEmpty()) return - - val head = pathPieces.first() - val tail = pathPieces.drop(1) - - if (head !in tree) tree[head] = OpossumResources() - tree.getValue(head).addResource(tail) - } - - fun addResource(path: String) { - val pathPieces = path.split("/").filter { it.isNotEmpty() } - - addResource(pathPieces) - } - - fun isFile() = tree.isEmpty() - - fun isPathAFile(path: String): Boolean { - val pathPieces = path.split("/").filter { it.isNotEmpty() } - - return isPathAFile(pathPieces) - } - - private fun isPathAFile(pathPieces: List): Boolean { - if (pathPieces.isEmpty()) return isFile() - - val head = pathPieces.first() - val tail = pathPieces.drop(1) - - return head !in tree || tree.getValue(head).isPathAFile(tail) + private fun writeReport(outputFile: File, opossumInput: OpossumInput) { + val jsonFile = createOrtTempDir().resolve("input.json") + val json = Json { + explicitNulls = false + encodeDefaults = true } - fun toFileList(): Set = - tree.flatMapTo(mutableSetOf()) { (key, value) -> - value.toFileList().map { resolvePath(key, it, isDirectory = false) } - }.plus("/") - } + jsonFile.writeText(json.encodeToString(OpossumInput.serializer(), opossumInput)) - @Serializable - internal data class OpossumFrequentLicense( - val shortName: String, - val fullName: String?, - val defaultText: String? - ) : Comparable { - override fun compareTo(other: OpossumFrequentLicense) = - compareValuesBy( - this, - other, - { it.shortName }, - { it.fullName }, - { it.defaultText } - ) + jsonFile.packZip(outputFile) + jsonFile.delete() } - @Serializable - internal data class OpossumExternalAttributionSource( - val name: String, - val priority: Int - ) - @Serializable internal data class OpossumInput( val metadata: OpossumInputMetadata = OpossumInputMetadata(), @@ -579,33 +472,148 @@ class OpossumReporter( val fileCreationDate: String = LocalDateTime.now().toString() ) - private fun writeReport(outputFile: File, opossumInput: OpossumInput) { - val jsonFile = createOrtTempDir().resolve("input.json") - val json = Json { - explicitNulls = false - encodeDefaults = true + @Serializable(with = OpossumResourcesSerializer::class) + internal data class OpossumResources( + val tree: MutableMap = mutableMapOf() + ) { + private fun addResource(pathPieces: List) { + if (pathPieces.isEmpty()) { + return + } + + val head = pathPieces.first() + val tail = pathPieces.drop(1) + + if (head !in tree) { + tree[head] = OpossumResources() + } + + tree.getValue(head).addResource(tail) } - jsonFile.writeText(json.encodeToString(OpossumInput.serializer(), opossumInput)) - jsonFile.packZip(outputFile) - jsonFile.delete() - } + fun addResource(path: String) { + val pathPieces = path.split("/").filter { it.isNotEmpty() } - internal fun createOpossumInput(input: ReporterInput, maxDepth: Int = Int.MAX_VALUE): OpossumInput { - return OpossumInputCreator().create(input, maxDepth) - } + addResource(pathPieces) + } - override fun generateReport(input: ReporterInput, outputDir: File): List> { - val reportFileResult = runCatching { - val opossumInput = createOpossumInput(input, config.maxDepth) + fun isFile() = tree.isEmpty() - outputDir.resolve("report.opossum").also { - writeReport(it, opossumInput) + fun isPathAFile(path: String): Boolean { + val pathPieces = path.split("/").filter { it.isNotEmpty() } + + return isPathAFile(pathPieces) + } + + private fun isPathAFile(pathPieces: List): Boolean { + if (pathPieces.isEmpty()) { + return isFile() } + + val head = pathPieces.first() + val tail = pathPieces.drop(1) + + return head !in tree || tree.getValue(head).isPathAFile(tail) } - return listOf(reportFileResult) + fun toFileList(): Set = + tree.flatMapTo(mutableSetOf()) { (key, value) -> + value.toFileList().map { resolvePath(key, it, isDirectory = false) } + }.plus("/") } + + @Serializable + internal data class OpossumSignal( + @Transient + val uuid: UUID = UUID.randomUUID(), + val source: OpossumSignalSource, + val attributionConfidence: Int = 80, + val packageType: String?, + val packageNamespace: String?, + val packageName: String?, + val packageVersion: String?, + val copyright: String?, + val licenseName: String?, + val url: String?, + val preSelected: Boolean, + val followUp: OpossumFollowUp?, + val excludeFromNotice: Boolean, + val comment: String? + ) { + companion object { + fun create( + source: String, + id: Identifier? = null, + url: String? = null, + license: SpdxExpression? = null, + copyright: String? = null, + comment: String? = null, + preSelected: Boolean = false, + followUp: Boolean = false, + excludeFromNotice: Boolean = false + ): OpossumSignal { + return OpossumSignal( + source = OpossumSignalSource(name = source), + packageType = id?.getPurlType(), + packageNamespace = id?.namespace, + packageName = id?.name, + packageVersion = id?.version, + copyright = copyright, + licenseName = license?.toString(), + url = url, + preSelected = preSelected, + followUp = OpossumFollowUp.FOLLOW_UP.takeIf { followUp }, + excludeFromNotice = excludeFromNotice, + comment = comment + ) + } + } + + fun matches(other: OpossumSignal): Boolean = + source == other.source + && packageType == other.packageType + && packageNamespace == other.packageNamespace + && packageName == other.packageName + && packageVersion == other.packageVersion + && copyright == other.copyright + && licenseName == other.licenseName + && url == other.url + && preSelected == other.preSelected + && comment == other.comment + } + + @Serializable + internal data class OpossumSignalSource( + val name: String, + val documentConfidence: Int = 80 + ) + + @Serializable + internal enum class OpossumFollowUp { + FOLLOW_UP + } + + @Serializable + internal data class OpossumFrequentLicense( + val shortName: String, + val fullName: String?, + val defaultText: String? + ) : Comparable { + override fun compareTo(other: OpossumFrequentLicense) = + compareValuesBy( + this, + other, + { it.shortName }, + { it.fullName }, + { it.defaultText } + ) + } + + @Serializable + internal data class OpossumExternalAttributionSource( + val name: String, + val priority: Int + ) } private fun DependencyNode.getDependencies(): List =