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/FirstSignInNavigationTest.kt b/app/src/test/java/org/justinvisser/wolfskin/tv/FirstSignInNavigationTest.kt index aa1faf95..07cd1e37 100644 --- a/app/src/test/java/org/justinvisser/wolfskin/tv/FirstSignInNavigationTest.kt +++ b/app/src/test/java/org/justinvisser/wolfskin/tv/FirstSignInNavigationTest.kt @@ -84,7 +84,7 @@ class FirstSignInNavigationTest { coEvery { views.getUserViews(userId = user.id) } returns successQueryResult(listOf(movies, shows)) for ((library, kind, count) in listOf(Triple(movies, BaseItemKind.MOVIE, 6), Triple(shows, BaseItemKind.SERIES, 29))) { coEvery { items.getItems(userId = user.id, parentId = library.id, recursive = true, - includeItemTypes = listOf(kind), fields = listOf(ItemFields.PROVIDER_IDS, ItemFields.SORT_NAME, ItemFields.DATE_CREATED), + includeItemTypes = listOf(kind), fields = listOf(ItemFields.PROVIDER_IDS, ItemFields.DATE_CREATED), sortBy = listOf(ItemSortBy.SORT_NAME), startIndex = 0, limit = 200) } returns successQueryResult( List(count) { BaseItemDto(id = UUID.randomUUID(), name = "${library.name} title $it", type = kind) }) } diff --git a/app/src/test/java/org/justinvisser/wolfskin/tv/LibraryReadFacadeTest.kt b/app/src/test/java/org/justinvisser/wolfskin/tv/LibraryReadFacadeTest.kt index 92afbe07..a2fb8b0d 100644 --- a/app/src/test/java/org/justinvisser/wolfskin/tv/LibraryReadFacadeTest.kt +++ b/app/src/test/java/org/justinvisser/wolfskin/tv/LibraryReadFacadeTest.kt @@ -51,7 +51,7 @@ class LibraryReadFacadeTest { val offsets = mutableListOf() coEvery { items.getItems(userId = user, parentId = library.id, recursive = true, includeItemTypes = listOf(BaseItemKind.MOVIE), - fields = listOf(ItemFields.PROVIDER_IDS, ItemFields.SORT_NAME, ItemFields.DATE_CREATED), + fields = listOf(ItemFields.PROVIDER_IDS, ItemFields.DATE_CREATED), sortBy = listOf(ItemSortBy.SORT_NAME), startIndex = capture(offsets), limit = 200) } coAnswers { if (offsets.last() == 0) successQueryResult(firstChunk, 202) else { assertEquals(200, offsets.last()) diff --git a/app/src/test/java/org/justinvisser/wolfskin/tv/PersonPickerSessionTest.kt b/app/src/test/java/org/justinvisser/wolfskin/tv/PersonPickerSessionTest.kt index 8e13dca4..95fcbb52 100644 --- a/app/src/test/java/org/justinvisser/wolfskin/tv/PersonPickerSessionTest.kt +++ b/app/src/test/java/org/justinvisser/wolfskin/tv/PersonPickerSessionTest.kt @@ -128,6 +128,8 @@ class PersonPickerSessionTest { private fun awaitLoaded(user: JellyfinUser) { try { rule.waitUntil(10_000) { + // Setup navigation writes Compose state asynchronously; publish it before pumping replacement composition. + androidx.compose.runtime.snapshots.Snapshot.sendApplyNotifications() Shadows.shadowOf(Looper.getMainLooper()).idle() models.lastOrNull()?.signedInPerson == user.name!!.lowercase() && models.last().conductarr.identity.value?.signedIn == true && 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/test/java/org/justinvisser/wolfskin/tv/TitleReadFacadeTest.kt b/app/src/test/java/org/justinvisser/wolfskin/tv/TitleReadFacadeTest.kt index fb7bf06e..7ea534e6 100644 --- a/app/src/test/java/org/justinvisser/wolfskin/tv/TitleReadFacadeTest.kt +++ b/app/src/test/java/org/justinvisser/wolfskin/tv/TitleReadFacadeTest.kt @@ -33,6 +33,8 @@ import com.github.damontecres.wholphin.test.TestTracks import org.jellyfin.sdk.model.api.MediaStreamType import org.jellyfin.sdk.model.api.request.GetEpisodesRequest import org.jellyfin.sdk.model.api.request.GetItemsRequest +import org.jellyfin.sdk.model.api.ItemFields +import org.jellyfin.sdk.model.api.request.GetNextUpRequest import org.junit.Assert.* import org.junit.After import org.junit.Before @@ -219,6 +221,43 @@ class TitleReadFacadeTest { assertEquals(2, TitleState(data, ShellPerson("justin", "Justin", "J", false)) {}.season) } + @Test fun leanEpisodeListsKeepSingleItemPlaybackTracksEvenWithoutNextUp() = runTest { + val series = BaseItemDto(id = seriesId, name = "Series", type = BaseItemKind.SERIES, people = emptyList(), providerIds = mapOf("Tmdb" to "42")) + val episode = file(firstId, BaseItemKind.EPISODE).copy(seriesId = seriesId, parentIndexNumber = 3, indexNumber = 1) + coEvery { library.getItem(seriesId) } returns successResponse(series) + coEvery { library.getItem(firstId) } returns successResponse(episode) + coEvery { shows.getNextUp(any()) } coAnswers { + assertTrue(firstArg().fields.orEmpty().isEmpty()) + successQueryResult(emptyList()) + } + coEvery { items.getItems(any()) } returns successQueryResult( + listOf(BaseItemDto(id = seasonId, type = BaseItemKind.SEASON, indexNumber = 3))) + coEvery { shows.getEpisodes(any()) } coAnswers { + val fields = firstArg().fields.orEmpty() + assertFalse(fields.any { it in listOf(ItemFields.MEDIA_SOURCES, ItemFields.MEDIA_STREAMS, ItemFields.PEOPLE, ItemFields.PARENT_ID) }) + successQueryResult(listOf(episode.copy(mediaSources = null, overview = "Episode synopsis"))) + } + val page = facade.read(seriesId, 3, preferences) + assertEquals(firstId.toString(), page.playTarget?.id) + assertEquals("MKV", page.versions.single().container) + assertEquals(listOf(3), page.versions.single().audio.map { it.index }) + val saved = requireNotNull(facade.readSaved(seriesId, preferences)) + assertEquals(page.versions, saved.versions) + } + + @Test fun performerCreditListsOmitStreamsButKeepCharacterRoles() = runTest { + coEvery { library.getItem(firstId) } returns successResponse( + BaseItemDto(id = firstId, type = BaseItemKind.PERSON, name = "Actor")) + coEvery { items.getItems(any()) } coAnswers { + val fields = firstArg().fields.orEmpty() + assertFalse(fields.any { it in listOf(ItemFields.MEDIA_SOURCES, ItemFields.MEDIA_STREAMS, ItemFields.PARENT_ID) }) + assertTrue(ItemFields.PEOPLE in fields) + successQueryResult(listOf(file(secondId).copy(people = listOf( + org.jellyfin.sdk.model.api.BaseItemPerson(id = firstId, name = "Actor", role = "Chef"))))) + } + assertEquals("Chef", facade.readPerson(firstId, 0).credits.single().character) + } + @Test fun compactJellyfinSourceIdUsesTheSameVersionPersistenceKey() = runTest { coEvery { library.getItem(firstId) } returns successResponse(file(firstId)) coEvery { playback.savePlayVersion(any(), any()) } returns null 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/app/src/wolfskin/java/org/justinvisser/wolfskin/tv/read/LibraryReadFacade.kt b/app/src/wolfskin/java/org/justinvisser/wolfskin/tv/read/LibraryReadFacade.kt index 775235f2..3c1d01ce 100644 --- a/app/src/wolfskin/java/org/justinvisser/wolfskin/tv/read/LibraryReadFacade.kt +++ b/app/src/wolfskin/java/org/justinvisser/wolfskin/tv/read/LibraryReadFacade.kt @@ -72,7 +72,7 @@ class LibraryReadFacade @Inject constructor(private val api: ApiClient) { val response = api.itemsApi.getItems( userId = userId, parentId = library.id, recursive = true, includeItemTypes = listOf(if (destination == "movies") BaseItemKind.MOVIE else BaseItemKind.SERIES), - fields = listOf(ItemFields.PROVIDER_IDS, ItemFields.SORT_NAME, ItemFields.DATE_CREATED), + fields = listOf(ItemFields.PROVIDER_IDS, ItemFields.DATE_CREATED), sortBy = listOf(ItemSortBy.SORT_NAME), startIndex = offset, limit = WolfskinTokens.Behaviour.libraryChunk.toInt(), ).content diff --git a/app/src/wolfskin/java/org/justinvisser/wolfskin/tv/read/TitleReadFacade.kt b/app/src/wolfskin/java/org/justinvisser/wolfskin/tv/read/TitleReadFacade.kt index ff727717..1530b8df 100644 --- a/app/src/wolfskin/java/org/justinvisser/wolfskin/tv/read/TitleReadFacade.kt +++ b/app/src/wolfskin/java/org/justinvisser/wolfskin/tv/read/TitleReadFacade.kt @@ -53,7 +53,8 @@ class TitleReadFacade @Inject constructor( @param:ApplicationContext private val context: Context, ) { private val json = Json { ignoreUnknownKeys = true } - private val fields = listOf(ItemFields.PROVIDER_IDS, ItemFields.MEDIA_SOURCES, ItemFields.MEDIA_STREAMS, ItemFields.OVERVIEW, ItemFields.PEOPLE, ItemFields.PARENT_ID) + private val episodeFields = listOf(ItemFields.PROVIDER_IDS, ItemFields.OVERVIEW) + private val creditFields = listOf(ItemFields.PROVIDER_IDS, ItemFields.OVERVIEW, ItemFields.PEOPLE) /** Resolve through this user's Jellyfin permissions before granting a catalogue card playback. */ suspend fun findHeld(tmdb: Long, media: String): UUID? { @@ -88,12 +89,12 @@ class TitleReadFacade @Inject constructor( val series = if (opened.type == BaseItemKind.EPISODE && opened.seriesId != null) readItem(opened.seriesId!!) else opened val seasons = if (series.type == BaseItemKind.SERIES) api.itemsApi.getItems(GetItemsRequest(parentId = series.id, includeItemTypes = listOf(BaseItemKind.SEASON), recursive = false, sortBy = listOf(ItemSortBy.INDEX_NUMBER))).content.items.orEmpty() else emptyList() val next = if (opened.type == BaseItemKind.EPISODE || opened.type == BaseItemKind.MOVIE) opened else { - api.tvShowsApi.getNextUp(GetNextUpRequest(seriesId = series.id, userId = repository.currentUser?.id, limit = 1, fields = fields, enableResumable = true)).content.items.firstOrNull()?.let { api.userLibraryApi.getItem(it.id).content } + api.tvShowsApi.getNextUp(GetNextUpRequest(seriesId = series.id, userId = repository.currentUser?.id, limit = 1, enableResumable = true)).content.items.firstOrNull()?.let { readItem(it.id) } } val selectedNumber = season ?: next?.parentIndexNumber ?: seasons.firstOrNull()?.indexNumber val selected = seasons.firstOrNull { it.indexNumber == selectedNumber } ?: seasons.firstOrNull() - val episodes = selected?.let { api.tvShowsApi.getEpisodes(GetEpisodesRequest(seriesId = series.id, seasonId = it.id, fields = fields)).content.items.orEmpty() }.orEmpty() - val target = next ?: episodes.firstOrNull { it.type == BaseItemKind.EPISODE } + val episodes = selected?.let { api.tvShowsApi.getEpisodes(GetEpisodesRequest(seriesId = series.id, seasonId = it.id, fields = episodeFields)).content.items.orEmpty() }.orEmpty() + val target = next ?: episodes.firstOrNull { it.type == BaseItemKind.EPISODE }?.let { readItem(it.id) } val rows = (listOf(series, opened) + listOfNotNull(target) + episodes).distinctBy { it.id } val numbers = seasons.mapNotNull { it.indexNumber } val result = build(rows, numbers, selected?.indexNumber, opened.id.toString(), target?.id?.toString(), preferences) @@ -165,7 +166,7 @@ class TitleReadFacade @Inject constructor( } else emptyList() val person = previous.firstOrNull() ?: api.userLibraryApi.getItem(id).content val credits = api.itemsApi.getItems(GetItemsRequest(personIds = listOf(id), recursive = true, - includeItemTypes = listOf(BaseItemKind.MOVIE, BaseItemKind.SERIES), fields = fields, startIndex = start, limit = 200, + includeItemTypes = listOf(BaseItemKind.MOVIE, BaseItemKind.SERIES), fields = creditFields, startIndex = start, limit = 200, sortBy = listOf(ItemSortBy.PRODUCTION_YEAR, ItemSortBy.SORT_NAME))).content.items.orEmpty() val rows = listOf(person) + (previous.drop(1) + credits).distinctBy { it.id } if (personCacheFile(id) != file) throw kotlinx.coroutines.CancellationException("Performer identity changed") 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..2c1c4c56 100644 --- a/docker/README.md +++ b/docker/README.md @@ -27,6 +27,64 @@ 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. +### Full-suite test diagnostics + +The Gradle daemon heap (`org.gradle.jvmargs` in `gradle.properties`) is separate +from the forked test-worker heap. The default test worker uses 512 MiB; changing +the daemon's `-Xmx` does not increase it. A larger worker heap did not resolve +the person-picker fixture's missing Compose snapshot notifications. + +Gradle 9.8.2 runs previously failed test classes first on the next invocation. +An immediate green `make test` rerun can therefore hide an order-sensitive +failure rather than prove a fix. For comparisons, clear only generated binary +test-result history (`build/test-results//binary` in each module) before +each run and record actual class-start order; XML file listing order is not +execution order. Keep worker heap and parallelism unchanged between samples. +`PersonPickerSessionTest` must also pass late in the full app JVM, not just alone +or promoted to the front by previous failures. + +## 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, @@ -570,15 +628,29 @@ retain the approved frames except for the requested feedback changes: - Every Discover heading uses a 104 px band, including a 60 px title-to-card gap. Home heading geometry is unchanged. - Hero/backdrop decoding is bounded to 1920×1080; upright hero sheets request - 420×630. The shared request builder removes conflicting Jellyfin size caps, - preserves tags and quality 96, and selects a suitable TMDB tier. The 64 MiB - memory cache, 128 MiB disk cache and four image-read limit remain unchanged. + 420×630. The shared request builder replaces conflicting Jellyfin size/encoding + parameters: cards use WebP quality 80, while hero backdrops/sheets retain JPEG + quality 96. Poster/tile/wide/episode requests are 256×384, 400×225, 544×306 and + 400×280; Jellyfin fill preserves aspect ratio before the UI crops. Tags and + authentication query parameters remain intact. TMDB uses w342 for posters and + portraits, w780 for landscape cards and w1280 for hero backdrops, never original. + Lossy WebP decoding is guaranteed below minSdk 23; API 28+ additionally checks + decoder support and requests JPEG if WebP is unavailable. The 64 MiB memory + cache, 128 MiB disk cache and four image-read limit remain unchanged. `HomeViewportTest` covers short/long poster, tile and wide rows, both row directions, first focus and native Back from the actual title surface. Native proof records real Bear season-three metadata through the title read facade; the proof-only harness is not part of the shipped tests or APK. +Episode lists request only ProviderIds and Overview. Next up reads only identity, +then hydrates the selected playback target through the existing single-item read, +including when Next up is empty. Media sources/streams and cast remain on detail +reads for header track/version controls. Performer credits omit media sources, +streams and ParentId but retain People because their cards show character roles. +Movies/Shows grids retain ProviderIds and DateCreated; server-side SortName +ordering does not require returning SortName on every item. + ## Title pages and people diff --git a/wolfskin-design/src/main/java/org/justinvisser/wolfskin/design/WolfskinImages.kt b/wolfskin-design/src/main/java/org/justinvisser/wolfskin/design/WolfskinImages.kt index 349fd2d2..e9b63760 100644 --- a/wolfskin-design/src/main/java/org/justinvisser/wolfskin/design/WolfskinImages.kt +++ b/wolfskin-design/src/main/java/org/justinvisser/wolfskin/design/WolfskinImages.kt @@ -1,6 +1,8 @@ package org.justinvisser.wolfskin.design import android.content.Context +import android.graphics.ImageDecoder +import android.os.Build import android.net.Uri import coil3.ComponentRegistry import coil3.ImageLoader @@ -23,6 +25,8 @@ object WolfskinImages { private var networkConfiguration: (ComponentRegistry.Builder.() -> Unit)? = null private const val MEBIBYTE = 1024L * 1024L private const val IMAGE_READS = 4 + private const val CARD_QUALITY = "80" + private const val HERO_QUALITY = "96" private val readPermits = Semaphore(IMAGE_READS) /** The app supplies authenticated networking without exposing its server/session to design code. */ @@ -78,32 +82,34 @@ object WolfskinImages { private val jellyfinImagePath = Regex("/Items/[^/]+/Images/[^/]+(?:/\\d+)?", RegexOption.IGNORE_CASE) private val tmdbSizePath = Regex("/t/p/(?:w\\d+|original)/") - private val dimensionParameters = setOf("width", "height", "maxwidth", "maxheight", "fillwidth", "fillheight") + private val imageParameters = setOf("width", "height", "maxwidth", "maxheight", "fillwidth", "fillheight", "quality", "format") + // Lossy WebP is mandatory from API 14; minSdk is 23. Newer platforms expose a decoder capability query. + private val supportsWebp by lazy { Build.VERSION.SDK_INT < 28 || ImageDecoder.isMimeTypeSupported("image/webp") } private fun sourceForSize(source: Any, width: Int, height: Int): Any { if (source !is String) return source val uri = Uri.parse(source) if (uri.scheme != "https" && uri.scheme != "http") return source if (uri.host == "image.tmdb.org") { - val size = if (height > width) when { - width <= 500 -> "w500" - width <= 780 -> "w780" - else -> "original" - } else when { - width <= 780 -> "w780" - width <= 1280 -> "w1280" - else -> "original" + val size = when { + height >= width -> "w342" + width > WolfskinTokens.Image.wideRequestWidth -> "w1280" + else -> "w780" } return uri.buildUpon().path(uri.path?.replace(tmdbSizePath, "/t/p/$size/")).build().toString() } if (!jellyfinImagePath.matches(uri.path.orEmpty())) return source + val hero = width == WolfskinTokens.Image.heroRequestWidth && height == WolfskinTokens.Image.heroRequestHeight || + width == WolfskinTokens.Image.heroPosterRequestWidth && height == WolfskinTokens.Image.heroPosterRequestHeight val builder = uri.buildUpon().clearQuery() for (name in uri.queryParameterNames) { - if (name.lowercase() !in dimensionParameters) { + if (name.lowercase() !in imageParameters) { uri.getQueryParameters(name).forEach { value -> builder.appendQueryParameter(name, value) } } } return builder.appendQueryParameter("fillWidth", width.toString()) - .appendQueryParameter("fillHeight", height.toString()).build().toString() + .appendQueryParameter("fillHeight", height.toString()) + .appendQueryParameter("format", if (hero || !supportsWebp) "Jpg" else "Webp") + .appendQueryParameter("quality", if (hero) HERO_QUALITY else CARD_QUALITY).build().toString() } } diff --git a/wolfskin-design/src/main/java/org/justinvisser/wolfskin/design/WolfskinTokens.kt b/wolfskin-design/src/main/java/org/justinvisser/wolfskin/design/WolfskinTokens.kt index fa730cd2..6b77b2ef 100644 --- a/wolfskin-design/src/main/java/org/justinvisser/wolfskin/design/WolfskinTokens.kt +++ b/wolfskin-design/src/main/java/org/justinvisser/wolfskin/design/WolfskinTokens.kt @@ -220,10 +220,10 @@ object WolfskinTokens { object Image { const val posterRequestWidth = 256 const val posterRequestHeight = 384 - const val tileRequestWidth = 416 - const val tileRequestHeight = 234 - const val wideRequestWidth = 560 - const val wideRequestHeight = 315 + const val tileRequestWidth = 400 + const val tileRequestHeight = 225 + const val wideRequestWidth = 544 + const val wideRequestHeight = 306 const val heroRequestWidth = 1920 const val heroRequestHeight = 1080 const val heroPosterRequestWidth = 420 diff --git a/wolfskin-design/src/test/java/org/justinvisser/wolfskin/design/WolfskinImagesTest.kt b/wolfskin-design/src/test/java/org/justinvisser/wolfskin/design/WolfskinImagesTest.kt new file mode 100644 index 00000000..dfefe549 --- /dev/null +++ b/wolfskin-design/src/test/java/org/justinvisser/wolfskin/design/WolfskinImagesTest.kt @@ -0,0 +1,61 @@ +package org.justinvisser.wolfskin.design + +import android.app.Application +import android.net.Uri +import org.junit.Assert.* +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.RuntimeEnvironment +import org.robolectric.annotation.Config + +/** Card URLs bound server work and transfer size, while hero quality and cache identity stay intact. */ +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [34], application = Application::class) +class WolfskinImagesTest { + private data class Surface(val name: String, val width: Int, val height: Int, val tier: String, val hero: Boolean = false) + private val surfaces = listOf( + Surface("grid/Home/Discover/person poster", 256, 384, "w342"), + Surface("Home tile", 400, 225, "w780"), + Surface("Home wide", 544, 306, "w780"), + Surface("episode still", 400, 280, "w780"), + Surface("cast portrait", 200, 200, "w342"), + Surface("person portrait", 240, 240, "w342"), + Surface("hero poster", 420, 630, "w342", true), + Surface("Home/Discover/title hero", 1920, 1080, "w1280", true), + ) + + private fun request(source: Any, surface: Surface): Any = WolfskinImages.request( + RuntimeEnvironment.getApplication(), source, surface.width, surface.height, surface.height > surface.width).data + + @Test fun jellyfinSurfacesReplaceStaleDimensionsAndEncodingWithoutLosingIdentity() { + for (surface in surfaces) { + val source = "https://jelly.example/Items/123/Images/Backdrop/0?tag=stable&api_key=kept&MaxWidth=1280&height=720&FillHeight=720&Quality=96&Format=Png" + val rewritten = request(source, surface).toString() + val uri = Uri.parse(rewritten) + assertEquals(surface.name, surface.width.toString(), uri.getQueryParameter("fillWidth")) + assertEquals(surface.name, surface.height.toString(), uri.getQueryParameter("fillHeight")) + assertEquals(surface.name, if (surface.hero) "Jpg" else "Webp", uri.getQueryParameter("format")) + assertEquals(surface.name, if (surface.hero) "96" else "80", uri.getQueryParameter("quality")) + assertEquals("stable", uri.getQueryParameter("tag")) + assertEquals("kept", uri.getQueryParameter("api_key")) + assertEquals(setOf("tag", "api_key", "fillWidth", "fillHeight", "format", "quality"), uri.queryParameterNames) + assertEquals(rewritten, request(rewritten, surface)) + } + } + + @Test fun tmdbSurfacesNeverDownloadOriginalArtwork() { + for (surface in surfaces) { + for (oldTier in listOf("original", "w500", "w780")) { + assertEquals(surface.name, "https://image.tmdb.org/t/p/${surface.tier}/art.jpg?identity=kept", + request("https://image.tmdb.org/t/p/$oldTier/art.jpg?identity=kept", surface)) + } + } + } + + @Test fun localAndUnrelatedSourcesRemainUntouched() { + for (source in listOf("file:///android_asset/poster.webp", "https://cdn.example/poster.jpg?width=800", "https://jelly.example/Users/123/Images/Primary", Uri.parse("content://art/1"))) { + assertEquals(source, request(source, surfaces.first())) + } + } +} diff --git a/wolfskin-design/tokens.json b/wolfskin-design/tokens.json index 70571dc3..cc1a845b 100644 --- a/wolfskin-design/tokens.json +++ b/wolfskin-design/tokens.json @@ -1492,8 +1492,8 @@ }, "tileRequest": { "px": [ - 416, - 234 + 400, + 225 ], "decode": "ARGB_8888", "source": "index.html:549", @@ -1501,8 +1501,8 @@ }, "wideRequest": { "px": [ - 560, - 315 + 544, + 306 ], "decode": "ARGB_8888", "source": "index.html:401", diff --git a/wolfskin-design/tokens.sha256 b/wolfskin-design/tokens.sha256 index 713ae2f4..405b0abe 100644 --- a/wolfskin-design/tokens.sha256 +++ b/wolfskin-design/tokens.sha256 @@ -2,7 +2,7 @@ b3f096b692c395534177cc82ac68f5b801870bc7e28f0dd5cd7c17e2103920a3 discover-token 85109c394255f6120a621fbefe8e58df82832974ccd6a6a975a61567f641ea2a generate_tokens.py 25985087986a46f00e2819917d0421ff2b437ec21c54f6ce6412b84c0eb05302 home-tokens.json be3af0ddcb0d67abef0805d19d7a33cdb55428bcace656b50f0a38935f212028 shell-tokens.json -bb64e45654fce2cd3ba595b505fb8b5a540526cef696dfa974be25f028d3e119 src/main/java/org/justinvisser/wolfskin/design/WolfskinTokens.kt +580ed1710468ccbd97082afaa445e96ae71510ebace711cc735ea389ecb9249d src/main/java/org/justinvisser/wolfskin/design/WolfskinTokens.kt e0a2f5df2f249bce3f6e13f4a0320c7b18b85777eaf04a27a428d6683a6aad28 title-tokens.json 36d5b2646dff2e4055d2683e4c284111a6886c311e9c924b73061fa785bc863d tokens.css -3226c6fb9c7e35c5bde90aaf01630b238cfcb9a768ff60c49461e19a200c9125 tokens.json +226f4bd07c0b105f8640beff71fe2c0c95d0f8862e624cea511384a4591b44ad tokens.json