diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 6a73a66..b56d578 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -92,6 +92,7 @@ jobs: working-directory: shared - run: bun install --frozen-lockfile - run: bun lint + - run: bun test - run: bun pack - uses: actions/upload-artifact@v7 with: diff --git a/PLAN.md b/PLAN.md index d115843..867caa9 100644 --- a/PLAN.md +++ b/PLAN.md @@ -30,12 +30,58 @@ fetches feeds itself and syncs through Firebase. - **Formats:** RSS 2.0, Atom and JSON Feed. - **Images:** never stored. The article keeps the image URL and the reader loads it from the original site. -- **https only:** no plain http anywhere. Addresses typed or imported as - `http://` are rewritten to `https://` before the first try (a feed that then - fails shows as failing); feed, site, icon and image addresses from feeds are - kept only when https (http upgraded); links and pictures in article text are - upgraded to https, and a picture that cannot load over https is dropped. The - web app's content policy loads pictures and media from https and data only. +- **https only:** no plain http is ever fetched. An `http://` address typed + or pasted is refused ("Reader can't add http addresses. Try https + instead."). Any other plain http feed (one listed in an OPML file, a Feedly + search result, or a followed feed saved with http, as older devices and + accounts hold) is tried at its https address (`shared/src/https.ts`, ported + in `android/core/.../HttpsUpgrade.kt`, cases in `shared/fixtures/https.json`): + it counts only when the answer is a 200 that stays on https, parses as a + feed, and looks like the same feed (the same name ignoring capitals and + spacing, an article in common by guid or link, or a site on the same host as + the known site or the http address, ignoring `www.`; a feed the person + renamed is not compared by name, since its own name is not kept). An + imported feed that passes is added at https and one that fails is left out + and counted; a search result that fails is not shown. An address whose host + is not a public name (an IP address or a number browsers read as one, a + name of one label, `localhost`, or a name under `.local`, `.internal`, + `.home.arpa` or `.lan`), or that has a user name or password, is never + tried and fails (`probeAllowed`), since these addresses come from files, + searches and accounts, not from the person, and trying them must not reach + the device or its network. A followed http feed + is never put on the schedule and never counts as due; each one is tried once + each time the app starts (alongside its checks of due feeds), at most six + at once, and again on each later refresh while its tries get no definite + answer (no answer at all, as offline or after a timeout, or a 408, 425, 429 + or 5xx), at most three tries without one (`UNSETTLED_TRIES`) until the app + starts again. Background checks (the extension's timer, Android's WorkManager) + never try them; the app does it when it opens. One that passes keeps its id + (so its articles, marks, settings and group stay), takes the https address + as a change of its own (`updatedAt`, sent to the account, so other devices + move too) and takes the answer as its first check (on the web the answer + goes to the extension with the feed's address, as `first`, and starts its + schedule, as an added feed's answer does; answers that would make one + message larger than 8 MiB of JSON are spread over several `track` + messages, each listing every feed; the extension skips a `first` that is + not a fetch's answer and leaves that feed due); one that fails, or whose https + address is already followed, shows "This feed's address is http, not + https, so Reader can't fetch it." A redirect from https to plain http is + followed at the https form of its address, never at http (the extension's + content policy has `upgrade-insecure-requests`, which browsers apply to + every redirect; Android follows such redirects itself), and fails when + nothing answers there. A site that sends its https address back to http + then loops; the extension tells that from no answer by asking the address + again without following redirects, and reports a redirect answer as not on + https, a definite answer ("The feed is not on https." when adding). A feed's id is therefore the hash of the + address it was first followed at, so adding and importing also treat a feed + whose address matches as already followed, including one followed at the + http form of the address; adding that https address moves such a feed there + when it passes the same comparison, and otherwise adds the https feed as a + feed of its own beside it. Feed, site, icon and image + addresses from feeds are kept only when https (http upgraded); links and + pictures in article text are upgraded to https, and a picture that cannot + load over https is dropped. The web app's content policy loads pictures and + media from https and data only. ## Data model @@ -184,9 +230,13 @@ from a device. over the weekend and quiet hours are skipped. Waits range from 15 minutes to 7 days, never shorter than the site's `Retry-After` or `Cache-Control: max-age`, and a failing feed backs off from 15 minutes by doubling, or longer when the - site asks. Opening the app and the refresh control force a check of every - feed, so freshness comes from the reader and the schedule only has to avoid - missing posts. Every fetch sends `ETag` and `If-Modified-Since`; a `304` ends + site asks. Opening the app queues a check of every feed that is due, + without waiting for it and without showing a refresh (the web sends + `checkDue`, which the extension answers as soon as the checks are queued, + then takes in what the extension found while the app was closed; the + results land through the usual drain), and the refresh controls force a + check of the feeds they cover, so freshness comes from the reader and the + schedule only has to avoid missing posts. Every fetch sends `ETag` and `If-Modified-Since`; a `304` ends it. The chance a feed has news since its last successful check is `1 - (beta / (beta + activity))^alpha` over the expected activity since then; a feed never checked counts as one, a failing feed as 0.5 if it is due and @@ -241,7 +291,8 @@ decides). Web with the extension closed gets nothing until a device opens. store) and synced like entries. The panel opens where the article does, and as a full page on a phone. It holds sort (Newest or Oldest), words and topics (Starred has only the sort); the unread-only switch is in the list's - bar, not here. A feed's panel also has its group and Unsubscribe. + bar, not here. A feed's panel also has its group and Unsubscribe. The panel + also holds the reading font, which is one choice for the whole app (see below). - **Words and topics** (`shared/src/filter.ts`): type a word or phrase and press Enter; two buttons choose whether new words hide matching articles or, once any "only show" word exists, keep only articles that have one. Words match the title @@ -249,7 +300,14 @@ decides). Web with the extension closed gets nothing until a device opens. whitespace matches any whitespace, a word boundary is required at an end only when that end is a word character, and each list is compiled once. Every topic the feed supplies starts on; tap one to turn it off, and an article whose - topics are all off is hidden. Chips never move when toggled. A feed's own + topics are all off is hidden. Every list of topics or words (the panel's + words and topics, and an article's topics, on Starred too) is ordered by how + many of the view's articles carry or match each (most first, then by name; an + article lists each of its topics once), wrapping into up to three rows; when + they need more, they go in three rows read down each column and scroll + sideways. Chips never move when toggled. On a phone each topic chip, and + each word's remove button, takes taps 44 pixels tall (48 dp on Android) + around its smaller outline; both lists use the same chip. A feed's own settings also apply in every group and view it appears in. - **Managing feeds and groups:** a feed's panel has its name (rename), a group field (type a name, with the existing groups suggested; empty for no group) and @@ -266,41 +324,108 @@ decides). Web with the extension closed gets nothing until a device opens. profiles, Mastodon profiles. YouTube `@handle` pages are found through their page link. Any other text searches Feedly's public feed search through the extension (a dependency to watch: it is not a documented open API and the - query goes to Feedly). -- **OPML import:** drop a file on the panel or choose one; folders are kept; - unusable addresses are skipped; the panel says when a file lists nothing or - nothing new. + query goes to Feedly). A result listed at plain http is tried at its https + address before it is shown (see https only), so searching can contact the + sites of up to eight results the person never picked. +- **OPML import:** drop a file on the panel or choose one; an "Add to group" + field above it, the same type-or-pick field as a feed's group, puts all the + imported feeds in the group named (made if new); left empty ("Folders from + the file") it keeps the file's own folders; + unusable addresses are skipped; plain http feeds go as https only (see https + only) and the panel counts those left out; it says when a file lists nothing + or nothing new. A file's `htmlUrl` is kept as the feed's site, and written + back on export. - **Feed pictures:** the feed's own icon, else the site's `/favicon.ico`, else a letter tile. The sidebar rows keep one icon column in both states. - **Refreshing:** the Reader name goes to All items and checks every feed, as does `Shift R`; clicking the row of the view you are in, the `r` key and pulling the list down from its top on a phone check the feeds of that view, as does the - Refresh button in the footer that ends every non-empty list ("End of list" - and Refresh, shown once every row is drawn); there is no Refresh - button in the bars and no gesture at the bottom. Opening the app checks every feed. A refresh drops the read articles + Refresh bar that ends every non-empty list (shown once every row is drawn); + there is no Refresh button in the bars and no gesture at the bottom. Opening + the app checks only the feeds that are due, in the background: nothing turns, + the logo does not pulse, the list keeps its rows and Refresh stays usable. A + refresh drops the read articles kept in the list as soon as it starts; on a phone the list shows a turning - marker until it ends. + marker until it ends. Only a refresh the person asks for shows: an import's + first check of its feeds runs behind the loading screen, and every other + check and every change from the account lands unseen. - **Unread only:** one switch for the whole app, an eye button in the list's bar (at the bottom on a phone), on by default; it lists only unread articles in every feed and group (not in Starred) and hides feeds in the sidebar that have nothing unread (the one you are viewing stays). Changing view closes the open article. A failed feed is not colored: after two failures in a row a small muted marker appears, with the reason in the feed's panel and in a band at the top of its list. +- **Reading font:** Literata (the default) or Public Sans, chosen in every view's + panel and applied everywhere. It sets article text, article titles and the + large page headings; the rest of the interface is always Public Sans, and + counts, bylines and code use the system monospace. It is kept on the device + only (`h-reader:reading-font` in the web's local storage, the `reader` shared + preferences on Android, both `literata` or `public-sans`) and is not synced. + The web serves both fonts from the app's own site (fontsource variable + weights, upright and italic; the browser fetches only the scripts and styles + a page uses), so every script fontsource offers (Latin, Cyrillic, Greek, + Vietnamese) is covered. Android bundles both, upright and italic, cut with + fonttools from Google Fonts' variable files by `android/scripts/cut-fonts.sh` + (pinned sources; run it again to remake `res/font`): weights 400 to 800, + Literata's optical size fixed at 12 pt, and Literata kept to Basic Latin, + Latin-1, Latin Extended-A and B, Vietnamese, combining marks, general + punctuation, currency signs, modern Greek and the Cyrillic of Russian, + Ukrainian, Belarusian, Bulgarian, Serbian and Macedonian (Public Sans has + Latin only and keeps every glyph). Polytonic Greek, other Cyrillic, and + Greek and Cyrillic in Public Sans fall back to the system font. - **Speed:** the list draws only the first 60 rows and more as the end nears; sorting and the word and topic filters are computed once per change, not per - toggle. With 20,000 articles, toggling unread only takes about 60 ms, all on + toggle. The view's topic and word counts, and the topic order each row + looks its topics up in, are worked out once per view and kept while only + read or starred marks change. With 20,000 articles, toggling unread only takes about 60 ms, all on the main thread. - **Reading state:** opening an article does not mark - it read, leaving it does; read articles stay listed until the view changes; + it read, leaving it does; read articles, here or on another device, stay + listed until the view changes; stepping past either end closes the article. - **Sidebar order:** groups come first, then loose feeds; each list is ordered by unread count (most first), then working before failing, then by name; the feeds inside a group follow the same rule, and a group counts as failing only when all of its feeds - do. The `[` and `]` keys follow the same order. The order is worked out at - load and again after each refresh, and holds in between; counts still update live, and feeds added - later go after the rest. -- **Loading logo:** the logo pulses while the app loads, feeds refresh, or a feed is - being added (not with reduced motion). + do. The `[` and `]` keys follow the same order. The order is worked out when + the loading screen lets the person in and again after each refresh they ask + for (not after the checks made on opening), and holds in between; counts + still update live, and feeds added later go after the rest. +- **Loading logo:** the sidebar logo pulses while feeds refresh at the + person's request or a feed is being added (not with reduced motion). +- **Loading screen:** while what is loading can still change the lists, an + opaque screen covers the app and takes every tap, key and swipe (Back still + leaves the app from the list on Android): at opening, while stored data is + read (and, on the web, the extension is found and what it fetched while the + app was closed is taken in); while the account's data arrives, at opening + when signed in and after signing in or replacing this device's data, until + its feeds and settings have come from the server, the entry listener's first + changes are stored and the first pass has the last 90 days (sync offline, + failing or waiting for a choice does not hold it); and after an import, + until its feeds' first check is in. It is blank for its first 300 ms, so an + instant load shows nothing, then shows the turning logo for at least 700 ms; + after 15 s it lets the person in whatever is still loading, which goes on + unseen. Each time it lets the person in, the lists are settled: the article + list and the sidebar take in everything that has arrived and the sidebar + order is worked out again. The state machine is `web/src/lib/load-gate.ts` + and `android/core/.../LoadGate.kt`, with the same timing and tests. +- **No pop-in:** once the person is in, articles and feeds that arrive in the + background (checks, the extension's drains, WorkManager, the account) wait + for the lists to be settled: the article list takes them in when the view + changes, unread only is switched, or a refresh the person asked for ends; the + sidebar when unread only is switched or such a refresh ends. A feed the + sidebar does not list (nothing unread under unread only) stays out when its + articles arrive, and an article read on another device stays listed until + the view changes, as one read here does. A list or sidebar that shows + nothing takes in what arrives, since no row can move; a feed the person adds + shows at once. Unread counts still change as marks and articles arrive. +- **Refresh dot:** a small accent dot on the sidebar logo and on the phone's menu + button ("Some feeds are due for a check") says a refresh would help: a feed's + scheduled check is more than 30 minutes late (`OVERDUE_MS`, twice the shortest + wait, so the background timers normally get there first) and nothing is + checking feeds. It is worked out from the earliest next check, one pass per + change and one timer, and clears once a refresh has saved the late feeds. The + web learns each feed's next check, and whether the extension is checking, from + its drain replies. - **First run:** with no feeds, the reader asks for a first feed and, below that, offers sign-in ("Already use Reader?" and a "Sign in to sync your feeds" button, the same sign-in as the sidebar's), for someone whose feeds @@ -327,7 +452,8 @@ decides). Web with the extension closed gets nothing until a device opens. such readers break when the site changes. - **Navigation:** one page app; the extension plays the part a service worker does for a PWA (fetching, schedule, background). On a phone the menu button in the - list's bottom bar opens the feed list over the articles (the title is plain text), + list's bottom bar opens the feed list over the articles, as does tapping the view's name + beside it ("Show feeds"), as does a swipe in from the left edge on the web (the Android app opens it only from the button, since a sideways swipe on a card marks it read); moving between the article list and an article adds browser history entries, so Back steps out. On Android, Back on the article list @@ -340,7 +466,11 @@ decides). Web with the extension closed gets nothing until a device opens. read, share and open the original, which on a phone appear only there; view settings, add feed and sync have Back and the screen's name. On Android the bars slide away while scrolling down and come back on the way up, on every new screen or article, and - whenever the article is too short to scroll. Wide screens keep the bars at the top. + whenever the article is too short to scroll; there every scrolling screen ends with + room for the bar's full height and a margin, the same whether the bar is shown or + not, so the end clears the bar and there is room to scroll back up and bring it back. + The web's bars always show and sit below the content, so it needs no such room. Wide + screens keep the bars at the top. - **Gestures (phone):** swipe a card sideways to mark it read or unread; swipe an article left or right and it slides to the next or previous one in the list's order (a pager on Android); a swipe of at least 90 pixels (90 dp on Android) past the first or last slides @@ -348,11 +478,12 @@ decides). Web with the extension closed gets nothing until a device opens. while a shorter one springs back; pull down at the top of the list to check the view's feeds (Material's pull to refresh on Android, the same marker on the web). -- **End of list:** the last row of a list that has articles says "End of list" and - has a "Refresh" button that checks the view's feeds as pulling down does; while - they are checked the button keeps its label, is disabled and its icon turns - (still when the system or browser turns motion off). It sits above the bottom - bar; there is no pull-up gesture. +- **End of list:** the last row of a list that has articles is a full-width + "Refresh" bar, tappable anywhere across it, that checks the view's feeds as + pulling down does; while they are checked it keeps its label, is disabled and + its icon turns (still when the system or browser turns motion off). It sits + above the bottom bar; there is no pull-up gesture. A list with no articles + shows only a line of text, with no picture. - **Sidebar:** a brand header (the icon and "Reader") stays in place while the sidebar collapses to an icon strip, which opens over the list when hovered; icons and rows keep their positions so the change is a smooth width and fade. diff --git a/android/app/src/main/assets/licenses/OFL-Fraunces.txt b/android/app/src/main/assets/licenses/OFL-Literata.txt similarity index 97% rename from android/app/src/main/assets/licenses/OFL-Fraunces.txt rename to android/app/src/main/assets/licenses/OFL-Literata.txt index 22fa7ce..9d3b4e3 100644 --- a/android/app/src/main/assets/licenses/OFL-Fraunces.txt +++ b/android/app/src/main/assets/licenses/OFL-Literata.txt @@ -1,4 +1,4 @@ -Copyright 2018 The Fraunces Project Authors (https://github.com/undercasetype/Fraunces) +Copyright 2017 The Literata Project Authors (https://github.com/googlefonts/literata) This Font Software is licensed under the SIL Open Font License, Version 1.1. This license is copied below, and is also available with a FAQ at: diff --git a/android/app/src/main/kotlin/cc/hafa/reader/MainActivity.kt b/android/app/src/main/kotlin/cc/hafa/reader/MainActivity.kt index c1d4713..9af932c 100644 --- a/android/app/src/main/kotlin/cc/hafa/reader/MainActivity.kt +++ b/android/app/src/main/kotlin/cc/hafa/reader/MainActivity.kt @@ -18,7 +18,7 @@ class MainActivity : ComponentActivity() { super.onCreate(savedInstanceState) enableEdgeToEdge() setContent { - ReaderTheme { + ReaderTheme(viewModel.readingFont) { ReaderScreen(viewModel) } } diff --git a/android/app/src/main/kotlin/cc/hafa/reader/ReaderApplication.kt b/android/app/src/main/kotlin/cc/hafa/reader/ReaderApplication.kt index 882856d..2ea9c9b 100644 --- a/android/app/src/main/kotlin/cc/hafa/reader/ReaderApplication.kt +++ b/android/app/src/main/kotlin/cc/hafa/reader/ReaderApplication.kt @@ -8,6 +8,7 @@ import android.os.Bundle import android.util.Log import cc.hafa.reader.core.HttpFeedFetcher import cc.hafa.reader.core.ReaderModel +import cc.hafa.reader.core.ReaderPreferences import cc.hafa.reader.core.sync.SyncEngine import cc.hafa.reader.data.ReaderDatabase import cc.hafa.reader.data.RoomReaderStore @@ -51,11 +52,14 @@ class ReaderApplication : Application() { // Set once a screen shows, so sync listens to the account; a process started only to check feeds just uploads. private val onScreen = AtomicBoolean(false) + /** The reader's small choices on this device. */ + val preferences: ReaderPreferences by lazy { SharedReaderPreferences(this) } + /** Everything the reader holds, for the whole process. */ val model: ReaderModel by lazy { ReaderModel( store = RoomReaderStore(ReaderDatabase.open(this).dao()), - preferences = SharedReaderPreferences(this), + preferences = preferences, fetcher = HttpFeedFetcher(OkHttpClient()), scope = scope, onBackgroundWork = { RefreshWorker.finishInBackground(this) }, diff --git a/android/app/src/main/kotlin/cc/hafa/reader/data/RoomReaderStore.kt b/android/app/src/main/kotlin/cc/hafa/reader/data/RoomReaderStore.kt index 38098f9..04e0b16 100644 --- a/android/app/src/main/kotlin/cc/hafa/reader/data/RoomReaderStore.kt +++ b/android/app/src/main/kotlin/cc/hafa/reader/data/RoomReaderStore.kt @@ -7,6 +7,7 @@ import cc.hafa.reader.core.Feed import cc.hafa.reader.core.FeedModel import cc.hafa.reader.core.FeedSchedule import cc.hafa.reader.core.ReaderPreferences +import cc.hafa.reader.core.ReadingFont import cc.hafa.reader.core.ReaderStore import cc.hafa.reader.core.Sort import cc.hafa.reader.core.ViewSettings @@ -172,6 +173,10 @@ class RoomReaderStore(private val dao: ReaderDao) : ReaderStore { private const val UNREAD_KEY = "unread-only" private const val VIEW_KEY = "view" +private const val READING_FONT_KEY = "reading-font" +// stored values, the same as the web's +private const val LITERATA_VALUE = "literata" +private const val PUBLIC_SANS_VALUE = "public-sans" /** The reader's small choices in shared preferences. */ class SharedReaderPreferences(context: Context) : ReaderPreferences { @@ -184,4 +189,8 @@ class SharedReaderPreferences(context: Context) : ReaderPreferences { override var lastView: String get() = shared.getString(VIEW_KEY, null) ?: "all" set(value) = shared.edit { putString(VIEW_KEY, value) } + + override var readingFont: ReadingFont + get() = if (shared.getString(READING_FONT_KEY, null) == PUBLIC_SANS_VALUE) ReadingFont.PublicSans else ReadingFont.Literata + set(value) = shared.edit { putString(READING_FONT_KEY, if (value == ReadingFont.PublicSans) PUBLIC_SANS_VALUE else LITERATA_VALUE) } } diff --git a/android/app/src/main/kotlin/cc/hafa/reader/sync/FirestoreAccountStore.kt b/android/app/src/main/kotlin/cc/hafa/reader/sync/FirestoreAccountStore.kt index 42c66e9..34b7f53 100644 --- a/android/app/src/main/kotlin/cc/hafa/reader/sync/FirestoreAccountStore.kt +++ b/android/app/src/main/kotlin/cc/hafa/reader/sync/FirestoreAccountStore.kt @@ -39,6 +39,7 @@ import com.google.firebase.firestore.Source import kotlinx.coroutines.tasks.await import java.util.concurrent.Executor import java.util.concurrent.Executors +import java.util.concurrent.atomic.AtomicBoolean // Errors the rules or the data give, which no retry fixes. private val REFUSALS = setOf( @@ -146,6 +147,8 @@ class FirestoreAccountStore(private val db: FirebaseFirestore, private val uid: } override fun listenEntries(sinceMs: Long, onChanges: (List) -> Unit, onError: (Throwable) -> Unit): Stop { + // The first snapshot is passed on even when empty, so sync knows the listener has caught up. + val heard = AtomicBoolean(false) val registration = entries.whereGreaterThan("syncedAt", timestamp(sinceMs)).addSnapshotListener(snapshotExecutor) { snapshot, error -> if (error != null) { onError(error) @@ -153,7 +156,7 @@ class FirestoreAccountStore(private val db: FirebaseFirestore, private val uid: val changes = snapshot.documentChanges .filter { change -> change.type != DocumentChange.Type.REMOVED && !change.document.metadata.hasPendingWrites() } .map { change -> EntryChange(change.document.id, parseEntry(change.document.data), syncedAt(change.document)) } - if (changes.isNotEmpty()) { + if (changes.isNotEmpty() || heard.compareAndSet(false, true)) { onChanges(changes) } } diff --git a/android/app/src/main/kotlin/cc/hafa/reader/ui/AccountScreen.kt b/android/app/src/main/kotlin/cc/hafa/reader/ui/AccountScreen.kt index 8ffd42d..613c52c 100644 --- a/android/app/src/main/kotlin/cc/hafa/reader/ui/AccountScreen.kt +++ b/android/app/src/main/kotlin/cc/hafa/reader/ui/AccountScreen.kt @@ -13,14 +13,11 @@ import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.ExperimentalLayoutApi import androidx.compose.foundation.layout.FlowRow import androidx.compose.foundation.layout.Row -import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.offset import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.size -import androidx.compose.foundation.rememberScrollState import androidx.compose.foundation.shape.CircleShape -import androidx.compose.foundation.verticalScroll import androidx.compose.material.icons.Icons import androidx.compose.material.icons.automirrored.outlined.Login import androidx.compose.material.icons.automirrored.outlined.Logout @@ -194,11 +191,7 @@ fun AccountScreen(viewModel: ReaderViewModel) { bottomBar = { scrollBehavior -> BackBottomBar(stringResource(R.string.sync_heading), scrollBehavior, viewModel::back) }, ) { padding -> Column( - Modifier - .fillMaxSize() - .padding(padding) - .verticalScroll(rememberScrollState()) - .padding(horizontal = 20.dp, vertical = 24.dp), + Modifier.scrollingScreen(padding), verticalArrangement = Arrangement.spacedBy(12.dp), ) { if (report.status == SyncStatus.Waiting) { diff --git a/android/app/src/main/kotlin/cc/hafa/reader/ui/AddFeedScreen.kt b/android/app/src/main/kotlin/cc/hafa/reader/ui/AddFeedScreen.kt index e7802ac..a01b400 100644 --- a/android/app/src/main/kotlin/cc/hafa/reader/ui/AddFeedScreen.kt +++ b/android/app/src/main/kotlin/cc/hafa/reader/ui/AddFeedScreen.kt @@ -7,17 +7,14 @@ import androidx.compose.foundation.clickable import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.Row -import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.heightIn import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.size import androidx.compose.foundation.layout.width -import androidx.compose.foundation.rememberScrollState import androidx.compose.foundation.shape.RoundedCornerShape import androidx.compose.foundation.text.KeyboardActions import androidx.compose.foundation.text.KeyboardOptions -import androidx.compose.foundation.verticalScroll import androidx.compose.material.icons.Icons import androidx.compose.material.icons.automirrored.outlined.Login import androidx.compose.material.icons.filled.Menu @@ -121,11 +118,7 @@ fun AddFeedScreen(snapshot: Snapshot, viewModel: ReaderViewModel) { bottomBar = { scrollBehavior -> BackBottomBar(stringResource(R.string.add_feed), scrollBehavior, viewModel::back) }, ) { padding -> Column( - Modifier - .fillMaxSize() - .padding(padding) - .verticalScroll(rememberScrollState()) - .padding(horizontal = 20.dp, vertical = 24.dp), + Modifier.scrollingScreen(padding), verticalArrangement = Arrangement.spacedBy(24.dp), ) { AddFeedForm(snapshot, viewModel, first = false) @@ -151,11 +144,7 @@ fun FirstFeedScreen(snapshot: Snapshot, viewModel: ReaderViewModel, onMenu: () - val arriving = report.email.isNotEmpty() && !report.feedsKnown && report.status in setOf(SyncStatus.Connecting, SyncStatus.Syncing, SyncStatus.Synced) Column( - Modifier - .fillMaxSize() - .padding(padding) - .verticalScroll(rememberScrollState()) - .padding(24.dp), + Modifier.scrollingScreen(padding, margin = 24.dp), verticalArrangement = Arrangement.Center, ) { if (arriving) { @@ -186,6 +175,8 @@ private fun AddFeedForm(snapshot: Snapshot, viewModel: ReaderViewModel, first: B val adding = snapshot.state.adding // a topic search, as opposed to adding an address or a file var searching by remember { mutableStateOf(false) } + // the group imported feeds go into; blank keeps the file's folders + var importGroup by remember { mutableStateOf("") } // When an address could not be added and was searched for instead, finding nothing gives the address's problem. fun search(text: String, problem: FeedProblem? = null) { @@ -242,7 +233,7 @@ private fun AddFeedForm(snapshot: Snapshot, viewModel: ReaderViewModel, first: B if (source == null) { viewModel.notify(Notice.Text(R.string.import_failed)) } else { - viewModel.importOpml(source) + viewModel.importOpml(source, importGroup.trim().ifEmpty { null }) } } } @@ -269,7 +260,7 @@ private fun AddFeedForm(snapshot: Snapshot, viewModel: ReaderViewModel, first: B Column(verticalArrangement = Arrangement.spacedBy(24.dp)) { if (first) { Column(verticalArrangement = Arrangement.spacedBy(8.dp)) { - Text(stringResource(R.string.empty_title), fontFamily = Fraunces, fontWeight = FontWeight.Bold, fontSize = 24.sp) + Text(stringResource(R.string.empty_title), fontFamily = ReaderTheme.readingFamily, fontWeight = FontWeight.Bold, fontSize = 24.sp) Text(stringResource(R.string.empty_body), fontSize = 14.sp, lineHeight = 20.sp, color = colors.muted) } } @@ -349,8 +340,16 @@ private fun AddFeedForm(snapshot: Snapshot, viewModel: ReaderViewModel, first: B } HorizontalDivider(color = colors.line) Column(verticalArrangement = Arrangement.spacedBy(12.dp)) { - Text(stringResource(R.string.import_title), fontFamily = Fraunces, fontWeight = FontWeight.Bold, fontSize = 18.sp) + Text(stringResource(R.string.import_title), fontFamily = ReaderTheme.readingFamily, fontWeight = FontWeight.Bold, fontSize = 18.sp) Text(stringResource(R.string.import_body), fontSize = 14.sp, lineHeight = 20.sp, color = colors.muted) + GroupField( + importGroup, + snapshot.view.folders, + label = stringResource(R.string.import_group_label), + placeholder = stringResource(R.string.import_keep_folders), + enabled = !adding, + onEdit = { group -> importGroup = group }, + ) val shape = RoundedCornerShape(12.dp) Column( Modifier diff --git a/android/app/src/main/kotlin/cc/hafa/reader/ui/ArticleScreen.kt b/android/app/src/main/kotlin/cc/hafa/reader/ui/ArticleScreen.kt index 9afcc90..b85251d 100644 --- a/android/app/src/main/kotlin/cc/hafa/reader/ui/ArticleScreen.kt +++ b/android/app/src/main/kotlin/cc/hafa/reader/ui/ArticleScreen.kt @@ -13,8 +13,6 @@ import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.BoxWithConstraints import androidx.compose.foundation.layout.Column -import androidx.compose.foundation.layout.ExperimentalLayoutApi -import androidx.compose.foundation.layout.FlowRow import androidx.compose.foundation.layout.IntrinsicSize import androidx.compose.foundation.layout.PaddingValues import androidx.compose.foundation.layout.Row @@ -93,8 +91,8 @@ import androidx.compose.ui.unit.sp import cc.hafa.reader.R import cc.hafa.reader.core.Entry import cc.hafa.reader.core.Selection +import cc.hafa.reader.core.articleTopics import cc.hafa.reader.core.decodeHtmlEntities -import cc.hafa.reader.core.ViewSettings import cc.hafa.reader.core.showsLeadImage import cc.hafa.reader.core.topicOff import coil3.SingletonImageLoader @@ -306,21 +304,12 @@ private fun ArticlePage( start = padding.calculateStartPadding(layoutDirection) + side, end = padding.calculateEndPadding(layoutDirection) + side, top = 24.dp, - bottom = padding.calculateBottomPadding() + 32.dp, + bottom = scrollEndPadding(), ), verticalArrangement = Arrangement.spacedBy(16.dp), ) { item(key = "head") { Column(verticalArrangement = Arrangement.spacedBy(12.dp)) { - val dateText = if (entry.dated) articleDate(entry.published) else "" - val feedTitle = state.feeds.firstOrNull { feed -> feed.id == entry.feedId }?.title ?: "" - val meta = listOf(feedTitle, entry.author, dateText).filter { part -> part.isNotEmpty() }.joinToString(" · ") - Text( - meta, - fontFamily = FontFamily.Monospace, - fontSize = 12.sp, lineHeight = 16.sp, - color = colors.muted, - ) val title = remember(entry.title, entry.url, colors) { if (entry.url.isEmpty()) { AnnotatedString(entry.title) @@ -335,12 +324,21 @@ private fun ArticlePage( } Text( title, - fontFamily = Fraunces, + fontFamily = ReaderTheme.readingFamily, fontWeight = FontWeight.Bold, fontSize = 30.sp, lineHeight = 36.sp, style = LocalTextStyle.current.copy(lineBreak = LineBreak.Heading), ) + val dateText = if (entry.dated) articleDate(entry.published) else "" + val feedTitle = state.feeds.firstOrNull { feed -> feed.id == entry.feedId }?.title ?: "" + val meta = listOf(feedTitle, entry.author, dateText).filter { part -> part.isNotEmpty() }.joinToString(" · ") + Text( + meta, + fontFamily = FontFamily.Monospace, + fontSize = 12.sp, lineHeight = 16.sp, + color = colors.muted, + ) } } if (showImage) { @@ -360,13 +358,18 @@ private fun ArticlePage( // The actions and topics wait for the body, so they do not show under the title and then jump down. if (split != null && entry.categories.isNotEmpty()) { item(key = "categories") { + val topics = remember(entry.categories, view.topicOrder) { articleTopics(entry.categories, view.topicOrder) } // Starred has no topic filter, so there the topics are plain labels. if (state.selection == Selection.Starred) { - Text(entry.categories.joinToString(" · "), fontSize = 12.sp, lineHeight = 16.sp, color = colors.muted) + ChipRows(topics) { topic -> TopicLabel(topic) } } else { // The chips read the view's settings, which change at once; only the list waits for a refresh. - CategoryChips(entry.categories, view.viewSettings) { topic -> - viewModel.model.toggleTopic(topic, fromArticle = true) + ChipRows(topics) { topic -> + TopicChip( + topic, + off = topicOff(view.viewSettings, topic), + onToggle = { viewModel.model.toggleTopic(topic, fromArticle = true) }, + ) } } } @@ -375,17 +378,6 @@ private fun ArticlePage( } } -/** The article's topics as filter chips; turning one off hides its articles from the view. */ -@OptIn(ExperimentalLayoutApi::class) -@Composable -private fun CategoryChips(categories: List, settings: ViewSettings, onToggle: (String) -> Unit) { - FlowRow(horizontalArrangement = Arrangement.spacedBy(8.dp)) { - for (category in categories) { - TopicChip(category, off = topicOff(settings, category), onToggle = { onToggle(category) }) - } - } -} - /** * The drag past the first or last page, which the pager leaves unused: the * pages follow it, and on release one of at least [threshold] pixels slides @@ -470,7 +462,7 @@ private fun BodyBlock(block: HtmlBlock) { val links = TextLinkStyles(SpanStyle(color = colors.accent)) val text = remember(block.html, links) { AnnotatedString.fromHtml(block.html, linkStyles = links) } val quoted = block.quoteDepth > 0 || block.kind == TextKind.Caption - val style = bodyStyle(block) + val style = bodyStyle(block, ReaderTheme.readingFamily) Row( Modifier .fillMaxWidth() @@ -530,8 +522,8 @@ private val HEADING_STYLES = listOf( TextStyle(fontSize = 16.sp, lineHeight = 24.sp, fontWeight = FontWeight.SemiBold), ) -private fun bodyStyle(block: HtmlBlock.Text): TextStyle { - val base = TextStyle(fontFamily = PublicSans) +private fun bodyStyle(block: HtmlBlock.Text, family: FontFamily): TextStyle { + val base = TextStyle(fontFamily = family) return when (block.kind) { TextKind.Heading -> base.merge(HEADING_STYLES[(block.level - 1).coerceIn(0, HEADING_STYLES.size - 1)]) TextKind.Caption -> base.copy(fontSize = 14.sp, lineHeight = 20.sp, fontStyle = FontStyle.Italic) @@ -561,6 +553,7 @@ private fun BodyTable(table: HtmlBlock.Table) { .fillMaxHeight() .border(0.5.dp, colors.line) .padding(horizontal = 8.dp, vertical = 6.dp), + fontFamily = ReaderTheme.readingFamily, fontSize = 14.sp, lineHeight = 20.sp, fontWeight = if (cell.header) FontWeight.SemiBold else FontWeight.Normal, diff --git a/android/app/src/main/kotlin/cc/hafa/reader/ui/ChipRows.kt b/android/app/src/main/kotlin/cc/hafa/reader/ui/ChipRows.kt new file mode 100644 index 0000000..e4bdb11 --- /dev/null +++ b/android/app/src/main/kotlin/cc/hafa/reader/ui/ChipRows.kt @@ -0,0 +1,74 @@ +package cc.hafa.reader.ui + +import androidx.compose.foundation.horizontalScroll +import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.BoxWithConstraints +import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.rememberScrollState +import androidx.compose.runtime.Composable +import androidx.compose.ui.Modifier +import androidx.compose.ui.layout.Layout +import androidx.compose.ui.semantics.isTraversalGroup +import androidx.compose.ui.semantics.semantics +import androidx.compose.ui.semantics.traversalIndex +import androidx.compose.ui.unit.Constraints +import androidx.compose.ui.unit.IntOffset +import androidx.compose.ui.unit.dp +import cc.hafa.reader.core.TOPIC_ROWS +import cc.hafa.reader.core.rowCount +import cc.hafa.reader.core.wrappedRows + +/** + * A [chip] for each of [items], laid left to right in up to [TOPIC_ROWS] rows; when they need more, they go in that + * many rows read down each column, and the block scrolls sideways. Each chip sits in its own traversal group, ordered + * by its place in [items], so screen readers go through them in that order either way. Every list of topics or words + * is shown this way. + */ +@Composable +fun ChipRows(items: List, chip: @Composable (Item) -> Unit) { + BoxWithConstraints(Modifier.fillMaxWidth()) { + val available = constraints.maxWidth + val content = @Composable { + items.forEachIndexed { index, item -> + Box( + Modifier.semantics { + isTraversalGroup = true + traversalIndex = index.toFloat() + }, + ) { chip(item) } + } + } + Layout(content, Modifier.semantics { isTraversalGroup = true }.horizontalScroll(rememberScrollState())) { measurables, _ -> + val gapPx = 8.dp.roundToPx() + val placeables = measurables.map { measurable -> measurable.measure(Constraints()) } + val wrapped = wrappedRows(placeables.map { placeable -> placeable.width }, available, gapPx) + val positions: List + val width: Int + val height: Int + if (rowCount(wrapped) <= TOPIC_ROWS) { + val rowHeights = IntArray(rowCount(wrapped)) + wrapped.forEachIndexed { index, position -> + rowHeights[position.row] = maxOf(rowHeights[position.row], placeables[index].height) + } + val rowTops = rowHeights.runningFold(0) { top, rowHeight -> top + rowHeight } + positions = wrapped.map { position -> IntOffset(position.left, rowTops[position.row]) } + width = wrapped.indices.maxOfOrNull { index -> wrapped[index].left + placeables[index].width } ?: 0 + height = rowTops.last() + } else { + val rowHeight = placeables.maxOf { placeable -> placeable.height } + val columnPositions = ArrayList(placeables.size) + var left = 0 + for (column in placeables.chunked(TOPIC_ROWS)) { + column.forEachIndexed { row, _ -> columnPositions.add(IntOffset(left, row * rowHeight)) } + left += column.maxOf { placeable -> placeable.width } + gapPx + } + positions = columnPositions + width = left - gapPx + height = rowHeight * TOPIC_ROWS + } + layout(width, height) { + placeables.forEachIndexed { index, placeable -> placeable.place(positions[index]) } + } + } + } +} diff --git a/android/app/src/main/kotlin/cc/hafa/reader/ui/Components.kt b/android/app/src/main/kotlin/cc/hafa/reader/ui/Components.kt index 0f81030..66214ca 100644 --- a/android/app/src/main/kotlin/cc/hafa/reader/ui/Components.kt +++ b/android/app/src/main/kotlin/cc/hafa/reader/ui/Components.kt @@ -4,7 +4,15 @@ import android.content.res.Resources import androidx.compose.animation.core.animate import androidx.compose.foundation.BorderStroke import androidx.compose.foundation.background +import androidx.compose.foundation.border +import androidx.compose.foundation.clickable import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.ExperimentalLayoutApi +import androidx.compose.foundation.layout.asPaddingValues +import androidx.compose.foundation.layout.calculateEndPadding +import androidx.compose.foundation.layout.calculateStartPadding +import androidx.compose.foundation.layout.isImeVisible +import androidx.compose.foundation.layout.navigationBars import androidx.compose.foundation.layout.PaddingValues import androidx.compose.foundation.layout.RowScope import androidx.compose.foundation.layout.Spacer @@ -12,13 +20,17 @@ import androidx.compose.foundation.layout.safeDrawing import androidx.compose.foundation.layout.width import androidx.compose.foundation.layout.WindowInsets import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.heightIn import androidx.compose.foundation.layout.ime +import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.union import androidx.compose.foundation.layout.statusBars import androidx.compose.foundation.layout.windowInsetsTopHeight import androidx.compose.foundation.layout.size +import androidx.compose.foundation.rememberScrollState import androidx.compose.foundation.shape.RoundedCornerShape +import androidx.compose.foundation.verticalScroll import androidx.compose.material.icons.Icons import androidx.compose.material.icons.automirrored.outlined.ArrowBack import androidx.compose.material3.BottomAppBar @@ -55,10 +67,13 @@ import androidx.compose.ui.draw.clip import androidx.compose.ui.graphics.Color import androidx.compose.ui.graphics.vector.ImageVector import androidx.compose.ui.input.nestedscroll.nestedScroll +import androidx.compose.ui.platform.LocalLayoutDirection import androidx.compose.ui.platform.LocalResources import androidx.compose.ui.res.stringResource +import androidx.compose.ui.semantics.Role import androidx.compose.ui.semantics.heading import androidx.compose.ui.semantics.semantics +import androidx.compose.ui.semantics.stateDescription import androidx.compose.ui.text.font.FontFamily import androidx.compose.ui.text.font.FontWeight import androidx.compose.ui.text.style.TextDecoration @@ -82,7 +97,11 @@ private const val NOT_FOUND = 404 private const val GONE = 410 private const val TOO_MANY_REQUESTS = 429 -/** An icon button labeled for screen readers; a toggle that reads as on or off when [pressed] is given. */ +/** + * An icon button labeled for screen readers; a toggle that reads as on or off + * when [pressed] is given. [dotted] puts a due dot on the icon's corner (see + * [WithDueDot]) and has screen readers say a refresh is suggested. + */ @Composable fun IconAction( icon: ImageVector, @@ -91,18 +110,26 @@ fun IconAction( tint: Color = ReaderTheme.colors.muted, iconSize: Dp = 24.dp, pressed: Boolean? = null, + dotted: Boolean = false, ) { + val suggestion = stringResource(R.string.refresh_suggested) + val announced = Modifier.semantics { + if (dotted) { + stateDescription = suggestion + } + } if (pressed == null) { - IconButton(onClick = onClick, colors = IconButtonDefaults.iconButtonColors(contentColor = tint)) { - Icon(icon, contentDescription = label, modifier = Modifier.size(iconSize)) + IconButton(onClick = onClick, modifier = announced, colors = IconButtonDefaults.iconButtonColors(contentColor = tint)) { + WithDueDot(dotted) { Icon(icon, contentDescription = label, modifier = Modifier.size(iconSize)) } } } else { IconToggleButton( checked = pressed, onCheckedChange = { onClick() }, + modifier = announced, colors = IconButtonDefaults.iconToggleButtonColors(contentColor = tint, checkedContentColor = tint), ) { - Icon(icon, contentDescription = label, modifier = Modifier.size(iconSize)) + WithDueDot(dotted) { Icon(icon, contentDescription = label, modifier = Modifier.size(iconSize)) } } } } @@ -161,6 +188,51 @@ fun ReaderScaffold( ) } +// Material's bottom app bar height, without the system bars under it. +private val BOTTOM_BAR_HEIGHT = 80.dp + +// Space past the end of a screen's scrolling content, above the bottom bar. +private val SCROLL_END_MARGIN = 32.dp + +/** + * The space below a screen's scrolling content: the bottom bar's full height, + * the system bars under it, and a margin. It stays the same while the bar + * slides away, so the end clears the bar and there is always room to scroll + * back and bring the bar back. + */ +@Composable +fun scrollEndPadding(): Dp = + BOTTOM_BAR_HEIGHT + WindowInsets.navigationBars.asPaddingValues().calculateBottomPadding() + SCROLL_END_MARGIN + +/** + * The outer padding of a screen's scrolling column, from the scaffold's + * [padding]: the column runs on under the bottom bar, so its scrolling room + * does not change as the bar slides away, except while typing, when it stops + * above the keyboard and the bar so the field in use stays in sight. + */ +@OptIn(ExperimentalLayoutApi::class) +@Composable +private fun underBarPadding(padding: PaddingValues): PaddingValues { + val layoutDirection = LocalLayoutDirection.current + return PaddingValues( + start = padding.calculateStartPadding(layoutDirection), + top = padding.calculateTopPadding(), + end = padding.calculateEndPadding(layoutDirection), + bottom = if (WindowInsets.isImeVisible) padding.calculateBottomPadding() else 0.dp, + ) +} + +/** + * The modifier of a screen's full-size scrolling column, from the scaffold's [padding]: [underBarPadding] outside the + * scroll, then [margin] at the sides, a top margin and [scrollEndPadding] at the end inside it. + */ +@Composable +fun Modifier.scrollingScreen(padding: PaddingValues, margin: Dp = 20.dp): Modifier = + fillMaxSize() + .padding(underBarPadding(padding)) + .verticalScroll(rememberScrollState()) + .padding(start = margin, top = 24.dp, end = margin, bottom = scrollEndPadding()) + /** Slide a hidden bottom bar back into view. */ @OptIn(ExperimentalMaterial3Api::class) suspend fun BottomAppBarScrollBehavior.showBar() { @@ -177,20 +249,36 @@ fun BackBottomBar(title: String, scrollBehavior: BottomAppBarScrollBehavior, onB } } -/** A screen's name in its bottom bar, taking the room the buttons leave. */ +/** + * A screen's name in its bottom bar, taking the room the buttons leave; with + * [onClick], tapping it does what [clickLabel] says. + */ @Composable -fun RowScope.BarTitle(title: String) { - Text( - title, - modifier = Modifier +fun RowScope.BarTitle(title: String, onClick: (() -> Unit)? = null, clickLabel: String? = null) { + val tappable = if (onClick == null) { + Modifier + } else { + Modifier + .heightIn(min = 48.dp) + .clip(RoundedCornerShape(8.dp)) + .clickable(onClickLabel = clickLabel, role = Role.Button, onClick = onClick) + } + Box( + Modifier .weight(1f) - .padding(horizontal = 8.dp) - .semantics { heading() }, - fontSize = 18.sp, - fontWeight = FontWeight.SemiBold, - maxLines = 1, - overflow = TextOverflow.Ellipsis, - ) + .then(tappable) + .padding(horizontal = 8.dp), + contentAlignment = Alignment.CenterStart, + ) { + Text( + title, + modifier = Modifier.semantics { heading() }, + fontSize = 18.sp, + fontWeight = FontWeight.SemiBold, + maxLines = 1, + overflow = TextOverflow.Ellipsis, + ) + } } /** The page color behind the status bar, for screens whose bar is at the bottom, so content scrolls under it unseen. */ @@ -246,7 +334,7 @@ fun SiteIcon(source: String, title: String, modifier: Modifier = Modifier, size: } } -/** A bordered button in the text color, with an optional leading [icon], drawn with [iconModifier]. */ +/** A bordered button in the text color, with an optional leading [icon]. */ @Composable fun OutlineButton( label: String, @@ -254,7 +342,6 @@ fun OutlineButton( modifier: Modifier = Modifier, icon: ImageVector? = null, enabled: Boolean = true, - iconModifier: Modifier = Modifier, ) { val colors = ReaderTheme.colors OutlinedButton( @@ -267,7 +354,7 @@ fun OutlineButton( contentPadding = if (icon == null) ButtonDefaults.ContentPadding else ButtonDefaults.ButtonWithIconContentPadding, ) { if (icon != null) { - Icon(icon, null, modifier = iconModifier.size(ButtonDefaults.IconSize)) + Icon(icon, null, modifier = Modifier.size(ButtonDefaults.IconSize)) Spacer(Modifier.width(ButtonDefaults.IconSpacing)) } Text(label) @@ -306,7 +393,10 @@ fun ConfirmButton(label: String, confirmLabel: String, confirming: Boolean, onCl } } -/** A topic as a filter chip, struck through while [off]; [count] shows after it when given. */ +/** + * A topic as a filter chip, struck through while [off]; [count] shows after it when given. The one chip for topics in + * the settings and under an article; Material keeps its touch area at least 48dp tall. + */ @Composable fun TopicChip(topic: String, off: Boolean, onToggle: () -> Unit, count: Int? = null) { val colors = ReaderTheme.colors @@ -333,7 +423,19 @@ fun TopicChip(topic: String, off: Boolean, onToggle: () -> Unit, count: Int? = n ) } -/** A row of choices of which exactly one is on. */ +/** A topic in a chip's outline that does nothing when pressed, for views without a topic filter. */ +@Composable +fun TopicLabel(topic: String) { + val colors = ReaderTheme.colors + Box( + Modifier.heightIn(min = 32.dp).border(1.dp, colors.line, FilterChipDefaults.shape).padding(horizontal = 12.dp), + contentAlignment = Alignment.Center, + ) { + Text(topic, style = MaterialTheme.typography.labelLarge, color = colors.text) + } +} + +/** A row of choices of which exactly one is on, each at least 48dp tall. */ @Composable fun Segmented(options: List>, value: Value, onChange: (Value) -> Unit, modifier: Modifier = Modifier) { val colors = ReaderTheme.colors @@ -343,6 +445,8 @@ fun Segmented(options: List>, value: Value, onChange selected = option == value, onClick = { onChange(option) }, shape = SegmentedButtonDefaults.itemShape(index, options.size), + // Material draws these 40dp tall; a finger needs 48. + modifier = Modifier.heightIn(min = 48.dp), colors = SegmentedButtonDefaults.colors( activeContainerColor = colors.accentSoft, activeContentColor = colors.text, @@ -364,6 +468,8 @@ fun problemText(resources: Resources, problem: FeedProblem): String = FeedProblem.NotAFeed -> resources.getString(R.string.error_not_a_feed) FeedProblem.NotWebAddress -> resources.getString(R.string.error_not_https) FeedProblem.NotHttps -> resources.getString(R.string.error_not_https_feed) + FeedProblem.HttpRefused -> resources.getString(R.string.error_http_refused) + FeedProblem.HttpFeed -> resources.getString(R.string.error_http_feed) FeedProblem.Unreadable -> resources.getString(R.string.error_unreadable) is FeedProblem.Status -> when (problem.status) { UNAUTHORIZED, FORBIDDEN -> resources.getString(R.string.error_refused) diff --git a/android/app/src/main/kotlin/cc/hafa/reader/ui/DueDot.kt b/android/app/src/main/kotlin/cc/hafa/reader/ui/DueDot.kt new file mode 100644 index 0000000..4e5aebf --- /dev/null +++ b/android/app/src/main/kotlin/cc/hafa/reader/ui/DueDot.kt @@ -0,0 +1,61 @@ +package cc.hafa.reader.ui + +import androidx.compose.foundation.background +import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.offset +import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.layout.size +import androidx.compose.foundation.shape.CircleShape +import androidx.compose.runtime.Composable +import androidx.compose.runtime.getValue +import androidx.compose.runtime.produceState +import androidx.compose.runtime.remember +import androidx.compose.ui.Alignment +import androidx.compose.ui.Modifier +import androidx.compose.ui.unit.dp +import cc.hafa.reader.core.ReaderState +import cc.hafa.reader.core.refreshSuggestedAt +import kotlinx.coroutines.delay + +/** + * Whether a refresh would help now (see [refreshSuggestedAt]). The time is + * worked out again only when the feeds or the checking marks change, and one + * wait turns it on when that time comes. + */ +@Composable +fun refreshSuggested(state: ReaderState): Boolean { + val suggestedAt = remember(state.feeds, state.refreshing, state.checking) { refreshSuggestedAt(state) } + val suggested by produceState(initialValue = false, suggestedAt) { + val wait = suggestedAt - System.currentTimeMillis() + value = wait <= 0 + if (wait > 0 && suggestedAt != Long.MAX_VALUE) { + delay(wait) + value = true + } + } + return suggested +} + +/** + * [content] with, when [shown], a small accent dot on its top corner saying a + * refresh would help, ringed in the bar's color; the web's `due-dot.svelte`. + * Decorative: the control it marks says it in words. + */ +@Composable +fun WithDueDot(shown: Boolean, content: @Composable () -> Unit) { + Box { + content() + if (shown) { + val colors = ReaderTheme.colors + Box( + Modifier + .align(Alignment.TopEnd) + .offset(x = 4.dp, y = (-4).dp) + .size(12.dp) + .background(colors.side, CircleShape) + .padding(2.dp) + .background(colors.accent, CircleShape), + ) + } + } +} diff --git a/android/app/src/main/kotlin/cc/hafa/reader/ui/EntryList.kt b/android/app/src/main/kotlin/cc/hafa/reader/ui/EntryList.kt index c86f28f..6d0641c 100644 --- a/android/app/src/main/kotlin/cc/hafa/reader/ui/EntryList.kt +++ b/android/app/src/main/kotlin/cc/hafa/reader/ui/EntryList.kt @@ -6,7 +6,6 @@ import androidx.compose.animation.core.animateFloat import androidx.compose.animation.core.infiniteRepeatable import androidx.compose.animation.core.rememberInfiniteTransition import androidx.compose.animation.core.tween -import androidx.compose.foundation.Image import androidx.compose.foundation.background import androidx.compose.foundation.clickable import androidx.compose.foundation.layout.Arrangement @@ -20,7 +19,6 @@ import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.heightIn import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.size -import androidx.compose.foundation.layout.width import androidx.compose.foundation.lazy.LazyColumn import androidx.compose.foundation.lazy.LazyListState import androidx.compose.foundation.lazy.items @@ -57,8 +55,8 @@ import androidx.compose.ui.draw.clip import androidx.compose.ui.draw.rotate import androidx.compose.ui.layout.ContentScale import androidx.compose.ui.platform.LocalLayoutDirection -import androidx.compose.ui.res.painterResource import androidx.compose.ui.res.stringResource +import androidx.compose.ui.semantics.Role import androidx.compose.ui.text.font.FontFamily import androidx.compose.ui.text.font.FontWeight import androidx.compose.ui.text.style.TextOverflow @@ -68,6 +66,7 @@ import cc.hafa.reader.R import cc.hafa.reader.core.Entry import cc.hafa.reader.core.Selection import cc.hafa.reader.core.ViewTitle +import cc.hafa.reader.core.articleTopics import coil3.compose.AsyncImage import kotlinx.coroutines.launch import java.time.Instant @@ -97,8 +96,9 @@ fun EntryList(snapshot: Snapshot, viewModel: ReaderViewModel, listState: LazyLis ReaderScaffold( bottomBar = { scrollBehavior -> ReaderBottomBar(scrollBehavior) { - IconAction(Icons.Filled.Menu, stringResource(R.string.feeds), onMenu, tint = colors.text) - BarTitle(viewTitleText(view.viewTitle)) + val suggested = refreshSuggested(state) + IconAction(Icons.Filled.Menu, stringResource(R.string.feeds), onMenu, tint = colors.text, dotted = suggested) + BarTitle(viewTitleText(view.viewTitle), onClick = onMenu, clickLabel = stringResource(R.string.show_feeds)) IconAction( if (state.unreadOnly) Icons.Outlined.VisibilityOff else Icons.Outlined.Visibility, stringResource(R.string.unread_only), @@ -125,7 +125,7 @@ fun EntryList(snapshot: Snapshot, viewModel: ReaderViewModel, listState: LazyLis contentPadding = PaddingValues( start = padding.calculateStartPadding(layoutDirection), end = padding.calculateEndPadding(layoutDirection), - bottom = padding.calculateBottomPadding(), + bottom = scrollEndPadding(), ), ) { val problem = shownFeed?.error ?: "" @@ -136,6 +136,7 @@ fun EntryList(snapshot: Snapshot, viewModel: ReaderViewModel, listState: LazyLis EntryCard( entry = entry, source = feedTitles[entry.feedId] ?: "", + topics = remember(entry.categories, view.topicOrder) { articleTopics(entry.categories, view.topicOrder) }, active = entry.id == state.currentId, onOpen = { viewModel.openEntry(entry.id) }, onToggleRead = { model.toggleReadFor(entry.id) }, @@ -153,9 +154,8 @@ fun EntryList(snapshot: Snapshot, viewModel: ReaderViewModel, listState: LazyLis .fillParentMaxSize() .padding(32.dp), horizontalAlignment = Alignment.CenterHorizontally, - verticalArrangement = Arrangement.spacedBy(12.dp, Alignment.CenterVertically), + verticalArrangement = Arrangement.Center, ) { - Image(painterResource(R.drawable.sleeping_dot), contentDescription = null, modifier = Modifier.width(160.dp)) Text(stringResource(R.string.no_entries), fontSize = 14.sp, lineHeight = 20.sp, color = colors.muted) } } @@ -169,9 +169,10 @@ fun EntryList(snapshot: Snapshot, viewModel: ReaderViewModel, listState: LazyLis private const val FULL_TURN = 360f private const val TURN_MS = 1000 -/** The end of the list: a note and a button that checks the view's feeds as pulling down does; its icon turns while they are checked. */ +/** The end of the list: a full-width row that checks the view's feeds as pulling down does; its icon turns while they are checked. */ @Composable private fun ListEnd(refreshing: Boolean, onRefresh: () -> Unit) { + val colors = ReaderTheme.colors val turning = refreshing && animationsOn() val angle = if (turning) { rememberInfiniteTransition(label = "refresh").animateFloat( @@ -183,22 +184,21 @@ private fun ListEnd(refreshing: Boolean, onRefresh: () -> Unit) { } else { 0f } - Column( - Modifier - .fillMaxWidth() - .padding(horizontal = 16.dp, vertical = 24.dp), - horizontalAlignment = Alignment.CenterHorizontally, - verticalArrangement = Arrangement.spacedBy(12.dp), - ) { - Text(stringResource(R.string.end_of_list), fontSize = 14.sp, lineHeight = 20.sp, color = ReaderTheme.colors.muted) - OutlineButton( - stringResource(R.string.refresh), - onClick = onRefresh, - modifier = Modifier.heightIn(min = 48.dp), - icon = Icons.Outlined.Refresh, - enabled = !refreshing, - iconModifier = Modifier.rotate(angle), - ) + val content = if (refreshing) colors.muted else colors.text + Column { + Row( + Modifier + .fillMaxWidth() + .heightIn(min = 56.dp) + .clickable(enabled = !refreshing, role = Role.Button, onClick = onRefresh) + .padding(horizontal = 16.dp), + horizontalArrangement = Arrangement.spacedBy(8.dp, Alignment.CenterHorizontally), + verticalAlignment = Alignment.CenterVertically, + ) { + Icon(Icons.Outlined.Refresh, contentDescription = null, tint = content, modifier = Modifier.size(18.dp).rotate(angle)) + Text(stringResource(R.string.refresh), fontSize = 14.sp, lineHeight = 20.sp, color = content) + } + HorizontalDivider(color = colors.line) } } @@ -240,6 +240,7 @@ private fun age(published: Long): String { private fun EntryCard( entry: Entry, source: String, + topics: List, active: Boolean, onOpen: () -> Unit, onToggleRead: () -> Unit, @@ -326,7 +327,7 @@ private fun EntryCard( } Row(verticalAlignment = Alignment.CenterVertically) { Text( - entry.categories.joinToString(" · "), + topics.joinToString(" · "), modifier = Modifier.weight(1f), fontSize = 12.sp, lineHeight = 16.sp, color = colors.read, diff --git a/android/app/src/main/kotlin/cc/hafa/reader/ui/ReaderScreen.kt b/android/app/src/main/kotlin/cc/hafa/reader/ui/ReaderScreen.kt index f2b1e52..14201a7 100644 --- a/android/app/src/main/kotlin/cc/hafa/reader/ui/ReaderScreen.kt +++ b/android/app/src/main/kotlin/cc/hafa/reader/ui/ReaderScreen.kt @@ -21,14 +21,18 @@ import androidx.compose.runtime.remember import androidx.compose.runtime.rememberCoroutineScope import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier +import androidx.compose.ui.input.pointer.PointerEventPass +import androidx.compose.ui.input.pointer.pointerInput import androidx.compose.ui.platform.LocalResources import androidx.compose.ui.platform.LocalUriHandler import androidx.compose.ui.platform.UriHandler import androidx.compose.ui.unit.dp import androidx.lifecycle.compose.collectAsStateWithLifecycle import cc.hafa.reader.R +import cc.hafa.reader.core.GatePhase import cc.hafa.reader.core.viewKey import kotlinx.coroutines.launch +import java.text.NumberFormat /** * The phone layout: the article list, with the feed list in a drawer opened @@ -39,6 +43,7 @@ import kotlinx.coroutines.launch @Composable fun ReaderScreen(viewModel: ReaderViewModel) { val snapshot by viewModel.snapshot.collectAsStateWithLifecycle() + val gate by viewModel.gate.collectAsStateWithLifecycle() val colors = ReaderTheme.colors val drawerState = rememberDrawerState(DrawerValue.Closed) val scope = rememberCoroutineScope() @@ -54,6 +59,7 @@ fun ReaderScreen(viewModel: ReaderViewModel) { viewModel.notices.collect { notice -> val message = when (notice) { is Notice.Text -> resources.getString(notice.text) + is Notice.Count -> resources.getQuantityString(notice.text, notice.count, NumberFormat.getInstance().format(notice.count)) is Notice.Problem -> problemText(resources, notice.problem) } snackbarHostState.showSnackbar(message, withDismissAction = true) @@ -65,14 +71,7 @@ fun ReaderScreen(viewModel: ReaderViewModel) { .fillMaxSize() .background(colors.page), ) { - if (current == null || !current.state.loaded) { - Logo( - Modifier - .size(48.dp) - .align(Alignment.Center), - loading = true, - ) - } else { + if (current != null && current.state.loaded) { val state = current.state val listKey = "${viewKey(state.selection)}|${state.unreadOnly}|${current.view.viewSettings.sort}" LaunchedEffect(listKey) { listState.scrollToItem(0) } @@ -95,6 +94,33 @@ fun ReaderScreen(viewModel: ReaderViewModel) { ScreenContent(current, viewModel, listState) { scope.launch { drawerState.open() } } } } + if (gate.blocks) { + LoadingCover(showLogo = gate.phase == GatePhase.Shown) + // Back still leaves the app from the list, but does not move between screens behind the cover. + BackHandler(enabled = drawerState.isOpen || viewModel.screen == Screen.Detail) {} + } + } + } +} + +/** The loading screen: an opaque cover that takes every touch, so nothing half loaded shows or can be tapped, with the turning logo once [showLogo]. */ +@Composable +private fun LoadingCover(showLogo: Boolean) { + Box( + Modifier + .fillMaxSize() + .background(ReaderTheme.colors.page) + .pointerInput(Unit) { + awaitPointerEventScope { + while (true) { + awaitPointerEvent(PointerEventPass.Initial).changes.forEach { change -> change.consume() } + } + } + }, + contentAlignment = Alignment.Center, + ) { + if (showLogo) { + Logo(Modifier.size(48.dp), loading = true) } } } diff --git a/android/app/src/main/kotlin/cc/hafa/reader/ui/ReaderViewModel.kt b/android/app/src/main/kotlin/cc/hafa/reader/ui/ReaderViewModel.kt index 9bfc7d4..51c6c31 100644 --- a/android/app/src/main/kotlin/cc/hafa/reader/ui/ReaderViewModel.kt +++ b/android/app/src/main/kotlin/cc/hafa/reader/ui/ReaderViewModel.kt @@ -1,7 +1,9 @@ package cc.hafa.reader.ui import android.app.Application +import android.os.SystemClock import android.util.Log +import androidx.annotation.PluralsRes import androidx.annotation.StringRes import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf @@ -12,9 +14,12 @@ import cc.hafa.reader.R import cc.hafa.reader.core.FeedHit import cc.hafa.reader.core.FeedProblem import cc.hafa.reader.core.FeedProblemException +import cc.hafa.reader.core.LoadGate import cc.hafa.reader.core.ReaderModel import cc.hafa.reader.core.ReaderState import cc.hafa.reader.core.ReaderView +import cc.hafa.reader.core.ReadingFont +import cc.hafa.reader.core.loadingBusy import cc.hafa.reader.core.Selection import cc.hafa.reader.core.sync.RetentionDays import cc.hafa.reader.core.sync.SyncEngine @@ -28,10 +33,13 @@ import kotlinx.coroutines.CoroutineExceptionHandler import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.channels.Channel +import kotlinx.coroutines.delay import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.asStateFlow +import kotlinx.coroutines.flow.collectLatest import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.filter @@ -76,6 +84,14 @@ sealed interface Notice { @param:StringRes val text: Int, ) : Notice + /** A message that names a count. */ + data class Count( + /** Plurals resource of the message, taking the count as its argument. */ + @param:PluralsRes val text: Int, + /** The count. */ + val count: Int, + ) : Notice + /** Why a feed cannot be added. */ data class Problem( /** What went wrong. */ @@ -122,6 +138,14 @@ class ReaderViewModel(application: Application) : AndroidViewModel(application) .flowOn(Dispatchers.Default) .stateIn(viewModelScope, SharingStarted.Eagerly, null) + // Whether the account's data holds the loading screen: from opening and from signing in or replacing this device's data, until the screen lets the person in. + private val accountAwaited = MutableStateFlow(true) + + private val mutableGate = MutableStateFlow(LoadGate.started(SystemClock.elapsedRealtime())) + + /** The loading screen, which holds the person while what can still change the lists loads. */ + val gate: StateFlow = mutableGate.asStateFlow() + private val noticeChannel = Channel(Channel.BUFFERED) /** Messages for the snackbar, each shown once. */ @@ -135,6 +159,12 @@ class ReaderViewModel(application: Application) : AndroidViewModel(application) var detail by mutableStateOf(Detail.Article) private set + private val preferences = application.readerApplication.preferences + + /** The font for article text and titles on this device. */ + var readingFont by mutableStateOf(preferences.readingFont) + private set + init { viewModelScope.launch(logFailures) { // the drawer has no room for why sign-in failed; the account screen says it @@ -147,11 +177,38 @@ class ReaderViewModel(application: Application) : AndroidViewModel(application) .collect { notify(Notice.Text(R.string.sync_error)) } } viewModelScope.launch(logFailures) { - model.load() - model.refresh() + model.openChecks() + } + viewModelScope.launch(logFailures) { + combine(model.state, syncUi, accountAwaited) { state, sync, awaited -> loadingBusy(state, sync.report, awaited) } + .distinctUntilChanged() + .collectLatest { busy -> + var wake = stepGate(busy) + while (wake != null) { + delay((wake - SystemClock.elapsedRealtime()).coerceAtLeast(0)) + wake = stepGate(busy) + } + } } } + /** + * Move the loading screen on, with [busy] saying whether something that + * can still change the lists is loading; when it next changes on its own, + * or null. Letting the person in settles the lists and stops the + * account's data from holding them again until they next sign in. + */ + private fun stepGate(busy: Boolean): Long? { + val before = mutableGate.value + val next = before.step(busy, SystemClock.elapsedRealtime()) + mutableGate.value = next + if (before.blocks && !next.blocks) { + accountAwaited.value = false + model.settle() + } + return next.wakeAt() + } + /** Show [notice] in the snackbar. */ fun notify(notice: Notice) { noticeChannel.trySend(notice) @@ -182,6 +239,12 @@ class ReaderViewModel(application: Application) : AndroidViewModel(application) } } + /** Choose the reading font, and remember it. */ + fun chooseReadingFont(font: ReadingFont) { + readingFont = font + preferences.readingFont = font + } + /** Open the add-feed screen. */ fun startAdding() { closeArticle() @@ -216,6 +279,7 @@ class ReaderViewModel(application: Application) : AndroidViewModel(application) fun signIn() { val engine = syncEngine.value if (engine != null) { + accountAwaited.value = true engine.signIn() } else if (syncBroken.value) { brokenSignIn.value = true @@ -235,6 +299,7 @@ class ReaderViewModel(application: Application) : AndroidViewModel(application) /** Replace this device's data with the waiting account's. */ fun replaceLocal() { + accountAwaited.value = true syncEngine.value?.replaceLocal() } @@ -306,13 +371,15 @@ class ReaderViewModel(application: Application) : AndroidViewModel(application) } /** - * Import OPML text, then show the list when anything was new; when nothing - * was, or the file cannot be read, the snackbar says why. + * Import OPML text into [group] (null keeps the file's folders, empty is + * no group), then show the list when anything was new. When nothing was, + * when http feeds were left out, or when the file cannot be read, the + * snackbar says so. */ - fun importOpml(source: String) { + fun importOpml(source: String, group: String?) { viewModelScope.launch(logFailures) { val result = try { - model.importOpml(source) + model.importOpml(source, group) } catch (cancelled: CancellationException) { throw cancelled } catch (failure: Exception) { @@ -323,9 +390,12 @@ class ReaderViewModel(application: Application) : AndroidViewModel(application) notify(Notice.Text(R.string.import_failed)) } else if (result.found == 0) { notify(Notice.Text(R.string.import_none)) + } else if (result.skipped > 0) { + notify(Notice.Count(R.plurals.import_skipped_http, result.skipped)) } else if (result.added == 0) { notify(Notice.Text(R.string.import_nothing_new)) - } else { + } + if (result != null && result.added > 0) { finishAdding() } } diff --git a/android/app/src/main/kotlin/cc/hafa/reader/ui/SettingsScreen.kt b/android/app/src/main/kotlin/cc/hafa/reader/ui/SettingsScreen.kt index eb6b8c0..6cdc367 100644 --- a/android/app/src/main/kotlin/cc/hafa/reader/ui/SettingsScreen.kt +++ b/android/app/src/main/kotlin/cc/hafa/reader/ui/SettingsScreen.kt @@ -3,17 +3,12 @@ package cc.hafa.reader.ui import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.ColumnScope -import androidx.compose.foundation.layout.ExperimentalLayoutApi -import androidx.compose.foundation.layout.FlowRow import androidx.compose.foundation.layout.Row -import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.size -import androidx.compose.foundation.rememberScrollState import androidx.compose.foundation.text.KeyboardActions import androidx.compose.foundation.text.KeyboardOptions -import androidx.compose.foundation.verticalScroll import androidx.compose.material.icons.Icons import androidx.compose.material.icons.outlined.Close import androidx.compose.material3.DropdownMenuItem @@ -32,6 +27,7 @@ import androidx.compose.material3.TextFieldColors import androidx.compose.runtime.Composable import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.produceState import androidx.compose.runtime.remember import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment @@ -49,16 +45,21 @@ import androidx.compose.ui.text.style.TextOverflow import androidx.compose.ui.unit.dp import androidx.compose.ui.unit.sp import cc.hafa.reader.R +import cc.hafa.reader.core.ReadingFont import cc.hafa.reader.core.Selection import cc.hafa.reader.core.Sort -import cc.hafa.reader.core.hasWord +import cc.hafa.reader.core.byCount +import cc.hafa.reader.core.knownWordCounts import cc.hafa.reader.core.topicOff +import cc.hafa.reader.core.wordCounts +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.withContext /** Which list of words the field adds to. */ private enum class WordMode { Hide, Only } /** The current view's settings: name, sort, words, topics, and the feed's group or the group's removal. */ -@OptIn(ExperimentalLayoutApi::class, ExperimentalMaterial3Api::class) +@OptIn(ExperimentalMaterial3Api::class) @Composable fun SettingsScreen(snapshot: Snapshot, viewModel: ReaderViewModel) { val (state, view) = snapshot @@ -73,8 +74,9 @@ fun SettingsScreen(snapshot: Snapshot, viewModel: ReaderViewModel) { var draft by remember { mutableStateOf("") } var confirming by rememberConfirming() val words = if (mode == WordMode.Hide) settings.hideWords else settings.onlyWords - val wordCounts = remember(view.baseEntries, words) { - words.associateWith { word -> view.baseEntries.count { entry -> hasWord(listOf(word), entry) } } + val entries = view.baseEntries + val wordCounts by produceState(knownWordCounts(entries, words) ?: emptyMap(), entries, words) { + value = withContext(Dispatchers.Default) { wordCounts(entries, words) } } fun changeWords(next: List) { @@ -95,15 +97,11 @@ fun SettingsScreen(snapshot: Snapshot, viewModel: ReaderViewModel) { bottomBar = { scrollBehavior -> BackBottomBar(stringResource(R.string.settings), scrollBehavior, viewModel::back) }, ) { padding -> Column( - Modifier - .fillMaxSize() - .padding(padding) - .verticalScroll(rememberScrollState()) - .padding(horizontal = 20.dp, vertical = 24.dp), + Modifier.scrollingScreen(padding), verticalArrangement = Arrangement.spacedBy(32.dp), ) { Column(verticalArrangement = Arrangement.spacedBy(8.dp)) { - Text(title, fontFamily = Fraunces, fontWeight = FontWeight.Bold, fontSize = 30.sp, lineHeight = 36.sp) + Text(title, fontFamily = ReaderTheme.readingFamily, fontWeight = FontWeight.Bold, fontSize = 30.sp, lineHeight = 36.sp) if (feed != null && feed.error.isNotEmpty()) { Text(problemText(feed.error), fontSize = 14.sp, lineHeight = 20.sp, color = MaterialTheme.colorScheme.error) } @@ -133,6 +131,17 @@ fun SettingsScreen(snapshot: Snapshot, viewModel: ReaderViewModel) { ) } + Column(verticalArrangement = Arrangement.spacedBy(8.dp)) { + Text(stringResource(R.string.reading_font_label), fontSize = 14.sp, lineHeight = 20.sp, fontWeight = FontWeight.Medium) + Segmented( + listOf(ReadingFont.Literata to stringResource(R.string.font_literata), ReadingFont.PublicSans to stringResource(R.string.font_public_sans)), + viewModel.readingFont, + viewModel::chooseReadingFont, + Modifier.fillMaxWidth(), + ) + Text(stringResource(R.string.reading_font_hint), fontSize = 12.sp, lineHeight = 16.sp, color = colors.muted) + } + // Starred lists what was starred by hand, so it offers only the sort. if (selection != Selection.Starred) Column(verticalArrangement = Arrangement.spacedBy(12.dp)) { Text(stringResource(R.string.rules_label), fontSize = 14.sp, lineHeight = 20.sp, fontWeight = FontWeight.Medium) @@ -154,38 +163,34 @@ fun SettingsScreen(snapshot: Snapshot, viewModel: ReaderViewModel) { colors = readerFieldColors(), ) if (words.isNotEmpty()) { - FlowRow(horizontalArrangement = Arrangement.spacedBy(8.dp)) { - for (word in words) { - val removeLabel = stringResource(R.string.remove_rule) - InputChip( - selected = false, - onClick = { changeWords(words.filter { known -> known != word }) }, - label = { Text(word) }, - trailingIcon = { - Row(verticalAlignment = Alignment.CenterVertically, horizontalArrangement = Arrangement.spacedBy(6.dp)) { - Text((wordCounts[word] ?: 0).toString(), fontFamily = FontFamily.Monospace, fontSize = 12.sp, color = colors.muted) - Icon(Icons.Outlined.Close, contentDescription = removeLabel, modifier = Modifier.size(InputChipDefaults.IconSize)) - } - }, - border = InputChipDefaults.inputChipBorder(enabled = true, selected = false, borderColor = colors.line), - ) - } + ChipRows(byCount(words) { word -> wordCounts[word] ?: 0 }) { word -> + val removeLabel = stringResource(R.string.remove_rule, word) + InputChip( + selected = false, + onClick = { changeWords(words.filter { known -> known != word }) }, + label = { Text(word) }, + trailingIcon = { + Row(verticalAlignment = Alignment.CenterVertically, horizontalArrangement = Arrangement.spacedBy(6.dp)) { + Text((wordCounts[word] ?: 0).toString(), fontFamily = FontFamily.Monospace, fontSize = 12.sp, color = colors.muted) + Icon(Icons.Outlined.Close, contentDescription = removeLabel, modifier = Modifier.size(InputChipDefaults.IconSize)) + } + }, + border = InputChipDefaults.inputChipBorder(enabled = true, selected = false, borderColor = colors.line), + ) } } } - if (selection != Selection.Starred && view.topics.isNotEmpty()) { + if (selection != Selection.Starred && view.topicOrder.counts.isNotEmpty()) { Column(verticalArrangement = Arrangement.spacedBy(12.dp)) { Text(stringResource(R.string.topics), fontSize = 14.sp, lineHeight = 20.sp, fontWeight = FontWeight.Medium) - FlowRow(horizontalArrangement = Arrangement.spacedBy(8.dp)) { - for (item in view.topics) { - TopicChip( - item.topic, - off = topicOff(settings, item.topic), - onToggle = { model.toggleTopic(item.topic, fromArticle = false) }, - count = item.count, - ) - } + ChipRows(view.topicOrder.counts) { item -> + TopicChip( + item.topic, + off = topicOff(settings, item.topic), + onToggle = { model.toggleTopic(item.topic, fromArticle = false) }, + count = item.count, + ) } Text(stringResource(R.string.topics_hint), fontSize = 12.sp, lineHeight = 16.sp, color = colors.muted) } @@ -193,7 +198,14 @@ fun SettingsScreen(snapshot: Snapshot, viewModel: ReaderViewModel) { if (feed != null) { DividedSection { - GroupField(feed.folder, view.folders) { name -> model.setFolder(feed.id, name) } + GroupField( + feed.folder, + view.folders, + label = stringResource(R.string.group_label), + placeholder = stringResource(R.string.no_group), + hint = stringResource(R.string.group_hint), + onCommit = { name -> model.setFolder(feed.id, name) }, + ) ConfirmButton( label = stringResource(R.string.unsubscribe), confirmLabel = stringResource(R.string.confirm_unsubscribe), @@ -276,23 +288,40 @@ private fun CommitField(label: String, value: String, onCommit: (String) -> Unit ) } -/** The feed's group: type a name or pick one in use; empty takes the feed out of its group. */ +/** + * A group name field: type a name or pick one of [folders], suggested as it is typed. [onEdit] gets every edit and + * [onCommit] the name when Done is pressed or the field is left changed from [current]; [placeholder] is shown while + * empty. + */ @OptIn(ExperimentalMaterial3Api::class) @Composable -private fun GroupField(current: String, folders: List, onCommit: (String) -> Unit) { +fun GroupField( + current: String, + folders: List, + label: String, + placeholder: String, + hint: String? = null, + enabled: Boolean = true, + onEdit: (String) -> Unit = {}, + onCommit: (String) -> Unit = {}, +) { var text by remember(current) { mutableStateOf(current) } var focused by remember { mutableStateOf(false) } val focusManager = LocalFocusManager.current val suggestions = folders.filter { name -> name != text && name.contains(text.trim(), ignoreCase = true) } + fun edit(next: String) { + text = next + onEdit(next) + } ExposedDropdownMenuBox( - expanded = focused && suggestions.isNotEmpty(), + expanded = focused && enabled && suggestions.isNotEmpty(), onExpandedChange = {}, ) { OutlinedTextField( value = text, - onValueChange = { next -> text = next }, + onValueChange = ::edit, modifier = Modifier - .menuAnchor(ExposedDropdownMenuAnchorType.PrimaryEditable) + .menuAnchor(ExposedDropdownMenuAnchorType.PrimaryEditable, enabled) .fillMaxWidth() .onFocusChanged { focus -> if (focused && !focus.isFocused && text.trim() != current) { @@ -300,23 +329,28 @@ private fun GroupField(current: String, folders: List, onCommit: (String } focused = focus.isFocused }, - label = { Text(stringResource(R.string.group_label)) }, - placeholder = { Text(stringResource(R.string.no_group)) }, - supportingText = { Text(stringResource(R.string.group_hint)) }, + enabled = enabled, + label = { Text(label) }, + placeholder = { Text(placeholder) }, + supportingText = if (hint != null) { + { Text(hint) } + } else { + null + }, singleLine = true, keyboardOptions = KeyboardOptions(capitalization = KeyboardCapitalization.Sentences, imeAction = ImeAction.Done), keyboardActions = KeyboardActions(onDone = { focusManager.clearFocus() }), colors = readerFieldColors(), ) ExposedDropdownMenu( - expanded = focused && suggestions.isNotEmpty(), + expanded = focused && enabled && suggestions.isNotEmpty(), onDismissRequest = {}, ) { for (name in suggestions) { DropdownMenuItem( text = { Text(name) }, onClick = { - text = name + edit(name) focusManager.clearFocus() }, contentPadding = ExposedDropdownMenuDefaults.ItemContentPadding, diff --git a/android/app/src/main/kotlin/cc/hafa/reader/ui/Sidebar.kt b/android/app/src/main/kotlin/cc/hafa/reader/ui/Sidebar.kt index 35742bf..3350d29 100644 --- a/android/app/src/main/kotlin/cc/hafa/reader/ui/Sidebar.kt +++ b/android/app/src/main/kotlin/cc/hafa/reader/ui/Sidebar.kt @@ -32,6 +32,8 @@ import androidx.compose.ui.Modifier import androidx.compose.ui.draw.clip import androidx.compose.ui.graphics.Color import androidx.compose.ui.res.stringResource +import androidx.compose.ui.semantics.semantics +import androidx.compose.ui.semantics.stateDescription import androidx.compose.ui.text.font.FontFamily import androidx.compose.ui.text.font.FontWeight import androidx.compose.ui.text.style.TextOverflow @@ -89,6 +91,8 @@ fun Sidebar(snapshot: Snapshot, viewModel: ReaderViewModel, onPicked: () -> Unit .fillMaxHeight() .padding(bottom = 12.dp), ) { + val suggested = refreshSuggested(state) + val suggestion = if (suggested) stringResource(R.string.refresh_suggested) else "" Row( Modifier .padding(NavigationDrawerItemDefaults.ItemPadding) @@ -99,11 +103,18 @@ fun Sidebar(snapshot: Snapshot, viewModel: ReaderViewModel, onPicked: () -> Unit viewModel.goHome() onPicked() } + .semantics { + if (suggested) { + stateDescription = suggestion + } + } .padding(horizontal = 16.dp), verticalAlignment = Alignment.CenterVertically, horizontalArrangement = Arrangement.spacedBy(12.dp), ) { - Logo(Modifier.size(32.dp), loading = !state.loaded || state.refreshing || state.adding, labeled = false) + WithDueDot(suggested) { + Logo(Modifier.size(32.dp), loading = !state.loaded || state.refreshing || state.adding, labeled = false) + } Text(stringResource(R.string.app_name), fontWeight = FontWeight.SemiBold, fontSize = 18.sp, color = colors.text, maxLines = 1) } LazyColumn(Modifier.weight(1f)) { diff --git a/android/app/src/main/kotlin/cc/hafa/reader/ui/Theme.kt b/android/app/src/main/kotlin/cc/hafa/reader/ui/Theme.kt index fc18021..adeb802 100644 --- a/android/app/src/main/kotlin/cc/hafa/reader/ui/Theme.kt +++ b/android/app/src/main/kotlin/cc/hafa/reader/ui/Theme.kt @@ -12,10 +12,12 @@ import androidx.compose.runtime.staticCompositionLocalOf import androidx.compose.ui.graphics.Color import androidx.compose.ui.text.font.Font import androidx.compose.ui.text.font.FontFamily +import androidx.compose.ui.text.font.FontStyle import androidx.compose.ui.text.font.FontVariation import androidx.compose.ui.text.font.FontWeight import androidx.compose.ui.unit.sp import cc.hafa.reader.R +import cc.hafa.reader.core.ReadingFont /** The web app's palette (web/src/app.css), one set per light or dark. */ @Immutable @@ -74,27 +76,51 @@ private val DarkColors = ReaderColors( private val LocalReaderColors = staticCompositionLocalOf { LightColors } -private fun publicSans(weight: Int) = - Font(R.font.public_sans, FontWeight(weight), variationSettings = FontVariation.Settings(FontVariation.weight(weight))) +private val LocalReadingFont = staticCompositionLocalOf { ReadingFont.Literata } -private fun fraunces(weight: Int) = - Font(R.font.fraunces, FontWeight(weight), variationSettings = FontVariation.Settings(FontVariation.weight(weight))) +// the bundled font files hold weights 400 to 800 only (scripts/cut-fonts.sh) +private fun publicSans(weight: Int, style: FontStyle) = + Font( + if (style == FontStyle.Italic) R.font.public_sans_italic else R.font.public_sans, + FontWeight(weight), + style, + variationSettings = FontVariation.Settings(FontVariation.weight(weight)), + ) + +private fun literata(weight: Int, style: FontStyle) = + Font( + if (style == FontStyle.Italic) R.font.literata_italic else R.font.literata, + FontWeight(weight), + style, + variationSettings = FontVariation.Settings(FontVariation.weight(weight)), + ) + +/** Public Sans, the interface's face and one of the reading fonts, with real italics. */ +val PublicSans = FontFamily( + listOf(400, 500, 600, 650, 700, 800).flatMap { weight -> listOf(publicSans(weight, FontStyle.Normal), publicSans(weight, FontStyle.Italic)) }, +) -/** Public Sans, the web app's text face. */ -val PublicSans = FontFamily(publicSans(400), publicSans(500), publicSans(600), publicSans(650), publicSans(700), publicSans(800)) +/** Literata, the default reading font, with real italics. */ +val Literata = FontFamily( + listOf(400, 500, 600, 700, 800).flatMap { weight -> listOf(literata(weight, FontStyle.Normal), literata(weight, FontStyle.Italic)) }, +) -/** Fraunces, the web app's heading face. */ -val Fraunces = FontFamily(fraunces(400), fraunces(600), fraunces(700), fraunces(800)) +/** The family [ReadingFont] draws with. */ +val ReadingFont.family: FontFamily + get() = when (this) { + ReadingFont.Literata -> Literata + ReadingFont.PublicSans -> PublicSans + } // The web sets no letter spacing, so neither does any style here. private val ReaderTypography = Typography().let { base -> Typography( - displayLarge = base.displayLarge.copy(fontFamily = Fraunces, letterSpacing = 0.sp), - displayMedium = base.displayMedium.copy(fontFamily = Fraunces, letterSpacing = 0.sp), - displaySmall = base.displaySmall.copy(fontFamily = Fraunces, letterSpacing = 0.sp), - headlineLarge = base.headlineLarge.copy(fontFamily = Fraunces, letterSpacing = 0.sp), - headlineMedium = base.headlineMedium.copy(fontFamily = Fraunces, letterSpacing = 0.sp), - headlineSmall = base.headlineSmall.copy(fontFamily = Fraunces, letterSpacing = 0.sp), + displayLarge = base.displayLarge.copy(fontFamily = PublicSans, letterSpacing = 0.sp), + displayMedium = base.displayMedium.copy(fontFamily = PublicSans, letterSpacing = 0.sp), + displaySmall = base.displaySmall.copy(fontFamily = PublicSans, letterSpacing = 0.sp), + headlineLarge = base.headlineLarge.copy(fontFamily = PublicSans, letterSpacing = 0.sp), + headlineMedium = base.headlineMedium.copy(fontFamily = PublicSans, letterSpacing = 0.sp), + headlineSmall = base.headlineSmall.copy(fontFamily = PublicSans, letterSpacing = 0.sp), titleLarge = base.titleLarge.copy(fontFamily = PublicSans, letterSpacing = 0.sp), titleMedium = base.titleMedium.copy(fontFamily = PublicSans, letterSpacing = 0.sp), titleSmall = base.titleSmall.copy(fontFamily = PublicSans, letterSpacing = 0.sp), @@ -107,9 +133,9 @@ private val ReaderTypography = Typography().let { base -> ) } -/** The reader's colors, following the system's light or dark setting. */ +/** The reader's colors, following the system's light or dark setting, and its [readingFont]. */ @Composable -fun ReaderTheme(content: @Composable () -> Unit) { +fun ReaderTheme(readingFont: ReadingFont = ReadingFont.Literata, content: @Composable () -> Unit) { val colors = if (isSystemInDarkTheme()) DarkColors else LightColors val scheme = if (isSystemInDarkTheme()) darkColorScheme() else lightColorScheme() MaterialTheme( @@ -134,13 +160,17 @@ fun ReaderTheme(content: @Composable () -> Unit) { outlineVariant = colors.line, ), ) { - CompositionLocalProvider(LocalReaderColors provides colors, content = content) + CompositionLocalProvider(LocalReaderColors provides colors, LocalReadingFont provides readingFont, content = content) } } -/** Access to the reader's palette. */ +/** Access to the reader's palette and reading font. */ object ReaderTheme { /** The palette in use. */ val colors: ReaderColors @Composable get() = LocalReaderColors.current + + /** The family for article text and titles. */ + val readingFamily: FontFamily + @Composable get() = LocalReadingFont.current.family } diff --git a/android/app/src/main/res/drawable/sleeping_dot.xml b/android/app/src/main/res/drawable/sleeping_dot.xml deleted file mode 100644 index 5b239df..0000000 --- a/android/app/src/main/res/drawable/sleeping_dot.xml +++ /dev/null @@ -1,31 +0,0 @@ - - - - - - - - - diff --git a/android/app/src/main/res/font/fraunces.ttf b/android/app/src/main/res/font/fraunces.ttf deleted file mode 100644 index 8210f94..0000000 Binary files a/android/app/src/main/res/font/fraunces.ttf and /dev/null differ diff --git a/android/app/src/main/res/font/literata.ttf b/android/app/src/main/res/font/literata.ttf new file mode 100644 index 0000000..34031ba Binary files /dev/null and b/android/app/src/main/res/font/literata.ttf differ diff --git a/android/app/src/main/res/font/literata_italic.ttf b/android/app/src/main/res/font/literata_italic.ttf new file mode 100644 index 0000000..6b43b70 Binary files /dev/null and b/android/app/src/main/res/font/literata_italic.ttf differ diff --git a/android/app/src/main/res/font/public_sans.ttf b/android/app/src/main/res/font/public_sans.ttf index c747e1d..4507d41 100644 Binary files a/android/app/src/main/res/font/public_sans.ttf and b/android/app/src/main/res/font/public_sans.ttf differ diff --git a/android/app/src/main/res/font/public_sans_italic.ttf b/android/app/src/main/res/font/public_sans_italic.ttf new file mode 100644 index 0000000..856074b Binary files /dev/null and b/android/app/src/main/res/font/public_sans_italic.ttf differ diff --git a/android/app/src/main/res/values/strings.xml b/android/app/src/main/res/values/strings.xml index ab333b2..3e86f7a 100644 --- a/android/app/src/main/res/values/strings.xml +++ b/android/app/src/main/res/values/strings.xml @@ -15,7 +15,13 @@ Nothing found. %1$s subscribers Import from another reader - Choose the OPML file exported from your old reader. Folders are kept. + Choose the OPML file exported from your old reader. + Add to group + Folders from the file + + Skipped %1$s http feed with no https version. + Skipped %1$s http feeds with no https version. + Choose an OPML file Export as OPML Could not save the file. @@ -31,14 +37,18 @@ Sort Newest Oldest + Reading font + + Applies to every view. + Literata + Public Sans Words Hide Only show Add a word or phrase, then press Enter - Remove + Remove %1$s Words match whole words in the title or summary, ignoring capitals and spacing. Topics - End of list Already use Reader? Sign in to sync your feeds Keep synced articles @@ -48,6 +58,8 @@ 1 year Forever Refresh + + Some feeds are due for a check Select a topic to turn it off. Articles with only turned-off topics are hidden. Name Group @@ -61,11 +73,14 @@ Star Share Feeds + Show feeds Back The site did not answer. Only https addresses can be added. The feed is not on https. + Reader can\'t add http addresses. Try https instead. + This feed\'s address is http, not https, so Reader can\'t fetch it. That address is not a feed and does not point to one. The site answered %1$d. The site refused to let Reader read it. diff --git a/android/core/src/main/kotlin/cc/hafa/reader/core/CheckQueue.kt b/android/core/src/main/kotlin/cc/hafa/reader/core/CheckQueue.kt index fad94e7..53de02c 100644 --- a/android/core/src/main/kotlin/cc/hafa/reader/core/CheckQueue.kt +++ b/android/core/src/main/kotlin/cc/hafa/reader/core/CheckQueue.kt @@ -98,6 +98,8 @@ class QueuedCheck internal constructor( * in batches: once no waited-for check is left unfinished and one is done, at * [QueueLimits.saveBatch], [QueueLimits.saveIntervalMs] after the last save * (with a timer when nothing else comes), or when nothing is left to run. + * [onBusy] hears whether anything is queued, running or unsaved each time + * that changes. */ class CheckQueue( private val scope: CoroutineScope, @@ -112,6 +114,7 @@ class CheckQueue( callback() } }, + private val onBusy: (Boolean) -> Unit = {}, ) { private val lock = Mutex() private val saveLock = Mutex() @@ -122,6 +125,7 @@ class CheckQueue( private var unsaved = mutableListOf>() private var lastSave = 0L private var saveTimerArmed = false + private var reportedBusy = false /** Queue checks, joining any already under way, and return each request's check. */ suspend fun enqueue(requests: List): List> = @@ -147,6 +151,7 @@ class CheckQueue( } saveIfReady() pump() + reportBusy() checks } @@ -249,6 +254,16 @@ class CheckQueue( } saveIfReady() pump() + reportBusy() + } + } + + // Called with the lock held. + private fun reportBusy() { + val busy = checking.isNotEmpty() + if (busy != reportedBusy) { + reportedBusy = busy + onBusy(busy) } } @@ -266,7 +281,10 @@ class CheckQueue( try { saveLock.withLock { save(batch.map { (_, finished) -> finished }) } } finally { - lock.withLock { batch.forEach { (finishedCheck, _) -> release(finishedCheck) } } + lock.withLock { + batch.forEach { (finishedCheck, _) -> release(finishedCheck) } + reportBusy() + } } } } else if (unsaved.isNotEmpty() && !saveTimerArmed) { diff --git a/android/core/src/main/kotlin/cc/hafa/reader/core/FeedCheck.kt b/android/core/src/main/kotlin/cc/hafa/reader/core/FeedCheck.kt index dc03c8e..a8dabaf 100644 --- a/android/core/src/main/kotlin/cc/hafa/reader/core/FeedCheck.kt +++ b/android/core/src/main/kotlin/cc/hafa/reader/core/FeedCheck.kt @@ -2,9 +2,11 @@ package cc.hafa.reader.core import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.withContext +import okhttp3.HttpUrl import okhttp3.HttpUrl.Companion.toHttpUrlOrNull import okhttp3.OkHttpClient import okhttp3.Request +import okhttp3.Response import okhttp3.coroutines.executeAsync import java.io.IOException import java.io.InterruptedIOException @@ -51,16 +53,32 @@ private fun Request.Builder.validator(name: String, value: String) { } } +/** Most redirects off https followed at their https addresses in one fetch, as many as browsers follow in all. */ +private const val MAX_UPGRADED_REDIRECTS = 20 + +/** [url] at https, its port moved from http's default to https's, as a browser upgrading an insecure request does. */ +private fun upgradedUrl(url: HttpUrl): HttpUrl = + if (url.isHttps) { + url + } else { + url.newBuilder().scheme("https").port(if (url.port == HTTP_PORT) HTTPS_PORT else url.port).build() + } + +private const val HTTP_PORT = 80 +private const val HTTPS_PORT = 443 + /** * A [FeedFetcher] over OkHttp; a failed request has status zero, and an answer * over [MAX_BODY_BYTES] has no text. Plain http addresses are refused unless - * [httpsOnly] is off, which only tests against a local server do. + * [httpsOnly] is off, which only tests against a local server do. A redirect + * from https to plain http is followed at its https address instead, as the + * web's extension does, so no plain http request is ever sent. */ class HttpFeedFetcher( client: OkHttpClient, private val httpsOnly: Boolean = true, ) : FeedFetcher { - // A redirect from https to plain http is not followed. + // A redirect between https and plain http comes back unfollowed, to be followed here at https. private val client = client.newBuilder().followSslRedirects(!httpsOnly).build() override suspend fun fetch(url: String, etag: String, lastModified: String, timeoutMs: Long): FetchOutcome { @@ -70,45 +88,62 @@ class HttpFeedFetcher( } else if (httpsOnly && !address.isHttps) { FetchOutcome(status = 0, insecure = true) } else withContext(Dispatchers.IO) { - val request = Request.Builder().url(address).apply { - header("User-Agent", USER_AGENT) - header("Accept", "*/*") - validator("If-None-Match", etag) - validator("If-Modified-Since", lastModified) - }.build() - val call = client.newCall(request) - call.timeout().timeout(timeoutMs, TimeUnit.MILLISECONDS) + val deadline = System.nanoTime() + TimeUnit.MILLISECONDS.toNanos(timeoutMs) + var next: HttpUrl = address + var outcome: FetchOutcome? = null + var hops = 0 try { - call.executeAsync().use { response -> - // A redirect left unfollowed leads off https, which counts as no answer. - if (httpsOnly && response.isRedirect) { - return@use FetchOutcome(status = 0, insecure = true) - } - // A body over the limit is left unread, so it counts as no readable feed. - val text = if (response.code == 200) { - if (response.body.source().request(MAX_BODY_BYTES + 1)) { - "" + while (outcome == null) { + val request = Request.Builder().url(next).apply { + header("User-Agent", USER_AGENT) + header("Accept", "*/*") + validator("If-None-Match", etag) + validator("If-Modified-Since", lastModified) + }.build() + val call = client.newCall(request) + call.timeout().timeout(maxOf(1L, deadline - System.nanoTime()), TimeUnit.NANOSECONDS) + call.executeAsync().use { response -> + val location = response.header("Location")?.let { target -> response.request.url.resolve(target) } + if (httpsOnly && response.isRedirect && location != null) { + if (hops < MAX_UPGRADED_REDIRECTS) { + hops += 1 + next = upgradedUrl(location) + } else { + outcome = FetchOutcome(status = 0) + } } else { - decodeBody(response.body.bytes(), response.header("Content-Type") ?: "") + outcome = answerOf(response) } - } else { - "" } - FetchOutcome( - status = response.code, - text = text, - url = response.request.url.toString(), - etag = response.header("ETag") ?: "", - lastModified = response.header("Last-Modified") ?: "", - hint = hintFromHeaders(response::header), - ) } } catch (_: InterruptedIOException) { - FetchOutcome(status = 0, timedOut = true) + outcome = FetchOutcome(status = 0, timedOut = true) } catch (_: IOException) { - FetchOutcome(status = 0) + outcome = FetchOutcome(status = 0) } + checkNotNull(outcome) + } + } + + /** What an answer says; a body over the limit is left unread, so it counts as no readable feed. */ + private fun answerOf(response: Response): FetchOutcome { + val text = if (response.code == 200) { + if (response.body.source().request(MAX_BODY_BYTES + 1)) { + "" + } else { + decodeBody(response.body.bytes(), response.header("Content-Type") ?: "") + } + } else { + "" } + return FetchOutcome( + status = response.code, + text = text, + url = response.request.url.toString(), + etag = response.header("ETag") ?: "", + lastModified = response.header("Last-Modified") ?: "", + hint = hintFromHeaders(response::header), + ) } } diff --git a/android/core/src/main/kotlin/cc/hafa/reader/core/FeedSearch.kt b/android/core/src/main/kotlin/cc/hafa/reader/core/FeedSearch.kt index 14f6ee0..77cc8de 100644 --- a/android/core/src/main/kotlin/cc/hafa/reader/core/FeedSearch.kt +++ b/android/core/src/main/kotlin/cc/hafa/reader/core/FeedSearch.kt @@ -6,7 +6,6 @@ import kotlinx.serialization.json.JsonObject import kotlinx.serialization.json.JsonPrimitive import kotlinx.serialization.json.doubleOrNull import okhttp3.HttpUrl.Companion.toHttpUrl -import okhttp3.HttpUrl.Companion.toHttpUrlOrNull private const val SEARCH_ADDRESS = "https://cloud.feedly.com/v3/search/feeds" private const val SEARCH_COUNT = "8" @@ -24,6 +23,8 @@ data class FeedHit( val icon: String, /** How many Feedly readers follow it. */ val subscribers: Long, + /** Address of the feed's site; empty if unknown. */ + val siteUrl: String = "", ) /** The Feedly search address for a query. */ @@ -37,7 +38,10 @@ fun feedSearchAddress(query: String): String = private fun JsonObject.string(name: String): String? = (this[name] as? JsonPrimitive)?.takeIf { value -> value.isString }?.content -/** The feeds in a Feedly search answer; empty when it cannot be read. */ +/** + * The feeds in a Feedly search answer; empty when it cannot be read. A plain + * http feed keeps its http address, to be tried over https before it is shown. + */ fun parseFeedSearch(text: String): List { val parsed = try { Json.parseToJsonElement(text) @@ -52,14 +56,15 @@ fun parseFeedSearch(text: String): List { null } else { FeedHit( - url = upgradedAddress(id.replace(FEED_PREFIX, "")), + url = webAddress(id.replace(FEED_PREFIX, "")), title = hit.string("title") ?: id, description = hit.string("description") ?: "", icon = upgradedAddress(hit.string("iconUrl") ?: ""), subscribers = (hit["subscribers"] as? JsonPrimitive)?.doubleOrNull?.toLong() ?: 0, + siteUrl = hit.string("website") ?: "", ) } } - .filter { hit -> hit.url.toHttpUrlOrNull()?.isHttps == true } + .filter { hit -> hit.url.isNotEmpty() } .distinctBy { hit -> hit.url } } diff --git a/android/core/src/main/kotlin/cc/hafa/reader/core/Filter.kt b/android/core/src/main/kotlin/cc/hafa/reader/core/Filter.kt index b5359ea..5927aa1 100644 --- a/android/core/src/main/kotlin/cc/hafa/reader/core/Filter.kt +++ b/android/core/src/main/kotlin/cc/hafa/reader/core/Filter.kt @@ -148,23 +148,91 @@ fun hasWord(words: List, entry: FilterEntry): Boolean { return matcher(entry.title) || matcher(entry.summary) } -/** The topics the articles carry, commonest first, with how many have each. */ +/** The inputs and result of the last [wordCounts], so the panel counts again only when a word or an article's text changes. */ +private class CountedWords(val entries: List, val words: List, val counts: Map) { + fun matches(otherEntries: List, otherWords: List): Boolean = + words == otherWords && entries.size == otherEntries.size && + entries.zip(otherEntries).all { (known, other) -> known.title === other.title && known.summary === other.summary } +} + +@Volatile +private var lastWordCounts: CountedWords? = null + +/** The last [wordCounts] for these articles and words, if it is still current. */ +fun knownWordCounts(entries: List, words: List): Map? = + lastWordCounts?.takeIf { cached -> cached.matches(entries, words) }?.counts + +/** + * How many of the articles each of the words matches in the title or summary. Slow on a large view, so it is meant + * to run off the main thread; the last result is kept and reused while the words and the articles' text stay the same. + */ +fun wordCounts(entries: List, words: List): Map { + val known = knownWordCounts(entries, words) + return if (known != null) { + known + } else { + val counts = words.associateWith { word -> + val matcher = matcherFor(listOf(word)) + entries.count { entry -> matcher(entry.title) || matcher(entry.summary) } + } + lastWordCounts = CountedWords(entries, words, counts) + counts + } +} + +/** The topics the articles carry, with how many articles carry each, commonest first and then by name. */ fun topicCounts(entries: List): List { val counts = LinkedHashMap() for (entry in entries) { + val seen = HashSet() for (topic in entry.categories) { - val key = topic.trim().lowercase() - val known = counts[key] - counts[key] = TopicCount(topic, (known?.count ?: 0) + 1) + val key = topicKey(topic) + if (seen.add(key)) { + val known = counts[key] + counts[key] = TopicCount(topic, (known?.count ?: 0) + 1) + } } } val collator = Collator.getInstance() - return counts.values.sortedWith { first, second -> - val byCount = second.count - first.count - if (byCount != 0) byCount else collator.compare(first.topic, second.topic) - } + return counts.values.sortedWith { first, second -> compareCounted(collator, first.count, first.topic, second.count, second.topic) } +} + +private fun compareCounted(collator: Collator, firstCount: Int, firstName: String, secondCount: Int, secondName: String): Int { + val byCount = secondCount - firstCount + return if (byCount != 0) byCount else collator.compare(firstName, secondName) +} + +/** The [names] with the highest counts first and then by name, the order every list of topics or words is shown in. */ +fun byCount(names: List, count: (String) -> Int): List { + val collator = Collator.getInstance() + return names + .map { name -> name to count(name) } + .sortedWith { (firstName, firstCount), (secondName, secondCount) -> + compareCounted(collator, firstCount, firstName, secondCount, secondName) + } + .map { (name, _) -> name } } +/** + * A view's topic [counts], commonest first, with the count of each topic looked up by its trimmed lowercase name; + * built once per view so ordering an article's topics does not go through all of them again. + */ +class TopicOrder( + /** The view's topics and how many of its articles carry each, commonest first and then by name. */ + val counts: List, +) { + private val byName: Map = counts.associate { item -> topicKey(item.topic) to item.count } + + /** How many of the view's articles carry [topic], ignoring capitals and surrounding space. */ + fun countOf(topic: String): Int = byName[topicKey(topic)] ?: 0 +} + +private fun topicKey(topic: String): String = topic.trim().lowercase() + +/** An article's topics, each once, in the view's [order]: most articles in the view first, then by name. */ +fun articleTopics(categories: List, order: TopicOrder): List = + byCount(categories.distinctBy(::topicKey), order::countOf) + /** Whether an article stays after the settings' filters are applied. */ fun passes(settings: ViewSettings, entry: FilterEntry): Boolean = compiledFilter(settings)(entry) @@ -185,7 +253,7 @@ private fun compiledFilter(settings: ViewSettings): (FilterEntry) -> Boolean { val only = if (settings.onlyWords.isEmpty()) null else matcherFor(settings.onlyWords) val filter: (FilterEntry) -> Boolean = { entry -> val topicsOff = entry.categories.isNotEmpty() && - entry.categories.all { topic -> topic.trim().lowercase() in off } + entry.categories.all { topic -> topicKey(topic) in off } val hidden = hide(entry.title) || hide(entry.summary) val kept = only == null || only(entry.title) || only(entry.summary) !(topicsOff || hidden) && kept diff --git a/android/core/src/main/kotlin/cc/hafa/reader/core/HttpsUpgrade.kt b/android/core/src/main/kotlin/cc/hafa/reader/core/HttpsUpgrade.kt new file mode 100644 index 0000000..7d6bb2f --- /dev/null +++ b/android/core/src/main/kotlin/cc/hafa/reader/core/HttpsUpgrade.kt @@ -0,0 +1,163 @@ +package cc.hafa.reader.core + +import kotlinx.coroutines.async +import kotlinx.coroutines.awaitAll +import kotlinx.coroutines.coroutineScope +import kotlinx.coroutines.sync.Semaphore +import kotlinx.coroutines.sync.withPermit +import okhttp3.HttpUrl.Companion.toHttpUrlOrNull + +/** Most https addresses tried at once. */ +const val PROBE_CONCURRENCY = 6 + +/** + * Most tries of a plain http feed's https address in one run that get no + * definite answer; after that it waits for the app to start again. + */ +const val UNSETTLED_TRIES = 3 + +private val TRAILING_DOT = Regex("\\.$") + +// A last label that is a number, as in an IPv4 address or the forms browsers read as one. +private val NUMBER_LABEL = Regex("^(?:0x[0-9a-f]*|[0-9]+)$", RegexOption.IGNORE_CASE) + +// Names that only mean something on the local network. +private val LOCAL_NAMES = listOf("localhost", "local", "internal", "home.arpa", "lan") + +private val SPACES_PATTERN = Regex("\\s+") +private val SCHEME_PATTERN = Regex("^https?://", RegexOption.IGNORE_CASE) +private const val SECURE_SCHEME = "https://" + +/** Whether an address is plain `http`, as the shared `isPlainHttp` says. */ +fun isPlainHttp(address: String): Boolean = jsTrim(address).toHttpUrlOrNull()?.isHttps == false + +/** + * An `https` address written as plain `http`, to find a feed followed at its + * http address; empty for anything else, as the shared `plainAddress` says. + */ +fun plainAddress(address: String): String { + val trimmed = jsTrim(address) + val secure = if (isPlainHttp(trimmed)) "" else upgradedAddress(trimmed) + return if (secure.isEmpty()) "" else "http://" + secure.removePrefix(SECURE_SCHEME) +} + +/** + * An `http` or `https` address written out the way browsers write it (the + * scheme and host in lower case, the scheme's own default port left out, an + * international name in its `xn--` form, the fragment kept); empty for + * anything else, and for an address with a user name or password, which + * browsers refuse to fetch. Plain http is kept as it is, so it can be shown + * as http and tried over https before it is used, as the shared `webAddress` does. + */ +fun webAddress(address: String): String = + browserWritten(jsTrim(address)) { url -> url.username.isEmpty() && url.password.isEmpty() } + +/** + * Whether a plain http address may be tried at its https address: its host + * is a public name, not an IP address (nor a number browsers read as one), a + * name of one label, `localhost`, or a name under `.local`, `.internal`, + * `.home.arpa` or `.lan`, and it has no user name or password, as the shared + * `probeAllowed` says. Addresses from a feed list or a search come from + * others, and trying them must not reach this device or its network. + */ +fun probeAllowed(address: String): Boolean { + val url = jsTrim(address).toHttpUrlOrNull() + val host = (url?.host ?: "").replace(TRAILING_DOT, "") + val labels = host.split(".") + return url != null && + url.username.isEmpty() && + url.password.isEmpty() && + labels.size > 1 && + ':' !in host && + !NUMBER_LABEL.matches(labels.last()) && + LOCAL_NAMES.none { name -> host == name || host.endsWith(".$name") } +} + +/** What is known of a plain http feed before its https address is tried. */ +data class KnownFeed( + /** Its plain http address. */ + val url: String, + /** Its name; empty when unknown. */ + val title: String, + /** Its site's address; empty when unknown. */ + val siteUrl: String, + /** Guids of its articles already held. */ + val guids: List = emptyList(), + /** Links of its articles already held. */ + val links: List = emptyList(), +) + +/** A plain http feed's https address, found to serve the same feed. */ +data class HttpsMatch( + /** The https address. */ + val url: String, + /** What it served. */ + val feed: ParsedFeed, + /** The answer it came in. */ + val outcome: FetchOutcome, +) + +// Statuses that ask to try again later rather than say anything of the address. +private val LATER_STATUSES = setOf(408, 425, 429) + +/** + * Whether a probe's answer settles if a feed has an https address: any answer + * from the site, a redirect off https included, except one asking to try again + * later (a timeout, too many requests, or a server error). No answer at all, + * as when offline or the fetch timed out, settles nothing. + */ +fun settledAnswer(outcome: FetchOutcome?): Boolean = + outcome != null && (outcome.insecure || (outcome.status != 0 && outcome.status < 500 && outcome.status !in LATER_STATUSES)) + +private fun normalTitle(title: String): String = title.lowercase().split(SPACES_PATTERN).filter { word -> word.isNotEmpty() }.joinToString(" ") + +private fun withoutScheme(address: String): String = address.replace(SCHEME_PATTERN, "") + +private fun siteHost(address: String): String = address.toHttpUrlOrNull()?.host?.removePrefix("www.") ?: "" + +/** + * Whether a feed read over https looks like the plain http one it stands in + * for: the same name (ignoring capitals and spacing), an article in common (by + * guid or link, ignoring the scheme), or a site on the same host as the known + * site or, failing that, as the http address itself (ignoring `www.`). + */ +fun sameFeed(known: KnownFeed, found: ParsedFeed): Boolean { + val title = normalTitle(known.title) + val guids = known.guids.mapTo(HashSet(), ::withoutScheme) + val links = known.links.filter { link -> link.isNotEmpty() }.mapTo(HashSet(), ::withoutScheme) + val shared = found.entries.any { entry -> + withoutScheme(entry.guid) in guids || (entry.url.isNotEmpty() && withoutScheme(entry.url) in links) + } + val knownHost = siteHost(known.siteUrl).ifEmpty { siteHost(known.url) } + val foundHost = siteHost(found.siteUrl) + return (title.isNotEmpty() && title == normalTitle(found.title)) || shared || (foundHost.isNotEmpty() && foundHost == knownHost) +} + +/** + * Try a plain http feed at its https address. It counts only when the answer + * is a 200 that stays on https, parses as a feed and passes [sameFeed]; + * otherwise, or when [known] is not plain http or not [probeAllowed], there + * is no match, and then nothing is fetched. + */ +suspend fun probeHttps(known: KnownFeed, fetcher: FeedFetcher): HttpsMatch? { + val url = if (isPlainHttp(known.url) && probeAllowed(known.url)) upgradedAddress(known.url) else "" + val outcome = if (url.isEmpty()) null else fetcher.fetch(url, "", "") + val feed = if (outcome != null && outcome.status == 200 && !outcome.insecure) { + try { + parseFeed(outcome.text, outcome.url.ifEmpty { url }) + } catch (_: NotAFeedException) { + null + } + } else { + null + } + return if (outcome != null && feed != null && sameFeed(known, feed)) HttpsMatch(url, feed, outcome) else null +} + +/** Run [work] on every item, at most [limit] at once, keeping the order. */ +suspend fun mapConcurrently(items: List, limit: Int, work: suspend (Item) -> Result): List { + val permits = Semaphore(limit) + return coroutineScope { + items.map { item -> async { permits.withPermit { work(item) } } }.awaitAll() + } +} diff --git a/android/core/src/main/kotlin/cc/hafa/reader/core/LoadGate.kt b/android/core/src/main/kotlin/cc/hafa/reader/core/LoadGate.kt new file mode 100644 index 0000000..87d9080 --- /dev/null +++ b/android/core/src/main/kotlin/cc/hafa/reader/core/LoadGate.kt @@ -0,0 +1,111 @@ +package cc.hafa.reader.core + +import cc.hafa.reader.core.sync.SyncReport +import cc.hafa.reader.core.sync.SyncStatus + +/** + * Whether something that can still change the lists is loading: the stored + * data, the first check of imported feeds, or, while the person waits for it + * ([awaited]), the signed-in account's data: sync is starting, or it has not + * caught up and is neither offline nor failing. + */ +fun loadingBusy(state: ReaderState, sync: SyncReport, awaited: Boolean): Boolean { + val arriving = sync.status == SyncStatus.Connecting || + (!sync.caughtUp && (sync.status == SyncStatus.Syncing || sync.status == SyncStatus.Synced)) + return !state.loaded || state.importing > 0 || (awaited && arriving) +} + +/** What the loading screen shows. */ +enum class GatePhase { + /** Nothing: the person is let in. */ + Open, + + /** A blank screen for a moment, so a quick load shows no logo. */ + Waiting, + + /** The turning logo. */ + Shown, +} + +/** How long the loading screen waits, stays and holds out. */ +data class GateTiming( + /** How long a load runs before the logo shows. */ + val delayMs: Long = 300, + /** Shortest time the logo stays once shown, so it never flickers. */ + val minShownMs: Long = 700, + /** Longest time a load holds the person; after it they are let in and the load goes on behind. */ + val timeoutMs: Long = 15_000, +) + +/** + * The loading screen, which holds the person while what can still change the + * lists loads; [step] moves it on. It matches the web app's `load-gate.ts`. + */ +data class LoadGate( + /** What the screen shows now. */ + val phase: GatePhase = GatePhase.Open, + /** When the current load began; null when nothing is loading. */ + val busySince: Long? = null, + /** When the logo appeared; null when it is not shown. */ + val shownSince: Long? = null, + /** Whether the current load ran out of time, so it goes on without holding anyone. */ + val timedOut: Boolean = false, +) { + /** Whether the screen holds the person: nothing behind it takes taps or keys. */ + val blocks: Boolean get() = phase != GatePhase.Open + + /** + * The loading screen at [now], with [busy] saying whether something that + * can still change the lists is loading. + * + * A load holds the person behind a blank screen at first, and behind the + * turning logo once it has run [GateTiming.delayMs]; an instant load shows + * no logo. Once shown, the logo stays at least [GateTiming.minShownMs]. A + * load still running after [GateTiming.timeoutMs] lets the person in and + * holds no one until it ends; the next one starts over. + */ + fun step(busy: Boolean, now: Long, timing: GateTiming = GateTiming()): LoadGate { + val since = busySince ?: now + return if (!busy) { + val shown = shownSince + if (phase == GatePhase.Shown && shown != null && now < shown + timing.minShownMs) { + copy(busySince = null, timedOut = false) + } else { + LoadGate() + } + } else if (timedOut) { + copy(phase = GatePhase.Open, shownSince = null) + } else if (now - since >= timing.timeoutMs) { + LoadGate(GatePhase.Open, since, null, timedOut = true) + } else if (phase == GatePhase.Shown) { + copy(busySince = since) + } else if (now - since >= timing.delayMs) { + LoadGate(GatePhase.Shown, since, now) + } else { + LoadGate(GatePhase.Waiting, since) + } + } + + /** When the screen next changes on its own, for a timer; null when it will not. */ + fun wakeAt(timing: GateTiming = GateTiming()): Long? { + val times = mutableListOf() + val since = busySince + val shown = shownSince + if (since != null && !timedOut) { + times.add(since + timing.timeoutMs) + if (phase == GatePhase.Waiting) { + times.add(since + timing.delayMs) + } + } + // A load that ended while the logo was up lets the person in once it has stayed long enough. + if (phase == GatePhase.Shown && since == null && shown != null) { + times.add(shown + timing.minShownMs) + } + return times.minOrNull() + } + + companion object { + /** A loading screen whose load begins at [now], as when the app opens. */ + fun started(now: Long): LoadGate = LoadGate(GatePhase.Waiting, now) + } +} diff --git a/android/core/src/main/kotlin/cc/hafa/reader/core/Model.kt b/android/core/src/main/kotlin/cc/hafa/reader/core/Model.kt index 5bf8dec..9cb8b72 100644 --- a/android/core/src/main/kotlin/cc/hafa/reader/core/Model.kt +++ b/android/core/src/main/kotlin/cc/hafa/reader/core/Model.kt @@ -144,6 +144,16 @@ sealed interface FeedProblem { override val code = "not-https" } + /** What was typed is a plain http address, which is never added. */ + data object HttpRefused : FeedProblem { + override val code = "http-refused" + } + + /** A followed feed's address is plain http, and no https version of it was found. */ + data object HttpFeed : FeedProblem { + override val code = "http-feed" + } + /** An answer that is not a feed, while adding one. */ data object NotAFeed : FeedProblem { override val code = "not-a-feed" @@ -170,6 +180,8 @@ fun feedProblemFromCode(code: String): FeedProblem? = code == FeedProblem.NotAFeed.code -> FeedProblem.NotAFeed code == FeedProblem.NotWebAddress.code -> FeedProblem.NotWebAddress code == FeedProblem.NotHttps.code -> FeedProblem.NotHttps + code == FeedProblem.HttpRefused.code -> FeedProblem.HttpRefused + code == FeedProblem.HttpFeed.code -> FeedProblem.HttpFeed code == FeedProblem.Unreadable.code -> FeedProblem.Unreadable code.startsWith("status:") -> code.removePrefix("status:").toIntOrNull()?.let(FeedProblem::Status) ?: FeedProblem.NoAnswer else -> FeedProblem.NoAnswer diff --git a/android/core/src/main/kotlin/cc/hafa/reader/core/Opml.kt b/android/core/src/main/kotlin/cc/hafa/reader/core/Opml.kt index 88b806f..09216ae 100644 --- a/android/core/src/main/kotlin/cc/hafa/reader/core/Opml.kt +++ b/android/core/src/main/kotlin/cc/hafa/reader/core/Opml.kt @@ -13,6 +13,8 @@ data class OpmlFeed( val title: String, /** The folder the feed sits in; empty for none. */ val folder: String, + /** Address of the feed's site; empty when the file gives none. */ + val siteUrl: String = "", ) /** An attribute's value whatever the case of its name, as exporters vary; null when absent. */ @@ -27,7 +29,7 @@ private fun collect(parent: Element, folder: String, feeds: MutableList" + val site = if (feed.siteUrl.isEmpty()) "" else " htmlUrl=\"${escapeAttribute(feed.siteUrl)}\"" + return "$indent" } /** Write feeds as OPML text, listing each group's feeds inside a folder outline and loose feeds after. */ @@ -79,4 +82,4 @@ fun buildOpml(feeds: List): String { } /** The feeds to export, with their names and groups. */ -fun opmlFeedsOf(feeds: List): List = feeds.map { feed -> OpmlFeed(feed.url, feed.title, feed.folder) } +fun opmlFeedsOf(feeds: List): List = feeds.map { feed -> OpmlFeed(feed.url, feed.title, feed.folder, feed.siteUrl) } diff --git a/android/core/src/main/kotlin/cc/hafa/reader/core/ReaderModel.kt b/android/core/src/main/kotlin/cc/hafa/reader/core/ReaderModel.kt index dbfa027..09a9fa7 100644 --- a/android/core/src/main/kotlin/cc/hafa/reader/core/ReaderModel.kt +++ b/android/core/src/main/kotlin/cc/hafa/reader/core/ReaderModel.kt @@ -90,16 +90,40 @@ interface ReaderPreferences { /** Key of the last view shown; see [viewKey]. */ var lastView: String + + /** The font for article text and titles on this device; not synced. */ + var readingFont: ReadingFont +} + +/** The font for article text and titles; the rest of the interface stays in Public Sans. */ +enum class ReadingFont { + /** Literata, the default. */ + Literata, + + /** Public Sans, the interface's own face. */ + PublicSans, } /** How an OPML import went. */ data class ImportResult( - /** Feeds the file lists. */ + /** Feeds the file lists with a web address. */ val found: Int, - /** How many of them were new. */ + /** How many of them were new and added. */ val added: Int, + /** Plain http feeds left out because no https version was found. */ + val skipped: Int = 0, ) +/** A feed with the http problem when its address is plain http, and without it otherwise. */ +fun markedHttp(feed: Feed): Feed = + if (isPlainHttp(feed.url)) { + feed.copy(error = FeedProblem.HttpFeed.code) + } else if (feed.error == FeedProblem.HttpFeed.code) { + feed.copy(error = "") + } else { + feed + } + /** Shortest time the refreshing marks stay on, so a quick check is still seen. */ private const val MIN_REFRESH_MS = 700L private const val MAX_DISCOVERY_TRIES = 12 @@ -110,12 +134,13 @@ fun hasScheme(text: String): Boolean = SCHEME_PATTERN.containsMatchIn(text) /** * What a person typed or a file listed, as an address: https added when no - * scheme is given, plain http upgraded to https, and empty when it is not a - * web address; the same text the web app makes of it, so feed ids match. + * scheme is given, plain http kept as http (so it can be refused or tried + * over https), and empty when it is not a web address; the same text the web + * app makes of it, so feed ids match. */ fun normalizeUrl(input: String): String { val trimmed = jsTrim(input) - return upgradedAddress(if (hasScheme(trimmed)) trimmed else "https://$trimmed") + return webAddress(if (hasScheme(trimmed)) trimmed else "https://$trimmed") } private val logger: Logger = Logger.getLogger("cc.hafa.reader") @@ -155,6 +180,8 @@ private fun hostOf(address: String): String = address.toHttpUrlOrNull()?.host ?: * a read article stays listed until the view changes or feeds are refreshed; * marking an open article unread by hand sticks when it is left. The sidebar * order is worked out at load and after each refresh, and held in between. + * Articles and feeds that arrive in the background wait for the lists to be + * settled (see [settle]), so rows never move under the person. * * [savingData] says whether the device asks to save data (Data Saver on * Android); then no neighbor's body is downloaded and no picture fetched ahead. @@ -179,6 +206,10 @@ class ReaderModel( private val writes = Channel Unit>(Channel.UNLIMITED) private val manual: MutableSet = ConcurrentHashMap.newKeySet() private val iconsTried: MutableSet = ConcurrentHashMap.newKeySet() + // Plain http feeds tried over https since the reader started with a definite answer, or being tried, by id. + private val httpTried: MutableSet = ConcurrentHashMap.newKeySet() + // Tries of each plain http feed's https address that got no definite answer since the reader started, by id. + private val httpMisses = ConcurrentHashMap() private val applyLock = Mutex() // Held around every change to the feeds and the save it queues, so edits and check results never overwrite each other. private val feedEdits = Any() @@ -248,7 +279,14 @@ class ReaderModel( override suspend fun clear() = clearLocal() } - private val queue = CheckQueue(scope, clock, ::entryFeed, ::checkFeed, ::applyChecks) + private val queue = CheckQueue( + scope, + clock, + ::entryFeed, + ::checkFeed, + ::applyChecks, + onBusy = { busy -> update { current -> current.copy(checking = busy) } }, + ) init { scope.launch { @@ -269,7 +307,12 @@ class ReaderModel( suspend fun load() { loadLock.withLock { if (!state.value.loaded) { - val feeds = store.loadFeeds() + val stored = store.loadFeeds() + val feeds = stored.map(::markedHttp) + val remarked = feeds.filterIndexed { index, feed -> feed != stored[index] } + if (remarked.isNotEmpty()) { + persist { store.putFeeds(remarked) } + } val entries = store.loadEntries() val saved = store.loadSettings() val settings = saved.mapValues { (_, item) -> item.settings } @@ -317,6 +360,7 @@ class ReaderModel( first.entries === second.entries && first.feeds === second.feeds && first.settings === second.settings && first.pinned === second.pinned && first.heldTopics === second.heldTopics && first.frozenRank === second.frozenRank && + first.heldEntries === second.heldEntries && first.heldFeeds === second.heldFeeds && first.selection == second.selection && first.unreadOnly == second.unreadOnly && first.currentId == second.currentId ) @@ -330,20 +374,82 @@ class ReaderModel( if (before.currentId.isNotEmpty()) { close() } - update { current -> current.copy(pinned = emptySet(), heldTopics = emptyMap()) } + update { current -> current.copy(pinned = emptySet(), heldTopics = emptyMap(), heldEntries = current.heldEntries?.let { current.entryIds() }) } } preferences.lastView = viewKey(chosen) } /** Turn the unread-only switch on or off, and remember it; turning it on closes an open article already read. */ fun setUnreadOnly(next: Boolean) { - update { current -> current.copy(unreadOnly = next, pinned = emptySet(), heldTopics = emptyMap()) } + update { current -> + current.copy( + unreadOnly = next, + pinned = emptySet(), + heldTopics = emptyMap(), + heldEntries = current.heldEntries?.let { current.entryIds() }, + heldFeeds = current.heldFeeds?.let { current.feedIds() }, + ) + } if (next && entryById(state.value.currentId)?.read == true) { close() } preferences.unreadOnly = next } + /** + * Let everything that has arrived into the lists, as when the loading + * screen lets the person in and after a refresh: the article list and the + * sidebar take in new articles and feeds, and the sidebar's order is + * worked out again. + */ + fun settle() { + update { current -> + current.copy( + frozenRank = frozenRankOf(current.feeds, current.entries, current.settings), + heldEntries = current.entryIds(), + heldFeeds = current.feedIds(), + ) + } + } + + private fun ReaderState.entryIds(): Set = entries.mapTo(HashSet()) { entry -> entry.id } + + private fun ReaderState.feedIds(): Set = feeds.mapTo(HashSet()) { feed -> feed.id } + + /** + * [next], which adds what arrived in the background to [current], with the + * lists held: a feed the sidebar does not list now stays out until it is + * next settled, so what arrives adds no rows, while a list or sidebar that + * showed nothing takes it in, since no row can move. + */ + private fun heldArrival(current: ReaderState, next: ReaderState): ReaderState { + val listShowed = viewOf(current).visible.isNotEmpty() + val unread = unreadCounts(current.entries, current.feeds, current.settings).byFeed + val listedNow = current.feeds.filter { feed -> feedListed(current, unread, feed) } + val sidebarEmpty = listedNow.isEmpty() + return next.copy( + heldEntries = if (current.heldEntries != null && !listShowed) next.entryIds() else next.heldEntries, + heldFeeds = if (current.heldFeeds == null) { + null + } else if (sidebarEmpty) { + next.feedIds() + } else { + listedNow.mapTo(HashSet()) { feed -> feed.id } + }, + frozenRank = if (current.heldFeeds != null && sidebarEmpty) frozenRankOf(next.feeds, next.entries, next.settings) else next.frozenRank, + ) + } + + /** Show what the person just added: the list takes in every article that has arrived, and the sidebar lists the feed. */ + private fun reveal(feedId: String) { + update { current -> + current.copy( + heldEntries = current.heldEntries?.let { current.entryIds() }, + heldFeeds = current.heldFeeds?.let { held -> held + feedId }, + ) + } + } + /** Change the current view's settings and save them. */ fun changeSettings(change: (ViewSettings) -> ViewSettings) { val key = viewKey(state.value.selection) @@ -656,30 +762,13 @@ class ReaderModel( if (open.unreadOnly && open.selection != Selection.Starred && entryById(open.currentId)?.read == true) { close() } + if (state.value.feeds.any { feed -> isPlainHttp(feed.url) && feed.id !in httpTried }) { + scope.launch { upgradeHttpFeeds() } + } val started = clock() try { - val now = clock() - val chosen = state.value.feeds.filter { feed -> feedIds.isEmpty() || feed.id in feedIds } - val requests = if (chosen.size <= SMALL_REFRESH) { - chosen.map { feed -> CheckRequest(feed.id, 1.0, foreground = true, timeoutMs = FOREGROUND_TIMEOUT_MS) } - } else { - val plan = planRefresh(chosen, now) { feed -> feed.schedule } - plan.ordered.mapIndexed { index, feed -> - val waited = index < plan.foreground - CheckRequest( - feed.id, - plan.probabilities[index], - foreground = waited, - timeoutMs = if (waited) FOREGROUND_TIMEOUT_MS else FETCH_TIMEOUT_MS, - ) - } - } - val checks = queue.enqueue(requests) - checks.filter { (_, request) -> request.foreground }.forEach { (check, _) -> check.settled.await() } - if (queue.busy()) { - onBackgroundWork() - } - update { current -> current.copy(frozenRank = frozenRankOf(current.feeds, current.entries, current.settings)) } + checkNow(feedIds) + settle() } finally { try { val remaining = minRefreshMillis - (clock() - started) @@ -692,26 +781,74 @@ class ReaderModel( } } + /** + * Check feeds now (every feed when [feedIds] is empty) and wait for the + * first ones (see [planRefresh]), showing nothing while they are checked. + */ + private suspend fun checkNow(feedIds: List) { + val now = clock() + // a plain http feed is never fetched; its https address is tried apart from the schedule + val chosen = state.value.feeds.filter { feed -> (feedIds.isEmpty() || feed.id in feedIds) && !isPlainHttp(feed.url) } + val requests = if (chosen.size <= SMALL_REFRESH) { + chosen.map { feed -> CheckRequest(feed.id, 1.0, foreground = true, timeoutMs = FOREGROUND_TIMEOUT_MS) } + } else { + val plan = planRefresh(chosen, now) { feed -> feed.schedule } + plan.ordered.mapIndexed { index, feed -> + val waited = index < plan.foreground + CheckRequest( + feed.id, + plan.probabilities[index], + foreground = waited, + timeoutMs = if (waited) FOREGROUND_TIMEOUT_MS else FETCH_TIMEOUT_MS, + ) + } + } + val checks = queue.enqueue(requests) + checks.filter { (_, request) -> request.foreground }.forEach { (check, _) -> check.settled.await() } + if (queue.busy()) { + onBackgroundWork() + } + } + /** Check the feeds in the current view now. */ suspend fun refreshView() { refresh(view().viewFeedIds) } + /** + * Start the checks the app makes when it opens, without waiting for them: + * load, try plain http feeds over https, and queue the feeds whose schedule + * says they are due. Nothing shows as refreshing, the list keeps its rows + * and the sidebar keeps the order worked out at load. + */ + suspend fun openChecks() { + load() + if (state.value.feeds.any { feed -> isPlainHttp(feed.url) && feed.id !in httpTried }) { + scope.launch { upgradeHttpFeeds() } + } + queue.enqueue(dueRequests(clock())) + if (queue.busy()) { + onBackgroundWork() + } + } + /** Check the feeds whose schedule says they are due, through the same queue, and wait until they are saved. */ suspend fun refreshDue() { val now = clock() - val due = state.value.loaded || store.loadFeeds().any { feed -> feed.schedule.nextFetch <= now } - if (!due) { - return + val due = state.value.loaded || store.loadFeeds().any { feed -> feed.schedule.nextFetch <= now && !isPlainHttp(feed.url) } + if (due) { + load() + queue.enqueue(dueRequests(now)).forEach { (check, _) -> check.stored.await() } } - load() - val requests = state.value.feeds - .filter { feed -> feed.schedule.nextFetch <= now } + } + + /** Background checks of the feeds due at [now]; plain http feeds are never due. */ + private fun dueRequests(now: Long): List = + state.value.feeds + .filter { feed -> feed.schedule.nextFetch <= now && !isPlainHttp(feed.url) } .map { feed -> CheckRequest(feed.id, refreshProbability(feed.schedule, now), foreground = false, timeoutMs = FETCH_TIMEOUT_MS) } - queue.enqueue(requests).forEach { (check, _) -> check.stored.await() } - } /** Wait until every queued check, including those a refresh left to the background, is saved. */ suspend fun awaitChecks() { @@ -728,13 +865,25 @@ class ReaderModel( update { current -> current.copy(adding = true) } try { val start = normalizeUrl(input) - if (start.toHttpUrlOrNull()?.isHttps != true) { + if (isPlainHttp(start)) { + throw FeedProblemException(FeedProblem.HttpRefused) + } else if (start.toHttpUrlOrNull()?.isHttps != true) { throw FeedProblemException(FeedProblem.NotWebAddress) } val found = discover(start) val id = feedId(found.url) - val feed = if (state.value.feeds.none { feed -> feed.id == id }) { - Feed( + val addresses = listOf(found.url) + val existing = followed(state.value.feeds, id, addresses) + val plain = plainAddress(found.url) + // a feed followed at the plain http form of this address moves here when it is the same feed; a different one stays as it is + val atHttp = state.value.feeds.firstOrNull { feed -> feed.url == plain && sameFeed(knownOf(feed), found.parsed) } + if (existing != null) { + select(Selection.FeedView(existing.id)) + } else if (atHttp != null) { + moveToHttps(listOf(atHttp to HttpsMatch(found.url, found.parsed, found.outcome))) + select(Selection.FeedView(atHttp.id)) + } else { + val feed = Feed( id = id, url = found.url, title = found.parsed.title.ifEmpty { hostOf(found.url) }, @@ -745,70 +894,85 @@ class ReaderModel( error = "", updatedAt = clock(), ) - } else { - null - } - // The same feed may have come from the account while the site was being looked up. - val added = feed != null && synchronized(feedEdits) { - var fresh = false - update { current -> - fresh = current.feeds.none { known -> known.id == id } - if (fresh) current.copy(feeds = current.feeds + feed) else current + // The same feed may have come from the account while the site was being looked up. + val added = synchronized(feedEdits) { + var fresh = false + update { current -> + fresh = followed(current.feeds, id, addresses) == null + if (fresh) current.copy(feeds = current.feeds + feed) else current + } + if (fresh) { + persist { store.putFeeds(listOf(feed)) } + } + fresh } - if (fresh) { - persist { store.putFeeds(listOf(feed)) } + if (added) { + forget(listOf("$FEED_TOMBSTONE$id")) + sendToSync(LocalChange(feeds = listOf(feed))) + val now = clock() + val result = withContext(workDispatcher) { checkOutcome(feed.schedule, feed.url, found.outcome, now) } + // the sidebar keeps its order, with the new feed last, until the next refresh + applyChecks(listOf(FinishedCheck(feed, result, now))) } - fresh + select(Selection.FeedView(followed(state.value.feeds, id, addresses)?.id ?: id)) } - if (feed != null && added) { - forget(listOf("$FEED_TOMBSTONE$id")) - sendToSync(LocalChange(feeds = listOf(feed))) - val now = clock() - val result = withContext(workDispatcher) { checkOutcome(feed.schedule, feed.url, found.outcome, now) } - // the sidebar keeps its order, with the new feed last, until the next refresh - applyChecks(listOf(FinishedCheck(feed, result, now))) + val shown = state.value.selection + if (shown is Selection.FeedView) { + reveal(shown.id) } - select(Selection.FeedView(id)) } finally { update { current -> current.copy(adding = false) } } } /** - * Subscribe to every feed in an OPML file and start checking them. Returns - * how many feeds the file lists and how many of them were new. + * Subscribe to every feed in an OPML file and start checking them: into + * [group] when given (empty for no group), else into the file's own + * folders. A plain http feed is tried over https and added there when it + * serves the same feed; otherwise it is left out. Returns how many feeds + * the file lists, how many of them were new, and how many http feeds were + * left out. */ - suspend fun importOpml(source: String): ImportResult { + suspend fun importOpml(source: String, group: String? = null): ImportResult { load() update { current -> current.copy(adding = true) } try { - return importParsed(withContext(workDispatcher) { prepareImport(source) }) + val listed = withContext(workDispatcher) { + parseOpml(source).mapNotNull { item -> normalizeUrl(item.url).ifEmpty { null }?.let { url -> item.copy(url = url) } } + } + val following = state.value.feeds.mapTo(HashSet()) { feed -> feed.url } + val resolved = mapConcurrently(listed, PROBE_CONCURRENCY) { item -> + val secure = upgradedAddress(item.url) + val url = if (isPlainHttp(item.url) && item.url !in following && secure !in following) { + probe(KnownFeed(item.url, item.title, item.siteUrl)).match?.url ?: "" + } else { + secure + } + item to url + } + return importResolved(resolved, group) } finally { update { current -> current.copy(adding = false) } } } - /** OPML feeds with web addresses, each with its id. */ - private fun prepareImport(source: String): List> = - parseOpml(source) - .map { item -> item.copy(url = normalizeUrl(item.url)) } - .filter { item -> item.url.toHttpUrlOrNull() != null } - .map { item -> item to feedId(item.url) } - - private fun importParsed(items: List>): ImportResult { - val known = state.value.feeds.mapTo(HashSet()) { feed -> feed.id } + /** Add listed feeds, each with the https address it resolved to (empty when none was found). */ + private fun importResolved(items: List>, group: String?): ImportResult { + val usable = items.filter { (_, url) -> url.toHttpUrlOrNull()?.isHttps == true } + val feeds = state.value.feeds val added = mutableListOf() - for ((item, id) in items) { - if (known.add(id)) { + for ((item, url) in usable) { + val id = feedId(url) + if (followed(feeds, id, listOf(url, item.url)) == null && added.none { feed -> feed.id == id }) { added.add( Feed( id = id, - url = item.url, - title = item.title.ifEmpty { hostOf(item.url) }, - siteUrl = "", - icon = faviconFor(item.url), + url = url, + title = item.title.ifEmpty { hostOf(url) }, + siteUrl = upgradedAddress(item.siteUrl), + icon = faviconFor(url), renamed = false, - folder = item.folder.trim(), + folder = (group ?: item.folder).trim(), error = "", updatedAt = clock(), ), @@ -824,21 +988,138 @@ class ReaderModel( } forget(added.map { feed -> "$FEED_TOMBSTONE${feed.id}" }) sendToSync(LocalChange(feeds = added)) - scope.launch { refresh() } - return ImportResult(found = items.size, added = added.size) + if (added.isNotEmpty()) { + // The loading screen holds the person until the new feeds' first check is in. + update { current -> current.copy(importing = current.importing + 1) } + scope.launch { + try { + checkNow(added.map { feed -> feed.id }) + } finally { + update { current -> current.copy(importing = current.importing - 1) } + } + } + } + return ImportResult(found = items.size, added = added.size, skipped = items.size - usable.size) } - /** Search Feedly for feeds about [query]; empty when nothing is found or it does not answer. */ + /** + * Search Feedly for feeds about [query]; empty when nothing is found or it + * does not answer. A plain http result is shown at its https address only + * when that serves the same feed, and left out otherwise. + */ suspend fun searchFeeds(query: String): List { update { current -> current.copy(adding = true) } try { val outcome = fetcher.fetch(feedSearchAddress(query), "", "") - return if (outcome.status == 200) withContext(workDispatcher) { parseFeedSearch(outcome.text) } else emptyList() + val found = if (outcome.status == 200) withContext(workDispatcher) { parseFeedSearch(outcome.text) } else emptyList() + val checked = mapConcurrently(found, PROBE_CONCURRENCY) { hit -> + if (isPlainHttp(hit.url)) hit.copy(url = probe(KnownFeed(hit.url, hit.title, hit.siteUrl)).match?.url ?: "") else hit + } + return checked.filter { hit -> hit.url.isNotEmpty() }.distinctBy { hit -> hit.url } } finally { update { current -> current.copy(adding = false) } } } + /** What one try of a plain http feed at its https address found. */ + private data class Probe( + /** The https address found to serve the same feed; null when none. */ + val match: HttpsMatch?, + /** Whether the answer settles the question, so a failure that does not is tried again later. */ + val settled: Boolean, + ) + + /** Try a plain http feed at its https address. */ + private suspend fun probe(known: KnownFeed): Probe = withContext(workDispatcher) { + var answer: FetchOutcome? = null + val match = probeHttps(known) { url, etag, lastModified, timeoutMs -> + fetcher.fetch(url, etag, lastModified, timeoutMs).also { outcome -> answer = outcome } + } + // an address that may not be tried never will be, so there is nothing to wait for + Probe(match, match != null || !probeAllowed(known.url) || settledAnswer(answer)) + } + + /** What is known of a followed feed to tell it from another; a name the person gave is not the feed's own. */ + private fun knownOf(feed: Feed): KnownFeed { + val held = state.value.entries.filter { entry -> entry.feedId == feed.id } + return KnownFeed(feed.url, if (feed.renamed) "" else feed.title, feed.siteUrl, held.map { entry -> entry.guid }, held.map { entry -> entry.url }) + } + + /** + * Try each plain http feed, once while the reader runs, at its https + * address; one whose try got no definite answer (offline, a timeout, a + * failing server) is tried again on the next refresh, up to + * [UNSETTLED_TRIES] tries. One that serves the + * same feed moves there (see [moveToHttps]). One that does not, or whose + * https address is already followed, keeps the http problem. + */ + private suspend fun upgradeHttpFeeds() { + val waiting = state.value.feeds.filter { feed -> isPlainHttp(feed.url) && httpTried.add(feed.id) } + val matches = mapConcurrently(waiting, PROBE_CONCURRENCY) { feed -> + val probe = probe(knownOf(feed)) + val misses = httpMisses.merge(feed.id, if (probe.settled) 0 else 1, Int::plus) ?: 0 + if (!probe.settled && misses < UNSETTLED_TRIES) { + httpTried.remove(feed.id) + } + probe.match?.let { match -> feed to match } + }.filterNotNull() + if (matches.isNotEmpty()) { + moveToHttps(matches) + } + } + + /** + * Move plain http feeds to the https addresses found to serve them: same + * id, articles, marks and group, with the new address saved and sent like + * a person's change, so other devices move too, and the answer taken as + * its first check. A feed whose address changed meanwhile, or whose https + * address is already followed, stays. + */ + private suspend fun moveToHttps(matches: List>) { + val found = matches.associate { (feed, match) -> feed.id to Triple(feed.url, match, feedId(match.url)) } + val now = clock() + val moved = mutableListOf>() + synchronized(feedEdits) { + update { current -> + moved.clear() + val ids = current.feeds.mapTo(HashSet()) { feed -> feed.id } + val idsByUrl = current.feeds.groupBy({ feed -> feed.url }, { feed -> feed.id }) + val feeds = current.feeds.map { feed -> + val candidate = found[feed.id]?.takeIf { (triedUrl, _, _) -> triedUrl == feed.url } + if (candidate == null) { + feed + } else { + val (_, match, newId) = candidate + val taken = (newId != feed.id && newId in ids) || + idsByUrl[match.url].orEmpty().any { id -> id != feed.id } + if (taken) { + feed + } else { + val schedule = feed.schedule.copy(etag = "", lastModified = "", hash = "", nextFetch = 0) + feed.copy(url = match.url, error = "", schedule = schedule, updatedAt = now).also { updated -> moved.add(updated to match) } + } + } + } + if (moved.isEmpty()) current else current.copy(feeds = feeds) + } + if (moved.isNotEmpty()) { + val feeds = moved.map { (feed, _) -> feed } + persist { store.putFeeds(feeds) } + sendToSync(LocalChange(feeds = feeds)) + } + } + if (moved.isNotEmpty()) { + val checks = withContext(workDispatcher) { + moved.map { (feed, match) -> FinishedCheck(feed, checkOutcome(feed.schedule, feed.url, match.outcome, now), now) } + } + applyChecks(checks) + } + } + + /** A feed among [feeds] with this id or one of these addresses. */ + private fun followed(feeds: List, id: String, urls: List): Feed? = + feeds.firstOrNull { feed -> feed.id == id || feed.url in urls } + /** * A feed's picture would not load: look once for the one its site * announces, and use it; nothing changes when the site names none. The new @@ -929,20 +1210,24 @@ class ReaderModel( } } - /** Store changes from the account; nothing here is sent back. */ + /** + * Store changes from the account; nothing here is sent back. New articles + * and feeds wait for the lists to be settled. + */ private suspend fun applyRemote(change: RemoteChange) { for ((id, removedAt) in change.removedFeeds) { dropFeed(id, removedAt) } val forgotten = mutableListOf() if (change.feeds.isNotEmpty()) { - val byId = change.feeds.associateBy { feed -> feed.id } + val incoming = change.feeds.map(::markedHttp) + val byId = incoming.associateBy { feed -> feed.id } synchronized(feedEdits) { update { current -> val known = current.feeds.mapTo(HashSet()) { feed -> feed.id } - current.copy(feeds = current.feeds.map { feed -> byId[feed.id] ?: feed } + change.feeds.filter { feed -> feed.id !in known }) + heldArrival(current, current.copy(feeds = current.feeds.map { feed -> byId[feed.id] ?: feed } + incoming.filter { feed -> feed.id !in known })) } - persist { store.putFeeds(change.feeds) } + persist { store.putFeeds(incoming) } } forgotten.addAll(change.feeds.map { feed -> "$FEED_TOMBSTONE${feed.id}" }) } @@ -951,7 +1236,16 @@ class ReaderModel( val byId = incoming.associateBy { entry -> entry.id } update { current -> val known = current.entries.mapTo(HashSet()) { entry -> entry.id } - current.copy(entries = current.entries.map { entry -> byId[entry.id] ?: entry } + incoming.filter { entry -> entry.id !in known }) + val shown = viewOf(current).visible.mapTo(HashSet()) { entry -> entry.id } + // An article read elsewhere stays listed until the view changes, as one read here does. + val readElsewhere = incoming.filter { entry -> entry.read && entry.id in shown }.map { entry -> entry.id } + heldArrival( + current, + current.copy( + entries = current.entries.map { entry -> byId[entry.id] ?: entry } + incoming.filter { entry -> entry.id !in known }, + pinned = if (readElsewhere.isEmpty()) current.pinned else current.pinned + readElsewhere, + ), + ) } persist { store.updateEntries(incoming) } } @@ -992,6 +1286,8 @@ class ReaderModel( aheadBodies = emptyMap(), aheadPictures = emptyList(), frozenRank = emptyMap(), + heldEntries = null, + heldFeeds = null, ) } persist { store.clear() } @@ -1042,10 +1338,11 @@ class ReaderModel( } savedFeeds = changed savedEntries = added - current.copy( + val next = current.copy( feeds = current.feeds.map { feed -> byId[feed.id] ?: feed }, entries = if (added.isEmpty()) current.entries else current.entries + added, ) + if (added.isEmpty()) next else heldArrival(current, next) } val bodies = pulled.flatMap { found -> found.bodies.entries }.associate { (id, html) -> id to html } if (savedFeeds.isNotEmpty() || savedEntries.isNotEmpty()) { diff --git a/android/core/src/main/kotlin/cc/hafa/reader/core/ReaderView.kt b/android/core/src/main/kotlin/cc/hafa/reader/core/ReaderView.kt index e20722f..d2a274b 100644 --- a/android/core/src/main/kotlin/cc/hafa/reader/core/ReaderView.kt +++ b/android/core/src/main/kotlin/cc/hafa/reader/core/ReaderView.kt @@ -39,15 +39,40 @@ data class ReaderState( val loaded: Boolean = false, /** Refreshes under way. */ val refreshCount: Int = 0, + /** Whether any feed is queued, being checked or waiting to be saved, for a refresh or in the background. */ + val checking: Boolean = false, /** Whether a feed is being added or imported. */ val adding: Boolean = false, /** Sidebar positions fixed at load and at each refresh; rows added later follow them. */ val frozenRank: Map? = null, + /** + * Ids of the articles the list may show; ones that arrive later wait until + * the list is next settled, so rows never move under the person. Null + * lets every article in. + */ + val heldEntries: Set? = null, + /** Ids of the feeds the sidebar may list, held like [heldEntries]; null lets every feed in. */ + val heldFeeds: Set? = null, + /** Imports whose feeds are being checked for the first time. */ + val importing: Int = 0, ) { /** Whether feeds are being checked now. */ val refreshing: Boolean get() = refreshCount > 0 } +/** + * When a refresh starts to help: once the first feed is overdue (see + * [overdueAt]), or [Long.MAX_VALUE] while nothing is overdue or feeds are + * being checked, so the suggestion clears once a refresh has saved them. A + * plain http feed is never on the schedule, so it is never overdue. + */ +fun refreshSuggestedAt(state: ReaderState): Long = + if (state.refreshing || state.checking) { + Long.MAX_VALUE + } else { + overdueAt(state.feeds.filter { feed -> !isPlainHttp(feed.url) }.map { feed -> feed.schedule.nextFetch }) + } + /** A feed in the sidebar, with what it is ordered by. */ data class FeedRow( /** The feed. */ @@ -112,8 +137,8 @@ data class ReaderView( val baseEntries: List, /** Entries in the current view, in the chosen order, as listed. */ val visible: List, - /** The topics in the current view, commonest first. */ - val topics: List, + /** The topics in the current view, commonest first, kept as the same object while they stay the same. */ + val topicOrder: TopicOrder, /** Name of the current view. */ val viewTitle: ViewTitle, /** Feeds in the current view; empty means every feed. */ @@ -336,6 +361,20 @@ val NEWEST_FIRST: Comparator = .thenBy { entry -> entry.url } .thenBy { entry -> entry.id } +@Volatile +private var lastTopicOrder: TopicOrder? = null + +/** The topics of [entries] in order, reusing the last order when the counts have not changed, so screens can skip. */ +private fun topicOrderOf(entries: List): TopicOrder { + val counts = topicCounts(entries) + val cached = lastTopicOrder + return if (cached != null && cached.counts == counts) { + cached + } else { + TopicOrder(counts).also { order -> lastTopicOrder = order } + } +} + /** Work out what the screens show from the reader's state. */ fun deriveView(state: ReaderState): ReaderView { val selection = state.selection @@ -348,9 +387,10 @@ fun deriveView(state: ReaderState): ReaderView { } val kept = baseEntries.filter { entry -> keeps(state, viewSettings, entry) }.map { entry -> entry.id }.toSet() val unreadOnly = state.unreadOnly && selection != Selection.Starred + val held = state.heldEntries val visible = ordered.filter { entry -> entry.id == state.currentId || - (entry.id in kept && !(unreadOnly && entry.read && entry.id !in state.pinned)) + ((held == null || entry.id in held) && entry.id in kept && !(unreadOnly && entry.read && entry.id !in state.pinned)) } val counts = unreadCounts(state.entries, state.feeds, state.settings) val unread = counts.byFeed @@ -372,7 +412,7 @@ fun deriveView(state: ReaderState): ReaderView { viewSettings = viewSettings, baseEntries = baseEntries, visible = visible, - topics = topicCounts(baseEntries), + topicOrder = topicOrderOf(baseEntries), viewTitle = viewTitle, viewFeedIds = viewFeedIds, current = state.entries.firstOrNull { entry -> entry.id == state.currentId }, @@ -405,11 +445,21 @@ data class SidebarRow( */ fun sidebarRows(state: ReaderState, view: ReaderView): List = listedRows(state, view.tree, view.unreadByFeed) +/** + * Whether the sidebar lists a feed, given each feed's [unread] count: it is + * the one open, or it was there when the sidebar was last settled and has + * something unread or unread only is off. + */ +fun feedListed(state: ReaderState, unread: Map, feed: Feed): Boolean { + val selection = state.selection + val held = state.heldFeeds + return (selection is Selection.FeedView && selection.id == feed.id) || + ((held == null || feed.id in held) && (!state.unreadOnly || (unread[feed.id] ?: 0) > 0)) +} + private fun listedRows(state: ReaderState, tree: List, unread: Map): List { val selection = state.selection - fun listed(feed: Feed): Boolean = - !state.unreadOnly || (unread[feed.id] ?: 0) > 0 || - (selection is Selection.FeedView && selection.id == feed.id) + fun listed(feed: Feed): Boolean = feedListed(state, unread, feed) val rows = mutableListOf() for (item in tree) { when (item) { diff --git a/android/core/src/main/kotlin/cc/hafa/reader/core/Resolve.kt b/android/core/src/main/kotlin/cc/hafa/reader/core/Resolve.kt index e016180..27b7293 100644 --- a/android/core/src/main/kotlin/cc/hafa/reader/core/Resolve.kt +++ b/android/core/src/main/kotlin/cc/hafa/reader/core/Resolve.kt @@ -31,9 +31,12 @@ private val MEDIUM_PATH_PATTERN = Regex("^/(@?[\\w.-]+)/?$") * escaped), as the shared `upgradedAddress` does; empty when it is not a web * address. */ -fun upgradedAddress(address: String): String { +fun upgradedAddress(address: String): String = browserWritten(httpsOnly(address)) { url -> url.isHttps } + +/** An http or https address that [accept] takes, written out as browsers write it; empty for anything else. */ +internal fun browserWritten(address: String, accept: (HttpUrl) -> Boolean): String { // OkHttp escapes "|" where browsers keep it, so it is carried through as a private character - val written = httpsOnly(address.replace("|", PIPE_STAND_IN)).toHttpUrlOrNull()?.takeIf { url -> url.isHttps }?.toString() ?: "" + val written = address.replace("|", PIPE_STAND_IN).toHttpUrlOrNull()?.takeIf(accept)?.toString() ?: "" val hash = written.indexOf('#') val whole = if (hash < 0) written else written.substring(0, hash + 1) + browserFragment(written.substring(hash + 1)) return whole.replace(ESCAPED_PIPE_STAND_IN, "|").replace(PIPE_STAND_IN, "|") diff --git a/android/core/src/main/kotlin/cc/hafa/reader/core/Schedule.kt b/android/core/src/main/kotlin/cc/hafa/reader/core/Schedule.kt index ea8eeb8..27577b4 100644 --- a/android/core/src/main/kotlin/cc/hafa/reader/core/Schedule.kt +++ b/android/core/src/main/kotlin/cc/hafa/reader/core/Schedule.kt @@ -218,6 +218,24 @@ fun chooseInterval(model: FeedModel, nowMs: Long): Double { } } +/** + * How late a feed's scheduled check may run before a refresh is suggested, in + * milliseconds. The background checks (about every 15 minutes here, every 5 in + * the browser extension) normally reach a due feed well within it, so only a + * feed they missed, as when the phone slept, counts. + */ +const val OVERDUE_MS = 2 * MIN_INTERVAL_MS + +/** + * When the first of these feeds becomes overdue: the earliest next check plus + * [OVERDUE_MS]; [Long.MAX_VALUE] when there are none. One pass over the next + * checks, so the caller can work it out on every change and wait once for it. + */ +fun overdueAt(nextChecks: Iterable): Long { + val earliest = nextChecks.minOrNull() + return if (earliest == null) Long.MAX_VALUE else earliest + OVERDUE_MS +} + /** Wait after the given number of failures in a row, doubling each time. */ fun retryInterval(failures: Int): Long = min(MAX_RETRY_MS.toDouble(), MIN_INTERVAL_MS * 2.0.pow(failures)).toLong() diff --git a/android/core/src/main/kotlin/cc/hafa/reader/core/TopicRows.kt b/android/core/src/main/kotlin/cc/hafa/reader/core/TopicRows.kt new file mode 100644 index 0000000..1207383 --- /dev/null +++ b/android/core/src/main/kotlin/cc/hafa/reader/core/TopicRows.kt @@ -0,0 +1,28 @@ +package cc.hafa.reader.core + +/** The most rows of topics shown before the topics scroll sideways. */ +const val TOPIC_ROWS = 3 + +/** Where one item goes in [wrappedRows]: its [row], counted from zero, and its [left] edge. */ +data class RowPosition(val row: Int, val left: Int) + +/** Where items of these widths go when laid left to right and wrapped at [available], with [gap] between neighbors. */ +fun wrappedRows(widths: List, available: Int, gap: Int): List { + val positions = ArrayList(widths.size) + var row = -1 + var used = 0 + for (width in widths) { + if (row < 0 || used + gap + width > available) { + row += 1 + positions.add(RowPosition(row, 0)) + used = width + } else { + positions.add(RowPosition(row, used + gap)) + used += gap + width + } + } + return positions +} + +/** The number of rows [positions] fill. */ +fun rowCount(positions: List): Int = positions.lastOrNull()?.row?.plus(1) ?: 0 diff --git a/android/core/src/main/kotlin/cc/hafa/reader/core/sync/AccountStore.kt b/android/core/src/main/kotlin/cc/hafa/reader/core/sync/AccountStore.kt index f472295..76407cf 100644 --- a/android/core/src/main/kotlin/cc/hafa/reader/core/sync/AccountStore.kt +++ b/android/core/src/main/kotlin/cc/hafa/reader/core/sync/AccountStore.kt @@ -120,7 +120,7 @@ interface AccountStore { /** The newest server write time among the account's entries, read from the server; zero for none. */ suspend fun newestEntrySyncedAt(): Long - /** Listen to entries written after [sinceMs] by the server's clock. */ + /** Listen to entries written after [sinceMs] by the server's clock; the first changes come even when there are none. */ fun listenEntries(sinceMs: Long, onChanges: (List) -> Unit, onError: (Throwable) -> Unit): Stop /** A page of entries newest first, after [after], read from the server. */ diff --git a/android/core/src/main/kotlin/cc/hafa/reader/core/sync/Documents.kt b/android/core/src/main/kotlin/cc/hafa/reader/core/sync/Documents.kt index d98d41d..eb521c9 100644 --- a/android/core/src/main/kotlin/cc/hafa/reader/core/sync/Documents.kt +++ b/android/core/src/main/kotlin/cc/hafa/reader/core/sync/Documents.kt @@ -6,6 +6,7 @@ import cc.hafa.reader.core.Feed import cc.hafa.reader.core.Sort import cc.hafa.reader.core.ViewSettings import cc.hafa.reader.core.upgradedAddress +import cc.hafa.reader.core.webAddress private const val DAY_MS = 24L * 60 * 60 * 1000 @@ -409,8 +410,8 @@ private fun hasTime(fields: Map): Boolean = fields["updatedAt"] is fun parseSubscription(fields: Map): SubscriptionDocument? { val stored = if (hasTime(fields)) { SubscriptionDocument( - // https only: plain http is upgraded, other addresses are dropped - url = upgradedAddress(text(fields, "url")), + // a plain http feed is kept as http, to be tried over https before any fetch; other addresses are dropped + url = webAddress(text(fields, "url")), title = text(fields, "title"), folder = text(fields, "folder"), renamed = flag(fields, "renamed"), diff --git a/android/core/src/main/kotlin/cc/hafa/reader/core/sync/Engine.kt b/android/core/src/main/kotlin/cc/hafa/reader/core/sync/Engine.kt index 669a4cd..2eb157c 100644 --- a/android/core/src/main/kotlin/cc/hafa/reader/core/sync/Engine.kt +++ b/android/core/src/main/kotlin/cc/hafa/reader/core/sync/Engine.kt @@ -221,6 +221,7 @@ class SyncEngine( session?.retention, if (known.uid == null) "" else known.photoUrl, session?.feedsKnown ?: false, + session?.caughtUp ?: false, ), ) } @@ -309,25 +310,27 @@ class SyncEngine( * everything on this device. */ fun deleteAccount() { - val account = uid ?: return - mutableUi.update { current -> current.copy(deleting = true, deleteFailed = false) } - scope.launch { - try { - // before anything stops, so a closed chooser leaves sync as it was - auth.ensureRecentSignIn() - emptyAccount(account) - if (preferences.get(ACCOUNT_KEY) == account) { - preferences.set(ACCOUNT_KEY, null) + val account = uid + if (account != null) { + mutableUi.update { current -> current.copy(deleting = true, deleteFailed = false) } + scope.launch { + try { + // before anything stops, so a closed chooser leaves sync as it was + auth.ensureRecentSignIn() + emptyAccount(account) + if (preferences.get(ACCOUNT_KEY) == account) { + preferences.set(ACCOUNT_KEY, null) + } + auth.signOut() + } catch (cancelled: CancellationException) { + throw cancelled + } catch (failure: Exception) { + logger.log(Level.WARNING, "could not delete the account", failure) + mutableUi.update { current -> current.copy(deleteFailed = true) } + } finally { + mutableUi.update { current -> current.copy(deleting = false) } + report() } - auth.signOut() - } catch (cancelled: CancellationException) { - throw cancelled - } catch (failure: Exception) { - logger.log(Level.WARNING, "could not delete the account", failure) - mutableUi.update { current -> current.copy(deleteFailed = true) } - } finally { - mutableUi.update { current -> current.copy(deleting = false) } - report() } } } diff --git a/android/core/src/main/kotlin/cc/hafa/reader/core/sync/Host.kt b/android/core/src/main/kotlin/cc/hafa/reader/core/sync/Host.kt index 45e542b..bc5c7be 100644 --- a/android/core/src/main/kotlin/cc/hafa/reader/core/sync/Host.kt +++ b/android/core/src/main/kotlin/cc/hafa/reader/core/sync/Host.kt @@ -41,6 +41,8 @@ data class SyncReport( val photoUrl: String = "", /** Whether the signed-in account's feeds have arrived from the server. */ val feedsKnown: Boolean = false, + /** Whether what the account had when this session began is stored here, all but entries older than the recent window; false when signed out. */ + val caughtUp: Boolean = false, ) /** Everything stored on this device, as sync reads it. */ diff --git a/android/core/src/main/kotlin/cc/hafa/reader/core/sync/Session.kt b/android/core/src/main/kotlin/cc/hafa/reader/core/sync/Session.kt index 237058d..1088216 100644 --- a/android/core/src/main/kotlin/cc/hafa/reader/core/sync/Session.kt +++ b/android/core/src/main/kotlin/cc/hafa/reader/core/sync/Session.kt @@ -120,6 +120,23 @@ class Session( var feedsKnown = false private set + // Whether the account's settings from the server, the entry listener's first changes, and the recent window of entries are stored here. + @Volatile + private var settingsIn = false + + @Volatile + private var entriesIn = false + + @Volatile + private var recentIn = false + + /** + * Whether what the account had when this session began is stored here: its + * feeds and settings from the server, the entry listener's first changes, + * and the recent window of entries; older ones may still be loading. + */ + val caughtUp: Boolean get() = feedsKnown && settingsIn && entriesIn && (recentIn || state.merged) + /** Whether something failed: a write, or a listener that has not come back yet. */ val failed: Boolean get() = writeFailed || broken.isNotEmpty() @@ -511,6 +528,10 @@ class Session( } applyRemote(RemoteChange(settings = pulled, removedSettings = removed)) queue(LocalChange(settings = merged.push.toMap(), removedSettings = merged.pushRemoved.toMap())) + if (settingsReconciled && !settingsIn) { + settingsIn = true + onStatus() + } } private fun listenEntries() { @@ -526,6 +547,10 @@ class Session( serially { try { onEntries(changes) + if (!entriesIn) { + entriesIn = true + onStatus() + } } catch (failure: Exception) { cursorHeld = true throw failure @@ -637,6 +662,7 @@ class Session( } val reached = upper if (!state.merged && reached != null && reached < cutoff) { + recentIn = true loadingOlder = true onStatus() delay(timing.olderPauseMs) diff --git a/android/core/src/test/kotlin/cc/hafa/reader/core/CheckQueueTest.kt b/android/core/src/test/kotlin/cc/hafa/reader/core/CheckQueueTest.kt index e907c56..4e08a16 100644 --- a/android/core/src/test/kotlin/cc/hafa/reader/core/CheckQueueTest.kt +++ b/android/core/src/test/kotlin/cc/hafa/reader/core/CheckQueueTest.kt @@ -28,6 +28,7 @@ private class Harness(private val test: TestScope, feeds: List, limits: Qu val calls = mutableListOf() val saves = mutableListOf>() + val busyReports = mutableListOf() private var now = 1_000_000L private var timers = mutableListOf Unit>>() private val byId = feeds.associateBy { item -> item.id } @@ -44,6 +45,7 @@ private class Harness(private val test: TestScope, feeds: List, limits: Qu save = { batch -> saves.add(batch.map { finished -> finished.feed.id }) }, limits = limits, setTimer = { delayMs, callback -> timers.add(now + delayMs to callback) }, + onBusy = { busy -> busyReports.add(busy) }, ) fun settle() = test.runCurrent() @@ -96,6 +98,21 @@ class CheckQueueTest { assertEquals(listOf("broken", "broken"), calls) } + @Test + fun `busy is reported once when checks start and once when the last is saved`() = runTest { + val first = feed("a") + val second = feed("b") + val harness = Harness(this, listOf(first, second)) + harness.queue.enqueue(listOf(harness.request(first), harness.request(second))) + harness.settle() + assertEquals(listOf(true), harness.busyReports) + harness.answer("a", done(first)) + assertEquals(listOf(true), harness.busyReports) + harness.answer("b", done(second)) + assertEquals(listOf("a", "b"), harness.saves.flatten()) + assertEquals(listOf(true, false), harness.busyReports) + } + @Test fun `at most the limit run at once, and at most two to one host`() = runTest { val feeds = listOf(feed("a", "same.example"), feed("b", "same.example"), feed("c", "same.example"), feed("d"), feed("e")) diff --git a/android/core/src/test/kotlin/cc/hafa/reader/core/FeedSearchTest.kt b/android/core/src/test/kotlin/cc/hafa/reader/core/FeedSearchTest.kt index 2f40929..2fb59ba 100644 --- a/android/core/src/test/kotlin/cc/hafa/reader/core/FeedSearchTest.kt +++ b/android/core/src/test/kotlin/cc/hafa/reader/core/FeedSearchTest.kt @@ -15,25 +15,26 @@ class FeedSearchTest { """.trimIndent() @Test - fun `hits are read with the feed prefix dropped, gaps filled and http moved to https`() { + fun `hits are read with the feed prefix dropped, gaps filled and http kept for a later try`() { assertEquals( listOf( - FeedHit("https://cooking.example/feed", "Cooking", "Recipes daily", "https://cooking.example/icon.png", 12345), - FeedHit("https://plain.example/rss", "feed/http://plain.example/rss", "", "", 0), + FeedHit("https://cooking.example/feed", "Cooking", "Recipes daily", "https://cooking.example/icon.png", 12345, "https://cooking.example/"), + FeedHit("http://plain.example/rss", "feed/http://plain.example/rss", "", "", 0), ), parseFeedSearch(answer), ) } @Test - fun `only feed ids count, http is upgraded and repeats are dropped`() { + fun `only feed ids with web addresses count, and repeats are dropped`() { val answer = """{"results":[ {"feedId":"feed/http://a.example/rss","title":"A"}, {"feedId":"feed/https://a.example/rss","title":"A again"}, + {"feedId":"feed/https://a.example/rss","title":"A once more"}, {"feedId":"topic/cooking","title":"Not a feed"}, {"feedId":"feed/ftp://b.example/rss","title":"Not web"} ]}""" - assertEquals(listOf("https://a.example/rss" to "A"), parseFeedSearch(answer).map { hit -> hit.url to hit.title }) + assertEquals(listOf("http://a.example/rss" to "A", "https://a.example/rss" to "A again"), parseFeedSearch(answer).map { hit -> hit.url to hit.title }) } @Test diff --git a/android/core/src/test/kotlin/cc/hafa/reader/core/FilterTest.kt b/android/core/src/test/kotlin/cc/hafa/reader/core/FilterTest.kt index c19c5c7..760820e 100644 --- a/android/core/src/test/kotlin/cc/hafa/reader/core/FilterTest.kt +++ b/android/core/src/test/kotlin/cc/hafa/reader/core/FilterTest.kt @@ -85,6 +85,41 @@ class FilterTest { ) } + @Test + fun `a topic an article lists twice counts once, and ties go by name`() { + val twice = story.copy(categories = listOf("Zoo", "zoo ", "Apple")) + assertEquals(listOf(TopicCount("Apple", 1), TopicCount("Zoo", 1)), topicCounts(listOf(twice))) + } + + @Test + fun `names go most counted first, then by name`() { + val counts = mapOf("budget" to 2, "senate" to 5, "night" to 2) + assertEquals( + listOf("senate", "budget", "night", "zoo"), + byCount(listOf("budget", "night", "senate", "zoo")) { name -> counts[name] ?: 0 }, + ) + } + + @Test + fun `an article's topics follow the view's counts, each once`() { + val counts = listOf(TopicCount("Politics", 3), TopicCount("Business", 2), TopicCount("U.S.", 2)) + assertEquals( + listOf("politics ", "Business", "U.S.", "US", "Zoo"), + articleTopics(listOf("U.S.", "Zoo", "politics ", "Business", "US"), TopicOrder(counts)), + ) + assertEquals(listOf("U.S."), articleTopics(listOf("U.S.", "u.s."), TopicOrder(counts))) + } + + @Test + fun `word counts count each word's articles and are kept while the text stays the same`() { + val other = Story("Night market opens", "Food stalls.", emptyList()) + val entries = listOf(story, other) + val counts = wordCounts(entries, listOf("night", "budget", "zoo")) + assertEquals(mapOf("night" to 2, "budget" to 1, "zoo" to 0), counts) + assertSame(counts, knownWordCounts(listOf(story, other.copy()), listOf("night", "budget", "zoo"))) + assertEquals(null, knownWordCounts(entries, listOf("night"))) + } + @Test fun `a list of words is compiled once and reused`() { val first = matcherFor(listOf("reuse check")) diff --git a/android/core/src/test/kotlin/cc/hafa/reader/core/HttpsRedirectTest.kt b/android/core/src/test/kotlin/cc/hafa/reader/core/HttpsRedirectTest.kt index 3efd24d..5e0e227 100644 --- a/android/core/src/test/kotlin/cc/hafa/reader/core/HttpsRedirectTest.kt +++ b/android/core/src/test/kotlin/cc/hafa/reader/core/HttpsRedirectTest.kt @@ -10,6 +10,7 @@ import kotlin.test.AfterTest import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertEquals +import kotlin.test.assertFalse import kotlin.test.assertTrue class HttpsRedirectTest { @@ -38,12 +39,26 @@ class HttpsRedirectTest { } @Test - fun `a redirect from https to plain http counts as no answer`() = runTest { + fun `a redirect from https to plain http is followed at https, never at http`() = runTest { + val upgraded = secure.url("/moved") + val target = upgraded.newBuilder().scheme("http").build() + secure.enqueue(MockResponse.Builder().code(302).addHeader("Location", target.toString()).build()) + secure.enqueue(MockResponse.Builder().body("").build()) + val outcome = fetcher.fetch(secure.url("/feed").toString(), "", "", FETCH_TIMEOUT_MS) + assertEquals(200, outcome.status) + assertEquals(upgraded.toString(), outcome.url) + secure.takeRequest() + assertEquals("/moved", secure.takeRequest().url.encodedPath) + } + + @Test + fun `a redirect to plain http with no https behind it fails without a plain request`() = runTest { secure.enqueue(MockResponse.Builder().code(302).addHeader("Location", plain.url("/feed").toString()).build()) val outcome = fetcher.fetch(secure.url("/feed").toString(), "", "", FETCH_TIMEOUT_MS) assertEquals(0, outcome.status) - assertTrue(outcome.insecure) - assertEquals(0, plain.requestCount) + assertFalse(outcome.insecure) + // The http server sees only the https attempt's handshake, which it cannot read as a request. + assertTrue(List(plain.requestCount) { plain.takeRequest() }.all { request -> request.failure != null }) } @Test diff --git a/android/core/src/test/kotlin/cc/hafa/reader/core/HttpsUpgradeTest.kt b/android/core/src/test/kotlin/cc/hafa/reader/core/HttpsUpgradeTest.kt new file mode 100644 index 0000000..18195ce --- /dev/null +++ b/android/core/src/test/kotlin/cc/hafa/reader/core/HttpsUpgradeTest.kt @@ -0,0 +1,141 @@ +package cc.hafa.reader.core + +import kotlinx.coroutines.delay +import kotlinx.coroutines.test.runTest +import kotlinx.serialization.json.Json +import kotlinx.serialization.json.JsonObject +import kotlinx.serialization.json.boolean +import kotlinx.serialization.json.int +import kotlinx.serialization.json.jsonArray +import kotlinx.serialization.json.jsonObject +import kotlinx.serialization.json.jsonPrimitive +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertNull + +/** The shared `https.json` fixture, and trying http feeds over https with a fake fetcher. */ +class HttpsUpgradeTest { + private val fixture: JsonObject = + Json.parseToJsonElement(checkNotNull(javaClass.classLoader.getResource("https.json")).readText()).jsonObject + + private fun JsonObject.text(name: String): String = getValue(name).jsonPrimitive.content + + private fun JsonObject.texts(name: String): List = getValue(name).jsonArray.map { item -> item.jsonPrimitive.content } + + @Test + fun `plain http addresses are told apart as the fixture says`() { + for (case in fixture.getValue("plainHttp").jsonArray.map { item -> item.jsonObject }) { + val input = case.text("input") + assertEquals(case.getValue("output").jsonPrimitive.boolean, isPlainHttp(input), input) + } + } + + @Test + fun `https addresses are written as http as the fixture says`() { + for (case in fixture.getValue("plainAddress").jsonArray.map { item -> item.jsonObject }) { + assertEquals(case.text("output"), plainAddress(case.text("input")), case.text("input")) + } + } + + @Test + fun `addresses are written out as the fixture says`() { + for (case in fixture.getValue("webAddress").jsonArray.map { item -> item.jsonObject }) { + assertEquals(case.text("output"), webAddress(case.text("input")), case.text("name")) + } + } + + @Test + fun `only public hosts are tried over https, as the fixture says`() { + for (case in fixture.getValue("probeAllowed").jsonArray.map { item -> item.jsonObject }) { + val input = case.text("input") + assertEquals(case.getValue("output").jsonPrimitive.boolean, probeAllowed(input), input) + } + } + + @Test + fun `answers settle a probe as the fixture says`() { + for (case in fixture.getValue("settledAnswer").jsonArray.map { item -> item.jsonObject }) { + val outcome = FetchOutcome(status = case.getValue("status").jsonPrimitive.int, insecure = case.getValue("insecure").jsonPrimitive.boolean) + assertEquals(case.getValue("settled").jsonPrimitive.boolean, settledAnswer(outcome), case.text("name")) + } + assertFalse(settledAnswer(null)) + } + + @Test + fun `feeds count as the same as the fixture says`() { + for (case in fixture.getValue("sameFeed").jsonArray.map { item -> item.jsonObject }) { + val known = case.getValue("known").jsonObject + val found = case.getValue("found").jsonObject + val links = found.texts("links") + val parsed = ParsedFeed( + title = found.text("title"), + siteUrl = found.text("siteUrl"), + iconUrl = "", + entries = found.texts("guids").mapIndexed { index, guid -> + FeedEntry(guid, "", links.getOrElse(index) { "" }, 0, "", "", "", emptyList(), "") + }, + ) + val knownFeed = KnownFeed(known.text("url"), known.text("title"), known.text("siteUrl"), known.texts("guids"), known.texts("links")) + assertEquals(case.getValue("same").jsonPrimitive.boolean, sameFeed(knownFeed, parsed), case.text("name")) + } + } + + @Test + fun `web addresses keep http and https and drop the rest`() { + assertEquals("http://example.com/rss", webAddress(" HTTP://Example.com/rss ")) + assertEquals("https://example.com/", webAddress("https://example.com")) + assertEquals("", webAddress("ftp://example.com/rss")) + assertEquals("", webAddress("")) + } + + private val known = KnownFeed("http://blog.example/rss", "Blog", "") + private val feed = "Bloghttps://blog.example/aa" + + @Test + fun `the https address is fetched and a matching feed taken`() = runTest { + val fetcher = FakeFetcher() + fetcher.answers["https://blog.example/rss"] = FetchOutcome(status = 200, text = feed, url = "https://blog.example/rss") + val match = probeHttps(known, fetcher) + assertEquals(listOf("https://blog.example/rss"), fetcher.requests) + assertEquals("https://blog.example/rss", match?.url) + assertEquals("Blog", match?.feed?.title) + } + + @Test + fun `failures, redirects off https, non-feeds and other feeds are refused`() = runTest { + val outcomes = listOf( + FetchOutcome(status = 0), + FetchOutcome(status = 404), + FetchOutcome(status = 0, insecure = true), + FetchOutcome(status = 200, text = "Hello"), + FetchOutcome(status = 200, text = "Parkedhttps://parking.example/"), + ) + for (outcome in outcomes) { + assertNull(probeHttps(known, { _, _, _, _ -> outcome }), outcome.toString()) + } + } + + @Test + fun `nothing is fetched for an address that is not plain http`() = runTest { + val fetcher = FakeFetcher() + assertNull(probeHttps(known.copy(url = "https://blog.example/rss"), fetcher)) + assertNull(probeHttps(known.copy(url = "ftp://blog.example/rss"), fetcher)) + assertEquals(emptyList(), fetcher.requests) + } + + @Test + fun `work runs a few at a time and keeps its order`() = runTest { + var running = 0 + var most = 0 + val results = mapConcurrently((1..7).toList(), 3) { item -> + running += 1 + most = maxOf(most, running) + delay(10L - item) + running -= 1 + item * 10 + } + assertEquals((1..7).map { item -> item * 10 }, results) + assertEquals(3, most) + } +} diff --git a/android/core/src/test/kotlin/cc/hafa/reader/core/LoadGateTest.kt b/android/core/src/test/kotlin/cc/hafa/reader/core/LoadGateTest.kt new file mode 100644 index 0000000..85280a4 --- /dev/null +++ b/android/core/src/test/kotlin/cc/hafa/reader/core/LoadGateTest.kt @@ -0,0 +1,93 @@ +package cc.hafa.reader.core + +import cc.hafa.reader.core.sync.SyncReport +import cc.hafa.reader.core.sync.SyncStatus +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertNull +import kotlin.test.assertTrue + +class LoadGateTest { + private val timing = GateTiming() + + /** The gate after each of [steps], a busy flag at a time, from the app opening at 0. */ + private fun run(vararg steps: Pair): LoadGate = + steps.fold(LoadGate.started(0)) { gate, (busy, now) -> gate.step(busy, now) } + + @Test + fun holdsThePersonFromTheStartWithoutTheLogo() { + val gate = LoadGate.started(0) + assertEquals(GatePhase.Waiting, gate.phase) + assertTrue(gate.blocks) + assertEquals(timing.delayMs, gate.wakeAt()) + } + + @Test + fun anInstantLoadNeverShowsTheLogo() { + assertEquals(GatePhase.Waiting, run(true to 50L).phase) + assertEquals(GatePhase.Open, run(true to 50L, false to 120L).phase) + } + + @Test + fun aSlowLoadShowsTheLogoThenLetsThePersonIn() { + val shown = run(true to timing.delayMs) + assertEquals(GatePhase.Shown, shown.phase) + assertEquals(timing.delayMs, shown.shownSince) + val done = shown.step(false, timing.delayMs + timing.minShownMs + 1) + assertEquals(GatePhase.Open, done.phase) + assertNull(done.wakeAt()) + } + + @Test + fun theLogoStaysItsShortestTime() { + val early = run(true to timing.delayMs, false to timing.delayMs + 100) + assertEquals(GatePhase.Shown, early.phase) + assertEquals(timing.delayMs + timing.minShownMs, early.wakeAt()) + assertEquals(GatePhase.Open, early.step(false, timing.delayMs + timing.minShownMs).phase) + } + + @Test + fun aLoadThatRunsTooLongLetsThePersonIn() { + val late = run(true to timing.delayMs, true to timing.timeoutMs) + assertEquals(GatePhase.Open, late.phase) + assertTrue(late.timedOut) + assertNull(late.wakeAt()) + assertFalse(late.step(true, timing.timeoutMs * 2).blocks) + } + + @Test + fun aNewLoadStartsOver() { + val later = run(false to 10L, true to 1000L) + assertEquals(GatePhase.Waiting, later.phase) + assertEquals(1000L, later.busySince) + assertEquals(GatePhase.Shown, later.step(true, 1000 + timing.delayMs).phase) + } + + @Test + fun aLoadAfterOneThatTimedOutHoldsAgain() { + val next = run(true to timing.timeoutMs, false to timing.timeoutMs + 1, true to timing.timeoutMs + 2) + assertEquals(GatePhase.Waiting, next.phase) + assertFalse(next.timedOut) + } + + @Test + fun wakesForTheLogoThenForTheTimeLimit() { + assertEquals(timing.delayMs, run(true to 10L).wakeAt()) + assertEquals(timing.timeoutMs, run(true to timing.delayMs).wakeAt()) + } + + @Test + fun theAccountHoldsThePersonWhileItArrivesButNotOfflineOrFailing() { + val loaded = ReaderState(loaded = true) + assertTrue(loadingBusy(ReaderState(), SyncReport(SyncStatus.Off), awaited = false)) + assertFalse(loadingBusy(loaded, SyncReport(SyncStatus.Off), awaited = true)) + assertTrue(loadingBusy(loaded, SyncReport(SyncStatus.Connecting), awaited = true)) + assertTrue(loadingBusy(loaded, SyncReport(SyncStatus.Syncing, "reader@example.com"), awaited = true)) + assertFalse(loadingBusy(loaded, SyncReport(SyncStatus.Offline, "reader@example.com"), awaited = true)) + assertFalse(loadingBusy(loaded, SyncReport(SyncStatus.Error, "reader@example.com"), awaited = true)) + assertFalse(loadingBusy(loaded, SyncReport(SyncStatus.Synced, "reader@example.com", caughtUp = true), awaited = true)) + assertFalse(loadingBusy(loaded, SyncReport(SyncStatus.Syncing, "reader@example.com"), awaited = false)) + assertTrue(loadingBusy(loaded.copy(importing = 1), SyncReport(SyncStatus.Off), awaited = false)) + } +} diff --git a/android/core/src/test/kotlin/cc/hafa/reader/core/NormalizeUrlTest.kt b/android/core/src/test/kotlin/cc/hafa/reader/core/NormalizeUrlTest.kt index 632e501..a2da353 100644 --- a/android/core/src/test/kotlin/cc/hafa/reader/core/NormalizeUrlTest.kt +++ b/android/core/src/test/kotlin/cc/hafa/reader/core/NormalizeUrlTest.kt @@ -10,9 +10,9 @@ class NormalizeUrlTest { "https://Example.com" to "https://example.com/", "HTTPS://example.com/a b" to "https://example.com/a%20b", "example.com" to "https://example.com/", - " http://example.com/feed " to "https://example.com/feed", - "http://example.com:80/x" to "https://example.com/x", - "http://example.com:8080/x" to "https://example.com:8080/x", + " http://example.com/feed " to "http://example.com/feed", + "http://example.com:80/x" to "http://example.com/x", + "http://example.com:8080/x" to "http://example.com:8080/x", "https://example.com:443/x" to "https://example.com/x", "https://EXAMPLE.com/Path?Q=1#Frag" to "https://example.com/Path?Q=1#Frag", "https://example.com/a%20b" to "https://example.com/a%20b", @@ -25,10 +25,11 @@ class NormalizeUrlTest { "ftp://example.com/feed" to "", "" to "", "not a url" to "", - "https://user:pass@example.com/" to "https://user:pass@example.com/", + // a user name or password is refused, as the shared webAddress fixture says + "https://user:pass@example.com/" to "", "https://example.com." to "https://example.com./", "https://example.com/feed?x=%7e" to "https://example.com/feed?x=%7e", - "mailto:x@example.com" to "https://mailto:x@example.com/", + "mailto:x@example.com" to "", "https://example.com/a^b`c" to "https://example.com/a%5Eb%60c", "https://[::1]:8443/feed" to "https://[::1]:8443/feed", "https://example.com/a?b=c|d#e|f" to "https://example.com/a?b=c|d#e|f", @@ -55,7 +56,7 @@ class NormalizeUrlTest { val hits = parseFeedSearch( """{"results":[{"feedId":"feed/http://Example.com","iconUrl":"http://Example.com/i.png"},{"feedId":"feed/https://example.com/"}]}""", ) - assertEquals(listOf("https://example.com/"), hits.map { hit -> hit.url }) - assertEquals("https://example.com/i.png", hits.single().icon) + assertEquals(listOf("http://example.com/", "https://example.com/"), hits.map { hit -> hit.url }) + assertEquals("https://example.com/i.png", hits.first().icon) } } diff --git a/android/core/src/test/kotlin/cc/hafa/reader/core/OpmlTest.kt b/android/core/src/test/kotlin/cc/hafa/reader/core/OpmlTest.kt index 680ea15..7c3a2bf 100644 --- a/android/core/src/test/kotlin/cc/hafa/reader/core/OpmlTest.kt +++ b/android/core/src/test/kotlin/cc/hafa/reader/core/OpmlTest.kt @@ -11,14 +11,14 @@ class OpmlTest { fun `lists feeds with their folders`() { val source = """x - + """ assertEquals( listOf( - OpmlFeed("https://one.example/feed", "One", "News"), + OpmlFeed("https://one.example/feed", "One", "News", "https://one.example/"), OpmlFeed("two.example/rss", "Two", ""), OpmlFeed("https://three.example/feed", "", ""), ), @@ -47,7 +47,7 @@ class OpmlTest { class BuildOpmlTest { private val feeds = listOf( OpmlFeed("https://a.example/feed?x=1&y=2", "A & \"B\" ", ""), - OpmlFeed("https://b.example/feed", "B", "Tech"), + OpmlFeed("https://b.example/feed", "B", "Tech", "https://b.example/?a=1&b=2"), OpmlFeed("https://c.example/feed", "C", "News"), ) @@ -56,6 +56,8 @@ class BuildOpmlTest { val text = buildOpml(feeds) assertContains(text, """text="A & "B" <c>"""") assertContains(text, """xmlUrl="https://a.example/feed?x=1&y=2"""") + assertContains(text, """htmlUrl="https://b.example/?a=1&b=2"""") + assertFalse("htmlUrl=\"\"" in text) assertTrue(text.indexOf("News") < text.indexOf("Tech")) assertTrue(text.indexOf("Tech") < text.indexOf("a.example")) } diff --git a/android/core/src/test/kotlin/cc/hafa/reader/core/ReaderModelTest.kt b/android/core/src/test/kotlin/cc/hafa/reader/core/ReaderModelTest.kt index 09528ac..eb80d6f 100644 --- a/android/core/src/test/kotlin/cc/hafa/reader/core/ReaderModelTest.kt +++ b/android/core/src/test/kotlin/cc/hafa/reader/core/ReaderModelTest.kt @@ -4,6 +4,8 @@ import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.launch import cc.hafa.reader.core.sync.RemoteChange +import cc.hafa.reader.core.sync.SyncReport +import cc.hafa.reader.core.sync.SyncStatus import kotlinx.coroutines.test.StandardTestDispatcher import kotlinx.coroutines.test.TestScope import kotlinx.coroutines.test.runCurrent @@ -49,6 +51,83 @@ class ReaderModelTest { private fun ReaderModel.titles(): List = view().visible.map { entry -> entry.title } + private fun article(id: String, feedId: String, published: Long, read: Boolean = false): Entry = + Entry(id, feedId, id, "Story $id", "", published, true, "", "", "", "", emptyList(), read, false, published) + + @Test + fun `articles arriving in the background wait until the list is next settled`() = runTest { + val model = model() + model.addWithItems("https://a.example/feed", "one") + val feed = model.state.value.feeds.single() + model.select(Selection.All) + model.settle() + model.syncHost.apply(RemoteChange(entries = listOf(article("later", feed.id, now)))) + assertEquals(listOf("one"), model.titles()) + assertEquals(2, model.view().unreadTotal) + model.select(Selection.Starred) + model.select(Selection.All) + assertEquals(listOf("Story later", "one"), model.titles()) + } + + @Test + fun `an empty list takes in what arrives, since no row can move`() = runTest { + val model = model() + model.addWithItems("https://a.example/feed", "one") + val feed = model.state.value.feeds.single() + model.select(Selection.Starred) + model.settle() + model.syncHost.apply(RemoteChange(entries = listOf(article("star", feed.id, now).copy(starred = true)))) + assertEquals(listOf("Story star"), model.titles()) + } + + @Test + fun `an article read elsewhere stays listed until the view changes`() = runTest { + val model = model() + model.addWithItems("https://a.example/feed", "one") + model.select(Selection.All) + model.settle() + val entry = model.state.value.entries.single() + model.syncHost.apply(RemoteChange(entries = listOf(entry.copy(read = true, updatedAt = now)))) + assertEquals(listOf("one"), model.titles()) + model.select(Selection.Starred) + model.select(Selection.All) + assertEquals(emptyList(), model.titles()) + } + + @Test + fun `a feed with nothing unread stays out of the sidebar when articles arrive`() = runTest { + val model = model() + model.addWithItems("https://a.example/feed", "one") + model.addWithItems("https://b.example/feed", "two") + val (first, second) = model.state.value.feeds + model.select(Selection.All) + model.toggleReadFor(model.state.value.entries.first { entry -> entry.feedId == second.id }.id) + model.settle() + fun listedIds() = sidebarRows(model.state.value, model.view()).mapNotNull { row -> row.feed?.id } + assertEquals(listOf(first.id), listedIds()) + model.syncHost.apply(RemoteChange(entries = listOf(article("later", second.id, now)))) + assertEquals(listOf(first.id), listedIds()) + model.settle() + assertEquals(setOf(first.id, second.id), listedIds().toSet()) + } + + @Test + fun `an import holds the loading screen until its feeds' first check`() = runTest { + val model = model() + val gate = CompletableDeferred() + fetcher.answers["https://x.example/rss"] = FetchOutcome(status = 200, text = rss("X", "one")) + fetcher.gates["https://x.example/rss"] = gate + model.importOpml("""""") + assertEquals(1, model.state.value.importing) + assertTrue(loadingBusy(model.state.value, SyncReport(SyncStatus.Off), awaited = true)) + assertEquals(0, model.state.value.refreshCount) + gate.complete(Unit) + model.awaitChecks() + runCurrent() + assertEquals(0, model.state.value.importing) + assertFalse(loadingBusy(model.state.value, SyncReport(SyncStatus.Off), awaited = true)) + } + @Test fun `a refresh cancelled during its shortest showing still ends its mark`() = runTest { val model = ReaderModel(store, preferences, fetcher, backgroundScope, clock = { now }, minRefreshMillis = 700) @@ -503,6 +582,40 @@ class ReaderModelTest { assertEquals(listOf("https://a.example/feed"), fetcher.requests) } + @Test + fun `opening checks only due feeds in the background without showing a refresh`() = runTest { + val first = model() + first.addWithItems("https://a.example/feed", "one") + now += MAX_INTERVAL_MS + first.addWithItems("https://b.example/feed", "two") + first.awaitWrites() + fetcher.requests.clear() + val gate = CompletableDeferred() + fetcher.gates["https://a.example/feed"] = gate + var backgroundCalls = 0 + val model = ReaderModel( + store, + preferences, + fetcher, + backgroundScope, + clock = { now }, + workDispatcher = StandardTestDispatcher(testScheduler), + minRefreshMillis = 0, + onBackgroundWork = { backgroundCalls += 1 }, + ) + model.openChecks() + runCurrent() + assertTrue(model.state.value.loaded) + assertFalse(model.state.value.refreshing) + assertEquals(1, backgroundCalls) + assertEquals(listOf("https://a.example/feed"), fetcher.requests) + val rank = model.state.value.frozenRank + gate.complete(Unit) + model.awaitChecks() + assertEquals(listOf("https://a.example/feed"), fetcher.requests) + assertTrue(rank === model.state.value.frozenRank) + } + @Test fun `the sidebar order holds until the next refresh`() = runTest { val model = model() @@ -566,16 +679,198 @@ class ReaderModelTest { } @Test - fun `only web addresses can be added, and http is fetched as https`() = runTest { + fun `only https addresses can be added, and a typed http one is refused without a fetch`() = runTest { val model = model() val refused = assertFailsWith { model.addFeed("ftp://files.example/feed") } assertEquals(FeedProblem.NotWebAddress, refused.problem) fetcher.answers["https://plain.example/feed"] = FetchOutcome(status = 200, text = rss("P", "one")) - model.addFeed("http://plain.example/feed") - assertEquals("https://plain.example/feed", model.state.value.feeds.single().url) + val plain = assertFailsWith { model.addFeed("http://plain.example/feed") } + assertEquals(FeedProblem.HttpRefused, plain.problem) + assertEquals(emptyList(), model.state.value.feeds) + assertEquals(emptyList(), fetcher.requests) + } + + @Test + fun `an import moves http feeds to https when it serves them, counts the rest, and uses the chosen group`() = runTest { + val model = model() + fetcher.answers["https://good.example/rss"] = FetchOutcome(status = 200, text = rss("Good", "one")) + val result = model.importOpml( + """ + + + """, + group = "News", + ) + assertEquals(ImportResult(found = 3, added = 2, skipped = 1), result) + assertEquals( + listOf("https://good.example/rss" to "News", "https://safe.example/rss" to "News"), + model.state.value.feeds.map { feed -> feed.url to feed.folder }, + ) + val loose = model.importOpml("""""", group = "") + assertEquals(1, loose.added) + assertEquals("", model.state.value.feeds.last().folder) + } + + @Test + fun `an import leaves alone an http feed already followed`() = runTest { + val model = model() + val old = Feed(feedId("http://blog.example/rss"), "http://blog.example/rss", "Blog", "", "", false, "", "") + model.syncHost.apply(RemoteChange(feeds = listOf(old))) + val result = model.importOpml("""""") + assertEquals(ImportResult(found = 1, added = 0, skipped = 0), result) + assertTrue(fetcher.requests.none { url -> url.contains("blog.example") }) + } + + @Test + fun `search results on http show at https when it serves the same feed, else are dropped`() = runTest { + val model = model() + fetcher.answers[feedSearchAddress("good")] = FetchOutcome( + status = 200, + text = """{"results":[{"feedId":"feed/http://good.example/rss","title":"Good"}, + {"feedId":"feed/http://parked.example/rss","title":"Parked"}, + {"feedId":"feed/http://gone.example/rss","title":"Gone"}]}""", + ) + fetcher.answers["https://good.example/rss"] = FetchOutcome(status = 200, text = rss("Good", "one")) + fetcher.answers["https://parked.example/rss"] = + FetchOutcome(status = 200, text = "Welcomehttps://hosting.example/") + assertEquals(listOf("https://good.example/rss"), model.searchFeeds("good").map { hit -> hit.url }) + } + + @Test + fun `a followed http feed moves to https once, keeping its id, articles and group, and is checked from the answer`() = runTest { + val model = model() + val oldId = feedId("http://blog.example/rss") + val old = Feed(oldId, "http://blog.example/rss", "Renamed", "", "", true, "News", "", updatedAt = 1) + val dead = Feed(feedId("http://dead.example/rss"), "http://dead.example/rss", "Dead", "", "", false, "", "") + val starred = Entry( + entryId(oldId, "one"), oldId, "one", "one", "", 0, true, "", "", "", "", emptyList(), + read = true, starred = true, addedAt = 1, updatedAt = 1, + ) + model.syncHost.apply(RemoteChange(feeds = listOf(old, dead), entries = listOf(starred))) + assertEquals(FeedProblem.HttpFeed.code, model.state.value.feeds.first { feed -> feed.id == oldId }.error) + fetcher.answers["https://blog.example/rss"] = FetchOutcome(status = 200, text = rss("Blog", "one", "two")) + fetcher.answers["https://dead.example/rss"] = FetchOutcome(status = 404) + model.refresh() + runCurrent() + model.awaitChecks() + model.awaitWrites() + val moved = model.state.value.feeds.first { feed -> feed.id == oldId } + assertEquals("https://blog.example/rss", moved.url) + assertEquals("News", moved.folder) + assertEquals("", moved.error) + assertTrue(moved.updatedAt > 1) + assertEquals( + listOf("one" to true, "two" to false), + model.state.value.entries.filter { entry -> entry.feedId == oldId }.map { entry -> entry.guid to entry.starred }, + ) + assertEquals(FeedProblem.HttpFeed.code, model.state.value.feeds.first { feed -> feed.id == dead.id }.error) + model.refresh() + runCurrent() + assertEquals(1, fetcher.requests.count { url -> url == "https://dead.example/rss" }) assertTrue(fetcher.requests.none { url -> url.startsWith("http://") }) } + /** Follow [feeds] as if they came from the account, then refresh and let the https tries finish. */ + private suspend fun TestScope.followAndRefresh(model: ReaderModel, vararg feeds: Feed) { + model.syncHost.apply(RemoteChange(feeds = feeds.toList())) + model.refresh() + runCurrent() + model.awaitChecks() + model.awaitWrites() + } + + private fun plainFeed(url: String, title: String = "Blog", renamed: Boolean = false): Feed = + Feed(feedId(url), url, title, "", "", renamed, "", "") + + @Test + fun `an http feed whose https address is already followed stays as it is`() = runTest { + val model = model() + fetcher.answers["https://blog.example/rss"] = FetchOutcome(status = 200, text = rss("Blog", "one")) + followAndRefresh(model, plainFeed("http://blog.example/rss"), plainFeed("https://blog.example/rss")) + assertEquals( + listOf("http://blog.example/rss", "https://blog.example/rss"), + model.state.value.feeds.map { feed -> feed.url }, + ) + } + + @Test + fun `an http feed that got no definite answer is tried again on the next refresh`() = runTest { + val model = model() + fetcher.answers["https://busy.example/rss"] = FetchOutcome(status = 503) + followAndRefresh(model, plainFeed("http://busy.example/rss"), plainFeed("http://offline.example/rss")) + model.refresh() + runCurrent() + model.awaitChecks() + assertEquals(2, fetcher.requests.count { url -> url == "https://busy.example/rss" }) + assertEquals(2, fetcher.requests.count { url -> url == "https://offline.example/rss" }) + assertTrue(model.state.value.feeds.all { feed -> isPlainHttp(feed.url) }) + } + + @Test + fun `a feed's https address is matched by its own name, not one the person gave it`() = runTest { + val model = model() + fetcher.answers["https://feeds.host.example/a"] = + FetchOutcome(status = 200, text = "Parkedhttps://parking.example/") + followAndRefresh(model, plainFeed("http://feeds.host.example/a", title = "Parked", renamed = true)) + assertEquals("http://feeds.host.example/a", model.state.value.feeds.single().url) + } + + @Test + fun `adding the https address of a feed followed over http moves that feed`() = runTest { + val model = model() + val old = plainFeed("http://blog.example/rss") + model.syncHost.apply(RemoteChange(feeds = listOf(old))) + fetcher.answers["https://blog.example/rss"] = FetchOutcome(status = 200, text = rss("Blog", "one")) + model.addFeed("https://blog.example/rss") + model.awaitWrites() + val moved = model.state.value.feeds.single() + assertEquals(old.id to "https://blog.example/rss", moved.id to moved.url) + assertEquals(now, moved.schedule.lastChecked) + assertEquals(Selection.FeedView(old.id), model.state.value.selection) + } + + @Test + fun `adding the https address of a different feed followed over http adds it beside the one followed`() = runTest { + val model = model() + val old = plainFeed("http://blog.example/rss") + model.syncHost.apply(RemoteChange(feeds = listOf(old))) + fetcher.answers["https://blog.example/rss"] = + FetchOutcome(status = 200, text = "Parkedhttps://parking.example/") + val id = feedId("https://blog.example/rss") + model.addFeed("https://blog.example/rss") + assertEquals( + listOf(old.id to "http://blog.example/rss", id to "https://blog.example/rss"), + model.state.value.feeds.map { feed -> feed.id to feed.url }, + ) + assertEquals(Selection.FeedView(id), model.state.value.selection) + model.addFeed("https://blog.example/rss") + assertEquals(2, model.state.value.feeds.size) + assertEquals(Selection.FeedView(id), model.state.value.selection) + model.awaitWrites() + } + + @Test + fun `an http feed with no definite answer is tried a few times a run, then left`() = runTest { + val model = model() + followAndRefresh(model, plainFeed("http://offline.example/rss")) + repeat(UNSETTLED_TRIES + 1) { + model.refresh() + runCurrent() + model.awaitChecks() + } + assertEquals(UNSETTLED_TRIES, fetcher.requests.count { url -> url == "https://offline.example/rss" }) + } + + @Test + fun `an http feed on a private host is never tried over https`() = runTest { + val model = model() + followAndRefresh(model, plainFeed("http://192.168.1.1/rss"), plainFeed("http://nas.local/rss")) + model.refresh() + runCurrent() + model.awaitChecks() + assertTrue(fetcher.requests.none { url -> "192.168.1.1" in url || "nas.local" in url }) + } + @Test fun `a small refresh waits for every feed with the short time`() = runTest { val timeouts = mutableListOf() @@ -701,7 +996,7 @@ class ReaderModelTest { val items = (1..30).joinToString("") { index -> "t$index$indextopic $index" } fetcher.answers["https://many.example/feed"] = FetchOutcome(status = 200, text = "M$items") model.addFeed("https://many.example/feed") - assertEquals(30, model.view().topics.size) + assertEquals(30, model.view().topicOrder.counts.size) } @Test diff --git a/android/core/src/test/kotlin/cc/hafa/reader/core/ScheduleTest.kt b/android/core/src/test/kotlin/cc/hafa/reader/core/ScheduleTest.kt index a54c5e7..7f6b42f 100644 --- a/android/core/src/test/kotlin/cc/hafa/reader/core/ScheduleTest.kt +++ b/android/core/src/test/kotlin/cc/hafa/reader/core/ScheduleTest.kt @@ -327,4 +327,29 @@ class ScheduleTest { assertEquals(listOf("flat", "monday"), ids(monday + 7 * day - hour)) assertEquals(listOf("monday", "flat"), ids(monday + 7 * day + 11 * hour)) } + + @Test + fun `feeds become overdue a while after their earliest next check`() { + assertEquals(monday + OVERDUE_MS, overdueAt(listOf(monday + day, monday, monday + 2 * day))) + assertEquals(OVERDUE_MS, overdueAt(listOf(0L))) + assertEquals(Long.MAX_VALUE, overdueAt(emptyList())) + assertEquals(2 * MIN_INTERVAL_MS, OVERDUE_MS) + } + + @Test + fun `a refresh is suggested only while nothing is checking feeds`() { + val feeds = listOf( + Feed("late", "https://late.example/feed", "Late", "", "", renamed = false, folder = "", error = "", schedule = FeedSchedule(nextFetch = monday)), + ) + assertEquals(monday + OVERDUE_MS, refreshSuggestedAt(ReaderState(feeds = feeds))) + assertEquals(Long.MAX_VALUE, refreshSuggestedAt(ReaderState(feeds = feeds, checking = true))) + assertEquals(Long.MAX_VALUE, refreshSuggestedAt(ReaderState(feeds = feeds, refreshCount = 1))) + assertEquals(Long.MAX_VALUE, refreshSuggestedAt(ReaderState())) + } + + @Test + fun `a plain http feed, never checked, is never overdue`() { + val plain = Feed("plain", "http://plain.example/feed", "Plain", "", "", renamed = false, folder = "", error = "", schedule = FeedSchedule(nextFetch = 0)) + assertEquals(Long.MAX_VALUE, refreshSuggestedAt(ReaderState(feeds = listOf(plain)))) + } } diff --git a/android/core/src/test/kotlin/cc/hafa/reader/core/TestDoubles.kt b/android/core/src/test/kotlin/cc/hafa/reader/core/TestDoubles.kt index d2dc80e..6d707fe 100644 --- a/android/core/src/test/kotlin/cc/hafa/reader/core/TestDoubles.kt +++ b/android/core/src/test/kotlin/cc/hafa/reader/core/TestDoubles.kt @@ -87,6 +87,7 @@ internal class MemoryStore : ReaderStore { internal class MemoryPreferences : ReaderPreferences { override var unreadOnly = true override var lastView = "all" + override var readingFont = ReadingFont.Literata } /** Answers by address; anything unknown does not answer. */ diff --git a/android/core/src/test/kotlin/cc/hafa/reader/core/TopicRowsTest.kt b/android/core/src/test/kotlin/cc/hafa/reader/core/TopicRowsTest.kt new file mode 100644 index 0000000..e50a6cd --- /dev/null +++ b/android/core/src/test/kotlin/cc/hafa/reader/core/TopicRowsTest.kt @@ -0,0 +1,30 @@ +package cc.hafa.reader.core + +import kotlin.test.Test +import kotlin.test.assertEquals + +class TopicRowsTest { + @Test + fun `items wrap when the next one would pass the available width`() { + assertEquals(0, rowCount(wrappedRows(emptyList(), 100, 8))) + assertEquals(1, rowCount(wrappedRows(listOf(46, 46), 100, 8))) + assertEquals(2, rowCount(wrappedRows(listOf(46, 47), 100, 8))) + assertEquals(3, rowCount(wrappedRows(listOf(40, 40, 40, 40, 40), 100, 8))) + } + + @Test + fun `positions give each item's row and left edge`() { + assertEquals( + listOf(RowPosition(0, 0), RowPosition(0, 48), RowPosition(1, 0)), + wrappedRows(listOf(40, 40, 40), 100, 8), + ) + } + + @Test + fun `an item wider than the space takes a row of its own`() { + assertEquals( + listOf(RowPosition(0, 0), RowPosition(1, 0), RowPosition(1, 28)), + wrappedRows(listOf(150, 20, 20), 100, 8), + ) + } +} diff --git a/android/core/src/test/kotlin/cc/hafa/reader/core/sync/DocumentsTest.kt b/android/core/src/test/kotlin/cc/hafa/reader/core/sync/DocumentsTest.kt index 1203ff8..63a5784 100644 --- a/android/core/src/test/kotlin/cc/hafa/reader/core/sync/DocumentsTest.kt +++ b/android/core/src/test/kotlin/cc/hafa/reader/core/sync/DocumentsTest.kt @@ -198,13 +198,13 @@ class DocumentsTest { } @Test - fun `account feeds and entries have plain http upgraded`() { + fun `account feeds keep a plain http address, and their other addresses and entries are upgraded`() { val feed = parseSubscription( subscriptionDocument( sampleFeed().copy(url = "http://old.example/rss", siteUrl = "http://old.example/", icon = "http://old.example/icon.png"), ).toFields(), ) - assertEquals("https://old.example/rss", feed?.url) + assertEquals("http://old.example/rss", feed?.url) assertEquals("https://old.example/", feed?.siteUrl) assertEquals("https://old.example/icon.png", feed?.icon) val entry = parseEntry(entryDocument(sampleEntry().copy(url = "http://old.example/1", imageUrl = "http://old.example/1.png")).toFields()) diff --git a/android/core/src/test/kotlin/cc/hafa/reader/core/sync/SyncEngineTest.kt b/android/core/src/test/kotlin/cc/hafa/reader/core/sync/SyncEngineTest.kt index 9b664a5..4833cdb 100644 --- a/android/core/src/test/kotlin/cc/hafa/reader/core/sync/SyncEngineTest.kt +++ b/android/core/src/test/kotlin/cc/hafa/reader/core/sync/SyncEngineTest.kt @@ -68,9 +68,7 @@ private class FakeAccount : AccountStore { val listener = sinceMs to onChanges entryListeners.add(listener) val changes = entries.filter { (_, value) -> value.second > sinceMs }.map { (id, value) -> EntryChange(id, value.first, value.second) } - if (changes.isNotEmpty()) { - onChanges(changes) - } + onChanges(changes) return Stop { entryListeners.remove(listener) } } diff --git a/android/scripts/cut-fonts.sh b/android/scripts/cut-fonts.sh new file mode 100755 index 0000000..3adad15 --- /dev/null +++ b/android/scripts/cut-fonts.sh @@ -0,0 +1,45 @@ +#!/bin/sh +# Cut the bundled fonts in app/src/main/res/font from Google Fonts' variable files. +# Needs python3 and network access; run from anywhere. +set -eu + +fonttools_version=4.67.0 +literata_commit=4e5f06dbb274a27ebe71ed54ea706b3ee40eabd9 +public_sans_commit=8b0a1d0f5983c89bc2b93f1b5fb55f9e252744b5 + +out=$(cd "$(dirname "$0")/../app/src/main/res/font" && pwd) +work=$(mktemp -d) +trap 'rm -rf "$work"' EXIT + +python3 -m venv "$work/venv" +"$work/venv/bin/pip" install --quiet "fonttools==$fonttools_version" + +fetch() { + curl -sSfL -o "$work/$3" "https://raw.githubusercontent.com/google/fonts/$1/ofl/$2" +} +fetch "$literata_commit" "literata/Literata%5Bopsz,wght%5D.ttf" literata.ttf +fetch "$literata_commit" "literata/Literata-Italic%5Bopsz,wght%5D.ttf" literata_italic.ttf +fetch "$public_sans_commit" "publicsans/PublicSans%5Bwght%5D.ttf" public_sans.ttf +fetch "$public_sans_commit" "publicsans/PublicSans-Italic%5Bwght%5D.ttf" public_sans_italic.ttf + +# Latin, Latin-1, Latin Extended-A and B, IPA modifiers, combining marks, Vietnamese, punctuation, currency, arrows. +latin="U+0000-024F,U+02B0-02FF,U+0300-036F,U+1EA0-1EF9,U+2000-206F,U+20A0-20C0,U+2116,U+2122,U+2190-2193,U+2212,U+2215" +# Modern Greek and the Cyrillic of Russian, Ukrainian, Belarusian, Bulgarian, Serbian and Macedonian, as fontsource's +# greek and cyrillic files; polytonic Greek and extended Cyrillic would add about 70 KB more and fall back instead. +greek="U+0370-0377,U+037A-037F,U+0384-038A,U+038C,U+038E-03A1,U+03A3-03FF" +cyrillic="U+0400-045F,U+0490-0491,U+04B0-04B1" + +for name in literata literata_italic; do + "$work/venv/bin/fonttools" varLib.instancer "$work/$name.ttf" opsz=12 wght=400:800 -q -o "$work/$name-instanced.ttf" + "$work/venv/bin/pyftsubset" "$work/$name-instanced.ttf" \ + --unicodes="$latin,$greek,$cyrillic" \ + --layout-features='*' --name-IDs='*' --name-languages='*' --notdef-outline \ + --output-file="$out/$name.ttf" +done + +# Public Sans covers Latin only, so it keeps every glyph and only drops the weights the app never uses. +for name in public_sans public_sans_italic; do + "$work/venv/bin/fonttools" varLib.instancer "$work/$name.ttf" wght=400:800 -q -o "$out/$name.ttf" +done + +ls -l "$out" diff --git a/extension/README.md b/extension/README.md index 289f97c..8d755d7 100644 --- a/extension/README.md +++ b/extension/README.md @@ -17,6 +17,14 @@ bun pack # reader-extension.zip for the Web Store bun icons # icons/ from ../design/icons/logo.svg; needs rsvg-convert ``` +## https only + +The manifest's `content_security_policy` adds `upgrade-insecure-requests` to +the service worker, so when a feed redirects to `http://` Chrome asks for the +`https://` address instead and never sends the plain request. It covers only +the extension's own requests, needs no permission, and the final-address +check in `outcomeOf` stays as a backstop. + ## Load unpacked 1. Open `chrome://extensions` and turn on **Developer mode**. diff --git a/extension/manifest.json b/extension/manifest.json index 33a057e..91987ec 100644 --- a/extension/manifest.json +++ b/extension/manifest.json @@ -6,6 +6,9 @@ "minimum_chrome_version": "119", "permissions": ["alarms", "storage", "unlimitedStorage"], "host_permissions": ["https://*/*"], + "content_security_policy": { + "extension_pages": "script-src 'self'; object-src 'self'; upgrade-insecure-requests" + }, "background": { "service_worker": "background.js", "type": "module" diff --git a/extension/package.json b/extension/package.json index 4f4f5bf..012ca7e 100644 --- a/extension/package.json +++ b/extension/package.json @@ -14,6 +14,7 @@ "scripts": { "export": "rm -rf dist && bun build src/background.ts --outdir dist --target browser --format esm --minify && cp manifest.json dist/manifest.json && cp -r icons dist/icons", "pack": "rm -rf store reader-extension.zip && bun build src/background.ts --outdir store --target browser --format esm --minify && cp manifest.json store/manifest.json && cp -r icons store/icons && bun scripts/store-manifest.ts store && (cd store && zip -r -X ../reader-extension.zip .) && rm -rf store", + "test": "bun test", "lint": "tsc -p . && biome check --error-on-warnings", "fmt": "biome check --write", "icons": "rm -rf icons && mkdir icons && for size in 16 32 48 128; do rsvg-convert -w $size -h $size ../design/icons/logo.svg -o icons/icon-$size.png; done" diff --git a/extension/src/background.test.ts b/extension/src/background.test.ts new file mode 100644 index 0000000..da45396 --- /dev/null +++ b/extension/src/background.test.ts @@ -0,0 +1,246 @@ +import { beforeEach, expect, test } from "bun:test"; +import type { + ExtensionRequest, + ExtensionResponse, +} from "../../shared/src/protocol.ts"; + +type Listener = ( + request: ExtensionRequest, + sender: { origin: string }, + sendResponse: (response: ExtensionResponse) => void, +) => boolean; + +const stored = new Map(); +const listeners: Listener[] = []; + +function storedCopies( + keys: string[] | Record, +): Record { + const wanted: [string, unknown][] = Array.isArray(keys) + ? keys.filter((key) => stored.has(key)).map((key) => [key, undefined]) + : Object.entries(keys); + return Object.fromEntries( + wanted.map(([key, fallback]) => [ + key, + stored.has(key) ? structuredClone(stored.get(key)) : fallback, + ]), + ); +} + +function ignore(): void { + // a listener the tests never call +} + +Object.assign(globalThis, { + chrome: { + storage: { + local: { + get: (keys: string[] | Record) => + Promise.resolve(storedCopies(keys)), + set: (items: Record) => { + for (const [key, value] of Object.entries(items)) { + stored.set(key, structuredClone(value)); + } + return Promise.resolve(); + }, + remove: (keys: string[]) => { + for (const key of keys) { + stored.delete(key); + } + return Promise.resolve(); + }, + }, + }, + runtime: { + getManifest: () => ({ + version: "0.0.0", + // biome-ignore lint/style/useNamingConvention: the manifest's own key + externally_connectable: { matches: ["https://reader.hafa.cc/*"] }, + }), + getPlatformInfo: () => Promise.resolve({}), + onMessageExternal: { + addListener: (listener: Listener) => { + listeners.push(listener); + }, + }, + onInstalled: { addListener: ignore }, + onStartup: { addListener: ignore }, + }, + alarms: { + get: () => Promise.resolve({ name: "due" }), + create: () => Promise.resolve(), + onAlarm: { addListener: ignore }, + }, + }, +}); + +const FEED_URL = "https://feed.example/rss"; +const RSS = `Feedhttps://feed.example/ + aa`; + +/** Answers the fake fetch gives; each test sets its own. */ +let respond: (init: RequestInit | undefined) => Promise = () => + Promise.reject(new TypeError("Failed to fetch")); +/** Addresses fetched, in order. */ +const fetched: string[] = []; + +Object.assign(globalThis, { + fetch: (input: string | URL | Request, init?: RequestInit) => { + fetched.push(String(input)); + return respond(init); + }, +}); + +await import("./background.ts"); + +function ask(request: ExtensionRequest): Promise { + const [listener] = listeners; + return new Promise((resolve, reject) => { + if (listener === undefined) { + reject(new Error("The background script added no listener.")); + } else { + listener(request, { origin: "https://reader.hafa.cc" }, resolve); + } + }); +} + +async function drained(): Promise< + Extract +> { + const response = await ask({ type: "drain" }); + if (response.type !== "drained") { + throw new Error(`Expected a drain, got ${response.type}.`); + } + return response; +} + +async function settle(): Promise { + // let the queue finish and save; nothing here waits on a real timer + for (let round = 0; round < 50; round += 1) { + // biome-ignore lint/performance/noAwaitInLoops: each turn lets the next step run + await new Promise((resolve) => setTimeout(resolve, 1)); + } +} + +beforeEach(() => { + stored.clear(); + fetched.length = 0; +}); + +test("checkDue answers once the checks are queued, and the drain says they are under way", async () => { + let answer: (response: Response) => void = ignore; + respond = (): Promise => + new Promise((resolve) => { + answer = resolve; + }); + expect( + await ask({ + type: "track", + feeds: [{ id: "feed", url: FEED_URL, empty: true }], + }), + ).toEqual({ type: "tracked" }); + const before = Date.now(); + expect(await ask({ type: "checkDue" })).toEqual({ type: "queued" }); + expect(fetched).toEqual([FEED_URL]); + + const during = await drained(); + expect(during.checking).toBe(true); + expect(during.pulled).toEqual([]); + expect(during.status).toEqual([ + { feedId: "feed", lastChecked: 0, error: "", nextCheck: 0 }, + ]); + + answer(new Response(RSS, { status: 200 })); + await settle(); + const after = await drained(); + expect(after.checking).toBe(false); + expect(after.pulled.map((pulled) => pulled.feedId)).toEqual(["feed"]); + expect(after.status[0]?.lastChecked).toBeGreaterThanOrEqual(before); + expect(after.status[0]?.nextCheck).toBeGreaterThan(before); +}); + +test("checkDue queues nothing for a feed that is not due", async () => { + respond = (): Promise => + Promise.resolve(new Response(RSS, { status: 200 })); + await ask({ + type: "track", + feeds: [ + { + id: "feed", + url: FEED_URL, + empty: true, + first: { + status: 200, + url: FEED_URL, + text: RSS, + etag: "", + lastModified: "", + hint: 0, + insecure: false, + }, + }, + ], + }); + expect(await ask({ type: "checkDue" })).toEqual({ type: "queued" }); + expect(fetched).toEqual([]); + const page = await drained(); + expect(page.checking).toBe(false); + expect(page.status[0]?.nextCheck).toBeGreaterThan(Date.now()); +}); + +test("a malformed first answer leaves the feed due instead of failing the track", async () => { + expect( + await ask({ + type: "track", + feeds: [ + { + id: "feed", + url: FEED_URL, + empty: true, + first: JSON.parse('{"status":200}'), + }, + ], + }), + ).toEqual({ type: "tracked" }); + const page = await drained(); + expect(page.status).toEqual([ + { feedId: "feed", lastChecked: 0, error: "", nextCheck: 0 }, + ]); +}); + +test("an address whose redirects never settle on https is reported as not on https", async () => { + const redirect = new Response(null); + Object.defineProperty(redirect, "type", { value: "opaqueredirect" }); + // following the redirects fails; asking without following shows a redirect + respond = (init): Promise => + init?.redirect === "manual" + ? Promise.resolve(redirect) + : Promise.reject(new TypeError("Failed to fetch")); + const response = await ask({ + type: "fetch", + url: FEED_URL, + etag: "", + lastModified: "", + }); + expect(response).toMatchObject({ + type: "fetched", + status: 0, + insecure: true, + }); +}); + +test("an address that does not answer at all is no answer", async () => { + respond = (): Promise => + Promise.reject(new TypeError("Failed to fetch")); + const response = await ask({ + type: "fetch", + url: FEED_URL, + etag: "", + lastModified: "", + }); + expect(response).toMatchObject({ + type: "fetched", + status: 0, + insecure: false, + }); +}); diff --git a/extension/src/background.ts b/extension/src/background.ts index 9c5e224..2552beb 100644 --- a/extension/src/background.ts +++ b/extension/src/background.ts @@ -4,6 +4,7 @@ import { judge, retrack, type ScheduledFeed, + takeFirstChecks, } from "../../shared/src/check.ts"; import { decodeBody, readCapped } from "../../shared/src/decode.ts"; import { @@ -141,7 +142,12 @@ const NO_ANSWER: FetchOutcome = { const NOT_HTTPS: FetchOutcome = { ...NO_ANSWER, insecure: true }; -/** Read an answer; one whose redirects left https is refused unread. */ +/** + * Read an answer; one whose redirects left https is refused unread. The + * manifest's `upgrade-insecure-requests` already sends every redirect hop as + * https, so this is only a backstop; a site that sends https back to http + * loops instead, which `failedFetch` reports. + */ async function outcomeOf( response: Response, url: string, @@ -200,10 +206,37 @@ async function fetchOutcome( }); outcome = await outcomeOf(response, url); } catch (error) { - outcome = - error instanceof DOMException && error.name === "TimeoutError" - ? undefined - : NO_ANSWER; + outcome = await failedFetch(url, error, timeoutMs); + } + } + return outcome; +} + +/** + * What a fetch that threw says: undefined when it timed out, `NOT_HTTPS` when + * the address answers with a redirect (so following it failed: a loop, as the + * browser turns a hop back to http into https again, or a target with nothing + * on https), and `NO_ANSWER` otherwise. + */ +async function failedFetch( + url: string, + error: unknown, + timeoutMs: number, +): Promise { + let outcome: FetchOutcome | undefined; + if (error instanceof DOMException && error.name === "TimeoutError") { + outcome = undefined; + } else { + try { + const response = await fetch(url, { + redirect: "manual", + cache: "no-store", + signal: AbortSignal.timeout(timeoutMs), + }); + await response.body?.cancel(); + outcome = response.type === "opaqueredirect" ? NOT_HTTPS : NO_ANSWER; + } catch { + outcome = NO_ANSWER; } } return outcome; @@ -328,8 +361,9 @@ function enqueue( }); } -async function checkDue(): Promise { - const checks = await enqueue((tracked, now) => +/** Queue background checks of the feeds whose schedule says they are due. */ +function queueDue(): Promise<{ check: CheckHandle; request: CheckRequest }[]> { + return enqueue((tracked, now) => tracked .filter((feed) => feed.nextFetch <= now) .map((feed) => ({ @@ -339,6 +373,11 @@ async function checkDue(): Promise { timeoutMs: FETCH_TIMEOUT_MS, })), ); +} + +/** Check the feeds that are due and resolve once they are saved. */ +async function checkDue(): Promise { + const checks = await queueDue(); await Promise.all(checks.map(({ check }) => check.stored)); } @@ -387,12 +426,14 @@ function track(feeds: TrackedFeed[]): Promise { retired: state.retired, }; const waiting = new Set(Object.keys(state.inboxSeqs)); - const next = retrack(feeds, previous, waiting, Date.now()); - const kept = new Set(next.tracked.map((feed) => feed.id)); + const now = Date.now(); + const next = retrack(feeds, previous, waiting, now); + const tracked = await takeFirstChecks(next.tracked, feeds, now); + const kept = new Set(tracked.map((feed) => feed.id)); await chrome.storage.local.set({ - trackedIds: next.tracked.map((feed) => feed.id), + trackedIds: tracked.map((feed) => feed.id), retired: next.retired, - ...feedEntries(next.tracked), + ...feedEntries(tracked), }); await chrome.storage.local.remove( state.trackedIds.filter((feedId) => !kept.has(feedId)).map(feedKey), @@ -418,9 +459,11 @@ function drain(): Promise { feedId: feed.id, lastChecked: feed.lastChecked, error: feed.failures >= 2 ? feed.error : "", + nextCheck: feed.nextFetch, }), ), more: page.more, + checking: queue.busy(), }; }); } @@ -456,6 +499,9 @@ async function handle(request: ExtensionRequest): Promise { } else if (request.type === "refresh") { await refresh(request.feedIds); response = { type: "refreshed" }; + } else if (request.type === "checkDue") { + await queueDue(); + response = { type: "queued" }; } else if (request.type === "drain") { response = await drain(); } else if (request.type === "ack" && Number.isFinite(request.upTo)) { diff --git a/shared/fixtures/https.json b/shared/fixtures/https.json new file mode 100644 index 0000000..734b13f --- /dev/null +++ b/shared/fixtures/https.json @@ -0,0 +1,111 @@ +{ + "plainHttp": [ + { "input": "http://example.com/rss", "output": true }, + { "input": " HTTP://Example.com/rss ", "output": true }, + { "input": "https://example.com/rss", "output": false }, + { "input": "ftp://example.com/rss", "output": false }, + { "input": "example.com/rss", "output": false }, + { "input": "", "output": false } + ], + "plainAddress": [ + { "input": "https://example.com/rss", "output": "http://example.com/rss" }, + { "input": " HTTPS://Example.com:8443/rss?a=1 ", "output": "http://example.com:8443/rss?a=1" }, + { "input": "http://example.com/rss", "output": "" }, + { "input": "ftp://example.com/rss", "output": "" }, + { "input": "", "output": "" } + ], + "webAddress": [ + { "name": "plain http kept", "input": " HTTP://Example.com/rss ", "output": "http://example.com/rss" }, + { "name": "https with a path added", "input": "https://example.com", "output": "https://example.com/" }, + { "name": "http's own port left out", "input": "http://example.com:80/rss", "output": "http://example.com/rss" }, + { "name": "https's port kept on http", "input": "http://example.com:443/rss", "output": "http://example.com:443/rss" }, + { "name": "https's own port left out", "input": "https://example.com:443/rss", "output": "https://example.com/rss" }, + { "name": "http's port kept on https", "input": "https://example.com:80/rss", "output": "https://example.com:80/rss" }, + { "name": "scheme and host in lower case, path kept", "input": "HTTPS://EXAMPLE.COM/Feed", "output": "https://example.com/Feed" }, + { "name": "trailing dot kept", "input": "http://example.com./rss", "output": "http://example.com./rss" }, + { "name": "fragment kept", "input": "http://example.com/rss#top", "output": "http://example.com/rss#top" }, + { "name": "international name", "input": "http://bücher.example/rss", "output": "http://xn--bcher-kva.example/rss" }, + { "name": "space escaped", "input": "http://example.com/a b", "output": "http://example.com/a%20b" }, + { "name": "user and password refused", "input": "http://user:secret@example.com/rss", "output": "" }, + { "name": "user refused", "input": "https://user@example.com/rss", "output": "" }, + { "name": "another scheme", "input": "ftp://example.com/rss", "output": "" }, + { "name": "empty", "input": "", "output": "" } + ], + "probeAllowed": [ + { "input": "http://blog.example.com/rss", "output": true }, + { "input": "http://bücher.example/rss", "output": true }, + { "input": "http://localhost/rss", "output": false }, + { "input": "http://LOCALHOST./rss", "output": false }, + { "input": "http://app.localhost/rss", "output": false }, + { "input": "http://printer.local/rss", "output": false }, + { "input": "http://wiki.corp.internal/rss", "output": false }, + { "input": "http://nas.home.arpa/rss", "output": false }, + { "input": "http://router.lan/rss", "output": false }, + { "input": "http://intranet/rss", "output": false }, + { "input": "http://127.0.0.1/rss", "output": false }, + { "input": "http://192.168.1.1:8080/rss", "output": false }, + { "input": "http://10.0.0.5/rss", "output": false }, + { "input": "http://169.254.169.254/latest", "output": false }, + { "input": "http://8.8.8.8/rss", "output": false }, + { "input": "http://2130706433/rss", "output": false }, + { "input": "http://0x7f.1/rss", "output": false }, + { "input": "http://[::1]/rss", "output": false }, + { "input": "http://[fe80::1]/rss", "output": false }, + { "input": "http://user:secret@blog.example.com/rss", "output": false }, + { "input": "not an address", "output": false } + ], + "settledAnswer": [ + { "name": "a feed", "status": 200, "insecure": false, "settled": true }, + { "name": "not found", "status": 404, "insecure": false, "settled": true }, + { "name": "refused", "status": 403, "insecure": false, "settled": true }, + { "name": "a redirect off https", "status": 0, "insecure": true, "settled": true }, + { "name": "no answer, offline or timed out", "status": 0, "insecure": false, "settled": false }, + { "name": "request timeout", "status": 408, "insecure": false, "settled": false }, + { "name": "too many requests", "status": 429, "insecure": false, "settled": false }, + { "name": "server error", "status": 503, "insecure": false, "settled": false } + ], + "sameFeed": [ + { + "name": "same title, ignoring capitals and spacing", + "known": { "url": "http://feeds.example/a", "title": "The Daily Post", "siteUrl": "", "guids": [], "links": [] }, + "found": { "title": "the daily post", "siteUrl": "https://elsewhere.example/", "guids": [], "links": [] }, + "same": true + }, + { + "name": "an article in common by guid", + "known": { "url": "http://feeds.example/a", "title": "Old name", "siteUrl": "", "guids": ["tag:post-1", "tag:post-2"], "links": [] }, + "found": { "title": "New name", "siteUrl": "", "guids": ["tag:post-2", "tag:post-3"], "links": [] }, + "same": true + }, + { + "name": "an article in common by link, ignoring the scheme", + "known": { "url": "http://feeds.example/a", "title": "Old name", "siteUrl": "", "guids": [], "links": ["http://blog.example/post-1"] }, + "found": { "title": "New name", "siteUrl": "", "guids": ["x"], "links": ["https://blog.example/post-1"] }, + "same": true + }, + { + "name": "site on the feed's own host, ignoring www", + "known": { "url": "http://www.blog.example/rss", "title": "My renamed feed", "siteUrl": "", "guids": [], "links": [] }, + "found": { "title": "Blog", "siteUrl": "https://blog.example/", "guids": [], "links": [] }, + "same": true + }, + { + "name": "site on the known site's host", + "known": { "url": "http://feeds.host.example/blog", "title": "Renamed", "siteUrl": "http://blog.example/", "guids": [], "links": [] }, + "found": { "title": "Blog", "siteUrl": "https://www.blog.example/home", "guids": [], "links": [] }, + "same": true + }, + { + "name": "a different feed", + "known": { "url": "http://blog.example/rss", "title": "Blog", "siteUrl": "", "guids": ["a"], "links": ["https://blog.example/a"] }, + "found": { "title": "Welcome to nginx", "siteUrl": "https://hosting.example/", "guids": ["b"], "links": ["https://hosting.example/b"] }, + "same": false + }, + { + "name": "nothing to compare", + "known": { "url": "http://feeds.host.example/blog", "title": "", "siteUrl": "", "guids": [], "links": [] }, + "found": { "title": "", "siteUrl": "", "guids": ["a"], "links": [""] }, + "same": false + } + ] +} diff --git a/shared/fixtures/ids.json b/shared/fixtures/ids.json index bce1d3e..8b9295f 100644 --- a/shared/fixtures/ids.json +++ b/shared/fixtures/ids.json @@ -13,6 +13,18 @@ "url": "https://www.youtube.com/feeds/videos.xml?channel_id=UCexampleChannel01" }, "output": "2020c506a61097ba06cf773aec62bfbe318cb4224fb9b26cc821dd010221e914" + }, + { + "input": { "url": "http://example.com/rss" }, + "output": "027efd3619b74e8e26de89e62e4b651f5ca1122f2cc397f4d0d4a86e536a1b4c" + }, + { + "input": { "url": " http://example.com/rss\t" }, + "output": "027efd3619b74e8e26de89e62e4b651f5ca1122f2cc397f4d0d4a86e536a1b4c" + }, + { + "input": { "url": "http://example.com:443/rss" }, + "output": "679704ea1a991916a22329655f8d47796b464950e1884faecafe848e18bfb6fd" } ], "entries": [ diff --git a/shared/src/check.test.ts b/shared/src/check.test.ts index 98eb477..ceac203 100644 --- a/shared/src/check.test.ts +++ b/shared/src/check.test.ts @@ -6,6 +6,7 @@ import { type ScheduledFeed, type Schedules, statusProblem, + takeFirstChecks, } from "./check.ts"; import { sha256Hex } from "./id.ts"; import type { FetchOutcome } from "./protocol.ts"; @@ -182,6 +183,81 @@ test("tracking a new address resets the feed but keeps what it learned", () => { }); }); +test("an answer already taken in counts as the first check of a feed starting over", async () => { + const first = answer(rss(["a", "b"]), { etag: '"1"' }); + const fresh = scheduled({ id: "new" }); + const known = scheduled({ + id: "known", + hash: "h", + lastChecked: NOW - HOUR, + nextFetch: NOW + HOUR, + }); + const [taken, kept, untouched] = await takeFirstChecks( + [fresh, known, scheduled({ id: "other" })], + [ + { id: "new", url: fresh.url, empty: false, first }, + { id: "known", url: known.url, empty: false, first }, + { id: "other", url: fresh.url, empty: false }, + ], + NOW, + ); + expect(taken).toEqual((await judge(fresh, first, NOW)).feed); + expect(taken?.lastChecked).toBe(NOW); + expect(taken?.nextFetch).toBeGreaterThan(NOW); + expect(taken?.etag).toBe('"1"'); + expect(kept).toEqual(known); + expect(untouched).toEqual(scheduled({ id: "other" })); +}); + +test("a malformed answer is skipped and leaves its feed due, without spoiling the rest", async () => { + const first = answer(rss(["a"])); + const { text: _text, ...withoutText } = first; + const malformed: unknown[] = [ + null, + "first", + 42, + withoutText, + { ...first, status: "200" }, + { ...first, insecure: 0 }, + ]; + const feeds = malformed.map((value, index) => ({ + id: `bad-${index}`, + url: "https://feed.example/rss", + empty: false, + // what a web app with a bug, or a newer one, might send + first: JSON.parse(JSON.stringify(value)), + })); + const tracked = [ + ...feeds.map((feed) => scheduled({ id: feed.id })), + scheduled({ id: "good" }), + ]; + const result = await takeFirstChecks( + tracked, + [...feeds, { id: "good", url: tracked[0]?.url ?? "", empty: false, first }], + NOW, + ); + expect(result.slice(0, -1)).toEqual(tracked.slice(0, -1)); + expect(result.at(-1)?.lastChecked).toBe(NOW); +}); + +test("an answer for a feed not tracked is ignored", async () => { + const tracked = [scheduled()]; + expect( + await takeFirstChecks( + tracked, + [ + { + id: "elsewhere", + url: "https://feed.example/rss", + empty: false, + first: answer(rss(["a"])), + }, + ], + NOW, + ), + ).toEqual(tracked); +}); + test("tracking starts a new feed due now and retires untracked ones", () => { const old = scheduled({ lastChecked: NOW, guids: ["a"] }); const next = retrack( diff --git a/shared/src/check.ts b/shared/src/check.ts index 2b95541..15e8363 100644 --- a/shared/src/check.ts +++ b/shared/src/check.ts @@ -28,7 +28,7 @@ export function statusProblem(status: number): string { } /** A tracked feed with its schedule and what its last check saw. */ -export interface ScheduledFeed extends Omit { +export interface ScheduledFeed extends Omit { /** `ETag` of the last body seen, sent back as `If-None-Match`. */ etag: string; /** `Last-Modified` of the last body seen, sent back as `If-Modified-Since`. */ @@ -339,3 +339,60 @@ export function retrack( ]; return { tracked, retired }; } + +/** Whether a value sent as `first` has every field of a fetch's answer, with its type. */ +function isFetchOutcome(value: unknown): value is FetchOutcome { + return ( + typeof value === "object" && + value !== null && + "status" in value && + typeof value.status === "number" && + "url" in value && + typeof value.url === "string" && + "text" in value && + typeof value.text === "string" && + "etag" in value && + typeof value.etag === "string" && + "lastModified" in value && + typeof value.lastModified === "string" && + "hint" in value && + typeof value.hint === "number" && + "insecure" in value && + typeof value.insecure === "boolean" + ); +} + +/** + * Count each answer the web app already took in (`first`) as the check of a + * feed whose schedule starts over (new, or reset by a new address), so it is + * not fetched again at once. Other feeds keep their schedules; what the answer + * held is not handed on again. A `first` that is not a fetch's answer, or + * that cannot be judged, is skipped, leaving that feed due. + */ +export async function takeFirstChecks( + tracked: ScheduledFeed[], + feeds: TrackedFeed[], + nowMs: number, +): Promise { + const firstById = new Map(); + for (const feed of feeds) { + const first: unknown = feed.first; + if (isFetchOutcome(first)) { + firstById.set(feed.id, first); + } + } + return await Promise.all( + tracked.map(async (feed) => { + const first = firstById.get(feed.id); + let scheduled = feed; + if (first !== undefined && feed.nextFetch === 0 && feed.hash === "") { + try { + scheduled = (await judge(feed, first, nowMs)).feed; + } catch { + scheduled = feed; + } + } + return scheduled; + }), + ); +} diff --git a/shared/src/filter.test.ts b/shared/src/filter.test.ts index 2141b97..4b401ae 100644 --- a/shared/src/filter.test.ts +++ b/shared/src/filter.test.ts @@ -1,11 +1,14 @@ import { expect, test } from "bun:test"; import { + articleTopics, + byCount, DEFAULT_SETTINGS, type FilterEntry, hasWord, keeper, passes, topicCounts, + topicRanks, type ViewSettings, } from "./filter.ts"; @@ -83,6 +86,42 @@ test("topics are counted, commonest first", () => { ]); }); +test("a topic an article lists twice counts once, and ties go by name", () => { + const twice = { ...story, categories: ["Zoo", "zoo ", "Apple"] }; + expect(topicCounts([twice])).toEqual([ + { topic: "Apple", count: 1 }, + { topic: "Zoo", count: 1 }, + ]); +}); + +test("names go most counted first, then by name", () => { + const counts = new Map([ + ["budget", 2], + ["senate", 5], + ["night", 2], + ]); + expect( + byCount( + ["budget", "night", "senate", "zoo"], + (name) => counts.get(name) ?? 0, + ), + ).toEqual(["senate", "budget", "night", "zoo"]); +}); + +test("an article's topics follow the view's counts, each once", () => { + const counts = [ + { topic: "Politics", count: 3 }, + { topic: "Business", count: 2 }, + { topic: "U.S.", count: 2 }, + ]; + const ranks = topicRanks(counts); + expect( + articleTopics(["U.S.", "Zoo", "politics ", "Business", "US"], ranks), + ).toEqual(["politics ", "Business", "U.S.", "US", "Zoo"]); + expect(articleTopics(["U.S.", "u.s."], ranks)).toEqual(["U.S."]); + expect(ranks.get("u.s.")).toBe(2); +}); + test("a list of words is compiled once and reused", () => { const OriginalRegExp = RegExp; let built = 0; diff --git a/shared/src/filter.ts b/shared/src/filter.ts index b36f998..7ca365d 100644 --- a/shared/src/filter.ts +++ b/shared/src/filter.ts @@ -126,24 +126,95 @@ function matchesEntry(matcher: Matcher, entry: FilterEntry): boolean { return matcher(entry.title) || matcher(entry.summary); } -/** The topics the articles carry, commonest first, with how many have each. */ +/** + * The topics the articles carry, with how many articles carry each, + * commonest first and then by name. + */ export function topicCounts( - entries: FilterEntry[], + entries: readonly FilterEntry[], ): { topic: string; count: number }[] { const counts = new Map(); for (const entry of entries) { + const seen = new Set(); for (const topic of entry.categories) { const key = topic.trim().toLowerCase(); - const known = counts.get(key); - counts.set(key, { topic, count: (known?.count ?? 0) + 1 }); + if (!seen.has(key)) { + seen.add(key); + const known = counts.get(key); + counts.set(key, { topic, count: (known?.count ?? 0) + 1 }); + } } } - return [...counts.values()].sort( - (first, second) => - second.count - first.count || first.topic.localeCompare(second.topic), + return [...counts.values()].sort((first, second) => + compareCounted(first.count, first.topic, second.count, second.topic), ); } +function compareCounted( + firstCount: number, + firstName: string, + secondCount: number, + secondName: string, +): number { + return secondCount - firstCount || firstName.localeCompare(secondName); +} + +/** + * The names with the highest counts first and then by name, the order every + * list of topics or words is shown in. + */ +export function byCount( + names: string[], + count: (name: string) => number, +): string[] { + return names + .map((name) => ({ name, count: count(name) })) + .sort((first, second) => + compareCounted(first.count, first.name, second.count, second.name), + ) + .map(({ name }) => name); +} + +/** + * Each topic's place in the view's order, by its trimmed lower-case form, + * from `topicCounts`; built once per view so each article looks its topics up. + */ +export function topicRanks( + counts: readonly { topic: string; count: number }[], +): Map { + const ranks = new Map(); + for (const [index, { topic }] of counts.entries()) { + ranks.set(topic.trim().toLowerCase(), index); + } + return ranks; +} + +/** + * An article's topics, each once, in the view's order (`topicRanks`): most + * articles in the view first, then by name; topics the view does not count go + * last, by name. + */ +export function articleTopics( + categories: string[], + ranks: ReadonlyMap, +): string[] { + const seen = new Set(); + const unique: { topic: string; rank: number }[] = []; + for (const topic of categories) { + const key = topic.trim().toLowerCase(); + if (!seen.has(key)) { + seen.add(key); + unique.push({ topic, rank: ranks.get(key) ?? Number.POSITIVE_INFINITY }); + } + } + return unique + .sort( + (first, second) => + first.rank - second.rank || first.topic.localeCompare(second.topic), + ) + .map(({ topic }) => topic); +} + /** * A test of whether an article stays after the settings' filters are * applied, built once for testing many articles. diff --git a/shared/src/https.test.ts b/shared/src/https.test.ts new file mode 100644 index 0000000..4b7d854 --- /dev/null +++ b/shared/src/https.test.ts @@ -0,0 +1,202 @@ +import { expect, test } from "bun:test"; +import type { ParsedFeed } from "./feed.ts"; +import { + isPlainHttp, + type KnownFeed, + mapConcurrently, + type ProbeOutcome, + plainAddress, + probeAllowed, + probeHttps, + sameFeed, + settledAnswer, + webAddress, +} from "./https.ts"; + +/** `fixtures/https.json`: which addresses are plain http, their http and written-out forms, which may be tried over https, which answers settle a probe, and which feeds count as the same. */ +interface HttpsFixture { + plainHttp: { input: string; output: boolean }[]; + plainAddress: { input: string; output: string }[]; + webAddress: { name: string; input: string; output: string }[]; + probeAllowed: { input: string; output: boolean }[]; + settledAnswer: { + name: string; + status: number; + insecure: boolean; + settled: boolean; + }[]; + sameFeed: { + name: string; + known: KnownFeed; + found: { title: string; siteUrl: string; guids: string[]; links: string[] }; + same: boolean; + }[]; +} + +const fixture: HttpsFixture = await Bun.file( + new URL("../fixtures/https.json", import.meta.url), +).json(); + +function parsedOf( + found: HttpsFixture["sameFeed"][number]["found"], +): ParsedFeed { + return { + title: found.title, + siteUrl: found.siteUrl, + iconUrl: "", + entries: found.guids.map((guid, index) => ({ + guid, + title: "", + url: found.links[index] ?? "", + published: 0, + author: "", + imageUrl: "", + summary: "", + categories: [], + html: "", + })), + }; +} + +test.each(fixture.plainHttp.map((item) => [item.input, item.output] as const))( + "isPlainHttp(%p) is %p", + (input, output) => { + expect(isPlainHttp(input)).toBe(output); + }, +); + +test.each( + fixture.plainAddress.map((item) => [item.input, item.output] as const), +)("plainAddress(%p) is %p", (input, output) => { + expect(plainAddress(input)).toBe(output); +}); + +test.each(fixture.settledAnswer.map((item) => [item.name, item] as const))( + "settledAnswer: %s", + (_name, item) => { + const outcome = { + status: item.status, + url: "", + text: "", + insecure: item.insecure, + }; + expect(settledAnswer(outcome)).toBe(item.settled); + }, +); + +test("settledAnswer: no answer from the extension settles nothing", () => { + expect(settledAnswer(undefined)).toBe(false); +}); + +test.each(fixture.sameFeed.map((item) => [item.name, item] as const))( + "sameFeed: %s", + (_name, item) => { + expect(sameFeed(item.known, parsedOf(item.found))).toBe(item.same); + }, +); + +test.each(fixture.webAddress.map((item) => [item.name, item] as const))( + "webAddress: %s", + (_name, item) => { + expect(webAddress(item.input)).toBe(item.output); + }, +); + +test.each( + fixture.probeAllowed.map((item) => [item.input, item.output] as const), +)("probeAllowed(%p) is %p", (input, output) => { + expect(probeAllowed(input)).toBe(output); +}); + +const KNOWN: KnownFeed = { + url: "http://blog.example/rss", + title: "Blog", + siteUrl: "", + guids: [], + links: [], +}; + +const FEED = `Bloghttps://blog.example/ + aa`; + +function answer(extra: Partial = {}): ProbeOutcome { + return { + status: 200, + url: "https://blog.example/rss", + text: FEED, + insecure: false, + ...extra, + }; +} + +test("probeHttps fetches the https address and takes a matching feed", async () => { + const asked: string[] = []; + const match = await probeHttps(KNOWN, (url) => { + asked.push(url); + return Promise.resolve(answer()); + }); + expect(asked).toEqual(["https://blog.example/rss"]); + expect(match?.url).toBe("https://blog.example/rss"); + expect(match?.feed.title).toBe("Blog"); +}); + +test("probeHttps refuses failures, redirects off https, non-feeds and other feeds", async () => { + const outcomes: (ProbeOutcome | undefined)[] = [ + undefined, + answer({ status: 0 }), + answer({ status: 404, text: "" }), + answer({ status: 0, insecure: true }), + answer({ text: "Hello" }), + answer({ + text: "Parkedhttps://parking.example/", + }), + ]; + for (const outcome of outcomes) { + // biome-ignore lint/performance/noAwaitInLoops: one fake answer at a time + const match = await probeHttps(KNOWN, () => Promise.resolve(outcome)); + expect(match).toBeUndefined(); + } +}); + +test("probeHttps tries nothing for an address that is not plain http or may not be tried", async () => { + let asked = 0; + function fetchAddress(): Promise { + asked += 1; + return Promise.resolve(answer()); + } + expect( + await probeHttps( + { ...KNOWN, url: "https://blog.example/rss" }, + fetchAddress, + ), + ).toBeUndefined(); + expect( + await probeHttps({ ...KNOWN, url: "ftp://blog.example/rss" }, fetchAddress), + ).toBeUndefined(); + expect( + await probeHttps({ ...KNOWN, url: "http://192.168.1.1/rss" }, fetchAddress), + ).toBeUndefined(); + expect( + await probeHttps({ ...KNOWN, url: "http://nas.local/rss" }, fetchAddress), + ).toBeUndefined(); + expect(asked).toBe(0); +}); + +test("mapConcurrently keeps order and limits how many run at once", async () => { + let running = 0; + let most = 0; + const results = await mapConcurrently( + [1, 2, 3, 4, 5, 6, 7], + 3, + async (item) => { + running += 1; + most = Math.max(most, running); + await new Promise((resolve) => setTimeout(resolve, 8 - item)); + running -= 1; + return item * 10; + }, + ); + expect(results).toEqual([10, 20, 30, 40, 50, 60, 70]); + expect(most).toBe(3); + expect(await mapConcurrently([], 3, () => Promise.resolve(1))).toEqual([]); +}); diff --git a/shared/src/https.ts b/shared/src/https.ts new file mode 100644 index 0000000..69ba122 --- /dev/null +++ b/shared/src/https.ts @@ -0,0 +1,230 @@ +import { upgradedAddress } from "./address.ts"; +import { type ParsedFeed, parseFeed } from "./feed.ts"; +import type { FetchOutcome } from "./protocol.ts"; + +/** Most https addresses tried at once. */ +export const PROBE_CONCURRENCY = 6; + +/** + * Most tries of a plain http feed's https address in one session that get no + * definite answer; after that it waits for the app to start again. + */ +export const UNSETTLED_TRIES = 3; + +const SPACES_PATTERN = /\s+/u; +const SCHEME_PATTERN = /^https?:\/\//iu; +const WWW_PATTERN = /^www\./u; +const TRAILING_DOT = /\.$/u; +// A last label that is a number, as in an IPv4 address or the forms browsers read as one. +const NUMBER_LABEL = /^(?:0x[0-9a-f]*|[0-9]+)$/iu; +// Names that only mean something on the local network. +const LOCAL_NAMES = ["localhost", "local", "internal", "home.arpa", "lan"]; + +function parsed(address: string): URL | undefined { + let url: URL | undefined; + try { + url = new URL(address); + } catch { + url = undefined; + } + return url; +} + +/** Whether an address is plain `http`. */ +export function isPlainHttp(address: string): boolean { + return parsed(address.trim())?.protocol === "http:"; +} + +/** + * An `https` address written as plain `http`, to find a feed followed at its + * http address; empty for anything else. + */ +export function plainAddress(address: string): string { + const url = parsed(address.trim()); + let result = ""; + if (url?.protocol === "https:") { + url.protocol = "http:"; + result = url.href; + } + return result; +} + +/** + * An `http` or `https` address written out the way browsers write it (the + * scheme and host in lower case, the scheme's own default port left out, an + * international name in its `xn--` form, the fragment kept); empty for + * anything else, and for an address with a user name or password, which + * browsers refuse to fetch. Plain http is kept as it is, so it can be shown + * as http and tried over https before it is used. + */ +export function webAddress(address: string): string { + const url = parsed(address.trim()); + return (url?.protocol === "http:" || url?.protocol === "https:") && + url.username === "" && + url.password === "" + ? url.href + : ""; +} + +/** + * Whether a plain http address may be tried at its https address: its host + * is a public name, not an IP address (nor a number browsers read as one), a + * name of one label, `localhost`, or a name under `.local`, `.internal`, + * `.home.arpa` or `.lan`, and it has no user name or password. Addresses + * from a feed list or a search come from others, and trying them must not + * reach this device or its network. + */ +export function probeAllowed(address: string): boolean { + const url = parsed(address.trim()); + const host = (url?.hostname ?? "").replace(TRAILING_DOT, ""); + const labels = host.split("."); + return ( + url !== undefined && + url.username === "" && + url.password === "" && + labels.length > 1 && + !host.startsWith("[") && + !NUMBER_LABEL.test(labels.at(-1) ?? "") && + !LOCAL_NAMES.some((name) => host === name || host.endsWith(`.${name}`)) + ); +} + +/** What is known of a plain http feed before its https address is tried. */ +export interface KnownFeed { + /** Its plain http address. */ + url: string; + /** Its name; empty when unknown. */ + title: string; + /** Its site's address; empty when unknown. */ + siteUrl: string; + /** Guids of its articles already held. */ + guids: string[]; + /** Links of its articles already held. */ + links: string[]; +} + +/** The parts of a fetch the probe reads. */ +export type ProbeOutcome = Pick< + FetchOutcome, + "status" | "url" | "text" | "insecure" +>; + +/** A plain http feed's https address, found to serve the same feed. */ +export interface HttpsMatch { + /** The https address. */ + url: string; + /** What it served. */ + feed: ParsedFeed; + /** The answer it came in. */ + outcome: Outcome; +} + +// Statuses that ask to try again later rather than say anything of the address. +const LATER_STATUSES = new Set([408, 425, 429]); + +/** + * Whether a probe's answer settles if a feed has an https address: any answer + * from the site, except one asking to try again later (a timeout, too many + * requests, or a server error). A redirect off https settles it too: Android + * sees one as it follows redirects itself, and the extension reports one when + * the https address redirects but following it never reaches an answer over + * https, as when the site sends https back to http and the browser's upgrade + * of that hop loops. No answer at all, as when offline or the fetch timed + * out, settles nothing. + */ +export function settledAnswer(outcome: ProbeOutcome | undefined): boolean { + return ( + outcome !== undefined && + (outcome.insecure || + (outcome.status !== 0 && + outcome.status < 500 && + !LATER_STATUSES.has(outcome.status))) + ); +} + +function normalTitle(title: string): string { + return title.toLowerCase().split(SPACES_PATTERN).filter(Boolean).join(" "); +} + +function withoutScheme(address: string): string { + return address.replace(SCHEME_PATTERN, ""); +} + +function siteHost(address: string): string { + return (parsed(address)?.hostname ?? "").replace(WWW_PATTERN, ""); +} + +/** + * Whether a feed read over https looks like the plain http one it stands in + * for: the same name (ignoring capitals and spacing), an article in common (by + * guid or link, ignoring the scheme), or a site on the same host as the known + * site or, failing that, as the http address itself (ignoring `www.`). + */ +export function sameFeed(known: KnownFeed, found: ParsedFeed): boolean { + const title = normalTitle(known.title); + const guids = new Set(known.guids.map(withoutScheme)); + const links = new Set(known.links.filter(Boolean).map(withoutScheme)); + const shared = found.entries.some( + (entry) => + guids.has(withoutScheme(entry.guid)) || + (entry.url !== "" && links.has(withoutScheme(entry.url))), + ); + const knownHost = siteHost(known.siteUrl) || siteHost(known.url); + const foundHost = siteHost(found.siteUrl); + return ( + (title !== "" && title === normalTitle(found.title)) || + shared || + (foundHost !== "" && foundHost === knownHost) + ); +} + +/** + * Try a plain http feed at its https address. It counts only when the answer + * is a 200 that stays on https, parses as a feed and passes `sameFeed`; + * otherwise, or when `known` is not plain http or not `probeAllowed`, there + * is no match, and then nothing is fetched. + */ +export async function probeHttps( + known: KnownFeed, + fetchAddress: (url: string) => Promise, +): Promise | undefined> { + const url = + isPlainHttp(known.url) && probeAllowed(known.url) + ? upgradedAddress(known.url) + : ""; + const outcome = url === "" ? undefined : await fetchAddress(url); + let match: HttpsMatch | undefined; + if (outcome?.status === 200 && !outcome.insecure) { + let feed: ParsedFeed | undefined; + try { + feed = parseFeed(outcome.text, outcome.url || url); + } catch { + feed = undefined; + } + if (feed !== undefined && sameFeed(known, feed)) { + match = { url, feed, outcome }; + } + } + return match; +} + +/** Run `work` on every item, at most `limit` at once, keeping the order. */ +export async function mapConcurrently( + items: readonly Item[], + limit: number, + work: (item: Item) => Promise, +): Promise { + const results = new Array(items.length); + // one iterator shared by every worker, so each item is taken once + const pending = items.entries(); + async function worker(): Promise { + for (const [index, item] of pending) { + // biome-ignore lint/performance/noAwaitInLoops: each worker takes one item at a time + results[index] = await work(item); + } + } + await Promise.all( + Array.from({ length: Math.min(limit, items.length) }, worker), + ); + return results; +} diff --git a/shared/src/protocol.ts b/shared/src/protocol.ts index 62d0212..d351fe4 100644 --- a/shared/src/protocol.ts +++ b/shared/src/protocol.ts @@ -8,6 +8,8 @@ export interface TrackedFeed { url: string; /** The web app holds no articles for this feed, so it needs them all again. */ empty: boolean; + /** An answer from the address the web app already took in, to count as the first check of a schedule that starts over; absent when none. */ + first?: FetchOutcome; } /** What the extension pulled from one feed since the web app last asked. */ @@ -29,6 +31,8 @@ export interface FeedStatus { lastChecked: number; /** Why the last check failed; empty when it did not. */ error: string; + /** Milliseconds since the epoch of the next scheduled check; absent from older extensions. */ + nextCheck?: number; } /** A message the web app sends to the extension. */ @@ -42,6 +46,7 @@ export type ExtensionRequest = } | { type: "track"; feeds: TrackedFeed[] } | { type: "refresh"; feedIds: string[] } + | { type: "checkDue" } | { type: "drain" } | { type: "ack"; upTo: number }; @@ -59,7 +64,13 @@ export interface FetchOutcome { lastModified: string; /** Milliseconds the site asked us to wait before checking again. */ hint: number; - /** Refused because the address, or where it redirected, is not https; status is then zero. */ + /** + * Refused because the address, or where it redirected, is not https; status + * is then zero. The extension also sets it when the address redirects but + * following the redirects never reaches an answer, since its content policy + * turns each hop to http into https, and a site that sends https back to + * http then loops. + */ insecure: boolean; } @@ -69,6 +80,7 @@ export type ExtensionResponse = | ({ type: "fetched" } & FetchOutcome) | { type: "tracked" } | { type: "refreshed" } + | { type: "queued" } | { type: "drained"; /** Acknowledge up to this to remove what was drained; zero when nothing was. */ @@ -77,6 +89,8 @@ export type ExtensionResponse = status: FeedStatus[]; /** More is waiting than fit in this reply; drain again after acknowledging. */ more: boolean; + /** Feeds are being checked or saved; absent from older extensions. */ + checking?: boolean; } | { type: "acked" } | { type: "error"; message: string }; diff --git a/shared/src/schedule.test.ts b/shared/src/schedule.test.ts index 82e2b7d..cb504a4 100644 --- a/shared/src/schedule.test.ts +++ b/shared/src/schedule.test.ts @@ -15,8 +15,10 @@ import { MISS_PROBABILITY, missProbability, newProbability, + OVERDUE_MS, observe, observeWindow, + overdueAt, type PlannableFeed, PRIOR, patternFactors, @@ -421,3 +423,12 @@ test("Retry-After is whole seconds or an HTTP date, and nothing else", () => { expect(hint("Mon, 05 Jan 2026 13:00:00 GMT")).toBe(3_600_000); expect(hint("2026-01-05T13:00:00Z")).toBe(0); }); + +test("feeds become overdue a while after their earliest next check", () => { + expect(overdueAt([MONDAY + DAY, MONDAY, MONDAY + 2 * DAY])).toBe( + MONDAY + OVERDUE_MS, + ); + expect(overdueAt([0])).toBe(OVERDUE_MS); + expect(overdueAt([])).toBe(Number.POSITIVE_INFINITY); + expect(OVERDUE_MS).toBe(2 * MIN_INTERVAL_MS); +}); diff --git a/shared/src/schedule.ts b/shared/src/schedule.ts index f1d55bc..f4b38e3 100644 --- a/shared/src/schedule.ts +++ b/shared/src/schedule.ts @@ -492,6 +492,27 @@ export function planRefresh( }; } +/** + * How late a feed's scheduled check may run before a refresh is suggested, in + * milliseconds. The background timers (every 5 minutes in the extension, about + * every 15 on Android) normally reach a due feed well within it, so only a + * feed they missed, as when the browser or phone slept, counts. + */ +export const OVERDUE_MS = 2 * MIN_INTERVAL_MS; + +/** + * When the first of these feeds becomes overdue: the earliest next check plus + * `OVERDUE_MS`; `Infinity` when there are none. One pass over the next checks, + * so the caller can recompute it on every change and set one timer for it. + */ +export function overdueAt(nextChecks: Iterable): number { + let earliest = Number.POSITIVE_INFINITY; + for (const nextCheck of nextChecks) { + earliest = Math.min(earliest, nextCheck); + } + return earliest + OVERDUE_MS; +} + /** Wait after the given number of failures in a row, doubling each time. */ export function retryInterval(failures: number): number { return Math.min(MAX_RETRY_MS, MIN_INTERVAL_MS * 2 ** failures); diff --git a/web/bun.lock b/web/bun.lock index 2680333..7284a7b 100644 --- a/web/bun.lock +++ b/web/bun.lock @@ -6,7 +6,7 @@ "name": "hafa-reader-web", "devDependencies": { "@biomejs/biome": "^2.5.15", - "@fontsource-variable/fraunces": "^5.3.0", + "@fontsource-variable/literata": "^5.3.0", "@fontsource-variable/public-sans": "^5.3.0", "@happy-dom/global-registrator": "^21.0.1", "@lucide/svelte": "^1.55.0", @@ -135,7 +135,7 @@ "@firebase/webchannel-wrapper": ["@firebase/webchannel-wrapper@1.0.7", "", {}, "sha512-phBFwieDLvkZGYN9CE9ZFNEIoBVksprzsnCzQejCmCHtgwCXReeuRpoEGN9C4EbhONztv8NRV1tau6Rb9pONwQ=="], - "@fontsource-variable/fraunces": ["@fontsource-variable/fraunces@5.3.0", "", {}, "sha512-9BYGySn4AHEJdgp9Z28tQ3X+laJMEOITXkQarZXeloWQZDq5oOvXJ3kDA8c7MGIfpogIaZfjrQBqmda8POOCKA=="], + "@fontsource-variable/literata": ["@fontsource-variable/literata@5.3.0", "", {}, "sha512-1Jw7xvOcoGyB83lxB2nG4bfNbqtX2/TTbdh9sHAOmU064SvmSdulDtA5RBu5JYWf64fdaKX7xU+z3GO1T2bCHA=="], "@fontsource-variable/public-sans": ["@fontsource-variable/public-sans@5.3.0", "", {}, "sha512-AVfkmAt50BMXWpOO21FAntiJFKGX6xTc2dSL8dxtDteONe9IuRXJWGbs0EbG955vAMCq23ENeuopuW87cGWDSQ=="], diff --git a/web/package.json b/web/package.json index b8e48f0..dee6b22 100644 --- a/web/package.json +++ b/web/package.json @@ -22,7 +22,7 @@ }, "devDependencies": { "@biomejs/biome": "^2.5.15", - "@fontsource-variable/fraunces": "^5.3.0", + "@fontsource-variable/literata": "^5.3.0", "@fontsource-variable/public-sans": "^5.3.0", "@happy-dom/global-registrator": "^21.0.1", "@lucide/svelte": "^1.55.0", diff --git a/web/src/app.css b/web/src/app.css index 2c58f62..6b937c0 100644 --- a/web/src/app.css +++ b/web/src/app.css @@ -16,6 +16,8 @@ --ok: #2f9a5c; /* Material's error color, as on Android */ --danger: #b3261e; + /* article text and titles, unless Public Sans is chosen */ + --reading-font: "Literata Variable", Georgia, serif; } @media (prefers-color-scheme: dark) { @@ -52,7 +54,11 @@ --animate-dot: dot-pulse 1.4s ease-in-out infinite; --color-faded: var(--read); --font-sans: "Public Sans Variable", ui-sans-serif, system-ui, sans-serif; - --font-serif: "Fraunces Variable", Georgia, serif; + --font-reading: var(--reading-font); +} + +:root[data-reading-font="public-sans"] { + --reading-font: "Public Sans Variable", ui-sans-serif, system-ui, sans-serif; } html, diff --git a/web/src/lib/components/add-feed.svelte b/web/src/lib/components/add-feed.svelte index c7ceee2..fcf0c48 100644 --- a/web/src/lib/components/add-feed.svelte +++ b/web/src/lib/components/add-feed.svelte @@ -7,6 +7,7 @@ import { buildOpml } from "#lib/opml.ts"; import { type FeedHit, faviconFor, type Reader } from "#lib/reader.svelte.ts"; import FeedIcon from "./feed-icon.svelte"; + import TextField from "./text-field.svelte"; const { reader, @@ -25,6 +26,15 @@ let dragging = $state(false); // a topic search, as opposed to adding an address or a file let searching = $state(false); + // the group imported feeds go into; empty keeps the file's folders + let importGroup = $state(""); + + function editImportGroup(event: Event): void { + const { currentTarget } = event; + if (currentTarget instanceof HTMLInputElement) { + importGroup = currentTarget.value; + } + } function clearField(): void { fieldError = ""; @@ -105,12 +115,18 @@ async function importText(source: string): Promise { try { - const result = await reader.importOpml(source); + const result = await reader.importOpml( + source, + importGroup.trim() || undefined, + ); if (result.found === 0) { reader.notify(copy.importNone); + } else if (result.skipped > 0) { + reader.notify(copy.importSkippedHttp(result.skipped)); } else if (result.added === 0) { reader.notify(copy.importNothingNew); - } else { + } + if (result.added > 0) { ondone(); } } catch { @@ -137,6 +153,7 @@ url: feed.url, title: feed.title, folder: feed.folder, + siteUrl: feed.siteUrl, })), ); try { @@ -179,7 +196,7 @@
{#if first}
-

{copy.emptyTitle}

+

{copy.emptyTitle}

{copy.emptyBody}

{/if} @@ -267,8 +284,16 @@ {/if}
-

{copy.importTitle}

+

{copy.importTitle}

{copy.importBody}

+ -

- {#if entry.url} - {entry.title} - {:else} - {entry.title} - {/if} -

{#if showImage} {/if} -
+
{@html reader.body}
@@ -342,34 +350,22 @@ {/if}
- {#if entry.categories.length > 0} -
    - - {#each entry.categories as category, index (index)} -
  • - {#if reader.filtersApply} - - {:else} - {category} - {/if} -
  • - {/each} -
+ {#if topics.length > 0} + topic.trim().toLowerCase()}> + {#snippet chip( + topic: string, + )} + {#if reader.filtersApply} + logFailure(reader.toggleTopic(topic, true))} + /> + {:else} + + {/if} + {/snippet} + {/if}
{:else} diff --git a/web/src/lib/components/chip-rows.svelte b/web/src/lib/components/chip-rows.svelte new file mode 100644 index 0000000..6203022 --- /dev/null +++ b/web/src/lib/components/chip-rows.svelte @@ -0,0 +1,62 @@ + + + +
{ + boxWidth = width; + })} +> +
    + {#each items as item, index (key(item))} +
  • { + chipWidths[index] = width; + })} + > + {@render chip(item)} +
  • + {/each} +
+
diff --git a/web/src/lib/components/confirm-button.svelte b/web/src/lib/components/confirm-button.svelte new file mode 100644 index 0000000..85f6ba9 --- /dev/null +++ b/web/src/lib/components/confirm-button.svelte @@ -0,0 +1,51 @@ + + + diff --git a/web/src/lib/components/due-dot.svelte b/web/src/lib/components/due-dot.svelte new file mode 100644 index 0000000..ed4efb3 --- /dev/null +++ b/web/src/lib/components/due-dot.svelte @@ -0,0 +1,7 @@ + + diff --git a/web/src/lib/components/empty-list.svelte b/web/src/lib/components/empty-list.svelte deleted file mode 100644 index d7cf250..0000000 --- a/web/src/lib/components/empty-list.svelte +++ /dev/null @@ -1,30 +0,0 @@ - - - - diff --git a/web/src/lib/components/entry-card.svelte b/web/src/lib/components/entry-card.svelte index ead1d6e..87867fc 100644 --- a/web/src/lib/components/entry-card.svelte +++ b/web/src/lib/components/entry-card.svelte @@ -9,6 +9,7 @@ const { entry, source, + topics, active, onopen, ontoggle, @@ -16,6 +17,8 @@ }: { entry: Entry; source: string; + /** The article's topics in the view's order. */ + topics: string[]; active: boolean; onopen: () => void; ontoggle: () => void; @@ -151,9 +154,9 @@ : "line-clamp-3 pr-[4.5rem]"}" >{entry.excerpt} - {#if entry.categories.length > 0} + {#if topics.length > 0} {entry.categories.join(" · ")}{topics.join(" · ")} {/if} diff --git a/web/src/lib/components/entry-list.svelte b/web/src/lib/components/entry-list.svelte index ee5c8f3..71b76e0 100644 --- a/web/src/lib/components/entry-list.svelte +++ b/web/src/lib/components/entry-list.svelte @@ -10,7 +10,8 @@ import { type Axis, touchAxis } from "#lib/gestures.ts"; import { showsListEnd } from "#lib/list-end.ts"; import { logFailure, type Reader, viewKey } from "#lib/reader.svelte.ts"; - import EmptyList from "./empty-list.svelte"; + import { articleTopics } from "$shared/filter.ts"; + import DueDot from "./due-dot.svelte"; import EntryCard from "./entry-card.svelte"; const { @@ -162,14 +163,25 @@ - {reader.viewTitle} + {/if} -
- {#if shownFeed?.error} -

-

- {/if} - {#each rows as entry (entry.id)} - { - logFailure(reader.open(entry.id)); - onopen(); - }} - ontoggle={() => logFailure(reader.toggleReadFor(entry.id))} - onstar={() => logFailure(reader.toggleStarFor(entry.id))} - /> - {:else} -
- -

{copy.noEntries}

-
- {/each} - {#if showsListEnd(rows.length, reader.visible.length)} -
+
- - {/if} -
- +