Preserve TV Back viewports and person-switch authority
All checks were successful
review/delta-opus Tier C delta review: 0 blockers; 4 non-blocking notes (N1-N4)
gate/round2-candidate Full suite 1308/1308 and Opt 1.1.2/71 checks on this exact head (data/tv-r2-gate5/result.json)

This commit is contained in:
justinzeus 2026-11-08 18:37:55 +01:00
parent 9355288f79
commit 72849f0734
7 changed files with 215 additions and 52 deletions

View file

@ -21,6 +21,8 @@ import kotlinx.coroutines.flow.flatMapLatest
import kotlinx.coroutines.flow.flow
import kotlinx.coroutines.flow.map
import kotlinx.coroutines.flow.update
import kotlinx.coroutines.sync.Mutex
import kotlinx.coroutines.sync.withLock
import kotlinx.coroutines.withContext
import kotlinx.serialization.Serializable
import org.jellyfin.sdk.Jellyfin
@ -51,6 +53,7 @@ class ServerRepository
@param:IoDispatcher private val ioDispatcher: CoroutineDispatcher,
private val mostRecentServerProvider: MostRecentServerProvider,
) {
private val userChangeMutex = Mutex()
private var _current = MutableStateFlow<CurrentUser?>(null)
val current: StateFlow<CurrentUser?> = _current
@ -100,33 +103,35 @@ class ServerRepository
user: JellyfinUser,
): CurrentUser =
withContext(ioDispatcher) {
if (server.id != user.serverId) {
throw IllegalStateException("User is not part of the server")
}
Timber.v("Changing user to ${user.name} on ${server.url}")
val previous = _current.value
apiClient.update(baseUrl = server.url, accessToken = user.accessToken)
try {
val (updatedServer, userDto) = readSession(server)
var updatedUser =
user.copy(
id = userDto.id,
name = userDto.name,
)
serverDao.addOrUpdateServer(updatedServer)
updatedUser = serverDao.addOrUpdateUser(updatedUser)
userChangeMutex.withLock {
if (server.id != user.serverId) {
throw IllegalStateException("User is not part of the server")
}
Timber.v("Changing user to ${user.name} on ${server.url}")
val previous = _current.value
apiClient.update(baseUrl = server.url, accessToken = user.accessToken)
try {
val (updatedServer, userDto) = readSession(server)
var updatedUser =
user.copy(
id = userDto.id,
name = userDto.name,
)
serverDao.addOrUpdateServer(updatedServer)
updatedUser = serverDao.addOrUpdateUser(updatedUser)
val currentUser = CurrentUser(updatedServer, updatedUser)
currentCoroutineContext().ensureActive()
mostRecentServerProvider.save(currentUser)
currentCoroutineContext().ensureActive()
_current.value = currentUser
_currentUserDto.value = ServerUserConfig(userDto)
return@withContext currentUser
} catch (exception: Exception) {
// A failed or cancelled selection must not lend its token to the still-signed-in person.
apiClient.update(baseUrl = previous?.server?.url, accessToken = previous?.user?.accessToken)
throw exception
val currentUser = CurrentUser(updatedServer, updatedUser)
currentCoroutineContext().ensureActive()
mostRecentServerProvider.save(currentUser)
currentCoroutineContext().ensureActive()
_current.value = currentUser
_currentUserDto.value = ServerUserConfig(userDto)
currentUser
} catch (exception: Exception) {
// A failed or cancelled selection must not lend its token to the still-signed-in person.
apiClient.update(baseUrl = previous?.server?.url, accessToken = previous?.user?.accessToken)
throw exception
}
}
}

View file

@ -15,6 +15,8 @@ import kotlinx.coroutines.awaitCancellation
import kotlinx.coroutines.withTimeoutOrNull
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.async
import kotlinx.coroutines.NonCancellable
import kotlinx.coroutines.withContext
import kotlinx.coroutines.test.StandardTestDispatcher
import kotlinx.coroutines.test.runCurrent
import kotlinx.coroutines.test.runTest
@ -117,6 +119,36 @@ class ServerSavedSessionTest {
verifyFailedSwitch(cancel = true)
}
@Test fun cancelledOlderSwitchCannotReplaceTheNewerAuthenticatedToken() = runTest(dispatcher) {
var activeToken: String? = null
every { api.update(any(), any(), any(), any()) } answers { activeToken = secondArg() }
repository.restoreSavedSession()
val older = dao.addOrUpdateUser(user.copy(rowId = 0, id = UUID.randomUUID(), name = "Older selection", accessToken = "older-token"))
val newer = dao.addOrUpdateUser(user.copy(rowId = 0, id = UUID.randomUUID(), name = "Newer selection", accessToken = "newer-token"))
val entered = CompletableDeferred<Unit>()
val release = CompletableDeferred<Unit>()
coEvery { recent.save(any()) } just Runs
coEvery { users.getCurrentUser() } coAnswers {
if (activeToken == older.accessToken) {
entered.complete(Unit)
withContext(NonCancellable) { release.await() }
throw IOException("late failed selection")
}
successResponse(dto.copy(id = newer.id, name = newer.name))
}
val pending = async { runCatching { repository.changeUser(server, older) } }
entered.await()
pending.cancel()
val selected = async { repository.changeUser(server, newer) }
runCurrent()
release.complete(Unit)
pending.join()
val current = selected.await()
assertEquals(newer.id, current.user.id)
assertEquals(current, repository.current.value)
assertEquals(newer.accessToken, activeToken)
}
private suspend fun verifyFailedSwitch(cancel: Boolean) {
var activeToken: String? = null
every { api.update(any(), any(), any(), any()) } answers { activeToken = secondArg() }

View file

@ -132,6 +132,25 @@ class OfflinePersonSwitchTest {
assertEquals(original, current.value)
}
@Test fun failedAvatarSwitchRestoresOnlyTheOriginalHouseholdSession() {
failSwitch(fromAvatar = true)
await { conductarr.identity.value?.signedIn == true }
val identity = requireNotNull(conductarr.identity.value)
assertEquals("${server.id}:${users.first().id}", identity.person)
assertEquals("justin", identity.operator!!.requester)
assertEquals(original, current.value)
}
@Test fun expiredTargetCredentialsStillOpenTheSignInGate() {
coEvery { repository.changeUser(server, users[1]) } throws org.jellyfin.sdk.api.client.exception.InvalidStatusException(401)
failSwitch(fromAvatar = true)
await { rule.onAllNodesWithTag("wolfskin-setup-picker").fetchSemanticsNodes().isNotEmpty() }
rule.onNodeWithText("Mindel").performClick()
await { setupModel.stage == WolfskinSetupViewModel.Stage.Login }
rule.onNodeWithTag("profile-password").assertIsDisplayed()
assertEquals(original, current.value)
}
@Test fun failedLaunchPickerSelectionBackExitsWithoutChangingIdentity() {
failSwitch(fromAvatar = false)
back()

View file

@ -5,6 +5,8 @@ import android.os.Build
import android.os.Looper
import androidx.activity.ComponentActivity
import androidx.activity.BackEventCompat
import androidx.compose.foundation.layout.fillMaxSize
import androidx.compose.ui.Modifier
import androidx.compose.runtime.LaunchedEffect
import androidx.compose.runtime.CompositionLocalProvider
import androidx.compose.ui.input.key.Key
@ -19,15 +21,28 @@ import java.io.File
import androidx.lifecycle.viewModelScope
import androidx.navigation3.runtime.NavEntry
import androidx.navigation3.ui.NavDisplay
import com.github.damontecres.wholphin.MainContent
import com.github.damontecres.wholphin.SignedInContent
import com.github.damontecres.wholphin.data.CurrentUser
import com.github.damontecres.wholphin.data.ServerRepository
import com.github.damontecres.wholphin.preferences.AppPreferences
import com.github.damontecres.wholphin.preferences.UserPreferences
import com.github.damontecres.wholphin.services.NavigationManager
import com.github.damontecres.wholphin.services.BackdropService
import com.github.damontecres.wholphin.services.BackdropResult
import com.github.damontecres.wholphin.services.ScreensaverService
import com.github.damontecres.wholphin.services.ScreensaverState
import com.github.damontecres.wholphin.services.SetupDestination
import com.github.damontecres.wholphin.ui.nav.Destination
import com.github.damontecres.wholphin.util.WholphinDispatchers
import io.mockk.coEvery
import io.mockk.every
import io.mockk.mockk
import java.util.UUID
import kotlinx.coroutines.cancel
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.CoroutineDispatcher
import kotlinx.coroutines.test.StandardTestDispatcher
import org.junit.After
import org.junit.Assert.*
import org.junit.Rule
@ -44,21 +59,32 @@ import org.robolectric.annotation.GraphicsMode
/** Real NavDisplay and system/predictive Back must pop one title/person entry before the root ladder. */
@RunWith(RobolectricTestRunner::class)
@Config(sdk = [34, 36], application = Application::class, qualifiers = "w960dp-h540dp-xhdpi")
@Config(sdk = [34, 36], application = Application::class, qualifiers = "w960dp-h540dp-television-xhdpi")
@GraphicsMode(GraphicsMode.Mode.NATIVE)
@OptIn(kotlinx.coroutines.ExperimentalCoroutinesApi::class)
class TitleEntryBackTest {
@get:Rule val rule = createAndroidComposeRule<ComponentActivity>()
private var systemAnimationScale = 1f
@get:Rule val rule = createAndroidComposeRule<ComponentActivity>(
effectContext = object : androidx.compose.ui.MotionDurationScale {
override val scaleFactor get() = systemAnimationScale
},
)
private lateinit var viewModel: WolfskinTitleViewModel
private lateinit var conductarr: ConductarrSession
private var homeModel: WolfskinHomeViewModel? = null
private var shellModel: WolfskinShellViewModel? = null
private var previousFixtures: Boolean? = null
private var previousHomeIo: CoroutineDispatcher? = null
private var previousLeanback: Boolean? = null
@After fun dispose() {
rule.mainClock.autoAdvance = true
if (::viewModel.isInitialized) viewModel.viewModelScope.cancel()
homeModel?.viewModelScope?.cancel()
shellModel?.viewModelScope?.cancel()
previousHomeIo?.let { WholphinDispatchers.IO = it }
previousHomeIo = null
previousLeanback?.let { org.robolectric.Shadows.shadowOf(rule.activity.packageManager).setSystemFeature(android.content.pm.PackageManager.FEATURE_LEANBACK, it) }
previousFixtures?.let(ConductarrPreference::updateFixtures)
if (::conductarr.isInitialized) conductarr.close()
}
@ -72,10 +98,21 @@ class TitleEntryBackTest {
@Test fun reducedMotionHomeTitleBackKeepsTheExactPreOpenViewport() = verifyHomeViewport(reduced = true, depth = 1)
@Test fun deepHomeTitleBackKeepsTheExactPreOpenViewport() = verifyHomeViewport(reduced = false, depth = 8)
@Test fun sharedHomeTitleBackKeepsTheExactPreOpenViewport() = verifyHomeViewport(reduced = false, depth = 1, person = "shared")
@Test fun loadingHomeTitleBackKeepsTheExactPreOpenViewport() = verifyHomeViewport(reduced = false, depth = 1, person = "shared", paintLoading = true)
@Test fun systemAnimationsOffHomeTitleBackKeepsTheExactPreOpenViewport() = verifyHomeViewport(reduced = false, depth = 1, person = "shared", systemScale = 0f)
private fun verifyHomeViewport(reduced: Boolean, depth: Int, person: String = "justin") {
private fun verifyHomeViewport(reduced: Boolean, depth: Int, person: String = "justin", paintLoading: Boolean = false, systemScale: Float = 1f) {
// A television qualifier alone does not advertise Leanback or leave Robolectric's touch mode.
systemAnimationScale = systemScale
val packageManager = rule.activity.packageManager
previousLeanback = packageManager.hasSystemFeature(android.content.pm.PackageManager.FEATURE_LEANBACK)
org.robolectric.Shadows.shadowOf(packageManager).setSystemFeature(android.content.pm.PackageManager.FEATURE_LEANBACK, true)
val navigation = NavigationManager()
val session = WolfskinSession()
val heldReads = if (paintLoading) StandardTestDispatcher().also {
previousHomeIo = WholphinDispatchers.IO
WholphinDispatchers.IO = it
} else null
conductarr = ConductarrSession(MemoryCacheStore())
ConductarrPreference.initialize(rule.activity)
previousFixtures = ConductarrPreference.fixtures
@ -84,21 +121,40 @@ class TitleEntryBackTest {
val user = com.github.damontecres.wholphin.data.model.JellyfinUser(id = UUID.fromString("00000000-0000-0000-0000-000000000002"), name = person.replaceFirstChar { it.uppercase() }, serverId = server.id, accessToken = null)
viewModel = WolfskinTitleViewModel(rule.activity, mockk(), TitleFixtureProvider(rule.activity), navigation, conductarr,
mockk(relaxed = true), mockk(relaxed = true), trailerReturn = mockk(relaxed = true))
val backdrop = mockk<BackdropService>(relaxed = true)
every { backdrop.backdropFlow } returns MutableStateFlow(BackdropResult.NONE)
val app = WolfskinShellViewModel(rule.activity, session, mockk(relaxed = true), navigation, mockk(relaxed = true),
mockk(relaxed = true), mockk(relaxed = true), WolfskinSeeAllLists(), conductarr, mockk(relaxed = true),
backdrop, mockk(relaxed = true), WolfskinSeeAllLists(), conductarr, mockk(relaxed = true),
org.justinvisser.wolfskin.tv.update.FakeUpdateController(), mockk(relaxed = true))
shellModel = app
val preferences = UserPreferences(AppPreferences.getDefaultInstance(), null)
val current = CurrentUser(server, user)
val live = MutableStateFlow<CurrentUser?>(current)
val setup = androidx.compose.runtime.mutableStateListOf<SetupDestination>(SetupDestination.AppContent(current))
val trailer = mockk<TrailerReturnState>(relaxed = true)
val screensaver = mockk<ScreensaverService>(relaxed = true)
every { screensaver.state } returns MutableStateFlow(ScreensaverState(false, false, false, false))
rule.setContent { CompositionLocalProvider(LocalReducedMotion provides reduced) {
WolfskinApp(server, user, navigation, preferences, viewModel = app,
homeContent = { shell, homeServer, homeUser, modifier ->
val source = androidx.lifecycle.viewmodel.compose.viewModel {
WolfskinHomeViewModel(rule.activity, mockk(relaxed = true), mockk(), navigation, WolfskinSeeAllLists(), conductarr, session)
val inputMode = androidx.compose.ui.platform.LocalInputModeManager.current
androidx.compose.runtime.LaunchedEffect(inputMode) { inputMode.requestInputMode(androidx.compose.ui.input.InputMode.Keyboard) }
MainContent(setup, navigation, preferences, backdrop, screensaver, live, trailer,
modifier = Modifier.fillMaxSize(),
setupInterceptor = { destination, modifier ->
val entry = (destination as SetupDestination.AppContent).current
SignedInContent(entry, live, preferences.appPreferences.signInAutomatically, trailer,
{ navigation.backStack.lastOrNull() }) {
WolfskinApp(entry.server, entry.user, navigation, preferences, modifier, app,
homeContent = { shell, homeServer, homeUser, contentModifier ->
val source = androidx.lifecycle.viewmodel.compose.viewModel {
WolfskinHomeViewModel(rule.activity, mockk(relaxed = true), mockk(), navigation, WolfskinSeeAllLists(), conductarr, session)
}
homeModel = source
HomeDestination(shell, homeServer, homeUser, contentModifier, source)
},
titleContent = { route, shell, settings -> TitleDestination(route, shell, settings, viewModel = viewModel, fixtures = true) })
}
homeModel = source
HomeDestination(shell, homeServer, homeUser, modifier, source)
},
titleContent = { route, shell, settings -> TitleDestination(route, shell, settings, viewModel = viewModel, fixtures = true) })
true
})
} }
rule.waitUntil(30_000) {
androidx.compose.runtime.snapshots.Snapshot.sendApplyNotifications()
@ -106,6 +162,20 @@ class TitleEntryBackTest {
session.state != null && app.signedInPerson == person
}
rule.onRoot().performKeyInput { pressKey(Key.DirectionCenter) }
if (heldReads != null) {
rule.waitUntil(30_000) {
Snapshot.sendApplyNotifications()
Shadows.shadowOf(Looper.getMainLooper()).idle()
homeModel != null && session.personCommit == null
}
rule.onNodeWithTag("wolfskin-home").assertExists()
assertEquals(HomeAvailability.Loading, homeModel!!.snapshot.conductarr)
val loadingCaptures = File(System.getProperty("wolfskin.captureDirectory"), "home-loading-$person").apply { mkdirs() }
rule.onRoot().captureRoboImage(File(loadingCaptures, "sdk${Build.VERSION.SDK_INT}.png").absolutePath)
heldReads.scheduler.runCurrent()
WholphinDispatchers.IO = previousHomeIo!!
previousHomeIo = null
}
rule.waitUntil(30_000) {
androidx.compose.runtime.snapshots.Snapshot.sendApplyNotifications()
Shadows.shadowOf(Looper.getMainLooper()).idle()
@ -120,7 +190,8 @@ class TitleEntryBackTest {
val tag = "home-card:${snapshot.rows[home.row].id}:0"
val before = home.listState.let { it.firstVisibleItemIndex to it.firstVisibleItemScrollOffset }
val bounds = rule.onNodeWithTag(tag).fetchSemanticsNode().boundsInRoot
val captures = File(System.getProperty("wolfskin.captureDirectory"), "home-title-back-$person-${if (reduced) "reduced" else "animated"}-$depth").apply { mkdirs() }
val motion = if (systemScale == 0f) "system-off" else if (reduced) "reduced" else "animated"
val captures = File(System.getProperty("wolfskin.captureDirectory"), "home-title-back-$person-$motion-$depth${if (paintLoading) "-loading" else ""}").apply { mkdirs() }
rule.onRoot().captureRoboImage(File(captures, "sdk${Build.VERSION.SDK_INT}-before.png").absolutePath)
rule.onRoot().performKeyInput { pressKey(Key.DirectionCenter) }
rule.waitUntil(30_000) {
@ -144,6 +215,14 @@ class TitleEntryBackTest {
rule.onRoot().captureRoboImage(File(captures, "sdk${Build.VERSION.SDK_INT}-after.png").absolutePath)
val after = home.listState.let { it.firstVisibleItemIndex to it.firstVisibleItemScrollOffset }
val afterBounds = rule.onNodeWithTag(tag).fetchSemanticsNode().boundsInRoot
rule.mainClock.advanceTimeBy(1000)
Shadows.shadowOf(Looper.getMainLooper()).idleFor(java.time.Duration.ofMillis(1000))
rule.waitForIdle()
rule.onRoot().captureRoboImage(File(captures, "sdk${Build.VERSION.SDK_INT}-settled.png").absolutePath)
val settled = home.listState.let { it.firstVisibleItemIndex to it.firstVisibleItemScrollOffset }
val settledBounds = rule.onNodeWithTag(tag).fetchSemanticsNode().boundsInRoot
assertEquals("settled card bounds before=$bounds after=$settledBounds; list before=$before after=$settled", before, settled)
assertEquals(bounds, settledBounds)
assertEquals("card bounds before=$bounds after=$afterBounds; list before=$before after=$after", before, after)
assertEquals(bounds, afterBounds)
}

View file

@ -56,6 +56,7 @@ import kotlinx.coroutines.sync.withLock
import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.launch
import org.jellyfin.sdk.model.serializer.toUUIDOrNull
import org.jellyfin.sdk.api.client.exception.InvalidStatusException
import kotlinx.coroutines.withContext
import org.justinvisser.wolfskin.design.PeopleStatus
import org.justinvisser.wolfskin.design.SettingsList
@ -280,11 +281,14 @@ class WolfskinShellViewModel @Inject constructor(
coroutineContext.ensureActive()
val open = {
val current = repository.current.value
if (current?.server?.id == server.id && current.user.id == user.id) {
val isAuthenticationFailure = exception is InvalidStatusException && (exception.status == 401 || exception.status == 403)
if (!isAuthenticationFailure && current?.server?.id == server.id && current.user.id == user.id) {
signedInPerson = previousPerson
session.state?.restorePersonPicker(previousPerson, selection.fromLaunch)
session.state?.toast = org.justinvisser.wolfskin.design.RecoveryMessage.PersonSwitchUnavailable
} else openSwitchRecovery(server)
identityJob = viewModelScope.launch { retryConductarr(server, user) }
} else if (isAuthenticationFailure) setupNavigation.navigateTo(SetupDestination.UserList(server))
else openSwitchRecovery(server)
}
if (commit == null) open() else commit.fallback = open
}