Fix devotional download priority and loading/Retry UI by yukuku · Pull Request #308 · yukuku/androidbible

yukuku · 2026-10-02T07:58:21Z

A selected devotional reading can wait behind a slow active prefetch request, and network/HTTP errors or an NG response leave the screen without actionable download status. The reader now gives the selected source/date a dedicated foreground coroutine, shows queued/downloading/failure/unavailable states, and offers Retry while retaining offline readings.

Investigation

Traced the cache, 15-day prefetch (3 for RH), queue, synchronous OkHttp calls, per-phase timeouts, transactional persistence, success-only events, and activity lifecycle.
The single-worker bottleneck and log-only failures already exist before the April executor refactor (758793f5, Refactor DevotionDownloader to use ExecutorService instead of Thread #150). The May Kotlin conversion (5c14af08, REM-16: convert 6 Java files to Kotlin #187) preserves those behaviors. Git history does not establish the revamp as the cause of a new regression.
Queue equality only deduplicates some article types; Morning & Evening, Renungan Pagi, and My Utmost do not implement value equality, and active requests are outside the queue for every source.
Completion signals have no replay and the reader does not reconcile cache/status on start. Morning & Evening cache loading also incorrectly forces readyToUse=true for unavailable rows.
The reported fast API checks from a cloud network do not establish latency on the phone. Deterministic tests reproduce the application-side blocking and recovery failures without the production server.

Result

One foreground coroutine and one prefetch coroutine on a two-slot IO dispatcher, with a shared source/date registry for queued and active work. Selected queued work is promoted; an already active matching prefetch request is reused. Changing dates/sources cancels obsolete foreground OkHttp work before the latest foreground request starts.
A 30-second total call timeout bounds the complete HTTP transfer. Failures become terminal, retryable state; NG is separately unavailable. Retry revalidates the HTTP cache and bypasses the database, while deduplicating active work.
Cached readings remain visible during refresh and survive failed/unavailable refreshes. Cache insertion errors propagate and roll back rather than reporting false success. Every source respects stored readiness.
Replaying state plus reconciliation on start handles stopped/recreated screens. Saved source/date, current-selection filtering, and stable rendering prevent stale results from replacing content or disturbing scroll. Prefetch captures its source without retaining the activity.
The reader activity is Kotlin; API/database source identifiers and existing comments are preserved. Dependencies are explicit constructor arguments with production factories.
Loading and error/Retry states are centered in the viewport. The Material progress indicator and outlined Retry button use the activity language and reader colors. Loading text is Downloading... / Mengunduh..., and errors identify the devotion. Copy/Share require a rendered reading.

Validation

The full debug/release unit suites passed with 924 tests in each variant (0 failed, 0 skipped). The final layout and wording update passed ./gradlew assemblePlainDebug testPlainDebugUnitTest --tests '*Devotion*', covering all 39 devotional tests, including native screenshot centering and app-language checks.
Controlled-dispatcher/request tests: slow prefetch isolation, promotion, rapid selection/cancellation races, Retry, deduplication, source isolation, and shutdown.
Backend tests: scripted HTTP/network errors and NG, offline cache, refresh preservation, storage failure, and localhost stalled-body timeout/cancellation.
Robolectric reader and DAO tests: loading/failure/Retry, date/source changes, stopped/destroyed/recreated activities, language independent of device locale, reader colors, all-source cache readiness, and transactional rollback.
Native Robolectric screenshots capture queued, downloading, failure, unavailable, and reading screens in light and dark themes using a fixed local reading fixture. Run DevotionActivityTest to regenerate PNGs under Alkitab/build/snapshots/devotions/.
Automated tests make no requests to the production devotional API.

Same-repository PR targeting develop so the existing Android CI can build signed release APKs and publish PR previews. Do not merge.

github-actions · 2026-10-02T08:09:33Z

📱 Preview builds

Signed release builds of 4617af8 — version 5.0.0-dev.128 (24005690) — from this run.

Flavor	Application ID	APK
Alkitab	`yuku.alkitab`	Download (7.6 MB)
Quick Bible	`yuku.alkitab.kjv`	Download (7.5 MB)
Sabda Alkitab	`org.sabda.alkitab`	Download (7.6 MB)

Or open https://43b9bc72-alkitab-pr.yukuku.workers.dev on an Android device.

These share their application IDs and signature with the Play Store builds, so installing one replaces the corresponding installed app (data is kept).

This comment tracks the latest build for this PR; earlier builds keep their own URLs.

yukuku · 2026-10-06T03:41:31Z

 package yuku.alkitab.base.ac;

 import android.content.Intent;
+import android.content.res.ColorStateList;


rewrite Alkitab/src/main/java/yuku/alkitab/base/ac/DevotionActivity.java to kotlin, but do not remove existing comments.

Converted the activity to Kotlin and preserved all ten existing comments verbatim. Java-facing intent factories and enum helpers remain available. Kotlin reserves Enum.name, so the API/database identifier is sourceName; values such as me-en remain unchanged. Reader lifecycle, selection, language, and theme tests pass.

yukuku · 2026-10-06T03:42:40Z

+    override fun createRequest(key: DevotionDownloader.Key, refresh: Boolean): DevotionDownloader.Request {
+        val request = Request.Builder().url(url(key))
+        if (refresh) request.cacheControl(CacheControl.FORCE_NETWORK)
+        val call = client().newCall(request.build())
+        return object : DevotionDownloader.Request {
+            override fun cancel() = call.cancel()
+
+            override fun execute(): DevotionDownloader.State {
+                try {
+                    if (!refresh) {
+                        cached(key)?.let {
+                            return if (it.readyToUse) DevotionDownloader.State.READY else DevotionDownloader.State.UNAVAILABLE
+                        }
+                    }
+                    val output = call.execute().use { response ->
+                        if (!response.isSuccessful) throw IOException("Devotion HTTP ${response.code}")
+                        response.body.string()
+                    }
+                    if (call.isCanceled()) throw IOException("Devotion cancelled")
+                    val kind = DevotionActivity.DevotionKind.getByName(key.name)
+                        ?: throw IOException("Unknown devotion source ${key.name}")
+                    val article = kind.getArticle(key.date)
+                    article.fillIn(output)
+                    // An unsuccessful refresh must leave an offline reading intact.
+                    if (article.readyToUse || cached(key)?.readyToUse != true) store(article)
+                    if (!article.readyToUse) return DevotionDownloader.State.UNAVAILABLE
+                    AppEvents.emitDevotionDownloaded(key.name, key.date)
+                    return DevotionDownloader.State.READY
+                } catch (e: Exception) {
+                    AppLog.d("DevotionDownloader", "Download failed for ${key.name} ${key.date}", e)
+                    return DevotionDownloader.State.FAILED
+                }
+            }
+        }


the block of code is too terse, separate them into sections with light comments on top of the sections.

Separated the backend into storage lookup, HTTP retrieval, article parsing/cache updates, completion notification, and failure handling sections, each with a light comment. The comments describe the current behavior.

yukuku · 2026-10-06T03:43:07Z

+    private val foregroundExecutor: ExecutorService = Executors.newSingleThreadExecutor(),
+    private val prefetchExecutor: ExecutorService = Executors.newSingleThreadExecutor(),


Possible to use kotlin coroutine + Dispatchers ?

Yes. The scheduler now uses CoroutineScope(SupervisorJob()) and Dispatchers.IO.limitedParallelism(2), with at most one foreground and one prefetch coroutine. Requests remain deduplicated across both lanes. Obsolete foreground work cancels the underlying OkHttp call, since execute() is blocking; the next selection starts after that call releases its lane. Tests inject controlled CoroutineDispatchers, and a blocked-prefetch test exercises the same two-slot IO dispatcher.

yukuku · 2026-10-06T03:43:51Z

-    init {
-        executor.submit(::downloadLoop)
+    private class Work(val key: Key, val refresh: Boolean) {
+        var request: Request? = null


Do not use default parameters

Removed default constructor parameters from both DevotionDownloader and DevotionDownloadBackend. Constructors require explicit dependencies; create() factories supply production wiring, and tests pass their dependencies explicitly.

yukuku · 2026-10-06T03:44:26Z

                DevotionActivity.DevotionKind.RH -> ArticleRenunganHarian(date, body, readyToUse)
                DevotionActivity.DevotionKind.SH -> ArticleSantapanHarian(date, body, readyToUse)
-                DevotionActivity.DevotionKind.ME_EN -> ArticleMorningEveningEnglish(date, body, true)
+                DevotionActivity.DevotionKind.ME_EN -> ArticleMorningEveningEnglish(date, body, readyToUse)


why is this changed to readyToUse?

The server's NG response is persisted with readyToUse=false and a null body. Hardcoding true when loading Morning & Evening incorrectly turns that cached unavailable response into a ready article, then sends a null body to the renderer. Using the stored readyToUse flag preserves unavailable status and Retry when reopening the screen. The DAO tests verify NG cache round trips and readiness for every source.

yukuku · 2026-10-06T03:45:47Z

+`DevotionActivity.display()` reads the local cache first. Ready articles remain readable offline; non-ready rows represent the server's `NG` response and show an unavailable message with Retry. Selecting an uncached reading schedules a download. Opening the screen or switching sources also prefetches from today forward: 15 days for most sources, 3 for Renungan Harian. Prefetch captures the source at scheduling time, skips existing cache rows, and prunes rows with `touchTime` older than 180 days.
+
+`DevotionDownloader` uses two single-thread executors: one for the selected reading and one for prefetch. All queued and active requests share a registry keyed by `(source name, yyyyMMdd date)`, independent of article object equality. A queued selected reading is promoted to the foreground executor. A selected reading already active in prefetch reuses that request. Changing the selection cancels an obsolete foreground OkHttp call; its worker finishes before starting the latest selected request. Prefetch cannot occupy the foreground executor. Cancellation does not mark an obsolete selection as failed, and returning to a cancelling request restarts it after cancellation completes.
+
+The backend rechecks the database before a normal download, so work queued before another cache update does not redownload it. It requests `GET /devotion/get?name={kind}&date={yyyymmdd}` using a client derived from `Connections.okHttp`, retaining the shared HTTP cache, user agent, and connect/read/write timeouts. A 30-second total call timeout bounds connection attempts and complete response-body reading. Responses close after reading. HTTP, network, parsing, and persistence exceptions end in `FAILED`; they never persist a partial reading. `NG` ends in `UNAVAILABLE` and is stored as a non-ready row. A failed or unavailable refresh preserves any ready cached reading.


module docs do not need to be so detailed, it would lose sync between the code and docs. Can you also edit the existing one so that it is not super detailed? just give a high-level overview.

Rewrote the whole module document as a short high-level overview with key entry points and a brief testing note. Removed the detailed flow, state, database, timeout, and implementation descriptions so the document is easier to keep current.

Fix prioritized devotional downloads and retryable loading UI

3a8bde0

yukuku commented Oct 6, 2026

View reviewed changes

yukuku added 2 commits October 6, 2026 04:01

Address devotional review feedback and capture Robolectric screens

95f37ba

Center devotional download states and simplify status text

4617af8

yukuku merged commit 4df3f66 into develop Oct 6, 2026
4 checks passed

yukuku mentioned this pull request Oct 6, 2026

Add optional passage guide and per-passage reading-plan completion #307

Merged

		private val foregroundExecutor: ExecutorService = Executors.newSingleThreadExecutor(),
		private val prefetchExecutor: ExecutorService = Executors.newSingleThreadExecutor(),

Conversation

yukuku commented Oct 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Investigation

Result

Validation

Uh oh!

github-actions Bot commented Oct 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📱 Preview builds

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

yukuku commented Oct 2, 2026 •

edited

Loading

github-actions Bot commented Oct 2, 2026 •

edited

Loading