diff options
Diffstat (limited to 'app/src')
9 files changed, 278 insertions, 15 deletions
diff --git a/app/src/main/java/org/terst/listen/queue/DripReleaser.kt b/app/src/main/java/org/terst/listen/queue/DripReleaser.kt index e6c7b6b..675eb2a 100644 --- a/app/src/main/java/org/terst/listen/queue/DripReleaser.kt +++ b/app/src/main/java/org/terst/listen/queue/DripReleaser.kt @@ -45,6 +45,42 @@ class DripReleaser( } /** + * Pays out now, whatever the clock says. Returns how many episodes were released. + * + * A drip is a cadence you chose so you would not have to think about it, and most of the + * time that is the whole point. But a cadence with no override owns you: finish Tuesday's + * episode and the next one is due Thursday, and there is nothing to listen to and no way to + * ask. + * + * Brought forward, not added. The mark moves to now, so the scheduled release is a full + * interval away rather than landing on top of this one -- asking for the next episode is + * asking for it sooner, not asking for an extra. + */ + suspend fun releaseNow(subscriptionId: String): Int { + val subscription = subscriptions.getById(subscriptionId) ?: return 0 + // Only a show that is actually dripping. Anything else already lists everything it has. + if (!subscription.dripEnabled) return 0 + + val nowMs = now() + val count = subscription.dripCount.coerceAtLeast(1) + val batch = if (subscription.dripOldestFirst) { + episodes.unreleasedOldestFirst(subscription.id, count) + } else { + episodes.unreleasedNewestFirst(subscription.id, count) + } + + for (episode in batch) { + episodes.markDripReleased(episode.id, nowMs) + queue.append(episode.id) + } + + // Even when the archive had nothing left: the clock still restarts, so repeatedly asking + // an exhausted show does not queue up a debt that bursts when the publisher next posts. + subscriptions.setDripMark(subscription.id, nowMs) + return batch.size + } + + /** * Switches drip on for a show and settles what it already owes you. * * Anything played or already queued is released up front: hiding episodes you have heard diff --git a/app/src/main/java/org/terst/listen/ui/DripSettings.kt b/app/src/main/java/org/terst/listen/ui/DripSettings.kt index 97ba063..19a5e27 100644 --- a/app/src/main/java/org/terst/listen/ui/DripSettings.kt +++ b/app/src/main/java/org/terst/listen/ui/DripSettings.kt @@ -30,9 +30,7 @@ private val DRIP_COUNTS = listOf(1, 2, 3) fun DripSettings( subscription: SubscriptionEntity, busy: Boolean, - onEnable: (intervalMs: Long, count: Int, oldestFirst: Boolean) -> Unit, - onDisable: () -> Unit, - onFetchArchive: () -> Unit, + actions: DripActions, ) { Column( Modifier.fillMaxWidth(), @@ -45,22 +43,25 @@ fun DripSettings( detail = dripSummary(subscription), onCheckedChange = { wanted -> if (wanted) { - onEnable( + actions.onEnable( subscription.dripIntervalMs, subscription.dripCount, subscription.dripOldestFirst, ) } else { - onDisable() + actions.onDisable() } }, ) if (subscription.dripEnabled) { - DripCadence(subscription, onEnable) + DripCadence(subscription, actions.onEnable) + // Only while dripping, because it is the cadence this overrides. A show listing + // everything already has nothing to bring forward. + OutlinedButton(onClick = actions.onReleaseNow, enabled = !busy) { Text("Release the next one now") } } - OutlinedButton(onClick = onFetchArchive, enabled = !busy) { Text("Fetch archive") } + OutlinedButton(onClick = actions.onFetchArchive, enabled = !busy) { Text("Fetch archive") } } } @@ -116,3 +117,12 @@ private fun dripSummary(subscription: SubscriptionEntity): String { val order = if (subscription.dripOldestFirst) "oldest" else "newest" return "${subscription.dripCount} $order first, every $every" } + +/** What the drip controls do, bundled for the same reason [EpisodeActions] is. */ +data class DripActions( + val onEnable: (intervalMs: Long, count: Int, oldestFirst: Boolean) -> Unit, + val onDisable: () -> Unit, + /** Brings the next scheduled release forward to now. */ + val onReleaseNow: () -> Unit, + val onFetchArchive: () -> Unit, +) diff --git a/app/src/main/java/org/terst/listen/ui/LibraryScreen.kt b/app/src/main/java/org/terst/listen/ui/LibraryScreen.kt index 912d697..fea837a 100644 --- a/app/src/main/java/org/terst/listen/ui/LibraryScreen.kt +++ b/app/src/main/java/org/terst/listen/ui/LibraryScreen.kt @@ -23,6 +23,7 @@ import androidx.compose.material3.CircularProgressIndicator import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.material3.Icon import androidx.compose.material3.IconButton +import androidx.compose.material.icons.filled.PlayArrow import androidx.compose.material3.MaterialTheme import androidx.compose.material3.OutlinedTextField import androidx.compose.material3.Scaffold @@ -186,7 +187,7 @@ internal fun LibraryList( LazyColumn(Modifier.fillMaxSize()) { if (queue.isNotEmpty()) { item { SectionHeading("Up Next \u00b7 ${queue.size}") } - item { UpNextRow(queue.first(), onOpenQueue) } + item { UpNextRow(queue.first(), actionsFor(queue.first()).play, onOpenQueue) } } if (subscriptions.isEmpty()) { item { if (bedtime) EmptyBedtime() else EmptyLibrary() } @@ -228,9 +229,9 @@ private fun SectionHeading(text: String) { * Peter asked for that screen to stay -- "Leave it" -- so this is a way in, not a replacement. */ @Composable -private fun UpNextRow(episode: EpisodeEntity, onOpenQueue: () -> Unit) { +private fun UpNextRow(episode: EpisodeEntity, onPlay: () -> Unit, onOpenQueue: () -> Unit) { Row( - Modifier.fillMaxWidth().clickable(onClick = onOpenQueue).padding(horizontal = 16.dp, vertical = 10.dp), + Modifier.fillMaxWidth().clickable(onClick = onOpenQueue).padding(start = 16.dp, end = 4.dp, top = 10.dp, bottom = 10.dp), verticalAlignment = Alignment.CenterVertically, ) { AsyncImage( @@ -248,6 +249,12 @@ private fun UpNextRow(episode: EpisodeEntity, onOpenQueue: () -> Unit) { overflow = TextOverflow.Ellipsis, modifier = Modifier.weight(1f).padding(start = 12.dp), ) + // The reason this row exists. Opening the app and pressing play was two taps -- the row + // opened the queue and the queue had the button -- which is two too many for the thing + // the app is for. The row still opens the queue, for reordering and everything else. + IconButton(onClick = onPlay) { + Icon(Icons.Default.PlayArrow, contentDescription = "Play ${episode.title}") + } } } diff --git a/app/src/main/java/org/terst/listen/ui/ShowSettingsSheet.kt b/app/src/main/java/org/terst/listen/ui/ShowSettingsSheet.kt index 693fee3..f7780a5 100644 --- a/app/src/main/java/org/terst/listen/ui/ShowSettingsSheet.kt +++ b/app/src/main/java/org/terst/listen/ui/ShowSettingsSheet.kt @@ -71,11 +71,14 @@ fun ShowSettingsSheet( DripSettings( subscription = show, busy = busy, - onEnable = { intervalMs, count, oldestFirst -> - settings.enableDrip(show.id, intervalMs, count, oldestFirst) - }, - onDisable = { settings.disableDrip(show.id) }, - onFetchArchive = { settings.fetchArchive(show.id) }, + actions = DripActions( + onEnable = { intervalMs, count, oldestFirst -> + settings.enableDrip(show.id, intervalMs, count, oldestFirst) + }, + onDisable = { settings.disableDrip(show.id) }, + onReleaseNow = { settings.releaseNextNow(show.id) }, + onFetchArchive = { settings.fetchArchive(show.id) }, + ), ) HorizontalDivider() diff --git a/app/src/main/java/org/terst/listen/ui/ShowSettingsViewModel.kt b/app/src/main/java/org/terst/listen/ui/ShowSettingsViewModel.kt index 6bdf4c4..221f876 100644 --- a/app/src/main/java/org/terst/listen/ui/ShowSettingsViewModel.kt +++ b/app/src/main/java/org/terst/listen/ui/ShowSettingsViewModel.kt @@ -71,6 +71,24 @@ class ShowSettingsViewModel(app: Application, private val graph: UiGraph) : Andr } } + /** + * Brings the next drip release forward to now. + * + * A cadence with no override owns you: finish Tuesday's episode when the next is due + * Thursday and there is nothing to listen to and no way to ask. Says plainly when the + * archive is empty, because "nothing happened" and "it did not work" look identical. + */ + fun releaseNextNow(subscriptionId: String) { + viewModelScope.launch { + val released = graph.drip.releaseNow(subscriptionId) + _banner.value = if (released > 0) { + Banner("Queued $released more") + } else { + Banner("Nothing left in this show's archive to release") + } + } + } + fun disableDrip(subscriptionId: String) { viewModelScope.launch { graph.drip.disable(subscriptionId) diff --git a/app/src/test/java/org/terst/listen/feed/FeedFailureTextTest.kt b/app/src/test/java/org/terst/listen/feed/FeedFailureTextTest.kt new file mode 100644 index 0000000..88fe231 --- /dev/null +++ b/app/src/test/java/org/terst/listen/feed/FeedFailureTextTest.kt @@ -0,0 +1,71 @@ +package org.terst.listen.feed + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Test + +/** + * What a failed refresh says, and what it is safe to write down. + * + * The messages are read in two places -- the banner after trying to subscribe, and the error line + * under a show in the library -- and until 2026-11-05 they were whatever the machine said: a dead + * host put the literal word `UnknownHostException` under a podcast's name. + * + * [FeedFailureText.redact] is the other half, and it is a security control rather than tidiness. + * A private feed's URL carries its token in the query string and logcat is readable by anything + * with the permission, so a log line must never carry the whole address. + */ +class FeedFailureTextTest { + + @Test + fun `a feed that has moved reads differently from one having a bad day`() { + assertEquals("That feed is no longer at this address.", FeedFailureText.status(404)) + assertEquals("That feed is no longer at this address.", FeedFailureText.status(410)) + assertEquals("That feed's server is having trouble. It is worth trying later.", FeedFailureText.status(503)) + } + + @Test + fun `a refusal says it was refused rather than naming the code`() { + assertEquals("That feed refused the request.", FeedFailureText.status(401)) + assertEquals("That feed refused the request.", FeedFailureText.status(403)) + } + + @Test + fun `an unmapped status still gets a sentence`() { + val text = FeedFailureText.status(418) + assertFalse("a status code is not for the listener: $text", text.contains("418")) + assertEquals("That feed's server could not provide it.", text) + } + + /** The whole point: the part that identifies the feed, and nothing that authenticates to it. */ + @Test + fun `redacting keeps the host and drops the token`() { + assertEquals( + "https://feeds.example.com", + FeedFailureText.redact("https://feeds.example.com/private.xml?token=s3cret&user=peter"), + ) + } + + /** Some private feeds put credentials in the userinfo, which is worse than a query token. */ + @Test + fun `redacting drops credentials carried in the userinfo`() { + val redacted = FeedFailureText.redact("https://peter:hunter2@feeds.example.com/private.xml") + + assertEquals("https://feeds.example.com", redacted) + assertFalse(redacted.contains("hunter2")) + assertFalse(redacted.contains("peter")) + } + + /** + * Something with no structure cannot be redacted, so it is named rather than printed. This is + * the branch the security review called out: the exception for one of these carries the + * address in its message, so logging the cause would undo the redaction. + */ + @Test + fun `an address that will not parse is described, not quoted`() { + val redacted = FeedFailureText.redact("not a url ?token=s3cret") + + assertEquals("an unparsable address", redacted) + assertFalse(redacted.contains("s3cret")) + } +} diff --git a/app/src/test/java/org/terst/listen/playback/PlaybackAdvancerTest.kt b/app/src/test/java/org/terst/listen/playback/PlaybackAdvancerTest.kt index 28fbc42..2023833 100644 --- a/app/src/test/java/org/terst/listen/playback/PlaybackAdvancerTest.kt +++ b/app/src/test/java/org/terst/listen/playback/PlaybackAdvancerTest.kt @@ -1,5 +1,6 @@ package org.terst.listen.playback +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.test.runTest import org.junit.Assert.assertEquals import org.junit.Assert.assertNotNull @@ -420,6 +421,28 @@ class PlaybackAdvancerTest { assertTrue(advancer.start("a") is StartOutcome.Ready) } + /** + * A cancelled start is not a failed one. + * + * The catch that stops a thrown fetch escaping is deliberately broad, which would otherwise + * swallow cancellation too -- so navigating away mid-fetch would report "could not fetch + * audio" and leave a false error on screen. Structured concurrency also relies on + * CancellationException propagating. + */ + @Test + fun `a cancelled fetch stays cancelled rather than becoming a failure`() = runTest { + show() + episode("a") + val cancelling = PlaybackAdvancer( + episodes, subscriptions, queue, + sources = FakeSources(onFetch = { throw CancellationException("navigated away") }), + ) { now } + + val thrown = runCatching { cancelling.startFetchingIfNeeded("a") }.exceptionOrNull() + + assertTrue("got $thrown", thrown is CancellationException) + } + @Test fun `a fetch that throws is reported rather than escaping`() = runTest { show() diff --git a/app/src/test/java/org/terst/listen/queue/DripReleaserTest.kt b/app/src/test/java/org/terst/listen/queue/DripReleaserTest.kt index d5fedf7..757b0d4 100644 --- a/app/src/test/java/org/terst/listen/queue/DripReleaserTest.kt +++ b/app/src/test/java/org/terst/listen/queue/DripReleaserTest.kt @@ -258,4 +258,68 @@ class DripReleaserTest { assertEquals(false, subscriptions.rows.single().dripEnabled) assertNotNull(episodes.rows.single().dripReleasedAtMs) } + + /* + Squeezing the next one out by hand. + + A drip is a cadence somebody chose, and the point of choosing it is not having to think + about it. But a cadence you cannot override is a cadence that owns you: finishing the + current episode on a Tuesday when the next is due Thursday leaves nothing to listen to and + no way to ask. + + So this pays out now whatever the clock says, and then moves the mark so the scheduled one + does not arrive on top of it. Asking for one is asking to bring it forward, not to get an + extra. + */ + @Test + fun `releasing by hand pays out before the interval is up`() = runTest { + show(dripLastReleaseAtMs = now) + episode("a", publishedAtMs = now - 10 * day) + episode("b", publishedAtMs = now - 9 * day) + + assertEquals("nothing is owed yet", 0, releaser.release(subscriptions.rows.single())) + assertEquals(1, releaser.releaseNow("s1")) + assertEquals(listOf("a"), queued()) + } + + /** Brought forward, not added: the next scheduled one is a full interval from the squeeze. */ + @Test + fun `releasing by hand restarts the clock`() = runTest { + show(dripLastReleaseAtMs = now - 10 * day) + episode("a", publishedAtMs = now - 10 * day) + episode("b", publishedAtMs = now - 9 * day) + + releaser.releaseNow("s1") + + assertEquals(now, subscriptions.rows.single().dripLastReleaseAtMs) + } + + @Test + fun `releasing by hand respects the show's count and order`() = runTest { + show(dripCount = 2, dripOldestFirst = false, dripLastReleaseAtMs = now) + episode("a", publishedAtMs = now - 10 * day) + episode("b", publishedAtMs = now - 9 * day) + episode("c", publishedAtMs = now - 8 * day) + + assertEquals(2, releaser.releaseNow("s1")) + assertEquals(listOf("c", "b"), queued()) + } + + /** An archive with nothing left in it says nothing happened, rather than claiming it did. */ + @Test + fun `releasing by hand with nothing left releases nothing`() = runTest { + show(dripLastReleaseAtMs = now) + episode("a", publishedAtMs = now - 10 * day, dripReleasedAtMs = now - day) + + assertEquals(0, releaser.releaseNow("s1")) + } + + /** Only for a show that is actually dripping; anything else has a whole list already. */ + @Test + fun `releasing by hand does nothing for a show that is not dripping`() = runTest { + show(dripEnabled = false) + episode("a", publishedAtMs = now - 10 * day) + + assertEquals(0, releaser.releaseNow("s1")) + } } diff --git a/app/src/test/java/org/terst/listen/ui/ShowSettingsViewModelTest.kt b/app/src/test/java/org/terst/listen/ui/ShowSettingsViewModelTest.kt index 0ebd559..44ca546 100644 --- a/app/src/test/java/org/terst/listen/ui/ShowSettingsViewModelTest.kt +++ b/app/src/test/java/org/terst/listen/ui/ShowSettingsViewModelTest.kt @@ -106,6 +106,37 @@ class ShowSettingsViewModelTest { ) } + /* + Squeezing the next one out. + + A drip is a cadence you chose so you would not have to think about it, and a cadence with no + override owns you: finish Tuesday's episode when the next is due Thursday and there is + nothing to listen to and no way to ask. + */ + @Test + fun `releasing the next one now queues it and says how many`() { + show() + graph.episodes.rows += episodeRow("e1") + graph.episodes.rows += episodeRow("e2") + vm.enableDrip("s1", intervalMs = 1_000, count = 1, oldestFirst = true) + + vm.releaseNextNow("s1") + + assertEquals(Banner("Queued 1 more"), vm.banner.value) + assertEquals(2, graph.queueDao.entries.size) + } + + /** An exhausted archive has to say so, or it is indistinguishable from the button not working. */ + @Test + fun `releasing the next one with nothing left says the archive is empty`() { + show() + vm.enableDrip("s1", intervalMs = 1_000, count = 1, oldestFirst = true) + + vm.releaseNextNow("s1") + + assertEquals(Banner("Nothing left in this show's archive to release"), vm.banner.value) + } + @Test fun `enabling drip on a show with nothing to release says so rather than nothing`() { show() |