diff --git a/android/app/src/main/java/com/audiobookshelf/app/managers/DownloadItemManager.kt b/android/app/src/main/java/com/audiobookshelf/app/managers/DownloadItemManager.kt index 57ae0fc8..e9dd6150 100644 --- a/android/app/src/main/java/com/audiobookshelf/app/managers/DownloadItemManager.kt +++ b/android/app/src/main/java/com/audiobookshelf/app/managers/DownloadItemManager.kt @@ -34,6 +34,7 @@ class DownloadItemManager( private val scope = CoroutineScope(SupervisorJob() + Dispatchers.IO) private val activeCalls = ConcurrentHashMap() private val safFolderLocks = ConcurrentHashMap() + private val scanLocks = ConcurrentHashMap() private val reservations = mutableMapOf() private val lastPersistTime = mutableMapOf() private val finalizingItems = mutableSetOf() @@ -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 } diff --git a/android/app/src/main/java/com/audiobookshelf/app/models/DownloadItemPart.kt b/android/app/src/main/java/com/audiobookshelf/app/models/DownloadItemPart.kt index 7dadbca8..863a0192 100644 --- a/android/app/src/main/java/com/audiobookshelf/app/models/DownloadItemPart.kt +++ b/android/app/src/main/java/com/audiobookshelf/app/models/DownloadItemPart.kt @@ -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 ) } } diff --git a/android/app/src/main/java/com/audiobookshelf/app/plugins/AbsDownloader.kt b/android/app/src/main/java/com/audiobookshelf/app/plugins/AbsDownloader.kt index b2d6ebdf..e8e7c4ab 100644 --- a/android/app/src/main/java/com/audiobookshelf/app/plugins/AbsDownloader.kt +++ b/android/app/src/main/java/com/audiobookshelf/app/plugins/AbsDownloader.kt @@ -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)) } }) diff --git a/android/app/src/main/java/com/audiobookshelf/app/services/DownloadServiceHost.kt b/android/app/src/main/java/com/audiobookshelf/app/services/DownloadServiceHost.kt index d385eb19..1743c2df 100644 --- a/android/app/src/main/java/com/audiobookshelf/app/services/DownloadServiceHost.kt +++ b/android/app/src/main/java/com/audiobookshelf/app/services/DownloadServiceHost.kt @@ -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" diff --git a/android/app/src/test/java/com/audiobookshelf/app/managers/DownloadResumePolicyTest.kt b/android/app/src/test/java/com/audiobookshelf/app/managers/DownloadResumePolicyTest.kt new file mode 100644 index 00000000..092f495f --- /dev/null +++ b/android/app/src/test/java/com/audiobookshelf/app/managers/DownloadResumePolicyTest.kt @@ -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)) + } +} diff --git a/android/app/src/test/java/com/audiobookshelf/app/managers/InternalDownloadManagerTest.kt b/android/app/src/test/java/com/audiobookshelf/app/managers/InternalDownloadManagerTest.kt new file mode 100644 index 00000000..a3fcf82f --- /dev/null +++ b/android/app/src/test/java/com/audiobookshelf/app/managers/InternalDownloadManagerTest.kt @@ -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) -> ByteArray + ) : Closeable { + private val socket = ServerSocket(0) + val requests = Collections.synchronizedList(mutableListOf>()) + 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() + 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 = 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 + } + } +} diff --git a/components/widgets/DownloadProgressIndicator.vue b/components/widgets/DownloadProgressIndicator.vue index be1d502c..bc723521 100644 --- a/components/widgets/DownloadProgressIndicator.vue +++ b/components/widgets/DownloadProgressIndicator.vue @@ -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() } } - \ No newline at end of file + diff --git a/pages/downloading.vue b/pages/downloading.vue index 72415947..cd206e3a 100644 --- a/pages/downloading.vue +++ b/pages/downloading.vue @@ -4,7 +4,7 @@
No download item parts