Fix loading pauses between chapters when preloading is enabled

app/src/androidTest/kotlin/io/github/landwarderer/futon/reader/CrossChapterPrefetchReaderTest.kt

@@ -0,0 +1,176 @@
+package io.github.landwarderer.futon.reader
+
+import android.graphics.Bitmap
+import android.graphics.Color
+import androidx.lifecycle.ViewModelProvider
+import androidx.preference.PreferenceManager
+import androidx.test.core.app.ActivityScenario
+import androidx.test.ext.junit.runners.AndroidJUnit4
+import androidx.test.platform.app.InstrumentationRegistry
+import androidx.work.Configuration
+import androidx.work.WorkManager
+import dagger.hilt.android.testing.HiltAndroidRule
+import dagger.hilt.android.testing.HiltAndroidTest
+import io.github.landwarderer.futon.SampleData
+import io.github.landwarderer.futon.core.cache.MemoryContentCache
+import io.github.landwarderer.futon.core.cache.SafeDeferred
+import io.github.landwarderer.futon.core.model.TestMangaSource
+import io.github.landwarderer.futon.core.nav.ReaderIntent
+import io.github.landwarderer.futon.core.network.MangaHttpClient
+import io.github.landwarderer.futon.core.parser.MangaDataRepository
+import io.github.landwarderer.futon.core.prefs.AppSettings
+import io.github.landwarderer.futon.core.prefs.ReaderMode
+import io.github.landwarderer.futon.reader.ui.ReaderActivity
+import io.github.landwarderer.futon.reader.ui.ReaderState
+import io.github.landwarderer.futon.reader.ui.ReaderViewModel
+import kotlinx.coroutines.CompletableDeferred
+import kotlinx.coroutines.delay
+import kotlinx.coroutines.flow.first
+import kotlinx.coroutines.runBlocking
+import kotlinx.coroutines.withTimeout
+import okhttp3.OkHttpClient
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertFalse
+import org.junit.Before
+import org.junit.Rule
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.koitharu.kotatsu.parsers.model.ContentRating
+import org.koitharu.kotatsu.parsers.model.MangaChapter
+import org.koitharu.kotatsu.parsers.model.MangaPage
+import org.koitharu.kotatsu.parsers.model.MangaParserSource
+import java.io.ByteArrayOutputStream
+import java.io.Closeable
+import java.io.IOException
+import java.net.InetAddress
+import java.net.ServerSocket
+import java.util.concurrent.ConcurrentHashMap
+import javax.inject.Inject
+import kotlin.concurrent.thread
+
+/** Delay adjacent metadata until the old-page callback has run, then observe real image requests. */
+@HiltAndroidTest
+@RunWith(AndroidJUnit4::class)
+class CrossChapterPrefetchReaderTest {
+    @get:Rule val hiltRule = HiltAndroidRule(this)
+    @Inject lateinit var cache: MemoryContentCache
+    @Inject lateinit var data: MangaDataRepository
+    // Production initializes networking off the main thread; HiltTestApplication needs the same warm-up.
+    @Inject @MangaHttpClient lateinit var httpClient: OkHttpClient
+    private val context get() = InstrumentationRegistry.getInstrumentation().targetContext
+
+    @Before
+    fun setUp() {
+        try {
+            WorkManager.getInstance(context)
+        } catch (_: IllegalStateException) {
+            WorkManager.initialize(context, Configuration.Builder().build())
+        }
+        hiltRule.inject()
+    }
+
+    @Test
+    fun prefetchesAfterAsyncAppendInStandardAndWebtoonReaders() = runBlocking {
+        checkPrefetch(ReaderMode.STANDARD, enabled = true)
+        checkPrefetch(ReaderMode.WEBTOON, enabled = true)
+    }
+
+    @Test
+    fun disabledPreloadingStillAppendsMetadataWithoutPrefetchingImages() = runBlocking {
+        checkPrefetch(ReaderMode.STANDARD, enabled = false)
+    }
+
+    private suspend fun checkPrefetch(mode: ReaderMode, enabled: Boolean) {
+        cache.clear(TestMangaSource)
+        PreferenceManager.getDefaultSharedPreferences(context).edit()
+            .putString(AppSettings.KEY_PAGES_PRELOAD, if (enabled) "1" else "0")
+            .putBoolean(AppSettings.KEY_WEBTOON_PULL_GESTURE, false)
+            .commit()
+        PageServer().use { server ->
+            val id = -System.nanoTime()
+            val chapters = (29L..30L).map { chapterId ->
+                MangaChapter(
+                    id = chapterId, title = "Chapter $chapterId", number = chapterId.toFloat(), volume = 0,
+                    url = "https://example.invalid/$id/$chapterId", uploadDate = 0,
+                    scanlator = null, branch = null, source = TestMangaSource,
+                )
+            }
+            fun pages(chapter: Long) = (1..15).map { number ->
+                // MangaDex uses the default direct page URL resolver; all traffic stays on loopback.
+                MangaPage(chapter * 100 + number, "${server.url}/$chapter/$number", null, MangaParserSource.MANGADEX)
+            }
+            val nextPages = CompletableDeferred<Result<List<MangaPage>>>()
+            cache.putPages(TestMangaSource, chapters[0].url, SafeDeferred(CompletableDeferred(Result.success(pages(29)))))
+            cache.putPages(TestMangaSource, chapters[1].url, SafeDeferred(nextPages))
+            val manga = SampleData.mangaDetails.copy(
+                id = id, title = "Cross-chapter prefetch test", source = TestMangaSource,
+                url = "https://example.invalid/$id", publicUrl = "https://example.invalid/$id",
+                chapters = chapters, tags = emptySet(), contentRating = ContentRating.SAFE,
+                description = null, coverUrl = "", largeCoverUrl = null,
+            )
+            cache.putDetails(TestMangaSource, manga.url, SafeDeferred(CompletableDeferred(Result.success(manga))))
+            data.saveReaderMode(manga, mode)
+            val intent = ReaderIntent.Builder(context).mangaId(id).state(ReaderState(29, 14, 0)).build().intent
+            ActivityScenario.launch<ReaderActivity>(intent).use { scenario ->
+                lateinit var model: ReaderViewModel
+                scenario.onActivity { model = ViewModelProvider(it)[ReaderViewModel::class.java] }
+                withTimeout(30_000) { model.content.first { it.pages.isNotEmpty() } }
+                awaitRequest(server, "/29/15", mode)
+                assertFalse(model.content.value.pages.any { it.chapterId == 30L })
+                nextPages.complete(Result.success(pages(30)))
+                withTimeout(30_000) { model.content.first { content -> content.pages.any { it.chapterId == 30L } } }
+                if (enabled) {
+                    // PageLoader retains its existing six-item queue cap after the reader offers up to ten.
+                    awaitRequest(server, "/30/6", mode)
+                } else {
+                    delay(1500)
+                    assertFalse("/30/6" in server.requests)
+                }
+                assertEquals(29L, model.readingState.value?.chapterId)
+            }
+        }
+    }
+
+    private suspend fun awaitRequest(server: PageServer, path: String, mode: ReaderMode) {
+        try {
+            withTimeout(30_000) { while (path !in server.requests) delay(50) }
+        } catch (e: kotlinx.coroutines.TimeoutCancellationException) {
+            throw AssertionError("$mode expected $path; received ${server.requests}", e)
+        }
+    }
+
+    private class PageServer : Closeable {
+        private val socket = ServerSocket(0, 16, InetAddress.getByName("127.0.0.1"))
+        val url = "http://127.0.0.1:${socket.localPort}"
+        val requests = ConcurrentHashMap.newKeySet<String>()
+        private val png = ByteArrayOutputStream().also { stream ->
+            val bitmap = Bitmap.createBitmap(600, 900, Bitmap.Config.ARGB_8888)
+            bitmap.eraseColor(Color.rgb(205, 225, 235))
+            bitmap.compress(Bitmap.CompressFormat.PNG, 100, stream)
+            bitmap.recycle()
+        }.toByteArray()
+        private val worker = thread(name = "reader-test-pages", isDaemon = true) {
+            while (!socket.isClosed) {
+                try {
+                    socket.accept().use { client ->
+                        val reader = client.getInputStream().bufferedReader()
+                        val path = reader.readLine()?.split(' ')?.getOrNull(1) ?: return@use
+                        while (!reader.readLine().isNullOrEmpty()) Unit
+                        val output = client.getOutputStream()
+                        output.write("HTTP/1.1 200 OK\r\nContent-Type: image/png\r\nContent-Length: ${png.size}\r\nConnection: close\r\n\r\n".toByteArray())
+                        output.write(png)
+                        output.flush()
+                        requests += path
+                    }
+                } catch (e: IOException) {
+                    if (!socket.isClosed) throw e
+                }
+            }
+        }
+
+        override fun close() {
+            socket.close()
+            worker.join(1000)
+        }
+    }
+}

