Stabilize watchlist paging during sync updates#129
Open
anod wants to merge 1 commit into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the app-list SQL used by watchlist-style paging to ensure each app_list row joins to at most one changelog row per (app_id, version_code), preventing duplicate emitted rows that can trigger Compose Lazy key collisions when legacy/invalid changelog duplicates exist.
Changes:
- Update
AppListTable.Queries.createAppsListQuery()to join via a derived “latest changelog row per (app_id, code)” subquery, then join back tochangelogby_id. - Add a regression test that simulates legacy duplicate changelog rows (by dropping the unique index) and asserts the paging result does not duplicate apps.
- Update query-shape assertions in
AppListTableQueriesTestto reflect the new join structure.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| app/src/main/java/com/anod/appwatcher/database/AppListTable.kt | Changes the dynamic app-list query to join changelog through a “latest per version” derived table to prevent duplicated app rows. |
| app/src/test/java/com/anod/appwatcher/database/AppListTableQueriesTest.kt | Updates assertions to validate the new derived-table join shape. |
| app/src/test/java/com/anod/appwatcher/watchlist/WatchListPagingSourceRoomTest.kt | Adds a regression test covering duplicate changelog versions and a helper to insert legacy-duplicate data. |
b5facaf to
a47be1e
Compare
a47be1e to
f62c7ba
Compare
f62c7ba to
671138a
Compare
Capture a stable app row-id snapshot per WatchListPagingSource generation and page by those row IDs so status changes during sync cannot move the same app across offset windows and emit duplicate Lazy keys. Add regression coverage for rows moving across paging offsets while keeping recently-discovered row snapshot ordering valid. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
671138a to
de06f77
Compare
Comment on lines
56
to
+70
| val sortId = prefs.sortIndex | ||
| var limit = initialLimit | ||
| val items = mutableListOf<SectionItem>() | ||
| if (offset == 0 && config.showRecentlyInstalled) { | ||
| items.add(SectionItem.Recent) | ||
| // limit is already reduced in calculateOffsetAndLimit, but keep max guard | ||
| limit = max(0, limit) | ||
| } | ||
|
|
||
| val data = AppListTable.Queries.loadAppList( | ||
| sortId, config.showRecentlyDiscovered, config.tagId, filterQuery, SqlOffset(offset, limit), database.apps() | ||
| ) | ||
| val rowIds = loadRowIdSnapshot(sortId) | ||
| val pageRowIds = if (offset >= rowIds.size) { | ||
| emptyList() | ||
| } else { | ||
| rowIds.subList(offset, minOf(rowIds.size, offset + limit)) | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Crash investigation
Crashlytics logs for the sample event showed manual sync running while the watchlist was paging. During sync, apps including Chrome were marked as viewed, changing app status and reordering the STATUS DESC list between an initial 60-item load and later appends. That can emit the same app row in multiple pages and trigger Compose duplicate Lazy keys.
Validation