From 61c314a531a72b9d35e5087160f00b7e97bc6811 Mon Sep 17 00:00:00 2001 From: Nicholas Wallace Date: Thu, 27 Aug 2026 13:08:46 -0700 Subject: [PATCH 01/14] Initial download service startup fixes and range request fix --- android/app/build.gradle | 4 + .../app/managers/DownloadItemManager.kt | 169 +++++++++++++----- .../app/managers/DownloadResumePolicy.kt | 23 +++ .../app/managers/IncompleteDownloadCleanup.kt | 27 ++- .../app/managers/InternalDownloadManager.kt | 74 +++++++- .../audiobookshelf/app/models/DownloadItem.kt | 7 +- .../app/plugins/AbsDownloader.kt | 94 +++++----- .../app/services/DownloadService.kt | 2 +- .../app/services/DownloadServiceHost.kt | 98 ++++++++-- 9 files changed, 372 insertions(+), 126 deletions(-) create mode 100644 android/app/src/main/java/com/audiobookshelf/app/managers/DownloadResumePolicy.kt diff --git a/android/app/build.gradle b/android/app/build.gradle index b3725b88..b0349cd3 100644 --- a/android/app/build.gradle +++ b/android/app/build.gradle @@ -60,6 +60,9 @@ android { proguardFiles getDefaultProguardFile('proguard-android.txt'), 'proguard-rules.pro' } } + testOptions { + unitTests.returnDefaultValues = true + } } repositories { @@ -81,6 +84,7 @@ configurations.configureEach { } dependencies { + testImplementation "junit:junit:$junit_version" implementation "androidx.core:core-splashscreen:$coreSplashScreenVersion" implementation fileTree(include: ['*.jar'], dir: 'libs') implementation "androidx.appcompat:appcompat:$androidxAppCompatVersion" 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 b83ade16..9bf6eee9 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 @@ -5,6 +5,7 @@ import android.net.Uri import android.os.StatFs import android.util.Log import androidx.documentfile.provider.DocumentFile +import com.anggrayudi.storage.file.fullName import com.audiobookshelf.app.device.DeviceManager import com.audiobookshelf.app.device.FolderScanner import com.audiobookshelf.app.models.DownloadItem @@ -22,7 +23,6 @@ import kotlinx.coroutines.SupervisorJob import kotlinx.coroutines.cancel import kotlinx.coroutines.delay import kotlinx.coroutines.launch -import okhttp3.Call /** Manages the process-owned queue for app-managed downloads. */ class DownloadItemManager( @@ -32,10 +32,11 @@ class DownloadItemManager( ) { private val tag = "DownloadItemManager" private val scope = CoroutineScope(SupervisorJob() + Dispatchers.IO) - private val activeCalls = ConcurrentHashMap() + private val activeCalls = ConcurrentHashMap() private val safFolderLocks = ConcurrentHashMap() private val reservations = mutableMapOf() private val lastPersistTime = mutableMapOf() + private val finalizingItems = mutableSetOf() private var watcherRunning = false private val jacksonMapper = jacksonObjectMapper() @@ -58,10 +59,6 @@ class DownloadItemManager( fun onComplete(failed: Boolean) } - init { - IncompleteDownloadCleanup.cleanupExpired(context) - } - @Synchronized fun setEventEmitter(eventEmitter: DownloadEventEmitter) { clientEventEmitter = eventEmitter @@ -73,6 +70,15 @@ class DownloadItemManager( fun restoreQueue() { if (downloadItemQueue.isNotEmpty()) return DeviceManager.dbManager.getDownloadItems().forEach { item -> + item.downloadItemParts.filter { it.moved }.forEach { part -> + if (!finalizedFileExists(part)) { + Log.w(tag, "Finalized file is missing; resetting ${part.filename}") + part.moved = false + part.completed = false + part.completedDestinationUri = null + part.downloadId = null + } + } if (item.isDownloadFinished) { downloadItemQueue.add(item) checkDownloadItemFinished(item) @@ -80,19 +86,26 @@ class DownloadItemManager( } item.downloadItemParts.forEach { part -> if (part.moved) return@forEach - if (item.terminalFailureAt != null && part.failed) return@forEach + if (item.terminalFailureAt != null) { + part.downloadId = null + part.isMoving = false + part.failed = true + part.waitingForSpace = false + part.bytesDownloaded = File(part.destinationPath).takeIf(File::exists)?.length() ?: 0L + return@forEach + } part.downloadId = null part.isMoving = false part.failed = false - part.completed = false part.waitingForSpace = false - part.bytesDownloaded = File(part.destinationPath).takeIf(File::exists)?.length() ?: 0L + val stagingLength = File(part.destinationPath).takeIf(File::exists)?.length() ?: 0L + part.bytesDownloaded = stagingLength + if (part.completed && stagingLength <= 0L) part.completed = false } downloadItemQueue.add(item) if (item.terminalFailureAt != null) IncompleteDownloadCleanup.schedule(context, item) clientEventEmitter.onDownloadItem(item) } - checkUpdateDownloadQueue() notifyQueueChanged() } @@ -100,39 +113,66 @@ class DownloadItemManager( fun addDownloadItem(downloadItem: DownloadItem) { val existingItem = downloadItemQueue.find { it.id == downloadItem.id } if (existingItem != null) { - if (existingItem.terminalFailureAt != null) { - retryDownloadItem(existingItem) - checkUpdateDownloadQueue() - notifyQueueChanged() - } return } persist(downloadItem, force = true) downloadItemQueue.add(downloadItem) clientEventEmitter.onDownloadItem(downloadItem) + notifyQueueChanged() + } + + @Synchronized + fun retryDownloadItem(downloadItemId: String): Boolean { + val item = downloadItemQueue.find { it.id == downloadItemId } ?: return false + if (item.downloadItemParts.any { it in currentDownloadItemParts }) return false + if (item.isDownloadFinished) return false + synchronized(IncompleteDownloadCleanup) { + item.terminalFailureAt = null + item.stagingCleanupAt = null + IncompleteDownloadCleanup.cancel(context, item.id) + item.downloadItemParts.filter { !it.moved }.forEach { part -> + part.failed = false + part.isMoving = false + part.downloadId = null + part.retryCount = 0 + part.waitingForSpace = false + val stagingLength = File(part.destinationPath).takeIf(File::exists)?.length() ?: 0L + part.bytesDownloaded = stagingLength + part.completed = part.completed && stagingLength > 0L + } + persist(item, force = true) + } + clientEventEmitter.onDownloadItem(item) + notifyQueueChanged() + return true + } + + @Synchronized + fun resumeWork() { checkUpdateDownloadQueue() notifyQueueChanged() } - private fun retryDownloadItem(item: DownloadItem) { - item.terminalFailureAt = null - IncompleteDownloadCleanup.cancel(context, item.id) - item.downloadItemParts.filter { it.failed }.forEach { part -> - part.failed = false - part.completed = false - part.isMoving = false - part.downloadId = null - part.retryCount = 0 - } - persist(item, force = true) - } - @Synchronized fun cancelAll() { - activeCalls.values.forEach(Call::cancel) + activeCalls.values.forEach(InternalDownloadManager.DownloadHandle::cancel) activeCalls.clear() downloadItemQueue.forEach { item -> - item.downloadItemParts.forEach { part -> File(part.destinationPath).delete() } + item.downloadItemParts.forEach { part -> + File(part.destinationPath).delete() + if (part.moved && part.isInternalStorage) { + File(part.finalDestinationPath).delete() + } else if (part.moved) { + part.completedDestinationUri?.let { uri -> + try { + DocumentFile.fromSingleUri(context, Uri.parse(uri))?.delete() + } catch (e: Exception) { + Log.w(tag, "Could not delete cancelled SAF file ${part.filename}", e) + } + } + } + } + IncompleteDownloadCleanup.cancel(context, item.id) DeviceManager.dbManager.removeDownloadItem(item.id) } currentDownloadItemParts.clear() @@ -145,14 +185,26 @@ class DownloadItemManager( fun hasWork(): Boolean = downloadItemQueue.any { item -> item.downloadItemParts.any { part -> - (!part.completed && !part.failed) || part.isMoving + (!part.moved && !part.failed) || part.isMoving } } @Synchronized private fun checkUpdateDownloadQueue() { downloadItemQueue.toList().forEach { item -> - val slots = MAX_SIMULTANEOUS_DOWNLOADS - currentDownloadItemParts.size + var slots = MAX_SIMULTANEOUS_DOWNLOADS - currentDownloadItemParts.size + if (slots <= 0) return@forEach + item.downloadItemParts + .filter { part -> + part.completed && !part.moved && !part.failed && !part.isMoving && + part !in currentDownloadItemParts && File(part.destinationPath).exists() + } + .take(slots) + .forEach { part -> + currentDownloadItemParts.add(part) + part.downloadId = APP_MANAGED_DOWNLOAD_ID + } + slots = MAX_SIMULTANEOUS_DOWNLOADS - currentDownloadItemParts.size if (slots <= 0) return@forEach item.getNextDownloadItemParts(slots).forEach { part -> val existingFile = findSharedStorageFile(part) @@ -189,7 +241,7 @@ class DownloadItemManager( else DeviceManager.getServerConnectionConfig(item.serverConnectionConfigId)?.token ?: DeviceManager.token - activeCalls[part.id] = + val handle = InternalDownloadManager( stagingFile, part.fileSize, @@ -216,8 +268,10 @@ class DownloadItemManager( } }, { hasAvailableSpace(part) } - ) - .download(serverUrl(item, part), token) + ).download(serverUrl(item, part), token) + if (part in currentDownloadItemParts && !part.completed && !part.failed) { + activeCalls[part.id] = handle + } } @Synchronized @@ -277,6 +331,7 @@ class DownloadItemManager( part.completed = false part.downloadId = null item.terminalFailureAt = item.terminalFailureAt ?: System.currentTimeMillis() + item.stagingCleanupAt = null persist(item, force = true) IncompleteDownloadCleanup.schedule(context, item) notifyQueueChanged() @@ -341,13 +396,13 @@ class DownloadItemManager( } if (temporary.length() != staging.length()) throw IllegalStateException("SAF copy size mismatch") - val existing = folder.findFile(part.filename) + val existing = findDocumentByFilename(folder, part) if (existing != null && !existing.delete()) throw IllegalStateException("Could not replace existing file") if (!temporary.renameTo(part.filename)) throw IllegalStateException("Could not finalize SAF temporary file") val destination = - folder.findFile(part.filename) + findDocumentByFilename(folder, part) ?: throw IllegalStateException("Could not reopen finalized SAF file") if (destination.length() != staging.length()) throw IllegalStateException("SAF final size mismatch") @@ -380,8 +435,10 @@ class DownloadItemManager( checkDownloadItemFinished(item) } + @Synchronized private fun checkDownloadItemFinished(item: DownloadItem) { - if (!item.isDownloadFinished) return + if (!item.isDownloadFinished || !finalizingItems.add(item.id)) return + IncompleteDownloadCleanup.cancel(context, item.id) scope.launch { folderScanner.scanDownloadItem(item) { scanResult -> val event = @@ -397,6 +454,7 @@ class DownloadItemManager( } clientEventEmitter.onDownloadItemComplete(event) synchronized(this@DownloadItemManager) { + finalizingItems.remove(item.id) downloadItemQueue.remove(item) DeviceManager.dbManager.removeDownloadItem(item.id) notifyQueueChanged() @@ -457,7 +515,7 @@ class DownloadItemManager( } fun destroy() { - activeCalls.values.forEach(Call::cancel) + activeCalls.values.forEach(InternalDownloadManager.DownloadHandle::cancel) activeCalls.clear() scope.cancel() } @@ -479,13 +537,44 @@ class DownloadItemManager( if (segment == "." || segment == "..") return null folder = folder.findFile(segment) ?: return null } - val file = folder.findFile(part.filename) ?: return null + val file = findDocumentByFilename(folder, part) ?: return null if (!file.isFile) return null if (part.fileSize > 0L && file.length() != part.fileSize) return null if (part.fileSize <= 0L && file.length() <= 0L) return null return file } + private fun finalizedFileExists(part: DownloadItemPart): Boolean { + if (part.isInternalStorage) { + val file = File(part.finalDestinationPath) + return file.isFile && + if (part.fileSize > 0L) file.length() == part.fileSize else file.length() > 0L + } + part.completedDestinationUri?.let { uri -> + try { + val file = DocumentFile.fromSingleUri(context, Uri.parse(uri)) + if (file?.isFile == true && + (part.fileSize <= 0L || file.length() == part.fileSize)) return true + } catch (e: Exception) { + Log.w(tag, "Could not validate SAF file ${part.filename}", e) + } + } + return findSharedStorageFile(part) != null + } + + private fun findDocumentByFilename(folder: DocumentFile, part: DownloadItemPart): DocumentFile? { + folder.findFile(part.filename)?.let { return it } + val expectedBaseName = part.filename.substringBeforeLast('.') + return folder.listFiles().firstOrNull { document -> + document.name == part.filename || + document.fullName == part.filename || + (part.audioTrack != null && document.isFile && + (document.name ?: "").substringBeforeLast('.') == expectedBaseName) || + (part.audioTrack != null && document.isFile && + document.fullName.substringBeforeLast('.') == expectedBaseName) + } + } + private fun mimeTypeFor(part: DownloadItemPart): String = part.audioTrack?.mimeType ?: when (part.ebookFile?.ebookFormat?.lowercase()) { diff --git a/android/app/src/main/java/com/audiobookshelf/app/managers/DownloadResumePolicy.kt b/android/app/src/main/java/com/audiobookshelf/app/managers/DownloadResumePolicy.kt new file mode 100644 index 00000000..337cf351 --- /dev/null +++ b/android/app/src/main/java/com/audiobookshelf/app/managers/DownloadResumePolicy.kt @@ -0,0 +1,23 @@ +package com.audiobookshelf.app.managers + +internal object DownloadResumePolicy { + enum class InitialAction { COMPLETE, RESTART, FULL_DOWNLOAD, RANGE_DOWNLOAD } + + fun initialAction(existingBytes: Long, expectedSize: Long): InitialAction = + when { + expectedSize > 0L && existingBytes == expectedSize -> InitialAction.COMPLETE + expectedSize > 0L && existingBytes > expectedSize -> InitialAction.RESTART + existingBytes > 0L -> InitialAction.RANGE_DOWNLOAD + else -> InitialAction.FULL_DOWNLOAD + } + + fun unsatisfiedRangeSize(contentRange: String?): Long? { + if (contentRange == null) return null + return UNSATISFIED_CONTENT_RANGE.matchEntire(contentRange) + ?.groupValues + ?.get(1) + ?.toLongOrNull() + } + + private val UNSATISFIED_CONTENT_RANGE = Regex("bytes \\*/(\\d+)") +} diff --git a/android/app/src/main/java/com/audiobookshelf/app/managers/IncompleteDownloadCleanup.kt b/android/app/src/main/java/com/audiobookshelf/app/managers/IncompleteDownloadCleanup.kt index 4fd89085..f51716ee 100644 --- a/android/app/src/main/java/com/audiobookshelf/app/managers/IncompleteDownloadCleanup.kt +++ b/android/app/src/main/java/com/audiobookshelf/app/managers/IncompleteDownloadCleanup.kt @@ -1,9 +1,7 @@ package com.audiobookshelf.app.managers import android.content.Context -import android.net.Uri import android.util.Log -import androidx.documentfile.provider.DocumentFile import androidx.work.ExistingWorkPolicy import androidx.work.OneTimeWorkRequestBuilder import androidx.work.WorkManager @@ -14,7 +12,7 @@ import com.audiobookshelf.app.models.DownloadItem import java.io.File import java.util.concurrent.TimeUnit -/** Removes terminally failed downloads after their retention window elapses. */ +/** Removes only staging data for terminally failed downloads after their retention window. */ object IncompleteDownloadCleanup { private const val tag = "IncompleteDownloadCleanup" private const val RETENTION_MS = 24L * 60L * 60L * 1000L @@ -22,6 +20,7 @@ object IncompleteDownloadCleanup { fun schedule(context: Context, item: DownloadItem) { val failedAt = item.terminalFailureAt ?: return + if (item.stagingCleanupAt != null) return val delay = (failedAt + RETENTION_MS - System.currentTimeMillis()).coerceAtLeast(0L) val request = OneTimeWorkRequestBuilder() .setInitialDelay(delay, TimeUnit.MILLISECONDS) @@ -34,7 +33,8 @@ object IncompleteDownloadCleanup { WorkManager.getInstance(context).cancelUniqueWork(WORK_PREFIX + itemId) } - /** Removes failures retained longer than 24 hours when scheduled work did not run. */ + /** Cleans staging data retained longer than 24 hours when scheduled work did not run. */ + @Synchronized fun cleanupExpired(context: Context) { val now = System.currentTimeMillis() DeviceManager.dbManager.getDownloadItems() @@ -46,6 +46,7 @@ object IncompleteDownloadCleanup { private fun isEligible(item: DownloadItem, now: Long): Boolean { val failedAt = item.terminalFailureAt ?: return false + if (item.stagingCleanupAt != null) return false if (now - failedAt < RETENTION_MS) return false return item.downloadItemParts.all { part -> part.moved || (part.failed && !part.isMoving) @@ -55,21 +56,15 @@ object IncompleteDownloadCleanup { private fun deleteItem(context: Context, item: DownloadItem) { item.downloadItemParts.forEach { part -> deleteAppOwnedFile(context, File(part.destinationPath)) - if (part.isInternalStorage && part.moved) { - deleteAppOwnedFile(context, File(part.finalDestinationPath)) - } else if (!part.isInternalStorage && part.moved) { - part.completedDestinationUri?.let { uriString -> - try { - DocumentFile.fromSingleUri(context, Uri.parse(uriString))?.delete() - } catch (e: Exception) { - Log.w(tag, "Could not delete expired SAF document for ${part.filename}", e) - } - } + if (!part.moved) { + part.bytesDownloaded = 0L + part.completed = false } } - DeviceManager.dbManager.removeDownloadItem(item.id) + item.stagingCleanupAt = System.currentTimeMillis() + DeviceManager.dbManager.saveDownloadItem(item) cancel(context, item.id) - Log.i(tag, "Deleted terminally failed download item ${item.id}") + Log.i(tag, "Deleted staging files for terminally failed download item ${item.id}") } private fun deleteAppOwnedFile(context: Context, file: File) { diff --git a/android/app/src/main/java/com/audiobookshelf/app/managers/InternalDownloadManager.kt b/android/app/src/main/java/com/audiobookshelf/app/managers/InternalDownloadManager.kt index 59dc6026..1f251064 100644 --- a/android/app/src/main/java/com/audiobookshelf/app/managers/InternalDownloadManager.kt +++ b/android/app/src/main/java/com/audiobookshelf/app/managers/InternalDownloadManager.kt @@ -4,6 +4,8 @@ import android.util.Log import java.io.File import java.io.FileOutputStream import java.io.IOException +import java.util.concurrent.atomic.AtomicBoolean +import java.util.concurrent.atomic.AtomicReference import java.util.concurrent.TimeUnit import okhttp3.Call import okhttp3.Callback @@ -19,16 +21,62 @@ class InternalDownloadManager( private val hasAvailableSpace: () -> Boolean ) { private val tag = "InternalDownloadManager" + + interface DownloadHandle { + fun cancel() + } + + private class ActiveDownloadHandle : DownloadHandle { + private val cancelled = AtomicBoolean(false) + private val activeCall = AtomicReference() + + fun setCall(call: Call) { + activeCall.set(call) + if (cancelled.get()) call.cancel() + } + + override fun cancel() { + cancelled.set(true) + activeCall.get()?.cancel() + } + } /** * Starts or resumes a download. * * @param url download URL * @param token access token sent in the Authorization header - * @return active call, used to cancel a stalled transfer + * @return logical handle used to cancel the active request, including a restarted request */ - fun download(url: String, token: String): Call { + fun download(url: String, token: String): DownloadHandle { destinationFile.parentFile?.mkdirs() - val existingBytes = destinationFile.takeIf { it.exists() }?.length() ?: 0L + val handle = ActiveDownloadHandle() + startRequest(url, token, handle, allowRestart = true) + return handle + } + + private fun startRequest( + url: String, + token: String, + handle: ActiveDownloadHandle, + allowRestart: Boolean + ) { + var existingBytes = destinationFile.takeIf { it.exists() }?.length() ?: 0L + when (DownloadResumePolicy.initialAction(existingBytes, expectedSize)) { + DownloadResumePolicy.InitialAction.COMPLETE -> { + progressCallback.onProgress(existingBytes, 100L) + progressCallback.onComplete(false) + return + } + DownloadResumePolicy.InitialAction.RESTART -> { + if (!destinationFile.delete()) { + Log.e(tag, "Could not delete oversized staging file ${destinationFile.name}") + progressCallback.onComplete(true) + return + } + existingBytes = 0L + } + else -> Unit + } val request = Request.Builder() .url(url) @@ -37,6 +85,7 @@ class InternalDownloadManager( .apply { if (existingBytes > 0L) header("Range", "bytes=$existingBytes-") } .build() val call = client.newCall(request) + handle.setCall(call) call.enqueue( object : Callback { override fun onFailure(call: Call, e: IOException) { @@ -47,10 +96,20 @@ class InternalDownloadManager( override fun onResponse(call: Call, response: Response) { response.use { try { - if (response.code == 416 && expectedSize > 0L && existingBytes == expectedSize - ) { - progressCallback.onProgress(existingBytes, 100L) - progressCallback.onComplete(false) + if (response.code == 416) { + val serverSize = + DownloadResumePolicy.unsatisfiedRangeSize( + response.header("Content-Range")) + if (serverSize != null && serverSize > 0L && existingBytes == serverSize) { + progressCallback.onProgress(existingBytes, 100L) + progressCallback.onComplete(false) + } else if (allowRestart && destinationFile.delete()) { + Log.w(tag, "Restarting stale range from byte zero") + startRequest(url, token, handle, allowRestart = false) + } else { + Log.e(tag, "Could not recover invalid range at offset $existingBytes") + progressCallback.onComplete(true) + } return } val append = @@ -112,7 +171,6 @@ class InternalDownloadManager( } } ) - return call } private fun hasExpectedRange(response: Response, offset: Long): Boolean { diff --git a/android/app/src/main/java/com/audiobookshelf/app/models/DownloadItem.kt b/android/app/src/main/java/com/audiobookshelf/app/models/DownloadItem.kt index af1657b7..cfab08d1 100644 --- a/android/app/src/main/java/com/audiobookshelf/app/models/DownloadItem.kt +++ b/android/app/src/main/java/com/audiobookshelf/app/models/DownloadItem.kt @@ -20,7 +20,8 @@ data class DownloadItem( val itemSubfolder: String, val media: MediaType, val downloadItemParts: MutableList, - @JsonIgnore var terminalFailureAt: Long? = null + @JsonIgnore var terminalFailureAt: Long? = null, + @JsonIgnore var stagingCleanupAt: Long? = null ) { @get:JsonIgnore val isInternalStorage @@ -28,7 +29,9 @@ data class DownloadItem( @get:JsonIgnore val isDownloadFinished - get() = !downloadItemParts.any { !it.completed || it.isMoving || it.failed } + get() = downloadItemParts.isNotEmpty() && downloadItemParts.all { + it.completed && it.moved && !it.isMoving && !it.failed + } @JsonIgnore fun getNextDownloadItemParts(limit: Int): MutableList { 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 d9dde269..b2d6ebdf 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 @@ -87,54 +87,57 @@ class AbsDownloader : Plugin() { Log.d(tag, "Download library item $libraryItemId to folder $localFolderId / episode: $episodeId") val downloadId = if (episodeId.isEmpty()) libraryItemId else "$libraryItemId-$episodeId" - if (downloadItemManager.downloadItemQueue.find { it.id == downloadId } != null) { - Log.d(tag, "Download already started for this media entity $downloadId") - return call.resolve(JSObject("{\"error\":\"Download already started for this media entity\"}")) - } - - apiHandler.getLibraryItemWithProgress(libraryItemId, episodeId) { libraryItem -> - if (libraryItem == null) { - call.resolve(JSObject("{\"error\":\"Server request failed\"}")) - } else { - Log.d(tag, "Got library item from server ${libraryItem.id}") - - if (localFolderId == "") { - localFolderId = "internal-${libraryItem.mediaType}" + DownloadServiceHost.retryExisting(mainActivity, downloadId) { result -> + when (result) { + DownloadServiceHost.ExistingDownloadResult.RETRIED -> call.resolve() + DownloadServiceHost.ExistingDownloadResult.ACTIVE -> { + Log.d(tag, "Download already started for this media entity $downloadId") + call.resolve(JSObject("{\"error\":\"Download already started for this media entity\"}")) } - var localFolder = DeviceManager.dbManager.getLocalFolder(localFolderId) - - if (localFolder == null && localFolderId.startsWith("internal-")) { - Log.d(tag, "Creating new App Storage internal LocalFolder $localFolderId") - localFolder = LocalFolder(localFolderId, "Internal App Storage", "", "", "", "internal", libraryItem.mediaType) - DeviceManager.dbManager.saveLocalFolder(localFolder) - } - - if (localFolder != null) { - if (episodeId.isNotEmpty() && libraryItem.mediaType != "podcast") { - Log.e(tag, "Library item is not a podcast but episode was requested") - call.resolve(JSObject("{\"error\":\"Invalid library item not a podcast\"}")) - } else if (episodeId.isNotEmpty()) { - val podcast = libraryItem.media as Podcast - val episode = podcast.episodes?.find { podcastEpisode -> - podcastEpisode.id == episodeId - } - if (episode == null) { - call.resolve(JSObject("{\"error\":\"Invalid podcast episode not found\"}")) + DownloadServiceHost.ExistingDownloadResult.SERVICE_START_FAILED -> + call.resolve(JSObject("{\"error\":\"Unable to start the Android download service\"}")) + DownloadServiceHost.ExistingDownloadResult.NOT_FOUND -> { + apiHandler.getLibraryItemWithProgress(libraryItemId, episodeId) { libraryItem -> + if (libraryItem == null) { + call.resolve(JSObject("{\"error\":\"Server request failed\"}")) } else { - startLibraryItemDownload(libraryItem, localFolder, episode) - call.resolve() + Log.d(tag, "Got library item from server ${libraryItem.id}") + + if (localFolderId == "") localFolderId = "internal-${libraryItem.mediaType}" + var localFolder = DeviceManager.dbManager.getLocalFolder(localFolderId) + if (localFolder == null && localFolderId.startsWith("internal-")) { + Log.d(tag, "Creating new App Storage internal LocalFolder $localFolderId") + localFolder = LocalFolder(localFolderId, "Internal App Storage", "", "", "", "internal", libraryItem.mediaType) + DeviceManager.dbManager.saveLocalFolder(localFolder) + } + + if (localFolder == null) { + call.resolve(JSObject("{\"error\":\"Local Folder Not Found\"}")) + } else if (episodeId.isNotEmpty() && libraryItem.mediaType != "podcast") { + call.resolve(JSObject("{\"error\":\"Invalid library item not a podcast\"}")) + } else if (episodeId.isNotEmpty()) { + val podcast = libraryItem.media as Podcast + val episode = podcast.episodes?.find { it.id == episodeId } + if (episode == null) { + call.resolve(JSObject("{\"error\":\"Invalid podcast episode not found\"}")) + } else { + startLibraryItemDownload(libraryItem, localFolder, episode) { error -> resolveDownloadCall(call, error) } + } + } else { + startLibraryItemDownload(libraryItem, localFolder, null) { error -> resolveDownloadCall(call, error) } + } } - } else { - startLibraryItemDownload(libraryItem, localFolder, null) - call.resolve() } - } else { - call.resolve(JSObject("{\"error\":\"Local Folder Not Found\"}")) } } } } + private fun resolveDownloadCall(call: PluginCall, error: String?) { + if (error == null) call.resolve() + else call.resolve(JSObject().put("error", error)) + } + // Item filenames could be the same if they are in sub-folders, this will make them unique private fun getFilenameFromRelPath(relPath: String): String { var cleanedRelPath = relPath.replace("\\", "_").replace("/", "_") @@ -155,7 +158,12 @@ class AbsDownloader : Plugin() { return newTitle } - private fun startLibraryItemDownload(libraryItem: LibraryItem, localFolder: LocalFolder, episode:PodcastEpisode?) { + private fun startLibraryItemDownload( + libraryItem: LibraryItem, + localFolder: LocalFolder, + episode: PodcastEpisode?, + callback: (String?) -> Unit + ) { val isInternal = localFolder.id.startsWith("internal-") val finalInternalFolderPath = "${mainActivity.filesDir}/downloads/${libraryItem.id}" @@ -224,8 +232,8 @@ class AbsDownloader : Plugin() { downloadItem.downloadItemParts.add(downloadItemPart) } - DownloadServiceHost.enqueue(mainActivity, downloadItem) - } + DownloadServiceHost.enqueue(mainActivity, downloadItem, callback) + } else callback("No downloadable files found") } else { // Podcast episode download val podcastTitle = cleanStringForFileSystem(libraryItem.media.metadata.title) @@ -262,7 +270,7 @@ class AbsDownloader : Plugin() { downloadItem.downloadItemParts.add(downloadItemPart) } - DownloadServiceHost.enqueue(mainActivity, downloadItem) + DownloadServiceHost.enqueue(mainActivity, downloadItem, callback) } } } diff --git a/android/app/src/main/java/com/audiobookshelf/app/services/DownloadService.kt b/android/app/src/main/java/com/audiobookshelf/app/services/DownloadService.kt index 51e9ae31..3ee79eaf 100644 --- a/android/app/src/main/java/com/audiobookshelf/app/services/DownloadService.kt +++ b/android/app/src/main/java/com/audiobookshelf/app/services/DownloadService.kt @@ -28,7 +28,7 @@ class DownloadService : Service() { ACTION_CANCEL -> DownloadServiceHost.cancelAll(this) else -> { startForegroundWithType(DownloadServiceHost.notificationStrings(this).preparing) - DownloadServiceHost.ensure(this) + DownloadServiceHost.startWork(this) } } return START_STICKY 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 38f64a74..d385eb19 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 @@ -1,16 +1,25 @@ package com.audiobookshelf.app.services import android.content.Context +import android.util.Log import androidx.core.content.ContextCompat import com.audiobookshelf.app.device.FolderScanner import com.audiobookshelf.app.managers.DbManager import com.audiobookshelf.app.managers.DownloadItemManager +import com.audiobookshelf.app.managers.IncompleteDownloadCleanup import com.audiobookshelf.app.models.DownloadItem import com.getcapacitor.JSObject import java.util.Collections +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.Job +import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.launch /** Shared process owner used by the foreground service and the Capacitor bridge. */ object DownloadServiceHost { + enum class ExistingDownloadResult { NOT_FOUND, ACTIVE, RETRIED, SERVICE_START_FAILED } + data class NotificationStrings( val preparing: String, val downloadingFile: String, @@ -21,9 +30,11 @@ object DownloadServiceHost { private var manager: DownloadItemManager? = null private var bridgeEmitter: DownloadItemManager.DownloadEventEmitter = NoopEmitter - private var service: DownloadService? = null + @Volatile private var service: DownloadService? = null @Volatile private var bridgeReady = false private val deferredCompletions = Collections.synchronizedList(mutableListOf()) + private val scope = CoroutineScope(SupervisorJob() + Dispatchers.IO) + private var restoreJob: Job? = null @Synchronized fun ensure(context: Context): DownloadItemManager { @@ -31,7 +42,11 @@ object DownloadServiceHost { val appContext = context.applicationContext DbManager.initialize(appContext) manager = DownloadItemManager(FolderScanner(appContext), appContext, ForwardingEmitter) - manager!!.restoreQueue() + restoreJob = scope.launch { + IncompleteDownloadCleanup.cleanupExpired(appContext) + manager!!.restoreQueue() + onRestoreComplete(appContext) + } } return manager!! } @@ -41,14 +56,12 @@ object DownloadServiceHost { fun attachBridge(context: Context, emitter: DownloadItemManager.DownloadEventEmitter) { bridgeReady = false bridgeEmitter = emitter - val queue = ensure(context) - queue.setEventEmitter(ForwardingEmitter) + ensure(context).setEventEmitter(ForwardingEmitter) bridgeReady = true val completions = synchronized(deferredCompletions) { deferredCompletions.toList().also { deferredCompletions.clear() } } completions.forEach(bridgeEmitter::onDownloadItemComplete) - if (queue.hasWork()) startService(context) } @Synchronized @@ -57,14 +70,44 @@ object DownloadServiceHost { bridgeEmitter = NoopEmitter } - @Synchronized - fun enqueue(context: Context, item: DownloadItem) { - ensure(context).addDownloadItem(item) - startService(context) + fun enqueue(context: Context, item: DownloadItem, callback: (String?) -> Unit) { + val queue = ensure(context) + scope.launch { + restoreJob?.join() + queue.addDownloadItem(item) + if (startService(context)) callback(null) + else callback("Unable to start the Android download service") + } } - @Synchronized - fun cancelAll(context: Context) { ensure(context).cancelAll() } + fun retryExisting( + context: Context, + downloadItemId: String, + callback: (ExistingDownloadResult) -> Unit + ) { + val queue = ensure(context) + scope.launch { + restoreJob?.join() + val existing = queue.downloadItemQueue.find { it.id == downloadItemId } + if (existing == null) { + callback(ExistingDownloadResult.NOT_FOUND) + } else if (!queue.retryDownloadItem(downloadItemId)) { + callback(ExistingDownloadResult.ACTIVE) + } else if (startService(context)) { + callback(ExistingDownloadResult.RETRIED) + } else { + callback(ExistingDownloadResult.SERVICE_START_FAILED) + } + } + } + + fun cancelAll(context: Context) { + val queue = ensure(context) + scope.launch { + restoreJob?.join() + queue.cancelAll() + } + } fun setNotificationStrings( context: Context, @@ -96,10 +139,9 @@ object DownloadServiceHost { preferences.getString(KEY_CANCEL, DEFAULT_CANCEL) ?: DEFAULT_CANCEL) } - @Synchronized fun attachService(downloadService: DownloadService) { - service = downloadService - service?.onQueueChanged(ensure(downloadService).hasWork()) + synchronized(this) { service = downloadService } + startWork(downloadService) } @Synchronized @@ -107,8 +149,31 @@ object DownloadServiceHost { if (service === downloadService) service = null } - private fun startService(context: Context) { - ContextCompat.startForegroundService(context, DownloadService.intent(context)) + fun startWork(context: Context) { + val queue = ensure(context) + scope.launch { + restoreJob?.join() + queue.resumeWork() + } + } + + private fun onRestoreComplete(context: Context) { + val attachedService = synchronized(this) { service } + if (attachedService != null) { + manager?.resumeWork() + } else if (bridgeReady && manager?.hasWork() == true) { + startService(context) + } + } + + private fun startService(context: Context): Boolean { + return try { + ContextCompat.startForegroundService(context, DownloadService.intent(context)) + true + } catch (e: RuntimeException) { + Log.e(TAG, "Could not start download foreground service", e) + false + } } private object ForwardingEmitter : DownloadItemManager.DownloadEventEmitter { @@ -144,4 +209,5 @@ object DownloadServiceHost { private const val DEFAULT_WAITING_FOR_STORAGE = "Waiting for available storage" private const val DEFAULT_DOWNLOADS = "Downloads" private const val DEFAULT_CANCEL = "Cancel" + private const val TAG = "DownloadServiceHost" } From 79896073135d2b7c7ad39945bdd1c7d59bdca3b2 Mon Sep 17 00:00:00 2001 From: Nicholas Wallace Date: Thu, 27 Aug 2026 13:26:51 -0700 Subject: [PATCH 02/14] Retry from beginning of file on failure and restore --- .../app/managers/DownloadItemManager.kt | 62 +++++++++++-------- 1 file changed, 37 insertions(+), 25 deletions(-) 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 9bf6eee9..57ae0fc8 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 @@ -84,23 +84,17 @@ class DownloadItemManager( checkDownloadItemFinished(item) return@forEach } + var resetFailed = false item.downloadItemParts.forEach { part -> if (part.moved) return@forEach - if (item.terminalFailureAt != null) { - part.downloadId = null - part.isMoving = false - part.failed = true - part.waitingForSpace = false - part.bytesDownloaded = File(part.destinationPath).takeIf(File::exists)?.length() ?: 0L - return@forEach - } - part.downloadId = null - part.isMoving = false - part.failed = false - part.waitingForSpace = false - val stagingLength = File(part.destinationPath).takeIf(File::exists)?.length() ?: 0L - part.bytesDownloaded = stagingLength - if (part.completed && stagingLength <= 0L) part.completed = false + if (!resetPartForFreshDownload(part)) resetFailed = true + } + if (resetFailed) { + item.terminalFailureAt = item.terminalFailureAt ?: System.currentTimeMillis() + item.stagingCleanupAt = null + } + if (item.terminalFailureAt != null) { + item.downloadItemParts.filter { !it.moved }.forEach { it.failed = true } } downloadItemQueue.add(item) if (item.terminalFailureAt != null) IncompleteDownloadCleanup.schedule(context, item) @@ -127,19 +121,18 @@ class DownloadItemManager( if (item.downloadItemParts.any { it in currentDownloadItemParts }) return false if (item.isDownloadFinished) return false synchronized(IncompleteDownloadCleanup) { + var resetFailed = false + item.downloadItemParts.filter { !it.moved }.forEach { part -> + if (!resetPartForFreshDownload(part)) resetFailed = true + } + if (resetFailed) { + item.downloadItemParts.filter { !it.moved }.forEach { it.failed = true } + persist(item, force = true) + return false + } item.terminalFailureAt = null item.stagingCleanupAt = null IncompleteDownloadCleanup.cancel(context, item.id) - item.downloadItemParts.filter { !it.moved }.forEach { part -> - part.failed = false - part.isMoving = false - part.downloadId = null - part.retryCount = 0 - part.waitingForSpace = false - val stagingLength = File(part.destinationPath).takeIf(File::exists)?.length() ?: 0L - part.bytesDownloaded = stagingLength - part.completed = part.completed && stagingLength > 0L - } persist(item, force = true) } clientEventEmitter.onDownloadItem(item) @@ -562,6 +555,25 @@ class DownloadItemManager( return findSharedStorageFile(part) != null } + /** Resets an unmoved part when recovery crosses a service-session boundary. */ + private fun resetPartForFreshDownload(part: DownloadItemPart): Boolean { + val stagingFile = File(part.destinationPath) + if (stagingFile.exists() && !stagingFile.delete()) { + Log.e(tag, "Could not delete staging file ${part.filename}") + part.failed = true + return false + } + part.completed = false + part.bytesDownloaded = 0L + part.progress = 0L + part.failed = false + part.isMoving = false + part.downloadId = null + part.retryCount = 0 + part.waitingForSpace = false + return true + } + private fun findDocumentByFilename(folder: DocumentFile, part: DownloadItemPart): DocumentFile? { folder.findFile(part.filename)?.let { return it } val expectedBaseName = part.filename.substringBeforeLast('.') From c800ed727afd1ffa1008bcff437bf41ffa8b66d1 Mon Sep 17 00:00:00 2001 From: Nicholas Wallace Date: Thu, 27 Aug 2026 14:52:14 -0700 Subject: [PATCH 03/14] Fix concurrent podcast download cover issue --- .../app/managers/DownloadItemManager.kt | 98 +++++++--- .../app/models/DownloadItemPart.kt | 6 +- .../app/plugins/AbsDownloader.kt | 5 +- .../app/services/DownloadServiceHost.kt | 6 +- .../app/managers/DownloadResumePolicyTest.kt | 41 ++++ .../managers/InternalDownloadManagerTest.kt | 176 ++++++++++++++++++ .../widgets/DownloadProgressIndicator.vue | 6 +- pages/downloading.vue | 3 +- 8 files changed, 306 insertions(+), 35 deletions(-) create mode 100644 android/app/src/test/java/com/audiobookshelf/app/managers/DownloadResumePolicyTest.kt create mode 100644 android/app/src/test/java/com/audiobookshelf/app/managers/InternalDownloadManagerTest.kt 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