Merge pull request 'Fix post-sign-in crash from snapshot state writes' (#38) from fm/tv-hotfix-snapshot-write into main
This commit is contained in:
commit
e794dec253
5 changed files with 199 additions and 3 deletions
|
|
@ -0,0 +1,56 @@
|
|||
// 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.junit4.createAndroidComposeRule
|
||||
import kotlinx.coroutines.runBlocking
|
||||
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.*
|
||||
import org.robolectric.RobolectricTestRunner
|
||||
import org.robolectric.annotation.Config
|
||||
|
||||
/** No drawn screen may prewarm memory before the production Discover observer starts. */
|
||||
@RunWith(RobolectricTestRunner::class)
|
||||
@Config(sdk = [34], application = Application::class)
|
||||
class DiscoverSnapshotReadTest {
|
||||
@get:Rule val rule = createAndroidComposeRule<ComponentActivity>()
|
||||
|
||||
@Test fun signedInHostObservesFreshShellThroughTheRealAsyncProvider() {
|
||||
FakeConductarr().use { fake ->
|
||||
ConductarrSession(MemoryCacheStore()).use { session ->
|
||||
session.select("server:shared", SHARED_TOKEN, fake.url)
|
||||
runBlocking { session.authenticate() }
|
||||
val identity = requireNotNull(session.identity.value)
|
||||
assertTrue(identity.signedIn)
|
||||
val shell = ShellState(listOf(ShellPerson("shared", "Shared", "S", true)), "shared")
|
||||
lateinit var current: DiscoverState
|
||||
rule.setContent {
|
||||
current = rememberDiscoverState(shell, shell::enterDestination, {}, identity = identity, fixtures = false,
|
||||
loadFixture = { error("Production observer must not load fixtures") })
|
||||
}
|
||||
rule.waitUntil(30_000) {
|
||||
org.robolectric.Shadows.shadowOf(android.os.Looper.getMainLooper()).idle()
|
||||
current.snapshot.status == DiscoverStatus.Ready
|
||||
}
|
||||
rule.waitForIdle()
|
||||
rule.runOnIdle {
|
||||
assertTrue(fake.calls.any { it.url.encodedPath.endsWith("/feed") })
|
||||
assertEquals(0, current.focusedRow)
|
||||
assertEquals(0, current.column)
|
||||
current.focus(current.firstShelf)
|
||||
}
|
||||
rule.waitForIdle()
|
||||
rule.runOnIdle {
|
||||
assertNotNull(current.focusedLane)
|
||||
assertEquals(current.firstShelf, current.focusedRow)
|
||||
assertEquals(0, current.column)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -78,6 +78,13 @@ resolves consistently in native captures; the approved JSON weights stay unchang
|
|||
Nav, picker names, labels and pill weights use that same mapping. Nav label colour
|
||||
fades for 200 ms ease-out; the blob retains its critically damped spring.
|
||||
|
||||
Shell view accessors are read-only, including the first read from Discover's
|
||||
`snapshotFlow`. Construction and explicit person/navigation events retain root
|
||||
and destination memory; reading an unknown destination returns an unsaved default.
|
||||
Per-person focus, cards and scrollers still survive route changes, while reselecting
|
||||
the current root resets it. JVM regressions exercise both read-only snapshots and
|
||||
the signed-in asynchronous Discover host without a screen prewarming its memory.
|
||||
|
||||
Settings pills have a separate 1 dp stroke (2 px on the stage), not a focus ring.
|
||||
The stroke is inset within the fixed 20 dp / 40 px height; horizontal border-box
|
||||
padding includes it. The 6 px dash / 4 px gap follows the browser's 3:2 stroke-width
|
||||
|
|
|
|||
|
|
@ -57,6 +57,7 @@ class ShellState(
|
|||
people = value
|
||||
if (selected != null) person = selected
|
||||
pickerIndex = people.indexOf(person).coerceAtLeast(0)
|
||||
preparePersonMemory()
|
||||
}
|
||||
var person by mutableStateOf(people.firstOrNull { it.key == initialPerson } ?: people.first())
|
||||
private set
|
||||
|
|
@ -73,7 +74,7 @@ class ShellState(
|
|||
var requestsVisible by mutableStateOf(false)
|
||||
var toast by mutableStateOf<String?>(null)
|
||||
private val personRequests = mutableMapOf<String, ComingInState>()
|
||||
val requests: ComingInState get() = personRequests.getOrPut(person.key) { ComingInState() }
|
||||
val requests: ComingInState get() = personRequests.getValue(person.key)
|
||||
private var requestReturnRoute: String? = null
|
||||
var handOff by mutableStateOf<String?>(null)
|
||||
var settingsRow by mutableStateOf(0)
|
||||
|
|
@ -85,17 +86,30 @@ class ShellState(
|
|||
var contentFocus: ShellContentFocus? = null
|
||||
var inputRevision = 0L
|
||||
private set
|
||||
|
||||
init { preparePersonMemory() }
|
||||
|
||||
private fun preparePersonMemory() {
|
||||
personRequests.getOrPut(person.key) { ComingInState() }
|
||||
tabs.forEach(::retainView)
|
||||
retainView(route)
|
||||
}
|
||||
|
||||
private fun retainView(destination: String) {
|
||||
views.getOrPut(person.key to destination) { RememberedView() }
|
||||
}
|
||||
val tabs: List<String> get() = if (person.hasAnime) ANIME_TABS else BASE_TABS
|
||||
val currentTab: Int get() = tabs.indexOf(rootRoute).coerceAtLeast(0)
|
||||
val view: RememberedView get() = rememberedView(route)
|
||||
val navSavedAt: String? get() = if (requestsVisible && requests.snapshot.status == ComingInStatus.Unavailable) requests.snapshot.savedAt ?: view.savedAt else view.savedAt
|
||||
|
||||
/** Screen options share the same per-person memory as their root's focus and scroller. */
|
||||
/** Reads never register defaults: snapshotFlow evaluates accessors in a read-only snapshot. */
|
||||
fun rememberedView(destination: String): RememberedView =
|
||||
views.getOrPut(person.key to destination) { RememberedView() }
|
||||
views[person.key to destination] ?: RememberedView()
|
||||
|
||||
/** External intents and upstream pops remain authoritative over the screen. */
|
||||
fun enterDestination(destination: String) {
|
||||
retainView(destination)
|
||||
route = destination
|
||||
if (destination in tabs) rootRoute = destination
|
||||
handOff = null
|
||||
|
|
@ -159,6 +173,7 @@ class ShellState(
|
|||
pickerVisible = false
|
||||
navFocused = false
|
||||
settingsRow = 0
|
||||
retainView("settings")
|
||||
route = "settings"
|
||||
onAction(ShellAction.Destination(route))
|
||||
}
|
||||
|
|
@ -187,6 +202,7 @@ class ShellState(
|
|||
rootRoute = "home"
|
||||
requestReturnRoute = null
|
||||
requestsVisible = false
|
||||
preparePersonMemory()
|
||||
views[person.key to route] = RememberedView()
|
||||
onAction(ShellAction.Person(selected))
|
||||
}
|
||||
|
|
@ -226,6 +242,7 @@ class ShellState(
|
|||
} else {
|
||||
val target = tabs[navIndex]
|
||||
requestReturnRoute = null
|
||||
retainView(target)
|
||||
if (target == route) views[person.key to target] = RememberedView()
|
||||
route = target
|
||||
rootRoute = target
|
||||
|
|
|
|||
|
|
@ -0,0 +1,70 @@
|
|||
// SPDX-License-Identifier: GPL-2.0-only
|
||||
package org.justinvisser.wolfskin.design
|
||||
|
||||
import android.app.Application
|
||||
import androidx.compose.runtime.LaunchedEffect
|
||||
import androidx.compose.runtime.snapshotFlow
|
||||
import androidx.compose.runtime.snapshots.Snapshot
|
||||
import androidx.compose.ui.test.junit4.createComposeRule
|
||||
import kotlinx.coroutines.flow.first
|
||||
import org.junit.Assert.*
|
||||
import org.junit.Rule
|
||||
import org.junit.Test
|
||||
import org.junit.runner.RunWith
|
||||
import org.robolectric.RobolectricTestRunner
|
||||
import org.robolectric.annotation.Config
|
||||
|
||||
/** A first read must work without composition prewarming the person's route memory. */
|
||||
@RunWith(RobolectricTestRunner::class)
|
||||
@Config(sdk = [34], application = Application::class)
|
||||
class ShellSnapshotReadTest {
|
||||
@get:Rule val rule = createComposeRule()
|
||||
|
||||
@Test fun unseenViewCanBeReadInAReadOnlySnapshot() {
|
||||
val shell = ShellState(fixturePeople(), "justin")
|
||||
val snapshot = Snapshot.takeSnapshot()
|
||||
try {
|
||||
snapshot.enter {
|
||||
assertEquals(0, shell.view.row)
|
||||
assertTrue(shell.view.cards.isEmpty())
|
||||
assertNull(shell.view.savedAt)
|
||||
assertEquals(0, shell.rememberedView("never-seen").row)
|
||||
assertNotSame(shell.rememberedView("never-seen"), shell.rememberedView("never-seen"))
|
||||
}
|
||||
} finally { snapshot.dispose() }
|
||||
}
|
||||
|
||||
@Test fun launchedSnapshotFlowReadsDefaultsAndThenRetainsExplicitNavigationMemory() {
|
||||
val shell = ShellState(fixturePeople(), "justin")
|
||||
var observed: RememberedView? = null
|
||||
rule.setContent {
|
||||
LaunchedEffect(shell) { observed = snapshotFlow { shell.view }.first() }
|
||||
}
|
||||
rule.runOnIdle {
|
||||
val home = requireNotNull(observed)
|
||||
assertEquals(0, home.row)
|
||||
assertTrue(home.cards.isEmpty())
|
||||
home.row = 3
|
||||
home.cards["shelf"] = 2
|
||||
shell.enterDestination("movies")
|
||||
assertEquals(0, shell.view.row)
|
||||
shell.view.row = 5
|
||||
shell.enterDestination("home")
|
||||
assertSame(home, shell.view)
|
||||
assertEquals(3, shell.view.row)
|
||||
assertEquals(2, shell.view.cards["shelf"])
|
||||
shell.enterDestination("movies")
|
||||
assertEquals(5, shell.view.row)
|
||||
shell.focusPerson(1)
|
||||
shell.key(ShellKey.Ok)
|
||||
shell.enterDestination("movies")
|
||||
assertEquals(0, shell.view.row)
|
||||
shell.view.row = 7
|
||||
shell.pickerVisible = true
|
||||
shell.focusPerson(0)
|
||||
shell.key(ShellKey.Ok)
|
||||
shell.enterDestination("movies")
|
||||
assertEquals(5, shell.view.row)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -0,0 +1,46 @@
|
|||
// SPDX-License-Identifier: GPL-2.0-only
|
||||
package org.justinvisser.wolfskin.design
|
||||
|
||||
import java.io.File
|
||||
import org.junit.Assert.*
|
||||
import org.junit.Test
|
||||
|
||||
/** Guard known state writers; runtime tests cover transitive property reads. */
|
||||
class SnapshotReadAuditTest {
|
||||
@Test fun snapshotObserversDoNotCallKnownStateWriters() {
|
||||
val roots = listOf(File("src/main/java"), File("../app/src/wolfskin"))
|
||||
val violations = roots.flatMap { root ->
|
||||
assertTrue("Missing owned source root: $root", root.isDirectory)
|
||||
root.walkTopDown().filter { it.isFile && it.extension == "kt" }.flatMap { file ->
|
||||
observerBodies(file.readText()).filter { writingCalls.containsMatchIn(it) }.map { "$file: $it" }
|
||||
}.toList()
|
||||
}
|
||||
assertTrue(violations.joinToString("\n"), violations.isEmpty())
|
||||
}
|
||||
|
||||
@Test fun shellReadAccessorDoesNotInsertMemory() {
|
||||
val source = File("src/main/java/org/justinvisser/wolfskin/design/ShellState.kt").readText()
|
||||
val accessor = source.substringAfter("fun rememberedView(").substringBefore("/**")
|
||||
assertFalse("rememberedView must not insert into snapshot state", Regex("getOrPut|\\.put\\s*\\(|\\]\\s*=").containsMatchIn(accessor))
|
||||
}
|
||||
|
||||
@Test fun knownWriterIsRejectedWithoutRejectingItsCollector() {
|
||||
val unsafe = observerBodies("snapshotFlow { state.focus(2) }").single()
|
||||
assertTrue(writingCalls.containsMatchIn(unsafe))
|
||||
val safe = observerBodies("snapshotFlow { state.focusedLane?.id to state.column }.collect { state.focus(2) }").single()
|
||||
assertFalse(writingCalls.containsMatchIn(safe))
|
||||
}
|
||||
|
||||
private fun observerBodies(source: String): List<String> = Regex("snapshotFlow\\s*\\{").findAll(source).map { match ->
|
||||
val start = match.range.last + 1
|
||||
var depth = 1
|
||||
var end = start
|
||||
while (end < source.length && depth > 0) {
|
||||
when (source[end]) { '{' -> depth++; '}' -> depth-- }
|
||||
end++
|
||||
}
|
||||
source.substring(start, end - 1)
|
||||
}.toList()
|
||||
|
||||
private val writingCalls = Regex("\\b(?:retainView|preparePersonMemory|enterDestination|focus|updateSnapshot|updateLanes|invalidateLanes|updateRatings|recordCenter|getOrPut|put|putAll|clear|remove)\\s*\\(")
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue