fix(scanoss): Fix SCANOSS-to-ORT model mapping in generateSummary()

isasmendiagus · isasmendiagus · commit 1683430489b9 · 2025-05-14T18:23:40.000+02:00
* Fix the association between sourceLocations and snippets in
  `generateSummary()`.

* Address confusion between `details.lines` and `result.filePath`
  when creating source locations.

* Extract primary PURL as identifier while storing all additional
  PURLs in additionalData for reference.

Signed-off-by: Agustin Isasmendi &lt;agustin.isasmendi@scanoss.com&gt;
diff --git a/plugins/scanners/scanoss/src/main/kotlin/ScanOssResultParser.kt b/plugins/scanners/scanoss/src/main/kotlin/ScanOssResultParser.kt
@@ -23,8 +23,11 @@ import com.scanoss.dto.ScanFileDetails
 import com.scanoss.dto.ScanFileResult
 import com.scanoss.dto.enums.MatchType
 
+import java.lang.invoke.MethodHandles
 import java.time.Instant
 
+import org.apache.logging.log4j.kotlin.loggerOf
+
 import org.ossreviewtoolkit.downloader.VcsHost
 import org.ossreviewtoolkit.model.CopyrightFinding
 import org.ossreviewtoolkit.model.LicenseFinding
@@ -38,6 +41,8 @@ import org.ossreviewtoolkit.utils.spdx.SpdxExpression
 import org.ossreviewtoolkit.utils.spdx.SpdxLicenseIdExpression
 import org.ossreviewtoolkit.utils.spdx.toExpression
 
+private val logger = loggerOf(MethodHandles.lookup().lookupClass())
+
 /**
  * Generate a summary from the given SCANOSS [result], using [startTime], [endTime] as metadata. This variant can be
  * used if the result is not read from a local file.
@@ -56,17 +61,24 @@ internal fun generateSummary(startTime: Instant, endTime: Instant, results: List
                 }
 
                 MatchType.snippet -> {
-                    val file = requireNotNull(details.file)
-                    val lines = requireNotNull(details.lines)
-                    val sourceLocations = convertLines(file, lines)
+                    val localFile = requireNotNull(result.filePath)
+                    val localLines = requireNotNull(details.lines)
+                    val sourceLocations = convertLines(localFile, localLines)
                     val snippets = getSnippets(details)
 
-                    snippets.forEach { snippet ->
-                        sourceLocations.forEach { sourceLocation ->
-                            // TODO: Aggregate the snippet by source file location.
-                            snippetFindings += SnippetFinding(sourceLocation, setOf(snippet))
+                    // The number of snippets should match the number of source locations.
+                    if (sourceLocations.size != snippets.size) {
+                        logger.warn {
+                            "Unexpected mismatch in '$localFile': " +
+                                "${sourceLocations.size} source locations vs ${snippets.size} snippets. " +
+                                "This indicates a potential issue with line range conversion."
                         }
                     }
+
+                    // Associate each source location with its corresponding snippet.
+                    sourceLocations.zip(snippets).forEach { (location, snippet) ->
+                        snippetFindings += SnippetFinding(location, setOf(snippet))
+                    }
                 }
 
                 MatchType.none -> {
@@ -134,32 +146,33 @@ private fun getCopyrightFindings(details: ScanFileDetails): List<CopyrightFindin
 }
 
 /**
- * Get the snippet findings from the given [details]. If a snippet returned by ScanOSS contains several Purls,
- * several snippets are created in ORT each containing a single Purl.
+ * Get the snippet findings from the given [details]. If a snippet returned by ScanOSS contains several PURLs,
+ * the function extracts the first PURL as the primary identifier while storing the remaining PURLs in additionalData
+ * to preserve the complete information.
  */
