refactor(reporter)!: Change to return per-file-format results

Instead of an all-or-nothing approach with throwing an exception, allow per-file-format results for reporters to signal that e.g. writing a JSON format succeeded while writing an XML format for the same reporter failed. This fixes the issue introduced in 5503c68 for the CycloneDX reporter to not cause a non-0 exit code anymore for the reporter CLI if one of the file formats failed to be created. Signed-off-by: Sebastian Schuberth <[email protected]>
oss-review-toolkit · Jul 18, 2024 · 8591cf5 · 8591cf5
1 parent 585b1fe
commit 8591cf5
Show file tree

Hide file tree

Showing 30 changed files with 320 additions and 217 deletions.
diff --git a/plugins/commands/reporter/src/main/kotlin/ReporterCommand.kt b/plugins/commands/reporter/src/main/kotlin/ReporterCommand.kt
@@ -289,7 +289,7 @@ class ReporterCommand : OrtCommand(
 
                         reporter to measureTimedValue {
                             val options = reportConfigMap[reporter.type] ?: PluginConfiguration.EMPTY
-                            runCatching { reporter.generateReport(input, outputDir, options) }
+                            reporter.generateReport(input, outputDir, options)
                         }
                     }
                 }.awaitAll()
@@ -300,18 +300,20 @@ class ReporterCommand : OrtCommand(
 
         reportDurationMap.value.forEach { (reporter, timedValue) ->
             val name = reporter.type
+            val fileResults = timedValue.value
 
-            timedValue.value.onSuccess { files ->
-                val fileList = files.joinToString { "'$it'" }
-                echo("Successfully created '$name' report(s) at $fileList in ${timedValue.duration}.")
-            }.onFailure { e ->
-                e.showStackTrace()
+            fileResults.forEach { fileResult ->
+                fileResult.onSuccess { file ->
+                    echo("Successfully created '$name' report at $file in ${timedValue.duration}.")
+                }.onFailure { e ->
+                    e.showStackTrace()
 
-                logger.error {
-                    "Could not create '$name' report in ${timedValue.duration}: ${e.collectMessages()}"
-                }
+                    logger.error {
+                        "Could not create '$name' report in ${timedValue.duration}: ${e.collectMessages()}"
+                    }
 
-                ++failureCount
+                    ++failureCount
+                }
             }
         }
 

diff --git a/plugins/reporters/asciidoc/src/funTest/kotlin/DocBookTemplateReporterFunTest.kt b/plugins/reporters/asciidoc/src/funTest/kotlin/DocBookTemplateReporterFunTest.kt
@@ -34,8 +34,8 @@ class DocBookTemplateReporterFunTest : StringSpec({
     "DocBook report is created from default template" {
         val expectedResultFile = getAssetFile("docbook-template-reporter-expected-result.xml")
 
-        val reportContent =
-            DocBookTemplateReporter().generateReport(ReporterInput(ORT_RESULT), tempdir()).single().readText()
+        val reportContent = DocBookTemplateReporter().generateReport(ReporterInput(ORT_RESULT), tempdir())
+            .single().getOrThrow().readText()
 
         reportContent should matchExpectedResult(
             expectedResultFile,

diff --git a/plugins/reporters/asciidoc/src/funTest/kotlin/HtmlTemplateReporterFunTest.kt b/plugins/reporters/asciidoc/src/funTest/kotlin/HtmlTemplateReporterFunTest.kt
@@ -33,7 +33,8 @@ class HtmlTemplateReporterFunTest : StringSpec({
         val expectedResultFile = getAssetFile("html-template-reporter-expected-result.html")
 
         val reporter = HtmlTemplateReporter()
-        val reportContent = reporter.generateReport(ReporterInput(ORT_RESULT), tempdir()).single().readText()
+        val reportContent = reporter.generateReport(ReporterInput(ORT_RESULT), tempdir())
+            .single().getOrThrow().readText()
 
         reportContent.patchAsciiDocTemplateResult() shouldBe patchExpectedResult(
             expectedResultFile,

diff --git a/plugins/reporters/asciidoc/src/funTest/kotlin/ManPageTemplateReporterFunTest.kt b/plugins/reporters/asciidoc/src/funTest/kotlin/ManPageTemplateReporterFunTest.kt
@@ -35,7 +35,8 @@ class ManPageTemplateReporterFunTest : StringSpec({
         val expectedResultFile = getAssetFile("manpage-template-reporter-expected-result.1")
         val reporter = ManPageTemplateReporter()
 
-        val reportContent = reporter.generateReport(ReporterInput(ORT_RESULT), tempdir()).single().readText()
+        val reportContent = reporter.generateReport(ReporterInput(ORT_RESULT), tempdir())
+            .single().getOrThrow().readText()
 
         reportContent should matchExpectedResult(
             expectedResultFile,

diff --git a/plugins/reporters/asciidoc/src/funTest/kotlin/PdfTemplateReporterFunTest.kt b/plugins/reporters/asciidoc/src/funTest/kotlin/PdfTemplateReporterFunTest.kt
@@ -22,8 +22,10 @@ package org.ossreviewtoolkit.plugins.reporters.asciidoc
 import io.kotest.assertions.throwables.shouldThrow
 import io.kotest.core.spec.style.StringSpec
 import io.kotest.engine.spec.tempdir
-import io.kotest.matchers.collections.contain
+import io.kotest.matchers.collections.shouldBeSingleton
+import io.kotest.matchers.collections.shouldContainAll
 import io.kotest.matchers.longs.beInRange
+import io.kotest.matchers.result.shouldBeSuccess
 import io.kotest.matchers.should
 import io.kotest.matchers.shouldBe
 
@@ -34,15 +36,19 @@ import org.ossreviewtoolkit.reporter.ReporterInput
 
 class PdfTemplateReporterFunTest : StringSpec({
     "The report is created successfully from an existing result and default template" {
-        val report = PdfTemplateReporter().generateReport(ReporterInput(ORT_RESULT), tempdir()).single()
+        val reportFileResults = PdfTemplateReporter().generateReport(ReporterInput(ORT_RESULT), tempdir())
 
-        report.reader().use {
-            val header = CharArray(4)
-            it.read(header) shouldBe header.size
-            String(header) shouldBe "%PDF"
-        }
+        reportFileResults.shouldBeSingleton {
+            it shouldBeSuccess { reportFile ->
+                reportFile.reader().use { stream ->
+                    val header = CharArray(4)
+                    stream.read(header) shouldBe header.size
+                    String(header) shouldBe "%PDF"
+                }
 
-        report.length() should beInRange(111000L..115000L)
+                reportFile.length() should beInRange(111000L..115000L)
+            }
+        }
     }
 
     "Report generation is aborted when path to non-existing PDF theme file is given" {
@@ -66,14 +72,12 @@ class PdfTemplateReporterFunTest : StringSpec({
     }
 
     "Advisor reports are generated if the result contains an advisor section" {
-        val reports =
-            PdfTemplateReporter().generateReport(
-                ReporterInput(ORT_RESULT_WITH_VULNERABILITIES),
-                tempdir()
-            )
+        val reportFileResults = PdfTemplateReporter().generateReport(
+            ReporterInput(ORT_RESULT_WITH_VULNERABILITIES),
+            tempdir()
+        )
 
-        val reportFileNames = reports.map { it.name }
-        reportFileNames should contain("AsciiDoc_vulnerability_report.pdf")
-        reportFileNames should contain("AsciiDoc_defect_report.pdf")
+        val reportFileNames = reportFileResults.mapNotNull { it.getOrNull()?.name }
+        reportFileNames.shouldContainAll("AsciiDoc_vulnerability_report.pdf", "AsciiDoc_defect_report.pdf")
     }
 })
diff --git a/plugins/reporters/asciidoc/src/main/kotlin/AsciiDocTemplateReporter.kt b/plugins/reporters/asciidoc/src/main/kotlin/AsciiDocTemplateReporter.kt
@@ -68,17 +68,21 @@ open class AsciiDocTemplateReporter(private val backend: String, override val ty
     protected open fun processTemplateOptions(outputDir: File, options: MutableMap<String, String>): Attributes =
         Attributes.builder().build()
 
-    final override fun generateReport(input: ReporterInput, outputDir: File, config: PluginConfiguration): List<File> {
+    final override fun generateReport(
+        input: ReporterInput,
+        outputDir: File,
+        config: PluginConfiguration
+    ): List<Result<File>> {
         val asciiDocOutputDir = createOrtTempDir("asciidoc")
 
         val templateOptions = config.options.toMutableMap()
         val asciidoctorAttributes = processTemplateOptions(asciiDocOutputDir, templateOptions)
-        val asciiDocFiles = generateAsciiDocFiles(input, asciiDocOutputDir, templateOptions)
-        val reports = processAsciiDocFiles(input, outputDir, asciiDocFiles, asciidoctorAttributes)
+        val asciiDocFileResults = generateAsciiDocFiles(input, asciiDocOutputDir, templateOptions)
+        val reportFileResults = processAsciiDocFiles(input, outputDir, asciiDocFileResults, asciidoctorAttributes)
 
         asciiDocOutputDir.safeDeleteRecursively()
 
-        return reports
+        return reportFileResults
     }
 
     /**
@@ -88,7 +92,7 @@ open class AsciiDocTemplateReporter(private val backend: String, override val ty
         input: ReporterInput,
         outputDir: File,
         options: MutableMap<String, String>
-    ): List<File> {
+    ): List<Result<File>> {
         if (FreemarkerTemplateProcessor.OPTION_TEMPLATE_PATH !in options) {
             options.putIfAbsent(
                 FreemarkerTemplateProcessor.OPTION_TEMPLATE_ID,
@@ -106,31 +110,28 @@ open class AsciiDocTemplateReporter(private val backend: String, override val ty
     }
 
     /**
-     * Generate the reports for the [asciiDocFiles] using Asciidoctor in [outputDir] applying the
+     * Generate the reports for the [asciiDocFileResults] using Asciidoctor in [outputDir] applying the
      * [asciidoctorAttributes].
      */
     protected open fun processAsciiDocFiles(
         input: ReporterInput,
         outputDir: File,
-        asciiDocFiles: List<File>,
+        asciiDocFileResults: List<Result<File>>,
         asciidoctorAttributes: Attributes
-    ): List<File> {
+    ): List<Result<File>> {
         val optionsBuilder = Options.builder()
             .attributes(asciidoctorAttributes)
             .backend(backend)
             .safe(SafeMode.UNSAFE)
 
-        val outputFiles = mutableListOf<File>()
-
-        asciiDocFiles.forEach { file ->
-            val outputFile = outputDir.resolve("${file.nameWithoutExtension}.$backend")
-
-            val options = optionsBuilder.toFile(outputFile).build()
-            asciidoctor.convertFile(file, options)
-
-            outputFiles += outputFile
+        return asciiDocFileResults.map { fileResult ->
+            // This implicitly passes through any failure from generating the Asciidoc file.
+            fileResult.mapCatching { file ->
+                outputDir.resolve("${file.nameWithoutExtension}.$backend").also { outputFile ->
+                    val options = optionsBuilder.toFile(outputFile).build()
+                    asciidoctor.convertFile(file, options)
+                }
+            }
         }
-
-        return outputFiles
     }
 }
diff --git a/plugins/reporters/ctrlx/src/funTest/kotlin/CtrlXAutomationReporterFunTest.kt b/plugins/reporters/ctrlx/src/funTest/kotlin/CtrlXAutomationReporterFunTest.kt
@@ -21,9 +21,10 @@ package org.ossreviewtoolkit.plugins.reporters.ctrlx
 
 import io.kotest.core.spec.style.StringSpec
 import io.kotest.engine.spec.tempdir
-import io.kotest.matchers.collections.containExactly
 import io.kotest.matchers.collections.haveSize
+import io.kotest.matchers.collections.shouldBeSingleton
 import io.kotest.matchers.nulls.shouldNotBeNull
+import io.kotest.matchers.result.shouldBeSuccess
 import io.kotest.matchers.should
 
 import kotlinx.serialization.json.decodeFromStream
@@ -47,6 +48,8 @@ class CtrlXAutomationReporterFunTest : StringSpec({
         val outputDir = tempdir()
         val reportFiles = CtrlXAutomationReporter().generateReport(ReporterInput(ORT_RESULT), outputDir)
 
-        reportFiles.map { it.name } should containExactly(REPORT_FILENAME)
+        reportFiles.shouldBeSingleton {
+            it shouldBeSuccess outputDir.resolve(REPORT_FILENAME)
+        }
     }
 })
diff --git a/plugins/reporters/ctrlx/src/main/kotlin/CtrlXAutomationReporter.kt b/plugins/reporters/ctrlx/src/main/kotlin/CtrlXAutomationReporter.kt
@@ -45,9 +45,11 @@ class CtrlXAutomationReporter : Reporter {
 
     override val type = "CtrlXAutomation"
 
-    override fun generateReport(input: ReporterInput, outputDir: File, config: PluginConfiguration): List<File> {
-        val reportFile = outputDir.resolve(REPORT_FILENAME)
-
+    override fun generateReport(
+        input: ReporterInput,
+        outputDir: File,
+        config: PluginConfiguration
+    ): List<Result<File>> {
         val packages = input.ortResult.getPackages(omitExcluded = true)
         val components = packages.mapTo(mutableListOf()) { (pkg, _) ->
             val qualifiedName = when (pkg.id.type) {
@@ -88,9 +90,14 @@ class CtrlXAutomationReporter : Reporter {
             )
         }
 
-        val info = FossInfo(components = components)
-        reportFile.outputStream().use { JSON.encodeToStream(info, it) }
+        val reportFileResult = runCatching {
+            val info = FossInfo(components = components)
+
+            outputDir.resolve(REPORT_FILENAME).apply {
+                outputStream().use { JSON.encodeToStream(info, it) }
+            }
+        }
 
-        return listOf(reportFile)
+        return listOf(reportFileResult)
     }
 }