diff options
Diffstat (limited to 'app/src')
13 files changed, 319 insertions, 4 deletions
diff --git a/app/src/androidTest/java/org/terst/listen/data/MigrationTest.kt b/app/src/androidTest/java/org/terst/listen/data/MigrationTest.kt index a261149..5ef4379 100644 --- a/app/src/androidTest/java/org/terst/listen/data/MigrationTest.kt +++ b/app/src/androidTest/java/org/terst/listen/data/MigrationTest.kt @@ -906,4 +906,48 @@ class MigrationTest { assertEquals("the crown jewel must survive", 314159L, it.getLong(0)) } } + + /** + * A show gains a tier, and existing shows default to the one that starves nothing. + * + * It is a scheduling hint for the server and changes nothing on the phone, so an older + * show arriving without one must keep behaving exactly as it did. + */ + @Test + fun migrate19To20GivesEveryShowATierThatStarvesNothing() { + helper.createDatabase(name, 19).apply { + execSQL( + """INSERT INTO subscriptions + (id, feedUrl, kind, title, addedAtMs, skipIntroMs, skipOutroMs, dripEnabled, + dripIntervalMs, dripCount, dripOldestFirst, skipAdChapters, bedtime) + VALUES ('s1', 'https://example.com/feed.xml', 'PODCAST', 'A Show', 1000, 0, 0, + 0, 86400000, 1, 1, 1, 0)""" + ) + execSQL( + """INSERT INTO episodes + (id, subscriptionId, guid, title, mediaUrl, mediaType, discoveredAtMs, + positionMs, downloadState) + VALUES ('e1', 's1', 'g1', 'One', 'https://cdn/1.mp3', 'audio/mpeg', 1000, + 271828, 'NONE')""" + ) + close() + } + + val db = helper.runMigrationsAndValidate(name, 20, true, AppDatabase.MIGRATION_19_20) + + db.query("SELECT tier, title FROM subscriptions WHERE id = 's1'").use { + it.moveToFirst() + assertEquals("a show nobody placed must not be starved", "ordinary", it.getString(0)) + assertEquals("A Show", it.getString(1)) + } + db.execSQL("UPDATE subscriptions SET tier = 'topical' WHERE id = 's1'") + db.query("SELECT tier FROM subscriptions WHERE id = 's1'").use { + it.moveToFirst() + assertEquals("topical", it.getString(0)) + } + db.query("SELECT positionMs FROM episodes WHERE id = 'e1'").use { + it.moveToFirst() + assertEquals("the crown jewel must survive", 271828L, it.getLong(0)) + } + } } diff --git a/app/src/main/java/org/terst/listen/data/AppDatabase.kt b/app/src/main/java/org/terst/listen/data/AppDatabase.kt index dc9c094..95a984b 100644 --- a/app/src/main/java/org/terst/listen/data/AppDatabase.kt +++ b/app/src/main/java/org/terst/listen/data/AppDatabase.kt @@ -34,7 +34,7 @@ class Converters { EpisodeTranscriptEntity::class, TranscriptCueEntity::class, ], - version = 19, + version = 20, exportSchema = true, ) @TypeConverters(Converters::class) @@ -499,6 +499,21 @@ abstract class AppDatabase : RoomDatabase() { } } + /** + * A show gains a tier: how soon the server should do its work. + * + * Existing shows default to "ordinary", which is the tier that starves nothing. The + * server treats an absent or unrecognised tier the same way, so a phone and a server + * that disagree about this column still agree about the show. + */ + val MIGRATION_19_20 = object : Migration(19, 20) { + override fun migrate(db: SupportSQLiteDatabase) { + db.execSQL( + "ALTER TABLE `subscriptions` ADD COLUMN `tier` TEXT NOT NULL DEFAULT 'ordinary'" + ) + } + } + fun get(context: Context): AppDatabase = instance ?: synchronized(this) { instance ?: build(context.applicationContext).also { instance = it } } @@ -513,6 +528,7 @@ abstract class AppDatabase : RoomDatabase() { MIGRATION_10_11, MIGRATION_11_12, MIGRATION_12_13, MIGRATION_13_14, MIGRATION_14_15, MIGRATION_15_16, MIGRATION_16_17, MIGRATION_17_18, MIGRATION_18_19, + MIGRATION_19_20, ) .addCallback(SEED_DEFAULT_FILTERS) // No fallbackToDestructiveMigration. A listener's playback positions and play diff --git a/app/src/main/java/org/terst/listen/data/Daos.kt b/app/src/main/java/org/terst/listen/data/Daos.kt index 43f713f..12c605c 100644 --- a/app/src/main/java/org/terst/listen/data/Daos.kt +++ b/app/src/main/java/org/terst/listen/data/Daos.kt @@ -144,6 +144,10 @@ interface SubscriptionDao { @Query("UPDATE subscriptions SET bedtime = :enabled WHERE id = :id") suspend fun setBedtime(id: String, enabled: Boolean) + /** How soon the server should do this show's work. See [org.terst.listen.data.ShowTier]. */ + @Query("UPDATE subscriptions SET tier = :tier WHERE id = :id") + suspend fun setTier(id: String, tier: String) + /** Tier 2 of the skip feature: how much of the top and tail of every episode to pass over. */ @Query("UPDATE subscriptions SET skipIntroMs = :introMs, skipOutroMs = :outroMs WHERE id = :id") suspend fun updateSkips(id: String, introMs: Long, outroMs: Long) diff --git a/app/src/main/java/org/terst/listen/data/Entities.kt b/app/src/main/java/org/terst/listen/data/Entities.kt index 1ef3bde..9101dd7 100644 --- a/app/src/main/java/org/terst/listen/data/Entities.kt +++ b/app/src/main/java/org/terst/listen/data/Entities.kt @@ -128,6 +128,14 @@ data class SubscriptionEntity( * -- see [AppDatabase.MIGRATION_8_9]. */ @ColumnInfo(defaultValue = "0") val bedtime: Boolean = false, + /** + * How soon the server should do this show's work -- see [ShowTier]. + * + * Stored as the wire value rather than the enum so a tier written by a later version reads + * back as something rather than crashing the query. Defaults to "ordinary", which starves + * nothing. + */ + @ColumnInfo(defaultValue = "ordinary") val tier: String = "ordinary", ) /** diff --git a/app/src/main/java/org/terst/listen/data/Repositories.kt b/app/src/main/java/org/terst/listen/data/Repositories.kt index 1dc5bad..2655546 100644 --- a/app/src/main/java/org/terst/listen/data/Repositories.kt +++ b/app/src/main/java/org/terst/listen/data/Repositories.kt @@ -164,7 +164,11 @@ class AppGraph(private val context: Context) { SubscriptionCollaborators( onDiscovered = { ruleApplier.apply(it) }, probe = { url -> ChaquopyAudioSource(context).probe(url) }, - onRefreshed = { feedUrls -> skipServer.pushSubscriptions(feedUrls) }, + // The tiers go with the list: they are about the same shows and the server + // drops a tier for a feed the same push removed. + onRefreshed = { feedUrls -> + skipServer.pushSubscriptions(feedUrls, placedTiers()) + }, refreshSkips = { skipRefresher.refreshAll() }, retryWaiting = { waitingDownloads.retryAll() }, ), @@ -194,6 +198,17 @@ class AppGraph(private val context: Context) { ) } + /** + * Where each show has been placed, for the shows placed anywhere but the default. + * + * Only the exceptions: sending "ordinary" for sixty shows would be the same information as + * sending nothing, in sixty times the space. + */ + private suspend fun placedTiers(): Map<String, String> = + subscriptions.getAll() + .filter { ShowTier.of(it.tier) != ShowTier.ORDINARY } + .associate { it.feedUrl to it.tier } + /** Shared between the download path and [skipRefresher], which write the server's answer * into exactly the same columns. */ val serverSkips: ServerSkips by lazy { diff --git a/app/src/main/java/org/terst/listen/data/ShowTier.kt b/app/src/main/java/org/terst/listen/data/ShowTier.kt new file mode 100644 index 0000000..f0a2d97 --- /dev/null +++ b/app/src/main/java/org/terst/listen/data/ShowTier.kt @@ -0,0 +1,37 @@ +package org.terst.listen.data + +/** + * How soon a show's server-side work is worth doing. + * + * Peter's framing: a few shows are news and stop being worth hearing within a day, a tier he + * keeps up with, and a long tail that is not topical at all. It matters because recognising one + * episode costs the server about five minutes, so a five-hour back-catalogue episode arriving + * the same morning as the news takes most of a cycle before the news is looked at. + * + * Three and no more. A number would invite fiddling and would not mean anything; these are the + * three distinctions that were actually described. + */ +enum class ShowTier(val wire: String, val label: String) { + /** News: worth hearing today and worthless by Thursday. */ + TOPICAL("topical", "Topical"), + + /** + * A show kept up with. The default, because it is what most of a library is and because a + * show nobody has placed should not be starved. + */ + ORDINARY("ordinary", "Keeping up"), + + /** A back catalogue, a slow drip, anything heard whenever. Its work waits. */ + ARCHIVE("archive", "Whenever"); + + companion object { + /** + * Reads a stored value, defaulting anything unrecognised to [ORDINARY]. + * + * A row written by a later version, or a column that has not been set, must not leave a + * show with no tier at all -- the default is the one that starves nothing. + */ + fun of(stored: String?): ShowTier = + entries.firstOrNull { it.wire == stored } ?: ORDINARY + } +} diff --git a/app/src/main/java/org/terst/listen/server/SkipServer.kt b/app/src/main/java/org/terst/listen/server/SkipServer.kt index 68a04ed..8f2141c 100644 --- a/app/src/main/java/org/terst/listen/server/SkipServer.kt +++ b/app/src/main/java/org/terst/listen/server/SkipServer.kt @@ -232,7 +232,15 @@ class SkipServer( * import, the queue rules, Android Auto's browse tree -- keeps working untouched, and the * server stays something the app prefers rather than depends on. */ - suspend fun pushSubscriptions(feedUrls: List<String>): Boolean { + suspend fun pushSubscriptions( + feedUrls: List<String>, + /** + * How soon each show's work is worth doing, keyed by feed URL, for the shows placed + * anywhere other than the default. Sent beside the list rather than folded into it, so + * a server that knows nothing about tiers reads the list exactly as before. + */ + tiers: Map<String, String> = emptyMap(), + ): Boolean { val target = config() ?: return false val body = buildString { append("""{"feeds":[""") @@ -240,7 +248,16 @@ class SkipServer( if (index > 0) append(",") append(JsonPrimitive(url)) } - append("]}") + append("]") + if (tiers.isNotEmpty()) { + append(""","tiers":{""") + tiers.entries.forEachIndexed { index, (url, tier) -> + if (index > 0) append(",") + append(JsonPrimitive(url)).append(":").append(JsonPrimitive(tier)) + } + append("}") + } + append("}") } return observe(target, transport.post(base(target) + "/api/subscriptions", target.token, body)) != null } 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 b387a2c..693fee3 100644 --- a/app/src/main/java/org/terst/listen/ui/ShowSettingsSheet.kt +++ b/app/src/main/java/org/terst/listen/ui/ShowSettingsSheet.kt @@ -18,6 +18,7 @@ import androidx.compose.runtime.collectAsState import androidx.compose.runtime.getValue import androidx.compose.ui.Modifier import androidx.compose.ui.unit.dp +import org.terst.listen.data.ShowTier import org.terst.listen.data.SubscriptionEntity /** @@ -63,6 +64,10 @@ fun ShowSettingsSheet( HorizontalDivider() + TierSetting(ShowTier.of(show.tier)) { settings.setTier(show.id, it) } + + HorizontalDivider() + DripSettings( subscription = show, busy = busy, 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 a84fa40..56d172f 100644 --- a/app/src/main/java/org/terst/listen/ui/ShowSettingsViewModel.kt +++ b/app/src/main/java/org/terst/listen/ui/ShowSettingsViewModel.kt @@ -10,6 +10,7 @@ import kotlinx.coroutines.launch import org.terst.listen.data.ArchiveOutcome import org.terst.listen.data.FingerprintKind import org.terst.listen.data.Graph +import org.terst.listen.data.ShowTier import org.terst.listen.fingerprint.IntroOutroDetector import org.terst.listen.fingerprint.MidrollDetector import org.terst.listen.fingerprint.SkipSummary @@ -115,6 +116,23 @@ class ShowSettingsViewModel(app: Application) : AndroidViewModel(app) { } /** Moves a show in or out of the bedtime list. */ + /** + * Where this show sits in the server's queue. + * + * The banner says what it does and what it does not, because it changes nothing the + * listener can hear and a setting that looks like it should is worth being plain about. + */ + fun setTier(subscriptionId: String, tier: ShowTier) { + viewModelScope.launch { + graph.subscriptions.setTier(subscriptionId, tier.wire) + _banner.value = when (tier) { + ShowTier.TOPICAL -> Banner("The server will do this show first") + ShowTier.ORDINARY -> Banner("The server will do this show in its usual turn") + ShowTier.ARCHIVE -> Banner("The server will do this show when nothing else is waiting") + } + } + } + fun setBedtime(subscriptionId: String, enabled: Boolean) { viewModelScope.launch { graph.subscriptions.setBedtime(subscriptionId, enabled) diff --git a/app/src/main/java/org/terst/listen/ui/TierSetting.kt b/app/src/main/java/org/terst/listen/ui/TierSetting.kt new file mode 100644 index 0000000..6121db0 --- /dev/null +++ b/app/src/main/java/org/terst/listen/ui/TierSetting.kt @@ -0,0 +1,45 @@ +package org.terst.listen.ui + +import androidx.compose.foundation.layout.Arrangement +import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.Row +import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.material3.FilterChip +import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.Text +import androidx.compose.runtime.Composable +import androidx.compose.ui.Modifier +import androidx.compose.ui.unit.dp +import org.terst.listen.data.ShowTier + +/** + * How soon the server should do this show's work. + * + * Chips rather than a dropdown: there are three, they are short, and the whole point is to see + * at a glance which one a show is in. The same shape the speed control already uses. + * + * It is a server-side scheduling hint and nothing else -- no episode is treated differently on + * the phone, and a show placed anywhere still plays, downloads and skips identically. The + * description says so, because a control that looks like it changes playback and does not is + * worse than no control. + */ +@Composable +fun TierSetting(current: ShowTier, onChoose: (ShowTier) -> Unit) { + Column(Modifier.fillMaxWidth(), verticalArrangement = Arrangement.spacedBy(4.dp)) { + Text("Priority", style = MaterialTheme.typography.bodyLarge) + Text( + "Which shows the server works on first. Nothing about playback changes.", + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + ) + Row(horizontalArrangement = Arrangement.spacedBy(8.dp)) { + for (tier in ShowTier.entries) { + FilterChip( + selected = tier == current, + onClick = { onChoose(tier) }, + label = { Text(tier.label) }, + ) + } + } + } +} diff --git a/app/src/test/java/org/terst/listen/data/FakeDaos.kt b/app/src/test/java/org/terst/listen/data/FakeDaos.kt index ec76368..f3b9a23 100644 --- a/app/src/test/java/org/terst/listen/data/FakeDaos.kt +++ b/app/src/test/java/org/terst/listen/data/FakeDaos.kt @@ -62,6 +62,10 @@ class FakeSubscriptionDao : SubscriptionDao { replace(id) { it.copy(bedtime = enabled) } } + override suspend fun setTier(id: String, tier: String) { + replace(id) { it.copy(tier = tier) } + } + override suspend fun updateSkips(id: String, introMs: Long, outroMs: Long) { replace(id) { it.copy(skipIntroMs = introMs, skipOutroMs = outroMs) } } diff --git a/app/src/test/java/org/terst/listen/data/ShowTierTest.kt b/app/src/test/java/org/terst/listen/data/ShowTierTest.kt new file mode 100644 index 0000000..45e1886 --- /dev/null +++ b/app/src/test/java/org/terst/listen/data/ShowTierTest.kt @@ -0,0 +1,49 @@ +package org.terst.listen.data + +import org.junit.Assert.assertEquals +import org.junit.Test + +/** + * Where a show sits in the server's queue. + * + * The default is load-bearing: a show nobody has placed must not be starved, and a value this + * version does not recognise -- written by a later one, or never set at all -- has to read as + * something rather than nothing. + */ +class ShowTierTest { + + @Test + fun `a stored tier reads back`() { + assertEquals(ShowTier.TOPICAL, ShowTier.of("topical")) + assertEquals(ShowTier.ARCHIVE, ShowTier.of("archive")) + } + + @Test + fun `nothing stored means keeping up`() { + assertEquals(ShowTier.ORDINARY, ShowTier.of(null)) + assertEquals(ShowTier.ORDINARY, ShowTier.of("")) + } + + /** A value from a later version must leave the show working, not unplaced. */ + @Test + fun `an unrecognised tier means keeping up`() { + assertEquals(ShowTier.ORDINARY, ShowTier.of("urgent-ish")) + } + + /** The wire value is what the server stores, so it must not drift from the enum. */ + @Test + fun `every tier round trips through its wire value`() { + for (tier in ShowTier.entries) { + assertEquals(tier, ShowTier.of(tier.wire)) + } + } + + /** Each one is offered to the listener by name, so none may be blank. */ + @Test + fun `every tier has something to show`() { + for (tier in ShowTier.entries) { + assertEquals(tier.label, tier.label.trim()) + assert(tier.label.isNotEmpty()) { "$tier has no label" } + } + } +} diff --git a/app/src/test/java/org/terst/listen/server/SkipServerTest.kt b/app/src/test/java/org/terst/listen/server/SkipServerTest.kt index 650c992..78c3987 100644 --- a/app/src/test/java/org/terst/listen/server/SkipServerTest.kt +++ b/app/src/test/java/org/terst/listen/server/SkipServerTest.kt @@ -396,4 +396,57 @@ class SkipServerTest { assertEquals(false, server(transport, token = null).isConfigured()) assertTrue("asking must not touch the network", transport.gets.isEmpty()) } + + /* + * The body the subscription push actually sends. + * + * Pinned because it is hand-built JSON going over the wire: a missing comma or brace is a + * push the server rejects, and the only symptom is a library that silently stops being + * mirrored. The tiers ride beside the list rather than inside it, so a server that knows + * nothing about them reads the list exactly as it always did. + */ + + @Test + fun `sends the feed list as json`() = runTest { + val transport = FakeTransport() + + server(transport).pushSubscriptions(listOf("https://a", "https://b")) + + assertEquals("""{"feeds":["https://a","https://b"]}""", transport.posts.single().second) + } + + @Test + fun `sends the tiers beside the list`() = runTest { + val transport = FakeTransport() + + server(transport).pushSubscriptions(listOf("https://a"), mapOf("https://a" to "topical")) + + assertEquals( + """{"feeds":["https://a"],"tiers":{"https://a":"topical"}}""", + transport.posts.single().second, + ) + } + + /** No tiers placed means no tiers sent, not an empty object. */ + @Test + fun `sends no tiers when none are placed`() = runTest { + val transport = FakeTransport() + + server(transport).pushSubscriptions(listOf("https://a"), emptyMap()) + + assertEquals("""{"feeds":["https://a"]}""", transport.posts.single().second) + } + + /** A feed URL carries query strings and ampersands; JSON quoting has to survive them. */ + @Test + fun `quotes a url that would otherwise break the json`() = runTest { + val transport = FakeTransport() + val awkward = """https://x/f?a=1&b="two"""" + + server(transport).pushSubscriptions(listOf(awkward), mapOf(awkward to "archive")) + + val body = transport.posts.single().second + assertTrue("body is not valid json: $body", body.startsWith("""{"feeds":[""")) + assertTrue("the quote was not escaped: $body", body.contains("""\"two\"""")) + } } |