diff --git a/advisor/src/main/kotlin/AdviceProviderFactory.kt b/advisor/src/main/kotlin/AdviceProviderFactory.kt index d5e2e52283b36..a43e327c1a0eb 100644 --- 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 { + companion object { + val ALL = Plugin.getAll() + } -/** - * A generic factory class for an [AdviceProvider]. - */ -abstract class AbstractAdviceProviderFactory( - override val type: String -) : AdviceProviderFactory { - abstract override fun create(config: AdvisorConfiguration): T + override fun create(config: Map): 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 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): AdvisorConfiguration } diff --git a/advisor/src/main/kotlin/Advisor.kt b/advisor/src/main/kotlin/Advisor.kt index 6d5f341f0463d..383a038b58fdc 100644 --- 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, 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() } - } + 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 index 75f50a6e2f8c2..410aeb94b1a3c 100644 --- 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,6 +82,11 @@ 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. @@ -89,8 +94,22 @@ class GitHubDefects(name: String, config: GitHubDefectsConfiguration) : AdvicePr const val DEFAULT_PARALLEL_REQUESTS = 4 } - class Factory : AbstractAdviceProviderFactory("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) = + AdvisorConfiguration(gitHubDefects = parseSpecificConfig(config)) + + private fun parseSpecificConfig(config: Map) = 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 index 68a3c7f64ff9a..1f9e9f7730e5a 100644 --- 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") { - 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) = + AdvisorConfiguration(nexusIq = parseSpecificConfig(config)) + + private fun parseSpecificConfig(config: Map) = 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 index 7cac430ed2d64..d03e50f245b15 100644 --- 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") { - 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) = + AdvisorConfiguration(ossIndex = parseSpecificConfig(config)) + + private fun parseSpecificConfig(config: Map) = 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 index c35039fcabd88..55c59d72c23bb 100644 --- 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") { + 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) = + AdvisorConfiguration(osv = parseSpecificConfig(config)) + + private fun parseSpecificConfig(config: Map) = 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 index e652bdeddc579..21511355e7f06 100644 --- 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") { - 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) = + AdvisorConfiguration(vulnerableCode = parseSpecificConfig(config)) + + private fun parseSpecificConfig(config: Map) = VulnerableCodeConfiguration( + serverUrl = config["serverUrl"], + apiKey = config["apiKey"] + ) } /** diff --git a/advisor/src/test/kotlin/AbstractAdviceProviderFactoryTest.kt b/advisor/src/test/kotlin/AbstractAdviceProviderFactoryTest.kt deleted file mode 100644 index 2e6f5a367d280..0000000000000 --- a/advisor/src/test/kotlin/AbstractAdviceProviderFactoryTest.kt +++ /dev/null @@ -1,112 +0,0 @@ -/* - * Copyright (C) 2021 The ORT Project Authors (see ) - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - * - * SPDX-License-Identifier: Apache-2.0 - * License-Filename: LICENSE - */ - -package org.ossreviewtoolkit.advisor - -import io.kotest.assertions.throwables.shouldThrow -import io.kotest.core.spec.style.WordSpec -import io.kotest.matchers.maps.beEmpty -import io.kotest.matchers.should -import io.kotest.matchers.shouldBe -import io.kotest.matchers.string.shouldContain - -import org.ossreviewtoolkit.advisor.advisors.VulnerableCode -import org.ossreviewtoolkit.model.config.AdvisorConfiguration -import org.ossreviewtoolkit.model.config.VulnerableCodeConfiguration - -class AbstractAdviceProviderFactoryTest : WordSpec({ - "forProvider" should { - "return the configuration for the selected provider" { - val advisorConfig = AdvisorConfiguration(vulnerableCode = VULNERABLE_CODE_CONFIG) - - val factory = object : AbstractAdviceProviderFactory(PROVIDER_NAME) { - override fun create(config: AdvisorConfiguration): AdviceProvider { - config.forProvider { vulnerableCode } shouldBe VULNERABLE_CODE_CONFIG - return VulnerableCode(type, VULNERABLE_CODE_CONFIG) - } - } - - factory.create(advisorConfig) - } - - "throw an exception if no configuration for the selected provider is defined" { - val factory = object : AbstractAdviceProviderFactory(PROVIDER_NAME) { - override fun create(config: AdvisorConfiguration): AdviceProvider { - val exception = shouldThrow { - config.forProvider { vulnerableCode } - } - - exception.message shouldContain PROVIDER_NAME - - return VulnerableCode(type, VULNERABLE_CODE_CONFIG) - } - } - - factory.create(AdvisorConfiguration()) - } - } - - "providerOptions" should { - "return the specific options for the selected provider" { - val providerOptions = mapOf("foo" to "bar") - val advisorConfig = AdvisorConfiguration(options = mapOf(PROVIDER_NAME to providerOptions)) - - val factory = object : AbstractAdviceProviderFactory(PROVIDER_NAME) { - override fun create(config: AdvisorConfiguration): AdviceProvider { - config.providerOptions() shouldBe providerOptions - - return VulnerableCode(type, VULNERABLE_CODE_CONFIG) - } - } - - factory.create(advisorConfig) - } - - "return an empty map if no options for the selected provider are available" { - val options = mapOf("anotherProvider" to mapOf("someOption" to "someValue")) - val advisorConfig = AdvisorConfiguration(options = options) - - val factory = object : AbstractAdviceProviderFactory(PROVIDER_NAME) { - override fun create(config: AdvisorConfiguration): AdviceProvider { - config.providerOptions() should beEmpty() - - return VulnerableCode(type, VULNERABLE_CODE_CONFIG) - } - } - - factory.create(advisorConfig) - } - - "return an empty map if no options are defined at all" { - val factory = object : AbstractAdviceProviderFactory(PROVIDER_NAME) { - override fun create(config: AdvisorConfiguration): AdviceProvider { - config.providerOptions() should beEmpty() - - return VulnerableCode(type, VULNERABLE_CODE_CONFIG) - } - } - - factory.create(AdvisorConfiguration()) - } - } -}) - -private const val PROVIDER_NAME = "testAdviceProvider" - -private val VULNERABLE_CODE_CONFIG = VulnerableCodeConfiguration("https://example.org/vc", "") diff --git a/advisor/src/test/kotlin/advisors/GitHubDefectsTest.kt b/advisor/src/test/kotlin/advisors/GitHubDefectsTest.kt index ba1e5d78ce3eb..5c2ec414ffcd8 100644 --- a/advisor/src/test/kotlin/advisors/GitHubDefectsTest.kt +++ b/advisor/src/test/kotlin/advisors/GitHubDefectsTest.kt @@ -434,9 +434,14 @@ private fun createAdvisor( labelFilter: List? = null, maxDefectsCount: Int? = null ): GitHubDefects { - val githubConfig = GitHubDefectsConfiguration(token = GITHUB_TOKEN, endpointUrl = url) - .run { labelFilter?.let { copy(labelFilter = it) } ?: this } - .run { maxDefectsCount?.let { copy(maxNumberOfIssuesPerRepository = it) } ?: this } + val githubConfig = GitHubDefectsConfiguration( + token = GITHUB_TOKEN, + endpointUrl = url ?: GitHubService.ENDPOINT, + labelFilter = labelFilter ?: GitHubDefects.DEFAULT_LABEL_FILTER, + maxNumberOfIssuesPerRepository = maxDefectsCount ?: Int.MAX_VALUE, + parallelRequests = GitHubDefects.DEFAULT_PARALLEL_REQUESTS + ) + val advisorConfig = AdvisorConfiguration(gitHubDefects = githubConfig) val factory = GitHubDefects.Factory() diff --git a/cli/src/main/kotlin/commands/AdvisorCommand.kt b/cli/src/main/kotlin/commands/AdvisorCommand.kt index b4c8c8d52628a..49ed5cf24b30a 100644 --- a/cli/src/main/kotlin/commands/AdvisorCommand.kt +++ b/cli/src/main/kotlin/commands/AdvisorCommand.kt @@ -39,6 +39,7 @@ import kotlin.time.toKotlinDuration import kotlinx.coroutines.runBlocking +import org.ossreviewtoolkit.advisor.AdviceProviderFactory import org.ossreviewtoolkit.advisor.Advisor import org.ossreviewtoolkit.cli.OrtCommand import org.ossreviewtoolkit.cli.utils.SeverityStats @@ -98,9 +99,10 @@ class AdvisorCommand : OrtCommand( private val providerFactories by option( "--advisors", "-a", - help = "The comma-separated advisors to use, any of ${Advisor.ALL.keys}." + help = "The comma-separated advisors to use, any of ${AdviceProviderFactory.ALL.keys}." ).convert { name -> - Advisor.ALL[name] ?: throw BadParameterValue("Advisor '$name' is not one of ${Advisor.ALL.keys}.") + AdviceProviderFactory.ALL[name] + ?: throw BadParameterValue("Advisor '$name' is not one of ${AdviceProviderFactory.ALL.keys}.") }.split(",").required() private val skipExcluded by option( diff --git a/model/src/main/kotlin/config/AdvisorConfiguration.kt b/model/src/main/kotlin/config/AdvisorConfiguration.kt index fc47ad85884f0..39c253946d9b8 100644 --- a/model/src/main/kotlin/config/AdvisorConfiguration.kt +++ b/model/src/main/kotlin/config/AdvisorConfiguration.kt @@ -50,14 +50,14 @@ data class GitHubDefectsConfiguration( * The access token to authenticate against the GitHub GraphQL endpoint. */ @JsonProperty(access = JsonProperty.Access.WRITE_ONLY) - val token: String?, + val token: String, /** * The URL of the GraphQL endpoint to be accessed by the service. If undefined, default is the endpoint of the * official GitHub GraphQL API. */ @JsonInclude(JsonInclude.Include.NON_DEFAULT) - val endpointUrl: String? = null, + val endpointUrl: String, /** * A list with labels to be used for filtering GitHub issues. With GitHub's data model for issues, it is not @@ -80,14 +80,14 @@ data class GitHubDefectsConfiguration( * (see https://docs.github.com/en/issues/using-labels-and-milestones-to-track-work/managing-labels#about-default-labels) */ @JsonInclude(JsonInclude.Include.NON_DEFAULT) - val labelFilter: List = listOf("!duplicate", "!enhancement", "!invalid", "!question", "*"), + val labelFilter: List, /** * The maximum number of defects that are retrieved from a single repository. If a repository contains more * issues, only this number is returned (the newest ones). Popular libraries hosted on GitHub can really have a * large number of issues; therefore, it makes sense to restrict the result set produced by this advisor. */ - val maxNumberOfIssuesPerRepository: Int? = null, + val maxNumberOfIssuesPerRepository: Int, /** * Determines the number of requests to the GitHub GraphQL API that are executed in parallel. Rather than querying @@ -95,7 +95,7 @@ data class GitHubDefectsConfiguration( * execution times for this advisor implementation. If unspecified, a default value for parallel executions as * defined in the _GitHubDefects_ class is used. */ - val parallelRequests: Int? = null + val parallelRequests: Int ) /** @@ -111,7 +111,7 @@ data class NexusIqConfiguration( * A URL to use as a base for browsing vulnerability details. Defaults to the server URL. */ @JsonInclude(JsonInclude.Include.NON_DEFAULT) - val browseUrl: String = serverUrl, + val browseUrl: String, /** * The username to use for authentication. If not both [username] and [password] are provided, authentication is