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 6b3403c6..ec61583d 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/MainActivity.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/MainActivity.kt @@ -32,7 +32,6 @@ 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 @@ -577,22 +576,12 @@ class MainActivityViewModel val restoredSession = if (signInAutomatically) { Timber.v("Sign in automatically") - serverRepository.restoreSavedSession() + serverRepository.restoreLastSession() } 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 aba2a0a6..6ec9d4b4 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/WholphinApplication.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/WholphinApplication.kt @@ -17,7 +17,6 @@ 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) @@ -55,7 +54,6 @@ class WholphinApplication : override fun onCreate() { super.onCreate() - networkStartup.start() if (BuildConfig.DEBUG) { StrictMode.setThreadPolicy( StrictMode.ThreadPolicy @@ -112,9 +110,6 @@ 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 68af5123..f72eb3b7 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,10 +51,6 @@ 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 0930129e..2c24c8f3 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,8 +8,6 @@ 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 @@ -103,7 +101,15 @@ class ServerRepository } Timber.v("Changing user to ${user.name} on ${server.url}") apiClient.update(baseUrl = server.url, accessToken = user.accessToken) - val (updatedServer, userDto) = readSession(server) + 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 + } var updatedUser = user.copy( id = userDto.id, @@ -119,45 +125,6 @@ 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 0591f4a1..3a114017 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,9 +26,6 @@ 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 @@ -94,14 +91,12 @@ object AppModule { @StandardOkHttpClient @Provides @Singleton - 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") }) + fun okHttpClient() = + OkHttpClient + .Builder() + .apply { + // TODO user agent, timeouts, logging, etc + }.build() @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 e0c8005f..d83ae269 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,11 +4,6 @@ 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 @@ -78,7 +73,10 @@ class CrashReportSender : ReportSender { private fun MostRecentServer.createApi(context: Context): ApiClient? = (this as? MostRecentServer.ServerAndUser)?.let { - val okHttpClient = EntryPointAccessors.fromApplication(context.applicationContext, CrashNetwork::class.java).client() + val okHttpClient = + OkHttpClient + .Builder() + .build() val okHttpFactory = OkHttpFactory(okHttpClient) val api = createJellyfin { @@ -91,11 +89,3 @@ 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 d8fe5695..1de5becd 100644 --- a/app/src/test/java/com/github/damontecres/wholphin/TestMainActivityViewModel.kt +++ b/app/src/test/java/com/github/damontecres/wholphin/TestMainActivityViewModel.kt @@ -32,8 +32,6 @@ 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 @@ -118,6 +116,9 @@ 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() @@ -131,7 +132,7 @@ class TestMainActivityViewModel { destination as SetupDestination.AppContent Assert.assertEquals(currentUser, destination.current) } - coVerify(exactly = 0) { serverRepository.restoreSavedSession() } + coVerify(exactly = 0) { serverRepository.restoreLastSession() } coVerify(exactly = 0) { serverRepository.getMostRecentServer() } } @@ -149,7 +150,7 @@ class TestMainActivityViewModel { viewModel.handleAppStart() coVerify(exactly = 0) { serverRepository.tryChangeUser(any(), any()) } - coVerify(exactly = 0) { serverRepository.restoreSavedSession() } + coVerify(exactly = 0) { serverRepository.restoreLastSession() } coVerify(exactly = 1) { serverRepository.getMostRecentServer() } val args = mutableListOf() verify { setupNavigationManager.navigateTo(capture(args)) } @@ -164,20 +165,6 @@ 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 { @@ -191,7 +178,7 @@ class TestMainActivityViewModel { coEvery { serverRepository.tryChangeUser(serverId, userId) } returns null - coEvery { serverRepository.restoreSavedSession() } returns + coEvery { serverRepository.restoreLastSession() } returns RestoredSession.ServerOnly(server) viewModel.handleAppStart() @@ -217,7 +204,7 @@ class TestMainActivityViewModel { signInAutomatically = true } every { serverRepository.current } returns MutableStateFlow(null) - coEvery { serverRepository.restoreSavedSession() } returns + coEvery { serverRepository.restoreLastSession() } returns RestoredSession.ServerOnly(server) coEvery { serverRepository.tryChangeUser(serverId, userId) @@ -225,7 +212,7 @@ class TestMainActivityViewModel { viewModel.handleAppStart() - coVerify(exactly = 1) { serverRepository.restoreSavedSession() } + coVerify(exactly = 1) { serverRepository.restoreLastSession() } val slot = slot() verify { setupNavigationManager.navigateTo(capture(slot)) } slot.captured.let { destination -> @@ -245,11 +232,11 @@ class TestMainActivityViewModel { signInAutomatically = true } every { serverRepository.current } returns MutableStateFlow(null) - coEvery { serverRepository.restoreSavedSession() } returns RestoredSession.None + coEvery { serverRepository.restoreLastSession() } returns RestoredSession.None viewModel.handleAppStart() - coVerify(exactly = 1) { serverRepository.restoreSavedSession() } + coVerify(exactly = 1) { serverRepository.restoreLastSession() } val slot = slot() verify { setupNavigationManager.navigateTo(capture(slot)) } slot.captured.let { destination -> @@ -267,7 +254,7 @@ class TestMainActivityViewModel { signInAutomatically = true } every { serverRepository.current } returns MutableStateFlow(currentUser) - coEvery { serverRepository.restoreSavedSession() } returns + coEvery { serverRepository.restoreLastSession() } returns RestoredSession.Success(currentUser) coEvery { intentService.parseIntent(any()) } returns IntentResult.NoOp @@ -284,7 +271,7 @@ class TestMainActivityViewModel { // destination as SetupDestination.AppContent // Assert.assertEquals(currentUser, destination.current) // } - coVerify(exactly = 0) { serverRepository.restoreSavedSession() } + coVerify(exactly = 0) { serverRepository.restoreLastSession() } 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 deleted file mode 100644 index 2724cc61..00000000 --- a/app/src/test/java/com/github/damontecres/wholphin/services/ServerSavedSessionTest.kt +++ /dev/null @@ -1,136 +0,0 @@ -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 3b4d3efa..b266bb96 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,7 +62,6 @@ 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 @@ -92,11 +91,12 @@ object TestModule { @StandardOkHttpClient @Provides @Singleton - fun okHttpClient(dns: PersistentDns) = AppModule.okHttpClient(dns) - - @Provides - @Singleton - fun dns(@ApplicationContext context: Context): PersistentDns = AppModule.dns(context) + fun okHttpClient() = + OkHttpClient + .Builder() + .apply { + // TODO user agent, timeouts, logging, etc + }.build() @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 07cd1e37..aa1faf95 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.DATE_CREATED), + includeItemTypes = listOf(kind), fields = listOf(ItemFields.PROVIDER_IDS, ItemFields.SORT_NAME, 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 a2fb8b0d..92afbe07 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.DATE_CREATED), + fields = listOf(ItemFields.PROVIDER_IDS, ItemFields.SORT_NAME, 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 95fcbb52..8e13dca4 100644 --- a/app/src/test/java/org/justinvisser/wolfskin/tv/PersonPickerSessionTest.kt +++ b/app/src/test/java/org/justinvisser/wolfskin/tv/PersonPickerSessionTest.kt @@ -128,8 +128,6 @@ 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 deleted file mode 100644 index 1fb29d31..00000000 --- a/app/src/test/java/org/justinvisser/wolfskin/tv/SavedPeoplePickerTest.kt +++ /dev/null @@ -1,92 +0,0 @@ -// 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 7ea534e6..fb7bf06e 100644 --- a/app/src/test/java/org/justinvisser/wolfskin/tv/TitleReadFacadeTest.kt +++ b/app/src/test/java/org/justinvisser/wolfskin/tv/TitleReadFacadeTest.kt @@ -33,8 +33,6 @@ 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 @@ -221,43 +219,6 @@ 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 b794391b..559b4365 100644 --- a/app/src/wolfskin/java/org/justinvisser/wolfskin/tv/ConductarrPreference.kt +++ b/app/src/wolfskin/java/org/justinvisser/wolfskin/tv/ConductarrPreference.kt @@ -21,7 +21,6 @@ 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 { @@ -55,8 +54,8 @@ object ConductarrPreference { object ConductarrModule { @Provides @Singleton - fun session(@ApplicationContext context: Context, @IoCoroutineScope scope: CoroutineScope, dns: PersistentDns): ConductarrSession = - ConductarrSession(ConductarrDiskCache(context), scope, dns) + fun session(@ApplicationContext context: Context, @IoCoroutineScope scope: CoroutineScope): ConductarrSession = + ConductarrSession(ConductarrDiskCache(context), scope) } /** 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 ce639277..5ba52caa 100644 --- a/app/src/wolfskin/java/org/justinvisser/wolfskin/tv/ConductarrSession.kt +++ b/app/src/wolfskin/java/org/justinvisser/wolfskin/tv/ConductarrSession.kt @@ -29,7 +29,6 @@ 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() @@ -51,7 +50,7 @@ class ConductarrSession( } try { val client = if (token.isNullOrBlank()) null else try { - ConductarrClient(baseUrl, token, PersonCacheStore(cache, person), timeouts = ClientTimeouts(3, 5, 8), dns = dns, onEvent = { event -> + ConductarrClient(baseUrl, token, PersonCacheStore(cache, person), timeouts = ClientTimeouts(3, 5, 8), 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 deleted file mode 100644 index 899341e5..00000000 --- a/app/src/wolfskin/java/org/justinvisser/wolfskin/tv/NetworkStartup.kt +++ /dev/null @@ -1,46 +0,0 @@ -// 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 3c1d01ce..775235f2 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.DATE_CREATED), + fields = listOf(ItemFields.PROVIDER_IDS, ItemFields.SORT_NAME, 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 1530b8df..ff727717 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,8 +53,7 @@ class TitleReadFacade @Inject constructor( @param:ApplicationContext private val context: Context, ) { private val json = Json { ignoreUnknownKeys = true } - private val episodeFields = listOf(ItemFields.PROVIDER_IDS, ItemFields.OVERVIEW) - private val creditFields = listOf(ItemFields.PROVIDER_IDS, ItemFields.OVERVIEW, ItemFields.PEOPLE) + private val fields = listOf(ItemFields.PROVIDER_IDS, ItemFields.MEDIA_SOURCES, ItemFields.MEDIA_STREAMS, ItemFields.OVERVIEW, ItemFields.PEOPLE, ItemFields.PARENT_ID) /** Resolve through this user's Jellyfin permissions before granting a catalogue card playback. */ suspend fun findHeld(tmdb: Long, media: String): UUID? { @@ -89,12 +88,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, enableResumable = true)).content.items.firstOrNull()?.let { readItem(it.id) } + 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 } } 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 = episodeFields)).content.items.orEmpty() }.orEmpty() - val target = next ?: episodes.firstOrNull { it.type == BaseItemKind.EPISODE }?.let { readItem(it.id) } + 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 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) @@ -166,7 +165,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 = creditFields, startIndex = start, limit = 200, + includeItemTypes = listOf(BaseItemKind.MOVIE, BaseItemKind.SERIES), fields = fields, 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 8030fcb9..5559f15a 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,7 +67,6 @@ 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 @@ -75,7 +74,6 @@ 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 deleted file mode 100644 index 4248a4c0..00000000 --- a/conductarr-client/src/main/kotlin/org/justinvisser/wolfskin/conductarr/DnsConnectionRetry.kt +++ /dev/null @@ -1,60 +0,0 @@ -// 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 deleted file mode 100644 index 2c65b657..00000000 --- a/conductarr-client/src/main/kotlin/org/justinvisser/wolfskin/conductarr/PersistentDns.kt +++ /dev/null @@ -1,142 +0,0 @@ -// 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 8c8495fb..98173fce 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,10 +2,6 @@ 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 @@ -18,24 +14,13 @@ fun main() = runBlocking { try { MockWebServer().use { server -> server.start() - 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)) + val baseUrl = server.url("/").toString() server.enqueue(MockResponse.Builder().body(PROFILES_JSON).addHeader("ETag", "\"smoke\"").build()) - ConductarrClient(baseUrl, TEST_TOKEN, FileCacheStore(directory), dns = dns).use { client -> + ConductarrClient(baseUrl, TEST_TOKEN, FileCacheStore(directory)).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") } - val reopened = PersistentDns({ File(directory, "dns.json") }, Dns { listOf(live) }, Executor { it.run() }) - ConductarrClient(baseUrl, TEST_TOKEN, FileCacheStore(directory), dns = reopened).use { client -> + ConductarrClient(baseUrl, TEST_TOKEN, FileCacheStore(directory)).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 deleted file mode 100644 index 58b72d98..00000000 --- a/conductarr-client/src/test/kotlin/org/justinvisser/wolfskin/conductarr/DnsConnectionRetryTest.kt +++ /dev/null @@ -1,68 +0,0 @@ -// 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 deleted file mode 100644 index b167b354..00000000 --- a/conductarr-client/src/test/kotlin/org/justinvisser/wolfskin/conductarr/PersistentDnsTest.kt +++ /dev/null @@ -1,160 +0,0 @@ -// 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 2c1c4c56..fa36fcc0 100644 --- a/docker/README.md +++ b/docker/README.md @@ -27,64 +27,6 @@ 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, @@ -628,29 +570,15 @@ 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 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. + 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. `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 e9b63760..349fd2d2 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,8 +1,6 @@ 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 @@ -25,8 +23,6 @@ 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. */ @@ -82,34 +78,32 @@ object WolfskinImages { private val jellyfinImagePath = Regex("/Items/[^/]+/Images/[^/]+(?:/\\d+)?", RegexOption.IGNORE_CASE) private val tmdbSizePath = Regex("/t/p/(?:w\\d+|original)/") - 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 val dimensionParameters = setOf("width", "height", "maxwidth", "maxheight", "fillwidth", "fillheight") 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 = when { - height >= width -> "w342" - width > WolfskinTokens.Image.wideRequestWidth -> "w1280" - else -> "w780" + val size = if (height > width) when { + width <= 500 -> "w500" + width <= 780 -> "w780" + else -> "original" + } else when { + width <= 780 -> "w780" + width <= 1280 -> "w1280" + else -> "original" } 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 imageParameters) { + if (name.lowercase() !in dimensionParameters) { uri.getQueryParameters(name).forEach { value -> builder.appendQueryParameter(name, value) } } } return builder.appendQueryParameter("fillWidth", width.toString()) - .appendQueryParameter("fillHeight", height.toString()) - .appendQueryParameter("format", if (hero || !supportsWebp) "Jpg" else "Webp") - .appendQueryParameter("quality", if (hero) HERO_QUALITY else CARD_QUALITY).build().toString() + .appendQueryParameter("fillHeight", height.toString()).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 6b77b2ef..fa730cd2 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 = 400 - const val tileRequestHeight = 225 - const val wideRequestWidth = 544 - const val wideRequestHeight = 306 + const val tileRequestWidth = 416 + const val tileRequestHeight = 234 + const val wideRequestWidth = 560 + const val wideRequestHeight = 315 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 deleted file mode 100644 index dfefe549..00000000 --- a/wolfskin-design/src/test/java/org/justinvisser/wolfskin/design/WolfskinImagesTest.kt +++ /dev/null @@ -1,61 +0,0 @@ -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 cc1a845b..70571dc3 100644 --- a/wolfskin-design/tokens.json +++ b/wolfskin-design/tokens.json @@ -1492,8 +1492,8 @@ }, "tileRequest": { "px": [ - 400, - 225 + 416, + 234 ], "decode": "ARGB_8888", "source": "index.html:549", @@ -1501,8 +1501,8 @@ }, "wideRequest": { "px": [ - 544, - 306 + 560, + 315 ], "decode": "ARGB_8888", "source": "index.html:401", diff --git a/wolfskin-design/tokens.sha256 b/wolfskin-design/tokens.sha256 index 405b0abe..713ae2f4 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 -580ed1710468ccbd97082afaa445e96ae71510ebace711cc735ea389ecb9249d src/main/java/org/justinvisser/wolfskin/design/WolfskinTokens.kt +bb64e45654fce2cd3ba595b505fb8b5a540526cef696dfa974be25f028d3e119 src/main/java/org/justinvisser/wolfskin/design/WolfskinTokens.kt e0a2f5df2f249bce3f6e13f4a0320c7b18b85777eaf04a27a428d6683a6aad28 title-tokens.json 36d5b2646dff2e4055d2683e4c284111a6886c311e9c924b73061fa785bc863d tokens.css -226f4bd07c0b105f8640beff71fe2c0c95d0f8862e624cea511384a4591b44ad tokens.json +3226c6fb9c7e35c5bde90aaf01630b238cfcb9a768ff60c49461e19a200c9125 tokens.json