advisor: Redesign Advisor class

Korbinian Singhammer · Korbinian Singhammer · commit 14286dd1ba85 · 2021-03-16T06:11:07.000-04:00
Let vulnerabilty providers, e.g., NexusIq inherit from the
VulnerabilityProvider class. Move responsibility to create providers
inside the Advisor class, which can then call the provider. This is a
preparation to let the Advisor handle multiple providers.

Signed-off-by: Korbinian Singhammer &lt;external.Korbinian.Singhammer2@bosch.io&gt;
diff --git a/advisor/src/main/kotlin/Advisor.kt b/advisor/src/main/kotlin/Advisor.kt
@@ -26,29 +26,22 @@ import java.util.ServiceLoader
 
 import kotlinx.coroutines.runBlocking
 
-import org.ossreviewtoolkit.model.AdvisorDetails
 import org.ossreviewtoolkit.model.AdvisorRecord
-import org.ossreviewtoolkit.model.AdvisorResult
 import org.ossreviewtoolkit.model.AdvisorRun
-import org.ossreviewtoolkit.model.AdvisorSummary
 import org.ossreviewtoolkit.model.OrtResult
-import org.ossreviewtoolkit.model.Package
 import org.ossreviewtoolkit.model.config.AdvisorConfiguration
-import org.ossreviewtoolkit.model.createAndLogIssue
 import org.ossreviewtoolkit.model.readValue
 import org.ossreviewtoolkit.utils.Environment
-import org.ossreviewtoolkit.utils.collectMessagesAsString
-import org.ossreviewtoolkit.utils.showStackTrace
 
 /**
- * The class to retrieve security advisories.
+ * The class to manage [VulnerabilityProvider]s that retrieve security advisories.
  */
