Fix concurrent podcast download cover issue

This commit is contained in:
Nicholas Wallace
2026-08-27 14:52:14 -07:00
parent 7989607313
commit c800ed727a
8 changed files with 306 additions and 35 deletions
@@ -34,6 +34,7 @@ class DownloadItemManager(
private val scope = CoroutineScope(SupervisorJob() + Dispatchers.IO)
private val activeCalls = ConcurrentHashMap<String, InternalDownloadManager.DownloadHandle>()
private val safFolderLocks = ConcurrentHashMap<String, Any>()
private val scanLocks = ConcurrentHashMap<String, Any>()
private val reservations = mutableMapOf<String, Long>()
private val lastPersistTime = mutableMapOf<String, Long>()
private val finalizingItems = mutableSetOf<String>()
@@ -51,7 +52,7 @@ class DownloadItemManager(
fun onDownloadItem(downloadItem: DownloadItem)
fun onDownloadItemPartUpdate(downloadItemPart: DownloadItemPart)
fun onDownloadItemComplete(jsobj: JSObject)
fun onQueueChanged(hasWork: Boolean)
fun onQueueChanged(hasWork: Boolean, hasItems: Boolean)
}
interface InternalProgressCallback {
@@ -77,6 +78,7 @@ class DownloadItemManager(
part.completed = false
part.completedDestinationUri = null
part.downloadId = null
part.reusedExistingFile = false
}
}
if (item.isDownloadFinished) {
@@ -153,9 +155,9 @@ class DownloadItemManager(
downloadItemQueue.forEach { item ->
item.downloadItemParts.forEach { part ->
File(part.destinationPath).delete()
if (part.moved && part.isInternalStorage) {
if (part.moved && !part.reusedExistingFile && part.isInternalStorage) {
File(part.finalDestinationPath).delete()
} else if (part.moved) {
} else if (part.moved && !part.reusedExistingFile) {
part.completedDestinationUri?.let { uri ->
try {
DocumentFile.fromSingleUri(context, Uri.parse(uri))?.delete()
@@ -176,7 +178,7 @@ class DownloadItemManager(
@Synchronized
fun hasWork(): Boolean =
downloadItemQueue.any { item ->
finalizingItems.isNotEmpty() || downloadItemQueue.any { item ->
item.downloadItemParts.any { part ->
(!part.moved && !part.failed) || part.isMoving
}
@@ -190,7 +192,8 @@ class DownloadItemManager(
item.downloadItemParts
.filter { part ->
part.completed && !part.moved && !part.failed && !part.isMoving &&
part !in currentDownloadItemParts && File(part.destinationPath).exists()
part !in currentDownloadItemParts && File(part.destinationPath).exists() &&
!hasActiveDestinationConflict(part)
}
.take(slots)
.forEach { part ->
@@ -205,9 +208,17 @@ class DownloadItemManager(
part.bytesDownloaded = existingFile.length()
part.progress = 100L
part.completedDestinationUri = existingFile.uri.toString()
part.reusedExistingFile = true
File(part.destinationPath).delete()
completePart(item, part)
clientEventEmitter.onDownloadItemPartUpdate(part)
return@forEach
}
if (completeFromExistingInternalCover(item, part)) return@forEach
if (hasActiveDestinationConflict(part)) {
leaveQueued(item, part)
} else if (part.fileSize <= 0L && currentDownloadItemParts.any { it.fileSize <= 0L }) {
leaveQueued(item, part)
} else if (tryReserve(part)) startDownload(item, part)
else {
part.waitingForSpace = true
@@ -327,6 +338,7 @@ class DownloadItemManager(
item.stagingCleanupAt = null
persist(item, force = true)
IncompleteDownloadCleanup.schedule(context, item)
clientEventEmitter.onDownloadItemPartUpdate(part)
notifyQueueChanged()
return
}
@@ -335,6 +347,7 @@ class DownloadItemManager(
part.downloadId = null
part.isMoving = false
persist(item, force = true)
clientEventEmitter.onDownloadItemPartUpdate(part)
}
private fun finalizeInternalFile(item: DownloadItem, part: DownloadItemPart) {
@@ -433,31 +446,33 @@ class DownloadItemManager(
if (!item.isDownloadFinished || !finalizingItems.add(item.id)) return
IncompleteDownloadCleanup.cancel(context, item.id)
scope.launch {
folderScanner.scanDownloadItem(item) { scanResult ->
val event =
JSObject().apply {
put("libraryItemId", item.id)
put("localFolderId", item.localFolder.id)
scanResult?.localLibraryItem?.let {
put("localLibraryItem", JSObject(jacksonMapper.writeValueAsString(it)))
val scanLock = scanLocks.computeIfAbsent(scanDestinationKey(item)) { Any() }
synchronized(scanLock) {
folderScanner.scanDownloadItem(item) { scanResult ->
val event =
JSObject().apply {
put("libraryItemId", item.id)
put("localFolderId", item.localFolder.id)
scanResult?.localLibraryItem?.let {
put("localLibraryItem", JSObject(jacksonMapper.writeValueAsString(it)))
}
scanResult?.localMediaProgress?.let {
put("localMediaProgress", JSObject(jacksonMapper.writeValueAsString(it)))
}
}
scanResult?.localMediaProgress?.let {
put("localMediaProgress", JSObject(jacksonMapper.writeValueAsString(it)))
}
}
clientEventEmitter.onDownloadItemComplete(event)
synchronized(this@DownloadItemManager) {
finalizingItems.remove(item.id)
downloadItemQueue.remove(item)
DeviceManager.dbManager.removeDownloadItem(item.id)
notifyQueueChanged()
clientEventEmitter.onDownloadItemComplete(event)
synchronized(this@DownloadItemManager) {
finalizingItems.remove(item.id)
downloadItemQueue.remove(item)
DeviceManager.dbManager.removeDownloadItem(item.id)
notifyQueueChanged()
}
}
}
}
}
private fun tryReserve(part: DownloadItemPart): Boolean {
if (part.fileSize <= 0L && currentDownloadItemParts.any { it.fileSize <= 0L }) return false
val staging = File(part.destinationPath)
staging.parentFile?.mkdirs()
val expectedSize = if (part.fileSize > 0L) part.fileSize else UNKNOWN_PART_RESERVATION_BYTES
@@ -504,7 +519,7 @@ class DownloadItemManager(
}
private fun notifyQueueChanged() {
clientEventEmitter.onQueueChanged(hasWork())
clientEventEmitter.onQueueChanged(hasWork(), downloadItemQueue.isNotEmpty())
}
fun destroy() {
@@ -537,6 +552,40 @@ class DownloadItemManager(
return file
}
private fun completeFromExistingInternalCover(
item: DownloadItem,
part: DownloadItemPart
): Boolean {
if (!part.isInternalStorage || !part.serverPath.endsWith("/cover")) return false
val file = File(part.finalDestinationPath)
if (!file.isFile || file.length() <= 0L) return false
if (part.fileSize > 0L && file.length() != part.fileSize) return false
part.bytesDownloaded = file.length()
part.progress = 100L
part.reusedExistingFile = true
File(part.destinationPath).delete()
completePart(item, part)
clientEventEmitter.onDownloadItemPartUpdate(part)
return true
}
private fun hasActiveDestinationConflict(part: DownloadItemPart): Boolean =
currentDownloadItemParts.any { activePart ->
activePart !== part && activePart.localFolderId == part.localFolderId &&
activePart.finalDestinationPath == part.finalDestinationPath
}
private fun leaveQueued(item: DownloadItem, part: DownloadItemPart) {
if (!part.waitingForSpace) return
part.waitingForSpace = false
part.downloadId = null
persist(item)
clientEventEmitter.onDownloadItemPartUpdate(part)
}
private fun scanDestinationKey(item: DownloadItem): String =
"${item.localFolder.id}:${item.itemFolderPath}"
private fun finalizedFileExists(part: DownloadItemPart): Boolean {
if (part.isInternalStorage) {
val file = File(part.finalDestinationPath)
@@ -571,6 +620,7 @@ class DownloadItemManager(
part.downloadId = null
part.retryCount = 0
part.waitingForSpace = false
part.reusedExistingFile = false
return true
}
@@ -38,7 +38,8 @@ data class DownloadItemPart(
var progress: Long,
var bytesDownloaded: Long,
@JsonIgnore var retryCount: Int = 0,
@JsonIgnore var waitingForSpace: Boolean = false
@JsonIgnore var waitingForSpace: Boolean = false,
@JsonIgnore var reusedExistingFile: Boolean = false
) {
companion object {
fun make(downloadItemId:String, filename:String, fileSize: Long, destinationFile: File, finalDestinationFile: File, subfolder:String, serverPath:String, localFolder: LocalFolder, ebookFile: EBookFile?, audioTrack: AudioTrack?, episode: PodcastEpisode?) :DownloadItemPart {
@@ -74,7 +75,8 @@ data class DownloadItemPart(
downloadId = null,
lastUpdateTime = null,
progress = 0,
bytesDownloaded = 0
bytesDownloaded = 0,
reusedExistingFile = false
)
}
}
@@ -38,8 +38,9 @@ class AbsDownloader : Plugin() {
override fun onDownloadItemComplete(jsobj:JSObject) {
notifyListeners("onItemDownloadComplete", jsobj)
}
override fun onQueueChanged(hasWork: Boolean) {
notifyListeners("onQueueChanged", JSObject().put("hasWork", hasWork))
override fun onQueueChanged(hasWork: Boolean, hasItems: Boolean) {
notifyListeners(
"onQueueChanged", JSObject().put("hasWork", hasWork).put("hasItems", hasItems))
}
})
@@ -185,8 +185,8 @@ object DownloadServiceHost {
override fun onDownloadItemComplete(jsobj: JSObject) {
if (bridgeReady) bridgeEmitter.onDownloadItemComplete(jsobj) else deferredCompletions.add(jsobj)
}
override fun onQueueChanged(hasWork: Boolean) {
bridgeEmitter.onQueueChanged(hasWork)
override fun onQueueChanged(hasWork: Boolean, hasItems: Boolean) {
bridgeEmitter.onQueueChanged(hasWork, hasItems)
service?.onQueueChanged(hasWork)
}
}
@@ -195,7 +195,7 @@ object DownloadServiceHost {
override fun onDownloadItem(downloadItem: DownloadItem) = Unit
override fun onDownloadItemPartUpdate(downloadItemPart: com.audiobookshelf.app.models.DownloadItemPart) = Unit
override fun onDownloadItemComplete(jsobj: JSObject) = Unit
override fun onQueueChanged(hasWork: Boolean) = Unit
override fun onQueueChanged(hasWork: Boolean, hasItems: Boolean) = Unit
}
private const val NOTIFICATION_PREFERENCES = "download_notifications"
@@ -0,0 +1,41 @@
package com.audiobookshelf.app.managers
import org.junit.Assert.assertEquals
import org.junit.Assert.assertNull
import org.junit.Test
class DownloadResumePolicyTest {
@Test
fun completeKnownFileDoesNotIssueRequest() {
assertEquals(
DownloadResumePolicy.InitialAction.COMPLETE,
DownloadResumePolicy.initialAction(100L, 100L))
}
@Test
fun partialAndUnknownFilesUseRange() {
assertEquals(
DownloadResumePolicy.InitialAction.RANGE_DOWNLOAD,
DownloadResumePolicy.initialAction(25L, 100L))
assertEquals(
DownloadResumePolicy.InitialAction.RANGE_DOWNLOAD,
DownloadResumePolicy.initialAction(25L, 0L))
}
@Test
fun oversizedFileRestartsAndEmptyFileDownloadsFully() {
assertEquals(
DownloadResumePolicy.InitialAction.RESTART,
DownloadResumePolicy.initialAction(101L, 100L))
assertEquals(
DownloadResumePolicy.InitialAction.FULL_DOWNLOAD,
DownloadResumePolicy.initialAction(0L, 100L))
}
@Test
fun parsesUnsatisfiedContentRange() {
assertEquals(787913771L, DownloadResumePolicy.unsatisfiedRangeSize("bytes */787913771"))
assertNull(DownloadResumePolicy.unsatisfiedRangeSize("bytes 0-99/100"))
assertNull(DownloadResumePolicy.unsatisfiedRangeSize(null))
}
}
@@ -0,0 +1,176 @@
package com.audiobookshelf.app.managers
import java.io.Closeable
import java.net.ServerSocket
import java.nio.file.Files
import java.util.Collections
import java.util.concurrent.CountDownLatch
import java.util.concurrent.TimeUnit
import java.util.concurrent.atomic.AtomicBoolean
import org.junit.After
import org.junit.Assert.assertEquals
import org.junit.Assert.assertFalse
import org.junit.Assert.assertNull
import org.junit.Assert.assertTrue
import org.junit.Test
class InternalDownloadManagerTest {
private var server: TestHttpServer? = null
@After
fun tearDown() {
server?.close()
}
@Test
fun exactKnownStagingFileCompletesWithoutHttpRequest() {
val destination = Files.createTempFile("abs-complete", ".part").toFile()
destination.writeBytes(byteArrayOf(1, 2, 3, 4))
val callback = RecordingCallback()
InternalDownloadManager(destination, 4L, callback) { true }
.download("http://127.0.0.1:1/download", "token")
assertTrue(callback.completed.await(1, TimeUnit.SECONDS))
assertFalse(callback.failed.get())
assertEquals(4L, destination.length())
destination.delete()
}
@Test
fun unknownSizeFullFileIsAcceptedFrom416ContentRange() {
server = TestHttpServer { _, _ ->
response(416, headers = listOf("Content-Range: bytes */4"))
}
val destination = Files.createTempFile("abs-unknown", ".part").toFile()
destination.writeBytes(byteArrayOf(1, 2, 3, 4))
val callback = RecordingCallback()
InternalDownloadManager(destination, 0L, callback) { true }
.download(server!!.url, "token")
assertTrue(callback.completed.await(3, TimeUnit.SECONDS))
assertFalse(callback.failed.get())
assertEquals("bytes=4-", server!!.requests.single()["range"])
destination.delete()
}
@Test
fun partialFileResumesWithRangeDuringLiveRetry() {
server = TestHttpServer { _, _ ->
response(206, byteArrayOf(3, 4), listOf("Content-Range: bytes 2-3/4"))
}
val destination = Files.createTempFile("abs-partial", ".part").toFile()
destination.writeBytes(byteArrayOf(1, 2))
val callback = RecordingCallback()
InternalDownloadManager(destination, 4L, callback) { true }
.download(server!!.url, "token")
assertTrue(callback.completed.await(3, TimeUnit.SECONDS))
assertFalse(callback.failed.get())
assertEquals("bytes=2-", server!!.requests.single()["range"])
assertTrue(destination.readBytes().contentEquals(byteArrayOf(1, 2, 3, 4)))
destination.delete()
}
@Test
fun stale416RestartsOnceFromByteZero() {
server = TestHttpServer { index, _ ->
if (index == 0) response(416, headers = listOf("Content-Range: bytes */2"))
else response(200, byteArrayOf(9, 8))
}
val destination = Files.createTempFile("abs-stale", ".part").toFile()
destination.writeBytes(byteArrayOf(1, 2, 3, 4))
val callback = RecordingCallback()
InternalDownloadManager(destination, 0L, callback) { true }
.download(server!!.url, "token")
assertTrue(callback.completed.await(3, TimeUnit.SECONDS))
assertFalse(callback.failed.get())
assertEquals(2, server!!.requests.size)
assertEquals("bytes=4-", server!!.requests[0]["range"])
assertNull(server!!.requests[1]["range"])
assertTrue(destination.readBytes().contentEquals(byteArrayOf(9, 8)))
destination.delete()
}
private class RecordingCallback : DownloadItemManager.InternalProgressCallback {
val completed = CountDownLatch(1)
val failed = AtomicBoolean(true)
override fun onProgress(totalBytesWritten: Long, progress: Long) = Unit
override fun onComplete(failed: Boolean) {
this.failed.set(failed)
completed.countDown()
}
}
private class TestHttpServer(
private val responder: (Int, Map<String, String>) -> ByteArray
) : Closeable {
private val socket = ServerSocket(0)
val requests = Collections.synchronizedList(mutableListOf<Map<String, String>>())
val url = "http://127.0.0.1:${socket.localPort}/download"
private val thread = Thread {
while (!socket.isClosed) {
try {
socket.accept().use { connection ->
val reader = connection.getInputStream().bufferedReader()
reader.readLine()
val headers = mutableMapOf<String, String>()
while (true) {
val line = reader.readLine() ?: break
if (line.isEmpty()) break
val separator = line.indexOf(':')
if (separator > 0) {
headers[line.substring(0, separator).lowercase()] =
line.substring(separator + 1).trim()
}
}
val index = requests.size
requests.add(headers)
connection.getOutputStream().use { output ->
output.write(responder(index, headers))
output.flush()
}
}
} catch (_: Exception) {
if (!socket.isClosed) throw IllegalStateException("Test HTTP server failed")
}
}
}.apply {
isDaemon = true
start()
}
override fun close() {
socket.close()
thread.join(1_000L)
}
}
companion object {
private fun response(
status: Int,
body: ByteArray = byteArrayOf(),
headers: List<String> = emptyList()
): ByteArray {
val reason =
when (status) {
200 -> "OK"
206 -> "Partial Content"
else -> "Range Not Satisfiable"
}
val head = buildString {
append("HTTP/1.1 $status $reason\r\n")
headers.forEach { append("$it\r\n") }
append("Content-Length: ${body.size}\r\n")
append("Connection: close\r\n\r\n")
}.toByteArray()
return head + body
}
}
}
@@ -79,7 +79,9 @@ export default {
this.$store.commit('globals/updateDownloadItemPart', itemPart)
},
onQueueChanged(data) {
if (!data.hasWork) this.$store.commit('globals/clearItemDownloads')
if (data.hasItems === false || (data.hasItems == null && !data.hasWork)) {
this.$store.commit('globals/clearItemDownloads')
}
}
},
async mounted() {
@@ -95,4 +97,4 @@ export default {
this.queueChangedListener?.remove()
}
}
</script>
</script>
+1 -2
View File
@@ -4,7 +4,7 @@
<div v-if="!downloadItemParts.length" class="py-6 text-center text-lg">No download item parts</div>
<template v-for="(itemPart, num) in downloadItemParts">
<div :key="itemPart.id" class="w-full">
<div :key="`${itemPart.downloadItemId}-${itemPart.id}`" class="w-full">
<div class="flex">
<div class="w-14">
<span v-if="itemPart.completed" class="material-symbols text-success">check_circle</span>
@@ -40,4 +40,3 @@ export default {
beforeDestroy() {}
}
</script>