app/src/main/kotlin/io/github/landwarderer/futon/reader/domain/ChapterPrefetch.kt

@@ -0,0 +1,17 @@
+package io.github.landwarderer.futon.reader.domain
+
+import io.github.landwarderer.futon.reader.ui.pager.ReaderPage
+
+/** Select by chapter identity: appending pages can also trim the start of the snapshot. */
+internal fun List<ReaderPage>.nextChapterPrefetchPages(currentChapterId: Long, limit: Int): List<ReaderPage> {
+    if (limit <= 0) return emptyList()
+    val boundary = indexOfLast { it.chapterId == currentChapterId }
+    if (boundary < 0) return emptyList()
+    val first = boundary + 1
+    val nextChapterId = getOrNull(first)?.chapterId ?: return emptyList()
+    var end = first
+    while (end < size && end - first < limit && this[end].chapterId == nextChapterId) {
+        end++
+    }
+    return subList(first, end)
+}

app/src/main/kotlin/io/github/landwarderer/futon/reader/domain/ChaptersLoader.kt

@@ -1,6 +1,6 @@
 package io.github.landwarderer.futon.reader.domain
 
-import android.util.LongSparseArray
+import androidx.collection.LongSparseArray
 import androidx.annotation.CheckResult
 import dagger.hilt.android.scopes.ViewModelScoped
 import kotlinx.coroutines.sync.Mutex
