diff options
Diffstat (limited to 'app/src/main')
3 files changed, 207 insertions, 27 deletions
diff --git a/app/src/main/java/org/terst/feedreader/data/Settings.kt b/app/src/main/java/org/terst/feedreader/data/Settings.kt index b4af70e..ce05974 100644 --- a/app/src/main/java/org/terst/feedreader/data/Settings.kt +++ b/app/src/main/java/org/terst/feedreader/data/Settings.kt @@ -2,6 +2,8 @@ package org.terst.feedreader.data import android.content.Context import androidx.datastore.preferences.core.edit +import androidx.datastore.preferences.core.intPreferencesKey +import androidx.datastore.preferences.core.longPreferencesKey import androidx.datastore.preferences.core.stringPreferencesKey import androidx.datastore.preferences.preferencesDataStore import kotlinx.coroutines.flow.Flow @@ -13,6 +15,9 @@ val Context.settingsDataStore by preferencesDataStore(name = "settings") private object SettingsKeys { val STASH_BASE_URL = stringPreferencesKey("stash_base_url") val STASH_API_KEY = stringPreferencesKey("stash_api_key") + val LAST_FETCH_AT = longPreferencesKey("last_fetch_at") + val LAST_FETCH_NEW_ARTICLES = intPreferencesKey("last_fetch_new_articles") + val LAST_ACKNOWLEDGED_FETCH_AT = longPreferencesKey("last_acknowledged_fetch_at") } /** Stash connection settings, configured on the Settings screen. Both blank until set. */ @@ -31,4 +36,32 @@ class Settings(private val context: Context) { prefs[SettingsKeys.STASH_API_KEY] = apiKey.trim() } } + + // Durable record of "when did a feed fetch last finish, and with how many new + // articles" -- deliberately NOT derived from WorkManager's WorkInfo.State. A + // PeriodicWorkRequest's state cycles back to ENQUEUED after every successful run + // (it never settles on SUCCEEDED the way a one-time request does), so anything + // watching for SUCCEEDED only ever sees manual/alarm-triggered one-time fetches -- + // the actual hourly background fetch is invisible to that approach. Writing the + // result here on every run (periodic or one-time alike) gives the UI one consistent + // signal regardless of which triggered it. + val lastFetchAt: Flow<Long> = + context.settingsDataStore.data.map { it[SettingsKeys.LAST_FETCH_AT] ?: 0L } + val lastFetchNewArticles: Flow<Int> = + context.settingsDataStore.data.map { it[SettingsKeys.LAST_FETCH_NEW_ARTICLES] ?: 0 } + val lastAcknowledgedFetchAt: Flow<Long> = + context.settingsDataStore.data.map { it[SettingsKeys.LAST_ACKNOWLEDGED_FETCH_AT] ?: 0L } + + suspend fun recordFetchResult(newArticles: Int) { + context.settingsDataStore.edit { prefs -> + prefs[SettingsKeys.LAST_FETCH_AT] = System.currentTimeMillis() + prefs[SettingsKeys.LAST_FETCH_NEW_ARTICLES] = newArticles + } + } + + // Persisted (not just in-memory Compose state) so that navigating away from a screen + // and back doesn't forget a fetch was already acknowledged and re-show its snackbar. + suspend fun acknowledgeFetch(at: Long) { + context.settingsDataStore.edit { prefs -> prefs[SettingsKeys.LAST_ACKNOWLEDGED_FETCH_AT] = at } + } } diff --git a/app/src/main/java/org/terst/feedreader/ui/ArticleListScreen.kt b/app/src/main/java/org/terst/feedreader/ui/ArticleListScreen.kt index cbad3e2..9d88374 100644 --- a/app/src/main/java/org/terst/feedreader/ui/ArticleListScreen.kt +++ b/app/src/main/java/org/terst/feedreader/ui/ArticleListScreen.kt @@ -1,5 +1,6 @@ package org.terst.feedreader.ui +import android.util.Log import androidx.activity.compose.BackHandler import androidx.compose.foundation.ExperimentalFoundationApi import androidx.compose.foundation.background @@ -21,6 +22,8 @@ import androidx.compose.material3.IconButton import androidx.compose.material3.ListItem import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Scaffold +import androidx.compose.material3.SnackbarHost +import androidx.compose.material3.SnackbarHostState import androidx.compose.material3.Text import androidx.compose.material3.TopAppBar import androidx.compose.runtime.Composable @@ -37,17 +40,24 @@ import androidx.compose.ui.layout.ContentScale import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.text.style.TextDecoration import androidx.compose.ui.unit.dp +import androidx.work.WorkInfo +import androidx.work.WorkManager import coil.compose.AsyncImage +import kotlinx.coroutines.flow.catch import kotlinx.coroutines.flow.combine import kotlinx.coroutines.launch import org.terst.feedreader.data.AppDatabase import org.terst.feedreader.data.ArticleListItem import org.terst.feedreader.data.FilterResult import org.terst.feedreader.data.FilterRuleEngine +import org.terst.feedreader.data.Settings import org.terst.feedreader.data.markReadBatchChunked +import org.terst.feedreader.work.FeedFetchWorker import java.text.DateFormat import java.util.Date +private const val TAG = "ArticleListScreen" + @OptIn(ExperimentalMaterial3Api::class, ExperimentalFoundationApi::class) @Composable fun ArticleListScreen( @@ -81,6 +91,58 @@ fun ArticleListScreen( // single feed's own list already says so in the top bar, so rows there don't repeat it. val showFeedName = feedId == UNREAD_FEED_ID || feedId == OLDER_FEED_ID || feedId == SAVED_FEED_ID + // A feed fetch always touches every subscription at once -- there's no such thing as + // "fetching just this one feed" or "fetching Saved." Showing "checking feed 7 of 18" + // or a new-articles snackbar on Saved (not feed-driven at all) or a single specific + // feed's screen would misleadingly imply the fetch is about whatever's being viewed. + // Scoped to the two views that actually aggregate across every subscription. + val showFetchStatus = feedId == UNREAD_FEED_ID || feedId == OLDER_FEED_ID + + val snackbarHostState = remember { SnackbarHostState() } + val settings = remember { Settings(context) } + val fetchWorkInfos by remember { + WorkManager.getInstance(context).getWorkInfosByTagFlow(FeedFetchWorker.FETCH_WORK_TAG) + // A pure status readout, not load-bearing for the article list itself -- an + // exception collected via collectAsState would otherwise propagate straight + // through Compose and crash the whole screen. Worst case here is just falling + // back to the plain "nothing's fetching" state instead of taking the app down. + .catch { e -> Log.e(TAG, "Fetch status flow failed", e); emit(emptyList()) } + }.collectAsState(initial = emptyList()) + // RUNNING only, deliberately not ENQUEUED: a periodic WorkRequest's state cycles + // straight back to ENQUEUED after every successful run rather than settling anywhere + // else, so it's ENQUEUED almost all the time between hourly runs -- treating that as + // "currently fetching" meant this permanently showed "Checking feeds…" instead of + // ever actually reaching "Up to date". + val runningFetch = fetchWorkInfos.firstOrNull { it.state == WorkInfo.State.RUNNING } + val isFetching = runningFetch != null + val fetchStatusText = runningFetch?.progress?.let { progress -> + val current = progress.getInt(FeedFetchWorker.KEY_CURRENT, -1) + val total = progress.getInt(FeedFetchWorker.KEY_TOTAL, -1) + if (current >= 0 && total > 0) "Checking feed ${current + 1} of $total…" else "Checking feeds…" + } ?: "Checking feeds…" + + // Driven by Settings' durable lastFetchAt/lastFetchNewArticles, NOT WorkInfo.SUCCEEDED + // -- a periodic fetch's WorkInfo never reaches SUCCEEDED (see above), so watching for + // that only ever caught manual/alarm-triggered one-time fetches, never the actual + // hourly background one. lastAcknowledgedFetchAt is persisted too, not just in-memory + // Compose state, so leaving and returning to this screen (a fresh composable instance) + // doesn't re-show a snackbar for a fetch that was already acknowledged elsewhere. + if (showFetchStatus) { + val lastFetchAt by settings.lastFetchAt.collectAsState(initial = 0L) + val lastFetchNewArticles by settings.lastFetchNewArticles.collectAsState(initial = 0) + val lastAcknowledgedFetchAt by settings.lastAcknowledgedFetchAt.collectAsState(initial = 0L) + LaunchedEffect(lastFetchAt) { + if (lastFetchAt > lastAcknowledgedFetchAt) { + if (lastFetchNewArticles > 0) { + snackbarHostState.showSnackbar( + if (lastFetchNewArticles == 1) "1 new article" else "$lastFetchNewArticles new articles" + ) + } + settings.acknowledgeFetch(lastFetchAt) + } + } + } + // Unread is the app's landing screen (root of the back stack) -- the system back // gesture/button here would otherwise exit the app entirely. Route it to the feed // list instead, matching the same forward affordance already in the top bar. @@ -140,7 +202,8 @@ fun ArticleListScreen( } } ) - } + }, + snackbarHost = { SnackbarHost(snackbarHostState) } ) { padding -> LazyColumn(modifier = Modifier.fillMaxSize().padding(padding)) { items(visibleArticles, key = { it.first.id }) { (article, result) -> diff --git a/app/src/main/java/org/terst/feedreader/work/FeedFetchWorker.kt b/app/src/main/java/org/terst/feedreader/work/FeedFetchWorker.kt index 5ec6bab..9c2962f 100644 --- a/app/src/main/java/org/terst/feedreader/work/FeedFetchWorker.kt +++ b/app/src/main/java/org/terst/feedreader/work/FeedFetchWorker.kt @@ -7,13 +7,24 @@ import androidx.work.NetworkType import androidx.work.PeriodicWorkRequestBuilder import androidx.work.WorkManager import androidx.work.WorkerParameters +import androidx.work.workDataOf +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.async +import kotlinx.coroutines.awaitAll +import kotlinx.coroutines.coroutineScope +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.Semaphore +import kotlinx.coroutines.sync.withPermit import org.terst.feedreader.data.AppDatabase import org.terst.feedreader.data.ArticleEntity import org.terst.feedreader.data.FeedEntity import org.terst.feedreader.data.FeedParser import org.terst.feedreader.data.Net +import org.terst.feedreader.data.Settings +import org.terst.feedreader.data.await import okhttp3.Request import java.security.MessageDigest +import java.util.concurrent.atomic.AtomicInteger import java.util.concurrent.TimeUnit /** @@ -27,33 +38,80 @@ class FeedFetchWorker(context: Context, params: WorkerParameters) : CoroutineWor private val client = Net.client override suspend fun doWork(): Result { - val db = AppDatabase.get(applicationContext) - val feeds = db.feedDao().getAll() + // WorkManager's "unique work name" dedup does NOT prevent this: schedulePeriodic + // and fetchNow use two different unique-work names (periodic and one-time requests + // can't share one), so an hourly periodic run and a pull-to-refresh/morning-alarm + // fetch firing at the same moment both start doWork() concurrently -- every feed + // gets fetched twice, and both instances race writes to the same feed rows' + // etag/lastModified/lastFetchedAt. This mutex is process-local, which is exactly + // right here: WorkManager runs this worker in the app's own process, never a + // separate one, so a single in-memory lock covers every real overlap case. + if (!fetchMutex.tryLock()) { + return Result.success() + } + try { + val db = AppDatabase.get(applicationContext) + val feeds = db.feedDao().getAll() + val completedCount = AtomicInteger(0) - for (feed in feeds) { - fetchOne(feed, db) + // Fetched with bounded concurrency, not one feed at a time: with a large + // subscription list (this app has seen 100+ feeds in the wild -- see + // AppDatabase's v9 migration comments), a strictly sequential loop means one + // slow/stalling server delays every feed queued behind it, and the whole run + // can take tens of seconds to minutes. Dispatchers.IO (not the CoroutineWorker + // default of Dispatchers.Default) because this is blocking network I/O, not + // CPU-bound work -- running it on Default would compete with the app's other + // CPU-bound coroutine work for that dispatcher's small, core-sized thread pool. + val newArticleCount = coroutineScope { + val semaphore = Semaphore(FETCH_CONCURRENCY) + feeds.map { feed -> + async(Dispatchers.IO) { + semaphore.withPermit { + val count = fetchOne(feed, db) + // KEY_CURRENT now means "how many feeds have finished," not + // "which one's currently being checked" -- there's no single + // "current" feed anymore under concurrent fetching, but a + // monotonically-advancing completed-count is just as good a + // progress signal for the UI. + setProgress(workDataOf(KEY_CURRENT to completedCount.incrementAndGet(), KEY_TOTAL to feeds.size)) + count + } + } + }.awaitAll().sum() + } + // Durable, not WorkInfo-state-based -- see Settings.recordFetchResult's doc + // comment for why periodic work's own state can't be used for this. + Settings(applicationContext).recordFetchResult(newArticleCount) + return Result.success(workDataOf(KEY_TOTAL to feeds.size, KEY_NEW_ARTICLES to newArticleCount)) + } finally { + fetchMutex.unlock() } - return Result.success() } - private suspend fun fetchOne(feed: FeedEntity, db: AppDatabase) { + /** Returns how many articles from this feed were genuinely new (not already stored). */ + private suspend fun fetchOne(feed: FeedEntity, db: AppDatabase): Int { val requestBuilder = Request.Builder().url(feed.id) feed.etag?.let { requestBuilder.header("If-None-Match", it) } feed.lastModified?.let { requestBuilder.header("If-Modified-Since", it) } - try { - client.newCall(requestBuilder.build()).execute().use { response -> + return try { + client.newCall(requestBuilder.build()).await().use { response -> val now = System.currentTimeMillis() if (response.code == 304) { - db.feedDao().update(feed.copy(lastFetchedAt = now, lastError = null)) - return + db.feedDao().updateFetchResult( + feed.id, feed.title, feed.siteUrl, now, feed.etag, feed.lastModified, lastError = null + ) + return@use 0 } if (!response.isSuccessful) { - db.feedDao().update(feed.copy(lastFetchedAt = now, lastError = "HTTP ${response.code}")) - return + db.feedDao().updateFetchResult( + feed.id, feed.title, feed.siteUrl, now, feed.etag, feed.lastModified, + lastError = "HTTP ${response.code}" + ) + return@use 0 } - val body = response.body ?: return + val body = response.body ?: return@use 0 val parsed = FeedParser.parse(body.byteStream()) val articles = parsed.items.map { item -> @@ -70,21 +128,28 @@ class FeedFetchWorker(context: Context, params: WorkerParameters) : CoroutineWor createdAt = now ) } - db.articleDao().insertNew(articles) - - db.feedDao().update( - feed.copy( - title = parsed.title.ifBlank { feed.title }, - siteUrl = parsed.siteUrl ?: feed.siteUrl, - lastFetchedAt = now, - etag = response.header("ETag") ?: feed.etag, - lastModified = response.header("Last-Modified") ?: feed.lastModified, - lastError = null - ) + // Room returns -1 for a row OnConflictStrategy.IGNORE actually skipped + // (already stored), and the real inserted rowid (>= 0) for a genuinely new + // one -- count only the latter so "N new articles" reflects reality. + val newCount = db.articleDao().insertNew(articles).count { it != -1L } + + db.feedDao().updateFetchResult( + id = feed.id, + title = parsed.title.ifBlank { feed.title }, + siteUrl = parsed.siteUrl ?: feed.siteUrl, + lastFetchedAt = now, + etag = response.header("ETag") ?: feed.etag, + lastModified = response.header("Last-Modified") ?: feed.lastModified, + lastError = null ) + newCount } } catch (e: Exception) { - db.feedDao().update(feed.copy(lastFetchedAt = System.currentTimeMillis(), lastError = e.message ?: "Fetch failed")) + db.feedDao().updateFetchResult( + feed.id, feed.title, feed.siteUrl, System.currentTimeMillis(), feed.etag, feed.lastModified, + lastError = e.message ?: "Fetch failed" + ) + 0 } } @@ -96,6 +161,22 @@ class FeedFetchWorker(context: Context, params: WorkerParameters) : CoroutineWor companion object { private const val UNIQUE_WORK_NAME = "feed_fetch_periodic" + // Common tag on every enqueue (periodic AND one-off) so ArticleListScreen can + // observe "is a fetch happening right now" via a single getWorkInfosByTagFlow + // query, regardless of which of the two triggered it. + const val FETCH_WORK_TAG = "feed_fetch" + const val KEY_CURRENT = "current" + const val KEY_TOTAL = "total" + const val KEY_NEW_ARTICLES = "newArticles" + + // Bounded, not unbounded (Dispatchers.IO can grow to 64+ threads on demand) -- + // enough to meaningfully overlap slow feeds without opening 100+ simultaneous + // connections to 100+ different hosts at once on a fetch run with a large + // subscription list. + private const val FETCH_CONCURRENCY = 8 + + private val fetchMutex = Mutex() + fun schedulePeriodic(context: Context) { val request = PeriodicWorkRequestBuilder<FeedFetchWorker>(60, TimeUnit.MINUTES) .setConstraints( @@ -103,6 +184,7 @@ class FeedFetchWorker(context: Context, params: WorkerParameters) : CoroutineWor .setRequiredNetworkType(NetworkType.CONNECTED) .build() ) + .addTag(FETCH_WORK_TAG) .build() WorkManager.getInstance(context).enqueueUniquePeriodicWork( UNIQUE_WORK_NAME, @@ -113,7 +195,9 @@ class FeedFetchWorker(context: Context, params: WorkerParameters) : CoroutineWor /** One-off immediate fetch, e.g. a manual pull-to-refresh. */ fun fetchNow(context: Context) { - val request = androidx.work.OneTimeWorkRequestBuilder<FeedFetchWorker>().build() + val request = androidx.work.OneTimeWorkRequestBuilder<FeedFetchWorker>() + .addTag(FETCH_WORK_TAG) + .build() WorkManager.getInstance(context).enqueueUniqueWork( "feed_fetch_now", androidx.work.ExistingWorkPolicy.KEEP, |