From a23c65023a6b29687a958d5413cac8aba69270a2 Mon Sep 17 00:00:00 2001 From: Nicolas Nobelis Date: Mon, 27 Nov 2023 14:30:19 +0100 Subject: [PATCH] feat(fossid-webapp): Identify snippet choice entries that have been removed 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 --- .../scanners/fossid/src/main/kotlin/FossId.kt | 44 ++++-- .../src/main/kotlin/FossIdScanResults.kt | 40 +++++ .../test/kotlin/FossIdSnippetChoiceTest.kt | 140 ++++++++++++++++++ 3 files changed, 212 insertions(+), 12 deletions(-) diff --git a/plugins/scanners/fossid/src/main/kotlin/FossId.kt b/plugins/scanners/fossid/src/main/kotlin/FossId.kt index 985851aae2321..0339317a49e24 100644 --- a/plugins/scanners/fossid/src/main/kotlin/FossId.kt +++ b/plugins/scanners/fossid/src/main/kotlin/FossId.kt @@ -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 @@ -328,7 +329,7 @@ class FossId internal constructor( "false positives." } - val rawResults = getRawResults(scanCode) + val rawResults = getRawResults(scanCode, snippetChoices?.choices.orEmpty()) createResultSummary( startTime, provenance, @@ -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): RawResults { val identifiedFiles = service.listIdentifiedFiles(config.user, config.apiKey, scanCode) .checkResponse("list identified files") .data!! @@ -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() val snippets = runBlocking(Dispatchers.IO) { pendingFiles.map { @@ -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() val snippetLicenseFindings = mutableSetOf() val snippetFindings = mapSnippetFindings( @@ -920,7 +925,7 @@ class FossId internal constructor( snippetChoices, snippetLicenseFindings ) - markFilesWithChosenSnippetsAsIdentified( + val newlyMarkedFiles = markFilesWithChosenSnippetsAsIdentified( scanCode, snippetChoices, snippetFindings, @@ -928,6 +933,17 @@ class FossId internal constructor( 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 { @@ -956,6 +972,7 @@ 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, @@ -963,8 +980,9 @@ class FossId internal constructor( snippetFindings: Set, pendingFiles: List, licenseFindings: Set - ) { + ): List { val licenseFindingsByPath = licenseFindings.groupBy { it.location.path } + val result = mutableListOf() runBlocking(Dispatchers.IO) { val candidatePathsToMark = snippetChoices.groupBy({ it.given.sourceLocation.path }) { @@ -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 { @@ -1053,5 +1072,6 @@ class FossId internal constructor( requests.awaitAll() } + return result } } diff --git a/plugins/scanners/fossid/src/main/kotlin/FossIdScanResults.kt b/plugins/scanners/fossid/src/main/kotlin/FossIdScanResults.kt index 8c316d8b1738a..a6ba925b15d2e 100644 --- a/plugins/scanners/fossid/src/main/kotlin/FossIdScanResults.kt +++ b/plugins/scanners/fossid/src/main/kotlin/FossIdScanResults.kt @@ -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, + snippetChoices: List +): List = + 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. */ diff --git a/plugins/scanners/fossid/src/test/kotlin/FossIdSnippetChoiceTest.kt b/plugins/scanners/fossid/src/test/kotlin/FossIdSnippetChoiceTest.kt index 17e3b6c869b0b..f1f434fbc475c 100644 --- a/plugins/scanners/fossid/src/test/kotlin/FossIdSnippetChoiceTest.kt +++ b/plugins/scanners/fossid/src/test/kotlin/FossIdSnippetChoiceTest.kt @@ -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 @@ -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" { @@ -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" { @@ -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" { @@ -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()) + } + } } }) @@ -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,