@@ -39,8 +39,11 @@ class ChaptersLoader @Inject constructor(
 		val index = if (isNext) chapters.indexOfFirst(predicate) else chapters.indexOfLast(predicate)
 		if (index == -1) return false
 		val newChapter = chapters.getOrNull(if (isNext) index + 1 else index - 1) ?: return false
+		if (mutex.withLock { newChapter.id in chapterPages }) return false
 		val newPages = loadChapter(newChapter.id)
-		mutex.withLock {
+		if (newPages.isEmpty()) return false
+		return mutex.withLock {
+			if (newChapter.id in chapterPages) return@withLock false
 			if (chapterPages.chaptersSize > 1) {
 				// trim pages
 				if (chapterPages.size > PAGES_TRIM_THRESHOLD) {
@@ -57,7 +60,6 @@ class ChaptersLoader @Inject constructor(
 				chapterPages.addFirst(newChapter.id, newPages)
 			}
 		}
-		return true
 	}
 
 	suspend fun loadSingleChapter(chapterId: Long): Boolean {

app/src/main/kotlin/io/github/landwarderer/futon/reader/ui/ReaderViewModel.kt

@@ -42,6 +42,7 @@ import io.github.landwarderer.futon.local.domain.model.LocalManga
 import io.github.landwarderer.futon.reader.domain.ChaptersLoader
 import io.github.landwarderer.futon.reader.domain.DetectReaderModeUseCase
 import io.github.landwarderer.futon.reader.domain.PageLoader
+import io.github.landwarderer.futon.reader.domain.nextChapterPrefetchPages
 import io.github.landwarderer.futon.reader.ui.config.ReaderSettings
 import io.github.landwarderer.futon.reader.ui.pager.ReaderUiState
 import io.github.landwarderer.futon.scrobbling.discord.ui.DiscordRpc
@@ -546,8 +547,15 @@ class ReaderViewModel @Inject constructor(
         val prevJob = loadingJob
         loadingJob = launchLoadingJob(Dispatchers.IO) {
             prevJob?.join()
-            chaptersLoader.loadPrevNextChapter(mangaDetails.requireValue(), currentId, isNext)
-            content.value = ReaderContent(chaptersLoader.snapshot(), null)
+            val appended = chaptersLoader.loadPrevNextChapter(mangaDetails.requireValue(), currentId, isNext)
+            val pages = chaptersLoader.snapshot()
+            content.value = ReaderContent(pages, null)
+            if (appended && isNext && pageLoader.isPrefetchApplicable()) {
+                val prefetchPages = pages.nextChapterPrefetchPages(currentId, PREFETCH_LIMIT)
+                if (prefetchPages.isNotEmpty()) {
+                    pageLoader.prefetch(prefetchPages)
+                }
+            }
         }
     }
 

app/src/test/kotlin/io/github/landwarderer/futon/reader/domain/ChapterPrefetchTest.kt

@@ -0,0 +1,42 @@
+package io.github.landwarderer.futon.reader.domain
+
+import io.github.landwarderer.futon.core.model.TestMangaSource
+import io.github.landwarderer.futon.reader.ui.pager.ReaderPage
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertTrue
+import org.junit.Test
+
+class ChapterPrefetchTest {
+    @Test
+    fun `prefetch starts at successor and respects the limit`() {
+        val pages = chapter(29, 15) + chapter(30, 20)
+        assertEquals(chapter(30, 20).take(10), pages.nextChapterPrefetchPages(29, 10))
+    }
+
+    @Test
+    fun `short successor does not spill into another chapter`() {
+        val pages = chapter(29, 15) + chapter(30, 2) + chapter(31, 15)
+        assertEquals(chapter(30, 2), pages.nextChapterPrefetchPages(29, 10))
+    }
+
+    @Test
+    fun `trimming older chapters does not change selection`() {
+        val pages = chapter(28, 100) + chapter(29, 30) + chapter(30, 12)
+        assertEquals(
+            pages.nextChapterPrefetchPages(29, 10),
+            pages.drop(100).nextChapterPrefetchPages(29, 10),
+        )
+    }
+
+    @Test
+    fun `missing boundary successor and empty snapshots are harmless`() {
+        assertTrue(emptyList<ReaderPage>().nextChapterPrefetchPages(29, 10).isEmpty())
+        assertTrue(chapter(29, 15).nextChapterPrefetchPages(29, 10).isEmpty())
+        assertTrue(chapter(30, 15).nextChapterPrefetchPages(29, 10).isEmpty())
+        assertTrue((chapter(29, 15) + chapter(30, 15)).nextChapterPrefetchPages(29, 0).isEmpty())
+    }
+
+    private fun chapter(id: Long, count: Int) = List(count) { index ->
+        ReaderPage(id * 1000 + index, "https://example.org/$id/$index", null, id, index, TestMangaSource)
+    }
+}

app/src/test/kotlin/io/github/landwarderer/futon/reader/domain/ChaptersLoaderTest.kt

@@ -0,0 +1,96 @@
+package io.github.landwarderer.futon.reader.domain
+
+import io.github.landwarderer.futon.core.model.TestMangaSource
+import io.github.landwarderer.futon.core.parser.MangaRepository
+import io.github.landwarderer.futon.details.data.MangaDetails
+import kotlinx.coroutines.test.runTest
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertFalse
+import org.junit.Assert.assertTrue
+import org.junit.Test
+import org.mockito.Mockito.mock
+import org.mockito.Mockito.times
+import org.mockito.Mockito.verify
+import org.mockito.Mockito.`when`
+import org.koitharu.kotatsu.parsers.model.MangaChapter
+import org.koitharu.kotatsu.parsers.model.MangaPage
+
+class ChaptersLoaderTest {
+    @Test
+    fun `repeated forward and backward requests load only once`() = runTest {
+        val fixture = fixture(3, 10)
+        fixture.loader.loadSingleChapter(2)
+        assertTrue(fixture.loader.loadPrevNextChapter(fixture.details, 2, true))
+        assertFalse(fixture.loader.loadPrevNextChapter(fixture.details, 2, true))
+        assertTrue(fixture.loader.loadPrevNextChapter(fixture.details, 2, false))
+        assertFalse(fixture.loader.loadPrevNextChapter(fixture.details, 2, false))
+        assertEquals(listOf(1L, 2L, 3L), fixture.loader.snapshot().map { it.chapterId }.distinct())
+        fixture.chapters.forEach { verify(fixture.repository, times(1)).getPages(it) }
+    }
+
+    @Test
+    fun `forward append trims old pages without losing boundary`() = runTest {
+        val fixture = fixture(3, 70)
+        fixture.loader.loadSingleChapter(1)
+        fixture.loader.loadPrevNextChapter(fixture.details, 1, true)
+        assertTrue(fixture.loader.loadPrevNextChapter(fixture.details, 2, true))
+        assertEquals(listOf(2L, 3L), fixture.loader.snapshot().map { it.chapterId }.distinct())
+        assertEquals(10, fixture.loader.snapshot().nextChapterPrefetchPages(2, 10).size)
+    }
+
+    @Test
+    fun `missing and empty successors preserve current pages`() = runTest {
+        val fixture = fixture(2, 10)
+        fixture.loader.loadSingleChapter(1)
+        val original = fixture.loader.snapshot()
+        `when`(fixture.repository.getPages(fixture.chapters[1])).thenReturn(emptyList())
+        assertFalse(fixture.loader.loadPrevNextChapter(fixture.details, 1, true))
+        assertFalse(fixture.loader.loadPrevNextChapter(fixture.details, 2, true))
+        assertEquals(original, fixture.loader.snapshot())
+    }
+
+    @Test
+    fun `failed successor leaves current pages usable`() = runTest {
+        val fixture = fixture(2, 10)
+        fixture.loader.loadSingleChapter(1)
+        val original = fixture.loader.snapshot()
+        `when`(fixture.repository.getPages(fixture.chapters[1])).thenThrow(IllegalStateException("offline"))
+        try {
+            fixture.loader.loadPrevNextChapter(fixture.details, 1, true)
+            throw AssertionError("Expected failure")
+        } catch (_: IllegalStateException) {
+            assertEquals(original, fixture.loader.snapshot())
+        }
+    }
+
+    private suspend fun fixture(count: Int, pagesCount: Int): Fixture {
+        val repository = mock(MangaRepository::class.java)
+        val factory = mock(MangaRepository.Factory::class.java)
+        `when`(factory.create(TestMangaSource)).thenReturn(repository)
+        val chapters = List(count) { index ->
+            MangaChapter(
+                id = index + 1L, title = "Chapter ${index + 1}", number = index + 1f, volume = 0,
+                url = "https://example.org/${index + 1}", uploadDate = 0L,
+                scanlator = null, branch = null, source = TestMangaSource,
+            )
+        }
+        for (chapter in chapters) {
+            val pages = List(pagesCount) { index ->
+                MangaPage(chapter.id * 1000 + index, "https://example.org/$index", null, TestMangaSource)
+            }
+            `when`(repository.getPages(chapter)).thenReturn(pages)
+        }
+        val details = mock(MangaDetails::class.java)
+        `when`(details.allChapters).thenReturn(chapters)
+        val loader = ChaptersLoader(factory)
+        loader.init(details)
+        return Fixture(loader, repository, details, chapters)
+    }
+
+    private data class Fixture(
+        val loader: ChaptersLoader,
+        val repository: MangaRepository,
+        val details: MangaDetails,
+        val chapters: List<MangaChapter>,
+    )
+}