refactor(advisor): Mitgrate plugins to ConfigurablePluginFactory

Relates to #6507. Signed-off-by: Sebastian Schuberth <[email protected]>
oss-review-toolkit · Mar 5, 2023 · 0db7e0f · 0db7e0f
1 parent 5cfca01
commit 0db7e0f
Show file tree

Hide file tree

Showing 11 changed files with 111 additions and 186 deletions.
diff --git a/advisor/src/main/kotlin/AdviceProviderFactory.kt b/advisor/src/main/kotlin/AdviceProviderFactory.kt
@@ -19,52 +19,27 @@
 
 package org.ossreviewtoolkit.advisor
 
-import java.util.ServiceLoader
-
 import org.ossreviewtoolkit.model.config.AdvisorConfiguration
-import org.ossreviewtoolkit.model.config.Options
+import org.ossreviewtoolkit.utils.common.ConfigurablePluginFactory
 import org.ossreviewtoolkit.utils.common.Plugin
-import org.ossreviewtoolkit.utils.ort.ORT_CONFIG_FILENAME
-import org.ossreviewtoolkit.utils.ort.ortConfigDirectory
 
 /**
- * A common interface for use with [ServiceLoader] that all [AbstractAdviceProviderFactory] classes need to
- * implement.
+ * The extension point for [AdviceProvider]s.
  */
