Skip to content

Commit 4aaacb1

Browse files
committed
refactor: Make ProvenanceFileStorage operate on stream instead of files
Resolves #7118. Signed-off-by: Sebastian Schuberth <[email protected]>
1 parent bf94393 commit 4aaacb1

File tree

6 files changed

+63
-114
lines changed

6 files changed

+63
-114
lines changed

model/src/funTest/kotlin/utils/PostgresProvenanceFileStorageFunTest.kt

Lines changed: 18 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,10 @@
1919

2020
package org.ossreviewtoolkit.model.utils
2121

22-
import io.kotest.core.TestConfiguration
2322
import io.kotest.core.spec.style.WordSpec
24-
import io.kotest.engine.spec.tempfile
2523
import io.kotest.matchers.shouldBe
2624

27-
import java.io.File
25+
import java.io.InputStream
2826

2927
import org.ossreviewtoolkit.model.ArtifactProvenance
3028
import org.ossreviewtoolkit.model.Hash
@@ -46,18 +44,6 @@ private val VCS_PROVENANCE = RepositoryProvenance(
4644
resolvedRevision = "0000000000000000000000000000000000000000"
4745
)
4846

49-
private fun File.readTextAndDelete(): String {
50-
val text = readText()
51-
delete()
52-
53-
return text
54-
}
55-
56-
private fun TestConfiguration.writeTempFile(content: String): File =
57-
tempfile().apply {
58-
writeText(content)
59-
}
60-
6147
class PostgresProvenanceFileStorageFunTest : WordSpec({
6248
val postgresListener = PostgresListener()
6349
lateinit var storage: PostgresProvenanceFileStorage
@@ -68,35 +54,35 @@ class PostgresProvenanceFileStorageFunTest : WordSpec({
6854
storage = PostgresProvenanceFileStorage(postgresListener.dataSource, FileArchiverConfiguration.TABLE_NAME)
6955
}
7056

71-
"hasFile()" should {
72-
"return false when no file for the given provenance has been added" {
73-
storage.hasFile(VCS_PROVENANCE) shouldBe false
57+
"hasData()" should {
58+
"return false when no data for the given provenance has been added" {
59+
storage.hasData(VCS_PROVENANCE) shouldBe false
7460
}
7561

76-
"return true when a file for the given provenance has been added" {
77-
storage.putFile(VCS_PROVENANCE, writeTempFile("content"))
62+
"return true when data for the given provenance has been added" {
63+
storage.putData(VCS_PROVENANCE, InputStream.nullInputStream())
7864

79-
storage.hasFile(VCS_PROVENANCE) shouldBe true
65+
storage.hasData(VCS_PROVENANCE) shouldBe true
8066
}
8167
}
8268

83-
"putFile()" should {
84-
"return the file corresponding to the given provenance given such file has been added" {
85-
storage.putFile(VCS_PROVENANCE, writeTempFile("VCS"))
86-
storage.putFile(SOURCE_ARTIFACT_PROVENANCE, writeTempFile("source artifact"))
69+
"putData()" should {
70+
"return the data corresponding to the given provenance given such data has been added" {
71+
storage.putData(VCS_PROVENANCE, "VCS".byteInputStream())
72+
storage.putData(SOURCE_ARTIFACT_PROVENANCE, "source artifact".byteInputStream())
8773

88-
storage.getFile(VCS_PROVENANCE) shouldNotBeNull { readTextAndDelete() shouldBe "VCS" }
89-
storage.getFile(SOURCE_ARTIFACT_PROVENANCE) shouldNotBeNull {
90-
readTextAndDelete() shouldBe "source artifact"
74+
storage.getData(VCS_PROVENANCE) shouldNotBeNull { String(use { readBytes() }) shouldBe "VCS" }
75+
storage.getData(SOURCE_ARTIFACT_PROVENANCE) shouldNotBeNull {
76+
String(use { readBytes() }) shouldBe "source artifact"
9177
}
9278
}
9379

9480
"return the overwritten file corresponding to the given provenance" {
95-
storage.putFile(SOURCE_ARTIFACT_PROVENANCE, writeTempFile("source artifact"))
96-
storage.putFile(SOURCE_ARTIFACT_PROVENANCE, writeTempFile("source artifact updated"))
81+
storage.putData(SOURCE_ARTIFACT_PROVENANCE, "source artifact".byteInputStream())
82+
storage.putData(SOURCE_ARTIFACT_PROVENANCE, "source artifact updated".byteInputStream())
9783

98-
storage.getFile(SOURCE_ARTIFACT_PROVENANCE) shouldNotBeNull {
99-
readTextAndDelete() shouldBe "source artifact updated"
84+
storage.getData(SOURCE_ARTIFACT_PROVENANCE) shouldNotBeNull {
85+
String(use { readBytes() }) shouldBe "source artifact updated"
10086
}
10187
}
10288
}

model/src/main/kotlin/utils/FileArchiver.kt

Lines changed: 11 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
package org.ossreviewtoolkit.model.utils
2121

2222
import java.io.File
23-
import java.io.IOException
2423

2524
import kotlin.time.measureTime
2625
import kotlin.time.measureTimedValue
@@ -63,7 +62,7 @@ class FileArchiver(
6362
/**
6463
* Return whether an archive corresponding to [provenance] exists.
6564
*/
66-
fun hasArchive(provenance: KnownProvenance): Boolean = storage.hasFile(provenance)
65+
fun hasArchive(provenance: KnownProvenance): Boolean = storage.hasData(provenance)
6766

6867
/**
6968
* Archive all files in [directory] matching any of the configured patterns in the [storage].
@@ -91,7 +90,7 @@ class FileArchiver(
9190

9291
logger.info { "Archived directory '$directory' in $zipDuration." }
9392

94-
val writeDuration = measureTime { storage.putFile(provenance, zipFile) }
93+
val writeDuration = measureTime { storage.putData(provenance, zipFile.inputStream()) }
9594

9695
logger.info { "Wrote archive of directory '$directory' to storage in $writeDuration." }
9796

@@ -102,26 +101,22 @@ class FileArchiver(
102101
* Unarchive the archive corresponding to [provenance].
103102
*/
104103
fun unarchive(directory: File, provenance: KnownProvenance): Boolean {
105-
val (zipFile, readDuration) = measureTimedValue { storage.getFile(provenance) }
104+
val (zipInputStream, readDuration) = measureTimedValue { storage.getData(provenance) }
106105

107106
logger.info { "Read archive of directory '$directory' from storage in $readDuration." }
108107

109-
if (zipFile == null) return false
108+
if (zipInputStream == null) return false
110109

111-
return try {
112-
val unzipDuration = measureTime { zipFile.unpackZip(directory) }
110+
return runCatching {
111+
val unzipDuration = measureTime { zipInputStream.unpackZip(directory) }
113112

114-
logger.info { "Unarchived directory '$directory' in $unzipDuration." }
113+
logger.info { "Unarchived data for $provenance to '$directory' in $unzipDuration." }
115114

116115
true
117-
} catch (e: IOException) {
118-
e.showStackTrace()
116+
}.onFailure {
117+
it.showStackTrace()
119118

120-
logger.error { "Could not extract ${zipFile.absolutePath}: ${e.collectMessages()}" }
121-
122-
false
123-
} finally {
124-
zipFile.delete()
125-
}
119+
logger.error { "Failed to unarchive data for $provenance: ${it.collectMessages()}" }
120+
}.isSuccess
126121
}
127122
}

model/src/main/kotlin/utils/FileProvenanceFileStorage.kt

Lines changed: 10 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,7 @@
1919

2020
package org.ossreviewtoolkit.model.utils
2121

22-
import java.io.File
23-
import java.io.IOException
22+
import java.io.InputStream
2423

2524
import org.apache.logging.log4j.kotlin.Logging
2625

@@ -29,7 +28,6 @@ import org.ossreviewtoolkit.model.HashAlgorithm
2928
import org.ossreviewtoolkit.model.KnownProvenance
3029
import org.ossreviewtoolkit.model.RepositoryProvenance
3130
import org.ossreviewtoolkit.utils.common.collectMessages
32-
import org.ossreviewtoolkit.utils.ort.createOrtTempFile
3331
import org.ossreviewtoolkit.utils.ort.storage.FileStorage
3432

3533
/**
@@ -55,34 +53,24 @@ class FileProvenanceFileStorage(
5553
}
5654
}
5755

58-
override fun hasFile(provenance: KnownProvenance): Boolean {
56+
override fun hasData(provenance: KnownProvenance): Boolean {
5957
val filePath = getFilePath(provenance)
6058

6159
return storage.exists(filePath)
6260
}
6361

64-
override fun putFile(provenance: KnownProvenance, file: File) {
65-
storage.write(getFilePath(provenance), file.inputStream())
62+
override fun putData(provenance: KnownProvenance, data: InputStream) {
63+
storage.write(getFilePath(provenance), data)
6664
}
6765

68-
override fun getFile(provenance: KnownProvenance): File? {
66+
override fun getData(provenance: KnownProvenance): InputStream? {
6967
val filePath = getFilePath(provenance)
7068

71-
val file = createOrtTempFile(suffix = File(filename).extension)
72-
73-
return try {
74-
storage.read(filePath).use { inputStream ->
75-
file.outputStream().use { outputStream ->
76-
inputStream.copyTo(outputStream)
77-
}
78-
}
79-
80-
file
81-
} catch (e: IOException) {
82-
logger.error { "Could not read from $filePath: ${e.collectMessages()}" }
83-
84-
null
85-
}
69+
return runCatching {
70+
storage.read(filePath)
71+
}.onFailure {
72+
logger.error { "Could not read from $filePath: ${it.collectMessages()}" }
73+
}.getOrNull()
8674
}
8775

8876
private fun getFilePath(provenance: KnownProvenance): String = "${provenance.hash()}/$filename"

model/src/main/kotlin/utils/PostgresProvenanceFileStorage.kt

Lines changed: 8 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,7 @@
1919

2020
package org.ossreviewtoolkit.model.utils
2121

22-
import java.io.File
23-
import java.io.IOException
22+
import java.io.InputStream
2423

2524
import javax.sql.DataSource
2625

@@ -44,7 +43,6 @@ import org.ossreviewtoolkit.model.RepositoryProvenance
4443
import org.ossreviewtoolkit.model.utils.DatabaseUtils.checkDatabaseEncoding
4544
import org.ossreviewtoolkit.model.utils.DatabaseUtils.tableExists
4645
import org.ossreviewtoolkit.model.utils.DatabaseUtils.transaction
47-
import org.ossreviewtoolkit.utils.ort.createOrtTempFile
4846

4947
/**
5048
* A [DataSource]-based implementation of [ProvenanceFileStorage] that stores files associated by [KnownProvenance] in a
@@ -81,27 +79,27 @@ class PostgresProvenanceFileStorage(
8179
}
8280
}
8381

84-
override fun hasFile(provenance: KnownProvenance): Boolean =
82+
override fun hasData(provenance: KnownProvenance): Boolean =
8583
database.transaction {
8684
table.slice(table.provenance.count()).select {
8785
table.provenance eq provenance.storageKey()
8886
}.first()[table.provenance.count()].toInt()
8987
} == 1
9088

91-
override fun putFile(provenance: KnownProvenance, file: File) {
89+
override fun putData(provenance: KnownProvenance, data: InputStream) {
9290
database.transaction {
9391
table.deleteWhere {
9492
table.provenance eq provenance.storageKey()
9593
}
9694

97-
table.insert {
98-
it[this.provenance] = provenance.storageKey()
99-
it[zipData] = file.readBytes()
95+
table.insert { statement ->
96+
statement[this.provenance] = provenance.storageKey()
97+
statement[zipData] = data.use { it.readBytes() }
10098
}
10199
}
102100
}
103101

104-
override fun getFile(provenance: KnownProvenance): File? {
102+
override fun getData(provenance: KnownProvenance): InputStream? {
105103
val bytes = database.transaction {
106104
table.select {
107105
table.provenance eq provenance.storageKey()
@@ -110,16 +108,7 @@ class PostgresProvenanceFileStorage(
110108
}.firstOrNull()
111109
} ?: return null
112110

113-
val file = createOrtTempFile(suffix = ".zip")
114-
115-
try {
116-
file.writeBytes(bytes)
117-
} catch (e: IOException) {
118-
file.delete()
119-
throw e
120-
}
121-
122-
return file
111+
return bytes.inputStream()
123112
}
124113
}
125114

model/src/main/kotlin/utils/ProvenanceFileStorage.kt

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,27 +18,28 @@
1818
*/
1919
package org.ossreviewtoolkit.model.utils
2020

21-
import java.io.File
21+
import java.io.InputStream
2222

2323
import org.ossreviewtoolkit.model.KnownProvenance
2424

2525
/**
26-
* A generic storage interface that associates a [KnownProvenance] with a file.
26+
* A generic storage interface that associates a [KnownProvenance] with a stream of data.
2727
*/
2828
interface ProvenanceFileStorage {
2929
/**
30-
* Return whether a file is associated by [provenance].
30+
* Return whether any data is associated by [provenance].
3131
*/
32-
fun hasFile(provenance: KnownProvenance): Boolean
32+
fun hasData(provenance: KnownProvenance): Boolean
3333

3434
/**
35-
* Associate [provenance] with the given [file]. Overwrites any existing association by [provenance].
35+
* Associate [provenance] with the given [data]. Replaces any existing association by [provenance]. The function
36+
* implementation is responsible for closing the stream.
3637
*/
37-
fun putFile(provenance: KnownProvenance, file: File)
38+
fun putData(provenance: KnownProvenance, data: InputStream)
3839

3940
/**
40-
* Return the file associated by [provenance], or null if there is no such file. Note that the returned file is a
41-
* temporary file that the caller is responsible for.
41+
* Return the data associated by [provenance], or null if there is no such data. Note that it is the responsibility
42+
* of the caller to close the stream.
4243
*/
43-
fun getFile(provenance: KnownProvenance): File?
44+
fun getData(provenance: KnownProvenance): InputStream?
4445
}

scanner/src/main/kotlin/utils/FileListResolver.kt

Lines changed: 6 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -19,21 +19,18 @@
1919

2020
package org.ossreviewtoolkit.scanner.utils
2121

22-
import com.fasterxml.jackson.module.kotlin.readValue
23-
2422
import java.io.File
2523

26-
import org.ossreviewtoolkit.model.FileFormat
2724
import org.ossreviewtoolkit.model.HashAlgorithm
2825
import org.ossreviewtoolkit.model.KnownProvenance
26+
import org.ossreviewtoolkit.model.fromYaml
27+
import org.ossreviewtoolkit.model.toYaml
2928
import org.ossreviewtoolkit.model.utils.ProvenanceFileStorage
30-
import org.ossreviewtoolkit.model.writeValue
3129
import org.ossreviewtoolkit.scanner.FileList
3230
import org.ossreviewtoolkit.scanner.FileList.FileEntry
3331
import org.ossreviewtoolkit.scanner.provenance.ProvenanceDownloader
3432
import org.ossreviewtoolkit.utils.common.FileMatcher
3533
import org.ossreviewtoolkit.utils.common.VCS_DIRECTORIES
36-
import org.ossreviewtoolkit.utils.ort.createOrtTempFile
3734

3835
internal class FileListResolver(
3936
private val storage: ProvenanceFileStorage,
@@ -47,23 +44,16 @@ internal class FileListResolver(
4744
}
4845
}
4946

50-
fun has(provenance: KnownProvenance): Boolean = storage.hasFile(provenance)
47+
fun has(provenance: KnownProvenance): Boolean = storage.hasData(provenance)
5148
}
5249

5350
private fun ProvenanceFileStorage.putFileList(provenance: KnownProvenance, fileList: FileList) {
54-
val tempFile = createOrtTempFile(prefix = "file-list", suffix = ".yml")
55-
56-
tempFile.writeValue(fileList)
57-
putFile(provenance, tempFile)
58-
59-
tempFile.delete()
51+
putData(provenance, fileList.toYaml().byteInputStream())
6052
}
6153

6254
private fun ProvenanceFileStorage.getFileList(provenance: KnownProvenance): FileList? {
63-
val file = getFile(provenance) ?: return null
64-
65-
// Cannot rely on the extension of the file to reflect its type, so use YAML explicitly.
66-
return FileFormat.YAML.mapper.readValue<FileList>(file).also { file.delete() }
55+
val data = getData(provenance) ?: return null
56+
return String(data.use { it.readBytes() }).fromYaml<FileList>()
6757
}
6858

6959
private val IGNORED_DIRECTORY_MATCHER by lazy {

0 commit comments

Comments
 (0)