-abstract class Advisor(val advisorName: String, protected val config: AdvisorConfiguration) {
+class Advisor(private val providerFactory: VulnerabilityProviderFactory, private val config: AdvisorConfiguration) {
     companion object {
         private val LOADER = ServiceLoader.load(VulnerabilityProviderFactory::class.java)!!
 
         /**
-         * The list of all available advisors in the classpath.
+         * The list of all available [VulnerabilityProvider]s in the classpath.
          */
         val ALL by lazy { LOADER.iterator().asSequence().toList() }
     }
@@ -69,8 +62,10 @@ abstract class Advisor(val advisorName: String, protected val config: AdvisorCon
             "The provided ORT result file '${ortResultFile.canonicalPath}' does not contain an analyzer result."
         }
 
+        val provider = providerFactory.create(config)
+
         val packages = ortResult.getPackages(skipExcluded).map { it.pkg }
-        val results = runBlocking { retrievePackageVulnerabilities(packages) }
+        val results = runBlocking { provider.retrievePackageVulnerabilities(packages) }
             .mapKeysTo(sortedMapOf()) { (pkg, _) -> pkg.id }
 
         val advisorRecord = AdvisorRecord(results)
@@ -80,38 +75,4 @@ abstract class Advisor(val advisorName: String, protected val config: AdvisorCon
         val advisorRun = AdvisorRun(startTime, endTime, Environment(), config, advisorRecord)
         return ortResult.copy(advisor = advisorRun)
     }
-
-    protected abstract suspend fun retrievePackageVulnerabilities(
-        packages: List<Package>
-    ): Map<Package, List<AdvisorResult>>
-
-    protected fun createFailedResults(
-        startTime: Instant,
-        packages: List<Package>,
-        t: Throwable
-    ): Map<Package, List<AdvisorResult>> {
-        val endTime = Instant.now()
-
-        t.showStackTrace()
-
-        val failedResults = listOf(
-            AdvisorResult(
-                vulnerabilities = emptyList(),
-                advisor = AdvisorDetails(advisorName),
-                summary = AdvisorSummary(
-                    startTime = startTime,
-                    endTime = endTime,
-                    issues = listOf(
-                        createAndLogIssue(
-                            source = advisorName,
-                            message = "Failed to retrieve security vulnerabilities from $advisorName: " +
-                                    t.collectMessagesAsString()
-                        )
-                    )
-                )
-            )
-        )
-
-        return packages.associateWith { failedResults }
-    }
 }
diff --git a/advisor/src/main/kotlin/VulnerabilityProvider.kt b/advisor/src/main/kotlin/VulnerabilityProvider.kt
@@ -41,7 +41,7 @@ abstract class VulnerabilityProvider(val providerName: String) {
      * that associates each package with a list of [AdvisorResult]s. Needs to be implemented
      * by child classes.
      */
-    protected abstract suspend fun retrievePackageVulnerabilities(
+    abstract suspend fun retrievePackageVulnerabilities(
         packages: List<Package>
     ): Map<Package, List<AdvisorResult>>
 
diff --git a/advisor/src/main/kotlin/VulnerabilityProviderFactory.kt b/advisor/src/main/kotlin/VulnerabilityProviderFactory.kt
@@ -31,32 +31,32 @@ import org.ossreviewtoolkit.utils.ortConfigDirectory
  */
 interface VulnerabilityProviderFactory {
     /**
-     * The name to use to refer to the advisor.
+     * The name to use to refer to the provider.
      */
-    val advisorName: String
+    val providerName: String
 
     /**
-     * Create an [Advisor] using the specified [config].
+     * Create a [VulnerabilityProvider] using the specified [config].
      */
-    fun create(config: AdvisorConfiguration): Advisor
+    fun create(config: AdvisorConfiguration): VulnerabilityProvider
 }
 
 /**
- * A generic factory class for an [Advisor].
+ * A generic factory class for an [VulnerabilityProvider].
  */
-abstract class AbstractVulnerabilityProviderFactory<out T : Advisor>(
-    override val advisorName: String
+abstract class AbstractVulnerabilityProviderFactory<out T : VulnerabilityProvider>(
+    override val providerName: String
 ) : VulnerabilityProviderFactory {
     abstract override fun create(config: AdvisorConfiguration): T
 
     protected fun <T> getProviderConfiguration(config: AdvisorConfiguration, select: (AdvisorConfiguration) -> T?): T =
         select(config) ?: throw IllegalArgumentException(
-            "No $advisorName advisor configuration found in ${ortConfigDirectory.resolve(ORT_CONFIG_FILENAME)}"
+            "No configuration for $providerName found in ${ortConfigDirectory.resolve(ORT_CONFIG_FILENAME)}"
         )
 
     /**
-     * Return the advisor's name here to allow Clikt to display something meaningful when listing the scanners
+     * Return the provider's name here to allow Clikt to display something meaningful when listing the scanners
      * which are enabled by default via their factories.
      */
-    override fun toString() = advisorName
+    override fun toString() = providerName
 }
diff --git a/advisor/src/main/kotlin/advisors/NexusIq.kt b/advisor/src/main/kotlin/advisors/NexusIq.kt
@@ -24,21 +24,20 @@ import java.net.URI
 import java.time.Instant
 
 import org.ossreviewtoolkit.advisor.AbstractVulnerabilityProviderFactory
-import org.ossreviewtoolkit.advisor.Advisor
+import org.ossreviewtoolkit.advisor.VulnerabilityProvider
 import org.ossreviewtoolkit.clients.nexusiq.NexusIqService
 import org.ossreviewtoolkit.model.AdvisorDetails
 import org.ossreviewtoolkit.model.AdvisorResult
 import org.ossreviewtoolkit.model.AdvisorSummary
 import org.ossreviewtoolkit.model.Package
 import org.ossreviewtoolkit.model.Vulnerability
 import org.ossreviewtoolkit.model.config.AdvisorConfiguration
+import org.ossreviewtoolkit.model.config.NexusIqConfiguration
 import org.ossreviewtoolkit.model.utils.PurlType
 import org.ossreviewtoolkit.model.utils.getPurlType
 import org.ossreviewtoolkit.model.utils.toPurl
-import org.ossreviewtoolkit.utils.ORT_CONFIG_FILENAME
 import org.ossreviewtoolkit.utils.OkHttpClientHelper
 import org.ossreviewtoolkit.utils.log
-import org.ossreviewtoolkit.utils.ortConfigDirectory
 
 import retrofit2.HttpException
 
@@ -50,19 +49,12 @@ private const val REQUEST_CHUNK_SIZE = 100
 /**
  * A wrapper for [Nexus IQ Server](https://help.sonatype.com/iqserver) security vulnerability data.
  */
-class NexusIq(
-    name: String,
-    config: AdvisorConfiguration
-) : Advisor(name, config) {
+class NexusIq(name: String, private val nexusIqConfig: NexusIqConfiguration) : VulnerabilityProvider(name) {
     class Factory : AbstractVulnerabilityProviderFactory<NexusIq>("NexusIQ") {
-        override fun create(config: AdvisorConfiguration) = NexusIq(advisorName, config)
+        override fun create(config: AdvisorConfiguration) =
+            NexusIq(providerName, getProviderConfiguration(config) { it.nexusIq })
     }
 
-    private val nexusIqConfig = config.nexusIq
-        ?: throw IllegalArgumentException(
-            "No $advisorName advisor configuration found in ${ortConfigDirectory.resolve(ORT_CONFIG_FILENAME)}"
-        )
-
     private val service by lazy {
         NexusIqService.create(
             nexusIqConfig.serverUrl,
@@ -108,7 +100,7 @@ class NexusIq(
                     pkg to listOf(
                         AdvisorResult(
                             details.securityData.securityIssues.map { it.toVulnerability() },
-                            AdvisorDetails(advisorName),
+                            AdvisorDetails(providerName),
                             AdvisorSummary(startTime, endTime)
                         )
                     )
diff --git a/advisor/src/main/kotlin/advisors/VulnerableCode.kt b/advisor/src/main/kotlin/advisors/VulnerableCode.kt
@@ -28,26 +28,29 @@ import kotlinx.coroutines.awaitAll
 import kotlinx.coroutines.coroutineScope
 
 import org.ossreviewtoolkit.advisor.AbstractVulnerabilityProviderFactory
-import org.ossreviewtoolkit.advisor.Advisor
+import org.ossreviewtoolkit.advisor.VulnerabilityProvider
 import org.ossreviewtoolkit.clients.vulnerablecode.VulnerableCodeService
 import org.ossreviewtoolkit.model.AdvisorDetails
 import org.ossreviewtoolkit.model.AdvisorResult
 import org.ossreviewtoolkit.model.AdvisorSummary
 import org.ossreviewtoolkit.model.Package
 import org.ossreviewtoolkit.model.Vulnerability
 import org.ossreviewtoolkit.model.config.AdvisorConfiguration
+import org.ossreviewtoolkit.model.config.VulnerableCodeConfiguration
 import org.ossreviewtoolkit.model.utils.toPurl
-import org.ossreviewtoolkit.utils.ORT_CONFIG_FILENAME
 import org.ossreviewtoolkit.utils.OkHttpClientHelper
-import org.ossreviewtoolkit.utils.ortConfigDirectory
 
 /**
- * An [Advisor] implementation that obtains security vulnerability information from a
+ * An [VulnerabilityProvider] implementation that obtains security vulnerability information from a
  * [VulnerableCode][https://github.com/nexB/vulnerablecode] instance.
  */
-class VulnerableCode(name: String, config: AdvisorConfiguration) : Advisor(name, config) {
+class VulnerableCode(
+    name: String,
+    private val vulnerableCodeConfiguration: VulnerableCodeConfiguration
+) : VulnerabilityProvider(name) {
     class Factory : AbstractVulnerabilityProviderFactory<VulnerableCode>("VulnerableCode") {
-        override fun create(config: AdvisorConfiguration) = VulnerableCode(advisorName, config)
+        override fun create(config: AdvisorConfiguration) =
+            VulnerableCode(providerName, getProviderConfiguration(config) { it.vulnerableCode })
     }
 
     companion object {
@@ -73,10 +76,6 @@ class VulnerableCode(name: String, config: AdvisorConfiguration) : Advisor(name,
     }
 
     private val service by lazy {
-        val vulnerableCodeConfiguration = config.vulnerableCode
-            ?: throw IllegalArgumentException(
-                "No $advisorName advisor configuration found in ${ortConfigDirectory.resolve(ORT_CONFIG_FILENAME)}"
-            )
         VulnerableCodeService.create(vulnerableCodeConfiguration.serverUrl, OkHttpClientHelper.buildClient())
     }
 
@@ -103,7 +102,7 @@ class VulnerableCode(name: String, config: AdvisorConfiguration) : Advisor(name,
                     pkg to listOf(
                         AdvisorResult(
                             vulnerabilities,
-                            AdvisorDetails(advisorName),
+                            AdvisorDetails(providerName),
                             AdvisorSummary(startTime, endTime)
                         )
                     )
diff --git a/cli/src/main/kotlin/commands/AdvisorCommand.kt b/cli/src/main/kotlin/commands/AdvisorCommand.kt
@@ -43,7 +43,7 @@ import org.ossreviewtoolkit.utils.expandTilde
 import org.ossreviewtoolkit.utils.safeMkdirs
 
 class AdvisorCommand : CliktCommand(name = "advise", help = "Check dependencies for security vulnerabilities.") {
-    private val allAdvisorsByName = Advisor.ALL.associateBy { it.advisorName.toUpperCase() }
+    private val allVulnerabilityProvidersByName = Advisor.ALL.associateBy { it.providerName.toUpperCase() }
 
     private val input by option(
         "--ort-file", "-i",
@@ -76,12 +76,14 @@ class AdvisorCommand : CliktCommand(name = "advise", help = "Check dependencies
 
     private val globalOptionsForSubcommands by requireObject<GlobalOptions>()
 
-    private val advisorFactory by option(
+    private val providerFactory by option(
         "--advisor", "-a",
-        help = "The advisor to use, one of ${allAdvisorsByName.keys}"
-    ).convert { advisorName ->
-        allAdvisorsByName[advisorName.toUpperCase()]
-            ?: throw BadParameterValue("Advisor '$advisorName' is not one of ${allAdvisorsByName.keys}")
+        help = "The advisor to use, one of ${allVulnerabilityProvidersByName.keys}"
+    ).convert { providerName ->
+        allVulnerabilityProvidersByName[providerName.toUpperCase()]
+            ?: throw BadParameterValue(
+                "Advisor '$providerName' is not one of ${allVulnerabilityProvidersByName.keys}"
+            )
     }.required()
 
     private val skipExcluded by option(
@@ -101,9 +103,9 @@ class AdvisorCommand : CliktCommand(name = "advise", help = "Check dependencies
             }
         }
 
-        val advisor = advisorFactory.create(globalOptionsForSubcommands.config.advisor)
+        val advisor = Advisor(providerFactory, globalOptionsForSubcommands.config.advisor)
 
-        println("Using advisor '${advisor.advisorName}'.")
+        println("Using advisor '${providerFactory.providerName}'.")
 
         val ortResult = advisor.retrieveVulnerabilityInformation(input, skipExcluded).mergeLabels(labels)
 