-private fun getSnippets(details: ScanFileDetails): Set<Snippet> {
+private fun getSnippets(details: ScanFileDetails): List<Snippet> {
     val matched = requireNotNull(details.matched)
-    val fileUrl = requireNotNull(details.fileUrl)
+    val ossFile = requireNotNull(details.file)
     val ossLines = requireNotNull(details.ossLines)
     val url = requireNotNull(details.url)
-    val purls = requireNotNull(details.purls)
+    val purls = requireNotNull(details.purls).toMutableList()
+    val primaryPurl = purls.removeFirstOrNull().orEmpty()
 
     val license = details.licenseDetails.orEmpty()
         .map { license -> SpdxExpression.parse(license.name) }
         .toExpression()?.sorted() ?: SpdxLicenseIdExpression(SpdxConstants.NOASSERTION)
 
     val score = matched.substringBeforeLast("%").toFloat()
-    val locations = convertLines(fileUrl, ossLines)
+    val ossLocations = convertLines(ossFile, ossLines)
     // TODO: No resolved revision is available. Should a ArtifactProvenance be created instead ?
     val vcsInfo = VcsHost.parseUrl(url.takeUnless { it == "none" }.orEmpty())
     val provenance = RepositoryProvenance(vcsInfo, ".")
 
-    return buildSet {
-        purls.forEach { purl ->
-            locations.forEach { snippetLocation ->
-                add(Snippet(score, snippetLocation, provenance, purl, license))
-            }
-        }
+    val additionalData = purls.associateWith { "" }
+
+    // Create one snippet per location, using the first PURL as the primary identifier.
+    return ossLocations.map { snippetLocation ->
+        Snippet(score, snippetLocation, provenance, primaryPurl, license, additionalData)
     }
 }
 
diff --git a/plugins/scanners/scanoss/src/test/kotlin/ScanOssResultParserTest.kt b/plugins/scanners/scanoss/src/test/kotlin/ScanOssResultParserTest.kt
@@ -119,7 +119,7 @@ class ScanOssResultParserTest : WordSpec({
                         Snippet(
                             98.0f,
                             TextLocation(
-                                "https://osskb.org/api/file_contents/6ff2427335b985212c9b79dfa795799f",
+                                "src/main/java/com/vdurmont/semver4j/Requirement.java",
                                 1,
                                 710
                             ),
@@ -135,8 +135,47 @@ class ScanOssResultParserTest : WordSpec({
             )
         }
 
+        "handle multiple PURLs by extracting first as primary and storing remaining in additionalData" {
+            val results = readResource("/scanoss-multiple-purls.json").let {
+                JsonUtils.toScanFileResultsFromObject(JsonUtils.toJsonObject(it))
+            }
+
+            val time = Instant.now()
+            val summary = generateSummary(time, time, results)
+
+            // Verify we have one finding per source location, not per PURL.
+            summary.snippetFindings should haveSize(2)
+
+            with(summary.snippetFindings.first()) {
+                // Check source location (local file).
+                sourceLocation shouldBe TextLocation("hung_task.c", 12, 150)
+
+                // Verify first PURL is extracted as primary identifier.
+                snippets should haveSize(1)
+                snippets.first().purl shouldBe "pkg:github/kdrag0n/proton_bluecross"
+
+                // Verify remaining PURLs are stored in additionalData.
+                snippets.first().additionalData shouldBe
+                    mapOf(
+                        "pkg:github/fake/fake_repository" to ""
+                    )
+
+                // Check OSS location.
+                snippets.first().location shouldBe
+                    TextLocation("kernel/hung_task.c", 10, 148)
+            }
+
+            // Verify same behavior for second snippet.
+            with(summary.snippetFindings.last()) {
+                sourceLocation shouldBe TextLocation("hung_task.c", 540, 561)
+                snippets.first().purl shouldBe "pkg:github/kdrag0n/proton_bluecross"
+                snippets.first().location shouldBe
+                    TextLocation("kernel/hung_task.c", 86, 107)
+            }
+        }
+
         "combine the same license from different sources into a single expression" {
-            // When the same license appears in multiple sources (like scancode and file_header),
+            // When a license appears in multiple sources (like scancode and file_header),
             // combine them into a single expression rather than duplicating.
             val results = readResource("/scanoss-snippet-same-license-multiple-sources.json").let {
                 JsonUtils.toScanFileResultsFromObject(JsonUtils.toJsonObject(it))
@@ -161,7 +200,7 @@ class ScanOssResultParserTest : WordSpec({
             }
         }
 
-        "handle empty license array with NOASSERTION" {
+        "handle empty license array in snippet findings with NOASSERTION" {
             // When a component has an empty licenses array, use NOASSERTION.
 
             val results = readResource("/scanoss-snippet-no-license-data.json").let {
diff --git a/plugins/scanners/scanoss/src/test/kotlin/ScanOssScannerDirectoryTest.kt b/plugins/scanners/scanoss/src/test/kotlin/ScanOssScannerDirectoryTest.kt
@@ -103,7 +103,7 @@ class ScanOssScannerDirectoryTest : StringSpec({
                         Snippet(
                             99.0f,
                             TextLocation(
-                                "https://osskb.org/api/file_contents/871fb0c5188c2f620d9b997e225b0095",
+                                "utils/src/main/kotlin/random-data-05-06-11.kt", // This is the remote oss filepath
                                 128,
                                 367
                             ),
diff --git a/plugins/scanners/scanoss/src/test/resources/scanoss-multiple-purls.json b/plugins/scanners/scanoss/src/test/resources/scanoss-multiple-purls.json
@@ -0,0 +1,60 @@
+{
+  "hung_task.c": [
+    {
+      "component": "proton_bluecross",
+      "file": "kernel/hung_task.c",
+      "file_hash": "581734935cfbe570d280a1265aaa2a6b",
+      "file_url": "https://api.scanoss.com/file_contents/581734935cfbe570d280a1265aaa2a6b",
+      "id": "snippet",
+      "latest": "17",
+      "licenses": [
+        {
+          "checklist_url": "https://www.osadl.org/fileadmin/checklists/unreflicenses/GPL-2.0-only.txt",
+          "copyleft": "yes",
+          "incompatible_with": "Apache-1.0, Apache-1.1, Apache-2.0, BSD-4-Clause, BSD-4-Clause-UC, BSD-4.3TAHOE, ECL-2.0, FTL, IJG, LicenseRef-scancode-bsla-no-advert, Minpack, OpenSSL, PHP-3.01, Python-2.0, zlib-acknowledgement, XFree86-1.1",
+          "name": "GPL-2.0-only",
+          "osadl_updated": "2025-02-10T14:26:00+0000",
+          "patent_hints": "yes",
+          "source": "scancode",
+          "url": "https://spdx.org/licenses/GPL-2.0-only.html"
+        },
+        {
+          "name": "GPL-2.0-only WITH Linux-syscall-note",
+          "source": "scancode",
+          "url": "https://spdx.org/licenses/GPL-2.0-only WITH Linux-syscall-note.html"
+        },
+        {
+          "checklist_url": "https://www.osadl.org/fileadmin/checklists/unreflicenses/GPL-2.0-only.txt",
+          "copyleft": "yes",
+          "incompatible_with": "Apache-1.0, Apache-1.1, Apache-2.0, BSD-4-Clause, BSD-4-Clause-UC, BSD-4.3TAHOE, ECL-2.0, FTL, IJG, LicenseRef-scancode-bsla-no-advert, Minpack, OpenSSL, PHP-3.01, Python-2.0, zlib-acknowledgement, XFree86-1.1",
+          "name": "GPL-2.0-only",
+          "osadl_updated": "2025-02-10T14:26:00+0000",
+          "patent_hints": "yes",
+          "source": "scancode",
+          "url": "https://spdx.org/licenses/GPL-2.0-only.html"
+        }
+      ],
+      "lines": "12-150,540-561",
+      "matched": "35%",
+      "oss_lines": "10-148,86-107",
+      "purl": [
+        "pkg:github/kdrag0n/proton_bluecross",
+        "pkg:github/fake/fake_repository"
+      ],
+      "release_date": "2019-02-21",
+      "server": {
+        "kb_version": {
+          "daily": "25.03.27",
+          "monthly": "25.03"
+        },
+        "version": "5.4.10"
+      },
+      "source_hash": "45dd1e50621a8a32f88fbe0251a470ab",
+      "status": "pending",
+      "url": "https://github.com/kdrag0n/proton_bluecross",
+      "url_hash": "a9c1c67f0930dc42dbd40c29e565bcdd",
+      "vendor": "kdrag0n",
+      "version": "15"
+    }
+  ]
+}

Original file line number	Diff line number	Diff line change
`@@ -103,7 +103,7 @@ class ScanOssScannerDirectoryTest : StringSpec({`
`103`	`103`	`Snippet(`
`104`	`104`	`99.0f,`
`105`	`105`	`TextLocation(`
`106`		`- "https://osskb.org/api/file_contents/871fb0c5188c2f620d9b997e225b0095",`
	`106`	`+ "utils/src/main/kotlin/random-data-05-06-11.kt", // This is the remote oss filepath`
`107`	`107`	`128,`
`108`	`108`	`367`
`109`	`109`	`),`