Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid incomplete downloads in cache #3656

Merged
merged 2 commits into from
Jul 19, 2021
Merged

Avoid incomplete downloads in cache #3656

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4ef1f5c
Avoid incomplete downloads in cache
SpiritCroc Jul 10, 2021
512e1b3
Add changelog.d/3656.bugfix
SpiritCroc Jul 10, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/3656.bugfix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Avoid incomplete downloads in cache
16 changes: 14 additions & 2 deletions ...x-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/DefaultFileService.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,8 +131,14 @@ internal class DefaultFileService @Inject constructor(
Timber.v("Response size ${response.body?.contentLength()} - Stream available: ${!source.exhausted()}")

// Write the file to cache (encrypted version if the file is encrypted)
writeToFile(source.inputStream(), cachedFiles.file)
// Write to a tmp file first, so if we abort before done, we don't have a broken cached file
val tmpFile = File(cachedFiles.file.parentFile, "${cachedFiles.file.name}.tmp")
if (tmpFile.exists()) {
Timber.v("## FileService: discard aborted tmp file ${tmpFile.path}")
}
writeToFile(source.inputStream(), tmpFile)
response.close()
tmpFile.renameTo(cachedFiles.file)
} else {
Timber.v("## FileService: cache hit for $url")
}
Expand All @@ -145,15 +151,21 @@ internal class DefaultFileService @Inject constructor(
Timber.v("## FileService: decrypt file")
// Ensure the parent folder exists
cachedFiles.decryptedFile.parentFile?.mkdirs()
// Write to a tmp file first, so if we abort before done, we don't have a broken cached file
val tmpFile = File(cachedFiles.decryptedFile.parentFile, "${cachedFiles.decryptedFile.name}.tmp")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Create a helper to avoid repeated the same code twice?
For instance

internal class AtomicFileCreator(private val file: File) {
    val tmpFile = File(file.parentFile, "${file.name}.tmp")

    init {
        if (tmpFile.exists()) {
            Timber.v("## AtomicFileCreator: discard aborted tmp file ${tmpFile.path}")
        }
    }

    fun commit() {
        tmpFile.renameTo(file)
    }
}

if (tmpFile.exists()) {
Timber.v("## FileService: discard aborted tmp file ${tmpFile.path}")
}
val decryptSuccess = cachedFiles.file.inputStream().use { inputStream ->
cachedFiles.decryptedFile.outputStream().buffered().use { outputStream ->
tmpFile.outputStream().buffered().use { outputStream ->
MXEncryptedAttachments.decryptAttachment(
inputStream,
elementToDecrypt,
outputStream
)
}
}
tmpFile.renameTo(cachedFiles.decryptedFile)
if (!decryptSuccess) {
throw IllegalStateException("Decryption error")
}
Expand Down