From 87db6d4cf922abfc995a5168c325e6f17a1ae92e Mon Sep 17 00:00:00 2001 From: Martin Nonnenmacher Date: Tue, 14 Nov 2023 19:42:43 +0100 Subject: [PATCH] feat(scanner): Use the new properties that configure scan storage usage Use the previously introduced properties `readFromStorage` and `writeToStorage` to decide if scan results are read from or written to the configured scan storages. This provides more flexibility to the user than the previous mechanism which relied solely on the `matcher` property. This is mainly useful when using ORT programmatically in which case it can make sense to store scan results for other purposes than reusing them for follow-up scans. If `readFromStorage` is set to `true` but the `matcher` is `null`, scan results will not be read, as without the `matcher` there is no way to determine matching scan results. This is currently only the case for the `FossId` scanner which hardcodes the `matcher` to `null`. If the properties are not configured by the user, the defaults will behave exactly like before this change, meaning that scan results are read and written if the `matcher` is not `null`, and are not read and written otherwise. Signed-off-by: Martin Nonnenmacher --- scanner/src/main/kotlin/Scanner.kt | 4 +- scanner/src/test/kotlin/ScannerTest.kt | 92 +++++++++++++++++++++++++- 2 files changed, 92 insertions(+), 4 deletions(-) diff --git a/scanner/src/main/kotlin/Scanner.kt b/scanner/src/main/kotlin/Scanner.kt index 24a4506bb91fd..951358c0f7c4a 100644 --- a/scanner/src/main/kotlin/Scanner.kt +++ b/scanner/src/main/kotlin/Scanner.kt @@ -370,7 +370,7 @@ class Scanner( controller.addNestedScanResult(scanner, nestedProvenanceScanResult) // TODO: Run in coroutine. - if (scanner.matcher != null) { + if (scanner.writeToStorage) { storeNestedScanResult(pkg, nestedProvenanceScanResult) } } @@ -503,6 +503,7 @@ class Scanner( private fun readStoredPackageResults(controller: ScanController) { controller.scanners.forEach { scanner -> val scannerMatcher = scanner.matcher ?: return@forEach + if (!scanner.readFromStorage) return@forEach controller.packages.forEach pkg@{ pkg -> val nestedProvenance = controller.findNestedProvenance(pkg.id) ?: return@pkg @@ -532,6 +533,7 @@ class Scanner( private fun readStoredProvenanceResults(controller: ScanController) { controller.scanners.forEach { scanner -> val scannerMatcher = scanner.matcher ?: return@forEach + if (!scanner.readFromStorage) return@forEach controller.getAllProvenances().forEach provenance@{ provenance -> if (controller.hasScanResult(scanner, provenance)) return@provenance diff --git a/scanner/src/test/kotlin/ScannerTest.kt b/scanner/src/test/kotlin/ScannerTest.kt index a391d0260b7eb..f2fe8e5d6e2ae 100644 --- a/scanner/src/test/kotlin/ScannerTest.kt +++ b/scanner/src/test/kotlin/ScannerTest.kt @@ -66,6 +66,7 @@ import org.ossreviewtoolkit.scanner.provenance.ProvenanceDownloader import org.ossreviewtoolkit.utils.ort.createOrtTempDir import org.ossreviewtoolkit.utils.test.shouldNotBeNull +@Suppress("LargeClass") class ScannerTest : WordSpec({ "Creating the scanner" should { "throw an exception if no scanner wrappers are provided" { @@ -763,12 +764,31 @@ class ScannerTest : WordSpec({ } } - "scanning with a scanner that does not provide a matcher" should { - "not store the scan results" { + "scanning with a storage writer" should { + "store the scan results if writeToStorage is set to true" { + val pkgWithArtifact = Package.new(name = "artifact").withValidSourceArtifact() + + val scannerWrapper = FakePackageScannerWrapper() + + val writer = spyk(FakeProvenanceBasedStorageWriter()) + + val scanner = createScanner( + storageWriters = listOf(writer), + packageScannerWrappers = listOf(scannerWrapper) + ) + + scanner.scan(setOf(pkgWithArtifact), createContext()) + + verify(exactly = 1) { + writer.write(any()) + } + } + + "not store the scan results if writeToStorage is set to false" { val pkgWithArtifact = Package.new(name = "artifact").withValidSourceArtifact() val scannerWrapper = spyk(FakePackageScannerWrapper()) { - every { matcher } returns null + every { writeToStorage } returns false } val writer = spyk(FakeProvenanceBasedStorageWriter()) @@ -786,6 +806,72 @@ class ScannerTest : WordSpec({ } } + "scanning with a storage reader" should { + "read the scan results if the matcher is not null and readFromStorage is set to true" { + val pkgWithArtifact = Package.new(name = "artifact").withValidSourceArtifact() + + val scannerWrapper = FakePackageScannerWrapper() + + val reader = spyk(FakeProvenanceBasedStorageReader(scannerWrapper.details)) + + val scanner = createScanner( + storageReaders = listOf(reader), + packageScannerWrappers = listOf(scannerWrapper) + ) + + scanner.scan(setOf(pkgWithArtifact), createContext()) + + verify(exactly = 1) { + reader.read(any()) + reader.read(any(), any()) + } + } + + "not read the scan results if the matcher is null" { + val pkgWithArtifact = Package.new(name = "artifact").withValidSourceArtifact() + + val scannerWrapper = spyk(FakePackageScannerWrapper()) { + every { matcher } returns null + } + + val reader = spyk(FakeProvenanceBasedStorageReader(scannerWrapper.details)) + + val scanner = createScanner( + storageReaders = listOf(reader), + packageScannerWrappers = listOf(scannerWrapper) + ) + + scanner.scan(setOf(pkgWithArtifact), createContext()) + + verify(exactly = 0) { + reader.read(any()) + reader.read(any(), any()) + } + } + + "not read the scan results if readFromStorage is set to false" { + val pkgWithArtifact = Package.new(name = "artifact").withValidSourceArtifact() + + val scannerWrapper = spyk(FakePackageScannerWrapper()) { + every { readFromStorage } returns false + } + + val reader = spyk(FakeProvenanceBasedStorageReader(scannerWrapper.details)) + + val scanner = createScanner( + storageReaders = listOf(reader), + packageScannerWrappers = listOf(scannerWrapper) + ) + + scanner.scan(setOf(pkgWithArtifact), createContext()) + + verify(exactly = 0) { + reader.read(any()) + reader.read(any(), any()) + } + } + } + // TODO: Add tests for combinations of different types of storage readers and writers. // TODO: Add tests for using multiple types of scanner wrappers at once. // TODO: Add tests for a complex example with multiple types of scanner wrappers and storages.