Move person-switch client disposal off the main thread

app/src/test/java/org/justinvisser/wolfskin/tv/FakeConductarr.kt

@@ -11,6 +11,7 @@ import mockwebserver3.RecordedRequest
 
 internal const val SHARED_TOKEN = "0123456789abcdef0123456789abcdef"
 internal const val MINDEL_TOKEN = "fedcba9876543210fedcba9876543210"
+internal const val JUSTIN_TOKEN = "11111111111111111111111111111111"
 
 /** A loopback server supplies wire JSON, never app projections or live service state. */
 internal class FakeConductarr : Closeable {
@@ -75,6 +76,9 @@ internal class FakeConductarr : Closeable {
                 if (key == "operator" && request.headers["Authorization"]?.contains(MINDEL_TOKEN) == true) {
                     body = JsonObject(body + mapOf("name" to JsonPrimitive("Mindel"), "requester" to JsonPrimitive("mindel"), "subject" to JsonPrimitive("jellyfin:mindel"), "watch_user" to JsonPrimitive("mindel")))
                 }
+                if (key == "operator" && request.headers["Authorization"]?.contains(JUSTIN_TOKEN) == true) {
+                    body = JsonObject(body + mapOf("name" to JsonPrimitive("Justin"), "requester" to JsonPrimitive("justin"), "subject" to JsonPrimitive("jellyfin:justin"), "watch_user" to JsonPrimitive("justin")))
+                }
                 if (key in setOf("options", "discover") && request.url.queryParameter("media_type") == "tv") {
                     body = JsonObject(body + ("media_type" to JsonPrimitive("tv")))
                     if (key == "discover") body = JsonObject(body + ("items" to JsonArray(listOf(JsonObject(body.getValue("items").jsonArray.first().jsonObject + mapOf("media_type" to JsonPrimitive("tv"), "tmdb_id" to JsonPrimitive(44), "title" to JsonPrimitive("Dark")))))))

app/src/test/java/org/justinvisser/wolfskin/tv/PersonSwitchThreadingTest.kt

@@ -0,0 +1,252 @@
+package org.justinvisser.wolfskin.tv
+
+import android.app.Application
+import android.os.Looper
+import androidx.lifecycle.viewModelScope
+import androidx.test.core.app.ApplicationProvider
+import com.github.damontecres.wholphin.data.CurrentUser
+import com.github.damontecres.wholphin.data.ServerRepository
+import com.github.damontecres.wholphin.data.model.*
+import com.github.damontecres.wholphin.services.SetupDestination
+import com.github.damontecres.wholphin.services.SetupNavigationManager
+import io.mockk.*
+import java.util.UUID
+import java.util.concurrent.CopyOnWriteArrayList
+import java.util.concurrent.CountDownLatch
+import java.util.concurrent.TimeUnit
+import kotlinx.coroutines.*
+import kotlinx.coroutines.flow.last
+import kotlinx.coroutines.flow.first
+import kotlinx.coroutines.test.resetMain
+import kotlinx.coroutines.test.setMain
+import org.junit.After
+import org.junit.Assert.*
+import org.junit.Before
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.justinvisser.wolfskin.conductarr.*
+import org.justinvisser.wolfskin.design.ShellKey
+import org.justinvisser.wolfskin.tv.read.LibraryReadFacade
+import org.robolectric.RobolectricTestRunner
+import org.robolectric.annotation.Config
+
+/** Picker changes invalidate identity immediately without closing sockets on the UI thread. */
+@OptIn(ExperimentalCoroutinesApi::class)
+@RunWith(RobolectricTestRunner::class)
+@Config(sdk = [34], application = Application::class)
+class PersonSwitchThreadingTest {
+    private val context = ApplicationProvider.getApplicationContext<Application>()
+    private lateinit var previousUrl: String
+    private var previousFixtures = false
+
+    @Before fun setUp() {
+        Dispatchers.setMain(Dispatchers.Unconfined)
+        ConductarrPreference.initialize(context)
+        previousUrl = ConductarrPreference.baseUrl
+        previousFixtures = ConductarrPreference.fixtures
+        ConductarrPreference.updateFixtures(false)
+    }
+
+    @After fun tearDown() {
+        ConductarrPreference.updateBaseUrl(previousUrl)
+        ConductarrPreference.updateFixtures(previousFixtures)
+        Dispatchers.resetMain()
+        unmockkConstructor(ConductarrClient::class)
+    }
+
+    @Test fun pickerSwitchesKeepPersonDataAndDisposeEveryClientOffMainWithoutWaiting() = runBlocking {
+        assertSame(Looper.getMainLooper(), Looper.myLooper())
+        val closeThreads = CopyOnWriteArrayList<Boolean>()
+        val releaseCloses = CountDownLatch(1)
+        val closed = CountDownLatch(4)
+        val disposalErrors = CopyOnWriteArrayList<Throwable>()
+        val disposalScope = CoroutineScope(SupervisorJob() + Dispatchers.IO + CoroutineExceptionHandler { _, error -> disposalErrors.add(error) })
+        val failFirstClose = java.util.concurrent.atomic.AtomicBoolean(true)
+        mockkConstructor(ConductarrClient::class)
+        every { anyConstructed<ConductarrClient>().close() } answers {
+            val onMain = Looper.myLooper() == Looper.getMainLooper()
+            closeThreads.add(onMain)
+            check(!onMain) { "StrictMode: Conductarr socket close on main" }
+            check(releaseCloses.await(10, TimeUnit.SECONDS)) { "Picker waited for socket disposal" }
+            callOriginal()
+            closed.countDown()
+            if (failFirstClose.getAndSet(false)) throw java.io.IOException("Socket disposal fixture failure")
+        }
+        FakeConductarr().use { fake ->
+            ConductarrPreference.updateBaseUrl(fake.url)
+            val conductarr = ConductarrSession(MemoryCacheStore(), disposalScope)
+            val server = JellyfinServer(UUID.randomUUID(), "Jellyfin", fake.url, null)
+            val justin = JellyfinUser(id = UUID.randomUUID(), serverId = server.id, name = "Justin", accessToken = JUSTIN_TOKEN)
+            val users = listOf(justin, justin.copy(id = UUID.randomUUID(), name = "Mindel", accessToken = MINDEL_TOKEN),
+                justin.copy(id = UUID.randomUUID(), name = "Shared", accessToken = SHARED_TOKEN))
+            val repository = mockk<ServerRepository>(relaxed = true)
+            coEvery { repository.serverDao.getServer(server.id) } returns JellyfinServerUsers(server, users)
+            coEvery { repository.changeUser(server, any()) } coAnswers { CurrentUser(server, secondArg()) }
+            val setup = mockk<SetupNavigationManager>(relaxed = true)
+            val libraries = mockk<LibraryReadFacade>(relaxed = true)
+            coEvery { libraries.readDestinations(any()) } returns emptyMap()
+            val session = WolfskinSession()
+            val model = WolfskinShellViewModel(context, session, repository, mockk(relaxed = true), setup, mockk(relaxed = true),
+                libraries, WolfskinSeeAllLists(), conductarr, mockk(relaxed = true))
+            val clients = mutableListOf<ConductarrClient>()
+            try {
+                model.load(server, justin)
+                requireNotNull(model.onForeground(server, justin)).join()
+                clients.add(conductarr.identity.value!!.client)
+                val shell = session.state!!
+                shell.key(ShellKey.Ok)
+                for (index in listOf(1, 2, 0)) {
+                    val switched = CompletableDeferred<CurrentUser>()
+                    every { setup.navigateTo(any()) } answers { switched.complete((firstArg() as SetupDestination.AppContent).current) }
+                    shell.focusNav(shell.tabs.size)
+                    shell.key(ShellKey.Ok)
+                    shell.focusPerson(index)
+                    shell.key(ShellKey.Ok)
+                    assertEquals(users[index].id.toString(), shell.person.jellyfinUserId)
+                    assertEquals("home", shell.route)
+                    assertFalse(shell.pickerVisible)
+                    assertNull("Old identity must not serve the newly selected person", conductarr.identity.value)
+                    val current = withTimeout(5_000) { switched.await() }
+                    assertEquals(users[index].id, current.user.id)
+                    model.load(current.server, current.user)
+                    requireNotNull(model.onForeground(server, current.user)).join()
+                    assertEquals(users[index].name!!.lowercase(), conductarr.identity.value!!.operator!!.requester)
+                    assertEquals(users[index].name!!.lowercase(), model.signedInPerson)
+                    clients.add(conductarr.identity.value!!.client)
+                }
+                conductarr.close()
+                conductarr.close()
+                model.viewModelScope.cancel()
+                releaseCloses.countDown()
+                assertTrue("Every retired client must finish disposal", withContext(Dispatchers.IO) { closed.await(5, TimeUnit.SECONDS) })
+                assertEquals(listOf(false, false, false, false), closeThreads.toList())
+                for (client in clients) {
+                    val result = client.read(TvRoutes.operator(), ReadTrigger.FORCE).last()
+                    assertEquals("Retired clients cannot start another network read", ClientError.Unreachable, (result as ReadResult.Failure).error)
+                }
+                disposalScope.coroutineContext.job.children.toList().joinAll()
+                assertTrue("A failing socket close must be contained", disposalErrors.isEmpty())
+            } finally {
+                releaseCloses.countDown()
+                model.viewModelScope.cancel()
+                withContext(Dispatchers.IO) { conductarr.close() }
+                disposalScope.coroutineContext.job.children.toList().joinAll()
+                disposalScope.cancel()
+            }
+        }
+    }
+
+    @Test fun rapidPickerChoicesIgnoreLateSignInAndCanReturnToTheOriginalPerson() = runBlocking {
+        FakeConductarr().use { fake ->
+            ConductarrPreference.updateBaseUrl(fake.url)
+            ConductarrSession(MemoryCacheStore()).use { conductarr ->
+                val server = JellyfinServer(UUID.randomUUID(), "Jellyfin", fake.url, null)
+                val justin = JellyfinUser(id = UUID.randomUUID(), serverId = server.id, name = "Justin", accessToken = JUSTIN_TOKEN)
+                val users = listOf(justin, justin.copy(id = UUID.randomUUID(), name = "Mindel", accessToken = MINDEL_TOKEN),
+                    justin.copy(id = UUID.randomUUID(), name = "Shared", accessToken = SHARED_TOKEN))
+                val repository = mockk<ServerRepository>(relaxed = true)
+                coEvery { repository.serverDao.getServer(server.id) } returns JellyfinServerUsers(server, users)
+                val pending = CompletableDeferred<Unit>()
+                val release = CompletableDeferred<Unit>()
+                coEvery { repository.changeUser(server, any()) } coAnswers {
+                    val user = secondArg<JellyfinUser>()
+                    if (user.id == users[1].id) {
+                        pending.complete(Unit)
+                        withContext(NonCancellable) { release.await() }
+                    }
+                    CurrentUser(server, user)
+                }
+                val switched = CompletableDeferred<CurrentUser>()
+                val setup = mockk<SetupNavigationManager>(relaxed = true)
+                every { setup.navigateTo(any()) } answers { switched.complete((firstArg() as SetupDestination.AppContent).current) }
+                val libraries = mockk<LibraryReadFacade>(relaxed = true)
+                coEvery { libraries.readDestinations(any()) } returns emptyMap()
+                val session = WolfskinSession()
+                val model = WolfskinShellViewModel(context, session, repository, mockk(relaxed = true), setup, mockk(relaxed = true),
+                    libraries, WolfskinSeeAllLists(), conductarr, mockk(relaxed = true))
+                try {
+                    model.load(server, justin)
+                    requireNotNull(model.onForeground(server, justin)).join()
+                    val shell = session.state!!
+                    shell.key(ShellKey.Ok)
+                    for (index in listOf(1, 2, 0)) {
+                        shell.focusNav(shell.tabs.size)
+                        shell.key(ShellKey.Ok)
+                        shell.focusPerson(index)
+                        shell.key(ShellKey.Ok)
+                        if (index == 1) withTimeout(5_000) { pending.await() }
+                    }
+                    release.complete(Unit)
+                    val current = withTimeout(5_000) { switched.await() }
+                    assertEquals(justin.id, current.user.id)
+                    verify(exactly = 1) { setup.navigateTo(any()) }
+                    model.load(server, current.user)
+                    requireNotNull(model.onForeground(server, current.user)).join()
+                    assertEquals(justin.id.toString(), shell.person.jellyfinUserId)
+                    assertEquals("justin", model.signedInPerson)
+                    assertEquals("justin", conductarr.identity.value!!.operator!!.requester)
+                } finally {
+                    release.complete(Unit)
+                    model.viewModelScope.cancel()
+                }
+            }
+        }
+    }
+
+    @Test fun homeAndTitleRetriesReplaceSignedOutClientsWithoutMainThreadDisposal() = runBlocking {
+        val closeThreads = CopyOnWriteArrayList<Boolean>()
+        val closed = CountDownLatch(3)
+        val disposalScope = CoroutineScope(SupervisorJob() + Dispatchers.IO)
+        mockkConstructor(ConductarrClient::class)
+        every { anyConstructed<ConductarrClient>().close() } answers {
+            val onMain = Looper.myLooper() == Looper.getMainLooper()
+            closeThreads.add(onMain)
+            check(!onMain) { "StrictMode: retry socket close on main" }
+            callOriginal()
+            closed.countDown()
+        }
+        FakeConductarr().use { fake ->
+            ConductarrPreference.updateBaseUrl(fake.url)
+            val conductarr = ConductarrSession(MemoryCacheStore(), disposalScope)
+            val server = JellyfinServer(UUID.randomUUID(), "Jellyfin", fake.url, null)
+            val user = JellyfinUser(id = UUID.randomUUID(), serverId = server.id, name = "Justin", accessToken = JUSTIN_TOKEN)
+            val repository = mockk<ServerRepository>()
+            every { repository.currentUser } returns user
+            every { repository.currentServer } returns server
+            val home = mockk<org.justinvisser.wolfskin.tv.read.HomeReadFacade>()
+            coEvery { home.read(user.id) } returns org.justinvisser.wolfskin.design.HomeLibrary(resume = listOf(
+                org.justinvisser.wolfskin.design.HomeTitle("justin-film", "Justin film", "movie", held = true)))
+            val cache = mockk<org.justinvisser.wolfskin.tv.read.HomeLibraryCache>(relaxed = true)
+            every { cache.read(any()) } returns null
+            val homeModel = WolfskinHomeViewModel(context, home, cache, mockk(relaxed = true), WolfskinSeeAllLists(), conductarr)
+            val titleModel = WolfskinTitleViewModel(context, mockk(), mockk(), mockk(relaxed = true), conductarr, repository)
+            try {
+                conductarr.select("${server.id}:${user.id}", user.accessToken, fake.url)
+                conductarr.authenticate()
+                fake.status = 401
+                conductarr.authenticate()
+                assertFalse(conductarr.identity.value!!.signedIn)
+                fake.status = 200
+                homeModel.load(server, user, org.justinvisser.wolfskin.design.ShellPerson("justin", "Justin", "J", false, user.id.toString()), retry = true)
+                withTimeout(5_000) { conductarr.identity.first { it?.signedIn == true } }
+                assertEquals("justin", conductarr.identity.value!!.operator!!.requester)
+                assertTrue(homeModel.snapshot.rows.any { row -> row.cards.any { it.id == "justin-film" } })
+                fake.status = 401
+                conductarr.authenticate()
+                assertFalse(conductarr.identity.value!!.signedIn)
+                fake.status = 200
+                titleModel.retrySignIn()
+                assertEquals("justin", conductarr.identity.value!!.operator!!.requester)
+                assertTrue(conductarr.identity.value!!.signedIn)
+            } finally {
+                homeModel.viewModelScope.cancel()
+                titleModel.viewModelScope.cancel()
+                conductarr.close()
+                disposalScope.coroutineContext.job.children.toList().joinAll()
+                disposalScope.cancel()
+            }
+            assertEquals(0L, closed.count)
+            assertEquals(listOf(false, false, false), closeThreads.toList())
+        }
+    }
+}

app/src/test/java/org/justinvisser/wolfskin/tv/ShellStartupTimingTest.kt

@@ -58,8 +58,9 @@ class ShellStartupTimingTest {
                 val activity = async(Dispatchers.IO) { conductarr.identity.value!!.provider.requests().last() }
                 withTimeout(5_000) { activityEntered.await() }
                 val savedPeople = CompletableDeferred<Unit>()
+                val savedPeopleStarted = CompletableDeferred<Unit>()
                 val repository = mockk<ServerRepository>(relaxed = true)
-                coEvery { repository.serverDao.getServer(server.id) } coAnswers { savedPeople.await(); null }
+                coEvery { repository.serverDao.getServer(server.id) } coAnswers { savedPeopleStarted.complete(Unit); savedPeople.await(); null }
                 val navigation = mockk<NavigationManager>(relaxed = true)
                 val libraries = mockk<LibraryReadFacade>(relaxed = true)
                 coEvery { libraries.readDestinations(user.id) } returns emptyMap()
@@ -68,7 +69,7 @@ class ShellStartupTimingTest {
                     libraries, WolfskinSeeAllLists(), conductarr, mockk(relaxed = true))
                 val load = launch { model.load(server, user) }
                 try {
-                    yield()
+                    withTimeout(5_000) { savedPeopleStarted.await() }
                     val shell = requireNotNull(session.state) { "The current Jellyfin person must paint before saved-login I/O" }
                     assertEquals(PeopleStatus.Ready, shell.peopleStatus)
                     assertEquals(user.id.toString(), shell.person.jellyfinUserId)

app/src/test/java/org/justinvisser/wolfskin/tv/TitleAsyncFocusTest.kt

@@ -1,6 +1,7 @@
 package org.justinvisser.wolfskin.tv
 
 import android.app.Application
+import android.os.Looper
 import androidx.activity.ComponentActivity
 import androidx.compose.ui.input.key.Key
 import androidx.compose.ui.test.junit4.createAndroidComposeRule
@@ -35,6 +36,7 @@ import org.justinvisser.wolfskin.design.TitleVersion
 import org.justinvisser.wolfskin.design.WolfskinTheme
 import org.justinvisser.wolfskin.tv.read.TitleReadFacade
 import org.robolectric.RobolectricTestRunner
+import org.robolectric.Shadows
 import org.robolectric.annotation.Config
 import java.util.UUID
 
@@ -152,7 +154,10 @@ class TitleAsyncFocusTest {
         assertEquals("retry", viewModel.title!!.options[viewModel.title!!.column].id)
         rule.runOnIdle { viewModel.title!!.toast = null }
         press(Key.DirectionCenter)
-        rule.waitUntil(timeoutMillis = 5000) { reads.get() == 2 && viewModel.title?.toast != null }
+        rule.waitUntil(timeoutMillis = 5000) {
+            Shadows.shadowOf(Looper.getMainLooper()).idle()
+            reads.get() == 2 && viewModel.title?.toast != null
+        }
         rule.waitForIdle()
         assertEquals("Saved title", viewModel.title!!.data.item.name)
         assertEquals("20:42", shell.navSavedAt)