Fix Novelight chapter list: recover dropped chapters, refresh stale cache, correct reader labels

app/src/main/java/io/aatricks/easyreader/data/repository/ChapterListCache.kt

@@ -22,14 +22,20 @@ class ChapterListCache @Inject constructor(
 ) {
     companion object {
         const val FRESH_WINDOW_MS = 6L * 60 * 60 * 1000 // 6 hours
+
+        // Bump when a source's chapter-list parsing changes in a way that should invalidate every
+        // already-cached list. Entries written before versioning default to 0, so a bump forces a
+        // one-time refetch on next open (e.g. the Novelight paging fix that recovers dropped pages).
+        const val CACHE_VERSION = 1
     }
 
     @Serializable
     data class Entry(
         val chapters: List<ChapterInfo>,
         val fetchedAt: Long,
         val baseNovelUrl: String,
-        val sourceName: String
+        val sourceName: String,
+        val version: Int = 0
     )
 
     fun load(baseNovelUrl: String, sourceName: String): Entry? {
@@ -50,7 +56,8 @@ class ChapterListCache @Inject constructor(
                 chapters = chapters,
                 fetchedAt = System.currentTimeMillis(),
                 baseNovelUrl = baseNovelUrl,
-                sourceName = sourceName
+                sourceName = sourceName,
+                version = CACHE_VERSION
             )
             val temp = File.createTempFile("${file.name}.", ".tmp", file.parentFile)
             try {
@@ -75,7 +82,7 @@ class ChapterListCache @Inject constructor(
     }
 
     fun isFresh(entry: Entry): Boolean =
-        (System.currentTimeMillis() - entry.fetchedAt) < FRESH_WINDOW_MS
+        entry.version == CACHE_VERSION && (System.currentTimeMillis() - entry.fetchedAt) < FRESH_WINDOW_MS
 
     private fun fileFor(baseNovelUrl: String, sourceName: String): File =
         File(cacheDir, "${CacheKeyUtils.keyFor("$sourceName|$baseNovelUrl")}.json")

app/src/main/java/io/aatricks/easyreader/data/repository/source/NovelightSource.kt

@@ -3,14 +3,17 @@ package io.aatricks.easyreader.data.repository.source
 import io.aatricks.easyreader.data.local.PreferencesManager
 import io.aatricks.easyreader.data.model.ChapterInfo
 import io.aatricks.easyreader.data.model.ExploreItem
+import io.aatricks.easyreader.util.HttpRetry
 import kotlinx.coroutines.async
 import kotlinx.coroutines.awaitAll
 import kotlinx.coroutines.coroutineScope
+import kotlinx.coroutines.delay
 import kotlinx.coroutines.sync.Semaphore
 import kotlinx.coroutines.sync.withPermit
 import org.json.JSONObject
 import org.jsoup.Jsoup
 import org.jsoup.nodes.Document
+import java.io.IOException
 import java.net.URLEncoder
 import javax.inject.Inject
 
@@ -34,9 +37,13 @@ class NovelightSource @Inject constructor(
     override val version = "1.0.0"
 
     companion object {
-        private const val CHAPTERS_PER_PAGE = 50.0
+        private const val CHAPTERS_PER_PAGE = 50
         private const val MAX_CHAPTER_PAGES = 200
-        private const val CHAPTER_PAGE_CONCURRENCY = 3
+        private const val CHAPTER_PAGE_CONCURRENCY = 2
+        private const val MAX_PAGE_RETRIES = 3
+        private const val HTTP_FORBIDDEN = 403
+        private const val HTTP_REDIRECT_MIN = 300
+        private const val HTTP_REDIRECT_MAX = 399
         private val BOOK_ID_REGEX = Regex("""BOOK_ID\s*=\s*["'](\d+)["']""")
         private val CHAPTER_TITLE_REGEX =
             Regex("""^\s*(\d+(?:\.\d+)?)\s*chapter\b\s*[-:–—]?\s*(.*)$""", RegexOption.IGNORE_CASE)
@@ -161,60 +168,172 @@ class NovelightSource @Inject constructor(
 
     // --- Chapter list paging -----------------------------------------------------------------
 
-    private suspend fun loadAllChapters(bookId: String): List<ChapterInfo> = coroutineScope {
-        val firstPage = parseChapterPaginationHtml(fetchChapterPageHtml(bookId, 1))
-        if (firstPage.isEmpty()) return@coroutineScope emptyList()
+    /**
+     * The pagination endpoint exposes no page count, and out-of-range pages *clamp* to the last
+     * page's content instead of returning empty. So we estimate the page count from page 1 (the
+     * newest, highest-numbered chapters, ~[CHAPTERS_PER_PAGE] per page), fetch that range with
+     * bounded concurrency, then extend until a page adds no new chapters (the clamp) or is short
+     * (the real last page) — never relying on an empty page or [MAX_CHAPTER_PAGES] to stop. That
+     * termination is what keeps a large book from firing ~[MAX_CHAPTER_PAGES] redundant requests
+     * and tripping the site's rate limiter. A page inside the known range that never recovers from a
+     * transient failure aborts the whole load rather than silently leaving a hole; a gated
+     * (403/redirect) page degrades. See [fetchChapterPage].
+     */
+    internal suspend fun loadAllChapters(bookId: String): List<ChapterInfo> =
+        assembleChapters { page -> fetchChapterPage(bookId, page) }
 
-        // The pagination JSON exposes no page count, but page 1 holds the newest (highest-numbered)
-        // chapters, so the max chapter number estimates the page count. We parallel-fetch up to the
-        // estimate, then sequentially extend in case numbering is sparse and undercounts.
-        val maxNumber = firstPage.maxOfOrNull { it.number ?: 0.0 } ?: 0.0
-        val estimatedPages = ((maxNumber / CHAPTERS_PER_PAGE).toInt() + 1).coerceIn(1, MAX_CHAPTER_PAGES)
+    /** Paging orchestration, decoupled from page-fetching so the termination/gap/degrade rules are unit-testable. */
+    internal suspend fun assembleChapters(fetchPage: suspend (Int) -> PageOutcome): List<ChapterInfo> {
+        val acc = ChapterAccumulator()
+        val firstChapters = (fetchPage(1) as? PageOutcome.Loaded)?.chapters
+        if (firstChapters.isNullOrEmpty()) return emptyList()
+        acc.add(firstChapters)
 
-        val all = ArrayList(firstPage)
-        if (estimatedPages > 1) {
+        val estimatedPages = estimatePageCount(firstChapters)
+        collectKnownRange(acc, estimatedPages, fetchPage)
+        extendBeyondEstimate(acc, estimatedPages + 1, fetchPage)
+        return acc.sortedByNumber()
+    }
+
+    private fun estimatePageCount(firstChapters: List<ChapterInfo>): Int {
+        // Page 1 holds the newest, highest-numbered chapters (~CHAPTERS_PER_PAGE per page), so the max
+        // number estimates the page count. It can undercount slightly (sub-chapters), hence the extend.
+        val maxNumber = firstChapters.maxOfOrNull { it.number ?: 0.0 } ?: 0.0
+        return ((maxNumber / CHAPTERS_PER_PAGE).toInt() + 1).coerceIn(1, MAX_CHAPTER_PAGES)
+    }
+
+    /**
+     * Fetch the known-dense range (pages 2..[estimatedPages]) in parallel. Each page retries its own
+     * transient failures; a gated (premium) page is skipped, but a page that never recovers aborts the
+     * whole load rather than silently leaving a hole in the middle of the chapter list.
+     */
+    private suspend fun collectKnownRange(
+        acc: ChapterAccumulator,
+        estimatedPages: Int,
+        fetchPage: suspend (Int) -> PageOutcome
+    ) {
+        if (estimatedPages <= 1) return
+        coroutineScope {
             val semaphore = Semaphore(CHAPTER_PAGE_CONCURRENCY)
-            val rest = (2..estimatedPages).map { page ->
-                async {
-                    semaphore.withPermit {
-                        runCatching { parseChapterPaginationHtml(fetchChapterPageHtml(bookId, page)) }
-                            .getOrDefault(emptyList())
-                    }
+            val outcomes = (2..estimatedPages).map { page ->
+                async { semaphore.withPermit { page to fetchPage(page) } }
+            }.awaitAll().sortedBy { it.first }
+            for ((page, outcome) in outcomes) {
+                when (outcome) {
+                    is PageOutcome.Loaded -> acc.add(outcome.chapters)
+                    PageOutcome.Gated -> Unit
+                    PageOutcome.Failed ->
+                        throw IOException("Novelight chapter page $page failed after retries")
                 }
-            }.awaitAll().flatten()
-            all.addAll(rest)
+            }
         }
+    }
 
-        var page = estimatedPages + 1
-        while (page <= MAX_CHAPTER_PAGES) {
-            val more = runCatching { parseChapterPaginationHtml(fetchChapterPageHtml(bookId, page)) }
-                .getOrDefault(emptyList())
-            if (more.isEmpty()) break
-            all.addAll(more)
+    /**
+     * Extend past the (sometimes-undercounting) estimate, stopping at the real last page — detected by
+     * a short page or one that only repeats already-seen URLs (the endpoint's out-of-range clamp) — so
+     * a large book never fires ~[MAX_CHAPTER_PAGES] redundant requests.
+     */
+    private suspend fun extendBeyondEstimate(
+        acc: ChapterAccumulator,
+        startPage: Int,
+        fetchPage: suspend (Int) -> PageOutcome
+    ) {
+        var page = startPage
+        var advancing = true
+        while (advancing && page <= MAX_CHAPTER_PAGES) {
+            val chapters = (fetchPage(page) as? PageOutcome.Loaded)?.chapters
+            val added = chapters?.let { acc.add(it) } ?: 0
+            advancing = chapters != null && added > 0 && chapters.size >= CHAPTERS_PER_PAGE
             page++
         }
+    }
+
+    /** Accumulates chapters across pages, de-duping by URL (the clamp repeats a page's URLs). */
+    private class ChapterAccumulator {
+        private val seen = HashSet<String>()
+        private val all = ArrayList<ChapterInfo>()
+
+        fun add(chapters: List<ChapterInfo>): Int {
+            var added = 0
+            for (chapter in chapters) if (seen.add(chapter.url)) {
+                all.add(chapter)
+                added++
+            }
+            return added
+        }
 
-        all.distinctBy { it.url }.sortedBy { it.number ?: Double.MAX_VALUE }
+        fun sortedByNumber(): List<ChapterInfo> = all.sortedBy { it.number ?: Double.MAX_VALUE }
     }
 
-    private fun fetchChapterPageHtml(bookId: String, page: Int): String =
-        jsonField(ajaxGet("$baseUrl/book/ajax/chapter-pagination?book_id=$bookId&page=$page"), "html")
+    internal sealed interface PageOutcome {
+        data class Loaded(val chapters: List<ChapterInfo>) : PageOutcome
+        object Gated : PageOutcome
+        object Failed : PageOutcome
+    }
+
+    /** Fetch one chapter-pagination page, retrying transient failures (429/5xx/timeout) with backoff. */
+    internal suspend fun fetchChapterPage(bookId: String, page: Int): PageOutcome {
+        val url = "$baseUrl/book/ajax/chapter-pagination?book_id=$bookId&page=$page"
+        var attempt = 0
+        var outcome: PageOutcome? = null
+        while (outcome == null) {
+            outcome = when (val result = ajaxFetch(url)) {
+                is FetchResult.Body -> PageOutcome.Loaded(parseChapterPaginationHtml(jsonField(result.text, "html")))
+                FetchResult.Gated -> PageOutcome.Gated
+                is FetchResult.Retryable ->
+                    if (attempt >= MAX_PAGE_RETRIES) {
+                        PageOutcome.Failed
+                    } else {
+                        delay(HttpRetry.nextRetryDelayMs(result.retryAfterMs, url, attempt))
+                        attempt++
+                        null
+                    }
+            }
+        }
+        return outcome
+    }
 
     // --- HTTP --------------------------------------------------------------------------------
 
     private fun jsonField(body: String, field: String): String =
-        runCatching { JSONObject(body).optString(field) }.getOrDefault("")
+        runCatching { JSONObject(body).optString(field) }.getOrNull().orEmpty()
 
-    private fun ajaxGet(url: String): String {
+    private sealed interface FetchResult {
+        data class Body(val text: String) : FetchResult
+        object Gated : FetchResult
+        data class Retryable(val retryAfterMs: Long?, val cause: Exception? = null) : FetchResult
+    }
+
+    /**
+     * One raw XHR GET. 2xx → [FetchResult.Body]; 403 or a (non-followed) redirect → [FetchResult.Gated]
+     * (premium content); 408/429/5xx or a network error → [FetchResult.Retryable]; other 4xx → gated.
+     */
+    private fun ajaxFetch(url: String): FetchResult {
         val request = okhttp3.Request.Builder()
             .url(url)
             .header("User-Agent", userAgent)
             .header("Referer", baseUrl)
             .header("X-Requested-With", "XMLHttpRequest")
             .build()
-        okHttpClient.newCall(request).execute().use { response ->
-            if (!response.isSuccessful) throw java.io.IOException("Unexpected code $response")
-            return response.body.string()
+        return try {
+            okHttpClient.newCall(request).execute().use { response ->
+                when {
+                    response.isSuccessful -> FetchResult.Body(response.body.string())
+                    response.code == HTTP_FORBIDDEN || response.code in HTTP_REDIRECT_MIN..HTTP_REDIRECT_MAX ->
+                        FetchResult.Gated
+                    HttpRetry.shouldRetryResponseCode(response.code) ->
+                        FetchResult.Retryable(HttpRetry.parseRetryAfterMs(response.header("Retry-After")))
+                    else -> FetchResult.Gated
+                }
+            }
+        } catch (e: IOException) {
+            FetchResult.Retryable(retryAfterMs = null, cause = e)
         }
     }
+
+    private fun ajaxGet(url: String): String = when (val result = ajaxFetch(url)) {
+        is FetchResult.Body -> result.text
+        else -> throw IOException("Unexpected response for $url")
+    }
 }

app/src/main/java/io/aatricks/easyreader/ui/viewmodel/ReaderViewModel.kt

@@ -13,6 +13,7 @@ import io.aatricks.easyreader.data.repository.ImageDimensionCacheRepository
 import io.aatricks.easyreader.data.repository.LibraryRepository
 import io.aatricks.easyreader.ui.theme.AccentTheme
 import io.aatricks.easyreader.util.normalizeChapterList
+import io.aatricks.easyreader.util.resolveChapterLabelFromList
 import io.aatricks.easyreader.util.TextUtils
 import io.aatricks.easyreader.util.UrlSecurity
 import io.aatricks.easyreader.ui.viewmodel.ReaderProgressController.Companion.PAGED_POSITION_ITEM_SIZE_PX
@@ -603,6 +604,11 @@ class ReaderViewModel @Inject constructor(
             }
         }
 
+        // Prefer the chapter list's label when the content/URL-derived one carries the wrong number
+        // (Novelight reading URLs are /book/chapter/{id}, so the id leaks in as the chapter number).
+        val resolvedChapterTitle =
+            resolveChapterLabelFromList(content.url, chapterTitle, currentFullList) ?: chapterTitle
+
         updateState {
             it.copy(
                 content = content,
@@ -621,7 +627,7 @@ class ReaderViewModel @Inject constructor(
                 targetScrollPosition = initialPosition.targetScrollPosition,
                 hasReachedQuarterScreen = fromBottom || initialPosition.scrollProgress >= 25,
                 novelName = novelName,
-                chapterTitle = chapterTitle,
+                chapterTitle = resolvedChapterTitle,
                 baseTitle = baseTitle,
                 baseNovelUrl = libraryItem?.baseNovelUrl ?: "",
                 sourceName = libraryItem?.sourceName ?: "",
@@ -1354,11 +1360,16 @@ class ReaderViewModel @Inject constructor(
     }
 
     private suspend fun applyFullChapterList(normalizedChapters: List<ChapterInfo>) {
-        updateState {
-            it.copy(
+        updateState { state ->
+            // Now that the authoritative list is loaded, correct the header label if the current
+            // chapter's URL-derived number was wrong (e.g. Novelight's opaque /book/chapter/{id}).
+            val resolvedTitle = state.content?.url
+                ?.let { resolveChapterLabelFromList(it, state.chapterTitle, normalizedChapters) }
+            state.copy(
                 fullChapterList = normalizedChapters,
                 isChaptersLoading = false,
-                isFullChapterListLoaded = true
+                isFullChapterListLoaded = true,
+                chapterTitle = resolvedTitle ?: state.chapterTitle
             )
         }
         updateNavigationUrls()

app/src/main/java/io/aatricks/easyreader/util/ChapterListNormalizer.kt

@@ -20,6 +20,27 @@ internal fun normalizeChapterList(chapters: List<ChapterInfo>): List<ChapterInfo
         .toList()
 }
 
+/**
+ * The authoritative chapter label is the one parsed from the source's chapter list
+ * (e.g. "Chapter 102 - ..."), not one guessed from the reading URL: Novelight's
+ * `/book/chapter/{id}` URLs carry an opaque id, so URL-derived numbering shows the id instead of
+ * the chapter number. Returns the matching chapter's list title only when that list entry has a
+ * definite number that differs from [currentLabel]'s — a no-op for sources whose URL already yields
+ * the right number, and null when the url isn't in the (possibly not-yet-loaded) list so the caller
+ * keeps its own label.
+ */
+internal fun resolveChapterLabelFromList(
+    url: String,
+    currentLabel: String,
+    chapters: List<ChapterInfo>
+): String? {
+    val target = url.trim()
+    val match = if (target.isBlank()) null else chapters.firstOrNull { it.url == target }
+    val listNumber = match?.let { it.number ?: TextUtils.extractChapterNumber(it.title) }
+    val differs = listNumber != null && TextUtils.extractChapterNumber(currentLabel) != listNumber
+    return match?.title?.trim()?.takeIf { it.isNotBlank() && differs }
+}
+
 internal fun normalizeExploreItemDetails(item: ExploreItem): ExploreItem {
     val normalizedChapters = normalizeChapterList(item.chapters)
     val normalizedReadingUrl = item.readingUrl?.trim().takeUnless { it.isNullOrBlank() }

app/src/test/java/io/aatricks/easyreader/data/repository/ChapterListCacheTest.kt

@@ -4,7 +4,9 @@ import io.aatricks.easyreader.data.model.ChapterInfo
 import kotlinx.serialization.json.Json
 import org.junit.After
 import org.junit.Assert.assertEquals
+import org.junit.Assert.assertFalse
 import org.junit.Assert.assertNotNull
+import org.junit.Assert.assertTrue
 import org.junit.Before
 import org.junit.Test
 import java.io.File
@@ -63,4 +65,26 @@ class ChapterListCacheTest {
         assertNotNull(loaded2)
         assertEquals(chapters2, loaded2?.chapters)
     }
+
+    @Test
+    fun `entry cached before versioning is not fresh so it refetches once`() {
+        // Old entries (written before the version field existed) deserialize with version 0.
+        val legacy = ChapterListCache.Entry(
+            chapters = listOf(ChapterInfo("Ch 1", "url1")),
+            fetchedAt = System.currentTimeMillis(),
+            baseNovelUrl = "novel1",
+            sourceName = "Source1",
+            version = 0
+        )
+        assertFalse("a stale-version entry must not be treated as fresh", cache.isFresh(legacy))
+    }
+
+    @Test
+    fun `freshly saved entry is current version and fresh`() {
+        cache.save("novel1", "Source1", listOf(ChapterInfo("Ch 1", "url1")))
+        val loaded = cache.load("novel1", "Source1")
+        assertNotNull(loaded)
+        assertEquals(ChapterListCache.CACHE_VERSION, loaded?.version)
+        assertTrue(cache.isFresh(loaded!!))
+    }
 }