diff --git a/app/src/main/java/com/github/damontecres/wholphin/MainActivity.kt b/app/src/main/java/com/github/damontecres/wholphin/MainActivity.kt index ec61583d..6b3403c6 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/MainActivity.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/MainActivity.kt @@ -32,6 +32,7 @@ import androidx.tv.material3.ExperimentalTvMaterial3Api import androidx.tv.material3.MaterialTheme import androidx.tv.material3.Surface import com.github.damontecres.wholphin.data.ServerRepository +import com.github.damontecres.wholphin.data.RestoredSession import com.github.damontecres.wholphin.preferences.AppPreferences import com.github.damontecres.wholphin.preferences.PlayerBackend import com.github.damontecres.wholphin.services.AppUpgradeHandler @@ -576,12 +577,22 @@ class MainActivityViewModel val restoredSession = if (signInAutomatically) { Timber.v("Sign in automatically") - serverRepository.restoreLastSession() + serverRepository.restoreSavedSession() } else { serverRepository.getMostRecentServer() } Timber.d("Restore last session result=%s", restoredSession) setupNavigationManager.navigateTo(restoredSession.destination) + if (signInAutomatically && restoredSession is RestoredSession.Success) { + viewModelScope.launchDefault { + try { + serverRepository.revalidateSession(restoredSession.currentUser) + } catch (exception: Exception) { + if (exception is kotlinx.coroutines.CancellationException) throw exception + Timber.w(exception, "Saved launch session revalidation failed") + } + } + } } } } diff --git a/app/src/main/java/com/github/damontecres/wholphin/WholphinApplication.kt b/app/src/main/java/com/github/damontecres/wholphin/WholphinApplication.kt index 6ec9d4b4..aba2a0a6 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/WholphinApplication.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/WholphinApplication.kt @@ -17,6 +17,7 @@ import org.acra.ktx.initAcra import org.jellyfin.sdk.Jellyfin import org.jellyfin.sdk.model.ServerVersion import timber.log.Timber +import org.justinvisser.wolfskin.tv.NetworkStartup import javax.inject.Inject @OptIn(ExperimentalComposeRuntimeApi::class) @@ -54,6 +55,7 @@ class WholphinApplication : override fun onCreate() { super.onCreate() + networkStartup.start() if (BuildConfig.DEBUG) { StrictMode.setThreadPolicy( StrictMode.ThreadPolicy @@ -110,6 +112,9 @@ class WholphinApplication : @Inject lateinit var workerFactory: HiltWorkerFactory + @Inject + lateinit var networkStartup: NetworkStartup + override val workManagerConfiguration: Configuration get() = Configuration 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 2c24c8f3..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 @@ -8,6 +8,8 @@ import com.github.damontecres.wholphin.services.hilt.IoDispatcher import com.github.damontecres.wholphin.util.WholphinDispatchers import dagger.hilt.android.qualifiers.ApplicationContext import kotlinx.coroutines.CoroutineDispatcher +import kotlinx.coroutines.async +import kotlinx.coroutines.coroutineScope import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow @@ -101,15 +103,7 @@ class ServerRepository } Timber.v("Changing user to ${user.name} on ${server.url}") apiClient.update(baseUrl = server.url, accessToken = user.accessToken) - val userDto by apiClient.userApi.getCurrentUser() - val updatedServer = - try { - val sysInfo by apiClient.systemApi.getPublicSystemInfo() - server.copy(name = sysInfo.serverName, version = sysInfo.version) - } catch (ex: Exception) { - Timber.w(ex, "Exception fetching public system info") - server - } + val (updatedServer, userDto) = readSession(server) var updatedUser = user.copy( id = userDto.id, @@ -125,6 +119,45 @@ class ServerRepository return@withContext currentUser } + /** Restore only an unprotected saved login; the launch picker must not wait for the network. */ + suspend fun restoreSavedSession(): RestoredSession = withContext(ioDispatcher) { + val recent = mostRecentServerProvider.get() + if (recent !is MostRecentServer.ServerAndUser) return@withContext getMostRecentServerInternal(recent) + val saved = serverDao.getServer(recent.serverId) ?: return@withContext RestoredSession.None + val user = saved.users.firstOrNull { it.id == recent.userId } + if (user == null || user.isProtected) return@withContext RestoredSession.ServerOnly(saved.server) + apiClient.update(baseUrl = saved.server.url, accessToken = user.accessToken) + val current = CurrentUser(saved.server, user) + _currentUserDto.value = null + _current.value = current + RestoredSession.Success(current) + } + + /** A late launch refresh must never replace a person selected while it was in flight. */ + suspend fun revalidateSession(expected: CurrentUser) = withContext(ioDispatcher) { + val (server, userDto) = readSession(expected.server) + if (_current.value !== expected || userDto.id != expected.user.id) return@withContext + serverDao.addOrUpdateServer(server) + 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) + } + + private suspend fun readSession(server: JellyfinServer): Pair = coroutineScope { + val system = async { + try { + val info by apiClient.systemApi.getPublicSystemInfo() + server.copy(name = info.serverName, version = info.version) + } catch (exception: Exception) { + if (exception is kotlinx.coroutines.CancellationException) throw exception + Timber.w(exception, "Exception fetching public system info") + server + } + } + val user by apiClient.userApi.getCurrentUser() + system.await() to user + } + /** * Try to restore the most recent session * 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 3a114017..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 @@ -26,6 +26,9 @@ import org.jellyfin.sdk.api.okhttp.OkHttpFactory import org.jellyfin.sdk.createJellyfin import org.jellyfin.sdk.model.ClientInfo import org.jellyfin.sdk.model.DeviceInfo +import org.justinvisser.wolfskin.conductarr.PersistentDns +import org.justinvisser.wolfskin.conductarr.DnsConnectionRetry +import java.io.File import javax.inject.Qualifier import javax.inject.Singleton @@ -91,12 +94,14 @@ object AppModule { @StandardOkHttpClient @Provides @Singleton - fun okHttpClient() = - OkHttpClient - .Builder() - .apply { - // TODO user agent, timeouts, logging, etc - }.build() + fun okHttpClient(dns: PersistentDns) = + DnsConnectionRetry.configure(OkHttpClient.Builder(), dns).build() + + /** 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.noBackupFilesDir, "wolfskin-dns.json") }) @AuthOkHttpClient @Provides diff --git a/app/src/main/java/com/github/damontecres/wholphin/util/CrashReportSenderFactory.kt b/app/src/main/java/com/github/damontecres/wholphin/util/CrashReportSenderFactory.kt index d83ae269..e0c8005f 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/util/CrashReportSenderFactory.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/util/CrashReportSenderFactory.kt @@ -4,6 +4,11 @@ import android.content.Context import com.github.damontecres.wholphin.data.MostRecentServer import com.github.damontecres.wholphin.services.hilt.AppModule import com.github.damontecres.wholphin.services.hilt.DeviceModule +import com.github.damontecres.wholphin.services.hilt.StandardOkHttpClient +import dagger.hilt.EntryPoint +import dagger.hilt.InstallIn +import dagger.hilt.android.EntryPointAccessors +import dagger.hilt.components.SingletonComponent import com.google.auto.service.AutoService import kotlinx.coroutines.runBlocking import okhttp3.OkHttpClient @@ -73,10 +78,7 @@ class CrashReportSender : ReportSender { private fun MostRecentServer.createApi(context: Context): ApiClient? = (this as? MostRecentServer.ServerAndUser)?.let { - val okHttpClient = - OkHttpClient - .Builder() - .build() + val okHttpClient = EntryPointAccessors.fromApplication(context.applicationContext, CrashNetwork::class.java).client() val okHttpFactory = OkHttpFactory(okHttpClient) val api = createJellyfin { @@ -89,3 +91,11 @@ private fun MostRecentServer.createApi(context: Context): ApiClient? = }.createApi(baseUrl = serverUrl, accessToken = accessToken) api } + +/** Crash reporting shares the application's DNS hints without borrowing authenticated headers. */ +@EntryPoint +@InstallIn(SingletonComponent::class) +interface CrashNetwork { + @StandardOkHttpClient + fun client(): OkHttpClient +} diff --git a/app/src/test/java/com/github/damontecres/wholphin/TestMainActivityViewModel.kt b/app/src/test/java/com/github/damontecres/wholphin/TestMainActivityViewModel.kt index 1de5becd..d8fe5695 100644 --- a/app/src/test/java/com/github/damontecres/wholphin/TestMainActivityViewModel.kt +++ b/app/src/test/java/com/github/damontecres/wholphin/TestMainActivityViewModel.kt @@ -32,6 +32,8 @@ import io.mockk.mockkStatic import io.mockk.slot import io.mockk.unmockkStatic import io.mockk.verify +import kotlinx.coroutines.CompletableDeferred +import kotlinx.coroutines.test.runCurrent import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.flow @@ -116,9 +118,6 @@ class TestMainActivityViewModel { signInAutomatically = true } every { serverRepository.current } returns MutableStateFlow(currentUser) -// coEvery { serverRepository.tryRestoreSession(serverId, userId) } returns currentUser -// coEvery { serverRepository.restoreLastSession() } returns -// RestoredSession.Success(currentUser) viewModel.handleAppStart() @@ -132,7 +131,7 @@ class TestMainActivityViewModel { destination as SetupDestination.AppContent Assert.assertEquals(currentUser, destination.current) } - coVerify(exactly = 0) { serverRepository.restoreLastSession() } + coVerify(exactly = 0) { serverRepository.restoreSavedSession() } coVerify(exactly = 0) { serverRepository.getMostRecentServer() } } @@ -150,7 +149,7 @@ class TestMainActivityViewModel { viewModel.handleAppStart() coVerify(exactly = 0) { serverRepository.tryChangeUser(any(), any()) } - coVerify(exactly = 0) { serverRepository.restoreLastSession() } + coVerify(exactly = 0) { serverRepository.restoreSavedSession() } coVerify(exactly = 1) { serverRepository.getMostRecentServer() } val args = mutableListOf() verify { setupNavigationManager.navigateTo(capture(args)) } @@ -165,6 +164,20 @@ class TestMainActivityViewModel { } } + @Test + fun savedColdLaunchNavigatesBeforeSessionRevalidationCompletes() = runTest(testDispatcher) { + setupPreferences { signInAutomatically = true } + every { serverRepository.current } returns MutableStateFlow(null) + coEvery { serverRepository.restoreSavedSession() } returns RestoredSession.Success(currentUser) + val release = CompletableDeferred() + coEvery { serverRepository.revalidateSession(currentUser) } coAnswers { release.await() } + viewModel.handleAppStart() + runCurrent() + verify { setupNavigationManager.navigateTo(SetupDestination.AppContent(currentUser)) } + assertFalse(release.isCompleted) + release.complete(Unit) + } + @Test fun `Test auto sign in, hot load, with protected profile`() = runTest { @@ -178,7 +191,7 @@ class TestMainActivityViewModel { coEvery { serverRepository.tryChangeUser(serverId, userId) } returns null - coEvery { serverRepository.restoreLastSession() } returns + coEvery { serverRepository.restoreSavedSession() } returns RestoredSession.ServerOnly(server) viewModel.handleAppStart() @@ -204,7 +217,7 @@ class TestMainActivityViewModel { signInAutomatically = true } every { serverRepository.current } returns MutableStateFlow(null) - coEvery { serverRepository.restoreLastSession() } returns + coEvery { serverRepository.restoreSavedSession() } returns RestoredSession.ServerOnly(server) coEvery { serverRepository.tryChangeUser(serverId, userId) @@ -212,7 +225,7 @@ class TestMainActivityViewModel { viewModel.handleAppStart() - coVerify(exactly = 1) { serverRepository.restoreLastSession() } + coVerify(exactly = 1) { serverRepository.restoreSavedSession() } val slot = slot() verify { setupNavigationManager.navigateTo(capture(slot)) } slot.captured.let { destination -> @@ -232,11 +245,11 @@ class TestMainActivityViewModel { signInAutomatically = true } every { serverRepository.current } returns MutableStateFlow(null) - coEvery { serverRepository.restoreLastSession() } returns RestoredSession.None + coEvery { serverRepository.restoreSavedSession() } returns RestoredSession.None viewModel.handleAppStart() - coVerify(exactly = 1) { serverRepository.restoreLastSession() } + coVerify(exactly = 1) { serverRepository.restoreSavedSession() } val slot = slot() verify { setupNavigationManager.navigateTo(capture(slot)) } slot.captured.let { destination -> @@ -254,7 +267,7 @@ class TestMainActivityViewModel { signInAutomatically = true } every { serverRepository.current } returns MutableStateFlow(currentUser) - coEvery { serverRepository.restoreLastSession() } returns + coEvery { serverRepository.restoreSavedSession() } returns RestoredSession.Success(currentUser) coEvery { intentService.parseIntent(any()) } returns IntentResult.NoOp @@ -271,7 +284,7 @@ class TestMainActivityViewModel { // destination as SetupDestination.AppContent // Assert.assertEquals(currentUser, destination.current) // } - coVerify(exactly = 0) { serverRepository.restoreLastSession() } + coVerify(exactly = 0) { serverRepository.restoreSavedSession() } coVerify(exactly = 0) { serverRepository.getMostRecentServer() } } 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 new file mode 100644 index 00000000..2724cc61 --- /dev/null +++ b/app/src/test/java/com/github/damontecres/wholphin/services/ServerSavedSessionTest.kt @@ -0,0 +1,136 @@ +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.* +import com.github.damontecres.wholphin.ui.successResponse +import io.mockk.* +import java.util.UUID +import kotlinx.coroutines.CompletableDeferred +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.async +import kotlinx.coroutines.test.StandardTestDispatcher +import kotlinx.coroutines.test.runCurrent +import kotlinx.coroutines.test.runTest +import org.jellyfin.sdk.api.client.ApiClient +import org.jellyfin.sdk.api.client.extensions.systemApi +import org.jellyfin.sdk.api.client.extensions.userApi +import org.jellyfin.sdk.api.operations.SystemApi +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 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 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") + private val repository by lazy { ServerRepository(mockk(), mockk(), dao, api, dispatcher, recent) } + + @Before fun savedLogin() { + every { recent.get() } returns MostRecentServer.from(CurrentUser(server, user)) + 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() + coEvery { users.getCurrentUser() } coAnswers { pendingUser.await(); successResponse(dto) } + coEvery { system.getPublicSystemInfo() } coAnswers { systemStarted.complete(Unit); successResponse(info) } + val restored = repository.restoreSavedSession() as RestoredSession.Success + assertEquals(user, restored.currentUser.user) + assertEquals(restored.currentUser, repository.current.value) + val refresh = async { repository.revalidateSession(restored.currentUser) } + runCurrent() + systemStarted.await() + assertFalse(refresh.isCompleted) + pendingUser.complete(Unit) + refresh.await() + assertEquals("Refreshed name", repository.current.value!!.user.name) + assertEquals("Refreshed server", repository.current.value!!.server.name) + } + + @Test fun savedProtectedPeopleRemainLocked() = runTest(dispatcher) { + for (protected in listOf(user.copy(pin = "1234"), user.copy(requireLogin = true))) { + dao.updateUser(protected) + assertTrue(repository.restoreSavedSession() is RestoredSession.ServerOnly) + assertNull(repository.current.value) + } + } + + @Test fun lateLaunchRefreshCannotReplaceTheSelectedPerson() = 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 selected = dao.addOrUpdateUser(user.copy(rowId = 0, id = UUID.randomUUID(), name = "Other person")) + every { recent.get() } returns MostRecentServer.from(CurrentUser(server, selected)) + repository.restoreSavedSession() + pending.complete(Unit) + refresh.await() + assertEquals(selected, repository.current.value!!.user) + 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())) + repository.revalidateSession(first) + assertEquals(user, repository.current.value!!.user) + assertNull(repository.currentUserDto) + } +} diff --git a/app/src/test/java/com/github/damontecres/wholphin/ui/TestModule.kt b/app/src/test/java/com/github/damontecres/wholphin/ui/TestModule.kt index b266bb96..3b4d3efa 100644 --- a/app/src/test/java/com/github/damontecres/wholphin/ui/TestModule.kt +++ b/app/src/test/java/com/github/damontecres/wholphin/ui/TestModule.kt @@ -62,6 +62,7 @@ import org.jellyfin.sdk.api.okhttp.OkHttpFactory import org.jellyfin.sdk.model.ClientInfo import org.jellyfin.sdk.model.DeviceInfo import timber.log.Timber +import org.justinvisser.wolfskin.conductarr.PersistentDns import javax.inject.Singleton @Module @@ -91,12 +92,11 @@ object TestModule { @StandardOkHttpClient @Provides @Singleton - fun okHttpClient() = - OkHttpClient - .Builder() - .apply { - // TODO user agent, timeouts, logging, etc - }.build() + fun okHttpClient(dns: PersistentDns) = AppModule.okHttpClient(dns) + + @Provides + @Singleton + fun dns(@ApplicationContext context: Context): PersistentDns = AppModule.dns(context) @AuthOkHttpClient @Provides diff --git a/app/src/test/java/org/justinvisser/wolfskin/tv/SavedPeoplePickerTest.kt b/app/src/test/java/org/justinvisser/wolfskin/tv/SavedPeoplePickerTest.kt new file mode 100644 index 00000000..1fb29d31 --- /dev/null +++ b/app/src/test/java/org/justinvisser/wolfskin/tv/SavedPeoplePickerTest.kt @@ -0,0 +1,92 @@ +// SPDX-License-Identifier: GPL-2.0-only +package org.justinvisser.wolfskin.tv + +import android.app.Application +import androidx.activity.ComponentActivity +import androidx.compose.ui.test.assertIsDisplayed +import androidx.compose.ui.test.junit4.createAndroidComposeRule +import androidx.compose.ui.test.onNodeWithText +import androidx.compose.ui.test.onRoot +import com.github.takahirom.roborazzi.captureRoboImage +import java.io.File +import androidx.lifecycle.viewModelScope +import com.github.damontecres.wholphin.data.ServerRepository +import com.github.damontecres.wholphin.data.model.JellyfinServer +import com.github.damontecres.wholphin.data.model.JellyfinServerUsers +import com.github.damontecres.wholphin.data.model.JellyfinUser +import com.github.damontecres.wholphin.preferences.AppPreferences +import com.github.damontecres.wholphin.preferences.UserPreferences +import com.github.damontecres.wholphin.services.NavigationManager +import io.mockk.coEvery +import io.mockk.mockk +import java.util.UUID +import java.util.concurrent.CountDownLatch +import java.util.concurrent.atomic.AtomicBoolean +import kotlinx.coroutines.cancel +import org.junit.Assert.* +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.justinvisser.wolfskin.conductarr.MemoryCacheStore +import org.justinvisser.wolfskin.design.PeopleStatus +import org.justinvisser.wolfskin.tv.read.LibraryReadFacade +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config +import org.robolectric.annotation.GraphicsMode + +/** The actual picker displays every saved login while household revalidation has no response. */ +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [34], application = Application::class, qualifiers = "w960dp-h540dp-xhdpi") +@GraphicsMode(GraphicsMode.Mode.NATIVE) +class SavedPeoplePickerTest { + @get:Rule val rule = createAndroidComposeRule() + + @Test fun savedPeopleAreVisibleBeforeTheFirstNetworkResponse() { + ConductarrPreference.initialize(rule.activity) + val previousUrl = ConductarrPreference.baseUrl + val previousFixtures = ConductarrPreference.fixtures + val release = CountDownLatch(1) + val entered = AtomicBoolean(false) + FakeConductarr().use { fake -> + fake.beforeResponse = { entered.set(true); release.await() } + ConductarrPreference.updateBaseUrl(fake.url) + ConductarrPreference.updateFixtures(false) + ConductarrSession(MemoryCacheStore()).use { conductarr -> + val server = JellyfinServer(UUID.randomUUID(), "Saved server", fake.url, null) + val users = listOf("Justin" to JUSTIN_TOKEN, "Mindel" to MINDEL_TOKEN, "Shared" to SHARED_TOKEN).map { (name, token) -> + JellyfinUser(id = UUID.randomUUID(), serverId = server.id, name = name, accessToken = token) + } + val repository = mockk(relaxed = true) + coEvery { repository.serverDao.getServer(server.id) } returns JellyfinServerUsers(server, users) + val libraries = mockk(relaxed = true) + coEvery { libraries.readDestinations(any()) } returns emptyMap() + val session = WolfskinSession() + val navigation = NavigationManager() + val model = WolfskinShellViewModel(rule.activity, session, repository, navigation, mockk(relaxed = true), + mockk(relaxed = true), libraries, WolfskinSeeAllLists(), conductarr, mockk(relaxed = true)) + try { + rule.setContent { + WolfskinApp(server, users.first(), navigation, UserPreferences(AppPreferences.getDefaultInstance(), null), + viewModel = model, homeContent = { _, _, _, _ -> }) + } + rule.waitUntil(5_000) { entered.get() && session.state?.people?.size == 3 } + users.forEach { rule.onNodeWithText(it.name!!).assertIsDisplayed() } + rule.runOnIdle { + assertTrue(session.state!!.pickerVisible) + assertEquals(PeopleStatus.Ready, session.state!!.peopleStatus) + assertEquals(users.map { it.id.toString() }, session.state!!.people.map { it.jellyfinUserId }) + assertEquals(listOf("justin", "mindel", "shared"), session.state!!.people.map { it.key }) + assertEquals(1L, release.count) + } + val captures = File(requireNotNull(System.getProperty("wolfskin.captureDirectory"))).apply { mkdirs() } + rule.onRoot().captureRoboImage(File(captures, "saved-people-before-network.png")) + } finally { + release.countDown() + model.viewModelScope.cancel() + ConductarrPreference.updateBaseUrl(previousUrl) + ConductarrPreference.updateFixtures(previousFixtures) + } + } + } + } +} diff --git a/app/src/wolfskin/java/org/justinvisser/wolfskin/tv/ConductarrPreference.kt b/app/src/wolfskin/java/org/justinvisser/wolfskin/tv/ConductarrPreference.kt index 559b4365..b794391b 100644 --- a/app/src/wolfskin/java/org/justinvisser/wolfskin/tv/ConductarrPreference.kt +++ b/app/src/wolfskin/java/org/justinvisser/wolfskin/tv/ConductarrPreference.kt @@ -21,6 +21,7 @@ import kotlinx.coroutines.CoroutineScope import org.justinvisser.wolfskin.conductarr.CacheEntry import org.justinvisser.wolfskin.conductarr.CacheStore import org.justinvisser.wolfskin.conductarr.FileCacheStore +import org.justinvisser.wolfskin.conductarr.PersistentDns /** Advanced-only endpoint and fixture controls contain no credentials. */ object ConductarrPreference { @@ -54,8 +55,8 @@ object ConductarrPreference { object ConductarrModule { @Provides @Singleton - fun session(@ApplicationContext context: Context, @IoCoroutineScope scope: CoroutineScope): ConductarrSession = - ConductarrSession(ConductarrDiskCache(context), scope) + fun session(@ApplicationContext context: Context, @IoCoroutineScope scope: CoroutineScope, dns: PersistentDns): ConductarrSession = + ConductarrSession(ConductarrDiskCache(context), scope, dns) } /** Android may create noBackupFilesDir, so resolve it only during an IO cache operation. */ diff --git a/app/src/wolfskin/java/org/justinvisser/wolfskin/tv/ConductarrSession.kt b/app/src/wolfskin/java/org/justinvisser/wolfskin/tv/ConductarrSession.kt index 5ba52caa..ce639277 100644 --- a/app/src/wolfskin/java/org/justinvisser/wolfskin/tv/ConductarrSession.kt +++ b/app/src/wolfskin/java/org/justinvisser/wolfskin/tv/ConductarrSession.kt @@ -29,6 +29,7 @@ data class ConductarrIdentity( class ConductarrSession( private val cache: CacheStore, private val disposalScope: CoroutineScope = CoroutineScope(SupervisorJob() + Dispatchers.IO), + private val dns: PersistentDns? = null, ) : Closeable { private val mutableIdentity = MutableStateFlow(null) val identity: StateFlow = mutableIdentity.asStateFlow() @@ -50,7 +51,7 @@ class ConductarrSession( } try { val client = if (token.isNullOrBlank()) null else try { - ConductarrClient(baseUrl, token, PersonCacheStore(cache, person), timeouts = ClientTimeouts(3, 5, 8), onEvent = { event -> + ConductarrClient(baseUrl, token, PersonCacheStore(cache, person), timeouts = ClientTimeouts(3, 5, 8), dns = dns, onEvent = { event -> if (event == ClientEvent.NEEDS_SIGN_IN) { mutableIdentity.update { if (generation == currentGeneration) it?.copy(operator = null, signedIn = false, error = ClientError.NeedsSignIn) else it diff --git a/app/src/wolfskin/java/org/justinvisser/wolfskin/tv/NetworkStartup.kt b/app/src/wolfskin/java/org/justinvisser/wolfskin/tv/NetworkStartup.kt new file mode 100644 index 00000000..899341e5 --- /dev/null +++ b/app/src/wolfskin/java/org/justinvisser/wolfskin/tv/NetworkStartup.kt @@ -0,0 +1,46 @@ +// SPDX-License-Identifier: GPL-2.0-only +package org.justinvisser.wolfskin.tv + +import android.content.Context +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 javax.inject.Inject +import javax.inject.Provider +import javax.inject.Singleton +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.launch +import okhttp3.HttpUrl.Companion.toHttpUrlOrNull +import org.justinvisser.wolfskin.conductarr.PersistentDns + +/** Process warm-up never blocks application creation or serializes the two service lookups. */ +@Singleton +class NetworkStartup @Inject constructor( + @param:ApplicationContext private val context: Context, + private val recent: Provider, + private val dns: PersistentDns, + @param:IoCoroutineScope private val scope: CoroutineScope, +) { + fun start() { + scope.launch { + val url = when (val server = recent.get().get()) { + is MostRecentServer.Server -> server.serverUrl + is MostRecentServer.ServerAndUser -> server.serverUrl + MostRecentServer.None -> null + } + warm(url) + } + scope.launch { + ConductarrPreference.initialize(context) + warm(ConductarrPreference.baseUrl) + } + } + + private fun warm(url: String?) { + val host = url?.toHttpUrlOrNull()?.host ?: return + 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/ConductarrClient.kt b/conductarr-client/src/main/kotlin/org/justinvisser/wolfskin/conductarr/ConductarrClient.kt index 5559f15a..8030fcb9 100644 --- a/conductarr-client/src/main/kotlin/org/justinvisser/wolfskin/conductarr/ConductarrClient.kt +++ b/conductarr-client/src/main/kotlin/org/justinvisser/wolfskin/conductarr/ConductarrClient.kt @@ -67,6 +67,7 @@ class ConductarrClient( timeouts: ClientTimeouts = ClientTimeouts(), private val nowMillis: () -> Long = System::currentTimeMillis, private val onEvent: (ClientEvent) -> Unit = {}, + dns: PersistentDns? = null, ) : Closeable { private val base = baseUrl.toHttpUrl() private val scope: String @@ -74,6 +75,7 @@ class ConductarrClient( private val cacheIdentityLock = Any() private val catalogueReads = Semaphore(2) private val http = OkHttpClient.Builder() + .apply { if (dns != null) DnsConnectionRetry.configure(this, dns) } .connectTimeout(timeouts.connectSeconds, TimeUnit.SECONDS) .readTimeout(timeouts.readSeconds, TimeUnit.SECONDS) .callTimeout(timeouts.callSeconds, TimeUnit.SECONDS) 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 new file mode 100644 index 00000000..4248a4c0 --- /dev/null +++ b/conductarr-client/src/main/kotlin/org/justinvisser/wolfskin/conductarr/DnsConnectionRetry.kt @@ -0,0 +1,60 @@ +// SPDX-License-Identifier: GPL-2.0-only +package org.justinvisser.wolfskin.conductarr + +import java.io.IOException +import java.net.InetSocketAddress +import java.net.Proxy +import java.util.concurrent.ConcurrentHashMap +import okhttp3.Call +import okhttp3.EventListener +import okhttp3.Interceptor +import okhttp3.OkHttpClient +import okhttp3.Protocol +import okhttp3.Response + +/** Retry once with fresh DNS only if connection establishment failed before any HTTP headers were sent. */ +class DnsConnectionRetry(private val dns: PersistentDns) : Interceptor, EventListener.Factory { + private class Attempt { + @Volatile var connectionFailed = false + @Volatile var headersSent = false + } + private val attempts = ConcurrentHashMap() + + override fun create(call: Call): EventListener { + val attempt = Attempt() + attempts[call] = attempt + return object : EventListener() { + override fun connectFailed(call: Call, inetSocketAddress: InetSocketAddress, proxy: Proxy, protocol: Protocol?, ioe: IOException) { + attempt.connectionFailed = true + } + override fun requestHeadersStart(call: Call) { attempt.headersSent = true } + override fun callEnd(call: Call) { attempts.remove(call) } + override fun callFailed(call: Call, ioe: IOException) { attempts.remove(call) } + } + } + + override fun intercept(chain: Interceptor.Chain): Response { + // 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) { + if (!attempt.connectionFailed || attempt.headersSent || chain.call().isCanceled()) throw failure + try { + dns.lookupFresh(chain.request().url.host) + } catch (lookupFailure: IOException) { + lookupFailure.addSuppressed(failure) + throw lookupFailure + } + chain.proceed(chain.request()) + } + } + + companion object { + /** Install the same resolver and its pre-request recovery on a separately owned client. */ + fun configure(builder: OkHttpClient.Builder, dns: PersistentDns): OkHttpClient.Builder { + val retry = DnsConnectionRetry(dns) + return builder.dns(dns).eventListenerFactory(retry).addInterceptor(retry) + } + } +} 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 new file mode 100644 index 00000000..2c65b657 --- /dev/null +++ b/conductarr-client/src/main/kotlin/org/justinvisser/wolfskin/conductarr/PersistentDns.kt @@ -0,0 +1,142 @@ +// SPDX-License-Identifier: GPL-2.0-only +package org.justinvisser.wolfskin.conductarr + +import java.io.File +import java.io.IOException +import java.net.InetAddress +import java.net.UnknownHostException +import java.util.concurrent.ConcurrentHashMap +import java.util.concurrent.Executor +import java.util.concurrent.Executors +import kotlinx.serialization.Serializable +import kotlinx.serialization.Transient +import kotlinx.serialization.encodeToString +import okhttp3.Dns + +/** Last-known system answers stay usable for one day while a single background lookup refreshes them. */ +class PersistentDns( + private val file: () -> File, + private val resolver: Dns = Dns.SYSTEM, + private val executor: Executor = Executors.newFixedThreadPool(2) { task -> Thread(task, "wolfskin-dns").apply { isDaemon = true } }, + private val nowMillis: () -> Long = System::currentTimeMillis, +) : Dns { + private class Host { + @Volatile var answer: Answer? = null + @Volatile var refreshAfter = 0L + var refreshing = false + val resolutionLock = Any() + } + + @Serializable + private data class Answer( + val addresses: List, + val savedAtMillis: Long, + @Transient val resolvedAddresses: List = decodeAddresses(addresses), + ) + private val hosts = ConcurrentHashMap() + private val diskLock = Any() + @Volatile private var loaded = false + + override fun lookup(hostname: String): List { + load() + val host = hosts.computeIfAbsent(hostname) { Host() } + val saved = host.answer + if (saved != null && nowMillis() - saved.savedAtMillis in 0..MAX_STALENESS_MILLIS) { + refresh(hostname, host) + return saved.resolvedAddresses + } + return synchronized(host.resolutionLock) { + val current = host.answer + if (current != null && current !== saved && nowMillis() - current.savedAtMillis in 0..MAX_STALENESS_MILLIS) { + current.resolvedAddresses + } else resolve(hostname, host) + } + } + + /** A failed connection bypasses the cached answer and any completed background refresh. */ + fun lookupFresh(hostname: String): List { + load() + val host = hosts.computeIfAbsent(hostname) { Host() } + return synchronized(host.resolutionLock) { + host.answer = null + resolve(hostname, host) + } + } + + private fun refresh(hostname: String, host: Host) { + synchronized(host) { + if (host.refreshing || nowMillis() < host.refreshAfter) return + host.refreshing = true + } + executor.execute { + try { + synchronized(host.resolutionLock) { resolve(hostname, host) } + } catch (_: Exception) { + // Failed refreshes do not extend the lifetime of the last successful answer. + host.refreshAfter = nowMillis() + FAILURE_BACKOFF_MILLIS + } finally { + synchronized(host) { host.refreshing = false } + } + } + } + + private fun resolve(hostname: String, host: Host): List { + val addresses = resolver.lookup(hostname) + if (addresses.isEmpty()) throw UnknownHostException(hostname) + host.answer = Answer(addresses.map { it.hostAddress!! }, nowMillis(), addresses) + host.refreshAfter = nowMillis() + REFRESH_INTERVAL_MILLIS + persist() + return addresses + } + + private fun load() { + if (loaded) return + synchronized(diskLock) { + if (loaded) return + try { + val saved = file() + if (saved.isFile && saved.length() <= MAX_FILE_BYTES) { + val entries = WireJson.decodeFromString>(saved.readText()) + require(entries.size <= MAX_HOSTS) + entries.forEach { (name, answer) -> hosts[name] = Host().apply { this.answer = answer } } + } + } catch (_: IOException) { + // A missing or damaged hint file cannot prevent normal system resolution. + } catch (_: IllegalArgumentException) { + // Serialization and literal-validation errors invalidate the entire hint file. + } + loaded = true + } + } + + private fun persist() = synchronized(diskLock) { + try { + val entries = hosts.entries.mapNotNull { (name, host) -> host.answer?.let { name to it } } + .sortedByDescending { it.second.savedAtMillis }.take(MAX_HOSTS).toMap() + val target = file() + target.parentFile?.mkdirs() + val temporary = File(target.path + ".tmp") + temporary.writeText(WireJson.encodeToString(entries)) + if (!temporary.renameTo(target)) temporary.delete() + } catch (_: IOException) { + // The in-memory answer remains usable when private storage is unavailable. + } + } + + companion object { + const val MAX_STALENESS_MILLIS = 24 * 60 * 60 * 1000L + private const val REFRESH_INTERVAL_MILLIS = 60_000L + private const val FAILURE_BACKOFF_MILLIS = 2_000L + private const val MAX_HOSTS = 32 + private const val MAX_ADDRESSES = 32 + private const val MAX_FILE_BYTES = 64 * 1024L + private fun decodeAddresses(addresses: List): List { + require(addresses.isNotEmpty() && addresses.size <= MAX_ADDRESSES) + return addresses.map { address -> + // Numeric literals only: decoding the hint file must never perform DNS. + require(address.contains(':') || address.matches(Regex("[0-9]+\\.[0-9]+\\.[0-9]+\\.[0-9]+"))) + InetAddress.getByName(address) + } + } + } +} diff --git a/conductarr-client/src/test/kotlin/org/justinvisser/wolfskin/conductarr/ClientSmoke.kt b/conductarr-client/src/test/kotlin/org/justinvisser/wolfskin/conductarr/ClientSmoke.kt index 98173fce..8c8495fb 100644 --- a/conductarr-client/src/test/kotlin/org/justinvisser/wolfskin/conductarr/ClientSmoke.kt +++ b/conductarr-client/src/test/kotlin/org/justinvisser/wolfskin/conductarr/ClientSmoke.kt @@ -2,6 +2,10 @@ package org.justinvisser.wolfskin.conductarr import java.nio.file.Files +import java.io.File +import java.net.InetAddress +import java.util.concurrent.Executor +import okhttp3.Dns import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.toList import kotlinx.coroutines.runBlocking @@ -14,13 +18,24 @@ fun main() = runBlocking { try { MockWebServer().use { server -> server.start() - val baseUrl = server.url("/").toString() + val baseUrl = server.url("/").newBuilder().host("localhost").build().toString() + var resolutions = 0 + val dead = InetAddress.getByName("127.0.0.2") + val live = InetAddress.getByName("127.0.0.1") + val dns = PersistentDns({ File(directory, "dns.json") }, Dns { + resolutions++ + if (resolutions == 1) listOf(dead) else listOf(live) + }, Executor { it.run() }) + check(dns.lookup("localhost") == listOf(dead)) server.enqueue(MockResponse.Builder().body(PROFILES_JSON).addHeader("ETag", "\"smoke\"").build()) - ConductarrClient(baseUrl, TEST_TOKEN, FileCacheStore(directory)).use { client -> + ConductarrClient(baseUrl, TEST_TOKEN, FileCacheStore(directory), dns = dns).use { client -> val live = client.read(TvRoutes.profiles()).toList().single() as ReadResult.Value check(live.saved.value!!.profiles!!.single().name == "Justin") + check(resolutions == 2) + println("DNS SMOKE: cached 127.0.0.2 refused -> fresh 127.0.0.1 -> profile Justin; requests=${server.requestCount}, resolutions=$resolutions") } - ConductarrClient(baseUrl, TEST_TOKEN, FileCacheStore(directory)).use { client -> + val reopened = PersistentDns({ File(directory, "dns.json") }, Dns { listOf(live) }, Executor { it.run() }) + ConductarrClient(baseUrl, TEST_TOKEN, FileCacheStore(directory), dns = reopened).use { client -> val saved = client.read(TvRoutes.profiles(), ReadTrigger.CACHE_ONLY).first() as ReadResult.Value check(saved.saved.fromDisk) server.enqueue(MockResponse.Builder().code(304).build()) 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 new file mode 100644 index 00000000..b167b354 --- /dev/null +++ b/conductarr-client/src/test/kotlin/org/justinvisser/wolfskin/conductarr/PersistentDnsTest.kt @@ -0,0 +1,160 @@ +// SPDX-License-Identifier: GPL-2.0-only +package org.justinvisser.wolfskin.conductarr + +import java.io.File +import java.net.InetAddress +import java.net.Proxy +import java.net.UnknownHostException +import java.util.concurrent.Executor +import java.util.concurrent.TimeUnit +import mockwebserver3.MockResponse +import mockwebserver3.MockWebServer +import okhttp3.ConnectionPool +import okhttp3.Dns +import okhttp3.OkHttpClient +import okhttp3.Request +import okhttp3.RequestBody.Companion.toRequestBody +import org.junit.Assert.* +import org.junit.Rule +import org.junit.Test +import org.junit.rules.TemporaryFolder + +/** Fake time and queued refreshes prove stale answers, disk hints and dead-address recovery. */ +class PersistentDnsTest { + @get:Rule val directory = TemporaryFolder() + private var now = 1_000_000L + private val queued = ArrayDeque() + private val background = Executor { queued.add(it) } + private val old = InetAddress.getByName("127.0.0.2") + private val fresh = InetAddress.getByName("127.0.0.1") + private var answer = listOf(old) + private var lookups = 0 + private val resolver = Dns { lookups++; answer } + private val file get() = File(directory.root, "dns.json") + private fun cache(resolve: Dns = resolver) = PersistentDns({ file }, resolve, background, { now }) + + @Test fun staleAnswerReturnsBeforeBackgroundRefreshAndNextLookupUsesChangedIp() { + val dns = cache() + assertEquals(listOf(old), dns.lookup("jellyfin.test")) + now += 120_000 + answer = listOf(fresh) + assertEquals(listOf(old), dns.lookup("jellyfin.test")) + assertEquals(1, lookups) + assertEquals(listOf(old), dns.lookup("jellyfin.test")) + assertEquals(1, queued.size) + queued.removeFirst().run() + assertEquals(listOf(fresh), dns.lookup("jellyfin.test")) + assertEquals(2, lookups) + } + + @Test fun answersOlderThanOneDayCannotBeServedEvenWhenRefreshFails() { + val dns = cache() + dns.lookup("jellyfin.test") + now += PersistentDns.MAX_STALENESS_MILLIS + 1 + val expired = cache(Dns { throw UnknownHostException() }) + assertThrows(UnknownHostException::class.java) { expired.lookup("jellyfin.test") } + } + + @Test fun persistedAnswersRetainTimestampAndSystemAddressOrdering() { + answer = listOf(InetAddress.getByName("::1"), fresh) + cache().lookup("jellyfin.test") + now += 120_000 + val reopened = cache(Dns { error("Disk answer must return before resolving") }) + assertEquals(answer, reopened.lookup("jellyfin.test")) + assertEquals(1, queued.size) + now += PersistentDns.MAX_STALENESS_MILLIS + assertThrows(UnknownHostException::class.java) { + cache(Dns { throw UnknownHostException() }).lookup("jellyfin.test") + } + } + + @Test fun corruptOrNonnumericDiskAnswersAreIgnored() { + for (contents in listOf("not json", "{\"jellyfin.test\":{\"addresses\":[\"not-an-ip.test\"],\"savedAtMillis\":1000000}}")) { + file.writeText(contents) + answer = listOf(fresh) + assertEquals(listOf(fresh), cache().lookup("jellyfin.test")) + } + assertEquals(2, lookups) + } + + @Test fun negativeAnswersDoNotDelayRecoveryAndFailedRefreshDoesNotExtendStaleness() { + var offline = true + val dns = cache(Dns { if (offline) throw UnknownHostException(); listOf(fresh) }) + assertThrows(UnknownHostException::class.java) { dns.lookup("jellyfin.test") } + offline = false + assertEquals(listOf(fresh), dns.lookup("jellyfin.test")) + offline = true + now += 120_000 + assertEquals(listOf(fresh), dns.lookup("jellyfin.test")) + queued.removeFirst().run() + assertEquals(listOf(fresh), dns.lookup("jellyfin.test")) + assertTrue(queued.isEmpty()) + now += 2_001 + dns.lookup("jellyfin.test") + 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() + val dns = cache() + dns.lookup("jellyfin.test") + answer = listOf(fresh) + server.enqueue(MockResponse.Builder().body("new address").build()) + val client = DnsConnectionRetry.configure(OkHttpClient.Builder(), dns) + .proxy(Proxy.NO_PROXY).retryOnConnectionFailure(false).connectTimeout(1, TimeUnit.SECONDS) + .connectionPool(ConnectionPool(0, 1, TimeUnit.MILLISECONDS)).build() + try { + val url = server.url("/").newBuilder().host("jellyfin.test").build() + client.newCall(Request.Builder().url(url).post("body".toRequestBody()).build()).execute().use { + assertEquals("new address", it.body.string()) + } + assertEquals(2, lookups) + assertEquals(1, server.requestCount) + assertEquals(listOf(fresh), dns.lookup("jellyfin.test")) + } finally { + client.connectionPool.evictAll() + client.dispatcher.executorService.shutdown() + } + } + } + + @Test fun backgroundIpChangeIsUsedByTheNextHttpCall() { + MockWebServer().use { server -> + server.start() + val dns = cache() + dns.lookup("jellyfin.test") + now += 120_000 + answer = listOf(fresh) + assertEquals(listOf(old), dns.lookup("jellyfin.test")) + queued.removeFirst().run() + server.enqueue(MockResponse.Builder().body("refreshed").build()) + val client = DnsConnectionRetry.configure(OkHttpClient.Builder(), dns).proxy(Proxy.NO_PROXY).build() + try { + val url = server.url("/").newBuilder().host("jellyfin.test").build() + client.newCall(Request.Builder().url(url).build()).execute().use { assertEquals("refreshed", it.body.string()) } + assertEquals(2, lookups) + assertEquals(1, server.requestCount) + } finally { + client.connectionPool.evictAll() + client.dispatcher.executorService.shutdown() + } + } + } +} diff --git a/docker/README.md b/docker/README.md index fa36fcc0..56c53048 100644 --- a/docker/README.md +++ b/docker/README.md @@ -27,6 +27,48 @@ Upstream enables JVM unit tests only for debug variants; there is no `testDefaultReleaseUnitTest` task. `make test` uses the closest default-flavor task without changing upstream's test configuration. +## Saved-person launch and DNS + +Launch restores an unprotected saved Jellyfin login from local storage before +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. 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 `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. 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. + +A connection-establishment failure before any HTTP request headers were sent +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 +0.5 seconds on every launch. Press Left/Right then one OK; the selected Home must +open without a second picker. Open the avatar picker, change person with one OK, +and repeat after an offline launch to confirm saved people still paint. + +Compare timestamps of the launch pair of SDK log lines +`GET https://jelly.justinvisser.org/Users/Me` and +`GET https://jelly.justinvisser.org/System/Info/Public` (ignore later foreground +`Users/Me` refreshes). The spike's two slow launch gaps were 1.508 and 1.509 +seconds. The requests now overlap, so either may log first; the picker must not +wait for either response. The minified SDK log tag is not stable across builds. + ## Optimized side-by-side comparison The `optimized` build type inherits `release`: R8 optimization/minification,