Skip to content

Commit

Permalink
feat(fossid-webapp): Identify snippet choice entries that have been r…
Browse files Browse the repository at this point in the history 
…emoved

When one or several source locations of a 'marked as identified' file have
been deleted from the snippet choice section of the .ort.yml file, put
again the file to the 'pending' state. Its snippets will be then listed as
for all pending files.
To find out if some locations have been removed, the comment in the marked
as identified file is used to compare the location counts with the entries
in the .ort.yml file.
Please note that FossID UI should not be used by the user to manually
change the file state: The snippet choice feature is non-interactive and
doing so would disturb the logic.

Signed-off-by: Nicolas Nobelis <[email protected]>
  • Loading branch information
@nnobelis @mnonnenmacher
nnobelis authored and mnonnenmacher committed Mar 20, 2024
1 parent d7af736 commit a23c650
Show file tree
Hide file tree
Showing 3 changed files with 212 additions and 12 deletions.
44 changes: 32 additions & 12 deletions plugins/scanners/fossid/src/main/kotlin/FossId.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ import org.ossreviewtoolkit.clients.fossid.model.rules.RuleType
import org.ossreviewtoolkit.clients.fossid.model.status.DownloadStatus
import org.ossreviewtoolkit.clients.fossid.model.status.ScanStatus
import org.ossreviewtoolkit.clients.fossid.runScan
import org.ossreviewtoolkit.clients.fossid.unmarkAsIdentified
import org.ossreviewtoolkit.downloader.VersionControlSystem
import org.ossreviewtoolkit.model.Issue
import org.ossreviewtoolkit.model.LicenseFinding
Expand Down Expand Up @@ -328,7 +329,7 @@ class FossId internal constructor(
"false positives."
}

