Skip to content

Commit

Permalink
feat(model): Handle excluded affected paths in OrtResult.getIssues()
Browse files Browse the repository at this point in the history
A while ago, `Issue.affectedPath` has been introduced for issues which
are limited to a specific file. The goal was to filter out irrelevant
scan issues, in particular scan timeout errors for particular file which
is either outside of the VCS path, or matched by a path exclude.

So, in addition to filtering the issues against the VCS path [1], also
filter the issues against the path excludes. So, scan timeout errors
can now be fixed via a path exclude. However, the effect of this change
is for now limited to the evaluated model's statistics and not yet
visible in the WebApp / static HTML report. This will be implemented in
following changes.

Note: `Issue.affectedPath` is currently only set for scan timeout error,
      which is why only the scan issues are considered in the added
      logic. Also, the result is kept on a map, because matching all
      path excludes is relatively costly and faster to do only once.

[1]: 5839604

Signed-off-by: Frank Viernau <[email protected]>
  • Loading branch information
fviernau committed Apr 25, 2024
1 parent 1b7e954 commit 06db130
Show file tree
Hide file tree
Showing 5 changed files with 106 additions and 7 deletions.
27 changes: 26 additions & 1 deletion model/src/main/kotlin/OrtResult.kt
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,29 @@ data class OrtResult(
resolvedConfiguration.packageConfigurations.orEmpty().groupBy { it.id }
}

private val excludedAffectedPathIssuesForId: Map<Identifier, Set<Issue>> by lazy {
buildMap<Identifier, MutableSet<Issue>> {
scanner?.getAllScanResults().orEmpty().forEach { (id, scanResults) ->
scanResults.forEach { scanResult ->
val pathExcludes = getPathExcludes(id, scanResult.provenance)

scanResult.summary.issues.forEach { issue ->
if (issue.affectedPath != null && pathExcludes.any { it.matches(issue.affectedPath) }) {
getOrPut(id) { mutableSetOf() } += issue
}
}
}
}
}
}

private fun getPathExcludes(id: Identifier, provenance: Provenance) =
if (isProject(id)) {
repository.config.excludes.paths
} else {
getPackageConfigurations(id, provenance).flatMapTo(mutableSetOf()) { it.pathExcludes }
}

/**
* A map of projects and their excluded state. Calculating this map once brings massive performance improvements
* when querying projects in large analyzer results.
Expand Down Expand Up @@ -264,7 +287,9 @@ data class OrtResult(
if (omitExcluded && isExcluded(id)) return@mapNotNull null

val filteredIssues = issues.filterTo(mutableSetOf()) {
(!omitResolved || !isResolved(it)) && it.severity >= minSeverity
(!omitResolved || !isResolved(it))
&& it.severity >= minSeverity
&& it !in excludedAffectedPathIssuesForId[id].orEmpty()
}

filteredIssues.takeUnless { it.isEmpty() }?.let { id to it }
Expand Down
74 changes: 74 additions & 0 deletions model/src/test/kotlin/OrtResultTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ package org.ossreviewtoolkit.model

import io.kotest.assertions.throwables.shouldThrow
import io.kotest.core.spec.style.WordSpec
import io.kotest.matchers.collections.beEmpty
import io.kotest.matchers.collections.containExactlyInAnyOrder
import io.kotest.matchers.collections.haveSize
import io.kotest.matchers.collections.shouldContain
Expand Down Expand Up @@ -247,6 +248,79 @@ class OrtResultTest : WordSpec({

openIssues.map { it.message } shouldHaveSingleElement "Included issue"
}

"omit scan issues with excluded affected path" {
val projectId = Identifier("Maven:org.oss-review-toolkit:example-project:1.0")
val vcs = VcsInfo(
type = VcsType.GIT,
url = "https:/github.com/example.project.git",
revision = "0000000000000000000000000000000000000000",
path = ""
)

val ortResult = OrtResult.EMPTY.copy(
repository = Repository.EMPTY.copy(
vcs = vcs,
vcsProcessed = vcs,
config = RepositoryConfiguration(
excludes = Excludes(
paths = listOf(
PathExclude(
pattern = "test/**",
reason = PathExcludeReason.TEST_OF
)
)
)
)
),
analyzer = AnalyzerRun.EMPTY.copy(
result = AnalyzerResult.EMPTY.copy(
projects = setOf(
Project.EMPTY.copy(
id = projectId,
definitionFilePath = "pom.xml",
declaredLicenses = emptySet(),
vcsProcessed = vcs
)
)
)
),
scanner = ScannerRun.EMPTY.copy(
scanners = mapOf(projectId to setOf("ScanCode")),
provenances = setOf(
ProvenanceResolutionResult(
id = projectId,
packageProvenance = RepositoryProvenance(
vcsInfo = vcs,
resolvedRevision = vcs.revision
)
)
),
scanResults = setOf(
ScanResult(
provenance = RepositoryProvenance(
vcsInfo = vcs,
resolvedRevision = vcs.revision
),
scanner = ScannerDetails.EMPTY.copy(name = "ScanCode"),
summary = ScanSummary.EMPTY.copy(
issues = listOf(
Issue(
message = "Included issue",
source = "ScanCode",
affectedPath = "test/assets/asset.json"
)
)
)
)
)
)
)

val issues = ortResult.getOpenIssues()

issues should beEmpty()
}
}

"dependencyNavigator" should {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -915,10 +915,10 @@ statistics:
rule_violation_resolutions: 1
vulnerability_resolutions: 0
open_issues:
errors: 3
errors: 2
warnings: 0
hints: 0
severe: 3
severe: 2
open_rule_violations:
errors: 1
warnings: 1
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1006,10 +1006,10 @@
"vulnerability_resolutions" : 0
},
"open_issues" : {
"errors" : 3,
"errors" : 2,
"warnings" : 0,
"hints" : 0,
"severe" : 3
"severe" : 2
},
"open_rule_violations" : {
"errors" : 1,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -919,10 +919,10 @@ statistics:
rule_violation_resolutions: 1
vulnerability_resolutions: 0
open_issues:
errors: 3
errors: 2
warnings: 0
hints: 0
severe: 3
severe: 2
open_rule_violations:
errors: 1
warnings: 1
Expand Down

0 comments on commit 06db130

Please sign in to comment.