chore(opossum reporter): Add review feedback

Signed-off-by: alexzurbonsen <[email protected]>

Author: alexzurbonsen

plugins/reporters/opossum/build.gradle.kts

@@ -30,12 +30,13 @@ dependencies {
 
     ksp(projects.reporter)
 
+    implementation(libs.bundles.ks3)
+    implementation(libs.kotlinx.serialization.core)
+    implementation(libs.kotlinx.serialization.json)
+
     implementation(projects.downloader)
     implementation(projects.model)
     implementation(projects.utils.commonUtils)
     implementation(projects.utils.ortUtils)
     implementation(projects.utils.spdxUtils)
-
-    implementation(libs.kotlinx.serialization.core)
-    implementation(libs.kotlinx.serialization.json)
 }

plugins/reporters/opossum/src/funTest/kotlin/OpossumReporterFunTest.kt

@@ -19,14 +19,13 @@
 
 package org.ossreviewtoolkit.plugins.reporters.opossum
 
-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.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
@@ -48,7 +47,7 @@ class OpossumReporterFunTest : WordSpec({
         }
 
         "create a parseable result and contain some expected values" {
-            val json = Json.parseToJsonElement(reportStr).jsonObject
+            val json = OpossumReporter.JSON.parseToJsonElement(reportStr).jsonObject
 
             json["metadata"]?.jsonObject?.get("projectId")?.jsonPrimitive?.content shouldBe "0"
             json["resources"]?.jsonObject?.size shouldBe 2

plugins/reporters/opossum/src/main/kotlin/OpossumReporter.kt

@@ -19,25 +19,26 @@
 
 package org.ossreviewtoolkit.plugins.reporters.opossum
 
-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 io.ks3.java.typealiases.LocalDateTimeAsString
+import io.ks3.java.typealiases.UuidAsString
 
 import java.io.File
 import java.time.LocalDateTime
 import java.util.UUID
 
 import kotlin.math.min
 
+import kotlinx.serialization.builtins.MapSerializer
+import kotlinx.serialization.builtins.serializer
+import kotlinx.serialization.descriptors.buildClassSerialDescriptor
+import kotlinx.serialization.descriptors.SerialDescriptor
+import kotlinx.serialization.encoding.Decoder
+import kotlinx.serialization.encoding.Encoder
+import kotlinx.serialization.json.encodeToStream
+import kotlinx.serialization.json.Json
+import kotlinx.serialization.KSerializer
+import kotlinx.serialization.Serializable
+
 import org.apache.logging.log4j.kotlin.logger
 
 import org.ossreviewtoolkit.downloader.VcsHost
@@ -49,9 +50,9 @@ import org.ossreviewtoolkit.model.OrtResult
 import org.ossreviewtoolkit.model.Package
 import org.ossreviewtoolkit.model.Project
 import org.ossreviewtoolkit.model.ScanResult
-import org.ossreviewtoolkit.model.VcsInfo
 import org.ossreviewtoolkit.model.utils.getPurlType
 import org.ossreviewtoolkit.model.utils.toPurl
+import org.ossreviewtoolkit.model.VcsInfo
 import org.ossreviewtoolkit.plugins.api.OrtPlugin
 import org.ossreviewtoolkit.plugins.api.OrtPluginOption
 import org.ossreviewtoolkit.plugins.api.PluginDescriptor
@@ -104,6 +105,13 @@ class OpossumReporter(
     override val descriptor: PluginDescriptor = OpossumReporterFactory.descriptor,
     private val config: OpossumReporterConfig
 ) : Reporter {
+    companion object {
+        internal val JSON = Json {
+            explicitNulls = false
+            encodeDefaults = true
+        }
+    }
+
     override fun generateReport(input: ReporterInput, outputDir: File): List<Result<File>> {
         val reportFileResult = runCatching {
             val opossumInput = createOpossumInput(input, config.maxDepth)
@@ -116,42 +124,40 @@ class OpossumReporter(
         return listOf(reportFileResult)
     }
 
-    internal fun createOpossumInput(input: ReporterInput, maxDepth: Int = Int.MAX_VALUE): OpossumInput {
-        return OpossumInputCreator().create(input, maxDepth)
-    }
+    internal fun createOpossumInput(input: ReporterInput, maxDepth: Int = Int.MAX_VALUE): OpossumInput =
+        OpossumInputCreator().create(input, maxDepth)
 
     private fun writeReport(outputFile: File, opossumInput: OpossumInput) {
-        val jsonFile = createOrtTempDir().resolve("input.json")
-        val json = Json {
-            explicitNulls = false
-            encodeDefaults = true
-        }
+        val inputJson = createOrtTempDir().resolve("input.json")
 
-        jsonFile.writeText(json.encodeToString(OpossumInput.serializer(), opossumInput))
+        inputJson.writeReport(opossumInput)
 
-        jsonFile.packZip(outputFile)
-        jsonFile.delete()
+        inputJson.packZip(outputFile)
+        inputJson.delete()
     }
 
+    private fun File.writeReport(opossumInput: OpossumInput): File =
+        apply { outputStream().use { JSON.encodeToStream(opossumInput, it) } }
+
     @Serializable
-    internal data class OpossumInput(
+    data class OpossumInput(
         val metadata: OpossumInputMetadata = OpossumInputMetadata(),
         val resources: OpossumResources,
-        val externalAttributions: Map<@Serializable(UUIDSerializer::class) UUID, OpossumSignal>,
-        val resourcesToAttributions: Map<String, Set<@Serializable(UUIDSerializer::class) UUID>>,
+        val externalAttributions: Map<UuidAsString, OpossumSignalFlat>,
+        val resourcesToAttributions: Map<String, Set<UuidAsString>>,
         val attributionBreakpoints: Set<String>,
         val filesWithChildren: Set<String>,
         val frequentLicenses: Set<OpossumFrequentLicense>,
         val baseUrlsForSources: Map<String, String>,
         val externalAttributionSources: Map<String, OpossumExternalAttributionSource>
     ) {
-        internal fun getSignalsForFile(file: String): List<OpossumSignal> =
+        internal fun getSignalsForFile(file: String): List<OpossumSignalFlat> =
             resourcesToAttributions[file].orEmpty().mapNotNull { uuid -> externalAttributions[uuid] }
     }
 
-    internal class OpossumInputCreator {
+    class OpossumInputCreator {
         private val resources: OpossumResources = OpossumResources()
-        private val signals: MutableList<OpossumSignal> = mutableListOf()
+        private val uuidToSignals: MutableMap<UUID, OpossumSignal> = mutableMapOf()
         private val pathToSignals: MutableMap<String, MutableSet<UUID>> = mutableMapOf()
         private val packageToRoot: MutableMap<Identifier, MutableMap<String, Int>> = mutableMapOf()
         private val attributionBreakpoints: MutableSet<String> = mutableSetOf()
@@ -192,7 +198,7 @@ class OpossumReporter(
                 addScannerResults(id, results, maxDepth)
             }
 
-            val externalAttributions = signals.associateBy { it.uuid }
+            val externalAttributions = uuidToSignals.mapValues { OpossumSignalFlat.create(it.value) }
             val resourcesToAttributions = pathToSignals.mapKeys {
                 val trailingSlash = if (resources.isPathAFile(it.key)) "" else "/"
                 resolvePath("${it.key}$trailingSlash")
@@ -254,19 +260,21 @@ class OpossumReporter(
         private fun addSignal(signal: OpossumSignal, paths: Set<String>) {
             if (paths.isEmpty()) return
 
-            val matchingSignal = signals.find { it.matches(signal) }
+            val signalEntry = uuidToSignals.entries.find { it.value.matches(signal) }
 
-            val uuidOfSignal = if (matchingSignal == null) {
-                signals += signal
-                signal.uuid
+            val matchingUuid = signalEntry?.key
+            val uuidOfSignal = if (matchingUuid == null) {
+                val uuid = UUID.randomUUID()
+                uuidToSignals[uuid] = signal
+                uuid
             } else {
-                matchingSignal.uuid
+                matchingUuid
             }
 
             paths.forEach { path ->
                 logger.debug {
-                    "add signal ${signal.packageName} ${signal.packageVersion} with namespace " +
-                        "${signal.packageNamespace} of of source ${signal.source} to $path"
+                    "Add signal ${signal.base.packageName} ${signal.base.packageVersion} with namespace " +
+                        "${signal.base.packageNamespace} of of source ${signal.base.source} to $path."
                 }
 
                 resources.addResource(path)
@@ -467,13 +475,13 @@ class OpossumReporter(
     }
 
     @Serializable
-    internal data class OpossumInputMetadata(
+    data class OpossumInputMetadata(
         val projectId: String = "0",
-        val fileCreationDate: String = LocalDateTime.now().toString()
+        val fileCreationDate: LocalDateTimeAsString = LocalDateTime.now()
     )
 
     @Serializable(with = OpossumResourcesSerializer::class)
-    internal data class OpossumResources(
+    data class OpossumResources(
         val tree: MutableMap<String, OpossumResources> = mutableMapOf()
     ) {
         private fun addResource(pathPieces: List<String>) {
@@ -523,9 +531,7 @@ class OpossumReporter(
     }
 
     @Serializable
-    internal data class OpossumSignal(
-        @Transient
-        val uuid: UUID = UUID.randomUUID(),
+    data class OpossumSignalFlat(
         val source: OpossumSignalSource,
         val attributionConfidence: Int = 80,
         val packageType: String?,
@@ -541,6 +547,33 @@ class OpossumReporter(
         val comment: String?
     ) {
         companion object {
+            fun create(signal: OpossumSignal): OpossumSignalFlat =
+                OpossumSignalFlat(
+                    source = signal.base.source,
+                    attributionConfidence = signal.attributionConfidence,
+                    packageType = signal.base.packageType,
+                    packageNamespace = signal.base.packageNamespace,
+                    packageName = signal.base.packageName,
+                    packageVersion = signal.base.packageVersion,
+                    copyright = signal.base.copyright,
+                    licenseName = signal.base.licenseName,
+                    url = signal.base.url,
+                    preSelected = signal.base.preSelected,
+                    followUp = signal.followUp,
+                    excludeFromNotice = signal.excludeFromNotice,
+                    comment = signal.base.comment
+                )
+        }
+    }
+
+    data class OpossumSignal(
+        val base: OpossumSignalBase,
+        val attributionConfidence: Int = 80,
+        val followUp: OpossumFollowUp?,
+        val excludeFromNotice: Boolean
+    ) {
+        companion object {
+            @Suppress("LongParameterList")
             fun create(
                 source: String,
                 id: Identifier? = null,
@@ -551,50 +584,54 @@ class OpossumReporter(
                 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,
+            ): OpossumSignal =
+                OpossumSignal(
+                    base = OpossumSignalBase(
+                        source = OpossumSignalSource(name = source),
+                        packageType = id?.getPurlType().toString(),
+                        packageNamespace = id?.namespace,
+                        packageName = id?.name,
+                        packageVersion = id?.version,
+                        copyright = copyright,
+                        licenseName = license?.toString(),
+                        url = url,
+                        preSelected = preSelected,
+                        comment = comment
+                    ),
                     followUp = OpossumFollowUp.FOLLOW_UP.takeIf { followUp },
-                    excludeFromNotice = excludeFromNotice,
-                    comment = comment
+                    excludeFromNotice = excludeFromNotice
                 )
-            }
         }
 
-        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
+        data class OpossumSignalBase(
+            val source: OpossumSignalSource,
+            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 comment: String?
+        )
+
+        fun matches(other: OpossumSignal): Boolean = base == other.base
     }
 
     @Serializable
-    internal data class OpossumSignalSource(
+    data class OpossumSignalSource(
         val name: String,
         val documentConfidence: Int = 80
     )
 
     @Serializable
-    internal enum class OpossumFollowUp {
+    enum class OpossumFollowUp {
         FOLLOW_UP
     }
 
     @Serializable
-    internal data class OpossumFrequentLicense(
+    data class OpossumFrequentLicense(
         val shortName: String,
         val fullName: String?,
         val defaultText: String?
@@ -610,7 +647,7 @@ class OpossumReporter(
     }
 
     @Serializable
-    internal data class OpossumExternalAttributionSource(
+    data class OpossumExternalAttributionSource(
         val name: String,
         val priority: Int
     )
@@ -623,18 +660,6 @@ private fun DependencyNode.getDependencies(): List<DependencyNode> =
         }
     }
 
-private object UUIDSerializer : KSerializer<UUID> {
-    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<OpossumReporter.OpossumResources> {
     override val descriptor: SerialDescriptor = buildClassSerialDescriptor("Resource")
 

plugins/reporters/opossum/src/test/kotlin/OpossumReporterTest.kt

@@ -115,6 +115,7 @@ class OpossumReporterTest : WordSpec({
                 baseUrlsForSources shouldNot beNull()
                 externalAttributionSources shouldNot beNull()
             }
+
             (opossumInput.metadata.projectId).toInt() shouldBe 0
         }