val rawResults = getRawResults(scanCode)
val rawResults = getRawResults(scanCode, snippetChoices?.choices.orEmpty())
createResultSummary(
startTime,
provenance,
Expand Down Expand Up @@ -820,7 +821,7 @@ class FossId internal constructor(
* Get the different kind of results from the scan with [scanCode]
*/
@Suppress("UnsafeCallOnNullableType")
private suspend fun getRawResults(scanCode: String): RawResults {
private suspend fun getRawResults(scanCode: String, snippetChoices: List<SnippetChoice>): RawResults {
val identifiedFiles = service.listIdentifiedFiles(config.user, config.apiKey, scanCode)
.checkResponse("list identified files")
.data!!
Expand All @@ -840,11 +841,21 @@ class FossId internal constructor(

val pendingFiles = service.listPendingFiles(config.user, config.apiKey, scanCode)
.checkResponse("list pending files")
.data!!
.data!!.toMutableList()
logger.info {
"${pendingFiles.size} pending files have been returned for scan '$scanCode'."
}

pendingFiles += listUnmatchedSnippetChoices(markedAsIdentifiedFiles, snippetChoices).also { newPendingFiles ->
newPendingFiles.map {
logger.info {
"Marked as identified file '$it' is not in .ort.yml anymore or its configuration has been " +
"altered: putting it again as 'pending'."
}
service.unmarkAsIdentified(config.user, config.apiKey, scanCode, it, false)
}
}

val matchedLines = mutableMapOf<Int, MatchedLines>()
val snippets = runBlocking(Dispatchers.IO) {
pendingFiles.map {
Expand Down Expand Up @@ -904,13 +915,7 @@ class FossId internal constructor(
): ScanResult {
// TODO: Maybe get issues from FossID (see has_failed_scan_files, get_failed_files and maybe get_scan_log).

val issues = mutableListOf(
Issue(
source = name,
message = "This scan has ${rawResults.listPendingFiles.size} file(s) pending identification in FossID.",
severity = Severity.HINT
)
)
val issues = mutableListOf<Issue>()

val snippetLicenseFindings = mutableSetOf<LicenseFinding>()
val snippetFindings = mapSnippetFindings(
Expand All @@ -920,14 +925,25 @@ class FossId internal constructor(
snippetChoices,
snippetLicenseFindings
)
markFilesWithChosenSnippetsAsIdentified(
val newlyMarkedFiles = markFilesWithChosenSnippetsAsIdentified(
scanCode,
snippetChoices,
snippetFindings,
rawResults.listPendingFiles,
snippetLicenseFindings
)

val pendingFilesCount = (rawResults.listPendingFiles - newlyMarkedFiles.toSet()).size

issues.add(
0,
Issue(
source = name,
message = "This scan has $pendingFilesCount file(s) pending identification in FossID.",
severity = Severity.HINT
)
)

val ignoredFiles = rawResults.listIgnoredFiles.associateBy { it.path }

val (licenseFindings, copyrightFindings) = rawResults.markedAsIdentifiedFiles.ifEmpty {
Expand Down Expand Up @@ -956,15 +972,17 @@ class FossId internal constructor(
* non-chosen source location remaining. Only files in [listPendingFiles] are marked.
* Files marked as identified have a license identification and a source location (stored in a comment), using
* [licenseFindings] as reference.
* Returns the list of files that have been marked as identified.
*/
private fun markFilesWithChosenSnippetsAsIdentified(
scanCode: String,
snippetChoices: List<SnippetChoice> = emptyList(),
snippetFindings: Set<SnippetFinding>,
pendingFiles: List<String>,
licenseFindings: Set<LicenseFinding>
) {
): List<String> {
val licenseFindingsByPath = licenseFindings.groupBy { it.location.path }
val result = mutableListOf<String>()

runBlocking(Dispatchers.IO) {
val candidatePathsToMark = snippetChoices.groupBy({ it.given.sourceLocation.path }) {
Expand All @@ -987,6 +1005,7 @@ class FossId internal constructor(

requests += async {
service.markAsIdentified(config.user, config.apiKey, scanCode, path, false)
result += path
}

val filteredSnippetChoicesByPath = snippetChoices.filter {
Expand Down Expand Up @@ -1053,5 +1072,6 @@ class FossId internal constructor(

requests.awaitAll()
}
return result
}
}
40 changes: 40 additions & 0 deletions plugins/scanners/fossid/src/main/kotlin/FossIdScanResults.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,46 @@ private fun getLicenseFindingFromSnippetChoice(
}
}

/**
* Check all [markedAsIdentifiedFiles] if their snippet choices locations count or non-relevant snippets locations count
* matches the ones stored in the [OrtComment]: When not, it means some of this configuration has been removed and the
* files should be considered as pending again. Such files are returned.
*/
internal fun listUnmatchedSnippetChoices(
markedAsIdentifiedFiles: List<MarkedAsIdentifiedFile>,
snippetChoices: List<SnippetChoice>
): List<String> =
markedAsIdentifiedFiles.filterNot { markedAsIdentifiedFile ->
val markedFileName = markedAsIdentifiedFile.getFileName()
val snippetChoicesByName = snippetChoices.filter {
it.given.sourceLocation.path == markedFileName
}
val comment = markedAsIdentifiedFile.comments.values.firstOrNull {
it.comment.contains(ORT_NAME)
}?.runCatching {
jsonMapper.readValue(this.comment, OrtComment::class.java)
}?.onFailure {
logger.warn {
"File $markedFileName is marked as identified but it does not have a valid comment. " +
"It will be ignored. Exception: ${it.message}."
}
}?.getOrDefault(null)

when {
comment != null -> {
val snippetChoicesCount = snippetChoicesByName.count {
it.choice.reason == SnippetChoiceReason.ORIGINAL_FINDING
}
val notRelevantSnippetsCount = snippetChoicesByName.count {
it.choice.reason == SnippetChoiceReason.NO_RELEVANT_FINDING
}
snippetChoicesCount == comment.ort.snippetChoicesCount &&
notRelevantSnippetsCount == comment.ort.notRelevantSnippetsCount
}
else -> true
}
}.map { it.getFileName() }

/**
* Return the [PurlType] as determined from the given [url], or [PurlType.GENERIC] if there is no match.
*/
Expand Down
140 changes: 140 additions & 0 deletions plugins/scanners/fossid/src/test/kotlin/FossIdSnippetChoiceTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ import org.ossreviewtoolkit.clients.fossid.model.result.MatchType
import org.ossreviewtoolkit.clients.fossid.model.result.MatchedLines
import org.ossreviewtoolkit.clients.fossid.model.result.Snippet
import org.ossreviewtoolkit.clients.fossid.model.status.ScanStatus
import org.ossreviewtoolkit.clients.fossid.unmarkAsIdentified
import org.ossreviewtoolkit.downloader.VersionControlSystem
import org.ossreviewtoolkit.model.Severity
import org.ossreviewtoolkit.model.TextLocation
Expand Down Expand Up @@ -289,6 +290,10 @@ class FossIdSnippetChoiceTest : WordSpec({
val comment = jsonMapper.writeValueAsString(OrtComment(payload))
service.addFileComment(USER, API_KEY, scanCode, FILE_1, comment)
}

summary.issues.forAtLeastOne {
it.message shouldBe "This scan has 0 file(s) pending identification in FossID."
}
}

"mark a file with only non relevant snippets for a given snippet location as identified" {
Expand Down Expand Up @@ -330,6 +335,10 @@ class FossIdSnippetChoiceTest : WordSpec({
service.markAsIdentified(USER, API_KEY, scanCode, FILE_1, any())
}
summary.issues.filter { it.severity > Severity.HINT } should beEmpty()

summary.issues.forAtLeastOne {
it.message shouldBe "This scan has 0 file(s) pending identification in FossID."
}
}

"not mark a file with some non relevant snippets for a given snippet location as identified" {
Expand Down Expand Up @@ -379,6 +388,10 @@ class FossIdSnippetChoiceTest : WordSpec({
service.markAsIdentified(USER, API_KEY, scanCode, FILE_1, any())
}
summary.issues.filter { it.severity > Severity.HINT } should beEmpty()

summary.issues.forAtLeastOne {
it.message shouldBe "This scan has 1 file(s) pending identification in FossID."
}
}

"mark a file with only chosen and non relevant snippets for a given snippet location as identified" {
Expand Down Expand Up @@ -587,6 +600,124 @@ class FossIdSnippetChoiceTest : WordSpec({
summary.issues.filter { it.severity > Severity.HINT } should beEmpty()
summary.snippetFindings should beEmpty()
}

"put a marked as identified file back to pending if it has no snippet choice" {
val projectCode = projectCode(PROJECT)
val scanCode = scanCode(PROJECT, null)
val config = createConfig(deltaScans = false, fetchSnippetMatchedLines = true)
val vcsInfo = createVcsInfo()
val scan = createScan(vcsInfo.url, "${vcsInfo.revision}_other", scanCode)
val pkgId = createIdentifier(index = 42)

val choiceLocation = TextLocation(FILE_1, 10, 20)
val payload = OrtCommentPayload(mapOf("MIT" to listOf(choiceLocation)), 1, 0)
val comment = jsonMapper.writeValueAsString(mapOf(ORT_NAME to payload))
val markedAsIdentifiedFile = createMarkAsIdentifiedFile("MIT", FILE_1, comment)
val service = FossIdRestService.create(config.serverUrl)
.expectProjectRequest(projectCode)
.expectListScans(projectCode, listOf(scan))
.expectCheckScanStatus(scanCode, ScanStatus.FINISHED)
.expectCreateScan(projectCode, scanCode, vcsInfo, "")
.expectDownload(scanCode)
.mockFiles(
scanCode,
markedFiles = listOf(markedAsIdentifiedFile),
snippets = listOf(
createSnippet(0, FILE_1, PURL_1),
createSnippet(1, FILE_1, PURL_2),
createSnippet(2, FILE_1, PURL_3)
),
matchedLines = mapOf(
0 to MatchedLines.create((10..20).toList(), (10..20).toList()),
1 to MatchedLines.create((10..20).toList(), (10..20).toList()),
2 to MatchedLines.create((20..30).toList(), (20..30).toList())
)
)
.expectUnmarkAsIdentified(scanCode, FILE_1)

val fossId = createFossId(config)

val summary = fossId.scan(createPackage(pkgId, vcsInfo), snippetChoices = emptyList()).summary

summary.issues.forAtLeastOne {
it.message shouldBe "This scan has 1 file(s) pending identification in FossID."
}
summary.issues.filter { it.severity > Severity.HINT } should beEmpty()
summary.snippetFindings shouldHaveSize 2
summary.snippetFindings.first().apply {
sourceLocation.path shouldBe FILE_1
snippets shouldHaveSize 2
snippets.map { it.purl } shouldBe listOf(PURL_1, PURL_2)
}
summary.snippetFindings.last().apply {
sourceLocation.path shouldBe FILE_1
snippets shouldHaveSize 1
snippets.map { it.purl } shouldBe listOf(PURL_3)
}
coVerify {
service.unmarkAsIdentified(USER, API_KEY, scanCode, FILE_1, any())
}
}

"put a marked as identified file back to pending if some of its snippet choices have been deleted" {
val projectCode = projectCode(PROJECT)
val scanCode = scanCode(PROJECT, null)
val config = createConfig(deltaScans = false, fetchSnippetMatchedLines = true)
val vcsInfo = createVcsInfo()
val scan = createScan(vcsInfo.url, "${vcsInfo.revision}_other", scanCode)
val pkgId = createIdentifier(index = 42)

val choiceLocation = TextLocation(FILE_1, 10, 20)
val payload = OrtCommentPayload(mapOf("MIT" to listOf(choiceLocation)), 2, 0)
val comment = jsonMapper.writeValueAsString(mapOf(ORT_NAME to payload))
val markedAsIdentifiedFile = createMarkAsIdentifiedFile("MIT", FILE_1, comment)
val service = FossIdRestService.create(config.serverUrl)
.expectProjectRequest(projectCode)
.expectListScans(projectCode, listOf(scan))
.expectCheckScanStatus(scanCode, ScanStatus.FINISHED)
.expectCreateScan(projectCode, scanCode, vcsInfo, "")
.expectDownload(scanCode)
.mockFiles(
scanCode,
markedFiles = listOf(markedAsIdentifiedFile),
snippets = listOf(
createSnippet(0, FILE_1, PURL_1),
createSnippet(1, FILE_1, PURL_2),
createSnippet(2, FILE_1, PURL_3)
),
matchedLines = mapOf(
0 to MatchedLines.create((10..20).toList(), (10..20).toList()),
1 to MatchedLines.create((10..20).toList(), (10..20).toList()),
2 to MatchedLines.create((20..30).toList(), (20..30).toList())
)
)
.expectUnmarkAsIdentified(scanCode, FILE_1)

val snippetChoices = createSnippetChoices(
vcsInfo.url,
createSnippetChoice(choiceLocation, PURL_1, "")
)
val fossId = createFossId(config)

val summary = fossId.scan(createPackage(pkgId, vcsInfo), snippetChoices = snippetChoices).summary

summary.issues.forAtLeastOne {
it.message shouldBe "This scan has 1 file(s) pending identification in FossID."
}
summary.issues.filter { it.severity > Severity.HINT } should beEmpty()
// Two snippets have been chosen according to the count in the comment. However, the .ort.yml file contains
// only one snippet choice: Therefore, the file should be 'pending' again and the remaining snippet should
// be returned.
summary.snippetFindings shouldHaveSize 1
summary.snippetFindings.last().apply {
sourceLocation.path shouldBe FILE_1
snippets shouldHaveSize 1
snippets.map { it.purl } shouldBe listOf(PURL_3)
}
coVerify {
service.unmarkAsIdentified(USER, API_KEY, scanCode, FILE_1, any())
}
}
}
})

Expand Down Expand Up @@ -651,6 +782,15 @@ fun FossIdServiceWithVersion.expectMarkAsIdentified(scanCode: String, path: Stri
return this
}

/**
* Prepare this service mock to expect an unmark as identified call for the given [scanCode] and [path].
*/
fun FossIdServiceWithVersion.expectUnmarkAsIdentified(scanCode: String, path: String): FossIdServiceWithVersion {
coEvery { unmarkAsIdentified(USER, API_KEY, scanCode, path, any()) } returns
EntityResponseBody(status = 1)
return this
}

internal fun createSnippet(index: Int, file: String, purl: String): Snippet =
Snippet(
index,
Expand Down

0 comments on commit a23c650

Please sign in to comment.