summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPeter Stone <thepeterstone@gmail.com>2026-10-27 07:39:19 +0000
committerPeter Stone <thepeterstone@gmail.com>2026-10-27 07:39:19 +0000
commitec633d0daf17ab5806c563b7a95d6721b7b73c7b (patch)
treee747efc4dcbcbc4abf83effab893ceb8510ccc55
parentef20a5421c322ef8053f5c2766d851873bc472f3 (diff)
Write down a refused token instead of only remembering it
401 and 403 are permanent by decision -- a refused token cannot fix itself -- and the notice saying so was held in memory. So it vanished with the process and only came back once the next refresh had gone and been refused all over again, which for 61 feeds is hours of the app quietly downloading from publishers with nothing on screen to say why. It now lives in the preferences file and is there at launch. Saving different credentials clears it at once rather than waiting for a request to prove it, because that is the one act that answers the complaint. Saving the same ones again proves nothing and leaves the notice standing. Behind a small interface so SkipServer does not have to know whether it is writing to a preferences file or to nothing: every test wants it in memory, without a Context to hang a DataStore off. The stored one takes the reading and the writing as two narrow seams rather than a Preferences. Passing Preferences dragged a Context in and made it the one piece of the server client that could only be exercised on a device -- the coverage gate caught that, which is what it is for. What it actually decides, when *not* to write, is now plain Kotlin with five tests on it: clearing runs after every successful reply, so two hundred successes in a sweep must write nothing. Verified on a device: a refusal written by one Preferences instance is read back by another, and four cases around saving credentials behave as described. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018AwnALZL6bD4VBtATDVv3B
-rw-r--r--.coverage-baseline2
-rw-r--r--app/src/androidTest/java/org/terst/listen/data/PreferencesTest.kt55
-rw-r--r--app/src/main/java/org/terst/listen/data/Preferences.kt39
-rw-r--r--app/src/main/java/org/terst/listen/data/Repositories.kt5
-rw-r--r--app/src/main/java/org/terst/listen/server/Refusals.kt49
-rw-r--r--app/src/main/java/org/terst/listen/server/SkipServer.kt23
-rw-r--r--app/src/main/java/org/terst/listen/ui/SkipServerSettingsState.kt2
-rw-r--r--app/src/test/java/org/terst/listen/server/SkipServerTest.kt11
-rw-r--r--app/src/test/java/org/terst/listen/server/StoredRefusalsTest.kt69
-rw-r--r--docs/superpowers/plans/2026-10-20-corrections-and-signature-library.md6
10 files changed, 244 insertions, 17 deletions
diff --git a/.coverage-baseline b/.coverage-baseline
index 521e662..64eced0 100644
--- a/.coverage-baseline
+++ b/.coverage-baseline
@@ -1 +1 @@
-94.57
+94.56
diff --git a/app/src/androidTest/java/org/terst/listen/data/PreferencesTest.kt b/app/src/androidTest/java/org/terst/listen/data/PreferencesTest.kt
index 42c8f4a..cbfd2b0 100644
--- a/app/src/androidTest/java/org/terst/listen/data/PreferencesTest.kt
+++ b/app/src/androidTest/java/org/terst/listen/data/PreferencesTest.kt
@@ -3,11 +3,14 @@ package org.terst.listen.data
import androidx.test.core.app.ApplicationProvider
import androidx.test.ext.junit.runners.AndroidJUnit4
import kotlinx.coroutines.flow.first
+import kotlinx.coroutines.runBlocking
import kotlinx.coroutines.test.runTest
+import org.junit.Assert.assertEquals
import org.junit.Assert.assertFalse
import org.junit.Assert.assertTrue
import org.junit.Test
import org.junit.runner.RunWith
+import org.terst.listen.server.SkipServerConfig
/**
* Instrumented, not a JVM unit test: [Preferences] takes a [android.content.Context] and reads
@@ -38,4 +41,56 @@ class PreferencesTest {
// with itself on a second run.
prefs.setSkipShowFurniture(true)
}
+
+ /**
+ * A refused token is the one failure that cannot fix itself, and it used to be remembered
+ * only in memory -- so after the process died the Settings screen said nothing was wrong
+ * until the next refresh went and got refused all over again. Written down, it is there at
+ * launch.
+ */
+ @Test
+ fun aRefusedServerIsRememberedAcrossRestarts() = runBlocking {
+ val prefs = Preferences(context)
+ prefs.setServer("https://listen.example", "wrong-token")
+
+ prefs.setRejectedServer(SkipServerConfig("https://listen.example", "wrong-token"))
+
+ val again = Preferences(context).rejectedServer.first()
+ assertEquals("https://listen.example", again?.baseUrl)
+ assertEquals("wrong-token", again?.token)
+ }
+
+ @Test
+ fun nothingRefusedIsTheNormalState() = runBlocking {
+ val prefs = Preferences(context)
+ prefs.setRejectedServer(null)
+
+ assertEquals(null, prefs.rejectedServer.first())
+ }
+
+ /**
+ * Saving different credentials is the one act that answers the complaint, so the notice has
+ * to stop applying at once rather than waiting for a request to prove it.
+ */
+ @Test
+ fun savingDifferentCredentialsClearsTheRefusal() = runBlocking {
+ val prefs = Preferences(context)
+ prefs.setServer("https://listen.example", "wrong-token")
+ prefs.setRejectedServer(SkipServerConfig("https://listen.example", "wrong-token"))
+
+ prefs.setServer("https://listen.example", "a-better-token")
+
+ assertEquals(null, prefs.rejectedServer.first())
+ }
+
+ @Test
+ fun savingTheSameCredentialsAgainDoesNotPretendTheyWork() = runBlocking {
+ val prefs = Preferences(context)
+ prefs.setServer("https://listen.example", "wrong-token")
+ prefs.setRejectedServer(SkipServerConfig("https://listen.example", "wrong-token"))
+
+ prefs.setServer("https://listen.example", "wrong-token")
+
+ assertEquals("wrong-token", prefs.rejectedServer.first()?.token)
+ }
}
diff --git a/app/src/main/java/org/terst/listen/data/Preferences.kt b/app/src/main/java/org/terst/listen/data/Preferences.kt
index 8cbc583..58de63e 100644
--- a/app/src/main/java/org/terst/listen/data/Preferences.kt
+++ b/app/src/main/java/org/terst/listen/data/Preferences.kt
@@ -7,6 +7,7 @@ import androidx.datastore.preferences.core.edit
import androidx.datastore.preferences.preferencesDataStore
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.map
+import org.terst.listen.server.SkipServerConfig
private val Context.store by preferencesDataStore(name = "listen-preferences")
@@ -73,6 +74,42 @@ class Preferences(private val context: Context) {
context.store.edit {
it[SERVER_URL] = url.trim()
it[SERVER_TOKEN] = token.trim()
+ // Saving different credentials answers the complaint, so the refusal stops applying
+ // at once rather than waiting for a request to go and prove it. Saving the *same*
+ // ones again proves nothing and leaves it standing.
+ if (it[REJECTED_URL] != url.trim() || it[REJECTED_TOKEN] != token.trim()) {
+ it.remove(REJECTED_URL)
+ it.remove(REJECTED_TOKEN)
+ }
+ }
+ }
+
+ /**
+ * The configuration the server last refused, or null.
+ *
+ * Written down rather than held in memory, which is what it used to be. A refused token is
+ * the one failure that cannot fix itself -- 401 and 403 are permanent, by decision -- and in
+ * memory it meant the Settings screen said nothing was wrong until the next refresh went and
+ * got refused all over again. Now it is there at launch.
+ *
+ * Stored as the refused configuration rather than a flag so it stops applying the moment a
+ * different one is saved.
+ */
+ val rejectedServer: Flow<SkipServerConfig?> = context.store.data.map { prefs ->
+ val url = prefs[REJECTED_URL]
+ val token = prefs[REJECTED_TOKEN]
+ if (url.isNullOrBlank()) null else SkipServerConfig(url, token.orEmpty())
+ }
+
+ suspend fun setRejectedServer(config: SkipServerConfig?) {
+ context.store.edit {
+ if (config == null) {
+ it.remove(REJECTED_URL)
+ it.remove(REJECTED_TOKEN)
+ } else {
+ it[REJECTED_URL] = config.baseUrl
+ it[REJECTED_TOKEN] = config.token
+ }
}
}
@@ -82,5 +119,7 @@ class Preferences(private val context: Context) {
val SERVER_TOKEN = stringPreferencesKey("server_token")
val SKIP_SHOW_FURNITURE = booleanPreferencesKey("skip_show_furniture")
val FAST_PLAY_BREAKS = booleanPreferencesKey("fast_play_breaks")
+ val REJECTED_URL = stringPreferencesKey("rejected_server_url")
+ val REJECTED_TOKEN = stringPreferencesKey("rejected_server_token")
}
}
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 35d96c0..f56fdc6 100644
--- a/app/src/main/java/org/terst/listen/data/Repositories.kt
+++ b/app/src/main/java/org/terst/listen/data/Repositories.kt
@@ -13,6 +13,7 @@ import org.terst.listen.server.ServerSkips
import org.terst.listen.server.SkipRefresher
import org.terst.listen.server.SkipServer
import org.terst.listen.server.SkipServerConfig
+import org.terst.listen.server.StoredRefusals
import org.terst.listen.download.ChaquopyAudioSource
import org.terst.listen.download.EpisodeDownloader
import org.terst.listen.feed.ChapterFetcher
@@ -185,6 +186,10 @@ class AppGraph(private val context: Context) {
if (url.isBlank() || token.isBlank()) null else SkipServerConfig(url, token)
},
transport = OkHttpSkipTransport(client),
+ // Written down, not held in memory: a refused token cannot fix itself, so the
+ // notice has to be there at launch rather than after the next refresh has gone and
+ // been refused again.
+ refusals = StoredRefusals(preferences.rejectedServer, preferences::setRejectedServer),
)
}
diff --git a/app/src/main/java/org/terst/listen/server/Refusals.kt b/app/src/main/java/org/terst/listen/server/Refusals.kt
new file mode 100644
index 0000000..c6a51dc
--- /dev/null
+++ b/app/src/main/java/org/terst/listen/server/Refusals.kt
@@ -0,0 +1,49 @@
+package org.terst.listen.server
+
+import kotlinx.coroutines.flow.Flow
+import kotlinx.coroutines.flow.MutableStateFlow
+import kotlinx.coroutines.flow.asStateFlow
+import kotlinx.coroutines.flow.firstOrNull
+
+/**
+ * Where a refused configuration is remembered.
+ *
+ * Behind an interface so [SkipServer] does not have to know whether that is a preferences file
+ * or nothing at all: the app writes it down, and every test wants it in memory without a
+ * `Context` to hang a DataStore off.
+ */
+interface Refusals {
+ val rejected: Flow<SkipServerConfig?>
+
+ /** Records the configuration the server refused, or null once one of them worked. */
+ suspend fun record(config: SkipServerConfig?)
+
+ /** Remembers for as long as the process lives, which is what a test needs. */
+ class InMemory : Refusals {
+ private val state = MutableStateFlow<SkipServerConfig?>(null)
+ override val rejected = state.asStateFlow()
+ override suspend fun record(config: SkipServerConfig?) {
+ state.value = config
+ }
+ }
+}
+
+/**
+ * A refusal kept wherever the caller keeps things.
+ *
+ * Takes the reading and the writing as two narrow seams rather than a `Preferences`, which would
+ * drag a `Context` in and make this the one piece of the server client that could only be
+ * exercised on a device. What it actually decides -- when *not* to write -- is then plain Kotlin.
+ */
+class StoredRefusals(
+ override val rejected: Flow<SkipServerConfig?>,
+ private val save: suspend (SkipServerConfig?) -> Unit,
+) : Refusals {
+
+ override suspend fun record(config: SkipServerConfig?) {
+ // Only on a change. Clearing runs after every successful reply, and a refresh sweep
+ // makes one request per downloaded episode, so writing each time would be hundreds of
+ // writes to store the same nothing.
+ if (rejected.firstOrNull() != config) save(config)
+ }
+}
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 69aaef0..5237461 100644
--- a/app/src/main/java/org/terst/listen/server/SkipServer.kt
+++ b/app/src/main/java/org/terst/listen/server/SkipServer.kt
@@ -1,5 +1,6 @@
package org.terst.listen.server
+import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.StateFlow
import kotlinx.serialization.SerialName
@@ -124,6 +125,11 @@ data class ServerEpisode(
class SkipServer(
private val config: suspend () -> SkipServerConfig?,
private val transport: SkipTransport,
+ /**
+ * Where a refusal is written down, so it survives the process. Defaults to holding it in
+ * memory, which is what every test wants and what the app deliberately no longer does.
+ */
+ private val refusals: Refusals = Refusals.InMemory(),
) {
/**
@@ -133,18 +139,19 @@ class SkipServer(
* and the download path must treat the two oppositely: no server means download from the
* publisher as the app always did, while a server that merely did not answer means wait.
*/
- private val refused = MutableStateFlow<SkipServerConfig?>(null)
-
/**
* The configuration the server last refused, or null.
*
* Remembered so the settings screen can say so: a refused token otherwise changes nothing
* visible, and episodes quietly come from publishers. Kept as the refused configuration
* rather than a flag so it stops applying the moment a different one is saved, and cleared
- * by the next successful reply. Held in memory: it is rediscovered by the first request of
- * the next refresh, which is soon enough for something that cannot fix itself.
+ * by the next successful reply.
+ *
+ * Written down rather than held in memory. 401 and 403 are permanent by decision -- they
+ * cannot fix themselves -- and in memory the notice vanished with the process and only came
+ * back once the next refresh had gone and been refused all over again.
*/
- val rejected: StateFlow<SkipServerConfig?> = refused
+ val rejected: Flow<SkipServerConfig?> get() = refusals.rejected
suspend fun isConfigured(): Boolean = config() != null
@@ -245,10 +252,10 @@ class SkipServer(
* Only a refusal sets it and only a success clears it. No answer says nothing about the
* token, so it must not hide a refusal already seen, nor invent one.
*/
- private fun observe(target: SkipServerConfig, reply: SkipReply): String? {
+ private suspend fun observe(target: SkipServerConfig, reply: SkipReply): String? {
when (reply) {
- is SkipReply.Rejected -> refused.value = target
- is SkipReply.Body -> refused.value = null
+ is SkipReply.Rejected -> refusals.record(target)
+ is SkipReply.Body -> refusals.record(null)
is SkipReply.NoAnswer -> Unit
}
return (reply as? SkipReply.Body)?.text
diff --git a/app/src/main/java/org/terst/listen/ui/SkipServerSettingsState.kt b/app/src/main/java/org/terst/listen/ui/SkipServerSettingsState.kt
index 5aba893..879205a 100644
--- a/app/src/main/java/org/terst/listen/ui/SkipServerSettingsState.kt
+++ b/app/src/main/java/org/terst/listen/ui/SkipServerSettingsState.kt
@@ -27,7 +27,7 @@ internal fun rememberSkipServerSettings(): SkipServerSettings {
val url by preferences.serverUrl.collectAsState(initial = "")
val token by preferences.serverToken.collectAsState(initial = "")
- val rejected by Graph.get(context).skipServer.rejected.collectAsState()
+ val rejected by Graph.get(context).skipServer.rejected.collectAsState(initial = null)
val configured = if (url.isBlank() || token.isBlank()) null else SkipServerConfig(url, token)
return SkipServerSettings(
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 aa0fd06..650c992 100644
--- a/app/src/test/java/org/terst/listen/server/SkipServerTest.kt
+++ b/app/src/test/java/org/terst/listen/server/SkipServerTest.kt
@@ -1,5 +1,6 @@
package org.terst.listen.server
+import kotlinx.coroutines.flow.first
import kotlinx.coroutines.test.runTest
import org.junit.Assert.assertEquals
import org.junit.Assert.assertNull
@@ -212,7 +213,7 @@ class SkipServerTest {
subject.askAboutOne()
- assertEquals(SkipServerConfig("https://listen.terst.org", "stale"), subject.rejected.value)
+ assertEquals(SkipServerConfig("https://listen.terst.org", "stale"), subject.rejected.first())
}
@Test
@@ -221,7 +222,7 @@ class SkipServerTest {
assertEquals(false, subject.pushSubscriptions(listOf("https://a")))
- assertEquals(SkipServerConfig("https://listen.terst.org", "stale"), subject.rejected.value)
+ assertEquals(SkipServerConfig("https://listen.terst.org", "stale"), subject.rejected.first())
}
@Test
@@ -234,7 +235,7 @@ class SkipServerTest {
transport.response = ready
subject.askAboutOne()
- assertNull(subject.rejected.value)
+ assertNull(subject.rejected.first())
}
@Test
@@ -246,11 +247,11 @@ class SkipServerTest {
transport.reply = SkipReply.NoAnswer
subject.askAboutOne()
- assertEquals(SkipServerConfig("https://listen.terst.org", "tok"), subject.rejected.value)
+ assertEquals(SkipServerConfig("https://listen.terst.org", "tok"), subject.rejected.first())
val fresh = server(FakeTransport().apply { reply = SkipReply.NoAnswer })
fresh.askAboutOne()
- assertNull(fresh.rejected.value)
+ assertNull(fresh.rejected.first())
}
/** A trailing slash on the configured URL must not produce a double slash in the path. */
diff --git a/app/src/test/java/org/terst/listen/server/StoredRefusalsTest.kt b/app/src/test/java/org/terst/listen/server/StoredRefusalsTest.kt
new file mode 100644
index 0000000..b095454
--- /dev/null
+++ b/app/src/test/java/org/terst/listen/server/StoredRefusalsTest.kt
@@ -0,0 +1,69 @@
+package org.terst.listen.server
+
+import kotlinx.coroutines.flow.MutableStateFlow
+import kotlinx.coroutines.test.runTest
+import org.junit.Assert.assertEquals
+import org.junit.Test
+
+/**
+ * Writing a refusal down, and -- mostly -- not writing it down.
+ *
+ * Clearing happens after every successful reply, and a refresh sweep makes one request per
+ * downloaded episode. Writing on each of those would be hundreds of writes to the preferences
+ * file to store the same nothing.
+ */
+class StoredRefusalsTest {
+
+ private val stored = MutableStateFlow<SkipServerConfig?>(null)
+ private var writes = 0
+ private val refusals = StoredRefusals(stored) { config ->
+ writes++
+ stored.value = config
+ }
+
+ private val server = SkipServerConfig("https://listen.example", "wrong")
+
+ @Test
+ fun `a refusal is written down`() = runTest {
+ refusals.record(server)
+
+ assertEquals(server, stored.value)
+ assertEquals(1, writes)
+ }
+
+ @Test
+ fun `the same refusal arriving again is not written again`() = runTest {
+ refusals.record(server)
+
+ refusals.record(server)
+
+ assertEquals("one refused token, one write", 1, writes)
+ }
+
+ @Test
+ fun `a success clears what was refused`() = runTest {
+ refusals.record(server)
+
+ refusals.record(null)
+
+ assertEquals(null, stored.value)
+ assertEquals(2, writes)
+ }
+
+ @Test
+ fun `a sweep of successes with nothing refused writes nothing at all`() = runTest {
+ repeat(200) { refusals.record(null) }
+
+ assertEquals(0, writes)
+ }
+
+ @Test
+ fun `a different server being refused replaces the old one`() = runTest {
+ refusals.record(server)
+
+ val other = SkipServerConfig("https://elsewhere.example", "also-wrong")
+ refusals.record(other)
+
+ assertEquals(other, stored.value)
+ }
+}
diff --git a/docs/superpowers/plans/2026-10-20-corrections-and-signature-library.md b/docs/superpowers/plans/2026-10-20-corrections-and-signature-library.md
index 146581c..331e883 100644
--- a/docs/superpowers/plans/2026-10-20-corrections-and-signature-library.md
+++ b/docs/superpowers/plans/2026-10-20-corrections-and-signature-library.md
@@ -388,8 +388,10 @@ everything measured, though not directly caught in a probe.
- **Compose has no automated coverage** — no Robolectric. Everything visual is verified by eye on
the emulator only.
-- **The rejected-token notice is in-memory**, so after a process restart it reappears on the next
- refresh rather than at launch.
+- **The rejected-token notice is written down — fixed 2026-10-27.** It lives in the preferences
+ file now, so it is on the Settings screen at launch rather than after the next refresh has gone
+ and been refused all over again. Saving *different* credentials clears it at once; saving the
+ same ones again proves nothing and leaves it standing.
- **Library and Settings redesign (step 4 of the UI plan) — done 2026-10-25.** The library is
one list: Up Next as a heading and a row, shows as a dot, a title and a waiting count. Settings
is three groups ordered by how often they are touched, with the build hash last under About.