-interface AdviceProviderFactory : Plugin {
-    /**
-     * Create an [AdviceProvider] using the specified [config].
-     */
-    fun create(config: AdvisorConfiguration): AdviceProvider
-}
+interface AdviceProviderFactory : ConfigurablePluginFactory<AdviceProvider> {
+    companion object {
+        val ALL = Plugin.getAll<AdviceProviderFactory>()
+    }
 
-/**
- * A generic factory class for an [AdviceProvider].
- */
-abstract class AbstractAdviceProviderFactory<out T : AdviceProvider>(
-    override val type: String
-) : AdviceProviderFactory {
-    abstract override fun create(config: AdvisorConfiguration): T
+    override fun create(config: Map<String, String>): AdviceProvider = create(parseConfig(config))
 
     /**
-     * For providers that require configuration, return the typed configuration dedicated to provider [T] or throw if it
-     * does not exist.
+     * Create a new [AdviceProvider] with [config].
      */
-    protected fun <T : Any> AdvisorConfiguration.forProvider(select: AdvisorConfiguration.() -> T?): T =
-        requireNotNull(select()) {
-            "No configuration for '$type' found in '${ortConfigDirectory.resolve(ORT_CONFIG_FILENAME)}'."
-        }
-
-    /**
-     * Return a map with options for the [AdviceProvider] managed by this factory or an empty map if no options are
-     * available.
-     */
-    protected fun AdvisorConfiguration.providerOptions(): Options =
-        options.orEmpty()[type].orEmpty()
+    fun create(config: AdvisorConfiguration): AdviceProvider
 
     /**
-     * Return the provider's name here to allow Clikt to display something meaningful when listing the advisors which
-     * are enabled by default via their factories.
+     * Parse the [config] map into an object.
      */
-    override fun toString() = type
+    fun parseConfig(config: Map<String, String>): AdvisorConfiguration
 }
diff --git a/advisor/src/main/kotlin/Advisor.kt b/advisor/src/main/kotlin/Advisor.kt
@@ -34,7 +34,6 @@ import org.ossreviewtoolkit.model.Identifier
 import org.ossreviewtoolkit.model.OrtResult
 import org.ossreviewtoolkit.model.Package
 import org.ossreviewtoolkit.model.config.AdvisorConfiguration
-import org.ossreviewtoolkit.utils.common.Plugin
 import org.ossreviewtoolkit.utils.ort.Environment
 
 /**
@@ -45,12 +44,7 @@ class Advisor(
     private val providerFactories: List<AdviceProviderFactory>,
     private val config: AdvisorConfiguration
 ) {
-    companion object : Logging {
-        /**
-         * All [advice provider factories][AdviceProviderFactory] available in the classpath, associated by their names.
-         */
-        val ALL by lazy { Plugin.getAll<AdviceProviderFactory>() }
-    }
+    companion object : Logging
 
     /**
      * Query the [advice providers][providerFactories] and add the result to the provided [ortResult]. Excluded packages

diff --git a/advisor/src/main/kotlin/advisors/GitHubDefects.kt b/advisor/src/main/kotlin/advisors/GitHubDefects.kt
@@ -33,8 +33,8 @@ import kotlinx.coroutines.withContext
 
 import org.apache.logging.log4j.kotlin.Logging
 
-import org.ossreviewtoolkit.advisor.AbstractAdviceProviderFactory
 import org.ossreviewtoolkit.advisor.AdviceProvider
+import org.ossreviewtoolkit.advisor.AdviceProviderFactory
 import org.ossreviewtoolkit.clients.github.DateTime
 import org.ossreviewtoolkit.clients.github.GitHubService
 import org.ossreviewtoolkit.clients.github.Paging
@@ -82,15 +82,34 @@ import org.ossreviewtoolkit.utils.ort.showStackTrace
  */
 class GitHubDefects(name: String, config: GitHubDefectsConfiguration) : AdviceProvider(name) {
     companion object : Logging {
+        /**
+         * The default list of label to filter that typically indicate that an issue is not a defect.
+         */
+        val DEFAULT_LABEL_FILTER = listOf("!duplicate", "!enhancement", "!invalid", "!question", "*")
+
         /**
          * The default number of parallel requests executed by this advisor implementation. This value is used if the
          * corresponding property in the configuration is unspecified. It is chosen rather arbitrarily.
          */
         const val DEFAULT_PARALLEL_REQUESTS = 4
     }
 
-    class Factory : AbstractAdviceProviderFactory<GitHubDefects>("GitHubDefects") {
-        override fun create(config: AdvisorConfiguration) = GitHubDefects(type, config.forProvider { gitHubDefects })
+    class Factory : AdviceProviderFactory {
+        override val type = "GitHubDefects"
+
+        override fun create(config: AdvisorConfiguration) =
+            GitHubDefects(type, config.gitHubDefects ?: parseSpecificConfig(emptyMap()))
+
+        override fun parseConfig(config: Map<String, String>) =
+            AdvisorConfiguration(gitHubDefects = parseSpecificConfig(config))
+
+        private fun parseSpecificConfig(config: Map<String, String>) = GitHubDefectsConfiguration(
+            token = config["token"].orEmpty(),
+            endpointUrl = config["endpointUrl"] ?: GitHubService.ENDPOINT,
+            labelFilter = config["labelFilter"]?.split(',')?.map { it.trim() } ?: DEFAULT_LABEL_FILTER,
+            maxNumberOfIssuesPerRepository = config["maxNumberOfIssuesPerRepository"]?.toIntOrNull() ?: Int.MAX_VALUE,
+            parallelRequests = config["parallelRequests"]?.toIntOrNull() ?: DEFAULT_PARALLEL_REQUESTS,
+        )
     }
 
     /**
@@ -103,16 +122,16 @@ class GitHubDefects(name: String, config: GitHubDefectsConfiguration) : AdvicePr
     private val labelFilters = config.labelFilter.toLabelFilters()
 
     /** The maximum number of defects to retrieve. */
-    private val maxDefects = config.maxNumberOfIssuesPerRepository ?: Int.MAX_VALUE
+    private val maxDefects = config.maxNumberOfIssuesPerRepository
 
     /** The number of requests to be processed in parallel. */
-    private val parallelRequests = config.parallelRequests ?: DEFAULT_PARALLEL_REQUESTS
+    private val parallelRequests = config.parallelRequests
 
     /** The service for accessing the GitHub GraphQL API. */
     private val service by lazy {
         GitHubService.create(
-            token = config.token.orEmpty(),
-            url = config.endpointUrl ?: GitHubService.ENDPOINT,
+            token = config.token,
+            url = config.endpointUrl,
             client = HttpClient(OkHttp)
         )
     }

diff --git a/advisor/src/main/kotlin/advisors/NexusIq.kt b/advisor/src/main/kotlin/advisors/NexusIq.kt
@@ -26,8 +26,8 @@ import java.time.Instant
 
 import org.apache.logging.log4j.kotlin.Logging
 
-import org.ossreviewtoolkit.advisor.AbstractAdviceProviderFactory
 import org.ossreviewtoolkit.advisor.AdviceProvider
+import org.ossreviewtoolkit.advisor.AdviceProviderFactory
 import org.ossreviewtoolkit.clients.nexusiq.NexusIqService
 import org.ossreviewtoolkit.model.AdvisorCapability
 import org.ossreviewtoolkit.model.AdvisorDetails
@@ -63,8 +63,21 @@ private val READ_TIMEOUT = Duration.ofSeconds(60)
 class NexusIq(name: String, private val config: NexusIqConfiguration) : AdviceProvider(name) {
     companion object : Logging
 
-    class Factory : AbstractAdviceProviderFactory<NexusIq>("NexusIQ") {
-        override fun create(config: AdvisorConfiguration) = NexusIq(type, config.forProvider { nexusIq })
+    class Factory : AdviceProviderFactory {
+        override val type = "NexusIQ"
+
+        override fun create(config: AdvisorConfiguration) =
+            NexusIq(type, config.nexusIq ?: parseSpecificConfig(emptyMap()))
+
+        override fun parseConfig(config: Map<String, String>) =
+            AdvisorConfiguration(nexusIq = parseSpecificConfig(config))
+
+        private fun parseSpecificConfig(config: Map<String, String>) = NexusIqConfiguration(
+            serverUrl = config.getValue("serverUrl"),
+            browseUrl = config["browseUrl"] ?: config.getValue("serverUrl"),
+            username = config["username"],
+            password = config["password"]
+        )
     }
 
     override val details: AdvisorDetails = AdvisorDetails(providerName, enumSetOf(AdvisorCapability.VULNERABILITIES))

diff --git a/advisor/src/main/kotlin/advisors/OssIndex.kt b/advisor/src/main/kotlin/advisors/OssIndex.kt
@@ -25,8 +25,8 @@ import java.time.Instant
 
 import org.apache.logging.log4j.kotlin.Logging
 
-import org.ossreviewtoolkit.advisor.AbstractAdviceProviderFactory
 import org.ossreviewtoolkit.advisor.AdviceProvider
+import org.ossreviewtoolkit.advisor.AdviceProviderFactory
 import org.ossreviewtoolkit.clients.ossindex.OssIndexService
 import org.ossreviewtoolkit.model.AdvisorCapability
 import org.ossreviewtoolkit.model.AdvisorDetails
@@ -54,8 +54,18 @@ private const val BULK_REQUEST_SIZE = 128
 class OssIndex(name: String, config: OssIndexConfiguration) : AdviceProvider(name) {
     companion object : Logging
 
-    class Factory : AbstractAdviceProviderFactory<OssIndex>("OssIndex") {
-        override fun create(config: AdvisorConfiguration) = OssIndex(type, config.forProvider { ossIndex })
+    class Factory : AdviceProviderFactory {
+        override val type = "OssIndex"
+
+        override fun create(config: AdvisorConfiguration) =
+            OssIndex(type, config.ossIndex ?: parseSpecificConfig(emptyMap()))
+
+        override fun parseConfig(config: Map<String, String>) =
+            AdvisorConfiguration(ossIndex = parseSpecificConfig(config))
+
+        private fun parseSpecificConfig(config: Map<String, String>) = OssIndexConfiguration(
+            serverUrl = config["serverUrl"]
+        )
     }
 
     override val details = AdvisorDetails(providerName, enumSetOf(AdvisorCapability.VULNERABILITIES))

diff --git a/advisor/src/main/kotlin/advisors/Osv.kt b/advisor/src/main/kotlin/advisors/Osv.kt
@@ -26,8 +26,8 @@ import kotlinx.serialization.json.contentOrNull
 
 import org.apache.logging.log4j.kotlin.Logging
 
-import org.ossreviewtoolkit.advisor.AbstractAdviceProviderFactory
 import org.ossreviewtoolkit.advisor.AdviceProvider
+import org.ossreviewtoolkit.advisor.AdviceProviderFactory
 import org.ossreviewtoolkit.clients.osv.Ecosystem
 import org.ossreviewtoolkit.clients.osv.OsvService
 import org.ossreviewtoolkit.clients.osv.Severity
@@ -55,10 +55,18 @@ import us.springett.cvss.Cvss
 class Osv(name: String, config: OsvConfiguration) : AdviceProvider(name) {
     companion object : Logging
 
-    class Factory : AbstractAdviceProviderFactory<Osv>("OSV") {
+    class Factory : AdviceProviderFactory {
+        override val type = "OSV"
+
         override fun create(config: AdvisorConfiguration) =
-            // OSV does not require any dedicated configuration to be present.
-            Osv(type, config.forProvider { osv })
+            Osv(type, config.osv ?: parseSpecificConfig(emptyMap()))
+
+        override fun parseConfig(config: Map<String, String>) =
+            AdvisorConfiguration(osv = parseSpecificConfig(config))
+
+        private fun parseSpecificConfig(config: Map<String, String>) = OsvConfiguration(
+            serverUrl = config["serverUrl"]
+        )
     }
 
     override val details: AdvisorDetails = AdvisorDetails(providerName, enumSetOf(AdvisorCapability.VULNERABILITIES))

diff --git a/advisor/src/main/kotlin/advisors/VulnerableCode.kt b/advisor/src/main/kotlin/advisors/VulnerableCode.kt
@@ -22,8 +22,8 @@ package org.ossreviewtoolkit.advisor.advisors
 import java.net.URI
 import java.time.Instant
 
-import org.ossreviewtoolkit.advisor.AbstractAdviceProviderFactory
 import org.ossreviewtoolkit.advisor.AdviceProvider
+import org.ossreviewtoolkit.advisor.AdviceProviderFactory
 import org.ossreviewtoolkit.clients.vulnerablecode.VulnerableCodeService
 import org.ossreviewtoolkit.clients.vulnerablecode.VulnerableCodeService.PackagesWrapper
 import org.ossreviewtoolkit.model.AdvisorCapability
@@ -49,8 +49,19 @@ private const val BULK_REQUEST_SIZE = 100
  * [VulnerableCode][https://github.com/nexB/vulnerablecode] instance.
  */
 class VulnerableCode(name: String, config: VulnerableCodeConfiguration) : AdviceProvider(name) {
-    class Factory : AbstractAdviceProviderFactory<VulnerableCode>("VulnerableCode") {
-        override fun create(config: AdvisorConfiguration) = VulnerableCode(type, config.forProvider { vulnerableCode })
+    class Factory : AdviceProviderFactory {
+        override val type = "VulnerableCode"
+
+        override fun create(config: AdvisorConfiguration) =
+            VulnerableCode(type, config.vulnerableCode ?: parseSpecificConfig(emptyMap()))
+
+        override fun parseConfig(config: Map<String, String>) =
+            AdvisorConfiguration(vulnerableCode = parseSpecificConfig(config))
+
+        private fun parseSpecificConfig(config: Map<String, String>) = VulnerableCodeConfiguration(
+            serverUrl = config["serverUrl"],
+            apiKey = config["apiKey"]
+        )
     }
 
     /**

diff --git a/advisor/src/test/kotlin/AbstractAdviceProviderFactoryTest.kt b/advisor/src/test/kotlin/AbstractAdviceProviderFactoryTest.kt