From 052d0b8898c40abd6b357ac5f7802a31d53d4b29 Mon Sep 17 00:00:00 2001 From: justinzeus Date: Sat, 7 Nov 2026 01:23:33 +0100 Subject: [PATCH] Keep DNS retry safe for WebSockets and preserve saved user edits --- .../wholphin/data/JellyfinServerDao.kt | 4 ++ .../wholphin/data/ServerRepository.kt | 3 +- .../wholphin/services/hilt/AppModule.kt | 4 +- .../services/ServerSavedSessionTest.kt | 48 +++++++++++-- .../wolfskin/tv/NetworkStartup.kt | 3 +- .../wolfskin/conductarr/DnsConnectionRetry.kt | 3 +- .../wolfskin/conductarr/PersistentDns.kt | 2 +- .../conductarr/DnsConnectionRetryTest.kt | 68 +++++++++++++++++++ .../wolfskin/conductarr/PersistentDnsTest.kt | 16 +++++ docker/README.md | 10 ++- 10 files changed, 144 insertions(+), 17 deletions(-) create mode 100644 conductarr-client/src/test/kotlin/org/justinvisser/wolfskin/conductarr/DnsConnectionRetryTest.kt diff --git a/app/src/main/java/com/github/damontecres/wholphin/data/JellyfinServerDao.kt b/app/src/main/java/com/github/damontecres/wholphin/data/JellyfinServerDao.kt index f72eb3b7..68af5123 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/data/JellyfinServerDao.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/data/JellyfinServerDao.kt @@ -51,6 +51,10 @@ interface JellyfinServerDao { return user.copy(rowId = result.toInt()) } + /** Network name refreshes must not overwrite settings or protection changed since launch. */ + @Query("UPDATE users SET name = :name WHERE serverId = :serverId AND id = :userId") + fun updateUserName(serverId: UUID, userId: UUID, name: String?): Int + @Query("SELECT * FROM users WHERE serverId = :serverId AND id = :userId") fun getUser( serverId: UUID, diff --git a/app/src/main/java/com/github/damontecres/wholphin/data/ServerRepository.kt b/app/src/main/java/com/github/damontecres/wholphin/data/ServerRepository.kt index 95322ea4..0930129e 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/data/ServerRepository.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/data/ServerRepository.kt @@ -138,7 +138,8 @@ class ServerRepository val (server, userDto) = readSession(expected.server) if (_current.value !== expected || userDto.id != expected.user.id) return@withContext serverDao.addOrUpdateServer(server) - val user = serverDao.addOrUpdateUser(expected.user.copy(name = userDto.name)) + serverDao.updateUserName(expected.server.id, expected.user.id, userDto.name) + val user = serverDao.getUser(expected.server.id, expected.user.id) ?: return@withContext if (_current.compareAndSet(expected, CurrentUser(server, user))) updateUserDto(userDto) } diff --git a/app/src/main/java/com/github/damontecres/wholphin/services/hilt/AppModule.kt b/app/src/main/java/com/github/damontecres/wholphin/services/hilt/AppModule.kt index 3b7ec75e..0591f4a1 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/services/hilt/AppModule.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/services/hilt/AppModule.kt @@ -97,11 +97,11 @@ object AppModule { fun okHttpClient(dns: PersistentDns) = DnsConnectionRetry.configure(OkHttpClient.Builder(), dns).build() - /** Resolve filesDir lazily: Android may create it and lookup always runs off main. */ + /** Resolve noBackupFilesDir lazily: Android may create it and lookup always runs off main. */ @Provides @Singleton fun dns(@ApplicationContext context: Context): PersistentDns = - PersistentDns(file = { File(context.filesDir, "wolfskin-dns.json") }) + PersistentDns(file = { File(context.noBackupFilesDir, "wolfskin-dns.json") }) @AuthOkHttpClient @Provides diff --git a/app/src/test/java/com/github/damontecres/wholphin/services/ServerSavedSessionTest.kt b/app/src/test/java/com/github/damontecres/wholphin/services/ServerSavedSessionTest.kt index 6a8070ef..2724cc61 100644 --- a/app/src/test/java/com/github/damontecres/wholphin/services/ServerSavedSessionTest.kt +++ b/app/src/test/java/com/github/damontecres/wholphin/services/ServerSavedSessionTest.kt @@ -1,5 +1,8 @@ package com.github.damontecres.wholphin.services +import android.app.Application +import androidx.room.Room +import androidx.test.core.app.ApplicationProvider import android.content.Context import com.github.damontecres.wholphin.data.* import com.github.damontecres.wholphin.data.model.* @@ -20,20 +23,30 @@ import org.jellyfin.sdk.api.operations.UserApi import org.jellyfin.sdk.model.api.PublicSystemInfo import org.jellyfin.sdk.model.api.UserDto import org.junit.Assert.* +import org.junit.After import org.junit.Before import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config /** Saved identities can paint offline without unlocking protected logins or accepting a late identity. */ @OptIn(ExperimentalCoroutinesApi::class) +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [34], application = Application::class) class ServerSavedSessionTest { private val dispatcher = StandardTestDispatcher() private val server = JellyfinServer(UUID.randomUUID(), "Saved server", "https://jellyfin.test", "12.1.0") - private val user = JellyfinUser(id = UUID.randomUUID(), serverId = server.id, name = "Saved name", accessToken = "saved-token") + private var user = JellyfinUser(id = UUID.randomUUID(), serverId = server.id, name = "Saved name", accessToken = "saved-token") private val users = mockk() private val system = mockk() private val api = mockk(relaxed = true) private val recent = mockk() - private val dao = mockk(relaxed = true) + private val database by lazy { + Room.inMemoryDatabaseBuilder(ApplicationProvider.getApplicationContext(), AppDatabase::class.java) + .allowMainThreadQueries().build() + } + private val dao by lazy { database.serverDao() } private val dto = UserDto(id = user.id, name = "Refreshed name", hasPassword = false, hasConfiguredPassword = false, hasConfiguredEasyPassword = false) private val info = PublicSystemInfo(serverName = "Refreshed server", version = "12.1.0") @@ -41,14 +54,16 @@ class ServerSavedSessionTest { @Before fun savedLogin() { every { recent.get() } returns MostRecentServer.from(CurrentUser(server, user)) - every { dao.getServer(server.id) } returns JellyfinServerUsers(server, listOf(user)) - every { dao.addOrUpdateUser(any()) } answers { firstArg() } + dao.addOrUpdateServer(server) + user = dao.addOrUpdateUser(user) every { api.userApi } returns users every { api.systemApi } returns system coEvery { users.getCurrentUser() } returns successResponse(dto) coEvery { system.getPublicSystemInfo() } returns successResponse(info) } + @After fun closeDatabase() { database.close() } + @Test fun savedSessionIsUsableWhileUserRevalidationWaitsAndSystemInfoRunsInParallel() = runTest(dispatcher) { val pendingUser = CompletableDeferred() val systemStarted = CompletableDeferred() @@ -69,7 +84,7 @@ class ServerSavedSessionTest { @Test fun savedProtectedPeopleRemainLocked() = runTest(dispatcher) { for (protected in listOf(user.copy(pin = "1234"), user.copy(requireLogin = true))) { - every { dao.getServer(server.id) } returns JellyfinServerUsers(server, listOf(protected)) + dao.updateUser(protected) assertTrue(repository.restoreSavedSession() is RestoredSession.ServerOnly) assertNull(repository.current.value) } @@ -82,9 +97,8 @@ class ServerSavedSessionTest { coEvery { users.getCurrentUser() } coAnswers { entered.complete(Unit); pending.await(); successResponse(dto) } val refresh = async { repository.revalidateSession(first) } entered.await() - val selected = user.copy(id = UUID.randomUUID(), name = "Other person") + val selected = dao.addOrUpdateUser(user.copy(rowId = 0, id = UUID.randomUUID(), name = "Other person")) every { recent.get() } returns MostRecentServer.from(CurrentUser(server, selected)) - every { dao.getServer(server.id) } returns JellyfinServerUsers(server, listOf(selected)) repository.restoreSavedSession() pending.complete(Unit) refresh.await() @@ -92,6 +106,26 @@ class ServerSavedSessionTest { assertNull(repository.currentUserDto) } + @Test fun launchNameRefreshPreservesSettingsAndPinChangedWhileTheRequestWaits() = runTest(dispatcher) { + val first = (repository.restoreSavedSession() as RestoredSession.Success).currentUser + val entered = CompletableDeferred() + val pending = CompletableDeferred() + coEvery { users.getCurrentUser() } coAnswers { entered.complete(Unit); pending.await(); successResponse(dto) } + val refresh = async { repository.revalidateSession(first) } + entered.await() + val edited = user.copy( + pin = "4567", + appPreferences = user.appPreferences.copy(preferredAudioLanguage = "nld"), + config = user.config.copy(homeSettingsSource = HomePageSettingsSource.LOCAL), + ) + dao.updateUser(edited) + pending.complete(Unit) + refresh.await() + val expected = edited.copy(name = dto.name) + assertEquals(expected, dao.getUser(server.id, user.id)) + assertEquals(expected, repository.current.value!!.user) + } + @Test fun anotherUsersResponseCannotRenameTheSavedPerson() = runTest(dispatcher) { val first = (repository.restoreSavedSession() as RestoredSession.Success).currentUser coEvery { users.getCurrentUser() } returns successResponse(dto.copy(id = UUID.randomUUID())) diff --git a/app/src/wolfskin/java/org/justinvisser/wolfskin/tv/NetworkStartup.kt b/app/src/wolfskin/java/org/justinvisser/wolfskin/tv/NetworkStartup.kt index 9e9fc60f..899341e5 100644 --- a/app/src/wolfskin/java/org/justinvisser/wolfskin/tv/NetworkStartup.kt +++ b/app/src/wolfskin/java/org/justinvisser/wolfskin/tv/NetworkStartup.kt @@ -6,7 +6,6 @@ import com.github.damontecres.wholphin.data.MostRecentServer import com.github.damontecres.wholphin.data.MostRecentServerProvider import com.github.damontecres.wholphin.services.hilt.IoCoroutineScope import dagger.hilt.android.qualifiers.ApplicationContext -import java.net.UnknownHostException import javax.inject.Inject import javax.inject.Provider import javax.inject.Singleton @@ -40,7 +39,7 @@ class NetworkStartup @Inject constructor( private fun warm(url: String?) { val host = url?.toHttpUrlOrNull()?.host ?: return - try { dns.lookup(host) } catch (_: UnknownHostException) { + try { dns.lookup(host) } catch (_: Exception) { // An offline launch must still render the saved people. } } diff --git a/conductarr-client/src/main/kotlin/org/justinvisser/wolfskin/conductarr/DnsConnectionRetry.kt b/conductarr-client/src/main/kotlin/org/justinvisser/wolfskin/conductarr/DnsConnectionRetry.kt index 79647856..4248a4c0 100644 --- a/conductarr-client/src/main/kotlin/org/justinvisser/wolfskin/conductarr/DnsConnectionRetry.kt +++ b/conductarr-client/src/main/kotlin/org/justinvisser/wolfskin/conductarr/DnsConnectionRetry.kt @@ -34,7 +34,8 @@ class DnsConnectionRetry(private val dns: PersistentDns) : Interceptor, EventLis } override fun intercept(chain: Interceptor.Chain): Response { - val attempt = attempts.getValue(chain.call()) + // OkHttp deliberately disables event listeners on its derived WebSocket client. + val attempt = attempts[chain.call()] ?: return chain.proceed(chain.request()) return try { chain.proceed(chain.request()) } catch (failure: IOException) { diff --git a/conductarr-client/src/main/kotlin/org/justinvisser/wolfskin/conductarr/PersistentDns.kt b/conductarr-client/src/main/kotlin/org/justinvisser/wolfskin/conductarr/PersistentDns.kt index d6bcfd62..2c65b657 100644 --- a/conductarr-client/src/main/kotlin/org/justinvisser/wolfskin/conductarr/PersistentDns.kt +++ b/conductarr-client/src/main/kotlin/org/justinvisser/wolfskin/conductarr/PersistentDns.kt @@ -71,7 +71,7 @@ class PersistentDns( executor.execute { try { synchronized(host.resolutionLock) { resolve(hostname, host) } - } catch (_: UnknownHostException) { + } catch (_: Exception) { // Failed refreshes do not extend the lifetime of the last successful answer. host.refreshAfter = nowMillis() + FAILURE_BACKOFF_MILLIS } finally { diff --git a/conductarr-client/src/test/kotlin/org/justinvisser/wolfskin/conductarr/DnsConnectionRetryTest.kt b/conductarr-client/src/test/kotlin/org/justinvisser/wolfskin/conductarr/DnsConnectionRetryTest.kt new file mode 100644 index 00000000..58b72d98 --- /dev/null +++ b/conductarr-client/src/test/kotlin/org/justinvisser/wolfskin/conductarr/DnsConnectionRetryTest.kt @@ -0,0 +1,68 @@ +// SPDX-License-Identifier: GPL-2.0-only +package org.justinvisser.wolfskin.conductarr + +import java.net.InetAddress +import java.util.concurrent.CompletableFuture +import java.util.concurrent.Executors +import java.util.concurrent.TimeUnit +import java.util.concurrent.atomic.AtomicReference +import mockwebserver3.MockResponse +import mockwebserver3.MockWebServer +import okhttp3.Dispatcher +import okhttp3.Dns +import okhttp3.OkHttpClient +import okhttp3.Request +import okhttp3.Response +import okhttp3.WebSocket +import okhttp3.WebSocketListener +import org.junit.Assert.* +import org.junit.Rule +import org.junit.Test +import org.junit.rules.TemporaryFolder + +/** OkHttp's listener-free WebSocket client must retain a working upgrade and message path. */ +class DnsConnectionRetryTest { + @get:Rule val directory = TemporaryFolder() + + @Test fun derivedClientOpensWebSocketWithoutAnUncaughtDispatcherException() { + val opened = CompletableFuture() + val message = CompletableFuture() + val uncaught = AtomicReference() + val executor = Executors.newCachedThreadPool { task -> + Thread(task, "dns-websocket-test").apply { + uncaughtExceptionHandler = Thread.UncaughtExceptionHandler { _, failure -> + uncaught.set(failure) + opened.completeExceptionally(failure) + message.completeExceptionally(failure) + } + } + } + MockWebServer().use { server -> + server.enqueue(MockResponse.Builder().webSocketUpgrade(object : WebSocketListener() { + override fun onOpen(webSocket: WebSocket, response: Response) { webSocket.send("ready") } + }).build()) + server.start() + val dns = PersistentDns({ directory.root.resolve("dns.json") }, Dns { listOf(InetAddress.getByName("127.0.0.1")) }) + val client = DnsConnectionRetry.configure(OkHttpClient.Builder(), dns) + .dispatcher(Dispatcher(executor)).build().newBuilder().build() + val socket = client.newWebSocket(Request.Builder().url(server.url("/events")).build(), object : WebSocketListener() { + override fun onOpen(webSocket: WebSocket, response: Response) { opened.complete(response.code) } + override fun onMessage(webSocket: WebSocket, text: String) { message.complete(text) } + override fun onFailure(webSocket: WebSocket, t: Throwable, response: Response?) { + opened.completeExceptionally(t) + message.completeExceptionally(t) + } + }) + try { + assertEquals(101, opened.get(5, TimeUnit.SECONDS).toInt()) + assertEquals("ready", message.get(5, TimeUnit.SECONDS)) + assertNull(uncaught.get()) + println("WEBSOCKET SMOKE: derived cached-DNS client opened HTTP 101, received ready, no uncaught dispatcher exception") + } finally { + socket.cancel() + client.connectionPool.evictAll() + executor.shutdownNow() + } + } + } +} diff --git a/conductarr-client/src/test/kotlin/org/justinvisser/wolfskin/conductarr/PersistentDnsTest.kt b/conductarr-client/src/test/kotlin/org/justinvisser/wolfskin/conductarr/PersistentDnsTest.kt index bc66ffef..b167b354 100644 --- a/conductarr-client/src/test/kotlin/org/justinvisser/wolfskin/conductarr/PersistentDnsTest.kt +++ b/conductarr-client/src/test/kotlin/org/justinvisser/wolfskin/conductarr/PersistentDnsTest.kt @@ -94,6 +94,22 @@ class PersistentDnsTest { assertEquals(1, queued.size) } + @Test fun unexpectedRefreshExceptionKeepsTheLastAnswerAndAllowsAnotherRefresh() { + var failing = false + val dns = cache(Dns { if (failing) throw IllegalStateException("resolver unavailable"); answer }) + dns.lookup("jellyfin.test") + now += 120_000 + failing = true + assertEquals(listOf(old), dns.lookup("jellyfin.test")) + queued.removeFirst().run() + now += 2_001 + failing = false + answer = listOf(fresh) + assertEquals(listOf(old), dns.lookup("jellyfin.test")) + queued.removeFirst().run() + assertEquals(listOf(fresh), dns.lookup("jellyfin.test")) + } + @Test fun failedCachedAddressForcesFreshResolutionAndRetriesBeforeSendingPost() { MockWebServer().use { server -> server.start() diff --git a/docker/README.md b/docker/README.md index fdfbba0b..56c53048 100644 --- a/docker/README.md +++ b/docker/README.md @@ -34,14 +34,16 @@ network validation. The existing Wolfskin picker paints the saved names and identity-derived colors, then refreshes the household in the background. PIN and require-login profiles still go through the existing protected sign-in flow. `Users/Me` and `System/Info/Public` now start concurrently; a late launch response -cannot replace a different selected person. +cannot replace a different selected person. Name revalidation updates only the +name column and reloads the saved user, preserving newer settings and PINs. Jellyfin, Conductarr, artwork, playback and crash-report clients share one `PersistentDns` instance through the application graph. It keeps system-ordered -IPv6/IPv4 answers in memory and `filesDir/wolfskin-dns.json` (at most 32 hosts). +IPv6/IPv4 answers in memory and `noBackupFilesDir/wolfskin-dns.json` (at most 32 hosts). Saved answers return immediately for up to 24 hours and refresh off main after 60 seconds; failed background lookups back off for 2 seconds without extending -the last successful timestamp. Negative answers are not persisted or cached. +the last successful timestamp. Unexpected resolver exceptions are contained at +the background refresh and warm-up boundaries. Negative answers are not persisted or cached. Missing, oversized or corrupt hint files fall back to system resolution. Application creation warms the saved Jellyfin host and configured Conductarr host on parallel IO jobs; it does not change Android Private DNS. @@ -51,6 +53,8 @@ gets one fresh system lookup and one retry. Responses, read failures and uncertain mutation outcomes are not replayed. Conductarr retains its existing no-automatic-replay policy after a request starts. A DNS refresh changes the next new connection; already-working pooled connections are not forcibly closed. +OkHttp's derived WebSocket clients disable event listeners; those calls bypass +the HTTP retry bookkeeping while retaining the shared DNS resolver. Device acceptance (not established by JVM tests): with saved people present, cold-launch six times more than 60 seconds apart and require the picker within