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,