From 9c97f64381cf4a6be353d3a5c920e92744fbcff2 Mon Sep 17 00:00:00 2001 From: austinried <4966622+austinried@users.noreply.github.com> Date: Sat, 26 Sep 2026 01:47:48 +0000 Subject: [PATCH] address review: probe the active source so media falls back too An existing token-auth source with an already-synced library would fail playback before any API call ran: stream and cover-art URLs are built by uri() and fetched by ExoPlayer/Coil, outside stream(). Probe the active source once when it uses token auth so the 41 flips and persists before any URL is built. Also base the retry on the flag the request actually used (concurrent requests no longer lose their retry), report the fallback from ping() so Add Source flips the toggle instead of reporting success and saving token auth on, and make the callback non-null. --- .../subtracks/data/repo/SourceRepository.kt | 40 +++++++++----- .../data/source/subsonic/SubsonicClient.kt | 12 ++-- .../ui/settings/AddSourceViewModel.kt | 12 +++- .../data/repo/SourceRepositoryTest.kt | 55 ++++++++++++++++++- .../source/subsonic/SubsonicClientTest.kt | 2 +- 5 files changed, 98 insertions(+), 23 deletions(-) diff --git a/app/src/main/kotlin/com/subtracks/data/repo/SourceRepository.kt b/app/src/main/kotlin/com/subtracks/data/repo/SourceRepository.kt index b9fffda3..007a2317 100644 --- a/app/src/main/kotlin/com/subtracks/data/repo/SourceRepository.kt +++ b/app/src/main/kotlin/com/subtracks/data/repo/SourceRepository.kt @@ -31,6 +31,7 @@ class SourceRepository( @Volatile private var active: SubsonicMusicSource? = null + private var probedTokenAuthId: Long? = null init { scope.launch { @@ -38,8 +39,16 @@ class SourceRepository( db.sourcesDao().activeSubsonicConfig(), prefs.maxBitrate, prefs.streamFormat, - ) { config, maxBitrate, streamFormat -> config?.toMusicSource(maxBitrate, streamFormat) } - .collect { active = it } + ) { config, maxBitrate, streamFormat -> Triple(config, maxBitrate, streamFormat) } + .collect { (config, maxBitrate, streamFormat) -> + active = config?.toMusicSource(maxBitrate, streamFormat) + config + ?.takeIf { it.useTokenAuth && it.id != probedTokenAuthId } + ?.let { source -> + probedTokenAuthId = source.id + scope.launch { runCatching { source.toClient().check("ping") } } + } + } } } @@ -111,9 +120,12 @@ class SourceRepository( username: String, password: String, useTokenAuth: Boolean, - ): Result = + ): Result = withContext(Dispatchers.IO) { - runCatching { client(address, username, password, useTokenAuth).check("ping") } + var fellBack = false + runCatching { + client(address, username, password, useTokenAuth) { fellBack = true }.check("ping") + }.map { fellBack } } suspend fun activeMusicSource(): MusicSource? { @@ -121,20 +133,22 @@ class SourceRepository( return config.toMusicSource(prefs.maxBitrate.first(), prefs.streamFormat.first()) } + private fun SubsonicConfig.toClient(): SubsonicClient = + client( + address = address, + username = username, + password = password, + useTokenAuth = useTokenAuth, + onTokenAuthUnsupported = { disableTokenAuth(id) }, + ) + private fun SubsonicConfig.toMusicSource( maxBitrate: Int, streamFormat: String?, ): SubsonicMusicSource = SubsonicMusicSource( id = id, - client = - client( - address = address, - username = username, - password = password, - useTokenAuth = useTokenAuth, - onTokenAuthUnsupported = { disableTokenAuth(id) }, - ), + client = toClient(), maxBitrate = maxBitrate, streamFormat = streamFormat, ) @@ -151,7 +165,7 @@ class SourceRepository( username: String, password: String, useTokenAuth: Boolean, - onTokenAuthUnsupported: (() -> Unit)? = null, + onTokenAuthUnsupported: () -> Unit = {}, ) = SubsonicClient( baseUrl = normalizeAddress(address).toHttpUrl(), username = username, diff --git a/app/src/main/kotlin/com/subtracks/data/source/subsonic/SubsonicClient.kt b/app/src/main/kotlin/com/subtracks/data/source/subsonic/SubsonicClient.kt index e298c2b7..720db58f 100644 --- a/app/src/main/kotlin/com/subtracks/data/source/subsonic/SubsonicClient.kt +++ b/app/src/main/kotlin/com/subtracks/data/source/subsonic/SubsonicClient.kt @@ -18,7 +18,7 @@ class SubsonicClient( useTokenAuth: Boolean, private val http: OkHttpClient, private val userAgent: String = "subtracks/android", - private val onTokenAuthUnsupported: (() -> Unit)? = null, + private val onTokenAuthUnsupported: () -> Unit = {}, ) { @Volatile private var useTokenAuth: Boolean = useTokenAuth @@ -53,14 +53,16 @@ class SubsonicClient( method: String, params: Map = emptyMap(), body: (InputStream) -> T, - ): T = - try { + ): T { + val usedTokenAuth = useTokenAuth + return try { execute(method, params, body) } catch (failure: SubsonicException) { - if (failure.code != TOKEN_AUTH_UNSUPPORTED || !useTokenAuth) throw failure + if (failure.code != TOKEN_AUTH_UNSUPPORTED || !usedTokenAuth) throw failure disableTokenAuth() execute(method, params, body) } + } private fun execute( method: String, @@ -85,7 +87,7 @@ class SubsonicClient( private fun disableTokenAuth() { if (!useTokenAuth) return useTokenAuth = false - onTokenAuthUnsupported?.invoke() + onTokenAuthUnsupported() } companion object { diff --git a/app/src/main/kotlin/com/subtracks/ui/settings/AddSourceViewModel.kt b/app/src/main/kotlin/com/subtracks/ui/settings/AddSourceViewModel.kt index aba01642..3e5efabf 100644 --- a/app/src/main/kotlin/com/subtracks/ui/settings/AddSourceViewModel.kt +++ b/app/src/main/kotlin/com/subtracks/ui/settings/AddSourceViewModel.kt @@ -42,10 +42,16 @@ class AddSourceViewModel( val current = _state.value viewModelScope.launch { val result = sourceRepository.ping(current.address, current.username, current.password, current.useTokenAuth) - _state.update { - it.copy( + _state.update { state -> + val fellBack = result.getOrDefault(false) + state.copy( busy = false, - message = result.fold({ "Connection OK" }, { error -> "Failed: ${error.message}" }), + useTokenAuth = if (fellBack) false else state.useTokenAuth, + message = + result.fold( + { if (fellBack) "Server does not support token auth; using the password instead" else "Connection OK" }, + { error -> "Failed: ${error.message}" }, + ), isError = result.isFailure, ) } diff --git a/app/src/test/kotlin/com/subtracks/data/repo/SourceRepositoryTest.kt b/app/src/test/kotlin/com/subtracks/data/repo/SourceRepositoryTest.kt index 3cc8fc30..db67244e 100644 --- a/app/src/test/kotlin/com/subtracks/data/repo/SourceRepositoryTest.kt +++ b/app/src/test/kotlin/com/subtracks/data/repo/SourceRepositoryTest.kt @@ -7,11 +7,18 @@ import androidx.test.core.app.ApplicationProvider import androidx.test.ext.junit.runners.AndroidJUnit4 import com.subtracks.data.db.SubtracksDatabase import com.subtracks.data.prefs.fakeUserPreferences +import kotlinx.coroutines.delay import kotlinx.coroutines.flow.first +import kotlinx.coroutines.runBlocking import kotlinx.coroutines.test.runTest +import kotlinx.coroutines.withTimeout import okhttp3.OkHttpClient +import okhttp3.mockwebserver.MockResponse +import okhttp3.mockwebserver.MockWebServer import org.junit.After import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test import org.junit.runner.RunWith @@ -20,6 +27,7 @@ import org.junit.runner.RunWith class SourceRepositoryTest { private lateinit var db: SubtracksDatabase private lateinit var repository: SourceRepository + private val messages = ArrayList() @Before fun setUp() { @@ -29,7 +37,7 @@ class SourceRepositoryTest { .inMemoryDatabaseBuilder(context, SubtracksDatabase::class.java) .setDriver(BundledSQLiteDriver()) .build() - repository = SourceRepository(db, OkHttpClient(), fakeUserPreferences()) + repository = SourceRepository(db, OkHttpClient(), fakeUserPreferences(), showMessage = { messages += it }) } @After @@ -57,4 +65,49 @@ class SourceRepositoryTest { .address, ) } + + @Test + fun aTokenAuthSourceIsProbedAndSwitchedToPassword() = + runBlocking { + withServer { server -> + server.enqueue(MockResponse().setBody(failed(41))) + server.enqueue(MockResponse().setBody(OK)) + + repository.addSource("nav", server.url("/").toString(), "u", "s3cret", true) + + val config = withTimeout(5_000) { repository.activeConfig().first { it != null && !it.useTokenAuth } } + assertFalse(config!!.useTokenAuth) + withTimeout(5_000) { while (messages.isEmpty()) delay(10) } + assertEquals(1, messages.size) + } + } + + @Test + fun pingReportsWhetherItFellBackToThePassword() = + runBlocking { + withServer { server -> + server.enqueue(MockResponse().setBody(failed(41))) + server.enqueue(MockResponse().setBody(OK)) + server.enqueue(MockResponse().setBody(OK)) + + assertTrue(repository.ping(server.url("/").toString(), "u", "s3cret", true).getOrThrow()) + assertFalse(repository.ping(server.url("/").toString(), "u", "s3cret", false).getOrThrow()) + } + } + + private suspend fun withServer(block: suspend (MockWebServer) -> Unit) { + val server = MockWebServer() + server.start() + try { + block(server) + } finally { + server.shutdown() + } + } + + private fun failed(code: Int) = "" + + private companion object { + const val OK = "" + } } diff --git a/app/src/test/kotlin/com/subtracks/data/source/subsonic/SubsonicClientTest.kt b/app/src/test/kotlin/com/subtracks/data/source/subsonic/SubsonicClientTest.kt index e8f41dbe..7752ff14 100644 --- a/app/src/test/kotlin/com/subtracks/data/source/subsonic/SubsonicClientTest.kt +++ b/app/src/test/kotlin/com/subtracks/data/source/subsonic/SubsonicClientTest.kt @@ -27,7 +27,7 @@ class SubsonicClientTest { private fun client( tokenAuth: Boolean, - onTokenAuthUnsupported: (() -> Unit)? = null, + onTokenAuthUnsupported: () -> Unit = {}, ) = SubsonicClient( baseUrl = server.url("/"), username = "guest",