From 1d9991e5627c5ea0a8e2d8cf847578325b556630 Mon Sep 17 00:00:00 2001 From: schroda <50052685+schroda@users.noreply.github.com> Date: Fri, 1 Aug 2025 01:55:09 +0200 Subject: [PATCH] Feature/Improve chapter download with valid existing download handling (#1553) * Fix early exit on download for existing download for FolderProvider The current check only worked for the "ArchiveProvider". The "FolderProvider" never moved the existing download to the cache folder. In case the existing download is considered to be reusable, there is no need to proceed with the download logic. * Fix "ArchiveProvider#extractExistingDownload" The "ChaptersFilesProvider#extractExistingDownload" expects the download to be extracted into the final download folder. However, the "ArchiveProvider" extracted the download into the chapter download cache folder. * Add chapter download function call requirements --- .../manga/impl/ChapterDownloadHelper.kt | 4 + .../manga/impl/chapter/ChapterForDownload.kt | 2 +- .../fileProvider/ChaptersFilesProvider.kt | 116 +++++++++--------- .../fileProvider/impl/ArchiveProvider.kt | 5 +- 4 files changed, 69 insertions(+), 58 deletions(-) diff --git a/server/src/main/kotlin/suwayomi/tachidesk/manga/impl/ChapterDownloadHelper.kt b/server/src/main/kotlin/suwayomi/tachidesk/manga/impl/ChapterDownloadHelper.kt index 2c5ce377..05b37e93 100644 --- a/server/src/main/kotlin/suwayomi/tachidesk/manga/impl/ChapterDownloadHelper.kt +++ b/server/src/main/kotlin/suwayomi/tachidesk/manga/impl/ChapterDownloadHelper.kt @@ -2,6 +2,7 @@ package suwayomi.tachidesk.manga.impl import kotlinx.coroutines.CoroutineScope import org.jetbrains.exposed.sql.transactions.transaction +import suwayomi.tachidesk.manga.impl.chapter.getChapterDownloadReady import suwayomi.tachidesk.manga.impl.download.fileProvider.ChaptersFilesProvider import suwayomi.tachidesk.manga.impl.download.fileProvider.impl.ArchiveProvider import suwayomi.tachidesk.manga.impl.download.fileProvider.impl.FolderProvider @@ -32,6 +33,9 @@ object ChapterDownloadHelper { chapterId: Int, ): Boolean = provider(mangaId, chapterId).delete() + /** + * This function should never be called without calling [getChapterDownloadReady] beforehand. + */ suspend fun download( mangaId: Int, chapterId: Int, diff --git a/server/src/main/kotlin/suwayomi/tachidesk/manga/impl/chapter/ChapterForDownload.kt b/server/src/main/kotlin/suwayomi/tachidesk/manga/impl/chapter/ChapterForDownload.kt index b1852d86..ced35318 100644 --- a/server/src/main/kotlin/suwayomi/tachidesk/manga/impl/chapter/ChapterForDownload.kt +++ b/server/src/main/kotlin/suwayomi/tachidesk/manga/impl/chapter/ChapterForDownload.kt @@ -99,7 +99,7 @@ private class ChapterForDownload( if (!doPageCountsMatch) { log.debug { "use page count of downloaded chapter" } - updatePageCount(ChapterDownloadHelper.getImageCount(mangaId, chapterId)) + updatePageCount(downloadPageCount) } return asDataClass() diff --git a/server/src/main/kotlin/suwayomi/tachidesk/manga/impl/download/fileProvider/ChaptersFilesProvider.kt b/server/src/main/kotlin/suwayomi/tachidesk/manga/impl/download/fileProvider/ChaptersFilesProvider.kt index 6de0382d..939b7a7d 100644 --- a/server/src/main/kotlin/suwayomi/tachidesk/manga/impl/download/fileProvider/ChaptersFilesProvider.kt +++ b/server/src/main/kotlin/suwayomi/tachidesk/manga/impl/download/fileProvider/ChaptersFilesProvider.kt @@ -13,8 +13,8 @@ import libcore.net.MimeUtils import org.apache.commons.compress.archivers.zip.ZipArchiveEntry import org.jetbrains.exposed.sql.selectAll import org.jetbrains.exposed.sql.transactions.transaction -import org.jetbrains.exposed.sql.update import suwayomi.tachidesk.manga.impl.Page +import suwayomi.tachidesk.manga.impl.chapter.getChapterDownloadReady import suwayomi.tachidesk.manga.impl.download.model.DownloadChapter import suwayomi.tachidesk.manga.impl.util.createComicInfoFile import suwayomi.tachidesk.manga.impl.util.getChapterCachePath @@ -105,6 +105,27 @@ abstract class ChaptersFilesProvider( scope: CoroutineScope, step: suspend (DownloadChapter?, Boolean) -> Unit, ): Boolean { + val existingDownloadPageCount = + try { + getImageCount() + } catch (_: Exception) { + 0 + } + val pageCount = download.chapter.pageCount + + check(pageCount > 0) { "pageCount must be greater than 0 - ChapterForDownload#getChapterDownloadReady not called" } + check(existingDownloadPageCount == 0 || existingDownloadPageCount == pageCount) { + "existingDownloadPageCount must be 0 or equal to pageCount - ChapterForDownload#getChapterDownloadReady not called" + } + + val doesUnrecognizedDownloadExist = existingDownloadPageCount == pageCount + if (doesUnrecognizedDownloadExist) { + download.progress = 1f + step(download, false) + + return true + } + extractExistingDownload() val finalDownloadFolder = getChapterDownloadPath(mangaId, chapterId) @@ -113,57 +134,45 @@ abstract class ChaptersFilesProvider( val downloadCacheFolder = File(cacheChapterDir) downloadCacheFolder.mkdirs() - val pageCount = download.chapter.pageCount - if ( - downloadCacheFolder - .listFiles() - .orEmpty() - .filter { it.name != COMIC_INFO_FILE } - .size >= pageCount - ) { - download.progress = 1f - step(download, false) - } else { - for (pageNum in 0 until pageCount) { - var pageProgressJob: Job? = null - val fileName = Page.getPageName(pageNum, pageCount) // might have to change this to index stored in database + for (pageNum in 0 until pageCount) { + var pageProgressJob: Job? = null + val fileName = Page.getPageName(pageNum, pageCount) // might have to change this to index stored in database - val pageExistsInFinalDownloadFolder = ImageResponse.findFileNameStartingWith(finalDownloadFolder, fileName) != null - val pageExistsInCacheDownloadFolder = ImageResponse.findFileNameStartingWith(cacheChapterDir, fileName) != null + val pageExistsInFinalDownloadFolder = ImageResponse.findFileNameStartingWith(finalDownloadFolder, fileName) != null + val pageExistsInCacheDownloadFolder = ImageResponse.findFileNameStartingWith(cacheChapterDir, fileName) != null - val doesPageAlreadyExist = pageExistsInFinalDownloadFolder || pageExistsInCacheDownloadFolder - if (doesPageAlreadyExist) { - continue - } - - try { - Page - .getPageImage( - mangaId = download.mangaId, - chapterIndex = download.chapterIndex, - index = pageNum, - ) { flow -> - pageProgressJob = - flow - .sample(100) - .distinctUntilChanged() - .onEach { - download.progress = (pageNum.toFloat() + (it.toFloat() * 0.01f)) / pageCount - step( - null, - false, - ) // don't throw on canceled download here since we can't do anything - }.launchIn(scope) - }.first - .close() - } finally { - // always cancel the page progress job even if it throws an exception to avoid memory leaks - pageProgressJob?.cancel() - } - // TODO: retry on error with 2,4,8 seconds of wait - download.progress = ((pageNum + 1).toFloat()) / pageCount - step(download, false) + val doesPageAlreadyExist = pageExistsInFinalDownloadFolder || pageExistsInCacheDownloadFolder + if (doesPageAlreadyExist) { + continue } + + try { + Page + .getPageImage( + mangaId = download.mangaId, + chapterIndex = download.chapterIndex, + index = pageNum, + ) { flow -> + pageProgressJob = + flow + .sample(100) + .distinctUntilChanged() + .onEach { + download.progress = (pageNum.toFloat() + (it.toFloat() * 0.01f)) / pageCount + step( + null, + false, + ) // don't throw on canceled download here since we can't do anything + }.launchIn(scope) + }.first + .close() + } finally { + // always cancel the page progress job even if it throws an exception to avoid memory leaks + pageProgressJob?.cancel() + } + // TODO: retry on error with 2,4,8 seconds of wait + download.progress = ((pageNum + 1).toFloat()) / pageCount + step(download, false) } createComicInfoFile( @@ -180,17 +189,14 @@ abstract class ChaptersFilesProvider( handleSuccessfulDownload() - transaction { - ChapterTable.update({ ChapterTable.id eq chapterId }) { - it[ChapterTable.pageCount] = getImageCount() - } - } - File(cacheChapterDir).deleteRecursively() return true } + /** + * This function should never be called without calling [getChapterDownloadReady] beforehand. + */ override fun download(): FileDownload3Args Unit> = FileDownload3Args(::downloadImpl) diff --git a/server/src/main/kotlin/suwayomi/tachidesk/manga/impl/download/fileProvider/impl/ArchiveProvider.kt b/server/src/main/kotlin/suwayomi/tachidesk/manga/impl/download/fileProvider/impl/ArchiveProvider.kt index 45c73eb9..4caa9e87 100644 --- a/server/src/main/kotlin/suwayomi/tachidesk/manga/impl/download/fileProvider/impl/ArchiveProvider.kt +++ b/server/src/main/kotlin/suwayomi/tachidesk/manga/impl/download/fileProvider/impl/ArchiveProvider.kt @@ -10,6 +10,7 @@ import suwayomi.tachidesk.manga.impl.download.fileProvider.ChaptersFilesProvider import suwayomi.tachidesk.manga.impl.download.fileProvider.FileType import suwayomi.tachidesk.manga.impl.util.getChapterCachePath import suwayomi.tachidesk.manga.impl.util.getChapterCbzPath +import suwayomi.tachidesk.manga.impl.util.getChapterDownloadPath import suwayomi.tachidesk.manga.impl.util.getMangaDownloadDir import suwayomi.tachidesk.manga.impl.util.storage.FileDeletionHelper import suwayomi.tachidesk.server.ApplicationDirs @@ -37,13 +38,13 @@ class ArchiveProvider( override fun extractExistingDownload() { val outputFile = File(getChapterCbzPath(mangaId, chapterId)) - val chapterCacheFolder = File(getChapterCachePath(mangaId, chapterId)) + val chapterDownloadFolder = File(getChapterDownloadPath(mangaId, chapterId)) if (!outputFile.exists()) { return } - extractCbzFile(outputFile, chapterCacheFolder) + extractCbzFile(outputFile, chapterDownloadFolder) } override suspend fun handleSuccessfulDownload() {