Migrate reporter to new plugin API #9182

mnonnenmacher · 2024-09-22T13:52:04Z

Migrate the reporter to the new plugin API, see the commit messages for details.

codecov · 2024-09-22T18:30:46Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 68.09%. Comparing base (a5adf08) to head (d902e04).
Report is 18 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff            @@
##               main    #9182   +/-   ##
=========================================
  Coverage     68.09%   68.09%           
- Complexity     1291     1293    +2     
=========================================
  Files           250      250           
  Lines          8814     8814           
  Branches        916      916           
=========================================
  Hits           6002     6002           
  Misses         2427     2427           
  Partials        385      385

Flag	Coverage Δ
funTest-docker	`65.06% <ø> (ø)`
funTest-non-docker	`33.74% <ø> (+0.17%)`	⬆️
test	`35.87% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

plugins/reporters/spdx/src/funTest/kotlin/SpdxDocumentReporterFunTest.kt

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

plugins/reporters/web-app/src/main/kotlin/WebAppReporter.kt

plugins/reporters/spdx/src/main/kotlin/SpdxDocumentReporter.kt

plugins/reporters/asciidoc/src/main/kotlin/DocBookTemplateReporter.kt

plugins/reporters/asciidoc/src/main/kotlin/ManPageTemplateReporter.kt

plugins/reporters/asciidoc/src/main/kotlin/PdfTemplateReporter.kt

mnonnenmacher · 2024-11-07T20:30:06Z

plugins/reporters/cyclonedx/src/funTest/kotlin/CycloneDxReporterFunTest.kt

-            PluginConfiguration(options)
-        )
+    fun generateReport(result: OrtResult, options: Options): List<Result<File>> =
+        CycloneDxReporterFactory().create(PluginConfig(options)).generateReport(ReporterInput(result), outputDir)


I am using the factory now in several tests because it is the easiest way to create a reporter with the default configuration (the config class must not have default arguments). This is not nice and I will make a follow-up PR to simplify this.

plugins/compiler/src/main/kotlin/PluginSpecFactory.kt

sschuberth · 2024-11-08T13:19:27Z

plugins/api/src/main/kotlin/OrtPluginOption.kt

+     * A list of alternative names for the option. This can be used to make renaming options backward-compatible.
+     * Aliases are tried in the order they are defined until a value is found.
+     */
+    val aliases: Array<String> = []


The comment says it's a list although it's an array... which we barely use so far in the code base. Just curious, what's the reason to use an array here?

Lists are not allowed in annotation classes, it's a language limitation. Updated the comment.

Actually arrays could be more efficient if you can cope with their limitations. I wonder whether we should make more use of them anyway.

plugins/reporters/asciidoc/src/main/kotlin/AsciiDocTemplateReporter.kt

cli/src/funTest/kotlin/ExamplesFunTest.kt

sschuberth · 2024-11-08T13:31:29Z

plugins/reporters/asciidoc/src/main/kotlin/DocBookTemplateReporter.kt

-class DocBookTemplateReporter : AsciiDocTemplateReporter() {
+@OrtPlugin(
+    displayName = "DocBook Template Reporter",
+    description = "Generates DocBook files from Apache Freemarker templates.",


I somehow feel that mentioning AsciiDoc is at least equally important to mentioning Freemarker. So should we say "Generates DocBook from AsciiDoc files with Freemarker templates."

Similar below.

IMO the double "from" in "from AsciiDoc files from Apache Freemarker" does not read so nice now (I found my proposal from above better 😁), but we could improve that later.

sschuberth · 2024-11-08T14:40:22Z

plugins/reporters/freemarker/src/main/kotlin/PlainTextTemplateReporter.kt

+        internal val DEFAULT = PlainTextTemplateReporterConfig(
+            templateIds = listOf("NOTICE_DEFAULT"),
+            templatePaths = null
+        )


How about moving this to the top level instead of wrapping it in a companion object?

I actually like scoping such constants, to me PlainTextTemplateReporterConfig.DEFAULT also reads much better than DEFAULT_PLAIN_TEXT_TEMPLATE_REPORTER_CONFIG.

The discussion we once had in the core team came to the conclusion that such scoping is only useful for public properties. Or maybe it was non-private properties? Then you would be right for the internal case. Let's keep it as-is then.

plugins/reporters/fossid/src/main/kotlin/FossIdSnippetReporter.kt

plugins/reporters/freemarker/src/main/kotlin/FreemarkerTemplateProcessor.kt

plugins/reporters/fossid/src/test/kotlin/FossIdReporterTest.kt

plugins/commands/reporter/src/main/kotlin/ReporterCommand.kt

sschuberth

Approving, comments are optional to address.

Make the `defaultValue` property of the `OrtPluginOption` annotation optional. As annotation properties cannot be nullable, define a constant to mark if the default value should be ignored. This is a preparation for adding more properties to the annotation. Signed-off-by: Martin Nonnenmacher <martin.nonnenmacher@bosch.com>

Add the property `OrtPluginOption.aliases` to define optional aliases for configuration options. This can be used to rename options in a backward-compatible way. Signed-off-by: Martin Nonnenmacher <martin.nonnenmacher@bosch.com>

Signed-off-by: Martin Nonnenmacher <martin.nonnenmacher@bosch.com>

Convert `AsciiDocTemplateReporter` to an abstract class. This allows to simplify the constructor by making `backend` an abstract property. Signed-off-by: Martin Nonnenmacher <martin.nonnenmacher@bosch.com>

Migrate the reporters to the new plugin API. Reporter implementations get their configuration options via the `generateReport` function. With the new plugin API the configuration options are provided as a constructor parameter. With this change both ways are supported to allow for a step-by-step migration in the following commits. Signed-off-by: Martin Nonnenmacher <martin.nonnenmacher@bosch.com>

Add a config class for the reporter and use it instead of the options passed to the `generateReport` function. Signed-off-by: Martin Nonnenmacher <martin.nonnenmacher@bosch.com>

Add a config class for the reporter and use it instead of the options passed to the `generateReport` function. While add it, also add missing configuration options to the tests. Signed-off-by: Martin Nonnenmacher <martin.nonnenmacher@bosch.com>

Add a config class for the reporter and use it instead of the options passed to the `generateReport` function. Signed-off-by: Martin Nonnenmacher <martin.nonnenmacher@bosch.com>

Use dedicated function arguments for the template IDs and paths in `FreemarkerTemplateProcessor` to make the API typesafe and prepare for introducing configuration classes for the related reporters. Signed-off-by: Martin Nonnenmacher <martin.nonnenmacher@bosch.com>

Add a config class for the `PlainTextTemplateReporter` and use it instead of the options passed to the `generateReport` function. While at it, improve the configuration option names to be plural but keep supporting the old names for compatibility. Signed-off-by: Martin Nonnenmacher <martin.nonnenmacher@bosch.com>

Add config classes for the reporters and use them instead of the options passed to the `generateReport` function. Signed-off-by: Martin Nonnenmacher <martin.nonnenmacher@bosch.com>

Signed-off-by: Martin Nonnenmacher <martin.nonnenmacher@bosch.com>

Add config classes for the reporters and use them instead of the options passed to the `generateReport` function. Several tests that unsure the presence of configuration options can be removed, because this is now done by the generated plugin factory. Signed-off-by: Martin Nonnenmacher <martin.nonnenmacher@bosch.com>

All reporters have been migrated to receive their configuration options via the constructor, so remove the now unused `config` argument from the `generateReport` function. Signed-off-by: Martin Nonnenmacher <martin.nonnenmacher@bosch.com>

mnonnenmacher force-pushed the reporter-refactorings branch 2 times, most recently from c658c37 to aa85547 Compare September 22, 2024 18:18

mnonnenmacher force-pushed the reporter-refactorings branch 2 times, most recently from ef79cd6 to 63d352f Compare November 7, 2024 10:36

github-advanced-security bot found potential problems Nov 7, 2024

View reviewed changes

plugins/reporters/spdx/src/funTest/kotlin/SpdxDocumentReporterFunTest.kt Fixed Show fixed Hide fixed

github-advanced-security bot found potential problems Nov 7, 2024

View reviewed changes

plugins/reporters/opossum/src/main/kotlin/OpossumReporter.kt Dismissed Show dismissed Hide dismissed

plugins/reporters/web-app/src/main/kotlin/WebAppReporter.kt Dismissed Show dismissed Hide dismissed

mnonnenmacher force-pushed the reporter-refactorings branch from 63d352f to bd1d5e9 Compare November 7, 2024 16:37

github-advanced-security bot found potential problems Nov 7, 2024

View reviewed changes

mnonnenmacher force-pushed the reporter-refactorings branch from bd1d5e9 to 7a7227b Compare November 7, 2024 20:08

github-advanced-security bot found potential problems Nov 7, 2024

View reviewed changes

mnonnenmacher force-pushed the reporter-refactorings branch from 7a7227b to c02be70 Compare November 7, 2024 20:28

mnonnenmacher commented Nov 7, 2024

View reviewed changes

mnonnenmacher marked this pull request as ready for review November 7, 2024 20:30

mnonnenmacher requested a review from a team as a code owner November 7, 2024 20:30

mnonnenmacher force-pushed the reporter-refactorings branch from c02be70 to ce7814f Compare November 7, 2024 21:18

sschuberth requested changes Nov 8, 2024

View reviewed changes

mnonnenmacher force-pushed the reporter-refactorings branch from ce7814f to 54e1ad8 Compare November 8, 2024 16:06

mnonnenmacher requested a review from sschuberth November 8, 2024 16:09

sschuberth approved these changes Nov 8, 2024

View reviewed changes

sschuberth mentioned this pull request Nov 8, 2024

In SPDX reports, include licenseInfoFromFiles and file-level information for the scanned project itself as well #8485

Open

mnonnenmacher added 9 commits November 9, 2024 11:30

fix(plugins-api): Fix handling of default values for string list options

55507ff

Signed-off-by: Martin Nonnenmacher <martin.nonnenmacher@bosch.com>

refactor(asciidoc)!: Make AsciiDocTemplateReporter abstract

4b7d262

Convert `AsciiDocTemplateReporter` to an abstract class. This allows to simplify the constructor by making `backend` an abstract property. Signed-off-by: Martin Nonnenmacher <martin.nonnenmacher@bosch.com>

refactor(cyclonedx)!: Use a plugin config class

ca83651

Add a config class for the reporter and use it instead of the options passed to the `generateReport` function. Signed-off-by: Martin Nonnenmacher <martin.nonnenmacher@bosch.com>

refactor(evaluatedmodel)!: Use a plugin config class

a4b1f67

Add a config class for the reporter and use it instead of the options passed to the `generateReport` function. Signed-off-by: Martin Nonnenmacher <martin.nonnenmacher@bosch.com>

refactor(gitlab)!: Use a plugin config class

280091d

Add a config class for the reporter and use it instead of the options passed to the `generateReport` function. Signed-off-by: Martin Nonnenmacher <martin.nonnenmacher@bosch.com>

refactor(opossum)!: Use a plugin config class

aeaa83e

Add a config class for the reporter and use it instead of the options passed to the `generateReport` function. Signed-off-by: Martin Nonnenmacher <martin.nonnenmacher@bosch.com>

mnonnenmacher added 9 commits November 9, 2024 11:30

refactor(web-app)!: Use a plugin config class

530507f

Add a config class for the reporter and use it instead of the options passed to the `generateReport` function. Signed-off-by: Martin Nonnenmacher <martin.nonnenmacher@bosch.com>

refactor(asciidoc)!: Use a plugin config class

302fe52

Add config classes for the reporters and use them instead of the options passed to the `generateReport` function. Signed-off-by: Martin Nonnenmacher <martin.nonnenmacher@bosch.com>

docs(asciidoc): Slightly improve docs of PdfTemplateReporter

cd3d32b

Signed-off-by: Martin Nonnenmacher <martin.nonnenmacher@bosch.com>

chore(freemarker): Remove unused constants

8947721

Signed-off-by: Martin Nonnenmacher <martin.nonnenmacher@bosch.com>

mnonnenmacher force-pushed the reporter-refactorings branch from 54e1ad8 to d902e04 Compare November 9, 2024 10:30

mnonnenmacher merged commit 4596888 into main Nov 9, 2024
23 checks passed

mnonnenmacher deleted the reporter-refactorings branch November 9, 2024 16:05

sschuberth mentioned this pull request Nov 10, 2024

Ensure that all plugins implement Plugin or one of its sub-interfaces #6507

Closed

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate reporter to new plugin API #9182

Migrate reporter to new plugin API #9182

mnonnenmacher commented Sep 22, 2024 •

edited

Loading

codecov bot commented Sep 22, 2024 •

edited

Loading

mnonnenmacher Nov 7, 2024

sschuberth Nov 8, 2024

mnonnenmacher Nov 8, 2024 •

edited

Loading

sschuberth Nov 8, 2024

sschuberth Nov 8, 2024

sschuberth Nov 8, 2024

sschuberth Nov 8, 2024

mnonnenmacher Nov 8, 2024

sschuberth Nov 8, 2024

sschuberth left a comment •

edited

Loading

Migrate reporter to new plugin API #9182

Migrate reporter to new plugin API #9182

Conversation

mnonnenmacher commented Sep 22, 2024 • edited Loading

codecov bot commented Sep 22, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mnonnenmacher Nov 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sschuberth left a comment • edited Loading

Choose a reason for hiding this comment

mnonnenmacher commented Sep 22, 2024 •

edited

Loading

codecov bot commented Sep 22, 2024 •

edited

Loading

mnonnenmacher Nov 8, 2024 •

edited

Loading

sschuberth left a comment •

edited

Loading