fix(history): guard fluid pagination against infinite network loop on small session lists (#1002)
Co-authored-by: M57 <hy4ri@users.noreply.github.com>
This commit is contained in:
1 parent
2332bdd5d5
commit
7a2c453f58
3 files changed
+62
-24
No files matched your search
@@ -768,6 +768,7 @@ fun SessionsScreen(
|
|||||||
val info = listState.layoutInfo
|
val info = listState.layoutInfo
|
||||||
val lastVisible = info.visibleItemsInfo.lastOrNull()?.index ?: -1
|
val lastVisible = info.visibleItemsInfo.lastOrNull()?.index ?: -1
|
||||||
lastVisible >= 0 &&
|
lastVisible >= 0 &&
|
||||||
|
info.totalItemsCount >= AUTO_LOAD_THRESHOLD &&
|
||||||
lastVisible >= info.totalItemsCount - AUTO_LOAD_THRESHOLD
|
lastVisible >= info.totalItemsCount - AUTO_LOAD_THRESHOLD
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -64,6 +64,7 @@ data class SessionsUiState(
|
|||||||
val isLoadingMore: Boolean = false,
|
val isLoadingMore: Boolean = false,
|
||||||
val sessions: List<SessionInfo> = emptyList(),
|
val sessions: List<SessionInfo> = emptyList(),
|
||||||
val total: Int = 0,
|
val total: Int = 0,
|
||||||
|
val hasMore: Boolean = false,
|
||||||
val errorMessage: String? = null,
|
val errorMessage: String? = null,
|
||||||
val stats: SessionStats = SessionStats(),
|
val stats: SessionStats = SessionStats(),
|
||||||
val isLoadingStats: Boolean = false,
|
val isLoadingStats: Boolean = false,
|
||||||
@@ -89,7 +90,6 @@ data class SessionsUiState(
|
|||||||
val searchResults: List<SessionSearchResult> = emptyList(),
|
val searchResults: List<SessionSearchResult> = emptyList(),
|
||||||
val searchError: String? = null,
|
val searchError: String? = null,
|
||||||
) {
|
) {
|
||||||
val hasMore: Boolean get() = total > sessions.size
|
|
||||||
val isSearchMode: Boolean get() = searchQuery.isNotBlank()
|
val isSearchMode: Boolean get() = searchQuery.isNotBlank()
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -118,9 +118,12 @@ class SessionsViewModel :
|
|||||||
},
|
},
|
||||||
onSuccess = { data ->
|
onSuccess = { data ->
|
||||||
_uiState.update {
|
_uiState.update {
|
||||||
|
val newSessions = data.sessions.orEmpty().pinnedFirst()
|
||||||
|
val hasMore = data.sessions.size >= PAGE_SIZE && data.total > newSessions.size
|
||||||
it.copy(
|
it.copy(
|
||||||
sessions = data.sessions.orEmpty().pinnedFirst(),
|
sessions = newSessions,
|
||||||
total = data.total,
|
total = if (hasMore) data.total else newSessions.size,
|
||||||
|
hasMore = hasMore,
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
@@ -153,12 +156,15 @@ class SessionsViewModel :
|
|||||||
},
|
},
|
||||||
onStart = { _uiState.update { it.copy(isLoading = true, errorMessage = null) } },
|
onStart = { _uiState.update { it.copy(isLoading = true, errorMessage = null) } },
|
||||||
onSuccess = { data ->
|
onSuccess = { data ->
|
||||||
|
val sessionsList = data.sessions.orEmpty().pinnedFirst()
|
||||||
|
val hasMore = data.sessions.size >= PAGE_SIZE && data.total > sessionsList.size
|
||||||
_uiState.update {
|
_uiState.update {
|
||||||
it.copy(
|
it.copy(
|
||||||
isLoading = false,
|
isLoading = false,
|
||||||
isLoadingMore = false,
|
isLoadingMore = false,
|
||||||
sessions = data.sessions.orEmpty().pinnedFirst(),
|
sessions = sessionsList,
|
||||||
total = data.total,
|
total = if (hasMore) data.total else sessionsList.size,
|
||||||
|
hasMore = hasMore,
|
||||||
selectedIds = emptySet(),
|
selectedIds = emptySet(),
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
@@ -194,6 +200,13 @@ class SessionsViewModel :
|
|||||||
is NetworkResult.Success -> {
|
is NetworkResult.Success -> {
|
||||||
val data = result.data
|
val data = result.data
|
||||||
_uiState.update {
|
_uiState.update {
|
||||||
|
val newSessions =
|
||||||
|
(it.sessions + data.sessions)
|
||||||
|
.distinctBy { s -> s.id }
|
||||||
|
.pinnedFirst()
|
||||||
|
val receivedFullPage = data.sessions.size >= PAGE_SIZE
|
||||||
|
val addedNewItems = newSessions.size > it.sessions.size
|
||||||
|
val hasMore = receivedFullPage && addedNewItems && data.total > newSessions.size
|
||||||
it.copy(
|
it.copy(
|
||||||
isLoadingMore = false,
|
isLoadingMore = false,
|
||||||
// distinctBy guards offset-pagination churn: if a new
|
// distinctBy guards offset-pagination churn: if a new
|
||||||
@@ -201,11 +214,9 @@ class SessionsViewModel :
|
|||||||
// shift and the next page can repeat an id we already
|
// shift and the next page can repeat an id we already
|
||||||
// have — LazyColumn would crash on the duplicate key.
|
// have — LazyColumn would crash on the duplicate key.
|
||||||
// pinnedFirst keeps pinned sessions above the rest.
|
// pinnedFirst keeps pinned sessions above the rest.
|
||||||
sessions =
|
sessions = newSessions,
|
||||||
(it.sessions + data.sessions)
|
total = if (hasMore) data.total else newSessions.size,
|
||||||
.distinctBy { s -> s.id }
|
hasMore = hasMore,
|
||||||
.pinnedFirst(),
|
|
||||||
total = data.total,
|
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -470,11 +481,14 @@ class SessionsViewModel :
|
|||||||
when (result) {
|
when (result) {
|
||||||
is NetworkResult.Success -> {
|
is NetworkResult.Success -> {
|
||||||
_uiState.update {
|
_uiState.update {
|
||||||
|
val updatedSessions = it.sessions.filter { s -> s.id != sessionId }
|
||||||
|
val newTotal = (it.total - 1).coerceAtLeast(0)
|
||||||
it.copy(
|
it.copy(
|
||||||
deletingSessionIds = it.deletingSessionIds - sessionId,
|
deletingSessionIds = it.deletingSessionIds - sessionId,
|
||||||
sessions = it.sessions.filter { s -> s.id != sessionId },
|
sessions = updatedSessions,
|
||||||
searchResults = it.searchResults.filter { it.session_id != sessionId },
|
searchResults = it.searchResults.filter { it.session_id != sessionId },
|
||||||
total = (it.total - 1).coerceAtLeast(0),
|
total = newTotal,
|
||||||
|
hasMore = it.hasMore && newTotal > updatedSessions.size,
|
||||||
toastMessage = "Session deleted",
|
toastMessage = "Session deleted",
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -101,37 +101,60 @@ class SessionsViewModelTest {
|
|||||||
// ── Pagination (fluid load-more) ──────────────────────────────────────
|
// ── Pagination (fluid load-more) ──────────────────────────────────────
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
fun `loadMore appends the next page and dedupes overlapping ids`() {
|
fun `loadSessions with fewer than page size items caps total and sets hasMore false`() {
|
||||||
val vm = createViewModel()
|
val vm = createViewModel()
|
||||||
|
|
||||||
// Page 1: 2 sessions of 3 total — hasMore stays true.
|
// Server reports total = 10 (e.g. cross-profile count), but returns only 2 sessions (< PAGE_SIZE 50).
|
||||||
|
// hasMore must be false and total must be capped at 2 to prevent infinite auto-load loops.
|
||||||
coEvery { mockApi.getSessions(any(), any(), any()) } returns
|
coEvery { mockApi.getSessions(any(), any(), any()) } returns
|
||||||
Response.success(
|
Response.success(
|
||||||
SessionListResponse(
|
SessionListResponse(
|
||||||
sessions = listOf(SessionInfo("s-1"), SessionInfo("s-2")),
|
sessions = listOf(SessionInfo("s-1"), SessionInfo("s-2")),
|
||||||
total = 3,
|
total = 10,
|
||||||
),
|
),
|
||||||
)
|
)
|
||||||
vm.loadSessions()
|
vm.loadSessions()
|
||||||
testDispatcher.scheduler.advanceUntilIdle()
|
testDispatcher.scheduler.advanceUntilIdle()
|
||||||
|
|
||||||
assertEquals(listOf("s-1", "s-2"), vm.uiState.value.sessions.map { it.id })
|
assertEquals(listOf("s-1", "s-2"), vm.uiState.value.sessions.map { it.id })
|
||||||
|
assertEquals(2, vm.uiState.value.total)
|
||||||
|
assertFalse(vm.uiState.value.hasMore)
|
||||||
|
assertFalse(vm.uiState.value.isLoadingMore)
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `loadMore appends the next page and dedupes overlapping ids`() {
|
||||||
|
val vm = createViewModel()
|
||||||
|
|
||||||
|
val page1Sessions = (1..50).map { SessionInfo("s-$it") }
|
||||||
|
// Page 1: 50 sessions of 51 total — hasMore stays true.
|
||||||
|
coEvery { mockApi.getSessions(limit = 50, offset = 0, order = any()) } returns
|
||||||
|
Response.success(
|
||||||
|
SessionListResponse(
|
||||||
|
sessions = page1Sessions,
|
||||||
|
total = 51,
|
||||||
|
),
|
||||||
|
)
|
||||||
|
vm.loadSessions()
|
||||||
|
testDispatcher.scheduler.advanceUntilIdle()
|
||||||
|
assertEquals(50, vm.uiState.value.sessions.size)
|
||||||
assertTrue(vm.uiState.value.hasMore)
|
assertTrue(vm.uiState.value.hasMore)
|
||||||
assertFalse(vm.uiState.value.isLoadingMore)
|
assertFalse(vm.uiState.value.isLoadingMore)
|
||||||
|
|
||||||
// Page 2 overlaps page 1 (offset churn: a new session landed on top
|
// Page 2 overlaps page 1 (offset churn: a new session landed on top
|
||||||
// between loads) — the duplicate id must not double-append.
|
// between loads) — the duplicate id must not double-append.
|
||||||
coEvery { mockApi.getSessions(any(), any(), any()) } returns
|
coEvery { mockApi.getSessions(limit = 50, offset = 50, order = any()) } returns
|
||||||
Response.success(
|
Response.success(
|
||||||
SessionListResponse(
|
SessionListResponse(
|
||||||
sessions = listOf(SessionInfo("s-3"), SessionInfo("s-2")),
|
sessions = listOf(SessionInfo("s-51"), SessionInfo("s-50")),
|
||||||
total = 3,
|
total = 51,
|
||||||
),
|
),
|
||||||
)
|
)
|
||||||
vm.loadMore()
|
vm.loadMore()
|
||||||
testDispatcher.scheduler.advanceUntilIdle()
|
testDispatcher.scheduler.advanceUntilIdle()
|
||||||
|
|
||||||
assertEquals(listOf("s-1", "s-2", "s-3"), vm.uiState.value.sessions.map { it.id })
|
assertEquals(51, vm.uiState.value.sessions.size)
|
||||||
assertEquals(3, vm.uiState.value.total)
|
assertEquals(51, vm.uiState.value.total)
|
||||||
assertFalse(vm.uiState.value.hasMore)
|
assertFalse(vm.uiState.value.hasMore)
|
||||||
assertFalse(vm.uiState.value.isLoadingMore)
|
assertFalse(vm.uiState.value.isLoadingMore)
|
||||||
}
|
}
|
||||||
@@ -140,11 +163,12 @@ class SessionsViewModelTest {
|
|||||||
fun `loadMore is a no-op while a load is already running`() {
|
fun `loadMore is a no-op while a load is already running`() {
|
||||||
val vm = createViewModel()
|
val vm = createViewModel()
|
||||||
|
|
||||||
coEvery { mockApi.getSessions(any(), any(), any()) } returns
|
val page1Sessions = (1..50).map { SessionInfo("s-$it") }
|
||||||
|
coEvery { mockApi.getSessions(limit = 50, offset = 0, order = any()) } returns
|
||||||
Response.success(
|
Response.success(
|
||||||
SessionListResponse(
|
SessionListResponse(
|
||||||
sessions = listOf(SessionInfo("s-1")),
|
sessions = page1Sessions,
|
||||||
total = 2,
|
total = 52,
|
||||||
),
|
),
|
||||||
)
|
)
|
||||||
vm.loadSessions()
|
vm.loadSessions()
|
||||||
@@ -158,7 +182,6 @@ class SessionsViewModelTest {
|
|||||||
|
|
||||||
// loadSessions (1) + exactly one loadMore (1) = 2 API hits total.
|
// loadSessions (1) + exactly one loadMore (1) = 2 API hits total.
|
||||||
coVerify(exactly = 2) { mockApi.getSessions(any(), any(), any()) }
|
coVerify(exactly = 2) { mockApi.getSessions(any(), any(), any()) }
|
||||||
assertEquals(listOf("s-1"), vm.uiState.value.sessions.map { it.id })
|
|
||||||
assertFalse(vm.uiState.value.isLoadingMore)
|
assertFalse(vm.uiState.value.isLoadingMore)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in new issue
Block a user