diff --git a/app/src/main/kotlin/app/getarcane/android/ui/screens/containers/ContainerListScreen.kt b/app/src/main/kotlin/app/getarcane/android/ui/screens/containers/ContainerListScreen.kt index 9f1fa8c..832c72d 100644 --- a/app/src/main/kotlin/app/getarcane/android/ui/screens/containers/ContainerListScreen.kt +++ b/app/src/main/kotlin/app/getarcane/android/ui/screens/containers/ContainerListScreen.kt @@ -68,9 +68,15 @@ import app.getarcane.android.ui.theme.StatusRunning import app.getarcane.android.ui.theme.StatusUnknown import app.getarcane.sdk.models.base.SearchPaginationSort import app.getarcane.sdk.models.container.ContainerSummary +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.launch -private enum class StateFilter(val label: String) { All("All"), Running("Running"), Stopped("Stopped") } +private val ContainerStateFilter.label: String + get() = when (this) { + ContainerStateFilter.All -> "All" + ContainerStateFilter.Running -> "Running" + ContainerStateFilter.Stopped -> "Stopped" + } @OptIn(ExperimentalMaterial3Api::class) @Composable @@ -81,29 +87,40 @@ fun ContainerListScreen(onOpen: (String) -> Unit) { val envId = manager.activeEnvironmentId val scope = rememberCoroutineScope() - var state by remember { mutableStateOf>>(Loadable.Loading) } + var loadState by remember { + mutableStateOf(ContainerListLoadState>()) + } var search by remember { mutableStateOf("") } var sortAsc by remember { mutableStateOf(true) } - var filter by remember { mutableStateOf(StateFilter.All) } + var filter by remember { mutableStateOf(ContainerStateFilter.All) } var updateFilter by remember { mutableStateOf(ResourceUpdateFilter.ALL) } var refreshKey by remember { mutableStateOf(0) } - var refreshing by remember { mutableStateOf(false) } var menuOpen by remember { mutableStateOf(false) } LaunchedEffect(envId.rawValue, refreshKey) { if (client == null) return@LaunchedEffect - if (state !is Loadable.Success) state = Loadable.Loading - state = try { - Loadable.Success( - client.containers.list( - envId = envId, - query = SearchPaginationSort(start = 0, limit = -1), - ).data, + loadState = beginContainerReload(loadState) + loadState = try { + completeContainerLoad( + loadCompleteContainerCollection( + idOf = { it.id }, + loadAll = { + val response = client.containers.list( + envId = envId, + query = SearchPaginationSort(start = 0, limit = -1), + ) + CompleteContainerResponse( + items = response.data, + totalItems = response.pagination.totalItems, + ) + }, + ), ) + } catch (e: CancellationException) { + throw e } catch (e: Throwable) { - Loadable.Error(friendlyErrorMessage(e)) + failContainerLoad(friendlyErrorMessage(e)) } - refreshing = false } fun act(block: suspend () -> Unit) { @@ -124,7 +141,7 @@ fun ContainerListScreen(onOpen: (String) -> Unit) { DropdownMenuItem(text = { Text("Z–A") }, onClick = { sortAsc = false; menuOpen = false }, leadingIcon = { if (!sortAsc) Icon(Icons.AutoMirrored.Filled.Sort, null) }) HorizontalDivider() Text("Filter", style = MaterialTheme.typography.labelSmall, modifier = Modifier.padding(start = 12.dp, top = 8.dp)) - StateFilter.entries.forEach { f -> + ContainerStateFilter.entries.forEach { f -> DropdownMenuItem(text = { Text(f.label) }, onClick = { filter = f; menuOpen = false }, trailingIcon = { if (filter == f) Icon(Icons.AutoMirrored.Filled.Sort, null) }) } HorizontalDivider() @@ -148,31 +165,31 @@ fun ContainerListScreen(onOpen: (String) -> Unit) { modifier = Modifier.fillMaxWidth().padding(horizontal = 16.dp, vertical = 8.dp), ) PullToRefreshBox( - isRefreshing = refreshing, - onRefresh = { refreshing = true; refreshKey++ }, + isRefreshing = loadState.refreshing, + onRefresh = { + loadState = beginContainerRefresh(loadState) + refreshKey++ + }, modifier = Modifier.fillMaxSize(), ) { - when (val s = state) { + when (val s = loadState.content) { is Loadable.Loading -> SkeletonListLoadingView() is Loadable.Error -> ContentUnavailable("Error", Icons.Outlined.Inventory2, s.message, "Refresh") { refreshKey++ } is Loadable.Success -> { val pinnedIds = pinned.pinnedIds(PinnedItemsStore.Kind.CONTAINER, envId) - val filtered = s.value.filter { c -> - (filter == StateFilter.All || - (filter == StateFilter.Running && c.isRunning) || - (filter == StateFilter.Stopped && !c.isRunning)) && - updateFilter.matches(c.hasAvailableUpdate) && - (search.isBlank() || - c.names.any { it.contains(search, true) } || - c.image.contains(search, true)) - }.sortedWith(compareBy(String.CASE_INSENSITIVE_ORDER) { it.displayName }) - .let { if (sortAsc) it else it.reversed() } + val filtered = filterAndSortContainers( + containers = s.value, + search = search, + stateFilter = filter, + updateFilter = updateFilter, + sortAscending = sortAsc, + ) val pinnedItems = filtered.filter { it.id in pinnedIds } val running = filtered.filter { it.id !in pinnedIds && it.isRunning } val stopped = filtered.filter { it.id !in pinnedIds && !it.isRunning } - if (filtered.isEmpty() && search.isBlank() && filter == StateFilter.All && updateFilter == ResourceUpdateFilter.ALL) { + if (filtered.isEmpty() && search.isBlank() && filter == ContainerStateFilter.All && updateFilter == ResourceUpdateFilter.ALL) { ContentUnavailable("No Containers", Icons.Outlined.Inventory2, "No containers found in this environment.", "Refresh") { refreshKey++ } } else { LazyColumn(Modifier.fillMaxSize(), contentPadding = androidx.compose.foundation.layout.PaddingValues(bottom = 16.dp)) { diff --git a/app/src/main/kotlin/app/getarcane/android/ui/screens/containers/ContainerPagination.kt b/app/src/main/kotlin/app/getarcane/android/ui/screens/containers/ContainerPagination.kt new file mode 100644 index 0000000..fd9c751 --- /dev/null +++ b/app/src/main/kotlin/app/getarcane/android/ui/screens/containers/ContainerPagination.kt @@ -0,0 +1,88 @@ +package app.getarcane.android.ui.screens.containers + +import app.getarcane.android.core.Loadable +import app.getarcane.android.core.ResourceUpdateFilter +import app.getarcane.android.core.displayName +import app.getarcane.android.core.hasAvailableUpdate +import app.getarcane.android.core.isRunning +import app.getarcane.sdk.models.container.ContainerSummary + +internal data class CompleteContainerResponse( + val items: List, + val totalItems: Long, +) + +internal class IncompleteContainerCollectionException(message: String) : IllegalStateException(message) + +/** + * Normalizes the server's documented single-response "show all" result. The Arcane container + * endpoint has no unique sort binding, so offset traversal cannot guarantee a stable collection + * when containers change or share the same supported sort value. + */ +internal suspend fun loadCompleteContainerCollection( + idOf: (T) -> String, + loadAll: suspend () -> CompleteContainerResponse, +): List { + val response = loadAll() + val itemsById = linkedMapOf() + response.items.forEach { item -> itemsById.putIfAbsent(idOf(item), item) } + + val rawItemCount = response.items.size.toLong() + val uniqueItemCount = itemsById.size.toLong() + if (response.totalItems >= 0 && + response.totalItems != rawItemCount && + response.totalItems != uniqueItemCount + ) { + throw IncompleteContainerCollectionException( + "Container response counts: raw=$rawItemCount, unique=$uniqueItemCount; " + + "server reported ${response.totalItems}", + ) + } + return itemsById.values.toList() +} + +internal data class ContainerListLoadState( + val content: Loadable = Loadable.Loading, + val refreshing: Boolean = false, +) + +internal fun beginContainerReload( + state: ContainerListLoadState, +): ContainerListLoadState = + if (state.content is Loadable.Success) state else state.copy(content = Loadable.Loading) + +internal fun beginContainerRefresh( + state: ContainerListLoadState, +): ContainerListLoadState = + state.copy( + content = if (state.content is Loadable.Success) state.content else Loadable.Loading, + refreshing = true, + ) + +internal fun completeContainerLoad(value: T): ContainerListLoadState = + ContainerListLoadState(content = Loadable.Success(value)) + +internal fun failContainerLoad(message: String): ContainerListLoadState = + ContainerListLoadState(content = Loadable.Error(message)) + +internal enum class ContainerStateFilter { All, Running, Stopped } + +internal fun filterAndSortContainers( + containers: List, + search: String, + stateFilter: ContainerStateFilter, + updateFilter: ResourceUpdateFilter, + sortAscending: Boolean, +): List { + val filtered = containers.filter { container -> + (stateFilter == ContainerStateFilter.All || + (stateFilter == ContainerStateFilter.Running && container.isRunning) || + (stateFilter == ContainerStateFilter.Stopped && !container.isRunning)) && + updateFilter.matches(container.hasAvailableUpdate) && + (search.isBlank() || + container.names.any { it.contains(search, ignoreCase = true) } || + container.image.contains(search, ignoreCase = true)) + }.sortedWith(compareBy(String.CASE_INSENSITIVE_ORDER) { it.displayName }) + + return if (sortAscending) filtered else filtered.reversed() +} diff --git a/app/src/test/java/app/getarcane/android/ui/screens/containers/ContainerPaginationTest.kt b/app/src/test/java/app/getarcane/android/ui/screens/containers/ContainerPaginationTest.kt new file mode 100644 index 0000000..4e93ac0 --- /dev/null +++ b/app/src/test/java/app/getarcane/android/ui/screens/containers/ContainerPaginationTest.kt @@ -0,0 +1,257 @@ +package app.getarcane.android.ui.screens.containers + +import app.getarcane.android.core.Loadable +import app.getarcane.android.core.ResourceUpdateFilter +import app.getarcane.sdk.models.container.ContainerHostConfig +import app.getarcane.sdk.models.container.ContainerNetworkSettings +import app.getarcane.sdk.models.container.ContainerSummary +import app.getarcane.sdk.models.image.ImageUpdateInfo +import java.io.IOException +import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.runBlocking +import kotlinx.datetime.Instant +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertSame +import org.junit.Assert.assertThrows +import org.junit.Assert.assertTrue +import org.junit.Test + +class ContainerPaginationTest { + @Test + fun `show-all contract loads more than twenty containers in one finite request`() = runBlocking { + val source = (0 until 225).map { "container-$it" } + var requestCount = 0 + + val result = loadCompleteContainerCollection(idOf = { it }) { + requestCount++ + CompleteContainerResponse(source, totalItems = source.size.toLong()) + } + + assertEquals(source, result) + assertEquals(1, requestCount) + } + + @Test + fun `duplicate IDs are removed when reported total counts raw items`() = runBlocking { + var requestCount = 0 + + val result = loadCompleteContainerCollection(idOf = { it }) { + requestCount++ + CompleteContainerResponse( + items = listOf("container-1", "container-2", "container-2"), + totalItems = 3, + ) + } + + assertEquals(listOf("container-1", "container-2"), result) + assertEquals(1, requestCount) + } + + @Test + fun `duplicate IDs are removed when reported total counts unique items`() = runBlocking { + val result = loadCompleteContainerCollection(idOf = { it }) { + CompleteContainerResponse( + items = listOf("container-1", "container-2", "container-2"), + totalItems = 2, + ) + } + + assertEquals(listOf("container-1", "container-2"), result) + } + + @Test + fun `reported total mismatch fails instead of publishing an incomplete result`() { + val error = assertThrows(IncompleteContainerCollectionException::class.java) { + runBlocking { + loadCompleteContainerCollection(idOf = { it }) { + CompleteContainerResponse(listOf("container-1"), totalItems = 2) + } + } + } + + assertEquals( + "Container response counts: raw=1, unique=1; server reported 2", + error.message, + ) + } + + @Test + fun `empty complete collection terminates after one request`() = runBlocking { + var requestCount = 0 + + val result = loadCompleteContainerCollection(idOf = { it }) { + requestCount++ + CompleteContainerResponse(emptyList(), totalItems = 0) + } + + assertEquals(emptyList(), result) + assertEquals(1, requestCount) + } + + @Test + fun `failure is propagated without a partial result`() { + val expected = IOException("load failed") + + val actual = assertThrows(IOException::class.java) { + runBlocking { + loadCompleteContainerCollection(idOf = { it }) { + throw expected + } + } + } + + assertSame(expected, actual) + } + + @Test + fun `cancellation is propagated immediately`() { + val expected = CancellationException("cancelled") + + val actual = assertThrows(CancellationException::class.java) { + runBlocking { + loadCompleteContainerCollection(idOf = { it }) { + throw expected + } + } + } + + assertSame(expected, actual) + } + + @Test + fun `search and status filters see a match outside the former first page`() = runBlocking { + val containers = (0 until 125).map { index -> + container( + id = "container-$index", + name = if (index == 124) "late-search-match" else "container-$index", + state = if (index == 124) "running" else "exited", + ) + } + val complete = loadCompleteContainerCollection(idOf = ContainerSummary::id) { + CompleteContainerResponse(containers, totalItems = containers.size.toLong()) + } + + val result = filterAndSortContainers( + containers = complete, + search = "late-search", + stateFilter = ContainerStateFilter.Running, + updateFilter = ResourceUpdateFilter.ALL, + sortAscending = true, + ) + + assertEquals(listOf("container-124"), result.map { it.id }) + } + + @Test + fun `update filter sees a match outside the former first page`() = runBlocking { + val containers = (0 until 125).map { index -> + container( + id = "container-$index", + name = "container-$index", + state = "running", + hasUpdate = index == 124, + ) + } + val complete = loadCompleteContainerCollection(idOf = ContainerSummary::id) { + CompleteContainerResponse(containers, totalItems = containers.size.toLong()) + } + + val result = filterAndSortContainers( + containers = complete, + search = "", + stateFilter = ContainerStateFilter.All, + updateFilter = ResourceUpdateFilter.HAS_UPDATES, + sortAscending = true, + ) + + assertEquals(listOf("container-124"), result.map { it.id }) + } + + @Test + fun `initial state and reload expose loading without refresh`() { + val initial = ContainerListLoadState>() + val afterError = ContainerListLoadState>( + content = Loadable.Error("old error"), + ) + + assertTrue(initial.content is Loadable.Loading) + assertFalse(initial.refreshing) + assertTrue(beginContainerReload(afterError).content is Loadable.Loading) + } + + @Test + fun `refresh retains prior complete data while loading`() { + val complete = completeContainerLoad(listOf("container-1")) + + val refreshing = beginContainerRefresh(complete) + + assertEquals( + listOf("container-1"), + (refreshing.content as Loadable.Success).value, + ) + assertTrue(refreshing.refreshing) + } + + @Test + fun `refresh failure replaces prior content and clears refreshing`() { + val refreshing = beginContainerRefresh(completeContainerLoad(listOf("container-1"))) + + val failed = failContainerLoad>("refresh failed") + + assertTrue(refreshing.refreshing) + assertEquals("refresh failed", (failed.content as Loadable.Error).message) + assertFalse(failed.refreshing) + } + + @Test + fun `initial failure exposes error and clears loading indicators`() { + val failed = failContainerLoad>("initial failed") + + assertEquals("initial failed", (failed.content as Loadable.Error).message) + assertFalse(failed.refreshing) + } + + @Test + fun `successful empty load is distinct from loading and failure`() { + val empty = completeContainerLoad(emptyList()) + + assertEquals(emptyList(), (empty.content as Loadable.Success).value) + assertFalse(empty.refreshing) + } + + private fun container( + id: String, + name: String, + state: String, + hasUpdate: Boolean = false, + ): ContainerSummary = + ContainerSummary( + id = id, + names = listOf(name), + image = "example/image:latest", + imageId = "image-$id", + command = "", + created = 0, + ports = emptyList(), + state = state, + status = state, + hostConfig = ContainerHostConfig(), + networkSettings = ContainerNetworkSettings(), + mounts = emptyList(), + updateInfo = if (hasUpdate) updateInfo() else null, + ) + + private fun updateInfo(): ImageUpdateInfo = + ImageUpdateInfo( + hasUpdate = true, + updateType = "digest", + currentVersion = "1", + latestVersion = "2", + currentDigest = "sha256:current", + latestDigest = "sha256:latest", + checkTime = Instant.fromEpochMilliseconds(0), + responseTimeMs = 1, + error = "", + ) +} diff --git a/docs/ios-parity-task-list.md b/docs/ios-parity-task-list.md index 6015a9a..dde67a2 100644 --- a/docs/ios-parity-task-list.md +++ b/docs/ios-parity-task-list.md @@ -25,6 +25,7 @@ SDK, and Arcane server revisions in the resulting issue or pull request. | **Blocked/Hold** | Do not implement until the named external dependency or product decision changes. | | **Deferred** | Deliberately sequenced behind foundation work or not required for current parity. | | **Done/verify** | Later notes suggest the work progressed or landed; confirm current behavior and close or reopen with new evidence. | +| **Complete** | Required implementation, acceptance criteria, and validation evidence are complete. | Priorities are **P0** correctness/security, **P1** high-frequency workflow parity, **P2** resilience/native continuity, and **P3** maturity or optional expansion. Dependencies name task IDs; @@ -103,22 +104,39 @@ The standard checks are: recreation. - [ ] Device testing confirms no prior-server data flashes or actions remain available. -- [ ] **PAR-003 — Fix complete-container loading before local filtering** +- [x] **PAR-003 — Fix complete-container loading before local filtering** -- **Status:** Ready +- **Status:** Complete - **Priority:** P0 - **Dependencies:** None - **Scope:** Make the Containers tab filter a complete result set rather than the SDK's default first page of 20. Do not assume `limit = -1` is supported: inspect the Arcane handler and SDK semantics, then use explicit paging unless an unlimited query is documented and safely bounded. - **Acceptance criteria:** - - [ ] The server and SDK behavior for page size, start, limit, ordering, and terminal-page detection is + - [x] The server and SDK behavior for page size, start, limit, ordering, and terminal-page detection is documented in focused tests or issue evidence. - - [ ] An environment with more than 20 containers displays and filters across the full set without + - [x] An environment with more than 20 containers displays and filters across the full set without duplicates, omissions, or infinite requests. - - [ ] Search/status filters are proven to run after complete loading, or are moved server-side with + - [x] Search/status filters are proven to run after complete loading, or are moved server-side with equivalent semantics. - - [ ] Loading, partial-page failure, refresh, cancellation, and empty states are covered. + - [x] Loading, partial-page failure, refresh, cancellation, and empty states are covered. +- **Completion evidence (2026-07-17):** + - Review: draft PR [#41](https://github.com/getarcaneapp/android/pull/41); automated validation + is complete and focused manual device validation is pending. + - Source pins: Android base `ca211804fcb3223b7b65abb0d13a97afad81799e`, + libarcane-kotlin `89c8dd58886a099cdbea9cb9362c9262ba5851d9`, and Arcane + `b501c49cc9f3d3433494f8334178ac65a59a013d`. + - The SDK forwards `start` and `limit`; Arcane defaults them to `0` and `20`, documents + `limit = -1` as one-page "show all", bypasses offset slicing for that value, and reports the complete + `totalItems`. The app therefore uses one finite show-all request, de-duplicates by container ID, + validates the unique count before publishing, and only then applies local search and filters. + This avoids offset traversal across a changing collection whose supported sort keys have no + unique secondary ordering. + - `.\gradlew.bat :app:testDebugUnitTest --tests + "app.getarcane.android.ui.screens.containers.ContainerPaginationTest" --rerun-tasks` passed all + 14 focused tests (0 failures, 0 errors, 0 skipped). + - `.\gradlew.bat :app:testDebugUnitTest :app:assembleDebug` passed all 98 unit tests and produced + the debug APK; `git diff --check` passed. - [ ] **PAR-004 — Audit all complete-list call sites for silent pagination truncation** @@ -128,6 +146,10 @@ The standard checks are: - **Scope:** Inventory every list call whose UI or calculation claims fleet-wide or complete results. Prioritize environments, dashboard totals/cards, updates, all-environment image updates, and environment management. +- **PAR-003 follow-up:** Evaluate a reusable complete-list pattern that selects an endpoint-supported + show-all request or stable offset traversal and consistently handles count validation, + de-duplication, cancellation, and atomic failure. Keep call-site inventory and changes within + PAR-004. - **Acceptance criteria:** - [ ] A checked inventory records each caller as intentionally paged, intentionally bounded, or fixed. - [ ] All complete-environment callers work with more than 20 environments.