Decouple Cache and Download behaviour (#493)

* Separate cache dir from download dir

* Move downloader logic outside of caching/image download logic

* remove unnecessary method duplication

* moved download logic inside download provider

* optimize and handle partial downloads

* made code review changes

server/src/main/kotlin/suwayomi/tachidesk/manga/impl/Chapter.kt

@@ -327,9 +327,7 @@ object Chapter {
                 ChapterTable.select { (ChapterTable.manga eq mangaId) and (ChapterTable.sourceOrder eq chapterIndex) }
                     .first()[ChapterTable.id].value
 
-            val chapterDir = getChapterDir(mangaId, chapterId)
+            ChapterDownloadHelper.delete(mangaId, chapterId)
-
-            File(chapterDir).deleteRecursively()
 
             ChapterTable.update({ (ChapterTable.manga eq mangaId) and (ChapterTable.sourceOrder eq chapterIndex) }) {
                 it[isDownloaded] = false

server/src/main/kotlin/suwayomi/tachidesk/manga/impl/ChapterDownloadHelper.kt

@@ -0,0 +1,32 @@
+package suwayomi.tachidesk.manga.impl
+
+import kotlinx.coroutines.CoroutineScope
+import suwayomi.tachidesk.manga.impl.download.DownloadedFilesProvider
+import suwayomi.tachidesk.manga.impl.download.FolderProvider
+import suwayomi.tachidesk.manga.impl.download.model.DownloadChapter
+import java.io.InputStream
+
+object ChapterDownloadHelper {
+    fun getImage(mangaId: Int, chapterId: Int, index: Int): Pair<InputStream, String> {
+        return provider(mangaId, chapterId).getImage(index)
+    }
+
+    fun delete(mangaId: Int, chapterId: Int): Boolean {
+        return provider(mangaId, chapterId).delete()
+    }
+
+    suspend fun download(
+        mangaId: Int,
+        chapterId: Int,
+        download: DownloadChapter,
+        scope: CoroutineScope,
+        step: suspend (DownloadChapter?, Boolean) -> Unit
+    ): Boolean {
+        return provider(mangaId, chapterId).download(download, scope, step)
+    }
+
+    // return the appropriate provider based on how the download was saved. For the logic is simple but will evolve when new types of downloads are available
+    private fun provider(mangaId: Int, chapterId: Int): DownloadedFilesProvider {
+        return FolderProvider(mangaId, chapterId)
+    }
+}

server/src/main/kotlin/suwayomi/tachidesk/manga/impl/Page.kt

@@ -15,7 +15,6 @@ import org.jetbrains.exposed.sql.and
 import org.jetbrains.exposed.sql.select
 import org.jetbrains.exposed.sql.transactions.transaction
 import org.jetbrains.exposed.sql.update
-import suwayomi.tachidesk.manga.impl.util.getChapterDir
 import suwayomi.tachidesk.manga.impl.util.lang.awaitSingle
 import suwayomi.tachidesk.manga.impl.util.source.GetCatalogueSource.getCatalogueSourceOrStub
 import suwayomi.tachidesk.manga.impl.util.storage.ImageResponse.getImageResponse
@@ -82,11 +81,13 @@ object Page {
             }
         }
 
-        val chapterDir = getChapterDir(mangaId, chapterId)
-        File(chapterDir).mkdirs()
         val fileName = getPageName(index)
 
-        return getImageResponse(chapterDir, fileName, useCache) {
+        if (chapterEntry[ChapterTable.isDownloaded]) {
+            return ChapterDownloadHelper.getImage(mangaId, chapterId, index)
+        }
+
+        return getImageResponse(mangaId, chapterId, fileName, useCache) {
             source.fetchImage(tachiyomiPage).awaitSingle()
         }
     }

server/src/main/kotlin/suwayomi/tachidesk/manga/impl/download/DownloadedFilesProvider.kt

@@ -0,0 +1,20 @@
+package suwayomi.tachidesk.manga.impl.download
+
+import kotlinx.coroutines.CoroutineScope
+import suwayomi.tachidesk.manga.impl.download.model.DownloadChapter
+import java.io.InputStream
+
+/*
+* Base class for downloaded chapter files provider, example: Folder, Archive
+* */
+abstract class DownloadedFilesProvider(val mangaId: Int, val chapterId: Int) {
+    abstract fun getImage(index: Int): Pair<InputStream, String>
+
+    abstract suspend fun download(
+        download: DownloadChapter,
+        scope: CoroutineScope,
+        step: suspend (DownloadChapter?, Boolean) -> Unit
+    ): Boolean
+
+    abstract fun delete(): Boolean
+}

server/src/main/kotlin/suwayomi/tachidesk/manga/impl/download/Downloader.kt

@@ -13,17 +13,13 @@ import kotlinx.coroutines.Job
 import kotlinx.coroutines.cancelAndJoin
 import kotlinx.coroutines.currentCoroutineContext
 import kotlinx.coroutines.ensureActive
-import kotlinx.coroutines.flow.distinctUntilChanged
-import kotlinx.coroutines.flow.launchIn
-import kotlinx.coroutines.flow.onEach
-import kotlinx.coroutines.flow.sample
 import kotlinx.coroutines.isActive
 import kotlinx.coroutines.launch
 import mu.KotlinLogging
 import org.jetbrains.exposed.sql.and
 import org.jetbrains.exposed.sql.transactions.transaction
 import org.jetbrains.exposed.sql.update
-import suwayomi.tachidesk.manga.impl.Page.getPageImage
+import suwayomi.tachidesk.manga.impl.ChapterDownloadHelper
 import suwayomi.tachidesk.manga.impl.chapter.getChapterDownloadReady
 import suwayomi.tachidesk.manga.impl.download.model.DownloadChapter
 import suwayomi.tachidesk.manga.impl.download.model.DownloadState.Downloading
@@ -95,33 +91,7 @@ class Downloader(
                 download.chapter = getChapterDownloadReady(download.chapterIndex, download.mangaId)
                 step(download, false)
 
-                val pageCount = download.chapter.pageCount
+                ChapterDownloadHelper.download(download.mangaId, download.chapter.id, download, scope, this::step)
-                for (pageNum in 0 until pageCount) {
-                    var pageProgressJob: Job? = null
-                    try {
-                        getPageImage(
-                            mangaId = download.mangaId,
-                            chapterIndex = download.chapterIndex,
-                            index = pageNum,
-                            progressFlow = { 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)
-                }
                 download.state = Finished
                 transaction {
                     ChapterTable.update({ (ChapterTable.manga eq download.mangaId) and (ChapterTable.sourceOrder eq download.chapterIndex) }) {

server/src/main/kotlin/suwayomi/tachidesk/manga/impl/download/FolderProvider.kt

@@ -0,0 +1,87 @@
+package suwayomi.tachidesk.manga.impl.download
+
+import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.FlowPreview
+import kotlinx.coroutines.Job
+import kotlinx.coroutines.flow.distinctUntilChanged
+import kotlinx.coroutines.flow.launchIn
+import kotlinx.coroutines.flow.onEach
+import kotlinx.coroutines.flow.sample
+import suwayomi.tachidesk.manga.impl.Page
+import suwayomi.tachidesk.manga.impl.Page.getPageName
+import suwayomi.tachidesk.manga.impl.download.model.DownloadChapter
+import suwayomi.tachidesk.manga.impl.util.getChapterDir
+import suwayomi.tachidesk.manga.impl.util.storage.ImageResponse
+import java.io.File
+import java.io.FileInputStream
+import java.io.InputStream
+
+/*
+* Provides downloaded files when pages were downloaded into folders
+* */
+class FolderProvider(mangaId: Int, chapterId: Int) : DownloadedFilesProvider(mangaId, chapterId) {
+    override fun getImage(index: Int): Pair<InputStream, String> {
+        val chapterDir = getChapterDir(mangaId, chapterId)
+        val folder = File(chapterDir)
+        folder.mkdirs()
+        val file = folder.listFiles()?.get(index)
+        val fileType = file!!.name.substringAfterLast(".")
+        return Pair(FileInputStream(file).buffered(), "image/$fileType")
+    }
+
+    @OptIn(FlowPreview::class)
+    override suspend fun download(
+        download: DownloadChapter,
+        scope: CoroutineScope,
+        step: suspend (DownloadChapter?, Boolean) -> Unit
+    ): Boolean {
+        val pageCount = download.chapter.pageCount
+        val chapterDir = getChapterDir(mangaId, chapterId)
+        val folder = File(chapterDir)
+        folder.mkdirs()
+
+        for (pageNum in 0 until pageCount) {
+            var pageProgressJob: Job? = null
+            val fileName = getPageName(pageNum) // might have to change this to index stored in database
+            if (isExistingFile(folder, fileName)) 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.use { image ->
+                    val filePath = "$chapterDir/$fileName"
+                    ImageResponse.saveImage(filePath, image)
+                }
+            } 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)
+        }
+        return true
+    }
+
+    override fun delete(): Boolean {
+        val chapterDir = getChapterDir(mangaId, chapterId)
+        return File(chapterDir).deleteRecursively()
+    }
+
+    private fun isExistingFile(folder: File, fileName: String): Boolean {
+        val existingFile = folder.listFiles { file ->
+            file.isFile && file.name.startsWith(fileName)
+        }?.firstOrNull()
+        return existingFile?.exists() == true
+    }
+}

server/src/main/kotlin/suwayomi/tachidesk/manga/impl/util/DirName.kt

@@ -7,6 +7,7 @@ package suwayomi.tachidesk.manga.impl.util
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
 
+import org.jetbrains.exposed.sql.ResultRow
 import org.jetbrains.exposed.sql.select
 import org.jetbrains.exposed.sql.transactions.transaction
 import org.kodein.di.DI
@@ -21,17 +22,16 @@ import java.io.File
 
 private val applicationDirs by DI.global.instance<ApplicationDirs>()
 
-fun getMangaDir(mangaId: Int): String {
+fun getMangaDir(mangaId: Int, cache: Boolean = false): String {
-    val mangaEntry = transaction { MangaTable.select { MangaTable.id eq mangaId }.first() }
+    val mangaEntry = getMangaEntry(mangaId)
     val source = GetCatalogueSource.getCatalogueSourceOrStub(mangaEntry[MangaTable.sourceReference])
 
     val sourceDir = source.toString()
     val mangaDir = SafePath.buildValidFilename(mangaEntry[MangaTable.title])
-
+    return (if (cache) applicationDirs.cacheRoot else applicationDirs.mangaDownloadsRoot) + "/$sourceDir/$mangaDir"
-    return "${applicationDirs.mangaDownloadsRoot}/$sourceDir/$mangaDir"
 }
 
-fun getChapterDir(mangaId: Int, chapterId: Int): String {
+fun getChapterDir(mangaId: Int, chapterId: Int, cache: Boolean = false): String {
     val chapterEntry = transaction { ChapterTable.select { ChapterTable.id eq chapterId }.first() }
 
     val chapterDir = SafePath.buildValidFilename(
@@ -41,12 +41,12 @@ fun getChapterDir(mangaId: Int, chapterId: Int): String {
         }
     )
 
-    return getMangaDir(mangaId) + "/$chapterDir"
+    return getMangaDir(mangaId, cache) + "/$chapterDir"
 }
 
 /** return value says if rename/move was successful */
 fun updateMangaDownloadDir(mangaId: Int, newTitle: String): Boolean {
-    val mangaEntry = transaction { MangaTable.select { MangaTable.id eq mangaId }.first() }
+    val mangaEntry = getMangaEntry(mangaId)
     val source = GetCatalogueSource.getCatalogueSourceOrStub(mangaEntry[MangaTable.sourceReference])
 
     val sourceDir = source.toString()
@@ -66,3 +66,7 @@ fun updateMangaDownloadDir(mangaId: Int, newTitle: String): Boolean {
         true
     }
 }
+
+private fun getMangaEntry(mangaId: Int): ResultRow {
+    return transaction { MangaTable.select { MangaTable.id eq mangaId }.first() }
+}

server/src/main/kotlin/suwayomi/tachidesk/manga/impl/util/storage/ImageResponse.kt

@@ -9,6 +9,7 @@ package suwayomi.tachidesk.manga.impl.util.storage
 
 import okhttp3.Response
 import okhttp3.internal.closeQuietly
+import suwayomi.tachidesk.manga.impl.util.getChapterDir
 import java.io.File
 import java.io.FileInputStream
 import java.io.InputStream
@@ -29,7 +30,7 @@ object ImageResponse {
     }
 
     /** fetch a cached image response, calls `fetcher` if cache fails */
-    suspend fun getCachedImageResponse(saveDir: String, fileName: String, fetcher: suspend () -> Response): Pair<InputStream, String> {
+    private suspend fun getCachedImageResponse(saveDir: String, fileName: String, fetcher: suspend () -> Response): Pair<InputStream, String> {
         val cachedFile = findFileNameStartingWith(saveDir, fileName)
         val filePath = "$saveDir/$fileName"
         if (cachedFile != null) {
@@ -43,19 +44,7 @@ object ImageResponse {
         val response = fetcher()
 
         if (response.code == 200) {
-            val tmpSavePath = "$filePath.tmp"
+            val (actualSavePath, imageType) = saveImage(filePath, response.body!!.byteStream())
-            val tmpSaveFile = File(tmpSavePath)
-            response.body!!.source().saveTo(tmpSaveFile)
-
-            // find image type
-            val imageType = response.headers["content-type"]
-                ?: ImageUtil.findImageType { tmpSaveFile.inputStream() }?.mime
-                ?: "image/jpeg"
-
-            val actualSavePath = "$filePath.${imageType.substringAfter("/")}"
-
-            tmpSaveFile.renameTo(File(actualSavePath))
-
             return pathToInputStream(actualSavePath) to imageType
         } else {
             response.closeQuietly()
@@ -63,6 +52,21 @@ object ImageResponse {
         }
     }
 
+    fun saveImage(filePath: String, image: InputStream): Pair<String, String> {
+        val tmpSavePath = "$filePath.tmp"
+        val tmpSaveFile = File(tmpSavePath)
+        image.use { input -> tmpSaveFile.outputStream().use { output -> input.copyTo(output) } }
+
+        // find image type
+        val imageType = ImageUtil.findImageType { tmpSaveFile.inputStream() }?.mime
+            ?: "image/jpeg"
+
+        val actualSavePath = "$filePath.${imageType.substringAfter("/")}"
+
+        tmpSaveFile.renameTo(File(actualSavePath))
+        return Pair(actualSavePath, imageType)
+    }
+
     fun clearCachedImage(saveDir: String, fileName: String) {
         val cachedFile = findFileNameStartingWith(saveDir, fileName)
         cachedFile?.also {
@@ -70,7 +74,7 @@ object ImageResponse {
         }
     }
 
-    suspend fun getNoCacheImageResponse(fetcher: suspend () -> Response): Pair<InputStream, String> {
+    private suspend fun getNoCacheImageResponse(fetcher: suspend () -> Response): Pair<InputStream, String> {
         val response = fetcher()
 
         if (response.code == 200) {
@@ -88,11 +92,20 @@ object ImageResponse {
         }
     }
 
-    suspend fun getImageResponse(saveDir: String, fileName: String, useCache: Boolean = false, fetcher: suspend () -> Response): Pair<InputStream, String> {
+    suspend fun getImageResponse(saveDir: String, fileName: String, useCache: Boolean, fetcher: suspend () -> Response): Pair<InputStream, String> {
         return if (useCache) {
             getCachedImageResponse(saveDir, fileName, fetcher)
         } else {
             getNoCacheImageResponse(fetcher)
         }
     }
+
+    suspend fun getImageResponse(mangaId: Int, chapterId: Int, fileName: String, useCache: Boolean, fetcher: suspend () -> Response): Pair<InputStream, String> {
+        var saveDir = ""
+        if (useCache) {
+            saveDir = getChapterDir(mangaId, chapterId, true)
+            File(saveDir).mkdirs()
+        }
+        return getImageResponse(saveDir, fileName, useCache, fetcher)
+    }
 }

server/src/main/kotlin/suwayomi/tachidesk/server/ServerSetup.kt

@@ -38,6 +38,7 @@ private val logger = KotlinLogging.logger {}
 class ApplicationDirs(
     val dataRoot: String = ApplicationRootDir
 ) {
+    val cacheRoot = System.getProperty("java.io.tmpdir") + "/tachidesk"
     val extensionsRoot = "$dataRoot/extensions"
     val thumbnailsRoot = "$dataRoot/thumbnails"
     val mangaDownloadsRoot = serverConfig.downloadsPath.ifBlank { "$dataRoot/downloads" }