diff options
Diffstat (limited to 'app')
4 files changed, 83 insertions, 32 deletions
diff --git a/app/src/main/java/org/terst/feedreader/data/Daos.kt b/app/src/main/java/org/terst/feedreader/data/Daos.kt index 8a5f50e..ecaac43 100644 --- a/app/src/main/java/org/terst/feedreader/data/Daos.kt +++ b/app/src/main/java/org/terst/feedreader/data/Daos.kt @@ -22,13 +22,55 @@ interface FeedDao { @Insert(onConflict = OnConflictStrategy.REPLACE) suspend fun upsert(feed: FeedEntity) + // Batched, not one upsert() call per feed in a loop: an OPML import can realistically + // be 100+ feeds (this app has seen a 107-feed subscription list in the wild -- see + // AppDatabase's v9 migration comments), and a per-feed getById()+upsert() pair is 2N + // sequential suspend DB round-trips for N feeds. One @Insert(REPLACE) call covers the + // whole list. + @Insert(onConflict = OnConflictStrategy.REPLACE) + suspend fun upsertAll(feeds: List<FeedEntity>) + @Update suspend fun update(feed: FeedEntity) + // Targeted, not the full-row @Update above: FeedFetchWorker reads every subscribed + // feed once at the start of a fetch cycle, then (after a per-feed network round-trip + // that can take a while, especially across 100+ feeds) writes back a `.copy()` of + // that now-possibly-stale snapshot. If the user toggles readerModeEnabled or edits a + // feed's folder via FeedListScreen while that feed's fetch is still in flight, a + // full-row update() overwrites the WHOLE row from the stale pre-fetch copy -- + // including the field the user just changed -- silently reverting their edit. This + // only ever touches the columns a fetch actually produces, so it can't clobber + // anything the user might be changing concurrently. + @Query( + "UPDATE feeds SET title = :title, siteUrl = :siteUrl, lastFetchedAt = :lastFetchedAt, " + + "etag = :etag, lastModified = :lastModified, lastError = :lastError WHERE id = :id" + ) + suspend fun updateFetchResult( + id: String, + title: String, + siteUrl: String?, + lastFetchedAt: Long, + etag: String?, + lastModified: String?, + lastError: String? + ) + @Query("DELETE FROM feeds WHERE id = :id") suspend fun delete(id: String) } +// Persists the toggle and returns the exact confirmation-snackbar text, shared by +// FeedListScreen's per-row toggle and ReaderScreen's toggleFeedReaderMode (which layers +// its own extraction logic around this same persist-and-message step) -- previously two +// independently-typed copies of "Reader mode enabled/disabled for $label", one edit away +// from silently drifting to say different things for the same action. +suspend fun FeedDao.setReaderModeEnabled(feed: FeedEntity, enabled: Boolean): String { + update(feed.copy(readerModeEnabled = enabled)) + val label = feed.title.ifBlank { "this feed" } + return if (enabled) "Reader mode enabled for $label" else "Reader mode disabled for $label" +} + data class FeedUnreadCount(val feedId: String, val count: Int) // A projection excluding feedContentHtml/extractedContent -- the potentially diff --git a/app/src/main/java/org/terst/feedreader/ui/AddFeedScreen.kt b/app/src/main/java/org/terst/feedreader/ui/AddFeedScreen.kt index 38b0f32..1e8814c 100644 --- a/app/src/main/java/org/terst/feedreader/ui/AddFeedScreen.kt +++ b/app/src/main/java/org/terst/feedreader/ui/AddFeedScreen.kt @@ -54,30 +54,42 @@ fun AddFeedScreen(onDone: () -> Unit) { if (uri == null) return@rememberLauncherForActivityResult isBusy = true scope.launch { - val imported = withContext(Dispatchers.IO) { - context.contentResolver.openInputStream(uri)?.use { stream -> Opml.parse(stream) } ?: emptyList() - } - // A feed cross-listed under multiple OPML category folders (common when - // exported from a reader with folders/labels) appears once per folder here -- - // dedupe by feed URL before reporting a count, or "108 feeds" would be reported - // as some larger number even though upsert already collapses them to one row each. - val uniqueFeeds = imported.map { it.copy(xmlUrl = preferHttps(it.xmlUrl)) }.distinctBy { it.xmlUrl } - for (feed in uniqueFeeds) { - // upsert() REPLACEs the whole row -- for a feed that's already subscribed, - // building a bare FeedEntity here would silently wipe readerModeDisabled - // (and etag/lastModified/lastError) back to defaults on every re-import. - // Preserve the existing row's other fields; only title/siteUrl come from OPML. - val existing = db.feedDao().getById(feed.xmlUrl) - val entity = existing?.copy( - title = feed.title, - siteUrl = feed.htmlUrl ?: existing.siteUrl, - folder = feed.folder ?: existing.folder - ) ?: FeedEntity(id = feed.xmlUrl, title = feed.title, siteUrl = feed.htmlUrl, folder = feed.folder) - db.feedDao().upsert(entity) + try { + val imported = withContext(Dispatchers.IO) { + context.contentResolver.openInputStream(uri)?.use { stream -> Opml.parse(stream) } ?: emptyList() + } + // A feed cross-listed under multiple OPML category folders (common when + // exported from a reader with folders/labels) appears once per folder here -- + // dedupe by feed URL before reporting a count, or "108 feeds" would be reported + // as some larger number even though upsert already collapses them to one row each. + val uniqueFeeds = imported.map { it.copy(xmlUrl = preferHttps(it.xmlUrl)) }.distinctBy { it.xmlUrl } + withContext(Dispatchers.IO) { + // Loaded once, not one getById() per feed: upsert() REPLACEs the whole + // row, and for a feed that's already subscribed, building a bare + // FeedEntity here would silently wipe readerModeDisabled (and + // etag/lastModified/lastError) back to defaults on every re-import -- + // preserve the existing row's other fields, only title/siteUrl come + // from OPML. + val existingById = db.feedDao().getAll().associateBy { it.id } + val entities = uniqueFeeds.map { feed -> + val existing = existingById[feed.xmlUrl] + existing?.copy( + title = feed.title, + siteUrl = feed.htmlUrl ?: existing.siteUrl, + folder = feed.folder ?: existing.folder + ) ?: FeedEntity(id = feed.xmlUrl, title = feed.title, siteUrl = feed.htmlUrl, folder = feed.folder) + } + db.feedDao().upsertAll(entities) + } + FeedFetchWorker.fetchNow(context) + status = "Imported ${uniqueFeeds.size} feed(s)" + } catch (e: Exception) { + // A non-XML or corrupt/truncated file throws from Opml.parse -- without this, + // that crashed the whole app instead of just failing this one import. + status = "Couldn't import: ${e.message}" + } finally { + isBusy = false } - FeedFetchWorker.fetchNow(context) - status = "Imported ${uniqueFeeds.size} feed(s)" - isBusy = false } } diff --git a/app/src/main/java/org/terst/feedreader/ui/FeedListScreen.kt b/app/src/main/java/org/terst/feedreader/ui/FeedListScreen.kt index 90bfd7e..3f9a135 100644 --- a/app/src/main/java/org/terst/feedreader/ui/FeedListScreen.kt +++ b/app/src/main/java/org/terst/feedreader/ui/FeedListScreen.kt @@ -48,6 +48,7 @@ import kotlinx.coroutines.flow.combine import kotlinx.coroutines.launch import org.terst.feedreader.data.AppDatabase import org.terst.feedreader.data.FeedEntity +import org.terst.feedreader.data.setReaderModeEnabled import org.terst.feedreader.work.FeedFetchWorker @OptIn(ExperimentalMaterial3Api::class) @@ -170,11 +171,8 @@ fun FeedListScreen( onToggleReaderMode = { val newEnabled = !feed.readerModeEnabled scope.launch { - db.feedDao().update(feed.copy(readerModeEnabled = newEnabled)) - val label = feed.title.ifBlank { "this feed" } - snackbarHostState.showSnackbar( - if (newEnabled) "Reader mode enabled for $label" else "Reader mode disabled for $label" - ) + val message = db.feedDao().setReaderModeEnabled(feed, newEnabled) + snackbarHostState.showSnackbar(message) } } ) diff --git a/app/src/main/java/org/terst/feedreader/ui/ReaderScreen.kt b/app/src/main/java/org/terst/feedreader/ui/ReaderScreen.kt index b586fbc..ae63258 100644 --- a/app/src/main/java/org/terst/feedreader/ui/ReaderScreen.kt +++ b/app/src/main/java/org/terst/feedreader/ui/ReaderScreen.kt @@ -62,6 +62,7 @@ import org.terst.feedreader.data.ArticleEntity import org.terst.feedreader.data.FeedEntity import org.terst.feedreader.data.ensureStandaloneArticle import org.terst.feedreader.data.saveArticleToStash +import org.terst.feedreader.data.setReaderModeEnabled import org.terst.feedreader.readability.ReaderExtractor import java.net.URLEncoder @@ -229,8 +230,7 @@ private fun ReaderContent( val newEnabled = !f.readerModeEnabled feed = f.copy(readerModeEnabled = newEnabled) scope.launch { - db.feedDao().update(f.copy(readerModeEnabled = newEnabled)) - val feedLabel = f.title.ifBlank { "this feed" } + val message = db.feedDao().setReaderModeEnabled(f, newEnabled) if (newEnabled) { if (readerContent == null) { isExtracting = true @@ -243,13 +243,12 @@ private fun ReaderContent( } } isReaderMode = true - snackbarHostState.showSnackbar("Reader mode enabled for $feedLabel") } else { isReaderMode = false readerContent = null db.articleDao().clearExtractedContent(articleId) - snackbarHostState.showSnackbar("Reader mode disabled for $feedLabel") } + snackbarHostState.showSnackbar(message) } } |