fix(scanner): Store only distinct results of package scanners

When calling storage writers to store provenance scan results produced by package scanners, make sure that writers are invoked only once for each distinct combination of provenance and scan result. Before this change, if there were multiple root projects in a repository, the writers were called for each of them with the same scan result. This could lead to duplication when storing results. Signed-off-by: Oliver Heger <[email protected]>
oss-review-toolkit · Jun 20, 2024 · d42a87a · d42a87a
1 parent 9c7494f
commit d42a87a
Show file tree

Hide file tree

Showing 2 changed files with 43 additions and 11 deletions.
diff --git a/scanner/src/main/kotlin/Scanner.kt b/scanner/src/main/kotlin/Scanner.kt
@@ -356,15 +356,27 @@ class Scanner(
                     "Finished scan of ${nestedProvenance.root} with package scanner '${scanner.name}'."
                 }
 
+                val provenanceScanResultsToStore = mutableSetOf<Pair<KnownProvenance, ScanResult>>()
                 packagesWithIncompleteScanResult.forEach processResults@{ pkg ->
                     val nestedProvenanceScanResult = scanResult.toNestedProvenanceScanResult(nestedProvenance)
                     controller.addNestedScanResult(scanner, nestedProvenanceScanResult)
 
                     // TODO: Run in coroutine.
                     if (scanner.writeToStorage) {
-                        storeNestedScanResult(pkg, nestedProvenanceScanResult)
+                        storePackageScanResult(pkg, nestedProvenanceScanResult)
+
+                        nestedProvenanceScanResult.scanResults.forEach { (provenance, scanResults) ->
+                            scanResults.forEach { scanResult ->
+                                provenanceScanResultsToStore += provenance to scanResult
+                            }
+                        }
                     }
                 }
+
+                // Store only deduplicated provenance scan results.
+                provenanceScanResultsToStore.forEach { (provenance, scanResult) ->
+                    storeProvenanceScanResult(provenance, scanResult)
+                }
             }
         }
     }
@@ -601,16 +613,6 @@ class Scanner(
         }
     }
 
-    private fun storeNestedScanResult(pkg: Package, nestedProvenanceScanResult: NestedProvenanceScanResult) {
-        storePackageScanResult(pkg, nestedProvenanceScanResult)
-
-        nestedProvenanceScanResult.scanResults.forEach { (provenance, scanResults) ->
-            scanResults.forEach { scanResult ->
-                storeProvenanceScanResult(provenance, scanResult)
-            }
-        }
-    }
-
     private fun storeProvenanceScanResult(provenance: KnownProvenance, scanResult: ScanResult) {
         storageWriters.filterIsInstance<ProvenanceBasedScanStorageWriter>().forEach { writer ->
             runCatching {

diff --git a/scanner/src/test/kotlin/ScannerTest.kt b/scanner/src/test/kotlin/ScannerTest.kt
@@ -22,6 +22,7 @@ package org.ossreviewtoolkit.scanner
 import io.kotest.assertions.throwables.shouldThrow
 import io.kotest.core.spec.style.WordSpec
 import io.kotest.matchers.collections.beEmpty
+import io.kotest.matchers.collections.shouldContainExactly
 import io.kotest.matchers.maps.beEmpty as beEmptyMap
 import io.kotest.matchers.maps.containExactly
 import io.kotest.matchers.nulls.shouldNotBeNull
@@ -178,6 +179,35 @@ class ScannerTest : WordSpec({
 
             verify(exactly = 0) { provenanceDownloader.download(any()) }
         }
+
+        "store only a single scan result per provenance" {
+            val packageScanner = FakePackageScannerWrapper()
+
+            val pkgWithVcs1 = Package.new(name = "project1").withValidVcs()
+            val pkgWithVcs2 = Package.new(name = "project2").withValidVcs()
+            val pkgWithVcs3 = Package.new(name = "project3").withValidVcs()
+            val packageScannerResult = createScanResult(pkgWithVcs1.repositoryProvenance(), packageScanner.details)
+
+            val scannerWrapper = spyk(packageScanner) {
+                every { scanPackage(any(), any()) } returns packageScannerResult
+            }
+
+            val storedScanResults = mutableListOf<ScanResult>()
+            val writer = object : ProvenanceBasedScanStorageWriter {
+                override fun write(scanResult: ScanResult) {
+                    storedScanResults.add(scanResult)
+                }
+            }
+
+            val scanner = createScanner(
+                packageScannerWrappers = listOf(scannerWrapper),
+                storageWriters = listOf(writer)
+            )
+
+            scanner.scan(setOf(pkgWithVcs1, pkgWithVcs2, pkgWithVcs3), createContext())
+
+            storedScanResults shouldContainExactly listOf(packageScannerResult)
+        }
     }
 
     "Scanning with a provenance scanner" should {